EVP_FETCH: remove the need to transport the legacy NID through construction
authorRichard Levitte <levitte@openssl.org>
Wed, 8 May 2019 12:00:31 +0000 (14:00 +0200)
committerRichard Levitte <levitte@openssl.org>
Sun, 12 May 2019 20:43:38 +0000 (13:43 -0700)
Now that the legacy NID isn't used as a main index for fetched
algorithms, the legacy NID was just transported around unnecessarily.
This is removed, and the legacy NID is simply set by EVP_{API}_fetch()
after the construction process is done.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8878)

crypto/evp/digest.c
crypto/evp/evp_enc.c
crypto/evp/evp_fetch.c
crypto/evp/evp_locl.h
doc/internal/man3/evp_generic_fetch.pod

index 9b10a7f..dc50528 100644 (file)
@@ -494,13 +494,14 @@ int EVP_MD_CTX_ctrl(EVP_MD_CTX *ctx, int cmd, int p1, void *p2)
     return 0;
 }
 
-static void *evp_md_from_dispatch(int mdtype, const OSSL_DISPATCH *fns,
-                                    OSSL_PROVIDER *prov)
+static void *evp_md_from_dispatch(const OSSL_DISPATCH *fns,
+                                  OSSL_PROVIDER *prov)
 {
     EVP_MD *md = NULL;
     int fncnt = 0;
 
-    if ((md = EVP_MD_meth_new(mdtype, NID_undef)) == NULL)
+    /* EVP_MD_fetch() will set the legacy NID if available */
+    if ((md = EVP_MD_meth_new(NID_undef, NID_undef)) == NULL)
         return NULL;
 
     for (; fns->function_id != 0; fns++) {
@@ -587,17 +588,25 @@ static void evp_md_free(void *md)
     EVP_MD_meth_free(md);
 }
 
-static int evp_md_nid(void *vmd)
-{
-    EVP_MD *md = vmd;
-
-    return md->type;
-}
-
 EVP_MD *EVP_MD_fetch(OPENSSL_CTX *ctx, const char *algorithm,
                      const char *properties)
 {
-    return evp_generic_fetch(ctx, OSSL_OP_DIGEST, algorithm, properties,
-                             evp_md_from_dispatch, evp_md_upref,
-                             evp_md_free, evp_md_nid);
+    EVP_MD *md =
+        evp_generic_fetch(ctx, OSSL_OP_DIGEST, algorithm, properties,
+                          evp_md_from_dispatch, evp_md_upref,
+                          evp_md_free);
+
+#ifndef FIPS_MODE
+    /* TODO(3.x) get rid of the need for legacy NIDs */
+    if (md != NULL) {
+        /*
+         * FIPS module note: since internal fetches will be entirely
+         * provider based, we know that none of its code depends on legacy
+         * NIDs or any functionality that use them.
+         */
+        md->type = OBJ_sn2nid(algorithm);
+    }
+#endif
+
+    return md;
 }
index 29b707a..c05d48e 100644 (file)
@@ -1044,13 +1044,17 @@ int EVP_CIPHER_CTX_copy(EVP_CIPHER_CTX *out, const EVP_CIPHER_CTX *in)
     return 1;
 }
 
-static void *evp_cipher_from_dispatch(int nid, const OSSL_DISPATCH *fns,
+static void *evp_cipher_from_dispatch(const OSSL_DISPATCH *fns,
                                       OSSL_PROVIDER *prov)
 {
     EVP_CIPHER *cipher = NULL;
     int fnciphcnt = 0, fnctxcnt = 0;
 
-    if ((cipher = EVP_CIPHER_meth_new(nid, 0, 0)) == NULL)
+    /*
+     * The legacy NID is set by EVP_CIPHER_fetch() if the name exists in
+     * the object database.
+     */
+    if ((cipher = EVP_CIPHER_meth_new(0, 0, 0)) == NULL)
         return NULL;
 
     for (; fns->function_id != 0; fns++) {
@@ -1167,17 +1171,25 @@ static void evp_cipher_free(void *cipher)
     EVP_CIPHER_meth_free(cipher);
 }
 
-static int evp_cipher_nid(void *vcipher)
-{
-    EVP_CIPHER *cipher = vcipher;
-
-    return cipher->nid;
-}
-
 EVP_CIPHER *EVP_CIPHER_fetch(OPENSSL_CTX *ctx, const char *algorithm,
                              const char *properties)
 {
-    return evp_generic_fetch(ctx, OSSL_OP_CIPHER, algorithm, properties,
-                             evp_cipher_from_dispatch, evp_cipher_upref,
-                             evp_cipher_free, evp_cipher_nid);
+    EVP_CIPHER *cipher =
+        evp_generic_fetch(ctx, OSSL_OP_CIPHER, algorithm, properties,
+                          evp_cipher_from_dispatch, evp_cipher_upref,
+                          evp_cipher_free);
+
+#ifndef FIPS_MODE
+    /* TODO(3.x) get rid of the need for legacy NIDs */
+    if (cipher != NULL) {
+        /*
+         * FIPS module note: since internal fetches will be entirely
+         * provider based, we know that none of its code depends on legacy
+         * NIDs or any functionality that use them.
+         */
+        cipher->nid = OBJ_sn2nid(algorithm);
+    }
+#endif
+
+    return cipher;
 }
index dc66a69..fdd6209 100644 (file)
@@ -39,13 +39,11 @@ static const OPENSSL_CTX_METHOD default_method_store_method = {
 struct method_data_st {
     OPENSSL_CTX *libctx;
     const char *name;
-    int nid;
+    int id;
     OSSL_METHOD_CONSTRUCT_METHOD *mcm;
-    void *(*method_from_dispatch)(int nid, const OSSL_DISPATCH *,
-                                  OSSL_PROVIDER *);
+    void *(*method_from_dispatch)(const OSSL_DISPATCH *, OSSL_PROVIDER *);
     int (*refcnt_up_method)(void *method);
     void (*destruct_method)(void *method);
-    int (*nid_method)(void *method);
 };
 
 /*
@@ -121,10 +119,8 @@ static void *construct_method(const char *name, const OSSL_DISPATCH *fns,
                               OSSL_PROVIDER *prov, void *data)
 {
     struct method_data_st *methdata = data;
-    /* TODO(3.0) get rid of the need for legacy NIDs */
-    int legacy_nid = OBJ_sn2nid(name);
 
-    return methdata->method_from_dispatch(legacy_nid, fns, prov);
+    return methdata->method_from_dispatch(fns, prov);
 }
 
 static void destruct_method(void *method, void *data)
@@ -136,11 +132,10 @@ static void destruct_method(void *method, void *data)
 
 void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id,
                         const char *name, const char *properties,
-                        void *(*new_method)(int nid, const OSSL_DISPATCH *fns,
+                        void *(*new_method)(const OSSL_DISPATCH *fns,
                                             OSSL_PROVIDER *prov),
                         int (*upref_method)(void *),
-                        void (*free_method)(void *),
-                        int (*nid_method)(void *))
+                        void (*free_method)(void *))
 {
     OSSL_METHOD_STORE *store = get_default_method_store(libctx);
     OSSL_NAMEMAP *namemap = ossl_namemap_stored(libctx);
@@ -168,7 +163,6 @@ void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id,
         mcmdata.destruct_method = free_method;
         mcmdata.refcnt_up_method = upref_method;
         mcmdata.destruct_method = free_method;
-        mcmdata.nid_method = nid_method;
         method = ossl_method_construct(libctx, operation_id, name,
                                        properties, 0 /* !force_cache */,
                                        &mcm, &mcmdata);
index 0bb6d15..8876b06 100644 (file)
@@ -91,8 +91,7 @@ int is_partially_overlapping(const void *ptr1, const void *ptr2, int len);
 
 void *evp_generic_fetch(OPENSSL_CTX *ctx, int operation_id,
                         const char *algorithm, const char *properties,
-                        void *(*new_method)(int nid, const OSSL_DISPATCH *fns,
+                        void *(*new_method)(const OSSL_DISPATCH *fns,
                                             OSSL_PROVIDER *prov),
                         int (*upref_method)(void *),
-                        void (*free_method)(void *),
-                        int (*nid_method)(void *));
+                        void (*free_method)(void *));
index 8b0f542..2679a7e 100644 (file)
@@ -11,11 +11,10 @@ evp_generic_fetch - generic algorithm fetcher and method creator for EVP
 
  void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id,
                          const char *name, const char *properties,
-                         void *(*new_method)(int nid, const OSSL_DISPATCH *fns,
+                         void *(*new_method)(const OSSL_DISPATCH *fns,
                                              OSSL_PROVIDER *prov),
                          int (*upref_method)(void *),
-                         void (*free_method)(void *),
-                         int (*nid_method)(void *));
+                         void (*free_method)(void *));
 
 =head1 DESCRIPTION
 
@@ -42,10 +41,6 @@ one.
 
 frees the given method.
 
-=item nid_method()
-
-returns the nid associated with the given method.
-
 =back
 
 =head1 RETURN VALUES
@@ -80,7 +75,6 @@ And here's the implementation of the FOO method fetcher:
     /* typedef struct evp_foo_st EVP_FOO */
     struct evp_foo_st {
         OSSL_PROVIDER *prov;
-        int nid;
        CRYPTO_REF_COUNT refcnt;
         OSSL_OP_foo_newctx_fn *newctx;
         OSSL_OP_foo_init_fn *init;
@@ -93,7 +87,7 @@ And here's the implementation of the FOO method fetcher:
      * In this example, we have a public method creator and destructor.
      * It's not absolutely necessary, but is in the spirit of OpenSSL.
      */
-    EVP_FOO *EVP_FOO_meth_from_dispatch(int foo_type, const OSSL_DISPATCH *fns,
+    EVP_FOO *EVP_FOO_meth_from_dispatch(const OSSL_DISPATCH *fns,
                                         OSSL_PROVIDER *prov)
     {
         EVP_FOO *foo = NULL;
@@ -120,7 +114,6 @@ And here's the implementation of the FOO method fetcher:
                 break;
             }
         }
-        foo->nid = foo_type;
         foo->prov = prov;
         if (prov)
             ossl_provider_upref(prov);
@@ -138,10 +131,10 @@ And here's the implementation of the FOO method fetcher:
         }
     }
 
-    static void *foo_from_dispatch(int nid, const OSSL_DISPATCH *fns,
+    static void *foo_from_dispatch(const OSSL_DISPATCH *fns,
                                    OSSL_PROVIDER *prov)
     {
-        return EVP_FOO_meth_from_dispatch(nid, fns, prov);
+        return EVP_FOO_meth_from_dispatch(fns, prov);
     }
 
     static int foo_upref(void *vfoo)
@@ -162,8 +155,18 @@ And here's the implementation of the FOO method fetcher:
                            const char *name,
                            const char *properties)
     {
-        return evp_generic_fetch(ctx, OSSL_OP_FOO, name, properties,
-                                 foo_from_dispatch, foo_upref, foo_free);
+        EVP_FOO *foo =
+            evp_generic_fetch(ctx, OSSL_OP_FOO, name, properties,
+                              foo_from_dispatch, foo_upref, foo_free);
+
+        /*
+         * If this method exists in legacy form, with a constant NID for the
+         * given |name|, this is the spot to find that NID and set it in
+         * the newly constructed EVP_FOO instance.
+         */
+
+        return foo;
+
     }
 
 And finally, the library functions: