[PATCH] Follow OpenSSL's switch from AES128 to AES256 for session tickets
Piotr Sikora
piotrsikora at google.com
Sun Nov 6 22:31:49 UTC 2016
Hey Christian,
> # HG changeset patch
> # User Christian Klinger <c.klinger at lirum.at>
> # Date 1478468739 -3600
> # Node ID 9cfbbce1ec24a31c29ea2f20cb21e32e5173bc60
> # Parent 92ad1c92bcf93310bf59447dd581cac37af87adb
> Follow OpenSSL's switch from AES128 to AES256 for session tickets
This should be:
SSL: switch from AES128 to AES256 for TLS Session Tickets.
>
> OpenSSL switched from AES128 to AES256 for de-/encryption of session
> tickets in May 2016 (commit 05df5c2036f1244fe3df70de7d8079a5d86b999d),
> while we still used AES128, if `ssl_session_ticket_key` was set.
>
> Using AES128 for session tickets means that even when using an AES256
> cipher suite, the session's master key (and hence the whole
> connection) is effectively only protected by AES128, which silently
> weakens encryption.
>
> Hence, we follow OpenSSL's lead and switch from AES128 to AES256 for
> session tickets, if `ssl_session_ticket_key` is set.
>
> While the switch from AES128 to AES256 warrants reading additional
> 16 bytes from the key file, we nonetheless increase by 32 bytes,
> reading a total of 80 bytes instead of previously 48 bytes.
> The additional 16 bytes flow into the key for the SHA256 HMAC.
> Previously, the SHA256 HMAC only used a 16 byte key, and thereby used
> only half of SHA256 HMAC's 32 byte key space. We now provide a 32 byte
> key to the SHA256 HMAC and finally fully use the available key space.
>
> diff -r 92ad1c92bcf9 -r 9cfbbce1ec24 src/event/ngx_event_openssl.c
> --- a/src/event/ngx_event_openssl.c Fri Nov 04 19:12:19 2016 +0300
> +++ b/src/event/ngx_event_openssl.c Sun Nov 06 22:45:39 2016 +0100
> @@ -2832,7 +2832,7 @@
> ngx_int_t
> ngx_ssl_session_ticket_keys(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_array_t *paths)
> {
> - u_char buf[48];
> + u_char buf[80];
> ssize_t n;
> ngx_str_t *path;
> ngx_file_t file;
> @@ -2875,13 +2875,13 @@
> goto failed;
> }
>
> - if (ngx_file_size(&fi) != 48) {
> + if (ngx_file_size(&fi) != 80) {
> ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
> - "\"%V\" must be 48 bytes", &file.name);
> + "\"%V\" must be 80 bytes", &file.name);
> goto failed;
> }
>
> - n = ngx_read_file(&file, buf, 48, 0);
> + n = ngx_read_file(&file, buf, 80, 0);
>
> if (n == NGX_ERROR) {
> ngx_conf_log_error(NGX_LOG_CRIT, cf, ngx_errno,
> @@ -2889,10 +2889,10 @@
> goto failed;
> }
>
> - if (n != 48) {
> + if (n != 80) {
> ngx_conf_log_error(NGX_LOG_CRIT, cf, 0,
> ngx_read_file_n " \"%V\" returned only "
> - "%z bytes instead of 48", &file.name, n);
> + "%z bytes instead of 80", &file.name, n);
> goto failed;
> }
>
> @@ -2900,10 +2900,9 @@
> if (key == NULL) {
> goto failed;
> }
> -
Style: leave empty line.
> ngx_memcpy(key->name, buf, 16);
> - ngx_memcpy(key->aes_key, buf + 16, 16);
> - ngx_memcpy(key->hmac_key, buf + 32, 16);
> + ngx_memcpy(key->hmac_key, buf + 16, 32);
> + ngx_memcpy(key->aes_key, buf + 48, 32);
>
> if (ngx_close_file(file.fd) == NGX_FILE_ERROR) {
> ngx_log_error(NGX_LOG_ALERT, cf->log, ngx_errno,
> @@ -2962,7 +2961,7 @@
> c = ngx_ssl_get_connection(ssl_conn);
> ssl_ctx = c->ssl->session_ctx;
>
> - cipher = EVP_aes_128_cbc();
> + cipher = EVP_aes_256_cbc();
> #ifdef OPENSSL_NO_SHA256
> digest = EVP_sha1();
> #else
> @@ -2996,12 +2995,14 @@
> }
>
> #if OPENSSL_VERSION_NUMBER >= 0x10000000L
> - if (HMAC_Init_ex(hctx, key[0].hmac_key, 16, digest, NULL) != 1) {
> + if (HMAC_Init_ex(hctx, key[0].hmac_key, sizeof(key[0].hmac_key),
> + digest, NULL) != 1) {
Style, this should be:
if (HMAC_Init_ex(hctx, key[0].hmac_key, sizeof(key[0].hmac_key),
digest, NULL)
!= 1)
{
which is why I'm not sure if the use of sizeof() instead of hardcoded
"32" is a good choice, since it changes 1 line to 4 lines (here and in
other places).
> ngx_ssl_error(NGX_LOG_ALERT, c->log, 0, "HMAC_Init_ex() failed");
> return -1;
> }
> #else
> - HMAC_Init_ex(hctx, key[0].hmac_key, 16, digest, NULL);
> + HMAC_Init_ex(hctx, key[0].hmac_key, sizeof(key[0].hmac_key), digest,
> + NULL);
> #endif
>
> ngx_memcpy(name, key[0].name, 16);
> @@ -3031,12 +3032,14 @@
> (i == 0) ? " (default)" : "");
>
> #if OPENSSL_VERSION_NUMBER >= 0x10000000L
> - if (HMAC_Init_ex(hctx, key[i].hmac_key, 16, digest, NULL) != 1) {
> + if (HMAC_Init_ex(hctx, key[i].hmac_key, sizeof(key[i].hmac_key),
> + digest, NULL) != 1) {
> ngx_ssl_error(NGX_LOG_ALERT, c->log, 0, "HMAC_Init_ex() failed");
> return -1;
> }
> #else
> - HMAC_Init_ex(hctx, key[i].hmac_key, 16, digest, NULL);
> + HMAC_Init_ex(hctx, key[i].hmac_key, sizeof(key[i].hmac_key), digest,
> + NULL);
> #endif
>
> if (EVP_DecryptInit_ex(ectx, cipher, NULL, key[i].aes_key, iv) != 1) {
> diff -r 92ad1c92bcf9 -r 9cfbbce1ec24 src/event/ngx_event_openssl.h
> --- a/src/event/ngx_event_openssl.h Fri Nov 04 19:12:19 2016 +0300
> +++ b/src/event/ngx_event_openssl.h Sun Nov 06 22:45:39 2016 +0100
> @@ -118,8 +118,8 @@
>
> typedef struct {
> u_char name[16];
> - u_char aes_key[16];
> - u_char hmac_key[16];
> + u_char aes_key[32];
> + u_char hmac_key[32];
> } ngx_ssl_session_ticket_key_t;
>
> #endif
We might change the order here as well (name, hmac_key, aes_key).
Best regards,
Piotr Sikora
More information about the nginx-devel
mailing list