erlang / otp

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

TLS server using intermediate CA with `nameConstraints` extension always results in "Bad Certificate" #8482

Closed lukebakken closed 4 months ago

lukebakken commented 4 months ago

Describe the bug

This is the certificate chain I'm using:

Root CA -> Intermediate CA -> Server/Client certs
           nameConstraints

If the intermediate CA certificate specifies the nameConstraints extension, running a TLS server using Erlang, while requiring client certificates, always results in the following error:

=NOTICE REPORT==== 15-May-2024::06:15:00.194278 ===
TLS server: In state wait_cert at ssl_handshake.erl:2115 generated SERVER ALERT: Fatal - Bad Certificate

=ERROR REPORT==== 15-May-2024::06:15:00.195491 ===
ssl:handshake Error: {error,
                         {tls_alert,
                             {bad_certificate,
                                 "TLS server: In state wait_cert at ssl_handshake.erl:2115 generated SERVER ALERT: Fatal - Bad Certificate\n"}}}

Using openssl s_server and openssl s_client with the certificates works correctly.

To Reproduce

Please see the following repository with code and instructions for reproducing this issue:

https://github.com/lukebakken/erlang-otp-8482

Expected behavior

TLS connection succeeds

Affected versions

Testing with Erlang 26.2.5 linked to OpenSSL 3.3.0 on Ubuntu 22.04

IngelaAndin commented 4 months ago

Berfore looking further in to this would you check if the following will help?

Defalut = ssl:signature_algs(default, 'tlsv1.3'). [eddsa_ed25519,eddsa_ed448,ecdsa_secp521r1_sha512, ecdsa_secp384r1_sha384,ecdsa_secp256r1_sha256, ecdsa_brainpoolP512r1tls13_sha512, ecdsa_brainpoolP384r1tls13_sha384, ecdsa_brainpoolP256r1tls13_sha256,rsa_pss_pss_sha512, rsa_pss_pss_sha384,rsa_pss_pss_sha256,rsa_pss_rsae_sha512, rsa_pss_rsae_sha384,rsa_pss_rsae_sha256,rsa_pkcs1_sha512, rsa_pkcs1_sha384,rsa_pkcs1_sha256, {sha512,ecdsa}, {sha384,ecdsa}, {sha256,ecdsa}]

Then suppling the option: {signature_algs_cert, Default ++ [{sha, rsa}]}

After the release of OTP-27 we will make a patch for the case where this helps to report unsupported cert instead of bad cert #8476

lukebakken commented 4 months ago

Hi @IngelaAndin!

I added your suggested code in this commit -

https://github.com/lukebakken/erlang-otp-8482/commit/90b0dda7fe55418768c21d7e5c5ef2d2484458d4

Unfortunately, I see the same error as before. I have attached the full debug output from the server and client here: run-tls-client-output.txt run-tls-server-output.txt

IngelaAndin commented 4 months ago

Could you trace the functions:

         validate_extensions/4,
         validate_time/3,
         validate_issuer/4,
         validate_names/6,
         validate_signature/6,

in pubkey_cert for the server?

I guess there is some kind of mistake/mismatch in algorithm choice somewhere.

For connivance:

dbg:tracer().
dbg:p(all, [call]).
dbg:tpl(<Module>, <Function>, cx).
lukebakken commented 4 months ago

validate_extensions tracing: validate_extensions.txt

lukebakken commented 4 months ago

validate_time tracing: validate_time.txt

lukebakken commented 4 months ago

validate_issuer: validate_issuer.txt

lukebakken commented 4 months ago

validate_names: validate_names.txt

lukebakken commented 4 months ago

validate_signature: validate_signature.txt

IngelaAndin commented 4 months ago

@lukebakken Thank you for the traces, I can now see that the signature algorithms are not the issue here.

In the validate name trace we see the exception_from {pubkey_cert,validate_names,6} {throw,{bad_cert,name_not_permitted}}

So the name check fails.

Permitted names:

[{'GeneralSubtree',{dNSName,".bakken.io"},0,asn1_NOVALUE}, {'GeneralSubtree',{dNSName,"PROKOFIEV"},0,asn1_NOVALUE}, {'GeneralSubtree',{dNSName,"localhost"},0,asn1_NOVALUE}]

SubjectAltNames extension from cert: [{dNSName,"prokofiev.bakken.io"}, {dNSName,"PROKOFIEV"}, {dNSName,"localhost"}]

First one in SubjectAltName extension will not be equivalent to any of the permitted names.

lukebakken commented 4 months ago

First one in SubjectAltName extension will not be equivalent to any of the permitted names.

That's not how nameConstraints work when the allowed name starts with a .. When a permitted name starts with a ., it means that any name or sub-domain that matches should be allowed:

https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.10

When the constraint begins with a period, it MAY be expanded with one or more labels. That is, the constraint ".example.com" is satisfied by both host.example.com and my.host.example.com.

In my test, the constraint .bakken.io should permit prokofiev.bakken.io.

I should have also said, earlier, that if I do not use an intermediate CA, but specify the same nameConstraints value on the Root CA, which signs the leaf certificate, then everything works fine.

It does seem, in this case, to be the specific combination of using nameConstraints on an intermediate CA certificate.

IngelaAndin commented 4 months ago

First one in SubjectAltName extension will not be equivalent to any of the permitted names.

That's not how nameConstraints work when the allowed name starts with a .. When a permitted name starts with a ., it means that any name or sub-domain that matches should be allowed:

https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.10

Yes you are right, that might just have been my memory failing to remember the details of this check. It looks to be implemented, if it behaves correctly there might be some other reason the check fails.

When the constraint begins with a period, it MAY be expanded with one or more labels. That is, the constraint ".example.com" is satisfied by both host.example.com and my.host.example.com.

In my test, the constraint .bakken.io should permit prokofiev.bakken.io.

I should have also said, earlier, that if I do not use an intermediate CA, but specify the same nameConstraints value on the Root CA, which signs the leaf certificate, then everything works fine.

It does seem, in this case, to be the specific combination of using nameConstraints on an intermediate CA certificate.

The Root (anchor) cert is not part of the path validation in the same way as an intermediate cert, it will setup initial values for the evaluation process but except for expiration it is not really checked in the same manor as the rest of the chain. But still it is the name check that is failing. We could try tracing the pubkey_cert local functions is_permitted and is_excluded. As if we look at the validate_names function the case statement must be returning false.

validate_names(OtpCert, Permit, Exclude, Last, UserState, VerifyFun) ->
    case is_self_signed(OtpCert) andalso (not Last) of
    true ->
        UserState;
    false ->
        TBSCert = OtpCert#'OTPCertificate'.tbsCertificate,
        Subject = TBSCert#'OTPTBSCertificate'.subject,
        Extensions =
        extensions_list(TBSCert#'OTPTBSCertificate'.extensions),
        AltSubject =
        select_extension(?'id-ce-subjectAltName', Extensions),

        EmailAddress = extract_email(Subject),
        Name = [{directoryName, Subject}|EmailAddress],

        AltNames = case AltSubject of
               undefined ->
                   [];
               _ ->
                   AltSubject#'Extension'.extnValue
               end,

        case (is_permitted(Name, Permit) andalso
          is_permitted(AltNames, Permit) andalso
          (not is_excluded(Name, Exclude)) andalso
          (not is_excluded(AltNames, Exclude))) of
        true ->
            UserState;
        false ->
            verify_fun(OtpCert, {bad_cert, name_not_permitted},
                  UserState, VerifyFun)
        end
    end.
lukebakken commented 4 months ago

Yes you are right, that might just have been my memory failing to remember the details of this check.

It's 100% new to me 😸

I will trace that for you today!

lukebakken commented 4 months ago

pubkey_cert-is_permitted.txt pubkey_cert-is_excluded.txt

IngelaAndin commented 4 months ago

Could you add local function match_name to the permitted trace?

lukebakken commented 4 months ago

pubkey_cert-match_name.txt

IngelaAndin commented 4 months ago

@lukebakken Thank you for latest trace, so I though it was a public_key bug, that I tried to fix in #8508 but I was just thinking it was strange that this was not covered by the PKIX test suite, and it turns out it was test are failing now so will have to analyze it further.

IngelaAndin commented 4 months ago

Ok, think I sorted it out now.

lukebakken commented 4 months ago

I just built Erlang from master and verified that dfe17e68a913b2f78904550572e301ae fixes the issue. Thank you!

tubatodd commented 3 months ago

Thank you all for providing this patch. I was the original person who raised this issue to a third-party who ultimately filed this bug. What release will this fix be going into?