Allow empty extension lists in X.509
Empty X.509 extension lists should be rejected according to spec, but
this change reverts the parts of
https://boringssl-review.googlesource.com/c/boringssl/+/82087 that
reject empty extension lists, to avoid breakage due to invalid certs
with empty extension lists that were previously accepted as valid.
CRLs with empty extension lists are still rejected.
Change-Id: I945bec9684c5315d26ea364c62f7809f20be4909
Bug: 442221114
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/82707
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: Lily Chen <chlily@google.com>
diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index c137682..063ae8e 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -8977,6 +8977,9 @@
// A FALSE critical bit is encoded instead of omitted as DEFAULT.
// TODO(crbug.com/442221114): The parser should reject this.
"crypto/x509/test/unusual_tbs_critical_false_not_omitted.pem",
+ // Empty extension instead of omitting the entire field.
+ // TODO(crbug.com/442221114): The parser should reject this.
+ "crypto/x509/test/unusual_tbs_empty_extension_not_omitted.pem",
// ecdsa-with-SHA256 AlgorithmIdentifier parameters are NULL instead of
// omitted. We accept this due to b/167375496.
"crypto/x509/test/unusual_tbs_null_sigalg_param.pem",
@@ -9000,8 +9003,6 @@
// The following inputs were once accepted, and thus preserved in signature
// verification, but we no longer parse them at all.
const char *kInvalidPaths[] = {
- // Empty extension instead of omitting the entire field.
- "crypto/x509/test/unusual_tbs_empty_extension_not_omitted.pem",
// A v1 version is explicit encoded instead of omitted as DEFAULT.
"crypto/x509/test/unusual_tbs_v1_not_omitted.pem",
};
diff --git a/crypto/x509/x_x509.cc b/crypto/x509/x_x509.cc
index 5467777..a572042 100644
--- a/crypto/x509/x_x509.cc
+++ b/crypto/x509/x_x509.cc
@@ -187,13 +187,14 @@
OPENSSL_PUT_ERROR(ASN1, ASN1_R_DECODE_ERROR);
return nullptr;
}
+ // TODO(crbug.com/442221114, crbug.com/42290219): Empty extension lists
+ // should be rejected. Extensions is a SEQUENCE SIZE (1..MAX), so it cannot
+ // be empty. An empty extensions list is encoded by omitting the OPTIONAL
+ // field. libpki already rejects this.
const uint8_t *p = CBS_data(&wrapper);
ret->extensions = d2i_X509_EXTENSIONS(nullptr, &p, CBS_len(&wrapper));
if (ret->extensions == nullptr ||
- p != CBS_data(&wrapper) + CBS_len(&wrapper) ||
- // Extensions is a SEQUENCE SIZE (1..MAX), so it cannot be empty. An
- // empty extensions list is encoded by omitting the OPTIONAL field.
- sk_X509_EXTENSION_num(ret->extensions) == 0) {
+ p != CBS_data(&wrapper) + CBS_len(&wrapper)) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_DECODE_ERROR);
return nullptr;
}
