Re-introduce legacy EVP_PKEY types for provided keys
authorRichard Levitte <levitte@openssl.org>
Fri, 22 May 2020 13:41:28 +0000 (15:41 +0200)
committerRichard Levitte <levitte@openssl.org>
Sat, 23 May 2020 19:11:09 +0000 (21:11 +0200)
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)

crypto/evp/p_lib.c
crypto/evp/pmeth_lib.c
include/crypto/evp.h
include/openssl/evp.h

index 9eb9f49..d05f0f2 100644 (file)
@@ -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| 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;
 
         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.
      */
      * 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;
         }
             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;
         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;
 
         /*
         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;
 #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
     ENGINE_finish(x->pmeth_engine);
     x->pmeth_engine = NULL;
 # endif
-    x->type = EVP_PKEY_NONE;
 }
 #endif  /* FIPS_MODULE */
 
 }
 #endif  /* FIPS_MODULE */
 
@@ -1472,6 +1469,7 @@ static void evp_pkey_free_it(EVP_PKEY *x)
         x->keymgmt = NULL;
         x->keydata = NULL;
     }
         x->keymgmt = NULL;
         x->keydata = NULL;
     }
+    x->type = EVP_PKEY_NONE;
 }
 
 void EVP_PKEY_free(EVP_PKEY *x)
 }
 
 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;
 {
     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;
 
     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;
     }
 
         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
     /*
      * 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
index eca5178..e4327b3 100644 (file)
@@ -155,10 +155,10 @@ static EVP_PKEY_CTX *int_ctx_new(OPENSSL_CTX *libctx,
         goto common;
 
     /*
         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;
         /* If we have an engine, something went wrong somewhere... */
         if (!ossl_assert(e == NULL))
             return NULL;
index ee4b622..d1756cf 100644 (file)
@@ -518,9 +518,25 @@ const EVP_CIPHER *EVP_##cname##_ecb(void) { return &cname##_ecb; }
  *     (type != EVP_PKEY_NONE && pkey.ptr != NULL)      ## legacy (libcrypto only)
  *     || (keymgmt != NULL && keydata != NULL)          ## provider side
  *
  *     (type != EVP_PKEY_NONE && pkey.ptr != NULL)      ## legacy (libcrypto only)
  *     || (keymgmt != NULL && keydata != NULL)          ## provider side
  *
- * The easiest way to detect a legacy key is:           type != EVP_PKEY_NONE
- * The easiest way to detect a provider side key is:    keymgmt != NULL
+ * The easiest way to detect a legacy key is:
+ *
+ *     keymgmt == NULL && type != EVP_PKEY_NONE
+ *
+ * The easiest way to detect a provider side key is:
+ *
+ *     keymgmt != NULL
  */
  */
+#define evp_pkey_is_blank(pk)                                   \
+    ((pk)->type == EVP_PKEY_NONE && (pk)->keymgmt == NULL)
+#define evp_pkey_is_typed(pk)                                   \
+    ((pk)->type != EVP_PKEY_NONE || (pk)->keymgmt != NULL)
+#define evp_pkey_is_assigned(pk)                                \
+    ((pk)->pkey.ptr != NULL || (pk)->keydata != NULL)
+#define evp_pkey_is_legacy(pk)                                  \
+    ((pk)->type != EVP_PKEY_NONE && (pk)->keymgmt == NULL)
+#define evp_pkey_is_provided(pk)                                \
+    ((pk)->keymgmt != NULL)
+
 struct evp_pkey_st {
     /* == Legacy attributes == */
     int type;
 struct evp_pkey_st {
     /* == Legacy attributes == */
     int type;
index ea305c2..0d5ce07 100644 (file)
@@ -72,6 +72,8 @@
 # define EVP_PKEY_ED25519 NID_ED25519
 # define EVP_PKEY_X448 NID_X448
 # define EVP_PKEY_ED448 NID_ED448
 # define EVP_PKEY_ED25519 NID_ED25519
 # define EVP_PKEY_X448 NID_X448
 # define EVP_PKEY_ED448 NID_ED448
+/* Special indicator that the object is uniquely provider side */
+# define EVP_PKEY_KEYMGMT -1
 
 #ifdef  __cplusplus
 extern "C" {
 
 #ifdef  __cplusplus
 extern "C" {