From 4bfb96f2ad01d71836cfccceb7b15102f0f59055 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Mon, 19 Mar 2018 10:50:51 -0400 Subject: [PATCH] Place ticket keys into secure memory Place the session ticket AES and HMAC keys into secure memory. Reviewed-by: Richard Levitte Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/2351) --- ssl/s3_lib.c | 24 ++++++++++++------------ ssl/ssl_lib.c | 12 ++++++++---- ssl/ssl_locl.h | 11 ++++++++--- ssl/statem/statem_srvr.c | 6 +++--- ssl/t1_lib.c | 6 +++--- 5 files changed, 34 insertions(+), 25 deletions(-) diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index bbf49a205d..619326949c 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3798,8 +3798,8 @@ long ssl3_ctx_ctrl(SSL_CTX *ctx, int cmd, long larg, void *parg) { unsigned char *keys = parg; long tick_keylen = (sizeof(ctx->ext.tick_key_name) + - sizeof(ctx->ext.tick_hmac_key) + - sizeof(ctx->ext.tick_aes_key)); + sizeof(ctx->ext.secure->tick_hmac_key) + + sizeof(ctx->ext.secure->tick_aes_key)); if (keys == NULL) return tick_keylen; if (larg != tick_keylen) { @@ -3809,23 +3809,23 @@ long ssl3_ctx_ctrl(SSL_CTX *ctx, int cmd, long larg, void *parg) if (cmd == SSL_CTRL_SET_TLSEXT_TICKET_KEYS) { memcpy(ctx->ext.tick_key_name, keys, sizeof(ctx->ext.tick_key_name)); - memcpy(ctx->ext.tick_hmac_key, + memcpy(ctx->ext.secure->tick_hmac_key, keys + sizeof(ctx->ext.tick_key_name), - sizeof(ctx->ext.tick_hmac_key)); - memcpy(ctx->ext.tick_aes_key, + sizeof(ctx->ext.secure->tick_hmac_key)); + memcpy(ctx->ext.secure->tick_aes_key, keys + sizeof(ctx->ext.tick_key_name) + - sizeof(ctx->ext.tick_hmac_key), - sizeof(ctx->ext.tick_aes_key)); + sizeof(ctx->ext.secure->tick_hmac_key), + sizeof(ctx->ext.secure->tick_aes_key)); } else { memcpy(keys, ctx->ext.tick_key_name, sizeof(ctx->ext.tick_key_name)); memcpy(keys + sizeof(ctx->ext.tick_key_name), - ctx->ext.tick_hmac_key, - sizeof(ctx->ext.tick_hmac_key)); + ctx->ext.secure->tick_hmac_key, + sizeof(ctx->ext.secure->tick_hmac_key)); memcpy(keys + sizeof(ctx->ext.tick_key_name) + - sizeof(ctx->ext.tick_hmac_key), - ctx->ext.tick_aes_key, - sizeof(ctx->ext.tick_aes_key)); + sizeof(ctx->ext.secure->tick_hmac_key), + ctx->ext.secure->tick_aes_key, + sizeof(ctx->ext.secure->tick_aes_key)); } return 1; } diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index baf8a94aa6..062f5cef3f 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -3035,6 +3035,9 @@ SSL_CTX *SSL_CTX_new(const SSL_METHOD *meth) if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_SSL_CTX, ret, &ret->ex_data)) goto err; + if ((ret->ext.secure = OPENSSL_secure_zalloc(sizeof(*ret->ext.secure))) == NULL) + goto err; + /* No compression for DTLS */ if (!(meth->ssl3_enc->enc_flags & SSL_ENC_FLAG_DTLS)) ret->comp_methods = SSL_COMP_get_compression_methods(); @@ -3045,10 +3048,10 @@ SSL_CTX *SSL_CTX_new(const SSL_METHOD *meth) /* Setup RFC5077 ticket keys */ if ((RAND_bytes(ret->ext.tick_key_name, sizeof(ret->ext.tick_key_name)) <= 0) - || (RAND_bytes(ret->ext.tick_hmac_key, - sizeof(ret->ext.tick_hmac_key)) <= 0) - || (RAND_bytes(ret->ext.tick_aes_key, - sizeof(ret->ext.tick_aes_key)) <= 0)) + || (RAND_bytes(ret->ext.secure->tick_hmac_key, + sizeof(ret->ext.secure->tick_hmac_key)) <= 0) + || (RAND_bytes(ret->ext.secure->tick_aes_key, + sizeof(ret->ext.secure->tick_aes_key)) <= 0)) ret->options |= SSL_OP_NO_TICKET; if (RAND_bytes(ret->ext.cookie_hmac_key, @@ -3190,6 +3193,7 @@ void SSL_CTX_free(SSL_CTX *a) OPENSSL_free(a->ext.supportedgroups); #endif OPENSSL_free(a->ext.alpn); + OPENSSL_secure_free(a->ext.secure); CRYPTO_THREAD_lock_free(a->lock); diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 9d4e0f17a7..a28facdcc6 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -733,7 +733,13 @@ DEFINE_LHASH_OF(SSL_SESSION); /* Needed in ssl_cert.c */ DEFINE_LHASH_OF(X509_NAME); -# define TLSEXT_KEYNAME_LENGTH 16 +# define TLSEXT_KEYNAME_LENGTH 16 +# define TLSEXT_TICK_KEY_LENGTH 32 + +typedef struct ssl_ctx_ext_secure_st { + unsigned char tick_hmac_key[TLSEXT_TICK_KEY_LENGTH]; + unsigned char tick_aes_key[TLSEXT_TICK_KEY_LENGTH]; +} SSL_CTX_EXT_SECURE; struct ssl_ctx_st { const SSL_METHOD *method; @@ -927,8 +933,7 @@ struct ssl_ctx_st { void *servername_arg; /* RFC 4507 session ticket keys */ unsigned char tick_key_name[TLSEXT_KEYNAME_LENGTH]; - unsigned char tick_hmac_key[32]; - unsigned char tick_aes_key[32]; + SSL_CTX_EXT_SECURE *secure; /* Callback to support customisation of ticket key setting */ int (*ticket_key_cb) (SSL *ssl, unsigned char *name, unsigned char *iv, diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 8826b7f00f..4985cdc702 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -3831,9 +3831,9 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt) iv_len = EVP_CIPHER_iv_length(cipher); if (RAND_bytes(iv, iv_len) <= 0 || !EVP_EncryptInit_ex(ctx, cipher, NULL, - tctx->ext.tick_aes_key, iv) - || !HMAC_Init_ex(hctx, tctx->ext.tick_hmac_key, - sizeof(tctx->ext.tick_hmac_key), + tctx->ext.secure->tick_aes_key, iv) + || !HMAC_Init_ex(hctx, tctx->ext.secure->tick_hmac_key, + sizeof(tctx->ext.secure->tick_hmac_key), EVP_sha256(), NULL)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_NEW_SESSION_TICKET, diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 796e9d4827..174d7de3ce 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1349,11 +1349,11 @@ SSL_TICKET_RETURN tls_decrypt_ticket(SSL *s, const unsigned char *etick, ret = SSL_TICKET_NO_DECRYPT; goto err; } - if (HMAC_Init_ex(hctx, tctx->ext.tick_hmac_key, - sizeof(tctx->ext.tick_hmac_key), + if (HMAC_Init_ex(hctx, tctx->ext.secure->tick_hmac_key, + sizeof(tctx->ext.secure->tick_hmac_key), EVP_sha256(), NULL) <= 0 || EVP_DecryptInit_ex(ctx, EVP_aes_256_cbc(), NULL, - tctx->ext.tick_aes_key, + tctx->ext.secure->tick_aes_key, etick + TLSEXT_KEYNAME_LENGTH) <= 0) { goto err; } -- 2.25.1