Re-introduce legacy EVP_PKEY types for provided keys

EVP_PKEYs with provider side internal keys got the key type
EVP_PKEY_NONE.  This turned out to be too disruptive, so we try
instead to find a matching EVP_PKEY_ASN1_METHOD and use whatever
EVP_PKEY type it uses.

To make internal coding easier, we introduce a few internal macros to
distinguish what can be expected from a EVP_PKEY:

- evp_pkey_is_blank(), to detect an unassigned EVP_PKEY.
- evp_pkey_is_typed(), to detect that an EVP_PKEY has been assigned a
  type, which may be an old style type number or a EVP_KEYMGMT method.
- evp_pkey_is_assigned(), to detect that an EVP_PKEY has been assigned
  an key value.
- evp_pkey_is_legacy(), to detect that the internal EVP_PKEY key is a
  legacy one, i.e. will be handled via an EVP_PKEY_ASN1_METHOD and an
  EVP_PKEY_METHOD.
- evp_pkey_is_provided(), to detect that the internal EVP_PKEY key is
  a provider side one, i.e. will be handdled via an EVP_KEYMGMT and
  other provider methods.

This also introduces EVP_PKEY_KEYMGMT, to indicate that this EVP_PKEY
contains a provider side key for which there are no known
EVP_PKEY_ASN1_METHODs or EVP_PKEY_METHODs, i.e. these can only be
handled via EVP_KEYMGMT and other provider methods.

Fixes #11823

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/11913)

2 files changed, 33 insertions, 34 deletions

diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c
index 9eb9f4937b..d05f0f2cba 100644
--- a/crypto/evp/p_lib.c
+++ b/crypto/evp/p_lib.c
@@ -119,7 +119,7 @@ int EVP_PKEY_copy_parameters(EVP_PKEY *to, const EVP_PKEY *from)
      * If |to| is a legacy key and |from| isn't, we must downgrade |from|.
      * If that fails, this function fails.
      */
-    if (to->type != EVP_PKEY_NONE && from->keymgmt != NULL)
+    if (evp_pkey_is_legacy(to) && evp_pkey_is_provided(from))
         if (!evp_pkey_downgrade((EVP_PKEY *)from))
             return 0;
 
@@ -135,15 +135,15 @@ int EVP_PKEY_copy_parameters(EVP_PKEY *to, const EVP_PKEY *from)
      * like evp_keymgmt_util_copy() and evp_pkey_export_to_provider() called
      * further down help us find out if they are the same or not.
      */
-    if (to->type == EVP_PKEY_NONE && to->keymgmt == NULL) {
-        if (from->type != EVP_PKEY_NONE) {
+    if (evp_pkey_is_blank(to)) {
+        if (evp_pkey_is_legacy(from)) {
             if (EVP_PKEY_set_type(to, from->type) == 0)
                 return 0;
         } else {
             if (EVP_PKEY_set_type_by_keymgmt(to, from->keymgmt) == 0)
                 return 0;
         }
-    } else if (to->type != EVP_PKEY_NONE) {
+    } else if (evp_pkey_is_legacy(to)) {
         if (to->type != from->type) {
             EVPerr(EVP_F_EVP_PKEY_COPY_PARAMETERS, EVP_R_DIFFERENT_KEY_TYPES);
             goto err;
@@ -1357,19 +1357,17 @@ static int pkey_set_type(EVP_PKEY *pkey, ENGINE *e, int type, const char *str,
         pkey->engine = e;
 
         /*
-         * The EVP_PKEY_ASN1_METHOD |pkey_id| serves different purposes,
-         * depending on if we're setting this key to contain a legacy or
-         * a provider side "origin" key.  For a legacy key, we assign it
-         * to the |type| field, but for a provider side key, we assign it
-         * to the |save_type| field, because |type| is supposed to be set
-         * to EVP_PKEY_NONE in that case.
+         * The EVP_PKEY_ASN1_METHOD |pkey_id| retains its legacy key purpose
+         * for any key type that has a legacy implementation, regardless of
+         * if the internal key is a legacy or a provider side one.  When
+         * there is no legacy implementation for the key, the type becomes
+         * EVP_PKEY_KEYMGMT, which indicates that one should be cautious
+         * with functions that expect legacy internal keys.
          */
-        if (ameth != NULL) {
-            if (keymgmt != NULL)
-                pkey->save_type = ameth->pkey_id;
-            else if (pkey->ameth != NULL)
-                pkey->type = ameth->pkey_id;
-        }
+        if (ameth != NULL)
+            pkey->type = ameth->pkey_id;
+        else
+            pkey->type = EVP_PKEY_KEYMGMT;
 #endif
     }
     return 1;
@@ -1453,7 +1451,6 @@ void evp_pkey_free_legacy(EVP_PKEY *x)
     ENGINE_finish(x->pmeth_engine);
     x->pmeth_engine = NULL;
 # endif
-    x->type = EVP_PKEY_NONE;
 }
 #endif  /* FIPS_MODULE */
 
@@ -1472,6 +1469,7 @@ static void evp_pkey_free_it(EVP_PKEY *x)
         x->keymgmt = NULL;
         x->keydata = NULL;
     }
+    x->type = EVP_PKEY_NONE;
 }
 
 void EVP_PKEY_free(EVP_PKEY *x)
@@ -1661,32 +1659,33 @@ int evp_pkey_downgrade(EVP_PKEY *pk)
 {
     EVP_KEYMGMT *keymgmt = pk->keymgmt;
     void *keydata = pk->keydata;
-    int type = pk->save_type;
+    int type = pk->type;
     const char *keytype = NULL;
 
     /* If this isn't a provider side key, we're done */
     if (keymgmt == NULL)
         return 1;
 
-    /* Get the key type name for error reporting */
-    if (type != EVP_PKEY_NONE)
-        keytype = OBJ_nid2sn(type);
-    else
-        keytype =
-            evp_first_name(EVP_KEYMGMT_provider(keymgmt), keymgmt->name_id);
+    keytype = evp_first_name(EVP_KEYMGMT_provider(keymgmt), keymgmt->name_id);
 
     /*
-     * |save_type| was set when any of the EVP_PKEY_set_type functions
-     * was called.  It was set to EVP_PKEY_NONE if the key type wasn't
-     * recognised to be any of the legacy key types, and the downgrade
-     * isn't possible.
+     * If the type is EVP_PKEY_NONE, then we have a problem somewhere else
+     * in our code.  If it's not one of the well known EVP_PKEY_xxx values,
+     * it should at least be EVP_PKEY_KEYMGMT at this point.
+     * TODO(3.0) remove this check when we're confident that the rest of the
+     * code treats this correctly.
      */
-    if (type == EVP_PKEY_NONE) {
-        ERR_raise_data(ERR_LIB_EVP, EVP_R_UNKNOWN_KEY_TYPE,
-                       "key type = %s, can't downgrade", keytype);
+    if (!ossl_assert(type != EVP_PKEY_NONE)) {
+        ERR_raise_data(ERR_LIB_EVP, ERR_R_INTERNAL_ERROR,
+                       "keymgmt key type = %s but legacy type = EVP_PKEY_NONE",
+                       keytype);
         return 0;
     }
 
+    /* Prefer the legacy key type name for error reporting */
+    if (type != EVP_PKEY_KEYMGMT)
+        keytype = OBJ_nid2sn(type);
+
     /*
      * To be able to downgrade, we steal the provider side "origin" keymgmt
      * and keydata.  We've already grabbed the pointers, so all we need to
diff --git a/crypto/evp/pmeth_lib.c b/crypto/evp/pmeth_lib.c
index eca5178129..e4327b3a94 100644
--- a/crypto/evp/pmeth_lib.c
+++ b/crypto/evp/pmeth_lib.c
@@ -155,10 +155,10 @@ static EVP_PKEY_CTX *int_ctx_new(OPENSSL_CTX *libctx,
         goto common;
 
     /*
-     * If the key doesn't contain anything legacy, then it must be provided,
-     * so we extract the necessary information and use that.
+     * If the internal key is provided, we extract the keytype from its
+     * keymgmt and skip over the legacy code.
      */
-    if (pkey != NULL && pkey->type == EVP_PKEY_NONE) {
+    if (pkey != NULL && evp_pkey_is_provided(pkey)) {
         /* If we have an engine, something went wrong somewhere... */
         if (!ossl_assert(e == NULL))
             return NULL;