From afce590b74159f7df1452fb2c4aa990a52536c38 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Mon, 23 Mar 2020 13:21:21 +0100 Subject: [PATCH] TLS: Temporarly downgrade newly generated EVP_PKEYs to legacy The transfer of TLS encodedpoint to backends isn't yet fully supported in provider implementations. This is a temporary measure so as not to get stuck in other development. Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/11358) --- ssl/statem/extensions_clnt.c | 32 ++++++++++++++++++++++++++++++++ ssl/statem/extensions_srvr.c | 30 ++++++++++++++++++++++++++++++ ssl/statem/statem_clnt.c | 30 ++++++++++++++++++++++++++++++ ssl/statem/statem_srvr.c | 28 ++++++++++++++++++++++++++++ 4 files changed, 120 insertions(+) diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c index 776473e659..82e333628f 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -644,6 +644,21 @@ static int add_key_share(SSL *s, WPACKET *pkt, unsigned int curve_id) /* SSLfatal() already called */ return 0; } + + /* + * TODO(3.0) Remove this when EVP_PKEY_get1_tls_encodedpoint() + * knows how to get a key from an encoded point with the help of + * a OSSL_SERIALIZER deserializer. We know that EVP_PKEY_get0() + * downgrades an EVP_PKEY to contain a legacy key. + * + * THIS IS TEMPORARY + */ + EVP_PKEY_get0(key_share_key); + if (EVP_PKEY_id(key_share_key) == EVP_PKEY_NONE) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_ADD_KEY_SHARE, + ERR_R_EC_LIB); + goto err; + } } /* Encode the public key. */ @@ -1906,6 +1921,23 @@ int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x, ERR_R_MALLOC_FAILURE); return 0; } + + /* + * TODO(3.0) Remove this when EVP_PKEY_get1_tls_encodedpoint() + * knows how to get a key from an encoded point with the help of + * a OSSL_SERIALIZER deserializer. We know that EVP_PKEY_get0() + * downgrades an EVP_PKEY to contain a legacy key. + * + * THIS IS TEMPORARY + */ + EVP_PKEY_get0(skey); + if (EVP_PKEY_id(skey) == EVP_PKEY_NONE) { + EVP_PKEY_free(skey); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_STOC_KEY_SHARE, + ERR_R_INTERNAL_ERROR); + return 0; + } + if (!EVP_PKEY_set1_tls_encodedpoint(skey, PACKET_data(&encoded_pt), PACKET_remaining(&encoded_pt))) { SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PARSE_STOC_KEY_SHARE, diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index 549a207430..bafd62a0db 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -711,6 +711,21 @@ int tls_parse_ctos_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x, return 0; } + /* + * TODO(3.0) Remove this when EVP_PKEY_get1_tls_encodedpoint() + * knows how to get a key from an encoded point with the help of + * a OSSL_SERIALIZER deserializer. We know that EVP_PKEY_get0() + * downgrades an EVP_PKEY to contain a legacy key. + * + * THIS IS TEMPORARY + */ + EVP_PKEY_get0(s->s3.peer_tmp); + if (EVP_PKEY_id(s->s3.peer_tmp) == EVP_PKEY_NONE) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_CTOS_KEY_SHARE, + ERR_R_INTERNAL_ERROR); + return 0; + } + s->s3.group_id = group_id; if (!EVP_PKEY_set1_tls_encodedpoint(s->s3.peer_tmp, @@ -1736,6 +1751,21 @@ EXT_RETURN tls_construct_stoc_key_share(SSL *s, WPACKET *pkt, return EXT_RETURN_FAIL; } + /* + * TODO(3.0) Remove this when EVP_PKEY_get1_tls_encodedpoint() + * knows how to get a key from an encoded point with the help of + * a OSSL_SERIALIZER deserializer. We know that EVP_PKEY_get0() + * downgrades an EVP_PKEY to contain a legacy key. + * + * THIS IS TEMPORARY + */ + EVP_PKEY_get0(skey); + if (EVP_PKEY_id(skey) == EVP_PKEY_NONE) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE, + ERR_R_INTERNAL_ERROR); + return EXT_RETURN_FAIL; + } + /* Generate encoding of server key */ encoded_pt_len = EVP_PKEY_get1_tls_encodedpoint(skey, &encodedPoint); if (encoded_pt_len == 0) { diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index cdd413d1ef..7878d7b499 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -2227,6 +2227,21 @@ static int tls_process_ske_ecdhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey) return 0; } + /* + * TODO(3.0) Remove this when EVP_PKEY_get1_tls_encodedpoint() + * knows how to get a key from an encoded point with the help of + * a OSSL_SERIALIZER deserializer. We know that EVP_PKEY_get0() + * downgrades an EVP_PKEY to contain a legacy key. + * + * THIS IS TEMPORARY + */ + EVP_PKEY_get0(s->s3.peer_tmp); + if (EVP_PKEY_id(s->s3.peer_tmp) == EVP_PKEY_NONE) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PROCESS_SKE_ECDHE, + ERR_R_INTERNAL_ERROR); + return 0; + } + if (!EVP_PKEY_set1_tls_encodedpoint(s->s3.peer_tmp, PACKET_data(&encoded_pt), PACKET_remaining(&encoded_pt))) { @@ -3129,6 +3144,21 @@ static int tls_construct_cke_ecdhe(SSL *s, WPACKET *pkt) goto err; } + /* + * TODO(3.0) Remove this when EVP_PKEY_get1_tls_encodedpoint() + * knows how to get a key from an encoded point with the help of + * a OSSL_SERIALIZER deserializer. We know that EVP_PKEY_get0() + * downgrades an EVP_PKEY to contain a legacy key. + * + * THIS IS TEMPORARY + */ + EVP_PKEY_get0(ckey); + if (EVP_PKEY_id(skey) == EVP_PKEY_NONE) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CKE_ECDHE, + ERR_R_INTERNAL_ERROR); + goto err; + } + if (ssl_derive(s, ckey, skey, 0) == 0) { /* SSLfatal() already called */ goto err; diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 43f9811163..1bab800ced 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -2624,6 +2624,18 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt) goto err; } + /* + * TODO(3.0) Remove this when EVP_PKEY_get1_tls_encodedpoint() + * knows how to get a key from an encoded point with the help of + * a OSSL_SERIALIZER deserializer. We know that EVP_PKEY_get0() + * downgrades an EVP_PKEY to contain a legacy key. + * + * THIS IS TEMPORARY + */ + EVP_PKEY_get0(s->s3.tmp.pkey); + if (EVP_PKEY_id(s->s3.tmp.pkey) == EVP_PKEY_NONE) + goto err; + /* Encode the public key. */ encodedlen = EVP_PKEY_get1_tls_encodedpoint(s->s3.tmp.pkey, &encodedPoint); @@ -3207,6 +3219,22 @@ static int tls_process_cke_ecdhe(SSL *s, PACKET *pkt) ERR_R_EVP_LIB); goto err; } + + /* + * TODO(3.0) Remove this when EVP_PKEY_get1_tls_encodedpoint() + * knows how to get a key from an encoded point with the help of + * a OSSL_SERIALIZER deserializer. We know that EVP_PKEY_get0() + * downgrades an EVP_PKEY to contain a legacy key. + * + * THIS IS TEMPORARY + */ + EVP_PKEY_get0(ckey); + if (EVP_PKEY_id(ckey) == EVP_PKEY_NONE) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PROCESS_CKE_ECDHE, + ERR_R_INTERNAL_ERROR); + goto err; + } + if (EVP_PKEY_set1_tls_encodedpoint(ckey, data, i) == 0) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PROCESS_CKE_ECDHE, ERR_R_EC_LIB); -- 2.39.2