From 6400f338184b7acc94b8c21febdc68ec6f7fe3de Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 14 Sep 2016 11:10:37 +0100 Subject: [PATCH] Convert ClientVerify Construction to WPACKET Reviewed-by: Rich Salz --- ssl/ssl_locl.h | 4 +++- ssl/statem/statem_clnt.c | 51 +++++++++++++++++++++++++++------------- ssl/statem/statem_srvr.c | 2 +- ssl/t1_lib.c | 26 +++++++++++++++++++- 4 files changed, 64 insertions(+), 19 deletions(-) diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 26485f6570..841cb5ab42 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -2038,8 +2038,10 @@ __owur int tls_check_serverhello_tlsext_early(SSL *s, const PACKET *ext, const PACKET *session_id, SSL_SESSION **ret); -__owur int tls12_get_sigandhash(unsigned char *p, const EVP_PKEY *pk, +__owur int tls12_get_sigandhash(WPACKET *pkt, const EVP_PKEY *pk, const EVP_MD *md); +__owur int tls12_get_sigandhash_old(unsigned char *p, const EVP_PKEY *pk, + const EVP_MD *md); __owur int tls12_get_sigid(const EVP_PKEY *pk); __owur const EVP_MD *tls12_get_hash(unsigned char hash_alg); void ssl_set_sig_mask(uint32_t *pmask_a, SSL *s, int op); diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 703e6a664b..2b7809f483 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -2616,22 +2616,32 @@ int tls_client_key_exchange_post_work(SSL *s) int tls_construct_client_verify(SSL *s) { - unsigned char *p; EVP_PKEY *pkey; const EVP_MD *md = s->s3->tmp.md[s->cert->key - s->cert->pkeys]; EVP_MD_CTX *mctx; unsigned u = 0; - unsigned long n = 0; long hdatalen = 0; void *hdata; + unsigned char *sig = NULL; + WPACKET pkt; + + + if (!WPACKET_init(&pkt, s->init_buf)) { + /* Should not happen */ + SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_INTERNAL_ERROR); + goto err; + } + + if (!ssl_set_handshake_header2(s, &pkt, SSL3_MT_CERTIFICATE_VERIFY)) { + SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_INTERNAL_ERROR); + goto err; + } mctx = EVP_MD_CTX_new(); if (mctx == NULL) { SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_MALLOC_FAILURE); goto err; } - - p = ssl_handshake_start(s); pkey = s->cert->key->privatekey; hdatalen = BIO_get_mem_data(s->s3->handshake_buffer, &hdata); @@ -2639,24 +2649,25 @@ int tls_construct_client_verify(SSL *s) SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_INTERNAL_ERROR); goto err; } - if (SSL_USE_SIGALGS(s)) { - if (!tls12_get_sigandhash(p, pkey, md)) { - SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_INTERNAL_ERROR); - goto err; - } - p += 2; - n = 2; + if (SSL_USE_SIGALGS(s)&& !tls12_get_sigandhash(&pkt, pkey, md)) { + SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_INTERNAL_ERROR); + goto err; } #ifdef SSL_DEBUG fprintf(stderr, "Using client alg %s\n", EVP_MD_name(md)); #endif + sig = OPENSSL_malloc(EVP_PKEY_size(pkey)); + if (sig == NULL) { + SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_MALLOC_FAILURE); + goto err; + } if (!EVP_SignInit_ex(mctx, md, NULL) || !EVP_SignUpdate(mctx, hdata, hdatalen) || (s->version == SSL3_VERSION && !EVP_MD_CTX_ctrl(mctx, EVP_CTRL_SSL3_MASTER_SECRET, s->session->master_key_length, s->session->master_key)) - || !EVP_SignFinal(mctx, p + 2, &u, pkey)) { + || !EVP_SignFinal(mctx, sig, &u, pkey)) { SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_EVP_LIB); goto err; } @@ -2666,24 +2677,32 @@ int tls_construct_client_verify(SSL *s) if (pktype == NID_id_GostR3410_2001 || pktype == NID_id_GostR3410_2012_256 || pktype == NID_id_GostR3410_2012_512) - BUF_reverse(p + 2, NULL, u); + BUF_reverse(sig, NULL, u); } #endif - s2n(u, p); - n += u + 2; + if (!WPACKET_sub_memcpy_u16(&pkt, sig, u)) { + SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_INTERNAL_ERROR); + goto err; + } + /* Digest cached records and discard handshake buffer */ if (!ssl3_digest_cached_records(s, 0)) goto err; - if (!ssl_set_handshake_header(s, SSL3_MT_CERTIFICATE_VERIFY, n)) { + + if (!ssl_close_construct_packet(s, &pkt)) { SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_INTERNAL_ERROR); goto err; } + OPENSSL_free(sig); EVP_MD_CTX_free(mctx); return 1; err: + WPACKET_cleanup(&pkt); + OPENSSL_free(sig); EVP_MD_CTX_free(mctx); + ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); return 0; } diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 818f48d2e3..7536e6ef15 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -1918,7 +1918,7 @@ int tls_construct_server_key_exchange(SSL *s) if (md) { /* send signature algorithm */ if (SSL_USE_SIGALGS(s)) { - if (!tls12_get_sigandhash(p, pkey, md)) { + if (!tls12_get_sigandhash_old(p, pkey, md)) { /* Should never happen */ al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 4f5ea42b5a..177fa6ef69 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -3119,7 +3119,31 @@ static int tls12_find_nid(int id, const tls12_lookup *table, size_t tlen) return NID_undef; } -int tls12_get_sigandhash(unsigned char *p, const EVP_PKEY *pk, const EVP_MD *md) +int tls12_get_sigandhash(WPACKET *pkt, const EVP_PKEY *pk, const EVP_MD *md) +{ + int sig_id, md_id; + if (!md) + return 0; + md_id = tls12_find_id(EVP_MD_type(md), tls12_md, OSSL_NELEM(tls12_md)); + if (md_id == -1) + return 0; + sig_id = tls12_get_sigid(pk); + if (sig_id == -1) + return 0; + if (!WPACKET_put_bytes(pkt, md_id, 1) || !WPACKET_put_bytes(pkt, sig_id, 1)) + return 0; + + return 1; +} + +/* + * Old version of the tls12_get_sigandhash function used by code that has not + * yet been converted to WPACKET yet. It will be deleted once WPACKET conversion + * is complete. + * TODO - DELETE ME + */ +int tls12_get_sigandhash_old(unsigned char *p, const EVP_PKEY *pk, + const EVP_MD *md) { int sig_id, md_id; if (!md) -- 2.25.1