erlang / otp

Erlang/OTP
http://erlang.org
Apache License 2.0
11.36k stars 2.95k forks source link

ERL-423: Allow invalid certificate country attributes to be handled by a verify_fun #3398

Closed OTP-Maintainer closed 4 months ago

OTP-Maintainer commented 7 years ago

Original reporter: jwheare Affected version: OTP-19.3.1 Component: public_key Migrated from: https://bugs.erlang.org/browse/ERL-423


Certificate countries are meant to be 2 letter country codes, and erlang enforces this too strictly in my opinion. There exist (usually self-signed) certificates in the wild where the user has set the country to be the full country name rather than a country code, and it would be good to be able to catch this at the API level.

I would expect something like a bad_cert (invalid_issuer or invalid_signature) error to make it to an ssl connection's verify fun in this case, where an application could ignore it if it wants to be lenient. Instead, the connection always fails with an un-skippable {tls_alert, "certificate unknown"} error.

A workaround might be to catch the bad_range exit here:
https://github.com/erlang/otp/blob/773c4d4f0416f25e3c0c6939f8d0871dc4486bab/lib/public_key/src/pubkey_cert_records.erl#L65-L70

Or probably better, make the asn1 more lenient here:
https://github.com/erlang/otp/blob/773c4d4f0416f25e3c0c6939f8d0871dc4486bab/lib/public_key/asn1/OTP-PKIX.asn1#L227-L239

The error could then be reported as normal to any verification functions.
OTP-Maintainer commented 7 years ago

ingela said:

As we already have a workaround ASN-1 read spec it could be possible to make the workaround a little more forgiving.
I am not sure how big we should allow though in that case. 10 letters 20 letters? In that case, there would be no error generated to the verify fun. 

It would be fairly easy to make a  
{code:java}
{bad_cert, cert_decode_error}
{code}
but being specific about what part of the ASN-1 spec would proabably not be as trivial.
OTP-Maintainer commented 7 years ago

jwheare said:

I would favour not adding any limit in the asn-1 spec that would cause a hard failure. Perhaps it could accept any value but add a new validation check called from `public_key:validate`, something like `pubkey_cert:validate_country` that checks the length and produces `bad_cert`. Looking at the existing `bad_cert` failures, none of them are particularly appropriate. Perhaps a new `invalid_country` error would work.
OTP-Maintainer commented 7 years ago

ingela said:

The existing public_key:validate functions validate properties of certificates on a higher abstraction level. These are the properties of the X509 cert RFC, so it is not surprising that the failures does not match. It is sort of expected that the certificates shall follow the spec. The idea is however interesting. validate_country name would be very specific and I think it would be nice to have a more general validate_decode. {bad_cert, {cert_decode_error, Error}} where Error might be something like {countryname, invalid_length}. 
OTP-Maintainer commented 6 years ago

ingela said:

This would probably need new error handling in asn1 application and is not prioritized.
IngelaAndin commented 4 months ago

We think there is no reasonable way to work around this particular breakage of the specification in comparison to what we gain. We are considering some flexibility to public_key:path_validation function that might provide a way to handle decoding errors in general, but it would require the user to have its own ASN1 decoding function.