en EVP_PKEY_CTX_set_rsa_keygen_pubexp() BIGNUM management

Fixes #12635

As discussed in the issue, supporting the set0-like semantics long-term is not necessarily desirable, although necessary for short-term compatibility concerns. So I've deprecated the original method and added an equivalent that is explicitly labelled as set1.

I tried to audit existing usages of the (now-deprecated) API and update them to use set1 if that appeared to align with their expectations.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/12917)

10 files changed, 72 insertions, 16 deletions

diff --git a/CHANGES.md b/CHANGES.md
index c88629ebeb..595a7425ca 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -32,6 +32,11 @@ OpenSSL 3.0
 
    *Richard Levitte*
 
+ * Deprecated EVP_PKEY_CTX_set_rsa_keygen_pubexp() & introduced
+   EVP_PKEY_CTX_set1_rsa_keygen_pubexp(), which is now preferred.
+
+   *Jeremy Walch*
+
  * Changed all "STACK" functions to be macros instead of inline functions. Macro
    parameters are still checked for type safety at compile time via helper
    inline functions.
diff --git a/apps/genrsa.c b/apps/genrsa.c
index 04315a559b..f471814e08 100644
--- a/apps/genrsa.c
+++ b/apps/genrsa.c
@@ -188,7 +188,7 @@ opthelp:
         BIO_printf(bio_err, "Error allocating RSA public exponent\n");
         goto end;
     }
-    if (EVP_PKEY_CTX_set_rsa_keygen_pubexp(ctx, bn) <= 0) {
+    if (EVP_PKEY_CTX_set1_rsa_keygen_pubexp(ctx, bn) <= 0) {
         BIO_printf(bio_err, "Error setting RSA public exponent\n");
         goto end;
     }
diff --git a/crypto/evp/pmeth_lib.c b/crypto/evp/pmeth_lib.c
index 656a0e065a..a3a65857b8 100644
--- a/crypto/evp/pmeth_lib.c
+++ b/crypto/evp/pmeth_lib.c
@@ -402,6 +402,7 @@ void EVP_PKEY_CTX_free(EVP_PKEY_CTX *ctx)
 #if !defined(OPENSSL_NO_ENGINE) && !defined(FIPS_MODULE)
     ENGINE_finish(ctx->engine);
 #endif
+    BN_free(ctx->rsa_pubexp);
     OPENSSL_free(ctx);
 }
 
diff --git a/crypto/rsa/rsa_lib.c b/crypto/rsa/rsa_lib.c
index 858d3d72c8..475fca0f89 100644
--- a/crypto/rsa/rsa_lib.c
+++ b/crypto/rsa/rsa_lib.c
@@ -1345,7 +1345,9 @@ int EVP_PKEY_CTX_set_rsa_keygen_bits(EVP_PKEY_CTX *ctx, int bits)
     return 1;
 }
 
-int EVP_PKEY_CTX_set_rsa_keygen_pubexp(EVP_PKEY_CTX *ctx, BIGNUM *pubexp)
+static int evp_pkey_ctx_set_rsa_keygen_pubexp_intern(EVP_PKEY_CTX *ctx,
+                                                     BIGNUM *pubexp,
+                                                     int copy)
 {
     OSSL_PARAM_BLD *tmpl;
     OSSL_PARAM *params;
@@ -1362,9 +1364,15 @@ int EVP_PKEY_CTX_set_rsa_keygen_pubexp(EVP_PKEY_CTX *ctx, BIGNUM *pubexp)
         return -1;
 
     /* TODO(3.0): Remove this eventually when no more legacy */
-    if (ctx->op.keymgmt.genctx == NULL)
-        return EVP_PKEY_CTX_ctrl(ctx, EVP_PKEY_RSA, EVP_PKEY_OP_KEYGEN,
-                                 EVP_PKEY_CTRL_RSA_KEYGEN_PUBEXP, 0, pubexp);
+    if (ctx->op.keymgmt.genctx == NULL) {
+        if (copy == 1)
+            pubexp = BN_dup(pubexp);
+        ret = EVP_PKEY_CTX_ctrl(ctx, EVP_PKEY_RSA, EVP_PKEY_OP_KEYGEN,
+                                EVP_PKEY_CTRL_RSA_KEYGEN_PUBEXP, 0, pubexp);
+        if ((copy == 1) && (ret <= 0))
+            BN_free(pubexp);
+        return ret;
+    }
 
     if ((tmpl = OSSL_PARAM_BLD_new()) == NULL)
         return 0;
@@ -1377,9 +1385,28 @@ int EVP_PKEY_CTX_set_rsa_keygen_pubexp(EVP_PKEY_CTX *ctx, BIGNUM *pubexp)
 
     ret = EVP_PKEY_CTX_set_params(ctx, params);
     OSSL_PARAM_BLD_free_params(params);
+
+    /*
+     * Satisfy memory semantics for pre-3.0 callers of
+     * EVP_PKEY_CTX_set_rsa_keygen_pubexp(): their expectation is that input
+     * pubexp BIGNUM becomes managed by the EVP_PKEY_CTX on success.
+     */
+    if ((copy == 0) && (ret > 0))
+        ctx->rsa_pubexp = pubexp;
+
     return ret;
 }
 
+int EVP_PKEY_CTX_set_rsa_keygen_pubexp(EVP_PKEY_CTX *ctx, BIGNUM *pubexp)
+{
+    return evp_pkey_ctx_set_rsa_keygen_pubexp_intern(ctx, pubexp, 0);
+}
+
+int EVP_PKEY_CTX_set1_rsa_keygen_pubexp(EVP_PKEY_CTX *ctx, BIGNUM *pubexp)
+{
+    return evp_pkey_ctx_set_rsa_keygen_pubexp_intern(ctx, pubexp, 1);
+}
+
 int EVP_PKEY_CTX_set_rsa_keygen_primes(EVP_PKEY_CTX *ctx, int primes)
 {
     OSSL_PARAM params[2], *p = params;
diff --git a/crypto/rsa/rsa_pmeth.c b/crypto/rsa/rsa_pmeth.c
index e899fbd605..261f347a2d 100644
--- a/crypto/rsa/rsa_pmeth.c
+++ b/crypto/rsa/rsa_pmeth.c
@@ -653,9 +653,8 @@ static int pkey_rsa_ctrl_str(EVP_PKEY_CTX *ctx,
         BIGNUM *pubexp = NULL;
         if (!BN_asc2bn(&pubexp, value))
             return 0;
-        ret = EVP_PKEY_CTX_set_rsa_keygen_pubexp(ctx, pubexp);
-        if (ret <= 0)
-            BN_free(pubexp);
+        ret = EVP_PKEY_CTX_set1_rsa_keygen_pubexp(ctx, pubexp);
+        BN_free(pubexp);
         return ret;
     }
 
diff --git a/doc/man3/EVP_PKEY_CTX_ctrl.pod b/doc/man3/EVP_PKEY_CTX_ctrl.pod
index e5c187d950..7159d8885c 100644
--- a/doc/man3/EVP_PKEY_CTX_ctrl.pod
+++ b/doc/man3/EVP_PKEY_CTX_ctrl.pod
@@ -17,6 +17,7 @@ EVP_PKEY_CTX_set_rsa_pss_saltlen,
 EVP_PKEY_CTX_get_rsa_pss_saltlen,
 EVP_PKEY_CTX_set_rsa_keygen_bits,
 EVP_PKEY_CTX_set_rsa_keygen_pubexp,
+EVP_PKEY_CTX_set1_rsa_keygen_pubexp,
 EVP_PKEY_CTX_set_rsa_keygen_primes,
 EVP_PKEY_CTX_set_rsa_mgf1_md_name,
 EVP_PKEY_CTX_set_rsa_mgf1_md,
@@ -101,7 +102,7 @@ EVP_PKEY_CTX_set_kem_op
  int EVP_PKEY_CTX_set_rsa_pss_saltlen(EVP_PKEY_CTX *ctx, int saltlen);
  int EVP_PKEY_CTX_get_rsa_pss_saltlen(EVP_PKEY_CTX *ctx, int *saltlen);
  int EVP_PKEY_CTX_set_rsa_keygen_bits(EVP_PKEY_CTX *ctx, int mbits);
- int EVP_PKEY_CTX_set_rsa_keygen_pubexp(EVP_PKEY_CTX *ctx, BIGNUM *pubexp);
+ int EVP_PKEY_CTX_set1_rsa_keygen_pubexp(EVP_PKEY_CTX *ctx, BIGNUM *pubexp);
  int EVP_PKEY_CTX_set_rsa_keygen_primes(EVP_PKEY_CTX *ctx, int primes);
  int EVP_PKEY_CTX_set_rsa_mgf1_md_name(EVP_PKEY_CTX *ctx, const char *mdname,
                                      const char *mdprops);
@@ -177,6 +178,14 @@ EVP_PKEY_CTX_set_kem_op
  int EVP_PKEY_CTX_get1_id(EVP_PKEY_CTX *ctx, void *id);
  int EVP_PKEY_CTX_get1_id_len(EVP_PKEY_CTX *ctx, size_t *id_len);
 
+Deprecated since OpenSSL 3.0, can be hidden entirely by defining
+B<OPENSSL_API_COMPAT> with a suitable version value, see
+L<openssl_user_macros(7)>:
+
+ #include <openssl/rsa.h>
+
+ int EVP_PKEY_CTX_set_rsa_keygen_pubexp(EVP_PKEY_CTX *ctx, BIGNUM *pubexp);
+
 =head1 DESCRIPTION
 
 EVP_PKEY_CTX_ctrl() sends a control operation to the context I<ctx>. The key
@@ -282,10 +291,15 @@ The padding mode must already have been set to B<RSA_PKCS1_PSS_PADDING>.
 EVP_PKEY_CTX_set_rsa_keygen_bits() sets the RSA key length for
 RSA key generation to I<bits>. If not specified 2048 bits is used.
 
-EVP_PKEY_CTX_set_rsa_keygen_pubexp() sets the public exponent value for RSA key
-generation to I<pubexp>. Currently it should be an odd integer. The
-I<pubexp> pointer is used internally by this function so it should not be
-modified or freed after the call. If not specified 65537 is used.
+EVP_PKEY_CTX_set1_rsa_keygen_pubexp() sets the public exponent value for RSA key
+generation to the value stored in I<pubexp>. Currently it should be an odd
+integer. In accordance with the OpenSSL naming convention, the I<pubexp> pointer
+must be freed independently of the EVP_PKEY_CTX (ie, it is internally copied).
+If not specified 65537 is used.
+
+EVP_PKEY_CTX_set_rsa_keygen_pubexp() does the same as
+EVP_PKEY_CTX_set1_rsa_keygen_pubexp() except that there is no internal copy and
+therefore I<pubexp> should not be modified or freed after the call.
 
 EVP_PKEY_CTX_set_rsa_keygen_primes() sets the number of primes for
 RSA key generation to I<primes>. If not specified 2 is used.
diff --git a/include/crypto/evp.h b/include/crypto/evp.h
index 835224a7aa..23990f651c 100644
--- a/include/crypto/evp.h
+++ b/include/crypto/evp.h
@@ -95,6 +95,13 @@ struct evp_pkey_ctx_st {
     void *data;
     /* Indicator if digest_custom needs to be called */
     unsigned int flag_call_digest_custom:1;
+    /*
+     * Used to support taking custody of memory in the case of a provider being
+     * used with the deprecated EVP_PKEY_CTX_set_rsa_keygen_pubexp() API. This
+     * member should NOT be used for any other purpose and should be removed
+     * when said deprecated API is excised completely.
+     */
+    BIGNUM *rsa_pubexp;
 } /* EVP_PKEY_CTX */ ;
 
 #define EVP_PKEY_FLAG_DYNAMIC   1
diff --git a/include/openssl/rsa.h b/include/openssl/rsa.h
index 140c0d4412..24b2a7eb55 100644
--- a/include/openssl/rsa.h
+++ b/include/openssl/rsa.h
@@ -132,7 +132,9 @@ int EVP_PKEY_CTX_set_rsa_pss_saltlen(EVP_PKEY_CTX *ctx, int saltlen);
 int EVP_PKEY_CTX_get_rsa_pss_saltlen(EVP_PKEY_CTX *ctx, int *saltlen);
 
 int EVP_PKEY_CTX_set_rsa_keygen_bits(EVP_PKEY_CTX *ctx, int bits);
-int EVP_PKEY_CTX_set_rsa_keygen_pubexp(EVP_PKEY_CTX *ctx, BIGNUM *pubexp);
+DEPRECATEDIN_3_0(int EVP_PKEY_CTX_set_rsa_keygen_pubexp(EVP_PKEY_CTX *ctx,
+                                                        BIGNUM *pubexp))
+int EVP_PKEY_CTX_set1_rsa_keygen_pubexp(EVP_PKEY_CTX *ctx, BIGNUM *pubexp);
 int EVP_PKEY_CTX_set_rsa_keygen_primes(EVP_PKEY_CTX *ctx, int primes);
 int EVP_PKEY_CTX_set_rsa_pss_keygen_saltlen(EVP_PKEY_CTX *ctx, int saltlen);
 
diff --git a/test/acvp_test.c b/test/acvp_test.c
index de1a2e1fbc..74e2eace7e 100644
--- a/test/acvp_test.c
+++ b/test/acvp_test.c
@@ -1119,7 +1119,7 @@ static int rsa_keygen_test(int id)
         || !TEST_int_gt(EVP_PKEY_keygen_init(ctx), 0)
         || !TEST_true(EVP_PKEY_CTX_set_params(ctx, params))
         || !TEST_true(EVP_PKEY_CTX_set_rsa_keygen_bits(ctx, tst->mod))
-        || !TEST_true(EVP_PKEY_CTX_set_rsa_keygen_pubexp(ctx, e_bn))
+        || !TEST_true(EVP_PKEY_CTX_set1_rsa_keygen_pubexp(ctx, e_bn))
         || !TEST_int_gt(EVP_PKEY_keygen(ctx, &pkey), 0)
         || !TEST_true(pkey_get_bn_bytes(pkey, OSSL_PKEY_PARAM_RSA_TEST_P1,
                                         &p1, &p1_len))
diff --git a/util/libcrypto.num b/util/libcrypto.num
index db4a1aab2d..7a0965ba72 100644
--- a/util/libcrypto.num
+++ b/util/libcrypto.num
@@ -4984,7 +4984,8 @@ OSSL_CMP_MSG_read                       ?	3_0_0	EXIST::FUNCTION:CMP
 OSSL_CMP_MSG_write                      ?	3_0_0	EXIST::FUNCTION:CMP
 EVP_PKEY_gen                            ?	3_0_0	EXIST::FUNCTION:
 EVP_PKEY_CTX_set_rsa_keygen_bits        ?	3_0_0	EXIST::FUNCTION:RSA
-EVP_PKEY_CTX_set_rsa_keygen_pubexp      ?	3_0_0	EXIST::FUNCTION:RSA
+EVP_PKEY_CTX_set_rsa_keygen_pubexp      ?	3_0_0	EXIST::FUNCTION:DEPRECATEDIN_3_0,RSA
+EVP_PKEY_CTX_set1_rsa_keygen_pubexp     ?	3_0_0	EXIST::FUNCTION:RSA
 EVP_PKEY_CTX_set_rsa_keygen_primes      ?	3_0_0	EXIST::FUNCTION:RSA
 NCONF_new_with_libctx                   ?	3_0_0	EXIST::FUNCTION:
 CONF_modules_load_file_with_libctx      ?	3_0_0	EXIST::FUNCTION: