Remove gen_get_params & gen_gettable_params from keygen operation
authorShane Lontis <shane.lontis@oracle.com>
Thu, 7 May 2020 05:31:05 +0000 (15:31 +1000)
committerShane Lontis <shane.lontis@oracle.com>
Thu, 7 May 2020 05:31:05 +0000 (15:31 +1000)
EVP_PKEY_CTX_gettable_params() was missing code for the keygen operation.
After adding it it was noticed that it is probably not required for this type, so instead
the gen_get_params and gen_gettable_params have been remnoved from the provider interface.
gen_get_params was only implemented for ec to get the curve name. This seems redundant
since normally you would set parameters into the keygen_init() and then generate a key.
Normally you would expect to extract data from the key - not the object that we just set up
to do the keygen.

Added a simple settable and gettable test into a test that does keygen.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/11683)

crypto/evp/evp_local.h
crypto/evp/keymgmt_meth.c
crypto/evp/pmeth_gn.c
crypto/evp/pmeth_lib.c
doc/man7/provider-keymgmt.pod
include/crypto/evp.h
include/openssl/core_numbers.h
providers/implementations/keymgmt/ec_kmgmt.c
test/dsatest.c

index 4c822aa7d3fcb7fa07cc876311f0bd70234ddd5c..de7d69a26cb61e1da068ac24c00a3b8386134237 100644 (file)
@@ -86,8 +86,6 @@ struct evp_keymgmt_st {
     OSSL_OP_keymgmt_gen_set_template_fn *gen_set_template;
     OSSL_OP_keymgmt_gen_set_params_fn *gen_set_params;
     OSSL_OP_keymgmt_gen_settable_params_fn *gen_settable_params;
-    OSSL_OP_keymgmt_gen_get_params_fn *gen_get_params;
-    OSSL_OP_keymgmt_gen_gettable_params_fn *gen_gettable_params;
     OSSL_OP_keymgmt_gen_fn *gen;
     OSSL_OP_keymgmt_gen_cleanup_fn *gen_cleanup;
 
index 172dd21a719020650f336a69a7dae4cebd4a0867..b75d02f13625c7320f1d9128fb2c876535b7df98 100644 (file)
@@ -39,7 +39,7 @@ static void *keymgmt_from_dispatch(int name_id,
 {
     EVP_KEYMGMT *keymgmt = NULL;
     int setparamfncnt = 0, getparamfncnt = 0;
-    int setgenparamfncnt = 0, getgenparamfncnt = 0;
+    int setgenparamfncnt = 0;
     int importfncnt = 0, exportfncnt = 0;
 
     if ((keymgmt = keymgmt_new()) == NULL) {
@@ -77,20 +77,6 @@ static void *keymgmt_from_dispatch(int name_id,
                     OSSL_get_OP_keymgmt_gen_settable_params(fns);
             }
             break;
-        case OSSL_FUNC_KEYMGMT_GEN_GET_PARAMS:
-            if (keymgmt->gen_get_params == NULL) {
-                getgenparamfncnt++;
-                keymgmt->gen_get_params =
-                    OSSL_get_OP_keymgmt_gen_get_params(fns);
-            }
-            break;
-        case OSSL_FUNC_KEYMGMT_GEN_GETTABLE_PARAMS:
-            if (keymgmt->gen_gettable_params == NULL) {
-                getgenparamfncnt++;
-                keymgmt->gen_gettable_params =
-                    OSSL_get_OP_keymgmt_gen_gettable_params(fns);
-            }
-            break;
         case OSSL_FUNC_KEYMGMT_GEN:
             if (keymgmt->gen == NULL)
                 keymgmt->gen = OSSL_get_OP_keymgmt_gen(fns);
@@ -186,7 +172,6 @@ static void *keymgmt_from_dispatch(int name_id,
         || (getparamfncnt != 0 && getparamfncnt != 2)
         || (setparamfncnt != 0 && setparamfncnt != 2)
         || (setgenparamfncnt != 0 && setgenparamfncnt != 2)
-        || (getgenparamfncnt != 0 && getgenparamfncnt != 2)
         || (importfncnt != 0 && importfncnt != 2)
         || (exportfncnt != 0 && exportfncnt != 2)
         || (keymgmt->gen != NULL
@@ -342,23 +327,6 @@ const OSSL_PARAM *evp_keymgmt_gen_settable_params(const EVP_KEYMGMT *keymgmt)
     return keymgmt->gen_settable_params(provctx);
 }
 
-int evp_keymgmt_gen_get_params(const EVP_KEYMGMT *keymgmt, void *genctx,
-                               OSSL_PARAM params[])
-{
-    if (keymgmt->gen_get_params == NULL)
-        return 0;
-    return keymgmt->gen_get_params(genctx, params);
-}
-
-const OSSL_PARAM *evp_keymgmt_gen_gettable_params(const EVP_KEYMGMT *keymgmt)
-{
-    void *provctx = ossl_provider_ctx(EVP_KEYMGMT_provider(keymgmt));
-
-    if (keymgmt->gen_gettable_params == NULL)
-        return NULL;
-    return keymgmt->gen_gettable_params(provctx);
-}
-
 void *evp_keymgmt_gen(const EVP_KEYMGMT *keymgmt, void *genctx,
                       OSSL_CALLBACK *cb, void *cbarg)
 {
index dc1dad86ba9044af5506ea92f097da8500880ee3..a775d2bee73323f6c66114060f3df34e02b9e743 100644 (file)
@@ -210,8 +210,9 @@ int EVP_PKEY_gen(EVP_PKEY_CTX *ctx, EVP_PKEY **ppkey)
     {
         char curve_name[OSSL_MAX_NAME_SIZE] = "";
 
-        if (EVP_PKEY_CTX_get_ec_paramgen_curve_name(ctx, curve_name,
-                                                    sizeof(curve_name)) < 1
+        if (!EVP_PKEY_get_utf8_string_param(*ppkey, OSSL_PKEY_PARAM_EC_NAME,
+                                            curve_name, sizeof(curve_name),
+                                            NULL)
             || strcmp(curve_name, "SM2") != 0)
             goto end;
     }
index e262e873194d414174e287dc5ca7eb7c42f8864b..3476d83ea6fc8df523a42e4ed53b79db3a0a5d7f 100644 (file)
@@ -613,12 +613,6 @@ int EVP_PKEY_CTX_get_params(EVP_PKEY_CTX *ctx, OSSL_PARAM *params)
             && ctx->op.ciph.cipher->get_ctx_params != NULL)
         return ctx->op.ciph.cipher->get_ctx_params(ctx->op.ciph.ciphprovctx,
                                                    params);
-    if (EVP_PKEY_CTX_IS_GEN_OP(ctx)
-        && ctx->op.keymgmt.genctx != NULL
-        && ctx->keymgmt != NULL
-        && ctx->keymgmt->gen_get_params != NULL)
-        return evp_keymgmt_gen_get_params(ctx->keymgmt, ctx->op.keymgmt.genctx,
-                                          params);
     return 0;
 }
 
@@ -632,12 +626,10 @@ const OSSL_PARAM *EVP_PKEY_CTX_gettable_params(EVP_PKEY_CTX *ctx)
             && ctx->op.sig.signature != NULL
             && ctx->op.sig.signature->gettable_ctx_params != NULL)
         return ctx->op.sig.signature->gettable_ctx_params();
-
     if (EVP_PKEY_CTX_IS_ASYM_CIPHER_OP(ctx)
             && ctx->op.ciph.cipher != NULL
             && ctx->op.ciph.cipher->gettable_ctx_params != NULL)
         return ctx->op.ciph.cipher->gettable_ctx_params();
-
     return NULL;
 }
 
@@ -656,8 +648,7 @@ const OSSL_PARAM *EVP_PKEY_CTX_settable_params(EVP_PKEY_CTX *ctx)
             && ctx->op.ciph.cipher->settable_ctx_params != NULL)
         return ctx->op.ciph.cipher->settable_ctx_params();
     if (EVP_PKEY_CTX_IS_GEN_OP(ctx)
-            && ctx->keymgmt != NULL
-            && ctx->keymgmt->gen_settable_params != NULL)
+            && ctx->keymgmt != NULL)
         return evp_keymgmt_gen_settable_params(ctx->keymgmt);
 
     return NULL;
index 0669585b1a1e2db41400cd8841fb549c262a555e..4202a77b54d5711e04ed3634f5145513d4f37f17 100644 (file)
@@ -22,8 +22,6 @@ provider-keymgmt - The KEYMGMT library E<lt>-E<gt> provider functions
  int OP_keymgmt_gen_set_template(void *genctx, void *template);
  int OP_keymgmt_gen_set_params(void *genctx, const OSSL_PARAM params[]);
  const OSSL_PARAM *OP_keymgmt_gen_settable_params(void *provctx);
- int OP_keymgmt_gen_get_params(void *genctx, const OSSL_PARAM params[]);
- const OSSL_PARAM *OP_keymgmt_gen_gettable_params(void *provctx);
  void *OP_keymgmt_gen(void *genctx, OSSL_CALLBACK *cb, void *cbarg);
  void OP_keymgmt_gen_cleanup(void *genctx);
 
@@ -93,8 +91,6 @@ macros in L<openssl-core_numbers.h(7)>, as follows:
  OP_keymgmt_gen_set_template     OSSL_FUNC_KEYMGMT_GEN_SET_TEMPLATE
  OP_keymgmt_gen_set_params       OSSL_FUNC_KEYMGMT_GEN_SET_PARAMS
  OP_keymgmt_gen_settable_params  OSSL_FUNC_KEYMGMT_GEN_SETTABLE_PARAMS
- OP_keymgmt_gen_get_params       OSSL_FUNC_KEYMGMT_GEN_GET_PARAMS
- OP_keymgmt_gen_gettable_params  OSSL_FUNC_KEYMGMT_GEN_GETTABLE_PARAMS
  OP_keymgmt_gen                  OSSL_FUNC_KEYMGMT_GEN
  OP_keymgmt_gen_cleanup          OSSL_FUNC_KEYMGMT_GEN_CLEANUP
 
@@ -213,7 +209,6 @@ OP_keymgmt_free() should free the passed I<keydata>.
 
 OP_keymgmt_gen_init(), OP_keymgmt_gen_set_template(),
 OP_keymgmt_gen_set_params(), OP_keymgmt_gen_settable_params(),
-OP_keymgmt_gen_get_params(), OP_keymgmt_gen_gettable_params(),
 OP_keymgmt_gen() and OP_keymgmt_gen_cleanup() work together as a more
 elaborate context based key object constructor.
 
@@ -235,13 +230,6 @@ OP_keymgmt_gen_settable_params() should return a constant array of
 descriptor B<OSSL_PARAM>, for parameters that OP_keymgmt_gen_set_params() 
 can handle.
 
-OP_keymgmt_gen_get_params() should extract information data associated
-with the key object generation context I<genctx>.
-
-OP_keymgmt_gen_gettable_params() should return a constant array of
-descriptor B<OSSL_PARAM>, for parameters that OP_keymgmt_gen_get_params() 
-can handle.
-
 OP_keymgmt_gen() should perform the key object generation itself, and
 return the result.  The callback I<cb> should be called at regular
 intervals with indications on how the key object generation
index 852e82518eec15fd4dc80cc71608b5f80f0d4206..ee4b6221e68bf8d78292de60ad3a70d3fc545aff 100644 (file)
@@ -678,10 +678,6 @@ int evp_keymgmt_gen_set_params(const EVP_KEYMGMT *keymgmt, void *genctx,
                                const OSSL_PARAM params[]);
 const OSSL_PARAM *
 evp_keymgmt_gen_settable_params(const EVP_KEYMGMT *keymgmt);
-int evp_keymgmt_gen_get_params(const EVP_KEYMGMT *keymgmt, void *genctx,
-                               OSSL_PARAM params[]);
-const OSSL_PARAM *
-evp_keymgmt_gen_gettable_params(const EVP_KEYMGMT *keymgmt);
 void *evp_keymgmt_gen(const EVP_KEYMGMT *keymgmt, void *genctx,
                       OSSL_CALLBACK *cb, void *cbarg);
 void evp_keymgmt_gen_cleanup(const EVP_KEYMGMT *keymgmt, void *genctx);
index aaf281b1feecca59e32749ce219ffd678d928c8e..2cf2f27715e5a1fba09f01fb3d7eadb33b8f0274 100644 (file)
@@ -392,10 +392,8 @@ OSSL_CORE_MAKE_FUNC(void *, OP_keymgmt_new, (void *provctx))
 # define OSSL_FUNC_KEYMGMT_GEN_SET_TEMPLATE            3
 # define OSSL_FUNC_KEYMGMT_GEN_SET_PARAMS              4
 # define OSSL_FUNC_KEYMGMT_GEN_SETTABLE_PARAMS         5
-# define OSSL_FUNC_KEYMGMT_GEN_GET_PARAMS              6
-# define OSSL_FUNC_KEYMGMT_GEN_GETTABLE_PARAMS         7
-# define OSSL_FUNC_KEYMGMT_GEN                         8
-# define OSSL_FUNC_KEYMGMT_GEN_CLEANUP                 9
+# define OSSL_FUNC_KEYMGMT_GEN                         6
+# define OSSL_FUNC_KEYMGMT_GEN_CLEANUP                 7
 OSSL_CORE_MAKE_FUNC(void *, OP_keymgmt_gen_init,
                     (void *provctx, int selection))
 OSSL_CORE_MAKE_FUNC(int, OP_keymgmt_gen_set_template,
index 9466b4fd0b6320460d85a0db87dd5faab0b24e06..a48b2795477fe7062368037826dc9185a58cb649 100644 (file)
@@ -31,8 +31,6 @@ static OSSL_OP_keymgmt_gen_init_fn ec_gen_init;
 static OSSL_OP_keymgmt_gen_set_template_fn ec_gen_set_template;
 static OSSL_OP_keymgmt_gen_set_params_fn ec_gen_set_params;
 static OSSL_OP_keymgmt_gen_settable_params_fn ec_gen_settable_params;
-static OSSL_OP_keymgmt_gen_get_params_fn ec_gen_get_params;
-static OSSL_OP_keymgmt_gen_gettable_params_fn ec_gen_gettable_params;
 static OSSL_OP_keymgmt_gen_fn ec_gen;
 static OSSL_OP_keymgmt_gen_cleanup_fn ec_gen_cleanup;
 static OSSL_OP_keymgmt_free_fn ec_freedata;
@@ -679,39 +677,6 @@ static const OSSL_PARAM *ec_gen_settable_params(void *provctx)
     return settable;
 }
 
-static int ec_gen_get_params(void *genctx, OSSL_PARAM params[])
-{
-    struct ec_gen_ctx *gctx = genctx;
-    OSSL_PARAM *p;
-
-    if ((p = OSSL_PARAM_locate(params, OSSL_PKEY_PARAM_EC_NAME)) != NULL) {
-        int nid = EC_GROUP_get_curve_name(gctx->gen_group);
-        int ret = 0;
-        const char *curve_name = ec_curve_nid2name(nid);
-
-        switch (p->data_type) {
-        case OSSL_PARAM_UTF8_STRING:
-            ret = OSSL_PARAM_set_utf8_string(p, curve_name);
-            break;
-        case OSSL_PARAM_UTF8_PTR:
-            ret = OSSL_PARAM_set_utf8_ptr(p, curve_name);
-            break;
-        }
-        return ret;
-    }
-    return 1;
-}
-
-static const OSSL_PARAM *ec_gen_gettable_params(void *provctx)
-{
-    static OSSL_PARAM gettable[] = {
-        { OSSL_PKEY_PARAM_EC_NAME, OSSL_PARAM_UTF8_PTR, NULL, 0, 0 },
-        OSSL_PARAM_END
-    };
-
-    return gettable;
-}
-
 static int ec_gen_assign_group(EC_KEY *ec, EC_GROUP *group)
 {
     if (group == NULL) {
@@ -767,9 +732,6 @@ const OSSL_DISPATCH ec_keymgmt_functions[] = {
     { OSSL_FUNC_KEYMGMT_GEN_SET_PARAMS, (void (*)(void))ec_gen_set_params },
     { OSSL_FUNC_KEYMGMT_GEN_SETTABLE_PARAMS,
       (void (*)(void))ec_gen_settable_params },
-    { OSSL_FUNC_KEYMGMT_GEN_GET_PARAMS, (void (*)(void))ec_gen_get_params },
-    { OSSL_FUNC_KEYMGMT_GEN_GETTABLE_PARAMS,
-      (void (*)(void))ec_gen_gettable_params },
     { OSSL_FUNC_KEYMGMT_GEN, (void (*)(void))ec_gen },
     { OSSL_FUNC_KEYMGMT_GEN_CLEANUP, (void (*)(void))ec_gen_cleanup },
     { OSSL_FUNC_KEYMGMT_FREE, (void (*)(void))ec_freedata },
index eac4a17ed188d7041124e319081198673951494d..c9857d6c676ce604984ffe0137a4744f401d4cf2 100644 (file)
@@ -155,6 +155,7 @@ static int dsa_keygen_test(void)
     unsigned char seed_out[32];
     char group_out[32];
     size_t len = 0;
+    const OSSL_PARAM *settables = NULL;
     static const unsigned char seed_data[] = {
         0xa6, 0xf5, 0x28, 0x8c, 0x50, 0x77, 0xa5, 0x68,
         0x6d, 0x3a, 0xf5, 0xf1, 0xc6, 0x4c, 0xdc, 0x35,
@@ -244,6 +245,10 @@ static int dsa_keygen_test(void)
         goto end;
     if (!TEST_ptr(pg_ctx = EVP_PKEY_CTX_new_from_name(NULL, "DSA", NULL))
         || !TEST_int_gt(EVP_PKEY_paramgen_init(pg_ctx), 0)
+        || !TEST_ptr_null(EVP_PKEY_CTX_gettable_params(pg_ctx))
+        || !TEST_ptr(settables = EVP_PKEY_CTX_settable_params(pg_ctx))
+        || !TEST_ptr(OSSL_PARAM_locate_const(settables,
+                                             OSSL_PKEY_PARAM_FFC_PBITS))
         || !TEST_true(EVP_PKEY_CTX_set_dsa_paramgen_bits(pg_ctx, 2048))
         || !TEST_true(EVP_PKEY_CTX_set_dsa_paramgen_q_bits(pg_ctx, 224))
         || !TEST_true(EVP_PKEY_CTX_set_dsa_paramgen_seed(pg_ctx, seed_data,