Fix X509_ALGOR_set_md()

This API allocates internally and can leave a corrupted |alg| behind.
Change it to return an int so that callers can check for an error.
Also fix its only caller in rsa_md_to_algor().

This is an ABI change but will not break any callers.

Also add a small regress test for this API.

Change-Id: I7a5d1729dcd4c7726c3d4ead3740d478231f3611
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67187
Commit-Queue: Theo Buehler <theorbuehler@gmail.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>

crypto/x509/rsa_pss.c

diff --git a/crypto/x509/rsa_pss.c b/crypto/x509/rsa_pss.c
index 5974bfa..8e19b8c 100644
--- a/crypto/x509/rsa_pss.c
+++ b/crypto/x509/rsa_pss.c
@@ -125,7 +125,11 @@
   if (*palg == NULL) {
     return 0;
   }
-  X509_ALGOR_set_md(*palg, md);
+  if (!X509_ALGOR_set_md(*palg, md)) {
+    X509_ALGOR_free(*palg);
+    *palg = NULL;
+    return 0;
+  }
   return 1;
 }
 

crypto/x509/x509_test.cc

diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index d7f4313..4559b22 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -2814,6 +2814,25 @@
   }
 }
 
+TEST(X509Test, X509AlgorSetMd) {
+  bssl::UniquePtr<X509_ALGOR> alg(X509_ALGOR_new());
+  ASSERT_TRUE(alg);
+  EXPECT_TRUE(X509_ALGOR_set_md(alg.get(), EVP_sha256()));
+  const ASN1_OBJECT *obj;
+  const void *pval;
+  int ptype = 0;
+  X509_ALGOR_get0(&obj, &ptype, &pval, alg.get());
+  EXPECT_TRUE(obj);
+  EXPECT_EQ(OBJ_obj2nid(obj), NID_sha256);
+  EXPECT_EQ(ptype, V_ASN1_NULL); // OpenSSL has V_ASN1_UNDEF
+  EXPECT_EQ(pval, nullptr);
+  EXPECT_TRUE(X509_ALGOR_set_md(alg.get(), EVP_md5()));
+  X509_ALGOR_get0(&obj, &ptype, &pval, alg.get());
+  EXPECT_EQ(OBJ_obj2nid(obj), NID_md5);
+  EXPECT_EQ(ptype, V_ASN1_NULL);
+  EXPECT_EQ(pval, nullptr);
+}
+
 TEST(X509Test, X509NameSet) {
   bssl::UniquePtr<X509_NAME> name(X509_NAME_new());
   ASSERT_TRUE(name);

crypto/x509/x_algor.c

diff --git a/crypto/x509/x_algor.c b/crypto/x509/x_algor.c
index 819aee5..bdc77ae 100644
--- a/crypto/x509/x_algor.c
+++ b/crypto/x509/x_algor.c
@@ -123,7 +123,7 @@
 
 // Set up an X509_ALGOR DigestAlgorithmIdentifier from an EVP_MD
 
-void X509_ALGOR_set_md(X509_ALGOR *alg, const EVP_MD *md) {
+int X509_ALGOR_set_md(X509_ALGOR *alg, const EVP_MD *md) {
   int param_type;
 
   if (EVP_MD_flags(md) & EVP_MD_FLAG_DIGALGID_ABSENT) {
@@ -132,7 +132,7 @@
     param_type = V_ASN1_NULL;
   }
 
-  X509_ALGOR_set0(alg, OBJ_nid2obj(EVP_MD_type(md)), param_type, NULL);
+  return X509_ALGOR_set0(alg, OBJ_nid2obj(EVP_MD_type(md)), param_type, NULL);
 }
 
 // X509_ALGOR_cmp returns 0 if |a| and |b| are equal and non-zero otherwise.

include/openssl/x509.h

diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index 414451f..a072d6f 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -2696,7 +2696,7 @@
 
 // X509_ALGOR_set_md sets |alg| to the hash function |md|. Note this
 // AlgorithmIdentifier represents the hash function itself, not a signature
-// algorithm that uses |md|.
+// algorithm that uses |md|. It returns one on success and zero on error.
 //
 // Due to historical specification mistakes (see Section 2.1 of RFC 4055), the
 // parameters field is sometimes omitted and sometimes a NULL value. When used
@@ -2707,7 +2707,7 @@
 //
 // TODO(davidben): Rename this function, or perhaps just add a bespoke API for
 // constructing PSS and move on.
-OPENSSL_EXPORT void X509_ALGOR_set_md(X509_ALGOR *alg, const EVP_MD *md);
+OPENSSL_EXPORT int X509_ALGOR_set_md(X509_ALGOR *alg, const EVP_MD *md);
 
 // X509_ALGOR_cmp returns zero if |a| and |b| are equal, and some non-zero value
 // otherwise. Note this function can only be used for equality checks, not an