Skip to content
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.

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

x509-cert: support for non-RFC5280 profiles #984

Closed
tarcieri opened this issue Apr 5, 2023 · 8 comments · Fixed by #987
Closed

x509-cert: support for non-RFC5280 profiles #984

tarcieri opened this issue Apr 5, 2023 · 8 comments · Fixed by #987

Comments

@tarcieri
Copy link
Member

tarcieri commented Apr 5, 2023

This crate is described as implementing RFC5280, however we've had a number of requests to support various things which don't conform to the profile, namely serial numbers longer than 20 octets and negative serial numbers.

Some questions:

  1. What profiles do we want to support?
  2. How do we prevent non-RFC5280 certs from being used in contexts where they shouldn't? For example, how do we choose to implement RFC5280 § 6.1? Should we add checks that must be remembered to be called? Use separate types for RFC5280 certs versus other profiles?
  3. What is the impact on CRLs and OCSP?
@lumag
Copy link
Contributor

lumag commented Apr 5, 2023

  1. This might be a tough question, especially if one starts to worry about minor differences broght up by small RFCs. E.g. IDN/IRI, Utf8String vs BMP, etc. After some thought, I'm really leaning towards just two: latest&greatest and everything else.
  2. paragraph 6.1 explicitly RECOMMENDS against including such checks.
  3. If they are handled by the same code, there is no impact. We will parse them in the same way, check the signatures, etc. CRL is a part of RFC 5280, so in theory 20 char boundary also applies there. But OCSP (RFC 6960) doesn't even mention the SerialNumber length. It just tells that serialNumber is the serial number of the certificate for which status is being requested.

@tarcieri
Copy link
Member Author

tarcieri commented Apr 5, 2023

paragraph 6.1 explicitly RECOMMENDS against including such checks.

@lumag if I understand correctly that's in the context of supporting a mixed RFC5280/non-RFC5280 mode, where it's suggesting you perform the same set of checks for all of the certificates, rather than selectively applying RFC5280 to some of the certificates.

However, in a pure RFC5280 mode, definitionally you do want those checks to ensure all certificates are valid according to the profile. Question 2 was asking how that would be implemented (i.e. in such a way that non-conforming certs won't accidentally be considered)

@lumag
Copy link
Contributor

lumag commented Apr 5, 2023

I think 6.1 tells that even if the app has such checks, they should be separate from path-validation algorithm.

@tarcieri
Copy link
Member Author

tarcieri commented Apr 5, 2023

Perhaps we could have something like:

pub struct Certificate<Profile = Rfc5280> { ... }

...which would allow different profiles to define different validation rules.

@carl-wallace
Copy link
Contributor

I think "profile" and "validation" are a bit overloaded. Maybe "<Linter = Rfc5280>" would be closer.

@tarcieri
Copy link
Member Author

tarcieri commented Apr 5, 2023

"Profile" is the term of art from RFC5280. It's used in the name of the RFC and throughout the body, including in comparisons to earlier X.509 profiles (e.g. in Section 6.1)

@carl-wallace
Copy link
Contributor

Yeah, but also in more concrete sense in certificate policies. It's fine.

@richsalz
Copy link

richsalz commented Apr 5, 2023

I like the Profile idea. I'm guessing it means refactoring a bit, so that the base x509, for example, doesn't check on the length of the serial number, while the RFC5280 one does the enforcement. Something like Certificate<Profile=raw> ?

It would be good to have something like a complies method in the profile, so you can test if it's okay to cast :)

baloo added a commit to baloo/formats that referenced this issue Apr 5, 2023
This intends to allow user to relax checks when parsing certificate and
cover for non rfc5280 compliant x509 certificates.

Fixes RustCrypto#978
Fixes RustCrypto#984
baloo added a commit to baloo/formats that referenced this issue Apr 5, 2023
This allow the user to relax checks when parsing certificate and
cover for non rfc5280 compliant x509 certificates.

Fixes RustCrypto#978
Fixes RustCrypto#984
baloo added a commit to baloo/formats that referenced this issue Apr 5, 2023
This allow the user to relax checks when parsing certificate and
cover for non rfc5280 compliant x509 certificates.

Fixes RustCrypto#978
Fixes RustCrypto#984
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants