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>
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;
}
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);
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.
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
