From: jsing Date: Fri, 7 Jan 2022 15:46:30 +0000 (+0000) Subject: Convert legacy server to tls_key_share. X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=aebe267242e9806e5eb28d3c38fca8c3cc3b6154;p=openbsd Convert legacy server to tls_key_share. This requires a few more additions to the DHE key share code - we need to be able to either set the DHE parameters or specify the number of key bits for use with auto DHE parameters. Additionally, we need to be able to serialise the DHE parameters to send to the client. This removes the infamous 'tmp' struct from ssl3_state_internal_st. ok inoguchi@ tb@ --- diff --git a/lib/libssl/s3_lib.c b/lib/libssl/s3_lib.c index 54261c575a2..899432e947d 100644 --- a/lib/libssl/s3_lib.c +++ b/lib/libssl/s3_lib.c @@ -1,4 +1,4 @@ -/* $OpenBSD: s3_lib.c,v 1.221 2022/01/06 18:23:56 jsing Exp $ */ +/* $OpenBSD: s3_lib.c,v 1.222 2022/01/07 15:46:30 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -1565,10 +1565,6 @@ ssl3_free(SSL *s) ssl3_release_write_buffer(s); freezero(S3I(s)->hs.sigalgs, S3I(s)->hs.sigalgs_len); - DH_free(S3I(s)->tmp.dh); - EC_KEY_free(S3I(s)->tmp.ecdh); - freezero(S3I(s)->tmp.x25519, X25519_KEY_LENGTH); - tls_key_share_free(S3I(s)->hs.key_share); tls13_secrets_destroy(S3I(s)->hs.tls13.secrets); @@ -1601,14 +1597,6 @@ ssl3_clear(SSL *s) sk_X509_pop_free(s->internal->verified_chain, X509_free); s->internal->verified_chain = NULL; - DH_free(S3I(s)->tmp.dh); - S3I(s)->tmp.dh = NULL; - EC_KEY_free(S3I(s)->tmp.ecdh); - S3I(s)->tmp.ecdh = NULL; - S3I(s)->tmp.ecdh_nid = NID_undef; - freezero(S3I(s)->tmp.x25519, X25519_KEY_LENGTH); - S3I(s)->tmp.x25519 = NULL; - freezero(S3I(s)->hs.sigalgs, S3I(s)->hs.sigalgs_len); S3I(s)->hs.sigalgs = NULL; S3I(s)->hs.sigalgs_len = 0; diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h index 83b40d2dd3a..cc7b342247d 100644 --- a/lib/libssl/ssl_locl.h +++ b/lib/libssl/ssl_locl.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_locl.h,v 1.374 2022/01/06 18:23:56 jsing Exp $ */ +/* $OpenBSD: ssl_locl.h,v 1.375 2022/01/07 15:46:30 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -1168,15 +1168,6 @@ typedef struct ssl3_state_internal_st { SSL_HANDSHAKE hs; - struct { - DH *dh; - - EC_KEY *ecdh; /* holds short lived ECDH key */ - int ecdh_nid; - - uint8_t *x25519; - } tmp; - /* Connection binding to prevent renegotiation attacks */ unsigned char previous_client_finished[EVP_MAX_MD_SIZE]; unsigned char previous_client_finished_len; diff --git a/lib/libssl/ssl_srvr.c b/lib/libssl/ssl_srvr.c index 0496985351b..b66a2c108dd 100644 --- a/lib/libssl/ssl_srvr.c +++ b/lib/libssl/ssl_srvr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_srvr.c,v 1.130 2022/01/04 12:53:31 jsing Exp $ */ +/* $OpenBSD: ssl_srvr.c,v 1.131 2022/01/07 15:46:30 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -1309,23 +1309,23 @@ ssl3_send_server_done(SSL *s) static int ssl3_send_server_kex_dhe(SSL *s, CBB *cbb) { - DH *dh = NULL; - int al; + int nid = NID_dhKeyAgreement; - if ((dh = DH_new()) == NULL) + tls_key_share_free(S3I(s)->hs.key_share); + if ((S3I(s)->hs.key_share = tls_key_share_new_nid(nid)) == NULL) goto err; if (s->cert->dh_tmp_auto != 0) { size_t key_bits; if ((key_bits = ssl_dhe_params_auto_key_bits(s)) == 0) { - al = SSL_AD_INTERNAL_ERROR; SSLerror(s, ERR_R_INTERNAL_ERROR); - goto fatal_err; - } - - if (!ssl_kex_generate_dhe_params_auto(dh, key_bits)) + ssl3_send_alert(s, SSL3_AL_FATAL, + SSL_AD_INTERNAL_ERROR); goto err; + } + tls_key_share_set_key_bits(S3I(s)->hs.key_share, + key_bits); } else { DH *dh_params = s->cert->dh_tmp; @@ -1334,157 +1334,69 @@ ssl3_send_server_kex_dhe(SSL *s, CBB *cbb) SSL_C_PKEYLENGTH(S3I(s)->hs.cipher)); if (dh_params == NULL) { - al = SSL_AD_HANDSHAKE_FAILURE; SSLerror(s, SSL_R_MISSING_TMP_DH_KEY); - goto fatal_err; + ssl3_send_alert(s, SSL3_AL_FATAL, + SSL_AD_HANDSHAKE_FAILURE); + goto err; } - if (!ssl_kex_generate_dhe(dh, dh_params)) + if (!tls_key_share_set_dh_params(S3I(s)->hs.key_share, dh_params)) goto err; } - if (!ssl_kex_params_dhe(dh, cbb)) - goto err; - if (!ssl_kex_public_dhe(dh, cbb)) + if (!tls_key_share_generate(S3I(s)->hs.key_share)) goto err; - if (S3I(s)->tmp.dh != NULL) { - SSLerror(s, ERR_R_INTERNAL_ERROR); + if (!tls_key_share_params(S3I(s)->hs.key_share, cbb)) + goto err; + if (!tls_key_share_public(S3I(s)->hs.key_share, cbb)) goto err; - } - S3I(s)->tmp.dh = dh; return 1; - fatal_err: - ssl3_send_alert(s, SSL3_AL_FATAL, al); err: - DH_free(dh); - return 0; } static int -ssl3_send_server_kex_ecdhe_ecp(SSL *s, int nid, CBB *cbb) +ssl3_send_server_kex_ecdhe(SSL *s, CBB *cbb) { - uint16_t curve_id; - EC_KEY *ecdh; - CBB ecpoint; - int al; + CBB public; + int nid; - /* - * Only named curves are supported in ECDH ephemeral key exchanges. - * For supported named curves, curve_id is non-zero. - */ - if ((curve_id = tls1_ec_nid2curve_id(nid)) == 0) { + if ((nid = tls1_get_shared_curve(s)) == NID_undef) { SSLerror(s, SSL_R_UNSUPPORTED_ELLIPTIC_CURVE); + ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE); goto err; } - if (S3I(s)->tmp.ecdh != NULL) { - SSLerror(s, ERR_R_INTERNAL_ERROR); + tls_key_share_free(S3I(s)->hs.key_share); + if ((S3I(s)->hs.key_share = tls_key_share_new_nid(nid)) == NULL) goto err; - } - if ((S3I(s)->tmp.ecdh = EC_KEY_new()) == NULL) { - al = SSL_AD_HANDSHAKE_FAILURE; - SSLerror(s, SSL_R_MISSING_TMP_ECDH_KEY); - goto fatal_err; - } - S3I(s)->tmp.ecdh_nid = nid; - ecdh = S3I(s)->tmp.ecdh; - - if (!ssl_kex_generate_ecdhe_ecp(ecdh, nid)) + if (!tls_key_share_generate(S3I(s)->hs.key_share)) goto err; /* - * Encode the public key. - * - * Only named curves are supported in ECDH ephemeral key exchanges. - * In this case the ServerKeyExchange message has: - * [1 byte CurveType], [2 byte CurveName] - * [1 byte length of encoded point], followed by - * the actual encoded point itself. + * ECC key exchange - see RFC 8422, section 5.4. */ if (!CBB_add_u8(cbb, NAMED_CURVE_TYPE)) goto err; - if (!CBB_add_u16(cbb, curve_id)) + if (!CBB_add_u16(cbb, tls_key_share_group(S3I(s)->hs.key_share))) goto err; - if (!CBB_add_u8_length_prefixed(cbb, &ecpoint)) + if (!CBB_add_u8_length_prefixed(cbb, &public)) goto err; - if (!ssl_kex_public_ecdhe_ecp(ecdh, &ecpoint)) + if (!tls_key_share_public(S3I(s)->hs.key_share, &public)) goto err; if (!CBB_flush(cbb)) goto err; return 1; - fatal_err: - ssl3_send_alert(s, SSL3_AL_FATAL, al); err: return 0; } -static int -ssl3_send_server_kex_ecdhe_ecx(SSL *s, int nid, CBB *cbb) -{ - uint8_t *public_key = NULL, *private_key = NULL; - uint16_t curve_id; - CBB ecpoint; - int ret = 0; - - /* Generate an X25519 key pair. */ - if (S3I(s)->tmp.x25519 != NULL) { - SSLerror(s, ERR_R_INTERNAL_ERROR); - goto err; - } - if ((private_key = malloc(X25519_KEY_LENGTH)) == NULL) - goto err; - if ((public_key = malloc(X25519_KEY_LENGTH)) == NULL) - goto err; - X25519_keypair(public_key, private_key); - - /* Serialize public key. */ - if ((curve_id = tls1_ec_nid2curve_id(nid)) == 0) { - SSLerror(s, SSL_R_UNSUPPORTED_ELLIPTIC_CURVE); - goto err; - } - - if (!CBB_add_u8(cbb, NAMED_CURVE_TYPE)) - goto err; - if (!CBB_add_u16(cbb, curve_id)) - goto err; - if (!CBB_add_u8_length_prefixed(cbb, &ecpoint)) - goto err; - if (!CBB_add_bytes(&ecpoint, public_key, X25519_KEY_LENGTH)) - goto err; - if (!CBB_flush(cbb)) - goto err; - - S3I(s)->tmp.x25519 = private_key; - private_key = NULL; - ret = 1; - - err: - free(public_key); - freezero(private_key, X25519_KEY_LENGTH); - - return ret; -} - -static int -ssl3_send_server_kex_ecdhe(SSL *s, CBB *cbb) -{ - int nid; - - nid = tls1_get_shared_curve(s); - - if (nid == NID_X25519) - return ssl3_send_server_kex_ecdhe_ecx(s, nid, cbb); - - return ssl3_send_server_kex_ecdhe_ecp(s, nid, cbb); -} - int ssl3_send_server_key_exchange(SSL *s) { @@ -1791,147 +1703,73 @@ ssl3_get_client_kex_rsa(SSL *s, CBS *cbs) static int ssl3_get_client_kex_dhe(SSL *s, CBS *cbs) { - DH *dh_clnt = NULL; - DH *dh_srvr; - int invalid_key; uint8_t *key = NULL; size_t key_len = 0; + int invalid_key; int ret = 0; - if ((dh_srvr = S3I(s)->tmp.dh) == NULL) { + if (S3I(s)->hs.key_share == NULL) { ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE); SSLerror(s, SSL_R_MISSING_TMP_DH_KEY); goto err; } - if ((dh_clnt = DHparams_dup(dh_srvr)) == NULL) - goto err; - - if (!ssl_kex_peer_public_dhe(dh_clnt, cbs, &invalid_key)) { - ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); - SSLerror(s, SSL_R_BAD_PACKET_LENGTH); + if (!tls_key_share_peer_public(S3I(s)->hs.key_share, cbs, + &invalid_key)) goto err; - } if (invalid_key) { ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); SSLerror(s, SSL_R_BAD_DH_PUB_KEY_LENGTH); goto err; } - if (!ssl_kex_derive_dhe(dh_srvr, dh_clnt, &key, &key_len)) + if (!tls_key_share_derive(S3I(s)->hs.key_share, &key, &key_len)) goto err; if (!tls12_derive_master_secret(s, key, key_len)) goto err; - DH_free(S3I(s)->tmp.dh); - S3I(s)->tmp.dh = NULL; - ret = 1; err: freezero(key, key_len); - DH_free(dh_clnt); return ret; } static int -ssl3_get_client_kex_ecdhe_ecp(SSL *s, CBS *cbs) +ssl3_get_client_kex_ecdhe(SSL *s, CBS *cbs) { uint8_t *key = NULL; size_t key_len = 0; - EC_KEY *ecdh_peer = NULL; - EC_KEY *ecdh; CBS public; int ret = 0; - /* - * Use the ephemeral values we saved when generating the - * ServerKeyExchange message. - */ - if ((ecdh = S3I(s)->tmp.ecdh) == NULL) { - SSLerror(s, ERR_R_INTERNAL_ERROR); + if (S3I(s)->hs.key_share == NULL) { + ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE); + SSLerror(s, SSL_R_MISSING_TMP_DH_KEY); goto err; } - /* - * Get client's public key from encoded point in the ClientKeyExchange - * message. - */ if (!CBS_get_u8_length_prefixed(cbs, &public)) goto err; - if (CBS_len(cbs) != 0) + if (!tls_key_share_peer_public(S3I(s)->hs.key_share, &public, NULL)) goto err; - if ((ecdh_peer = EC_KEY_new()) == NULL) + if (!tls_key_share_derive(S3I(s)->hs.key_share, &key, &key_len)) goto err; - if (!ssl_kex_peer_public_ecdhe_ecp(ecdh_peer, S3I(s)->tmp.ecdh_nid, - &public)) - goto err; - - /* Derive the shared secret and compute master secret. */ - if (!ssl_kex_derive_ecdhe_ecp(ecdh, ecdh_peer, &key, &key_len)) - goto err; if (!tls12_derive_master_secret(s, key, key_len)) goto err; - EC_KEY_free(S3I(s)->tmp.ecdh); - S3I(s)->tmp.ecdh = NULL; - S3I(s)->tmp.ecdh_nid = NID_undef; - ret = 1; err: freezero(key, key_len); - EC_KEY_free(ecdh_peer); return ret; } -static int -ssl3_get_client_kex_ecdhe_ecx(SSL *s, CBS *cbs) -{ - uint8_t *shared_key = NULL; - CBS ecpoint; - int ret = 0; - - if (!CBS_get_u8_length_prefixed(cbs, &ecpoint)) - goto err; - if (CBS_len(cbs) != 0) - goto err; - if (CBS_len(&ecpoint) != X25519_KEY_LENGTH) - goto err; - - if ((shared_key = malloc(X25519_KEY_LENGTH)) == NULL) - goto err; - if (!X25519(shared_key, S3I(s)->tmp.x25519, CBS_data(&ecpoint))) - goto err; - - freezero(S3I(s)->tmp.x25519, X25519_KEY_LENGTH); - S3I(s)->tmp.x25519 = NULL; - - if (!tls12_derive_master_secret(s, shared_key, X25519_KEY_LENGTH)) - goto err; - - ret = 1; - - err: - freezero(shared_key, X25519_KEY_LENGTH); - - return ret; -} - -static int -ssl3_get_client_kex_ecdhe(SSL *s, CBS *cbs) -{ - if (S3I(s)->tmp.x25519 != NULL) - return ssl3_get_client_kex_ecdhe_ecx(s, cbs); - - return ssl3_get_client_kex_ecdhe_ecp(s, cbs); -} - static int ssl3_get_client_kex_gost(SSL *s, CBS *cbs) { diff --git a/lib/libssl/tls13_server.c b/lib/libssl/tls13_server.c index 739ef066099..c32ae227794 100644 --- a/lib/libssl/tls13_server.c +++ b/lib/libssl/tls13_server.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tls13_server.c,v 1.90 2022/01/05 17:10:02 jsing Exp $ */ +/* $OpenBSD: tls13_server.c,v 1.91 2022/01/07 15:46:30 jsing Exp $ */ /* * Copyright (c) 2019, 2020 Joel Sing * Copyright (c) 2020 Bob Beck @@ -360,8 +360,8 @@ tls13_server_engage_record_protection(struct tls13_ctx *ctx) SSL *s = ctx->ssl; int ret = 0; - if (!tls_key_share_derive(ctx->hs->key_share, - &shared_key, &shared_key_len)) + if (!tls_key_share_derive(ctx->hs->key_share, &shared_key, + &shared_key_len)) goto err; s->session->cipher = ctx->hs->cipher; diff --git a/lib/libssl/tls_internal.h b/lib/libssl/tls_internal.h index 7e2beadeacf..f7f939215ad 100644 --- a/lib/libssl/tls_internal.h +++ b/lib/libssl/tls_internal.h @@ -1,4 +1,4 @@ -/* $OpenBSD: tls_internal.h,v 1.3 2022/01/06 18:23:56 jsing Exp $ */ +/* $OpenBSD: tls_internal.h,v 1.4 2022/01/07 15:46:30 jsing Exp $ */ /* * Copyright (c) 2018, 2019, 2021 Joel Sing * @@ -18,6 +18,7 @@ #ifndef HEADER_TLS_INTERNAL_H #define HEADER_TLS_INTERNAL_H +#include #include #include "bytestring.h" @@ -64,8 +65,11 @@ void tls_key_share_free(struct tls_key_share *ks); uint16_t tls_key_share_group(struct tls_key_share *ks); int tls_key_share_nid(struct tls_key_share *ks); +void tls_key_share_set_key_bits(struct tls_key_share *ks, size_t key_bits); +int tls_key_share_set_dh_params(struct tls_key_share *ks, DH *dh_params); int tls_key_share_peer_pkey(struct tls_key_share *ks, EVP_PKEY *pkey); int tls_key_share_generate(struct tls_key_share *ks); +int tls_key_share_params(struct tls_key_share *ks, CBB *cbb); int tls_key_share_public(struct tls_key_share *ks, CBB *cbb); int tls_key_share_peer_params(struct tls_key_share *ks, CBS *cbs, int *invalid_params); diff --git a/lib/libssl/tls_key_share.c b/lib/libssl/tls_key_share.c index 6e390f4a24e..eb30a0ea694 100644 --- a/lib/libssl/tls_key_share.c +++ b/lib/libssl/tls_key_share.c @@ -1,6 +1,6 @@ -/* $OpenBSD: tls_key_share.c,v 1.2 2022/01/06 18:23:56 jsing Exp $ */ +/* $OpenBSD: tls_key_share.c,v 1.3 2022/01/07 15:46:30 jsing Exp $ */ /* - * Copyright (c) 2020 Joel Sing + * Copyright (c) 2020, 2021 Joel Sing * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -18,7 +18,9 @@ #include #include +#include #include +#include #include "bytestring.h" #include "ssl_locl.h" @@ -27,6 +29,7 @@ struct tls_key_share { int nid; uint16_t group_id; + size_t key_bits; DH *dhe; DH *dhe_peer; @@ -108,6 +111,28 @@ tls_key_share_nid(struct tls_key_share *ks) return ks->nid; } +void +tls_key_share_set_key_bits(struct tls_key_share *ks, size_t key_bits) +{ + ks->key_bits = key_bits; +} + +int +tls_key_share_set_dh_params(struct tls_key_share *ks, DH *dh_params) +{ + if (ks->nid != NID_dhKeyAgreement) + return 0; + if (ks->dhe != NULL || ks->dhe_peer != NULL) + return 0; + + if ((ks->dhe = DHparams_dup(dh_params)) == NULL) + return 0; + if ((ks->dhe_peer = DHparams_dup(dh_params)) == NULL) + return 0; + + return 1; +} + int tls_key_share_peer_pkey(struct tls_key_share *ks, EVP_PKEY *pkey) { @@ -126,10 +151,28 @@ tls_key_share_peer_pkey(struct tls_key_share *ks, EVP_PKEY *pkey) static int tls_key_share_generate_dhe(struct tls_key_share *ks) { - if (ks->dhe == NULL) + /* + * If auto params are not being used then we must already have DH + * parameters set. + */ + if (ks->key_bits == 0) { + if (ks->dhe == NULL) + return 0; + + return ssl_kex_generate_dhe(ks->dhe, ks->dhe); + } + + if (ks->dhe != NULL || ks->dhe_peer != NULL) + return 0; + + if ((ks->dhe = DH_new()) == NULL) + return 0; + if (!ssl_kex_generate_dhe_params_auto(ks->dhe, ks->key_bits)) + return 0; + if ((ks->dhe_peer = DHparams_dup(ks->dhe)) == NULL) return 0; - return ssl_kex_generate_dhe(ks->dhe, ks->dhe); + return 1; } static int @@ -199,6 +242,24 @@ tls_key_share_generate(struct tls_key_share *ks) return tls_key_share_generate_ecdhe_ecp(ks); } +static int +tls_key_share_params_dhe(struct tls_key_share *ks, CBB *cbb) +{ + if (ks->dhe == NULL) + return 0; + + return ssl_kex_params_dhe(ks->dhe, cbb); +} + +int +tls_key_share_params(struct tls_key_share *ks, CBB *cbb) +{ + if (ks->nid == NID_dhKeyAgreement) + return tls_key_share_params_dhe(ks, cbb); + + return 0; +} + static int tls_key_share_public_dhe(struct tls_key_share *ks, CBB *cbb) {