x509-cert: relax serialnumber parsing, allow rfc-invalid values

Fixes #978

Author: baloo

x509-cert/src/serial_number.rs

@@ -32,9 +32,6 @@ impl SerialNumber {
     /// Maximum length in bytes for a [`SerialNumber`]
     pub const MAX_LEN: Length = Length::new(20);
 
-    /// See notes in `SerialNumber::new` and `SerialNumber::decode_value`.
-    const MAX_DECODE_LEN: Length = Length::new(21);
-
     /// Create a new [`SerialNumber`] from a byte slice.
     ///
     /// The byte slice **must** represent a positive integer.
@@ -74,15 +71,15 @@ impl EncodeValue for SerialNumber {
 
 impl<'a> DecodeValue<'a> for SerialNumber {
     fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> Result<Self> {
+        // We explicitely allow any value to be parsed from PEM/DER formatted serial numbers
+        // as per RFC 5280 section 4.1.2.2:
+        //   Note: Non-conforming CAs may issue certificates with serial numbers
+        //   that are negative or zero.  Certificate users SHOULD be prepared to
+        //   gracefully handle such certificates.
+        //
+        // https://datatracker.ietf.org/doc/html/rfc5280#section-4.1.2.2
         let inner = Int::decode_value(reader, header)?;
 
-        // See the note in `SerialNumber::new`: we permit lengths of 21 bytes here,
-        // since some X.509 implementations interpret the limit of 20 bytes to refer
-        // to the pre-encoded value.
-        if inner.len() > SerialNumber::MAX_DECODE_LEN {
-            return Err(ErrorKind::Overlength.into());
-        }
-
         Ok(Self { inner })
     }
 }

x509-cert/tests/certificate.rs

@@ -11,7 +11,7 @@ use x509_cert::Certificate;
 use x509_cert::*;
 
 #[cfg(feature = "pem")]
-use der::DecodePem;
+use der::{pem::LineEnding, DecodePem, EncodePem};
 
 // TODO - parse and compare extension values
 const EXTENSIONS: &[(&str, bool)] = &[
@@ -412,3 +412,22 @@ fn decode_cert_negative_serial_number() {
     let reencoded = cert.to_der().unwrap();
     assert_eq!(der_encoded_cert, reencoded.as_slice());
 }
+
+#[cfg(feature = "pem")]
+#[test]
+fn decode_cert_overlength_serial_number() {
+    let der_encoded_cert = include_bytes!("examples/qualcomm.pem");
+
+    let cert = Certificate::from_pem(der_encoded_cert).unwrap();
+    assert_eq!(
+        cert.tbs_certificate.serial_number.as_bytes(),
+        &[
+            0, 132, 206, 11, 246, 160, 254, 130, 78, 229, 229, 6, 202, 168, 157, 120, 198, 21, 1,
+            98, 87, 113
+        ]
+    );
+    assert_eq!(cert.tbs_certificate.serial_number.as_bytes().len(), 22);
+
+    let reencoded = cert.to_pem(LineEnding::LF).unwrap();
+    assert_eq!(der_encoded_cert, reencoded.as_bytes());
+}

x509-cert/tests/examples/qualcomm.pem

@@ -0,0 +1,16 @@
+-----BEGIN CERTIFICATE-----
+MIICnDCCAiGgAwIBAgIWAITOC/ag/oJO5eUGyqideMYVAWJXcTAKBggqhkjOPQQD
+AzB2MSQwIgYDVQQKDBtRdWFsY29tbSBUZWNobm9sb2dpZXMsIEluYy4xKjAoBgNV
+BAsMIVF1YWxjb21tIENyeXB0b2dyYXBoaWMgT3BlcmF0aW9uczEiMCAGA1UEAwwZ
+UU1DIEF0dGVzdGF0aW9uIFJvb3QgQ0EgNDAeFw0xNzA4MDEyMjE2MzJaFw0yNzA4
+MDEyMjE2MzJaMH4xJDAiBgNVBAoMG1F1YWxjb21tIFRlY2hub2xvZ2llcywgSW5j
+LjEqMCgGA1UECwwhUXVhbGNvbW0gQ3J5cHRvZ3JhcGhpYyBPcGVyYXRpb25zMSow
+KAYDVQQDDCFRTUMgQXR0ZXN0YXRpb24gUm9vdCBDQSA0IFN1YkNBIDEwdjAQBgcq
+hkjOPQIBBgUrgQQAIgNiAAQDsjssSUEFLyyBe17UmO3pMzqKS+V1jfQkhq7a7zmH
+LCrPFmfaKLm0/szdzZxn+zwhoYen3fgJIuZUaip8wAQxLe4550c1ZBl3iSTvYUbe
+J+gBz2DiJHRBOtY1bQH35NWjZjBkMBIGA1UdEwEB/wQIMAYBAf8CAQAwDgYDVR0P
+AQH/BAQDAgEGMB0GA1UdDgQWBBTrVYStHPbaTn4k7bPerqZAmJcuXzAfBgNVHSME
+GDAWgBQBBnkODO3o7rgWy996xOf1BxR4VTAKBggqhkjOPQQDAwNpADBmAjEAmpM/
+Xvfawl4/A3jd0VVb6lOBh0Jy+zFz1Jz/hw+Xpm9G4XJCscBE7r7lbe2Xc1DHAjEA
+psnskI8pLJQwL80QzAwP3HvgyDUeedNpxnYNK797vqJu6uRMLsZBVHatLM1R4gyE
+-----END CERTIFICATE-----