diff options
-rw-r--r-- | include/openssl/ssl.h | 4 | ||||
-rw-r--r-- | ssl/s3_lib.c | 21 | ||||
-rw-r--r-- | ssl/ssl_err.c | 2 | ||||
-rw-r--r-- | ssl/ssl_locl.h | 3 | ||||
-rw-r--r-- | ssl/statem/statem_clnt.c | 85 | ||||
-rw-r--r-- | ssl/statem/statem_lib.c | 28 | ||||
-rw-r--r-- | ssl/statem/statem_srvr.c | 91 | ||||
-rw-r--r-- | ssl/t1_lib.c | 8 | ||||
-rw-r--r-- | ssl/tls13_enc.c | 47 | ||||
-rw-r--r-- | test/tls13secretstest.c | 8 |
10 files changed, 188 insertions, 109 deletions
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 5064e65cf5..2fd0e9fb42 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -2242,7 +2242,8 @@ int ERR_load_SSL_strings(void); # define SSL_F_SSL_WRITE_EX 433 # define SSL_F_STATE_MACHINE 353 # define SSL_F_TLS12_CHECK_PEER_SIGALG 333 -# define SSL_F_TLS13_CHANGE_CIPHER_STATE 435 +# define SSL_F_TLS13_CHANGE_CIPHER_STATE 440 +# define SSL_F_TLS13_SETUP_KEY_BLOCK 441 # define SSL_F_TLS1_CHANGE_CIPHER_STATE 209 # define SSL_F_TLS1_CHECK_DUPLICATE_EXTENSIONS 341 # define SSL_F_TLS1_ENC 401 @@ -2336,6 +2337,7 @@ int ERR_load_SSL_strings(void); # define SSL_R_BIO_NOT_SET 128 # define SSL_R_BLOCK_CIPHER_PAD_IS_WRONG 129 # define SSL_R_BN_LIB 130 +# define SSL_R_CANNOT_CHANGE_CIPHER 109 # define SSL_R_CA_DN_LENGTH_MISMATCH 131 # define SSL_R_CA_KEY_TOO_SMALL 397 # define SSL_R_CA_MD_TOO_WEAK 398 diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index bcc0f9e5fd..524f5308f3 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -4067,8 +4067,8 @@ EVP_PKEY *ssl_generate_pkey_curve(int id) } #endif -/* Derive premaster or master secret for ECDH/DH */ -int ssl_derive(SSL *s, EVP_PKEY *privkey, EVP_PKEY *pubkey, int genmaster) +/* Derive secrets for ECDH/DH */ +int ssl_derive(SSL *s, EVP_PKEY *privkey, EVP_PKEY *pubkey, int gensecret) { int rv = 0; unsigned char *pms = NULL; @@ -4093,9 +4093,20 @@ int ssl_derive(SSL *s, EVP_PKEY *privkey, EVP_PKEY *pubkey, int genmaster) if (EVP_PKEY_derive(pctx, pms, &pmslen) <= 0) goto err; - if (genmaster) { - /* Generate master secret and discard premaster */ - rv = ssl_generate_master_secret(s, pms, pmslen, 1); + 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. + */ + rv = tls13_generate_early_secret(s, NULL, 0) + && tls13_generate_handshake_secret(s, pms, pmslen); + OPENSSL_free(pms); + } else { + /* Generate master secret and discard premaster */ + rv = ssl_generate_master_secret(s, pms, pmslen, 1); + } pms = NULL; } else { /* Save premaster secret */ diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index b7ba8a5202..825c5638b8 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -239,6 +239,7 @@ static ERR_STRING_DATA SSL_str_functs[] = { {ERR_FUNC(SSL_F_STATE_MACHINE), "state_machine"}, {ERR_FUNC(SSL_F_TLS12_CHECK_PEER_SIGALG), "tls12_check_peer_sigalg"}, {ERR_FUNC(SSL_F_TLS13_CHANGE_CIPHER_STATE), "tls13_change_cipher_state"}, + {ERR_FUNC(SSL_F_TLS13_SETUP_KEY_BLOCK), "tls13_setup_key_block"}, {ERR_FUNC(SSL_F_TLS1_CHANGE_CIPHER_STATE), "tls1_change_cipher_state"}, {ERR_FUNC(SSL_F_TLS1_CHECK_DUPLICATE_EXTENSIONS), "tls1_check_duplicate_extensions"}, @@ -368,6 +369,7 @@ static ERR_STRING_DATA SSL_str_reasons[] = { {ERR_REASON(SSL_R_BLOCK_CIPHER_PAD_IS_WRONG), "block cipher pad is wrong"}, {ERR_REASON(SSL_R_BN_LIB), "bn lib"}, + {ERR_REASON(SSL_R_CANNOT_CHANGE_CIPHER), "cannot change cipher"}, {ERR_REASON(SSL_R_CA_DN_LENGTH_MISMATCH), "ca dn length mismatch"}, {ERR_REASON(SSL_R_CA_KEY_TOO_SMALL), "ca key too small"}, {ERR_REASON(SSL_R_CA_MD_TOO_WEAK), "ca md too weak"}, diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 527b2b40e2..e9ba0f62c5 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -2003,6 +2003,9 @@ __owur size_t tls1_final_finish_mac(SSL *s, const char *str, size_t slen, __owur int tls1_generate_master_secret(SSL *s, unsigned char *out, unsigned char *p, size_t len, size_t *secret_size); +__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_derive_secret(SSL *s, const unsigned char *insecret, const unsigned char *label, size_t labellen, diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 0429e43ee7..be3148df0f 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -136,18 +136,28 @@ static int ossl_statem_client13_read_transition(SSL *s, int mt) case TLS_ST_CR_SRVR_HELLO: if (s->hit) { - if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) { - st->hand_state = TLS_ST_CR_CHANGE; + if (mt == SSL3_MT_FINISHED) { + st->hand_state = TLS_ST_CR_FINISHED; return 1; } } else { - if (mt == SSL3_MT_CERTIFICATE) { + if (mt == SSL3_MT_CERTIFICATE_REQUEST) { + st->hand_state = TLS_ST_CR_CERT_REQ; + return 1; + } else if (mt == SSL3_MT_CERTIFICATE) { st->hand_state = TLS_ST_CR_CERT; return 1; } } break; + case TLS_ST_CR_CERT_REQ: + if (mt == SSL3_MT_CERTIFICATE) { + st->hand_state = TLS_ST_CR_CERT; + return 1; + } + break; + case TLS_ST_CR_CERT: /* * The CertificateStatus message is optional even if @@ -160,38 +170,14 @@ static int ossl_statem_client13_read_transition(SSL *s, int mt) /* Fall through */ case TLS_ST_CR_CERT_STATUS: - if (mt == SSL3_MT_CERTIFICATE_REQUEST) { - if (cert_req_allowed(s)) { - st->hand_state = TLS_ST_CR_CERT_REQ; - return 1; - } - goto err; - } - /* Fall through */ - - case TLS_ST_CR_CERT_REQ: - if (mt == SSL3_MT_SERVER_DONE) { - st->hand_state = TLS_ST_CR_SRVR_DONE; - return 1; - } - break; - - case TLS_ST_CW_FINISHED: - if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) { - st->hand_state = TLS_ST_CR_CHANGE; - return 1; - } - break; - - case TLS_ST_CR_CHANGE: if (mt == SSL3_MT_FINISHED) { st->hand_state = TLS_ST_CR_FINISHED; return 1; } break; + } - err: /* No valid transition found */ ssl3_send_alert(s, SSL3_AL_FATAL, SSL3_AD_UNEXPECTED_MESSAGE); SSLerr(SSL_F_OSSL_STATEM_CLIENT13_READ_TRANSITION, @@ -393,38 +379,22 @@ static WRITE_TRAN ossl_statem_client13_write_transition(SSL *s) /* Shouldn't happen */ return WRITE_TRAN_ERROR; - case TLS_ST_CR_SRVR_DONE: + case TLS_ST_CR_FINISHED: st->hand_state = (s->s3->tmp.cert_req != 0) ? TLS_ST_CW_CERT - : TLS_ST_CW_CHANGE; + : TLS_ST_CW_FINISHED; return WRITE_TRAN_CONTINUE; case TLS_ST_CW_CERT: /* If a non-empty Certificate we also send CertificateVerify */ st->hand_state = (s->s3->tmp.cert_req == 1) ? TLS_ST_CW_CERT_VRFY - : TLS_ST_CW_CHANGE; + : TLS_ST_CW_FINISHED; return WRITE_TRAN_CONTINUE; case TLS_ST_CW_CERT_VRFY: - st->hand_state = TLS_ST_CW_CHANGE; - return WRITE_TRAN_CONTINUE; - - case TLS_ST_CW_CHANGE: st->hand_state = TLS_ST_CW_FINISHED; return WRITE_TRAN_CONTINUE; case TLS_ST_CW_FINISHED: - if (s->hit) { - st->hand_state = TLS_ST_OK; - ossl_statem_set_in_init(s, 0); - return WRITE_TRAN_CONTINUE; - } - return WRITE_TRAN_FINISHED; - - case TLS_ST_CR_FINISHED: - if (s->hit) { - st->hand_state = TLS_ST_CW_CHANGE; - return WRITE_TRAN_CONTINUE; - } st->hand_state = TLS_ST_OK; ossl_statem_set_in_init(s, 0); return WRITE_TRAN_CONTINUE; @@ -666,6 +636,12 @@ WORK_STATE ossl_statem_client_post_work(SSL *s, WORK_STATE wst) #endif if (statem_flush(s) != 1) return WORK_MORE_B; + + if (SSL_IS_TLS13(s)) { + if (!s->method->ssl3_enc->change_cipher_state(s, + SSL3_CC_APPLICATION | SSL3_CHANGE_CIPHER_CLIENT_WRITE)) + return WORK_ERROR; + } break; } @@ -1343,6 +1319,21 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt) } #endif + /* + * In TLSv1.3 we have some post-processing to change cipher state, otherwise + * we're done with this message + */ + if (SSL_IS_TLS13(s) + && (!s->method->ssl3_enc->setup_key_block(s) + || !s->method->ssl3_enc->change_cipher_state(s, + SSL3_CC_HANDSHAKE | SSL3_CHANGE_CIPHER_CLIENT_WRITE) + || !s->method->ssl3_enc->change_cipher_state(s, + SSL3_CC_HANDSHAKE | SSL3_CHANGE_CIPHER_CLIENT_READ))) { + al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO, SSL_R_CANNOT_CHANGE_CIPHER); + goto f_err; + } + return MSG_PROCESS_CONTINUE_READING; f_err: ssl3_send_alert(s, SSL3_AL_FATAL, al); diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index 46ffb47e10..6d32666e43 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -330,7 +330,7 @@ MSG_PROCESS_RETURN tls_process_finished(SSL *s, PACKET *pkt) size_t md_len; /* If this occurs, we have missed a message */ - if (!s->s3->change_cipher_spec) { + if (!SSL_IS_TLS13(s) && !s->s3->change_cipher_spec) { al = SSL_AD_UNEXPECTED_MESSAGE; SSLerr(SSL_F_TLS_PROCESS_FINISHED, SSL_R_GOT_A_FIN_BEFORE_A_CCS); goto f_err; @@ -367,6 +367,32 @@ MSG_PROCESS_RETURN tls_process_finished(SSL *s, PACKET *pkt) s->s3->previous_server_finished_len = md_len; } + /* In TLS1.3 we also have to change cipher state */ + if (SSL_IS_TLS13(s)) { + if (s->server) { + if (!s->method->ssl3_enc->change_cipher_state(s, + SSL3_CC_APPLICATION | SSL3_CHANGE_CIPHER_SERVER_READ)) { + al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_FINISHED, SSL_R_CANNOT_CHANGE_CIPHER); + goto f_err; + } + } else { + if (!s->method->ssl3_enc->generate_master_secret(s, + s->session->master_key, s->handshake_secret, 0, + &s->session->master_key_length)) { + al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_FINISHED, SSL_R_CANNOT_CHANGE_CIPHER); + goto f_err; + } + if (!s->method->ssl3_enc->change_cipher_state(s, + SSL3_CC_APPLICATION | SSL3_CHANGE_CIPHER_CLIENT_READ)) { + al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_FINISHED, SSL_R_CANNOT_CHANGE_CIPHER); + goto f_err; + } + } + } + return MSG_PROCESS_FINISHED_READING; f_err: ssl3_send_alert(s, SSL3_AL_FATAL, al); diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index c8c31a3a67..108e638db4 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -94,15 +94,15 @@ static int ossl_statem_server13_read_transition(SSL *s, int mt) default: break; - case TLS_ST_SW_SRVR_DONE: + case TLS_ST_SW_FINISHED: if (s->s3->tmp.cert_request) { if (mt == SSL3_MT_CERTIFICATE) { st->hand_state = TLS_ST_SR_CERT; return 1; } } else { - if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) { - st->hand_state = TLS_ST_SR_CHANGE; + if (mt == SSL3_MT_FINISHED) { + st->hand_state = TLS_ST_SR_FINISHED; return 1; } } @@ -110,8 +110,8 @@ static int ossl_statem_server13_read_transition(SSL *s, int mt) case TLS_ST_SR_CERT: if (s->session->peer == NULL) { - if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) { - st->hand_state = TLS_ST_SR_CHANGE; + if (mt == SSL3_MT_FINISHED) { + st->hand_state = TLS_ST_SR_FINISHED; return 1; } } else { @@ -123,25 +123,11 @@ static int ossl_statem_server13_read_transition(SSL *s, int mt) break; case TLS_ST_SR_CERT_VRFY: - if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) { - st->hand_state = TLS_ST_SR_CHANGE; - return 1; - } - break; - - case TLS_ST_SR_CHANGE: if (mt == SSL3_MT_FINISHED) { st->hand_state = TLS_ST_SR_FINISHED; return 1; } break; - - case TLS_ST_SW_FINISHED: - if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) { - st->hand_state = TLS_ST_SR_CHANGE; - return 1; - } - break; } /* No valid transition found */ @@ -419,52 +405,33 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s) case TLS_ST_SW_SRVR_HELLO: if (s->hit) - st->hand_state = TLS_ST_SW_CHANGE; + st->hand_state = TLS_ST_SW_FINISHED; + else if (send_certificate_request(s)) + st->hand_state = TLS_ST_SW_CERT_REQ; else st->hand_state = TLS_ST_SW_CERT; return WRITE_TRAN_CONTINUE; - case TLS_ST_SW_CERT: - if (s->tlsext_status_expected) { - st->hand_state = TLS_ST_SW_CERT_STATUS; - return WRITE_TRAN_CONTINUE; - } - /* Fall through */ - - case TLS_ST_SW_CERT_STATUS: - if (send_certificate_request(s)) { - st->hand_state = TLS_ST_SW_CERT_REQ; - return WRITE_TRAN_CONTINUE; - } - /* Fall through */ - case TLS_ST_SW_CERT_REQ: - st->hand_state = TLS_ST_SW_SRVR_DONE; + st->hand_state = TLS_ST_SW_CERT; return WRITE_TRAN_CONTINUE; - case TLS_ST_SW_SRVR_DONE: - return WRITE_TRAN_FINISHED; - - case TLS_ST_SR_FINISHED: - if (s->hit) { - st->hand_state = TLS_ST_OK; - ossl_statem_set_in_init(s, 0); - return WRITE_TRAN_CONTINUE; - } - st->hand_state = TLS_ST_SW_CHANGE; - + case TLS_ST_SW_CERT: + if (s->tlsext_status_expected) + st->hand_state = TLS_ST_SW_CERT_STATUS; + else + st->hand_state = TLS_ST_SW_FINISHED; return WRITE_TRAN_CONTINUE; - - case TLS_ST_SW_CHANGE: + case TLS_ST_SW_CERT_STATUS: st->hand_state = TLS_ST_SW_FINISHED; return WRITE_TRAN_CONTINUE; case TLS_ST_SW_FINISHED: - if (s->hit) - return WRITE_TRAN_FINISHED; + return WRITE_TRAN_FINISHED; + case TLS_ST_SR_FINISHED: st->hand_state = TLS_ST_OK; ossl_statem_set_in_init(s, 0); return WRITE_TRAN_CONTINUE; @@ -740,6 +707,20 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst) sizeof(sctpauthkey), sctpauthkey); } #endif + /* + * TODO(TLS1.3): This actually causes a problem. We don't yet know + * whether the next record we are going to receive is an unencrypted + * alert, or an encrypted handshake message. We're going to need + * something clever in the record layer for this. + */ + if (SSL_IS_TLS13(s)) { + if (!s->method->ssl3_enc->setup_key_block(s) + || !s->method->ssl3_enc->change_cipher_state(s, + SSL3_CC_HANDSHAKE | SSL3_CHANGE_CIPHER_SERVER_WRITE) + || !s->method->ssl3_enc->change_cipher_state(s, + SSL3_CC_HANDSHAKE |SSL3_CHANGE_CIPHER_SERVER_READ)) + return WORK_ERROR; + } break; case TLS_ST_SW_CHANGE: @@ -782,6 +763,14 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst) 0, NULL); } #endif + if (SSL_IS_TLS13(s)) { + if (!s->method->ssl3_enc->generate_master_secret(s, + s->session->master_key, 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)) + return WORK_ERROR; + } break; } @@ -1001,7 +990,7 @@ WORK_STATE ossl_statem_server_post_process_message(SSL *s, WORK_STATE wst) #endif return WORK_FINISHED_CONTINUE; } - + return WORK_FINISHED_CONTINUE; } #ifndef OPENSSL_NO_SRP diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 19853cae4a..a6185a16f1 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -81,10 +81,10 @@ SSL3_ENC_METHOD const TLSv1_2_enc_data = { SSL3_ENC_METHOD const TLSv1_3_enc_data = { tls1_enc, tls1_mac, - tls1_setup_key_block, - tls1_generate_master_secret, - tls1_change_cipher_state, - tls1_final_finish_mac, + tls13_setup_key_block, + tls13_generate_master_secret, + tls13_change_cipher_state, + tls13_final_finish_mac, TLS_MD_CLIENT_FINISH_CONST, TLS_MD_CLIENT_FINISH_CONST_SIZE, TLS_MD_SERVER_FINISH_CONST, TLS_MD_SERVER_FINISH_CONST_SIZE, tls1_alert_code, diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c index 04dba3b23e..65b5dee768 100644 --- a/ssl/tls13_enc.c +++ b/ssl/tls13_enc.c @@ -214,6 +214,53 @@ int tls13_generate_master_secret(SSL *s, unsigned char *out, return tls13_generate_secret(s, prev, NULL, 0, out); } +/* + * Generates the mac for the Finished message. + * + * Returns the length of the MAC or 0 on error. + */ +size_t tls13_final_finish_mac(SSL *s, const char *str, size_t slen, + unsigned char *out) +{ + size_t hashlen; + const EVP_MD *md; + + /* + * TODO(TLS1.3): This is a dummy implementation for now. We need to come + * back and fill this in. + */ + md = ssl_handshake_md(s); + hashlen = EVP_MD_size(md); + memset(out, 0, hashlen); + + return hashlen; +} + +/* + * There isn't really a key block in TLSv1.3, but we still need this function + * for initialising the cipher and hash. + * + * Returns 1 on success or 0 on failure. + */ +int tls13_setup_key_block(SSL *s) +{ + const EVP_CIPHER *c; + const EVP_MD *hash; + int mac_type = NID_undef; + + s->session->cipher = s->s3->tmp.new_cipher; + if (!ssl_cipher_get_evp + (s->session, &c, &hash, &mac_type, NULL, NULL, 0)) { + SSLerr(SSL_F_TLS13_SETUP_KEY_BLOCK, SSL_R_CIPHER_OR_HASH_UNAVAILABLE); + return 0; + } + + s->s3->tmp.new_sym_enc = c; + s->s3->tmp.new_hash = hash; + + return 1; +} + const unsigned char client_handshake_traffic[] = "client handshake traffic secret"; const unsigned char client_application_traffic[] = diff --git a/test/tls13secretstest.c b/test/tls13secretstest.c index ceafd3d824..ccb8a12333 100644 --- a/test/tls13secretstest.c +++ b/test/tls13secretstest.c @@ -170,6 +170,14 @@ void RECORD_LAYER_reset_write_sequence(RECORD_LAYER *rl) { } +int ssl_cipher_get_evp(const SSL_SESSION *s, const EVP_CIPHER **enc, + const EVP_MD **md, int *mac_pkey_type, + size_t *mac_secret_size, SSL_COMP **comp, int use_etm) + +{ + return 0; +} + /* End of mocked out code */ static int test_secret(SSL *s, unsigned char *prk, |