Skip to content

Commit

Permalink
x509-cert: relax serialnumber parsing, allow rfc-invalid values
Browse files Browse the repository at this point in the history
  • Loading branch information
baloo committed Apr 4, 2023
1 parent 9adb377 commit 2c8219f
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 11 deletions.
17 changes: 7 additions & 10 deletions x509-cert/src/serial_number.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 })
}
}
Expand Down
21 changes: 20 additions & 1 deletion x509-cert/tests/certificate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)] = &[
Expand Down Expand Up @@ -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());
}
16 changes: 16 additions & 0 deletions x509-cert/tests/examples/qualcomm.pem
Original file line number Diff line number Diff line change
@@ -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-----

0 comments on commit 2c8219f

Please sign in to comment.