From 2c5dfdc357046a4d6f40afb3311f7c3f06c58ebe Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Mon, 5 Dec 2016 17:04:51 +0000 Subject: [PATCH] Make CertificateVerify TLS1.3 aware Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/2157) --- include/openssl/ssl.h | 4 +- ssl/ssl_locl.h | 8 +++ ssl/statem/statem_clnt.c | 28 ++++++++- ssl/statem/statem_lib.c | 92 +++++++++++++++++++++++++--- ssl/statem/statem_srvr.c | 21 +++++++ test/recipes/70-test_tls13messages.t | 2 + 6 files changed, 142 insertions(+), 13 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index d57e680dc0..c6001c005d 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -857,7 +857,9 @@ typedef enum { TLS_ST_SW_CHANGE, TLS_ST_SW_FINISHED, TLS_ST_SW_ENCRYPTED_EXTENSIONS, - TLS_ST_CR_ENCRYPTED_EXTENSIONS + TLS_ST_CR_ENCRYPTED_EXTENSIONS, + TLS_ST_CR_CERT_VRFY, + TLS_ST_SW_CERT_VRFY } OSSL_HANDSHAKE_STATE; /* diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 641438dfe3..02666f063f 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -967,6 +967,14 @@ struct ssl_st { /* client cert? */ /* This is used to hold the server certificate used */ struct cert_st /* CERT */ *cert; + + /* + * The hash of all messages prior to the CertificateVerify, and the length + * of that hash. + */ + unsigned char cert_verify_hash[EVP_MAX_MD_SIZE]; + size_t cert_verify_hash_len; + /* * the session_id_context is used to ensure sessions are only reused in * the appropriate context diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index d0d384c14b..ff57e9217a 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -169,12 +169,18 @@ static int ossl_statem_client13_read_transition(SSL *s, int mt) break; case TLS_ST_CR_CERT: + if (mt == SSL3_MT_CERTIFICATE_VERIFY) { + st->hand_state = TLS_ST_CR_CERT_VRFY; + return 1; + } + break; + + case TLS_ST_CR_CERT_VRFY: if (mt == SSL3_MT_FINISHED) { st->hand_state = TLS_ST_CR_FINISHED; return 1; } break; - } /* No valid transition found */ @@ -562,6 +568,8 @@ WORK_STATE ossl_statem_client_pre_work(SSL *s, WORK_STATE wst) /* * Perform any work that needs to be done after sending a message from the * client to the server. + case TLS_ST_SR_CERT_VRFY: + return SSL3_RT_MAX_PLAIN_LENGTH; */ WORK_STATE ossl_statem_client_post_work(SSL *s, WORK_STATE wst) { @@ -730,6 +738,9 @@ size_t ossl_statem_client_max_message_size(SSL *s) case TLS_ST_CR_CERT: return s->max_cert_list; + case TLS_ST_CR_CERT_VRFY: + return SSL3_RT_MAX_PLAIN_LENGTH; + case TLS_ST_CR_CERT_STATUS: return SSL3_RT_MAX_PLAIN_LENGTH; @@ -784,6 +795,9 @@ MSG_PROCESS_RETURN ossl_statem_client_process_message(SSL *s, PACKET *pkt) case TLS_ST_CR_CERT: return tls_process_server_certificate(s, pkt); + case TLS_ST_CR_CERT_VRFY: + return tls_process_cert_verify(s, pkt); + case TLS_ST_CR_CERT_STATUS: return tls_process_cert_status(s, pkt); @@ -1497,8 +1511,18 @@ MSG_PROCESS_RETURN tls_process_server_certificate(SSL *s, PACKET *pkt) X509_up_ref(x); s->session->peer = x; s->session->verify_result = s->verify_result; - x = NULL; + + /* Save the current hash state for when we receive the CertificateVerify */ + if (SSL_IS_TLS13(s) + && !ssl_handshake_hash(s, s->cert_verify_hash, + sizeof(s->cert_verify_hash), + &s->cert_verify_hash_len)) { + al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_SERVER_CERTIFICATE, ERR_R_INTERNAL_ERROR); + goto f_err; + } + ret = MSG_PROCESS_CONTINUE_READING; goto done; diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index 0395e9f002..1a6c1fccf8 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -72,30 +72,99 @@ int tls_close_construct_packet(SSL *s, WPACKET *pkt, int htype) return 1; } +/* + * Size of the to-be-signed TLS13 data, without the hash size itself: + * 64 bytes of value 32, 33 context bytes, 1 byte separator + */ +#define TLS13_TBS_START_SIZE 64 +#define TLS13_TBS_PREAMBLE_SIZE (TLS13_TBS_START_SIZE + 33 + 1) + +static int get_cert_verify_tbs_data(SSL *s, unsigned char *tls13tbs, + void **hdata, size_t *hdatalen) +{ + static const char *servercontext = "TLS 1.3, server CertificateVerify"; + static const char *clientcontext = "TLS 1.3, client CertificateVerify"; + + if (SSL_IS_TLS13(s)) { + size_t hashlen; + + /* Set the first 64 bytes of to-be-signed data to octet 32 */ + memset(tls13tbs, 32, TLS13_TBS_START_SIZE); + /* This copies the 33 bytes of context plus the 0 separator byte */ + if (s->statem.hand_state == TLS_ST_CR_CERT_VRFY + || s->statem.hand_state == TLS_ST_SW_CERT_VRFY) + strcpy((char *)tls13tbs + TLS13_TBS_START_SIZE, servercontext); + else + strcpy((char *)tls13tbs + TLS13_TBS_START_SIZE, clientcontext); + + /* + * If we're currently reading then we need to use the saved handshake + * hash value. We can't use the current handshake hash state because + * that includes the CertVerify itself. + */ + if (s->statem.hand_state == TLS_ST_CR_CERT_VRFY + || s->statem.hand_state == TLS_ST_SR_CERT_VRFY) { + memcpy(tls13tbs + TLS13_TBS_PREAMBLE_SIZE, s->cert_verify_hash, + s->cert_verify_hash_len); + hashlen = s->cert_verify_hash_len; + } else if (!ssl_handshake_hash(s, tls13tbs + TLS13_TBS_PREAMBLE_SIZE, + EVP_MAX_MD_SIZE, &hashlen)) { + return 0; + } + + *hdata = tls13tbs; + *hdatalen = TLS13_TBS_PREAMBLE_SIZE + hashlen; + } else { + size_t retlen; + + retlen = BIO_get_mem_data(s->s3->handshake_buffer, hdata); + if (retlen <= 0) + return 0; + *hdatalen = retlen; + } + + return 1; +} + int tls_construct_cert_verify(SSL *s, WPACKET *pkt) { EVP_PKEY *pkey; - const EVP_MD *md = s->s3->tmp.md[s->cert->key - s->cert->pkeys]; + const EVP_MD *md; EVP_MD_CTX *mctx = NULL; unsigned u = 0; - long hdatalen = 0; + size_t hdatalen = 0; void *hdata; unsigned char *sig = NULL; + unsigned char tls13tbs[TLS13_TBS_PREAMBLE_SIZE + EVP_MAX_MD_SIZE]; + + if (s->server) { + /* Only happens in TLSv1.3 */ + /* + * TODO(TLS1.3): This needs to change. We should not get this from the + * cipher. However, for now, we have not done the work to separate the + * certificate type from the ciphersuite + */ + pkey = ssl_get_sign_pkey(s, s->s3->tmp.new_cipher, &md); + if (pkey == NULL) + goto err; + } else { + md = s->s3->tmp.md[s->cert->key - s->cert->pkeys]; + pkey = s->cert->key->privatekey; + } mctx = EVP_MD_CTX_new(); if (mctx == NULL) { SSLerr(SSL_F_TLS_CONSTRUCT_CERT_VERIFY, ERR_R_MALLOC_FAILURE); goto err; } - pkey = s->cert->key->privatekey; - hdatalen = BIO_get_mem_data(s->s3->handshake_buffer, &hdata); - if (hdatalen <= 0) { + /* Get the data to be signed */ + if (!get_cert_verify_tbs_data(s, tls13tbs, &hdata, &hdatalen)) { SSLerr(SSL_F_TLS_CONSTRUCT_CERT_VERIFY, ERR_R_INTERNAL_ERROR); goto err; } - if (SSL_USE_SIGALGS(s)&& !tls12_get_sigandhash(pkt, pkey, md)) { + if (SSL_USE_SIGALGS(s) && !tls12_get_sigandhash(pkt, pkey, md)) { SSLerr(SSL_F_TLS_CONSTRUCT_CERT_VERIFY, ERR_R_INTERNAL_ERROR); goto err; } @@ -158,8 +227,9 @@ MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt) unsigned int len; X509 *peer; const EVP_MD *md = NULL; - long hdatalen = 0; + size_t hdatalen = 0; void *hdata; + unsigned char tls13tbs[TLS13_TBS_PREAMBLE_SIZE + EVP_MAX_MD_SIZE]; EVP_MD_CTX *mctx = EVP_MD_CTX_new(); @@ -240,8 +310,7 @@ MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt) goto f_err; } - hdatalen = BIO_get_mem_data(s->s3->handshake_buffer, &hdata); - if (hdatalen <= 0) { + if (!get_cert_verify_tbs_data(s, tls13tbs, &hdata, &hdatalen)) { SSLerr(SSL_F_TLS_PROCESS_CERT_VERIFY, ERR_R_INTERNAL_ERROR); al = SSL_AD_INTERNAL_ERROR; goto f_err; @@ -288,7 +357,10 @@ MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt) goto f_err; } - ret = MSG_PROCESS_CONTINUE_PROCESSING; + if (SSL_IS_TLS13(s)) + ret = MSG_PROCESS_CONTINUE_READING; + else + ret = MSG_PROCESS_CONTINUE_PROCESSING; if (0) { 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 f29f0f3412..d08ed6f77b 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -427,6 +427,10 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s) return WRITE_TRAN_CONTINUE; case TLS_ST_SW_CERT: + st->hand_state = TLS_ST_SW_CERT_VRFY; + return WRITE_TRAN_CONTINUE; + + case TLS_ST_SW_CERT_VRFY: st->hand_state = TLS_ST_SW_FINISHED; return WRITE_TRAN_CONTINUE; @@ -826,6 +830,12 @@ int ossl_statem_server_construct_message(SSL *s, WPACKET *pkt, *mt = SSL3_MT_CERTIFICATE; break; + case TLS_ST_SW_CERT_VRFY: + *confunc = tls_construct_cert_verify; + *mt = SSL3_MT_CERTIFICATE_VERIFY; + break; + + case TLS_ST_SW_KEY_EXCH: *confunc = tls_construct_server_key_exchange; *mt = SSL3_MT_SERVER_KEY_EXCHANGE; @@ -3109,6 +3119,17 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt) * certificate, while we do include it in statem_clnt.c */ sk = NULL; + + /* Save the current hash state for when we receive the CertificateVerify */ + if (SSL_IS_TLS13(s) + && !ssl_handshake_hash(s, s->cert_verify_hash, + sizeof(s->cert_verify_hash), + &s->cert_verify_hash_len)) { + al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_CLIENT_CERTIFICATE, ERR_R_INTERNAL_ERROR); + goto f_err; + } + ret = MSG_PROCESS_CONTINUE_READING; goto done; diff --git a/test/recipes/70-test_tls13messages.t b/test/recipes/70-test_tls13messages.t index 8d42058853..d6512b57ad 100755 --- a/test/recipes/70-test_tls13messages.t +++ b/test/recipes/70-test_tls13messages.t @@ -43,6 +43,8 @@ $ENV{CTLOG_FILE} = srctop_file("test", "ct", "log_list.conf"); checkhandshake::CLIENT_AUTH_HANDSHAKE], [TLSProxy::Message::MT_CERTIFICATE, checkhandshake::ALL_HANDSHAKES & ~checkhandshake::RESUME_HANDSHAKE], + [TLSProxy::Message::MT_CERTIFICATE_VERIFY, + checkhandshake::ALL_HANDSHAKES & ~checkhandshake::RESUME_HANDSHAKE], [TLSProxy::Message::MT_FINISHED, checkhandshake::ALL_HANDSHAKES], [TLSProxy::Message::MT_CERTIFICATE, -- 2.39.5