[PATCH] Follow OpenSSL's switch from AES128 to AES256 for session tickets

Christian Klinger c.klinger at lirum.at
Sat Nov 5 23:24:08 UTC 2016


# HG changeset patch
# User Christian Klinger <c.klinger at lirum.at>
# Date 1478383992 -3600
# Node ID 5719a734584d23a6bcd22a3e59dd36138d06b803
# Parent  92ad1c92bcf93310bf59447dd581cac37af87adb
Follow OpenSSL's switch from AES128 to AES256 for 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 5719a734584d 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	Sat Nov 05 23:13:12 2016 +0100
@@ -2832,8 +2832,10 @@
 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;
+    ssize_t                        name_len;
+    ssize_t                        key_len;
     ngx_str_t                     *path;
     ngx_file_t                     file;
     ngx_uint_t                     i;
@@ -2875,13 +2877,16 @@
             goto failed;
         }
 
-        if (ngx_file_size(&fi) != 48) {
+        // We expect 80 bytes, but we allow 48 bytes to not break legacy setups
+        if (ngx_file_size(&fi) != 80 && ngx_file_size(&fi) != 48) {
+            // We only mention 80 bytes in the log message, as that is what we
+            // want to gravitate towards.
             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,21 +2894,47 @@
             goto failed;
         }
 
-        if (n != 48) {
-            ngx_conf_log_error(NGX_LOG_CRIT, cf, 0,
-                               ngx_read_file_n " \"%V\" returned only "
-                               "%z bytes instead of 48", &file.name, n);
-            goto failed;
+        if (n != 80) {
+            if (n == 48) {
+                // 48 was the old ngx_ssl_session_ticket_key_t length. 48 bytes
+                // do not allow us to fully seed recent algos. We'd need
+                // 80 bytes to fully seed them. But to avoid breaking legacy
+                // setups for which 48 byte seeding was sufficient, we use those
+                // 48 bytes. 48 bytes on recent algos is not worse than 48 bytes
+                // on old algos. But although we're not aborting, we at least
+                // warn users that they should provide more randomness.
+                ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
+                                   ngx_read_file_n " \"%V\" returned only "
+                                   "%z bytes instead of 80. Please provide "
+                                   "80 bytes to fully seed used algorithms",
+                                   &file.name, n);
+            } else {
+                // Neither the new default 80 bytes, nor the legacy 48 bytes
+                // of randomness.
+                ngx_conf_log_error(NGX_LOG_CRIT, cf, 0,
+                                   ngx_read_file_n " \"%V\" returned only "
+                                   "%z bytes instead of 80", &file.name, n);
+                goto failed;
+            }
         }
 
         key = ngx_array_push(keys);
         if (key == NULL) {
             goto failed;
         }
-
-        ngx_memcpy(key->name, buf, 16);
-        ngx_memcpy(key->aes_key, buf + 16, 16);
-        ngx_memcpy(key->hmac_key, buf + 32, 16);
+        // Since the read in randomness need not fill the whole key, we zero
+        // out the key before to make sure keys agree if the randomness file
+        // is re-used across servers.
+        ngx_memzero(key, sizeof(ngx_ssl_session_ticket_key_t));
+        name_len = sizeof(key->name);
+        // When the random file contained only 48 bytes of randomnes, we rather
+        // split the available randomness between aes and hmac, instead of only
+        // seeding one of the two. key_len holds how many bytes of the
+        // randomness flows into seeding each of aes and hmac.
+        key_len = (n - name_len) / 2;
+        ngx_memcpy(key->name, buf, name_len);
+        ngx_memcpy(key->aes_key, buf + name_len, key_len);
+        ngx_memcpy(key->hmac_key, buf + name_len + key_len, key_len);
 
         if (ngx_close_file(file.fd) == NGX_FILE_ERROR) {
             ngx_log_error(NGX_LOG_ALERT, cf->log, ngx_errno,
@@ -2962,7 +2993,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 +3027,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) {
             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 +3064,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 5719a734584d 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	Sat Nov 05 23:13:12 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



More information about the nginx-devel mailing list