Don't downgrade keys in libssl

We were downgrading to legacy keys at various points in libssl in
order to get or set an encoded point. Now that the encoded point
functions work with provided keys this is no longer necessary.

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/11898)

5 files changed, 0 insertions, 131 deletions

diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index 764c52322d..ab2d98de60 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -648,21 +648,6 @@ 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. */
@@ -1926,22 +1911,6 @@ int tls_parse_stoc_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(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 aa71cec7e9..3a0fee6ebc 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -715,21 +715,6 @@ 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,
@@ -1757,21 +1742,6 @@ 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 67d8ae8ce6..7189940a62 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -2231,21 +2231,6 @@ 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))) {
@@ -3148,21 +3133,6 @@ 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 e5340b4e7f..036bfadbe5 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -2636,20 +2636,6 @@ 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) {
-            SSLfatal(s, SSL_AD_INTERNAL_ERROR, 0, ERR_R_EC_LIB);
-            goto err;
-        }
-
         /* Encode the public key. */
         encodedlen = EVP_PKEY_get1_tls_encodedpoint(s->s3.tmp.pkey,
                                                     &encodedPoint);
@@ -3234,21 +3220,6 @@ static int tls_process_cke_ecdhe(SSL *s, PACKET *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(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);
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index a59d992e47..68bd5f2611 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1217,17 +1217,6 @@ int tls12_check_peer_sigalg(SSL *s, uint16_t sig, EVP_PKEY *pkey)
     const SIGALG_LOOKUP *lu;
     int secbits = 0;
 
-    /*
-     * TODO(3.0) Remove this when we adapted this function for provider
-     * side keys.  We know that EVP_PKEY_get0() downgrades an EVP_PKEY
-     * to contain a legacy key.
-     *
-     * THIS IS TEMPORARY
-     */
-    EVP_PKEY_get0(pkey);
-    if (EVP_PKEY_id(pkey) == EVP_PKEY_NONE)
-        return 0;
-
     pkeyid = EVP_PKEY_id(pkey);
     /* Should never happen */
     if (pkeyid == -1)