From: tb Date: Sun, 11 Jun 2023 19:01:01 +0000 (+0000) Subject: Convert legacy server kex to one-shot sign/verify X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=bf68ec7f06ba6d068be0a3e0457d5bdca62fba40;p=openbsd Convert legacy server kex to one-shot sign/verify This converts ssl3_{get,send}_server_key_exchange() to EVP_DigestVerify() and EVP_DigestSign(). In order to do this, build the full signed_params up front and rework the way the key exchange parameters are constructed. This way we can do the verify and sign steps in one go and at the same use a more idiomatic approach with CBB/CBS. with/ok jsing --- diff --git a/lib/libssl/ssl_clnt.c b/lib/libssl/ssl_clnt.c index 2ab90b5c37b..6aea5901324 100644 --- a/lib/libssl/ssl_clnt.c +++ b/lib/libssl/ssl_clnt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_clnt.c,v 1.159 2023/06/11 18:50:51 tb Exp $ */ +/* $OpenBSD: ssl_clnt.c,v 1.160 2023/06/11 19:01:01 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -1299,13 +1299,17 @@ ssl3_get_server_kex_ecdhe(SSL *s, CBS *cbs) static int ssl3_get_server_key_exchange(SSL *s) { - CBS cbs, signature; + CBB cbb; + CBS cbs, params, signature; EVP_MD_CTX *md_ctx; - const unsigned char *param; - size_t param_len; + unsigned char *signed_params = NULL; + size_t signed_params_len; + size_t params_len; long alg_k, alg_a; int al, ret; + memset(&cbb, 0, sizeof(cbb)); + alg_k = s->s3->hs.cipher->algorithm_mkey; alg_a = s->s3->hs.cipher->algorithm_auth; @@ -1341,8 +1345,14 @@ ssl3_get_server_key_exchange(SSL *s) return (1); } - param = CBS_data(&cbs); - param_len = CBS_len(&cbs); + if (!CBB_init(&cbb, 0)) + goto err; + if (!CBB_add_bytes(&cbb, s->s3->client_random, SSL3_RANDOM_SIZE)) + goto err; + if (!CBB_add_bytes(&cbb, s->s3->server_random, SSL3_RANDOM_SIZE)) + goto err; + + CBS_dup(&cbs, ¶ms); if (alg_k & SSL_kDHE) { if (!ssl3_get_server_kex_dhe(s, &cbs)) @@ -1356,7 +1366,12 @@ ssl3_get_server_key_exchange(SSL *s) goto fatal_err; } - param_len -= CBS_len(&cbs); + if ((params_len = CBS_offset(&cbs)) > CBS_len(¶ms)) + goto err; + if (!CBB_add_bytes(&cbb, CBS_data(¶ms), params_len)) + goto err; + if (!CBB_finish(&cbb, &signed_params, &signed_params_len)) + goto err; /* if it was signed, check the signature */ if ((alg_a & SSL_aNULL) == 0) { @@ -1400,21 +1415,13 @@ ssl3_get_server_key_exchange(SSL *s) if (!EVP_DigestVerifyInit(md_ctx, &pctx, sigalg->md(), NULL, pkey)) goto err; - if (!EVP_DigestVerifyUpdate(md_ctx, s->s3->client_random, - SSL3_RANDOM_SIZE)) - goto err; if ((sigalg->flags & SIGALG_FLAG_RSA_PSS) && (!EVP_PKEY_CTX_set_rsa_padding(pctx, RSA_PKCS1_PSS_PADDING) || !EVP_PKEY_CTX_set_rsa_pss_saltlen(pctx, -1))) goto err; - if (!EVP_DigestVerifyUpdate(md_ctx, s->s3->server_random, - SSL3_RANDOM_SIZE)) - goto err; - if (!EVP_DigestVerifyUpdate(md_ctx, param, param_len)) - goto err; - if (EVP_DigestVerifyFinal(md_ctx, CBS_data(&signature), - CBS_len(&signature)) <= 0) { + if (EVP_DigestVerify(md_ctx, CBS_data(&signature), + CBS_len(&signature), signed_params, signed_params_len) <= 0) { al = SSL_AD_DECRYPT_ERROR; SSLerror(s, SSL_R_BAD_SIGNATURE); goto fatal_err; @@ -1428,6 +1435,7 @@ ssl3_get_server_key_exchange(SSL *s) } EVP_MD_CTX_free(md_ctx); + free(signed_params); return (1); @@ -1439,7 +1447,9 @@ ssl3_get_server_key_exchange(SSL *s) ssl3_send_alert(s, SSL3_AL_FATAL, al); err: + CBB_cleanup(&cbb); EVP_MD_CTX_free(md_ctx); + free(signed_params); return (-1); } diff --git a/lib/libssl/ssl_srvr.c b/lib/libssl/ssl_srvr.c index d0814a8455e..8edbf77156e 100644 --- a/lib/libssl/ssl_srvr.c +++ b/lib/libssl/ssl_srvr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_srvr.c,v 1.154 2023/06/11 18:50:51 tb Exp $ */ +/* $OpenBSD: ssl_srvr.c,v 1.155 2023/06/11 19:01:01 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -1431,12 +1431,13 @@ ssl3_send_server_kex_ecdhe(SSL *s, CBB *cbb) static int ssl3_send_server_key_exchange(SSL *s) { - CBB cbb, cbb_params, cbb_signature, server_kex; + CBB cbb, cbb_signature, cbb_signed_params, server_kex; + CBS params; const struct ssl_sigalg *sigalg = NULL; + unsigned char *signed_params = NULL; + size_t signed_params_len; unsigned char *signature = NULL; size_t signature_len = 0; - unsigned char *params = NULL; - size_t params_len; const EVP_MD *md = NULL; unsigned long type; EVP_MD_CTX *md_ctx = NULL; @@ -1445,7 +1446,7 @@ ssl3_send_server_key_exchange(SSL *s) int al; memset(&cbb, 0, sizeof(cbb)); - memset(&cbb_params, 0, sizeof(cbb_params)); + memset(&cbb_signed_params, 0, sizeof(cbb_signed_params)); if ((md_ctx = EVP_MD_CTX_new()) == NULL) goto err; @@ -1456,15 +1457,26 @@ ssl3_send_server_key_exchange(SSL *s) SSL3_MT_SERVER_KEY_EXCHANGE)) goto err; - if (!CBB_init(&cbb_params, 0)) + if (!CBB_init(&cbb_signed_params, 0)) goto err; + if (!CBB_add_bytes(&cbb_signed_params, s->s3->client_random, + SSL3_RANDOM_SIZE)) { + SSLerror(s, ERR_R_INTERNAL_ERROR); + goto err; + } + if (!CBB_add_bytes(&cbb_signed_params, s->s3->server_random, + SSL3_RANDOM_SIZE)) { + SSLerror(s, ERR_R_INTERNAL_ERROR); + goto err; + } + type = s->s3->hs.cipher->algorithm_mkey; if (type & SSL_kDHE) { - if (!ssl3_send_server_kex_dhe(s, &cbb_params)) + if (!ssl3_send_server_kex_dhe(s, &cbb_signed_params)) goto err; } else if (type & SSL_kECDHE) { - if (!ssl3_send_server_kex_ecdhe(s, &cbb_params)) + if (!ssl3_send_server_kex_ecdhe(s, &cbb_signed_params)) goto err; } else { al = SSL_AD_HANDSHAKE_FAILURE; @@ -1472,10 +1484,16 @@ ssl3_send_server_key_exchange(SSL *s) goto fatal_err; } - if (!CBB_finish(&cbb_params, ¶ms, ¶ms_len)) + if (!CBB_finish(&cbb_signed_params, &signed_params, + &signed_params_len)) + goto err; + + CBS_init(¶ms, signed_params, signed_params_len); + if (!CBS_skip(¶ms, 2 * SSL3_RANDOM_SIZE)) goto err; - if (!CBB_add_bytes(&server_kex, params, params_len)) + if (!CBB_add_bytes(&server_kex, CBS_data(¶ms), + CBS_len(¶ms))) goto err; /* Add signature unless anonymous. */ @@ -1507,22 +1525,8 @@ ssl3_send_server_key_exchange(SSL *s) SSLerror(s, ERR_R_EVP_LIB); goto err; } - if (!EVP_DigestSignUpdate(md_ctx, s->s3->client_random, - SSL3_RANDOM_SIZE)) { - SSLerror(s, ERR_R_EVP_LIB); - goto err; - } - if (!EVP_DigestSignUpdate(md_ctx, s->s3->server_random, - SSL3_RANDOM_SIZE)) { - SSLerror(s, ERR_R_EVP_LIB); - goto err; - } - if (!EVP_DigestSignUpdate(md_ctx, params, params_len)) { - SSLerror(s, ERR_R_EVP_LIB); - goto err; - } - if (!EVP_DigestSignFinal(md_ctx, NULL, &signature_len) || - !signature_len) { + if (!EVP_DigestSign(md_ctx, NULL, &signature_len, + signed_params, signed_params_len)) { SSLerror(s, ERR_R_EVP_LIB); goto err; } @@ -1530,7 +1534,8 @@ ssl3_send_server_key_exchange(SSL *s) SSLerror(s, ERR_R_MALLOC_FAILURE); goto err; } - if (!EVP_DigestSignFinal(md_ctx, signature, &signature_len)) { + if (!EVP_DigestSign(md_ctx, signature, &signature_len, + signed_params, signed_params_len)) { SSLerror(s, ERR_R_EVP_LIB); goto err; } @@ -1550,19 +1555,19 @@ ssl3_send_server_key_exchange(SSL *s) } EVP_MD_CTX_free(md_ctx); - free(params); free(signature); + free(signed_params); return (ssl3_handshake_write(s)); fatal_err: ssl3_send_alert(s, SSL3_AL_FATAL, al); err: - CBB_cleanup(&cbb_params); + CBB_cleanup(&cbb_signed_params); CBB_cleanup(&cbb); EVP_MD_CTX_free(md_ctx); - free(params); free(signature); + free(signed_params); return (-1); }