Check some ASN1_STRING types in crypto/x509
This adds runtime checks that types which are aliases of ASN1_STRING
are in fact the expected ASN.1 type. Not comprehensive -- I got the
obvious ones from x509.h. These checks are not generally covered by
unit tests, except for one which was easy to test as a sanity-check.
Bug: 445
Change-Id: I8cd689b6b1e6121fce62c7f0ab25fee7e2a0b2ff
Update-Note: Various X.509 functions will now fail given the wrong ASN1_STRING subtype.
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54286
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/x509/a_sign.c b/crypto/x509/a_sign.c
index ed9e79b..3711a00 100644
--- a/crypto/x509/a_sign.c
+++ b/crypto/x509/a_sign.c
@@ -67,6 +67,10 @@
int ASN1_item_sign(const ASN1_ITEM *it, X509_ALGOR *algor1, X509_ALGOR *algor2,
ASN1_BIT_STRING *signature, void *asn, EVP_PKEY *pkey,
const EVP_MD *type) {
+ if (signature->type != V_ASN1_BIT_STRING) {
+ OPENSSL_PUT_ERROR(ASN1, ASN1_R_WRONG_TYPE);
+ return 0;
+ }
EVP_MD_CTX ctx;
EVP_MD_CTX_init(&ctx);
if (!EVP_DigestSignInit(&ctx, NULL, type, NULL, pkey)) {
@@ -83,6 +87,11 @@
unsigned char *buf_in = NULL, *buf_out = NULL;
size_t inl = 0, outl = 0;
+ if (signature->type != V_ASN1_BIT_STRING) {
+ OPENSSL_PUT_ERROR(ASN1, ASN1_R_WRONG_TYPE);
+ goto err;
+ }
+
pkey = EVP_PKEY_CTX_get0_pkey(ctx->pctx);
// Write out the requested copies of the AlgorithmIdentifier.
diff --git a/crypto/x509/x509_cmp.c b/crypto/x509/x509_cmp.c
index 9105b2f..a85c7c1 100644
--- a/crypto/x509/x509_cmp.c
+++ b/crypto/x509/x509_cmp.c
@@ -232,6 +232,10 @@
return NULL;
}
+ if (serial->type != V_ASN1_INTEGER) {
+ return NULL;
+ }
+
x.cert_info = &cinf;
cinf.serialNumber = serial;
cinf.issuer = name;
diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
index 8165d24..ea32e7b 100644
--- a/crypto/x509/x509_lu.c
+++ b/crypto/x509/x509_lu.c
@@ -146,6 +146,9 @@
if ((ctx->method == NULL) || (ctx->method->get_by_issuer_serial == NULL)) {
return 0;
}
+ if (serial->type != V_ASN1_INTEGER) {
+ return 0;
+ }
return ctx->method->get_by_issuer_serial(ctx, type, name, serial, ret) > 0;
}
diff --git a/crypto/x509/x509_set.c b/crypto/x509/x509_set.c
index 0f0c5d0..8c920be 100644
--- a/crypto/x509/x509_set.c
+++ b/crypto/x509/x509_set.c
@@ -98,8 +98,12 @@
}
int X509_set_serialNumber(X509 *x, const ASN1_INTEGER *serial) {
- ASN1_INTEGER *in;
+ if (serial->type != V_ASN1_INTEGER) {
+ OPENSSL_PUT_ERROR(ASN1, ASN1_R_WRONG_TYPE);
+ return 0;
+ }
+ ASN1_INTEGER *in;
if (x == NULL) {
return 0;
}
diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index 1bcc569..c639ef0 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -4806,3 +4806,16 @@
X509_NAME_ENTRY_free(X509_NAME_delete_entry(name.get(), 1));
check_name("CN=Name,O=Org");
}
+
+// Tests that non-integer types are rejected when passed as an argument to
+// X509_set_serialNumber().
+TEST(X509Test, SetSerialNumberChecksASN1StringType) {
+ bssl::UniquePtr<X509> root = CertFromPEM(kRootCAPEM);
+ ASSERT_TRUE(root);
+
+ // Passing an IA5String to X509_set_serialNumber() should fail.
+ bssl::UniquePtr<ASN1_IA5STRING> str(ASN1_IA5STRING_new());
+ ASSERT_TRUE(str);
+ int r = X509_set_serialNumber(root.get(), str.get());
+ ASSERT_EQ(r, 0);
+}
diff --git a/crypto/x509/x509cset.c b/crypto/x509/x509cset.c
index 9558128..408723f 100644
--- a/crypto/x509/x509cset.c
+++ b/crypto/x509/x509cset.c
@@ -216,6 +216,11 @@
const ASN1_INTEGER *serial) {
ASN1_INTEGER *in;
+ if (serial->type != V_ASN1_INTEGER) {
+ OPENSSL_PUT_ERROR(ASN1, ASN1_R_WRONG_TYPE);
+ return 0;
+ }
+
if (revoked == NULL) {
return 0;
}
diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c
index a8e6c74..573c115 100644
--- a/crypto/x509/x_crl.c
+++ b/crypto/x509/x_crl.c
@@ -430,6 +430,9 @@
static int crl_lookup(X509_CRL *crl, X509_REVOKED **ret, ASN1_INTEGER *serial,
X509_NAME *issuer) {
+ if (serial->type != V_ASN1_INTEGER) {
+ return 0;
+ }
X509_REVOKED rtmp, *rev;
size_t idx;
rtmp.serialNumber = serial;
