Skip to content

proposal: crypto/x509: flag for more lax certificate parsing #70324

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

Closed
chris-jansson opened this issue Nov 13, 2024 · 7 comments
Closed

proposal: crypto/x509: flag for more lax certificate parsing #70324

chris-jansson opened this issue Nov 13, 2024 · 7 comments
Labels
Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone

Comments

@chris-jansson
Copy link

chris-jansson commented Nov 13, 2024

Proposal Details

Hello team!

I would like to propose a strict/lenient flag to be supported by the crypto/x509 library, so that certificates can be successfully parsed even if they technically violate some naming rules. Please see my use case below:


I need to load a set of X.509 certificates from a remote system into my Go application. The certs are not under my control, but as I've now learned, the Go standard crypto library won't parse them if the Subject field is illegal.

Specifically, when calling x509.ParseCertificate(), I get the following error:

x509: invalid RDNSequence: invalid attribute value: invalid PrintableString

I get this error on a couple certs, because they have an underscore character in the Subject field.

I understand that the ASN.1 PrintableString spec defines the legal characters, and these certs are in violation of that, but again, the certs are not under my control. Meanwhile, I can parse the certs successfully with openssl and Java crypto libraries. This seems overly strict of Go, as developers often work with certs they didn't create.

I also tried to find some third party Go crypto libraries that were more lenient, but I didn't have much luck. The only choice I seem to have is to copy/paste the standard library's parser code into my application, and remove the checks. I certainly don't like this idea, that could likely introduce vulnerabilities in my application, and loses the value of the trusted, vetted Go standard library.

Can this strictness please be re-evaluated? Thanks!

@gabyhelp
Copy link

Related Issues

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@seankhliao seankhliao changed the title crypto/x509: Add strict flag for parsing X.509 certificates proposal: crypto/x509: flag for more lax certificate parsing Nov 13, 2024
@seankhliao seankhliao added the Proposal-Crypto Proposal related to crypto packages or other security issues label Nov 13, 2024
@gopherbot gopherbot added this to the Proposal milestone Nov 13, 2024
@seankhliao
Copy link
Member

cc @golang/security

@chris-jansson
Copy link
Author

chris-jansson commented Nov 13, 2024

Thanks @gabyhelp for the link, that is a similar issue to what I am having, although the author there was satisfied without a resolution.

The ZCrypto suggestion is one that I looked at before posting this issue, but as that project states in its README, it is meant for experimental use only, and should not be used for production systems.

I unfortunately fall into the production systems category

@seankhliao
Copy link
Member

Looking at that, I understand that the positioning of the standard library's crypto packages hasn't changed. We generally agree with the rationale behind RFC 9413.

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Nov 13, 2024
@rolandshoemaker
Copy link
Member

To elaborate:

Are these certificates you are parsing publicly trusted? As the crypto/x509 package doc states: "The package targets the X.509 technical profile defined by the IETF (RFC 2459/3280/5280), and as further restricted by the CA/Browser Forum Baseline Requirements. There is minimal support for features outside of these profiles, as the primary goal of the package is to provide compatibility with the publicly trusted TLS certificate ecosystem and its policies and constraints."

We tend to try to eschew complexity needed for the myriad of additional uses of X509. Adding a lax parsing mode on its face seems extremely complicated, and would require us to carry multiple code paths in many places in order to support extremely niche uses of the package. On balance it seems unlikely to add enough value to justify the additional complexity.

@chris-jansson
Copy link
Author

To answer Roland, the certificates in question are internal to my organization, and come from a trusted source. I can't speak to the complexity of the implementation that would be required here, although I would have guessed fairly small.

I appreciate your thoughts. Hm. I certainly respect the effort to encourage protocol interoperability long-term, but nonetheless I am pretty taken aback at the lack of flexibility. A simple example that comes to mind is how curl allows you to bypass SSL verification, because sometimes you're just testing. And back to the original topic, sometimes you're simply working in less-than-perfect environments where you don't control all the variables.

I understand that the projects has guiding principles, though. As for me, I will certainly hesitate before using Go again on any project working with certificates or cryptography. It appears there is a fairly high risk of running afoul of the unbending rules.

@ianlancetaylor
Copy link
Member

You can, of course, copy the code and modify it for your purposes.

The question here is whether the Go project should add flexibility in the standard library. Since Go is very strict about backward compatibility, flexibility is a maintenance cost that has to be borne for the foreseeable future. And, flexibility is the antithesis of security, as each optional choice opens up additional possible security holes. And, the holes expand exponentially in the number of choices, as we have to consider all combinations of all choices.

We do have to be pragmatic: if many systems require a different approach, then we have to adapt. But with only a single report, where the data clearly violates the standards, there doesn't seem to be a strong argument here. Sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
None yet
Development

No branches or pull requests

6 participants