Skip to content

Reject CONTEXT-SPECIFIC tag class in serial number parser #249

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
afldl wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Reject CONTEXT-SPECIFIC tag class in serial number parser #249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ac62869
Reject CONTEXT-SPECIFIC tag class in serial number parser
afldl Jun 10, 2026
2cd636e
Reject CONTEXT-SPECIFIC tag class in serial number parser
afldl Jun 10, 2026
93e2bb6
Fix clippy collapsible_match in validate modules
afldl Jun 10, 2026
676248f
Fix clippy collapsible_match in validate modules
afldl Jun 10, 2026
9eebbe8
Fix clippy collapsible_match in validate modules
afldl Jun 10, 2026
6b9a4b7
Fix clippy collapsible_match in validate/extensions.rs
afldl Jun 10, 2026
362d317
Fix clippy collapsible_match in validate/structure.rs
afldl Jun 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions src/validate/extensions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,10 @@ impl<'a> Validator<'a> for X509ExtensionsValidator {
test_critical!(SHOULD NOT ext, l, "SubjectAltName");
for name in san.general_names() {
match name {
GeneralName::DNSName(ref s) | GeneralName::RFC822Name(ref s) => {
// should be an ia5string
if !s.as_bytes().iter().all(u8::is_ascii) {
l.warn(&format!("Invalid charset in 'SAN' entry '{s}'"));
}
GeneralName::DNSName(ref s) | GeneralName::RFC822Name(ref s)
if !s.as_bytes().iter().all(u8::is_ascii) =>
{
l.warn(&format!("Invalid charset in 'SAN' entry '{s}'"));

@chifflier chifflier Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems unrelated to the problem (and cargo fmt does not give the same result, FYI). Please remove this from the patch

}
_ => (),
}
Expand Down
9 changes: 4 additions & 5 deletions src/validate/structure.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,10 @@ impl<'a> Validator<'a> for TbsCertificateStructureValidator {
if let ParsedExtension::SubjectAlternativeName(san) = ext.parsed_extension() {
for name in san.general_names() {
match name {
GeneralName::DNSName(ref s) | GeneralName::RFC822Name(ref s) => {
// should be an ia5string
if !s.as_bytes().iter().all(u8::is_ascii) {
l.warn(&format!("Invalid charset in 'SAN' entry '{s}'"));
}
GeneralName::DNSName(ref s) | GeneralName::RFC822Name(ref s)
if !s.as_bytes().iter().all(u8::is_ascii) =>
{
l.warn(&format!("Invalid charset in 'SAN' entry '{s}'"));
Comment on lines +143 to +146

@chifflier chifflier Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto (unrelated)

}
_ => (),
}
Expand Down
22 changes: 19 additions & 3 deletions src/x509.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ use crate::public_key::*;

use asn1_rs::num_bigint::BigUint;
use asn1_rs::{
Alias, Any, BerError, BitString, BmpString, Choice, DerParser, Enumerated, FromDer, Header,
Input, Integer, OptTaggedExplicit, PrintableString, Sequence, Tag, Tagged, TeletexString,
UniversalString, Utf8String,
Alias, Any, BerError, BitString, BmpString, Choice, Class, DerParser, Enumerated, FromDer,
Header, Input, Integer, OptTaggedExplicit, PrintableString, Sequence, Tag, Tagged,
TeletexString, UniversalString, Utf8String,
};
use core::convert::TryFrom;
use data_encoding::HEXUPPER;
Expand Down Expand Up @@ -612,6 +612,10 @@ pub(crate) fn parse_serial(input: Input<'_>) -> IResult<Input<'_>, (&[u8], BigUi
// RFC 5280 4.1.2.2: "The serial number MUST be a positive integer"
// however, many CAs do not respect this and send integers with MSB set,
// so we do not use `as_biguint()`
// X.690 §8.1.2: tag class must be UNIVERSAL for INTEGER
if any.class() != Class::Universal {
return Err(Err::Error(X509Error::InvalidSerial));
}
Comment on lines +616 to +618

@chifflier chifflier Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To stay consistent with the next test, this should be changed to use .class().assert_eq() (see suggestion)

Suggested change
if any.class() != Class::Universal {
return Err(Err::Error(X509Error::InvalidSerial));
}
any.class()
.assert_eq(Class::Universal)
.map_err(|_| X509Error::InvalidSerial)?;

any.tag()
.assert_eq(Tag::Integer)
.map_err(|_| X509Error::InvalidSerial)?;
Expand Down Expand Up @@ -755,4 +759,16 @@ mod tests {
assert_eq!(v.not_before.to_datetime().year(), 2019);
assert_eq!(v.not_after.to_datetime().year(), 2029);
}

#[test]
fn test_serial_rejects_context_specific_tag() {
// Tag byte 0x02 (UNIVERSAL INTEGER) → 0x82 (CONTEXT-SPECIFIC [2])
// X.690 §8.1.2: tag class must match the expected type
let data: &[u8] = &[0x82, 0x01, 0x01]; // CONTEXT-SPECIFIC [2], length 1, value 1
let r = parse_serial(Input::from(data));
assert!(
r.is_err(),
"should reject CONTEXT-SPECIFIC tag for serial number"
);
}
}
Loading