From ec15acb6bc554b8f87a519c3519f5bf4d367ded9 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 13 Jan 2017 17:00:49 +0000 Subject: [PATCH] Construct the client side psk extension for TLSv1.3 Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/2259) --- include/openssl/ssl.h | 4 +- include/openssl/tls1.h | 1 + ssl/s3_lib.c | 32 ++++--- ssl/ssl_err.c | 1 + ssl/ssl_locl.h | 31 +++++-- ssl/statem/extensions.c | 10 ++- ssl/statem/extensions_clnt.c | 164 +++++++++++++++++++++++++++++++++++ ssl/statem/statem_lib.c | 2 +- ssl/statem/statem_locl.h | 2 + ssl/statem/statem_srvr.c | 2 +- ssl/tls13_enc.c | 113 ++++++++++++------------ test/tls13secretstest.c | 10 ++- 12 files changed, 290 insertions(+), 82 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 708d59b14c..e528689c70 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -76,7 +76,7 @@ extern "C" { # define SSL_MIN_RSA_MODULUS_LENGTH_IN_BYTES (512/8) # define SSL_MAX_KEY_ARG_LENGTH 8 -# define SSL_MAX_MASTER_KEY_LENGTH 48 +# define SSL_MAX_MASTER_KEY_LENGTH 64 /* The maximum number of encrypt/decrypt pipelines we can support */ # define SSL_MAX_PIPELINES 32 @@ -2282,6 +2282,7 @@ int ERR_load_SSL_strings(void); # define SSL_F_TLS_CONSTRUCT_CTOS_NPN 471 # define SSL_F_TLS_CONSTRUCT_CTOS_PADDING 472 # define SSL_F_TLS_CONSTRUCT_CTOS_PSK_KEX_MODES 509 +# define SSL_F_TLS_CONSTRUCT_CTOS_PSK 501 # define SSL_F_TLS_CONSTRUCT_CTOS_RENEGOTIATE 473 # define SSL_F_TLS_CONSTRUCT_CTOS_SCT 474 # define SSL_F_TLS_CONSTRUCT_CTOS_SERVER_NAME 475 @@ -2382,6 +2383,7 @@ int ERR_load_SSL_strings(void); # define SSL_R_BAD_LENGTH 271 # define SSL_R_BAD_PACKET_LENGTH 115 # define SSL_R_BAD_PROTOCOL_VERSION_NUMBER 116 +# define SSL_R_BAD_PSK_IDENTITY 114 # define SSL_R_BAD_RECORD_TYPE 443 # define SSL_R_BAD_RSA_ENCRYPT 119 # define SSL_R_BAD_SIGNATURE 123 diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h index 65569bd50d..328b266116 100644 --- a/include/openssl/tls1.h +++ b/include/openssl/tls1.h @@ -177,6 +177,7 @@ extern "C" { /* As defined for TLS1.3 */ # define TLSEXT_TYPE_key_share 40 +# define TLSEXT_TYPE_psk 41 # define TLSEXT_TYPE_supported_versions 43 # define TLSEXT_TYPE_psk_kex_modes 45 diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 1655333b13..6c74bd169d 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3545,20 +3545,23 @@ long ssl3_ctx_callback_ctrl(SSL_CTX *ctx, int cmd, void (*fp) (void)) return (1); } +const SSL_CIPHER *ssl3_get_cipher_by_id(uint32_t id) +{ + SSL_CIPHER c; + + c.id = id; + return OBJ_bsearch_ssl_cipher_id(&c, ssl3_ciphers, SSL3_NUM_CIPHERS); +} + /* * This function needs to check if the ciphers required are actually * available */ const SSL_CIPHER *ssl3_get_cipher_by_char(const unsigned char *p) { - SSL_CIPHER c; - const SSL_CIPHER *cp; - uint32_t id; - - id = 0x03000000 | ((uint32_t)p[0] << 8L) | (uint32_t)p[1]; - c.id = id; - cp = OBJ_bsearch_ssl_cipher_id(&c, ssl3_ciphers, SSL3_NUM_CIPHERS); - return cp; + return ssl3_get_cipher_by_id(0x03000000 + | ((uint32_t)p[0] << 8L) + | (uint32_t)p[1]); } int ssl3_put_cipher_by_char(const SSL_CIPHER *c, WPACKET *pkt, size_t *len) @@ -4103,13 +4106,14 @@ int ssl_derive(SSL *s, EVP_PKEY *privkey, EVP_PKEY *pubkey, int gensecret) if (gensecret) { if (SSL_IS_TLS13(s)) { /* - * TODO(TLS1.3): For now we just use the default early_secret, this - * will need to change later when other early_secrets will be - * possible. + * If we are resuming then we already generated the early secret + * when we created the ClientHello, so don't recreate it. */ - rv = tls13_generate_early_secret(s, NULL, 0) - && tls13_generate_handshake_secret(s, pms, pmslen); - OPENSSL_free(pms); + if (!s->hit) + rv = tls13_generate_secret(s, ssl_handshake_md(s), NULL, NULL, + 0, + (unsigned char *)&s->early_secret); + rv = rv && tls13_generate_handshake_secret(s, pms, pmslen); } else { /* Generate master secret and discard premaster */ rv = ssl_generate_master_secret(s, pms, pmslen, 1); diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 2d9401af62..2186f799fc 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -300,6 +300,7 @@ static ERR_STRING_DATA SSL_str_functs[] = { {ERR_FUNC(SSL_F_TLS_CONSTRUCT_CTOS_NPN), "tls_construct_ctos_npn"}, {ERR_FUNC(SSL_F_TLS_CONSTRUCT_CTOS_PADDING), "tls_construct_ctos_padding"}, + {ERR_FUNC(SSL_F_TLS_CONSTRUCT_CTOS_PSK), "tls_construct_ctos_psk"}, {ERR_FUNC(SSL_F_TLS_CONSTRUCT_CTOS_PSK_KEX_MODES), "tls_construct_ctos_psk_kex_modes"}, {ERR_FUNC(SSL_F_TLS_CONSTRUCT_CTOS_RENEGOTIATE), diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 17b377f507..7e5246c9f3 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -510,6 +510,11 @@ struct ssl_session_st { int ssl_version; /* what ssl version session info is being kept * in here? */ size_t master_key_length; + + /* + * For <=TLS1.2 this is the master_key. For TLS1.3 this is the resumption + * master secret + */ unsigned char master_key[SSL_MAX_MASTER_KEY_LENGTH]; /* session_id - valid? */ size_t session_id_length; @@ -569,6 +574,7 @@ struct ssl_session_st { size_t ticklen; /* Session ticket length */ /* Session lifetime hint in seconds */ unsigned long tick_lifetime_hint; + int tick_identity; } ext; # ifndef OPENSSL_NO_SRP char *srp_username; @@ -956,11 +962,12 @@ struct ssl_st { */ uint32_t mac_flags; /* - * The TLS1.3 early_secret and handshake_secret. The master_secret is stored - * in the session. + * The TLS1.3 secrets. The resumption master secret is stored in the + * session. */ unsigned char early_secret[EVP_MAX_MD_SIZE]; unsigned char handshake_secret[EVP_MAX_MD_SIZE]; + unsigned char master_secret[EVP_MAX_MD_SIZE]; unsigned char client_finished_secret[EVP_MAX_MD_SIZE]; unsigned char server_finished_secret[EVP_MAX_MD_SIZE]; unsigned char server_finished_hash[EVP_MAX_MD_SIZE]; @@ -1686,7 +1693,8 @@ typedef enum tlsext_index_en { TLSEXT_IDX_psk_kex_modes, TLSEXT_IDX_key_share, TLSEXT_IDX_cryptopro_bug, - TLSEXT_IDX_padding + TLSEXT_IDX_padding, + TLSEXT_IDX_psk } TLSEXT_INDEX; /* @@ -1726,6 +1734,9 @@ typedef enum tlsext_index_en { #define TLSEXT_KEX_MODE_FLAG_KE 1 #define TLSEXT_KEX_MODE_FLAG_KE_DHE 2 +/* An invalid index into the TLSv1.3 PSK identities */ +#define TLSEXT_PSK_BAD_IDENTITY -1 + #define SIGID_IS_PSS(sigid) ((sigid) == TLSEXT_SIGALG_rsa_pss_sha256 \ || (sigid) == TLSEXT_SIGALG_rsa_pss_sha384 \ || (sigid) == TLSEXT_SIGALG_rsa_pss_sha512) @@ -1986,6 +1997,7 @@ __owur int ssl_derive(SSL *s, EVP_PKEY *privkey, EVP_PKEY *pubkey, int genmaster); __owur EVP_PKEY *ssl_dh_to_pkey(DH *dh); +__owur const SSL_CIPHER *ssl3_get_cipher_by_id(uint32_t id); __owur const SSL_CIPHER *ssl3_get_cipher_by_char(const unsigned char *p); __owur int ssl3_put_cipher_by_char(const SSL_CIPHER *c, WPACKET *pkt, size_t *len); @@ -2110,7 +2122,8 @@ __owur int tls13_setup_key_block(SSL *s); __owur size_t tls13_final_finish_mac(SSL *s, const char *str, size_t slen, unsigned char *p); __owur int tls13_change_cipher_state(SSL *s, int which); -__owur int tls13_hkdf_expand(SSL *s, const unsigned char *secret, +__owur int tls13_hkdf_expand(SSL *s, const EVP_MD *md, + const unsigned char *secret, const unsigned char *label, size_t labellen, const unsigned char *hash, unsigned char *out, size_t outlen); @@ -2118,8 +2131,14 @@ __owur int tls13_derive_key(SSL *s, const unsigned char *secret, unsigned char *key, size_t keylen); __owur int tls13_derive_iv(SSL *s, const unsigned char *secret, unsigned char *iv, size_t ivlen); -__owur int tls13_generate_early_secret(SSL *s, const unsigned char *insecret, - size_t insecretlen); +__owur int tls13_derive_finishedkey(SSL *s, const EVP_MD *md, + const unsigned char *secret, + unsigned char *fin, size_t finlen); +int tls13_generate_secret(SSL *s, const EVP_MD *md, + const unsigned char *prevsecret, + const unsigned char *insecret, + size_t insecretlen, + unsigned char *outsecret); __owur int tls13_generate_handshake_secret(SSL *s, const unsigned char *insecret, size_t insecretlen); diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index dc992010ec..5837720eb7 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -264,12 +264,20 @@ static const EXTENSION_DEFINITION ext_defs[] = { NULL, NULL, NULL, tls_construct_stoc_cryptopro_bug, NULL, NULL }, { - /* Last in the list because it must be added as the last extension */ + /* Must be immediately before pre_shared_key */ + /* TODO(TLS1.3): Fix me */ TLSEXT_TYPE_padding, EXT_CLIENT_HELLO, NULL, /* We send this, but don't read it */ NULL, NULL, NULL, tls_construct_ctos_padding, NULL + }, + { + /* Required by the TLSv1.3 spec to always be the last extension */ + TLSEXT_TYPE_psk, + EXT_CLIENT_HELLO | EXT_TLS1_3_SERVER_HELLO | EXT_TLS_IMPLEMENTATION_ONLY + | EXT_TLS1_3_ONLY, + NULL, NULL, NULL, NULL, tls_construct_ctos_psk, NULL } }; diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c index 57f6564678..c6a8124c9e 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -656,6 +656,170 @@ int tls_construct_ctos_padding(SSL *s, WPACKET *pkt, X509 *x, size_t chainidx, return 1; } +/* + * Construct the pre_shared_key extension + */ +int tls_construct_ctos_psk(SSL *s, WPACKET *pkt, X509 *x, size_t chainidx, + int *al) +{ +#ifndef OPENSSL_NO_TLS1_3 + const SSL_CIPHER *cipher; + uint32_t now, ages, agems; + size_t hashsize, bindersize, binderoffset, msglen; + unsigned char *binder = NULL, *msgstart = NULL; + EVP_PKEY *mackey = NULL; + const EVP_MD *md; + EVP_MD_CTX *mctx = NULL; + unsigned char hash[EVP_MAX_MD_SIZE], binderkey[EVP_MAX_MD_SIZE]; + unsigned char finishedkey[EVP_MAX_MD_SIZE]; + const char resumption_label[] = "resumption psk binder key"; + int ret = 0; + + s->session->ext.tick_identity = TLSEXT_PSK_BAD_IDENTITY; + + /* + * If this is a new session then we have nothing to resume so don't add + * this extension. + */ + if (s->session->ext.ticklen == 0) + return 1; + + /* + * Technically the C standard just says time() returns a time_t and says + * nothing about the encoding of that type. In practice most implementations + * follow POSIX which holds it as an integral type in seconds since epoch. + * We've already made the assumption that we can do this in multiple places + * in the code, so portability shouldn't be an issue. + */ + now = (uint32_t)time(NULL); + ages = now - (uint32_t)s->session->time; + + /* + * Calculate age in ms. We're just doing it to nearest second. Should be + * good enough. + */ + agems = ages * (uint32_t)1000; + + if (ages != 0 && agems / (uint32_t)1000 != ages) { + /* + * Overflow. Shouldn't happen unless this is a *really* old session. If + * so we just ignore it. + */ + return 1; + } + + /* TODO(TLS1.3): Obfuscate the age here */ + + cipher = ssl3_get_cipher_by_id(s->session->cipher_id); + if (cipher == NULL) { + /* Don't recognise this cipher so we can't use the session. Ignore it */ + return 1; + } + md = ssl_md(cipher->algorithm2); + if (md == NULL) { + /* Shouldn't happen!! */ + SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR); + return 0; + } + hashsize = EVP_MD_size(md); + + /* Create the extension, but skip over the binder for now */ + if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_psk) + || !WPACKET_start_sub_packet_u16(pkt) + || !WPACKET_start_sub_packet_u16(pkt) + || !WPACKET_sub_memcpy_u16(pkt, s->session->ext.tick, + s->session->ext.ticklen) + || !WPACKET_put_bytes_u32(pkt, agems) + || !WPACKET_close(pkt) + || !WPACKET_get_total_written(pkt, &binderoffset) + || !WPACKET_start_sub_packet_u16(pkt) + || !WPACKET_sub_allocate_bytes_u8(pkt, hashsize, &binder) + || !WPACKET_close(pkt) + || !WPACKET_close(pkt) + || !WPACKET_get_total_written(pkt, &msglen) + /* + * We need to fill in all the sub-packet lengths now so we can + * calculate the HMAC of the message up to the binders + */ + || !WPACKET_fill_lengths(pkt)) { + SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR); + goto err; + } + + msgstart = WPACKET_get_curr(pkt) - msglen; + + /* Generate the early_secret */ + if (!tls13_generate_secret(s, md, NULL, s->session->master_key, + s->session->master_key_length, + (unsigned char *)&s->early_secret)) { + SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR); + goto err; + } + + /* + * Create the handshake hash for the binder key...the messages so far are + * empty! + */ + mctx = EVP_MD_CTX_new(); + if (mctx == NULL + || EVP_DigestInit_ex(mctx, md, NULL) <= 0 + || EVP_DigestFinal_ex(mctx, hash, NULL) <= 0) { + SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR); + goto err; + } + + /* Generate the binder key */ + if (!tls13_hkdf_expand(s, md, s->early_secret, + (unsigned char *)resumption_label, + sizeof(resumption_label) - 1, hash, binderkey, + hashsize)) { + SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR); + goto err; + } + + /* Generate the finished key */ + if (!tls13_derive_finishedkey(s, md, binderkey, finishedkey, hashsize)) { + SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR); + goto err; + } + + /* + * Get a hash of the ClientHello up to the start of the binders. + * TODO(TLS1.3): This will need to be tweaked when we implement + * HelloRetryRequest to include the digest of the previous messages here. + */ + if (EVP_DigestInit_ex(mctx, md, NULL) <= 0 + || EVP_DigestUpdate(mctx, msgstart, binderoffset) <= 0 + || EVP_DigestFinal_ex(mctx, hash, NULL) <= 0) { + SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR); + goto err; + } + + mackey = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL, finishedkey, hashsize); + bindersize = hashsize; + if (binderkey == NULL + || EVP_DigestSignInit(mctx, NULL, md, NULL, mackey) <= 0 + || EVP_DigestSignUpdate(mctx, hash, hashsize) <= 0 + || EVP_DigestSignFinal(mctx, binder, &bindersize) <= 0 + || bindersize != hashsize) { + SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR); + goto err; + } + + s->session->ext.tick_identity = 0; + + ret = 1; + err: + OPENSSL_cleanse(binderkey, sizeof(binderkey)); + EVP_PKEY_free(mackey); + EVP_MD_CTX_free(mctx); + + return ret; +#else + return 1; +#endif +} + /* * Parse the server's renegotiation binding and abort if it's not right */ diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index ad1466f9a9..3a6b672a0f 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -650,7 +650,7 @@ MSG_PROCESS_RETURN tls_process_finished(SSL *s, PACKET *pkt) } } else { if (!s->method->ssl3_enc->generate_master_secret(s, - s->session->master_key, s->handshake_secret, 0, + s->master_secret, s->handshake_secret, 0, &s->session->master_key_length)) { SSLerr(SSL_F_TLS_PROCESS_FINISHED, SSL_R_CANNOT_CHANGE_CIPHER); goto f_err; diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h index d15c69686e..67858057f7 100644 --- a/ssl/statem/statem_locl.h +++ b/ssl/statem/statem_locl.h @@ -290,6 +290,8 @@ int tls_construct_ctos_psk_kex_modes(SSL *s, WPACKET *pkt, X509 *x, size_t chainidx, int *al); int tls_construct_ctos_padding(SSL *s, WPACKET *pkt, X509 *x, size_t chainidx, int *al); +int tls_construct_ctos_psk(SSL *s, WPACKET *pkt, X509 *x, size_t chainidx, + int *al); int tls_parse_stoc_renegotiate(SSL *s, PACKET *pkt, X509 *x, size_t chainidx, int *al); int tls_parse_stoc_server_name(SSL *s, PACKET *pkt, X509 *x, size_t chainidx, diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 6b3d4f7749..20e521a9d6 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -800,7 +800,7 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst) #endif if (SSL_IS_TLS13(s)) { if (!s->method->ssl3_enc->generate_master_secret(s, - s->session->master_key, s->handshake_secret, 0, + s->master_secret, s->handshake_secret, 0, &s->session->master_key_length) || !s->method->ssl3_enc->change_cipher_state(s, SSL3_CC_APPLICATION | SSL3_CHANGE_CIPHER_SERVER_WRITE)) diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c index 449e6f9f36..7c217c11d3 100644 --- a/ssl/tls13_enc.c +++ b/ssl/tls13_enc.c @@ -23,13 +23,12 @@ static const unsigned char default_zeros[EVP_MAX_MD_SIZE]; * the location pointed to be |out|. The |hash| value may be NULL. Returns 1 on * success 0 on failure. */ -int tls13_hkdf_expand(SSL *s, const unsigned char *secret, +int tls13_hkdf_expand(SSL *s, const EVP_MD *md, const unsigned char *secret, const unsigned char *label, size_t labellen, const unsigned char *hash, unsigned char *out, size_t outlen) { const unsigned char label_prefix[] = "TLS 1.3, "; - const EVP_MD *md = ssl_handshake_md(s); EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_HKDF, NULL); int ret; size_t hkdflabellen; @@ -83,8 +82,8 @@ int tls13_derive_key(SSL *s, const unsigned char *secret, unsigned char *key, { static const unsigned char keylabel[] = "key"; - return tls13_hkdf_expand(s, secret, keylabel, sizeof(keylabel) - 1, NULL, - key, keylen); + return tls13_hkdf_expand(s, ssl_handshake_md(s), secret, keylabel, + sizeof(keylabel) - 1, NULL, key, keylen); } /* @@ -96,16 +95,17 @@ int tls13_derive_iv(SSL *s, const unsigned char *secret, unsigned char *iv, { static const unsigned char ivlabel[] = "iv"; - return tls13_hkdf_expand(s, secret, ivlabel, sizeof(ivlabel) - 1, NULL, - iv, ivlen); + return tls13_hkdf_expand(s, ssl_handshake_md(s), secret, ivlabel, + sizeof(ivlabel) - 1, NULL, iv, ivlen); } -static int tls13_derive_finishedkey(SSL *s, const unsigned char *secret, - unsigned char *fin, size_t finlen) +int tls13_derive_finishedkey(SSL *s, const EVP_MD *md, + const unsigned char *secret, + unsigned char *fin, size_t finlen) { static const unsigned char finishedlabel[] = "finished"; - return tls13_hkdf_expand(s, secret, finishedlabel, + return tls13_hkdf_expand(s, md, secret, finishedlabel, sizeof(finishedlabel) - 1, NULL, fin, finlen); } @@ -114,12 +114,12 @@ static int tls13_derive_finishedkey(SSL *s, const unsigned char *secret, * length |insecretlen|, generate a new secret and store it in the location * pointed to by |outsecret|. Returns 1 on success 0 on failure. */ -static int tls13_generate_secret(SSL *s, const unsigned char *prevsecret, - const unsigned char *insecret, - size_t insecretlen, - unsigned char *outsecret) +int tls13_generate_secret(SSL *s, const EVP_MD *md, + const unsigned char *prevsecret, + const unsigned char *insecret, + size_t insecretlen, + unsigned char *outsecret) { - const EVP_MD *md = ssl_handshake_md(s); size_t mdlen, prevsecretlen; int ret; EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_HKDF, NULL); @@ -154,17 +154,6 @@ static int tls13_generate_secret(SSL *s, const unsigned char *prevsecret, return ret == 0; } -/* - * Given an input secret |insecret| of length |insecretlen| generate the early - * secret. Returns 1 on success 0 on failure. - */ -int tls13_generate_early_secret(SSL *s, const unsigned char *insecret, - size_t insecretlen) -{ - return tls13_generate_secret(s, NULL, insecret, insecretlen, - (unsigned char *)&s->early_secret); -} - /* * Given an input secret |insecret| of length |insecretlen| generate the * handshake secret. This requires the early secret to already have been @@ -173,7 +162,8 @@ int tls13_generate_early_secret(SSL *s, const unsigned char *insecret, int tls13_generate_handshake_secret(SSL *s, const unsigned char *insecret, size_t insecretlen) { - return tls13_generate_secret(s, s->early_secret, insecret, insecretlen, + return tls13_generate_secret(s, ssl_handshake_md(s), s->early_secret, + insecret, insecretlen, (unsigned char *)&s->handshake_secret); } @@ -186,8 +176,10 @@ int tls13_generate_master_secret(SSL *s, unsigned char *out, unsigned char *prev, size_t prevlen, size_t *secret_size) { - *secret_size = EVP_MD_size(ssl_handshake_md(s)); - return tls13_generate_secret(s, prev, NULL, 0, out); + const EVP_MD *md = ssl_handshake_md(s); + + *secret_size = EVP_MD_size(md); + return tls13_generate_secret(s, md, prev, NULL, 0, out); } /* @@ -260,6 +252,8 @@ int tls13_change_cipher_state(SSL *s, int which) "server handshake traffic secret"; static const unsigned char server_application_traffic[] = "server application traffic secret"; + static const unsigned char resumption_master_secret[] = + "resumption master secret"; unsigned char key[EVP_MAX_KEY_LENGTH]; unsigned char *iv; unsigned char secret[EVP_MAX_MD_SIZE]; @@ -313,9 +307,7 @@ int tls13_change_cipher_state(SSL *s, int which) label = client_handshake_traffic; labellen = sizeof(client_handshake_traffic) - 1; } else { - int hashleni; - - insecret = s->session->master_key; + insecret = s->master_secret; label = client_application_traffic; labellen = sizeof(client_application_traffic) - 1; /* @@ -325,12 +317,6 @@ int tls13_change_cipher_state(SSL *s, int which) * previously saved value. */ hash = s->server_finished_hash; - hashleni = EVP_MD_CTX_size(s->s3->handshake_dgst); - if (hashleni < 0) { - SSLerr(SSL_F_TLS13_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); - goto err; - } - hashlen = (size_t)hashleni; } } else { if (which & SSL3_CC_HANDSHAKE) { @@ -340,42 +326,57 @@ int tls13_change_cipher_state(SSL *s, int which) label = server_handshake_traffic; labellen = sizeof(server_handshake_traffic) - 1; } else { - insecret = s->session->master_key; + insecret = s->master_secret; label = server_application_traffic; labellen = sizeof(server_application_traffic) - 1; } } - if (label != client_application_traffic) { - if (!ssl3_digest_cached_records(s, 1) - || !ssl_handshake_hash(s, hash, sizeof(hashval), &hashlen)) { - SSLerr(SSL_F_TLS13_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); - goto err; - } - - /* - * Save the hash of handshakes up to now for use when we calculate the - * client application traffic secret - */ - if (label == server_application_traffic) - memcpy(s->server_finished_hash, hash, hashlen); + if (!ssl3_digest_cached_records(s, 1) + || !ssl_handshake_hash(s, hashval, sizeof(hashval), &hashlen)) { + SSLerr(SSL_F_TLS13_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); + goto err; } - if (!tls13_hkdf_expand(s, insecret, label, labellen, hash, secret, - hashlen)) { + /* + * Save the hash of handshakes up to now for use when we calculate the + * client application traffic secret + */ + if (label == server_application_traffic) + memcpy(s->server_finished_hash, hashval, hashlen); + + if (!tls13_hkdf_expand(s, ssl_handshake_md(s), insecret, label, labellen, + hash, secret, hashlen)) { SSLerr(SSL_F_TLS13_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); goto err; } + if (label == client_application_traffic) { + /* + * We also create the resumption master secret, but this time use the + * hash for the whole handshake including the Client Finished + */ + if (!tls13_hkdf_expand(s, ssl_handshake_md(s), insecret, + resumption_master_secret, + sizeof(resumption_master_secret) - 1, + hashval, s->session->master_key, hashlen)) { + SSLerr(SSL_F_TLS13_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); + goto err; + } + s->session->master_key_length = hashlen; + } + /* TODO(size_t): convert me */ keylen = EVP_CIPHER_key_length(ciph); ivlen = EVP_CIPHER_iv_length(ciph); if (!tls13_derive_key(s, secret, key, keylen) || !tls13_derive_iv(s, secret, iv, ivlen) - || (finsecret != NULL && !tls13_derive_finishedkey(s, secret, - finsecret, - finsecretlen))) { + || (finsecret != NULL && !tls13_derive_finishedkey(s, + ssl_handshake_md(s), + secret, + finsecret, + finsecretlen))) { SSLerr(SSL_F_TLS13_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); goto err; } diff --git a/test/tls13secretstest.c b/test/tls13secretstest.c index 68ebb9b8a7..69b85b820e 100644 --- a/test/tls13secretstest.c +++ b/test/tls13secretstest.c @@ -196,13 +196,14 @@ static int test_secret(SSL *s, unsigned char *prk, unsigned char hash[EVP_MAX_MD_SIZE]; unsigned char key[KEYLEN]; unsigned char iv[IVLEN]; + const EVP_MD *md = ssl_handshake_md(s); if (!ssl_handshake_hash(s, hash, sizeof(hash), &hashsize)) { fprintf(stderr, "Failed to get hash\n"); return 0; } - if (!tls13_hkdf_expand(s, prk, label, labellen, hash, gensecret, + if (!tls13_hkdf_expand(s, md, prk, label, labellen, hash, gensecret, hashsize)) { fprintf(stderr, "Secret generation failed\n"); return 0; @@ -253,7 +254,12 @@ static int test_handshake_secrets(void) if (s == NULL) goto err; - if (!tls13_generate_early_secret(s, NULL, 0)) { + s->session = SSL_SESSION_new(); + if (s->session == NULL) + goto err; + + if (!tls13_generate_secret(s, ssl_handshake_md(s), NULL, NULL, 0, + (unsigned char *)&s->early_secret)) { fprintf(stderr, "Early secret generation failed\n"); goto err; } -- 2.25.1