Per PR feedback

justsmth · Nov 11, 2024 · 034e86d · 034e86d
1 parent 72e0317
commit 034e86d
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 23 deletions.
diff --git a/crypto/evp_extra/evp_extra_test.cc b/crypto/evp_extra/evp_extra_test.cc
@@ -3042,7 +3042,7 @@ struct ParamgenCBParam {
 static const ParamgenCBParam paramgenCBparams[] = {
   // DH_generate_parameters_ex makes a final call to `BN_GENCB_call(cb, 3, 0)`
   {"DH", EVP_PKEY_DH, "dh_paramgen_prime_len", "512", 3, 0},
-  // dsa_internal_paramgen makes a fubak call to `BN_GENCB_call(cb, 3, 1))`
+  // dsa_internal_paramgen makes a final call to `BN_GENCB_call(cb, 3, 1))`
   {"DSA", EVP_PKEY_DSA, "dsa_paramgen_bits", "512", 3, 1},
 };
 
@@ -3319,10 +3319,18 @@ TEST(EVPExtraTest, DSADigestSignFinalVerify) {
   size_t siglen = 0;
 
   {
+    EVP_PKEY_CTX* raw_pctx = nullptr;
+    const EVP_MD* raw_md = nullptr;
+
     bssl::UniquePtr<EVP_MD_CTX> md_ctx(EVP_MD_CTX_new());
     ASSERT_TRUE(md_ctx);
-    ASSERT_NE(1, EVP_DigestSignInit(md_ctx.get(), nullptr, EVP_md5(), nullptr, private_key.get()));
-    ASSERT_EQ(1, EVP_DigestSignInit(md_ctx.get(), nullptr, EVP_sha256(), nullptr, private_key.get()));
+    ASSERT_NE(1, EVP_DigestSignInit(md_ctx.get(), &raw_pctx, EVP_md5(), nullptr, private_key.get()));
+    // md_ctx takes ownership of raw_pctx
+    ASSERT_EQ(1, EVP_DigestSignInit(md_ctx.get(), &raw_pctx, EVP_sha256(), nullptr, private_key.get()));
+
+    ASSERT_EQ(1, EVP_PKEY_CTX_get_signature_md(raw_pctx, &raw_md));
+    ASSERT_EQ(EVP_sha256(), raw_md);
+
     ASSERT_EQ(1, EVP_DigestSignUpdate(md_ctx.get(), data, data_len));
     ASSERT_EQ(1, EVP_DigestSignFinal(md_ctx.get(), nullptr, &siglen));
     sig.resize(siglen);

diff --git a/crypto/evp_extra/p_dsa.c b/crypto/evp_extra/p_dsa.c
@@ -10,7 +10,7 @@
 #include "./internal.h"
 
 typedef struct {
-  int nbits;  // defaults to 2048A
+  int nbits;  // defaults to 2048
   int qbits;
   const EVP_MD *pmd;  // MD for paramgen
   const EVP_MD *md;   // MD for signing
@@ -104,7 +104,7 @@ static int pkey_dsa_paramgen(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey) {
       default:
         // This should not be possible.
         OPENSSL_PUT_ERROR(EVP, EVP_R_INVALID_OPERATION);
-        return 0;
+        goto end;
     }
   }
 
@@ -132,6 +132,7 @@ static int pkey_dsa_sign(EVP_PKEY_CTX *ctx, unsigned char *sig, size_t *siglen,
   GUARD_PTR(ctx->pkey);
   GUARD_PTR(ctx->pkey->pkey.ptr);
   GUARD_PTR(ctx->data);
+  GUARD_PTR(siglen);
 
   DSA_PKEY_CTX *dctx = ctx->data;
   DSA *dsa = ctx->pkey->pkey.dsa;
@@ -267,19 +268,18 @@ static int pkey_dsa_ctrl(EVP_PKEY_CTX *ctx, int type, int p1, void *p2) {
       }
     }
     case EVP_PKEY_CTRL_GET_MD:
+      if (p2 == NULL) {
+        return 0;
+      }
       *(const EVP_MD **)p2 = dctx->md;
       return 1;
 
-    case EVP_PKEY_CTRL_DIGESTINIT:
-    case EVP_PKEY_CTRL_PKCS7_SIGN:
-    case EVP_PKEY_CTRL_CMS_SIGN:
-      return 1;
-
     case EVP_PKEY_CTRL_PEER_KEY:
       OPENSSL_PUT_ERROR(EVP, EVP_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE);
       return -2;
     default:
-      return -2;
+      OPENSSL_PUT_ERROR(EVP, EVP_R_COMMAND_NOT_SUPPORTED);
+      return 0;
   }
 }
 
@@ -292,7 +292,9 @@ static int pkey_dsa_ctrl_str(EVP_PKEY_CTX *ctx, const char *type,
       OPENSSL_PUT_ERROR(EVP, EVP_R_INVALID_OPERATION);
       return 0;
     }
+OPENSSL_BEGIN_ALLOW_DEPRECATED
     return EVP_PKEY_CTX_set_dsa_paramgen_bits(ctx, (int)nbits);
+OPENSSL_END_ALLOW_DEPRECATED
   }
   if (strcmp(type, "dsa_paramgen_q_bits") == 0) {
     char *str_end = NULL;
@@ -301,7 +303,9 @@ static int pkey_dsa_ctrl_str(EVP_PKEY_CTX *ctx, const char *type,
       OPENSSL_PUT_ERROR(EVP, EVP_R_INVALID_OPERATION);
       return 0;
     }
+OPENSSL_BEGIN_ALLOW_DEPRECATED
     return EVP_PKEY_CTX_set_dsa_paramgen_q_bits(ctx, (int)qbits);
+OPENSSL_END_ALLOW_DEPRECATED
   }
   if (strcmp(type, "dsa_paramgen_md") == 0) {
     const EVP_MD *md = EVP_get_digestbyname(value);
@@ -310,7 +314,9 @@ static int pkey_dsa_ctrl_str(EVP_PKEY_CTX *ctx, const char *type,
       OPENSSL_PUT_ERROR(EVP, EVP_R_INVALID_DIGEST_TYPE);
       return 0;
     }
+OPENSSL_BEGIN_ALLOW_DEPRECATED
     return EVP_PKEY_CTX_set_dsa_paramgen_md(ctx, md);
+OPENSSL_END_ALLOW_DEPRECATED
   }
   return -2;
 }

diff --git a/crypto/evp_extra/p_methods.c b/crypto/evp_extra/p_methods.c
@@ -16,7 +16,6 @@ static const EVP_PKEY_METHOD *const non_fips_pkey_evp_methods[] = {
   &dsa_pkey_meth
 };
 
-// We intentionally omit |dh_asn1_meth| from this list. It is not serializable.
 const EVP_PKEY_ASN1_METHOD *const asn1_evp_pkey_methods[] = {
   &rsa_asn1_meth,
   &rsa_pss_asn1_meth,

diff --git a/crypto/fipsmodule/evp/evp_ctx_test.cc b/crypto/fipsmodule/evp/evp_ctx_test.cc
@@ -434,7 +434,6 @@ static void verify_DSA(const DSA* dsa, unsigned psize, unsigned qsize) {
 
 
 TEST_F(EvpPkeyCtxCtrlStrTest, DSAParamGen) {
-  // Test Cases from RFC 5869.
 
   {
     bssl::UniquePtr<EVP_PKEY_CTX> ctx(

diff --git a/crypto/fipsmodule/evp/internal.h b/crypto/fipsmodule/evp/internal.h
@@ -247,9 +247,6 @@ int EVP_RSA_PKEY_CTX_ctrl(EVP_PKEY_CTX *ctx, int optype, int cmd, int p1, void *
 #define EVP_PKEY_CTRL_DSA_PARAMGEN_BITS (EVP_PKEY_ALG_CTRL + 23)
 #define EVP_PKEY_CTRL_DSA_PARAMGEN_Q_BITS (EVP_PKEY_ALG_CTRL + 24)
 #define EVP_PKEY_CTRL_DSA_PARAMGEN_MD (EVP_PKEY_ALG_CTRL + 25)
-#define EVP_PKEY_CTRL_DIGESTINIT (EVP_PKEY_ALG_CTRL + 26)
-#define EVP_PKEY_CTRL_PKCS7_SIGN (EVP_PKEY_ALG_CTRL + 27)
-#define EVP_PKEY_CTRL_CMS_SIGN (EVP_PKEY_ALG_CTRL + 28)
 
 // EVP_PKEY_CTX_KEYGEN_INFO_COUNT is the maximum array length for
 // |EVP_PKEY_CTX->keygen_info|. The array length corresponds to the number of

diff --git a/include/openssl/evp.h b/include/openssl/evp.h
@@ -1297,21 +1297,21 @@ OPENSSL_EXPORT OPENSSL_DEPRECATED void EVP_cleanup(void);
 #define EVP_PKEY_DSA NID_dsa
 
 // EVP_PKEY_CTX_set_dsa_paramgen_bits sets the number of bits for DSA paramgen.
-// |nbits| must be larger than 256. Returns 1 on success, 0 otherwise.
-OPENSSL_EXPORT int EVP_PKEY_CTX_set_dsa_paramgen_bits(
+// |nbits| must be at least 512. Returns 1 on success, 0 otherwise.
+OPENSSL_EXPORT OPENSSL_DEPRECATED int EVP_PKEY_CTX_set_dsa_paramgen_bits(
     EVP_PKEY_CTX *ctx, int nbits);
 
 // EVP_PKEY_CTX_set_dsa_paramgen_md sets the digest function used for DSA
-// parameter generation. If not specified, one of SHA-1, SHA-224, or SHA-256 is
-// selected on the number of bits in |q|.
-OPENSSL_EXPORT int EVP_PKEY_CTX_set_dsa_paramgen_md(EVP_PKEY_CTX *ctx, const EVP_MD* md);
+// parameter generation. If not specified, one of SHA-1 (160), SHA-224 (224),
+// or SHA-256 (256) is selected based on the number of bits in |q|.
+OPENSSL_EXPORT OPENSSL_DEPRECATED int EVP_PKEY_CTX_set_dsa_paramgen_md(EVP_PKEY_CTX *ctx, const EVP_MD* md);
 
 // EVP_PKEY_CTX_set_dsa_paramgen_q_bits sets the number of bits in q to use for
 // DSA parameter generation. If not specified, the default is 256. If a digest
 // function is specified with |EVP_PKEY_CTX_set_dsa_paramgen_md| then this
 // parameter is ignored and the number of bits in q matches the size of the
-// digest.
-OPENSSL_EXPORT int EVP_PKEY_CTX_set_dsa_paramgen_q_bits(
+// digest. This function only accepts the values 160, 224 or 256 for |qbits|.
+OPENSSL_EXPORT OPENSSL_DEPRECATED int EVP_PKEY_CTX_set_dsa_paramgen_q_bits(
     EVP_PKEY_CTX *ctx, int qbits);