reject empty subjectAltName and issuerAltName sequences - #15472
Conversation
| // RFC 5280, so an empty sequence is malformed. The path validation engine | ||
| // represents a certificate with no subjectAltName using its own empty | ||
| // sequence and does not call this. | ||
| if gn_seq.clone().next().is_none() { |
There was a problem hiding this comment.
SequenceOf takes a MINIMUM_LEN, is there a reason not to just set that to 1?
There was a problem hiding this comment.
i looked at that first, but the SubjectAlternativeName and IssuerAlternativeName aliases are shared with the path validation engine, which stands in for a certificate with no SAN by parsing an empty sequence into SubjectAlternativeName (the \x30\x00 parse in cryptography-x509-verification/src/lib.rs). setting MINIMUM_LEN to 1 on the alias would make that parse fail. keeping the check in parse_general_names also means the other GeneralNames users that go through it, authorityCertIssuer and the CRL distribution point names, pick up the same 1..MAX constraint, since 5280 defines them all the same way. happy to move it to a type-level constraint if you'd rather give verification its own zero-minimum type.
There was a problem hiding this comment.
Oh, I think we should probably fix the \x30\x00 callsites to just store an Option<SequenceOf<>>
2 participants
RFC 5280 defines subjectAltName and issuerAltName as a GeneralNames value, which is a SEQUENCE of one or more GeneralName, but the parser accepted an empty sequence, so a malformed certificate or CRL carrying a zero-entry SAN or IAN parsed to an empty extension rather than being rejected. The check sits in parse_general_names rather than on the GeneralNames type alias because the path validation engine reuses that alias and relies on an empty sequence to stand in for a certificate with no SAN. This rejects an empty sequence when the extension is parsed, matching the constraint and the behaviour already in place for an empty extendedKeyUsage.