Fix HRR bug
If an HRR gets sent without a key_share (e.g. cookie only) then the code
fails when it should not.
@@ -530,32 +530,41 @@ int tls_construct_ctos_psk_kex_modes(SSL *s, WPACKET *pkt, unsigned int context,
#ifndef OPENSSL_NO_TLS1_3
static int add_key_share(SSL *s, WPACKET *pkt, unsigned int curve_id)
{
- unsigned char *encoded_point;
- EVP_PKEY *key_share_key;
+ unsigned char *encoded_point = NULL;
+ EVP_PKEY *key_share_key = NULL;
size_t encodedlen;
+ if (s->s3->tmp.pkey != NULL) {
+ assert(s->hello_retry_request);
+ if (!s->hello_retry_request) {
+ SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_INTERNAL_ERROR);
+ return 0;
+ }
+ /*
+ * Could happen if we got an HRR that wasn't requesting a new key_share
+ */
+ key_share_key = s->s3->tmp.pkey;
+ } else {
key_share_key = ssl_generate_pkey_curve(curve_id);
if (key_share_key == NULL) {
SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_EVP_LIB);
return 0;
}
+ }
/* Encode the public key. */
encodedlen = EVP_PKEY_get1_tls_encodedpoint(key_share_key,
&encoded_point);
if (encodedlen == 0) {
SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_EC_LIB);
- EVP_PKEY_free(key_share_key);
- return 0;
+ goto err;
}
/* Create KeyShareEntry */
if (!WPACKET_put_bytes_u16(pkt, curve_id)
|| !WPACKET_sub_memcpy_u16(pkt, encoded_point, encodedlen)) {
SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_INTERNAL_ERROR);
- EVP_PKEY_free(key_share_key);
- OPENSSL_free(encoded_point);
- return 0;
+ goto err;
}
/*
@@ -568,6 +577,11 @@ static int add_key_share(SSL *s, WPACKET *pkt, unsigned int curve_id)
OPENSSL_free(encoded_point);
return 1;
+ err:
+ if (s->s3->tmp.pkey == NULL)
+ EVP_PKEY_free(key_share_key);
+ OPENSSL_free(encoded_point);
+ return 0;
}
#endif
@@ -594,12 +608,6 @@ int tls_construct_ctos_key_share(SSL *s, WPACKET *pkt, unsigned int context,
return 0;
}
- if (s->s3->tmp.pkey != NULL) {
- /* Shouldn't happen! */
- SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, ERR_R_INTERNAL_ERROR);
- return 0;
- }
-
/*
* TODO(TLS1.3): Make the number of key_shares sent configurable. For
* now, just send one
