diff --git a/src/crl.rs b/src/crl.rs index cff5dd84..1537c50d 100644 --- a/src/crl.rs +++ b/src/crl.rs @@ -12,6 +12,7 @@ // ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +use crate::cert::lenient_certificate_serial_number; use crate::der::Tag; use crate::x509::{remember_extension, set_extension_once, Extension}; use crate::{der, signed_data, Error, SignatureAlgorithm, Time}; @@ -448,9 +449,9 @@ impl<'a> BorrowedRevokedCert<'a> { // gracefully handle such certificates. // Like the handling in cert.rs we choose to be lenient here, not enforcing the length // of a CRL revoked certificate's serial number is less than 20 octets in encoded form. - let serial_number = ring::io::der::positive_integer(der) + let serial_number = lenient_certificate_serial_number(der) .map_err(|_| Error::InvalidSerialNumber)? - .big_endian_without_leading_zero(); + .as_slice_less_safe(); let revocation_date = der::time_choice(der)?; diff --git a/tests/crl_tests.rs b/tests/crl_tests.rs index 723aa918..7963fbc3 100644 --- a/tests/crl_tests.rs +++ b/tests/crl_tests.rs @@ -2,6 +2,13 @@ use webpki::{BorrowedCertRevocationList, CertRevocationList, Error}; const REVOKED_SERIAL: &[u8] = &[0x03, 0xAE, 0x51, 0xDB, 0x51, 0x15, 0x5A, 0x3C]; +const REVOKED_SERIAL_NEGATIVE: &[u8] = &[0xfd, 0x78, 0xa8, 0x4e]; + +// because cert::Cert::serial is the raw DER integer encoding, this includes a leading +// zero if one is required to make the twos complement representation positive. in this case +// one is required. +const REVOKED_SERIAL_WITH_TOP_BIT_SET: &[u8] = &[0x00, 0x80, 0xfe, 0xed, 0xf0, 0x0d]; + #[test] fn parse_valid_crl() { // We should be able to parse a valid CRL without error, and find the revoked serial. @@ -105,20 +112,49 @@ fn parse_too_long_crl_number_crl() { #[test] fn parse_entry_negative_serial_crl() { - // Parsing a CRL that includes a revoked entry with a negative serial number shouldn't error - // up-front since the error is with a revoked entry. let crl = include_bytes!("crls/crl.negative.serial.der"); let crl = BorrowedCertRevocationList::from_der(&crl[..]).unwrap(); - // but searching for a revoked cert should error due to the entry with the negative serial number. - let res = crl.find_serial(REVOKED_SERIAL); - assert!(matches!(res, Err(Error::InvalidSerialNumber))); + assert!(crl + .find_serial(REVOKED_SERIAL) + .expect("looking for REVOKED_SERIAL failed") + .is_none()); + assert!(crl + .find_serial(REVOKED_SERIAL_NEGATIVE) + .expect("looking for REVOKED_SERIAL_NEGATIVE failed") + .is_some()); #[cfg(feature = "alloc")] { - // Constructing an owned CRL should error up-front since it will process the revoked certs. - let res = crl.to_owned(); - assert!(matches!(res, Err(Error::InvalidSerialNumber))); + let crl = crl.to_owned().unwrap(); + assert!(crl + .find_serial(REVOKED_SERIAL) + .expect("looking for REVOKED_SERIAL failed") + .is_none()); + assert!(crl + .find_serial(REVOKED_SERIAL_NEGATIVE) + .expect("looking for REVOKED_SERIAL_NEGATIVE failed") + .is_some()); + } +} + +#[test] +fn parse_entry_topbit_serial_crl() { + let crl = include_bytes!("crls/crl.topbit.serial.der"); + let crl = BorrowedCertRevocationList::from_der(&crl[..]).unwrap(); + + assert!(crl + .find_serial(REVOKED_SERIAL_WITH_TOP_BIT_SET) + .expect("failed to look for REVOKED_SERIAL_WITH_TOP_BIT_SET") + .is_some()); + + #[cfg(feature = "alloc")] + { + let crl = crl.to_owned().unwrap(); + assert!(crl + .find_serial(REVOKED_SERIAL_WITH_TOP_BIT_SET) + .expect("failed to look for REVOKED_SERIAL_WITH_TOP_BIT_SET") + .is_some()); } } diff --git a/tests/crls/crl.topbit.serial.der b/tests/crls/crl.topbit.serial.der new file mode 100644 index 00000000..cf328c3b Binary files /dev/null and b/tests/crls/crl.topbit.serial.der differ diff --git a/tests/crls/crl.topbit.serial.txt b/tests/crls/crl.topbit.serial.txt new file mode 100644 index 00000000..458d6958 --- /dev/null +++ b/tests/crls/crl.topbit.serial.txt @@ -0,0 +1,95 @@ +SEQUENCE { + SEQUENCE { + INTEGER { 1 } + SEQUENCE { + # ecdsa-with-SHA384 + OBJECT_IDENTIFIER { 1.2.840.10045.4.3.3 } + } + SEQUENCE { + SET { + SEQUENCE { + # countryName + OBJECT_IDENTIFIER { 2.5.4.6 } + PrintableString { "XX" } + } + } + SET { + SEQUENCE { + # organizationName + OBJECT_IDENTIFIER { 2.5.4.10 } + PrintableString { "Boulder Test" } + } + } + SET { + SEQUENCE { + # commonName + OBJECT_IDENTIFIER { 2.5.4.3 } + PrintableString { "(TEST) Elegant Elephant E1" } + } + } + } + UTCTime { "221010201207Z" } + UTCTime { "221019201206Z" } + SEQUENCE { + SEQUENCE { + INTEGER { `0080feedf00d` } + UTCTime { "221010191207Z" } + SEQUENCE { + SEQUENCE { + # reasonCode + OBJECT_IDENTIFIER { 2.5.29.21 } + OCTET_STRING { + ENUMERATED { `01` } + } + } + } + } + } + [0] { + SEQUENCE { + SEQUENCE { + # authorityKeyIdentifier + OBJECT_IDENTIFIER { 2.5.29.35 } + OCTET_STRING { + SEQUENCE { + [0 PRIMITIVE] { `01dabb7acb25208e5e79d6f996422f02412907be` } + } + } + } + SEQUENCE { + # cRLNumber + OBJECT_IDENTIFIER { 2.5.29.20 } + OCTET_STRING { + INTEGER { `171cce3de482ba61` } + } + } + SEQUENCE { + # issuingDistributionPoint + OBJECT_IDENTIFIER { 2.5.29.28 } + BOOLEAN { TRUE } + OCTET_STRING { + SEQUENCE { + [0] { + [0] { + [6 PRIMITIVE] { "http://c.boulder.test/66283756913588288/0.crl" } + } + } + [1 PRIMITIVE] { `ff` } + } + } + } + } + } + } + SEQUENCE { + # ecdsa-with-SHA384 + OBJECT_IDENTIFIER { 1.2.840.10045.4.3.3 } + } + BIT_STRING { + `00` + SEQUENCE { + INTEGER { `2f0e42149d361abc9ea1f3ad3a303c85876bb0fdc17e3959cce2c13c9d4746cb45d88348467db478318fa44714506b22` } + INTEGER { `57903e972922bd33817a4fea5517534adc2d2bdab6e69e41256bdb89c68d9ecfcb4bdcf78c349fb89d67cc237b9fbf33` } + } + } +}