Hello!

On Mon, Nov 07, 2016 at 12:09:40AM +0100, Christian Klinger via nginx-devel wrote:

> # HG changeset patch
> # User Christian Klinger <c.klinger@lirum.at>
> # Date 1478473338 -3600
> # Node ID 36f66e94771dd39e8948ba1023e5ca0677655840
> # Parent 92ad1c92bcf93310bf59447dd581cac37af87adb
> 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 36f66e94771d 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 Mon Nov 07 00:02:18 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;
> }

I don't think that breaking compatibility with previous
configurations is a good idea, even for advanced features. And in
this particular case it is trivial to preserve one.

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1479131639 -10800
# Mon Nov 14 16:53:59 2016 +0300
# Node ID 4e57f6096c0c9c64e8bae0bdc09d892b34db8845
# Parent b5f9e76d4f297a3885d696e8f3af0d84b6a4aaca
SSL: support AES256 encryption of tickets.

This implies changing ticket key size from 48 to 80 bytes, as both
HMAC and AES keys are 32 bytes now. Previous format is still supported
though emits a warning.

OpenSSL switched to using AES256 in 1.1.0, and so do we. While here,
order of HMAC and AES keys reverted to make the implementation compatible
with keys used by OpenSSL with SSL_CTX_set_tlsext_ticket_keys().

Prodded by Christian Klinger.

diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -2832,7 +2832,8 @@ ngx_ssl_session_rbtree_insert_value(ngx_
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];
+ size_t size;
ssize_t n;
ngx_str_t *path;
ngx_file_t file;
@@ -2875,13 +2876,21 @@ ngx_ssl_session_ticket_keys(ngx_conf_t *
goto failed;
}

- if (ngx_file_size(&fi) != 48) {
+ size = ngx_file_size(&fi);
+
+ if (size == 48) {
+ ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
+ "\"%V\" uses deprecated 48 bytes format, "
+ "use 80 bytes instead",
+ &file.name);
+
+ } else if (size != 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, size, 0);

if (n == NGX_ERROR) {
ngx_conf_log_error(NGX_LOG_CRIT, cf, ngx_errno,
@@ -2889,10 +2898,10 @@ ngx_ssl_session_ticket_keys(ngx_conf_t *
goto failed;
}

- if (n != 48) {
+ if ((size_t) n != size) {
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 %uz", &file.name, n, size);
goto failed;
}

@@ -2901,9 +2910,18 @@ ngx_ssl_session_ticket_keys(ngx_conf_t *
goto failed;
}

- ngx_memcpy(key->name, buf, 16);
- ngx_memcpy(key->aes_key, buf + 16, 16);
- ngx_memcpy(key->hmac_key, buf + 32, 16);
+ if (size == 48) {
+ key->size = 48;
+ ngx_memcpy(key->name, buf, 16);
+ ngx_memcpy(key->aes_key, buf + 16, 16);
+ ngx_memcpy(key->hmac_key, buf + 32, 16);
+
+ } else {
+ key->size = 80;
+ ngx_memcpy(key->name, buf, 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,
@@ -2948,6 +2966,7 @@ ngx_ssl_session_ticket_key_callback(ngx_
unsigned char *name, unsigned char *iv, EVP_CIPHER_CTX *ectx,
HMAC_CTX *hctx, int enc)
{
+ size_t size;
SSL_CTX *ssl_ctx;
ngx_uint_t i;
ngx_array_t *keys;
@@ -2962,7 +2981,6 @@ ngx_ssl_session_ticket_key_callback(ngx_
c = ngx_ssl_get_connection(ssl_conn);
ssl_ctx = c->ssl->session_ctx;

- cipher = EVP_aes_128_cbc();
#ifdef OPENSSL_NO_SHA256
digest = EVP_sha1();
#else
@@ -2984,6 +3002,15 @@ ngx_ssl_session_ticket_key_callback(ngx_
ngx_hex_dump(buf, key[0].name, 16) - buf, buf,
SSL_session_reused(ssl_conn) ? "reused" : "new");

+ if (key[0].size == 48) {
+ cipher = EVP_aes_128_cbc();
+ size = 16;
+
+ } else {
+ cipher = EVP_aes_256_cbc();
+ size = 32;
+ }
+
if (RAND_bytes(iv, EVP_CIPHER_iv_length(cipher)) != 1) {
ngx_ssl_error(NGX_LOG_ALERT, c->log, 0, "RAND_bytes() failed");
return -1;
@@ -2996,12 +3023,12 @@ ngx_ssl_session_ticket_key_callback(ngx_
}

#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, size, 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, size, digest, NULL);
#endif

ngx_memcpy(name, key[0].name, 16);
@@ -3030,13 +3057,22 @@ ngx_ssl_session_ticket_key_callback(ngx_
ngx_hex_dump(buf, key[i].name, 16) - buf, buf,
(i == 0) ? " (default)" : "");

+ if (key[0].size == 48) {
+ cipher = EVP_aes_128_cbc();
+ size = 16;
+
+ } else {
+ cipher = EVP_aes_256_cbc();
+ size = 32;
+ }
+
#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, size, 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, size, digest, NULL);
#endif

if (EVP_DecryptInit_ex(ectx, cipher, NULL, key[i].aes_key, iv) != 1) {
diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
--- a/src/event/ngx_event_openssl.h
+++ b/src/event/ngx_event_openssl.h
@@ -117,9 +117,10 @@ typedef struct {
#ifdef SSL_CTRL_SET_TLSEXT_TICKET_KEY_CB

typedef struct {
+ size_t size;
u_char name[16];
- u_char aes_key[16];
- u_char hmac_key[16];
+ u_char hmac_key[32];
+ u_char aes_key[32];
} ngx_ssl_session_ticket_key_t;

#endif

--
Maxim Dounin
http://nginx.org/

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel