question: should `GeneralName::DNSName` be treated according to RFC5280 7.1 & RFC1034 3.5

[georglauterbach]

While comparing GeneralNames, I noticed that the implementation of GeneralName may not adhere to the these RFCs:

1. RFC5280 7.2
2. RFC1034 3.5

---

Looking at the implementation

x509-parser/src/extensions/generalname.rs

Lines 30 to 35 in b28b903

	pub enum GeneralName<'a> {
	OtherName(Oid<'a>, Any<'a>),
	/// More or less an e-mail, the format is not checked.
	RFC822Name(&'a str),
	/// A hostname, format is not checked.
	DNSName(&'a str),

the DNSName field stores a &'a str which may be UTF-8 in Rust, but says RFC5280 7.2

one choice in GeneralName is the dNSName field, which is defined as type IA5String. [...] IA5String is limited to the set of ASCII characters. [...]

---

Moreover, the comparison of a &str is case-sensitive, while RFC1034 3.5 says

Note that while upper and lower case letters are allowed in domain names, no significance is attached to the case. That is, two names with the same spelling but different case are to be treated as if identical.

I am not 100% sure, but according to the RFC, we may need a case-insentive comparison here. Since PartialEq is derived, a new-type pattern could suffice for an ASCII-string that does case-insensitive comparisons.

[georglauterbach]

@chifflier could you please take a look at this? :)

[georglauterbach]

No response? No problem. Closing.

[cpu]

the DNSName field stores a &'a str which may be UTF-8 in Rust, but says RFC5280 7.2 ...

Agreed that this isn't a strict adherence to 5280. Unfortunately in the PKI space it's very difficult to thread the needle between IETF RFC text, ITU text, CABF policy, root program requirements, and extant real world behavior. Different projects place different emphasis on each based on their needs.

In this case x509-parser has taken a permissive view, parsing with ia5str_relaxed:

x509-parser/src/extensions/generalname.rs

Lines 171 to 178 in b7dcc93

	fn ia5str_relaxed(input: Input<'_>) -> Result<(Input<'_>, &str), Err<X509Error>> {
	let (rem, input) = input.take_split(input.input_len());
	// Relax constraints from RFC here: we are expecting an IA5String, but many certificates
	// are using unicode characters
	let s = std::str::from_utf8(input.as_bytes2())
	.map_err(|_| Err::Failure(X509Error::DerParser(InnerError::StringInvalidCharset)))?;
	Ok((rem, s))
	}

The comment in that function justifies the perspective of the author, that it's necessary to be loose here in order to parse RFC non-compliant certificates. Alongside this relaxed implementation there's a feature-gated validator (X509ExtensionsValidator) that warns for misencoded IA5Strings that you could apply:

x509-parser/src/validate/extensions.rs

Lines 100 to 105 in b7dcc93

	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}'"));
	}
	}

I think a newtype that made this stricter would probably be counter to the project's goals but that's my personal take.

Moreover, the comparison of a &str is case-sensitive

Indeed the derived PartialEq for GeneralName is not particularly standards compliant. Getting this right for each of the general name types is fraught and so I'm personally not super eager to wade into it without strong justification. Names are one of the worst parts of PKIX/X.509 :'(

I'll reopen the issue so we can think more on this topic as a group.

No response? No problem. Closing.

It may not be your intent, but this reads to me as an impolite way to bump attention on an issue in a project maintained by volunteers. Please consider a different approach, like sponsoring development for features/bugs you want prioritized, or asking more politely for attention if you think an issue has gone unnoticed.

[georglauterbach]

the DNSName field stores a &'a str which may be UTF-8 in Rust, but says RFC5280 7.2 ...

Agreed that this isn't a strict adherence to 5280. Unfortunately in the PKI space it's very difficult to thread the needle between IETF RFC text, ITU text, CABF policy, root program requirements, and extant real world behavior. Different projects place different emphasis on each based on their needs.

In this case x509-parser has taken a permissive view, parsing with ia5str_relaxed:

x509-parser/src/extensions/generalname.rs

Lines 171 to 178 in b7dcc93

	fn ia5str_relaxed(input: Input<'_>) -> Result<(Input<'_>, &str), Err<X509Error>> {
	let (rem, input) = input.take_split(input.input_len());
	// Relax constraints from RFC here: we are expecting an IA5String, but many certificates
	// are using unicode characters
	let s = std::str::from_utf8(input.as_bytes2())
	.map_err(|_| Err::Failure(X509Error::DerParser(InnerError::StringInvalidCharset)))?;
	Ok((rem, s))
	}

The comment in that function justifies the perspective of the author, that it's necessary to be loose here in order to parse RFC non-compliant certificates. Alongside this relaxed implementation there's a feature-gated validator (X509ExtensionsValidator) that warns for misencoded IA5Strings that you could apply:

x509-parser/src/validate/extensions.rs

Lines 100 to 105 in b7dcc93

	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}'"));
	}
	}

I think a newtype that made this stricter would probably be counter to the project's goals but that's my personal take.

Moreover, the comparison of a &str is case-sensitive

Indeed the derived PartialEq for GeneralName is not particularly standards compliant. Getting this right for each of the general name types is fraught and so I'm personally not super eager to wade into it without strong justification. Names are one of the worst parts of PKIX/X.509 :'(

I'll reopen the issue so we can think more on this topic as a group.

Sounds good to me!

No response? No problem. Closing.

It may not be your intent, but this reads to me as an impolite way to bump attention on an issue in a project maintained by volunteers. Please consider a different approach, like sponsoring development for features/bugs you want prioritized,

I was closing this issue not because I wanted attention. I did not mean to appear impolite. I am simply working on FOSS myself and hence need to keep my issue backlog ordered.

or asking more politely for attention if you think an issue has gone unnoticed.

I did that.

---

If you think it's worthwhile to keep this open, I don't mind at all, I just close stale issues from time to time.