Skip to content

Commit 59662a8

Browse files
committed
Fix use after free bug that could cause crash when session keys are rotated
Motivation: Due a bug in our code we could run into a use after free bug when we rotated the session keys while the session key lookup is in process. Modifications: - Don't store a reference to a shared pointer Result: No more risk of crash
1 parent 3f1724d commit 59662a8

2 files changed

Lines changed: 15 additions & 10 deletions

File tree

‎openssl-dynamic/src/main/c/ssl_private.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
#include <openssl/sha.h>
6969
#if OPENSSL_VERSION_NUMBER >= 0x30000000
7070
#include <openssl/core_names.h>
71+
#include <openssl/params.h>
7172
#endif
7273

7374
#define ERR_LEN 256
@@ -295,9 +296,6 @@ typedef struct tcn_ssl_ctxt_t tcn_ssl_ctxt_t;
295296
typedef struct {
296297
unsigned char key_name[SSL_SESSION_TICKET_KEY_NAME_LEN];
297298
unsigned char hmac_key[SSL_SESSION_TICKET_HMAC_KEY_LEN];
298-
#if OPENSSL_VERSION_NUMBER >= 0x30000000L
299-
OSSL_PARAM mac_params[3];
300-
#endif
301299
unsigned char aes_key[SSL_SESSION_TICKET_AES_KEY_LEN];
302300
} tcn_ssl_ticket_key_t;
303301

‎openssl-dynamic/src/main/c/sslcontext.c‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1332,6 +1332,14 @@ static int find_session_key(tcn_ssl_ctxt_t *c, unsigned char key_name[16], tcn_s
13321332
return result;
13331333
}
13341334

1335+
#if OPENSSL_VERSION_NUMBER >= 0x30000000L
1336+
static void fill_mac_params(OSSL_PARAM* params, unsigned char* hmac_key, int hmac_key_length) {
1337+
params[0] = OSSL_PARAM_construct_octet_string(OSSL_MAC_PARAM_KEY, hmac_key, hmac_key_length);
1338+
params[1] = OSSL_PARAM_construct_utf8_string(OSSL_MAC_PARAM_DIGEST, "sha256", 0);
1339+
params[2] = OSSL_PARAM_construct_end();
1340+
}
1341+
#endif
1342+
13351343
static int ssl_tlsext_ticket_key_cb(SSL *s,
13361344
unsigned char key_name[16],
13371345
unsigned char *iv,
@@ -1362,7 +1370,9 @@ static int ssl_tlsext_ticket_key_cb(SSL *s,
13621370
#if OPENSSL_VERSION_NUMBER < 0x30000000L
13631371
HMAC_Init_ex(hmac_ctx, key.hmac_key, 16, EVP_sha256(), NULL);
13641372
#else
1365-
EVP_MAC_CTX_set_params(mac_ctx, key.mac_params);
1373+
OSSL_PARAM local_mac_params[3];
1374+
fill_mac_params(local_mac_params, key.hmac_key, SSL_SESSION_TICKET_HMAC_KEY_LEN);
1375+
EVP_MAC_CTX_set_params(mac_ctx, local_mac_params);
13661376
#endif
13671377
apr_atomic_inc32(&c->ticket_keys_new);
13681378
return 1;
@@ -1374,7 +1384,9 @@ static int ssl_tlsext_ticket_key_cb(SSL *s,
13741384
#if OPENSSL_VERSION_NUMBER < 0x30000000L
13751385
HMAC_Init_ex(hmac_ctx, key.hmac_key, 16, EVP_sha256(), NULL);
13761386
#else
1377-
EVP_MAC_CTX_set_params(mac_ctx, key.mac_params);
1387+
OSSL_PARAM local_mac_params[3];
1388+
fill_mac_params(local_mac_params, key.hmac_key, SSL_SESSION_TICKET_HMAC_KEY_LEN);
1389+
EVP_MAC_CTX_set_params(mac_ctx, local_mac_params);
13781390
#endif
13791391
EVP_DecryptInit_ex(ctx, EVP_aes_128_cbc(), NULL, key.aes_key, iv );
13801392
if (!is_current_key) {
@@ -1421,11 +1433,6 @@ TCN_IMPLEMENT_CALL(void, SSLContext, setSessionTicketKeys0)(TCN_STDARGS, jlong c
14211433
key = b + (SSL_SESSION_TICKET_KEY_SIZE * i);
14221434
memcpy(ticket_keys[i].key_name, key, 16);
14231435
memcpy(ticket_keys[i].hmac_key, key + 16, 16);
1424-
#if OPENSSL_VERSION_NUMBER >= 0x30000000L
1425-
ticket_keys[i].mac_params[0] = OSSL_PARAM_construct_octet_string(OSSL_MAC_PARAM_KEY, ticket_keys[i].hmac_key, 16);
1426-
ticket_keys[i].mac_params[1] = OSSL_PARAM_construct_utf8_string(OSSL_MAC_PARAM_DIGEST, "sha256", 0);
1427-
ticket_keys[i].mac_params[2] = OSSL_PARAM_construct_end();
1428-
#endif
14291436
memcpy(ticket_keys[i].aes_key, key + 32, 16);
14301437
}
14311438
(*e)->ReleaseByteArrayElements(e, keys, b, 0);

0 commit comments

Comments
 (0)