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

ssl: Bad Certificate errors on perfectly valid SSL hosts #8588

Closed
1player opened this issue Jun 17, 2024 · 10 comments
Closed

ssl: Bad Certificate errors on perfectly valid SSL hosts #8588

1player opened this issue Jun 17, 2024 · 10 comments
Assignees
Labels
bug Issue is reported as a bug not a bug Issue is determined as not a bug by OTP team:PS Assigned to OTP team PS

Comments

@1player
Copy link

1player commented Jun 17, 2024

Describe the bug

Many HTTPS hosts cannot be connected to because of error:

{:error,
 {:tls_alert,
  {:bad_certificate,
   ~c"TLS client: In state wait_cert_cr at ssl_handshake.erl:2113 generated CLIENT ALERT: Fatal - Bad Certificate\n"}}}

To Reproduce

From an Elixir shell:

:ssl.connect('api.smtp2go.com', 443, cacerts: :public_key.cacerts_get())

Also try with:

Affected versions
At least 26.1.2 to 26.2.5 included.

Additional context

See also elixir-mint/mint#421

The maintainer of the Mint project explained this is caused by incorrect ordering of the certificate chain, but whatever the reason, other non-Erlang HTTPS libraries are perfectly happy with validating these websites, while Erlang fails because it's apparently too strict. Testing of these hosts with tools like https://www.ssllabs.com/ssltest/ presents no such validation issue.

I am building a web crawler that I've had to turn SSL verification off because of this very problem; the issue now is that hosts that once used to work, such as my transactional mail provider SMTP2GO at "api.smtp2go.com", after a certificate update might break and cause great disruption in production services. As it stands, I have been unable to use that service for the past month as Erlang decided the certificate is not valid anymore.

@1player 1player added the bug Issue is reported as a bug label Jun 17, 2024
@1player
Copy link
Author

1player commented Jun 17, 2024

Can reproduce with OTP 27 (elixir:1.17-otp-27)

CANNOT reproduce on OTP 25 (using image elixir:1.17-otp-25) because :ssl.connect does not set {:verify, :verify_peer} by default. When I add the option, fails with Fatal - Handshake Failure

@IngelaAndin IngelaAndin added the team:PS Assigned to OTP team PS label Jun 17, 2024
@IngelaAndin
Copy link
Contributor

When you verify the certificates you need to also supply appropriate other options for the verification. Like sha algorithm is not supported by default since OTP-26 . First example also needs to customize the host name check.

Erlang/OTP 27 [erts-15.0] [source-c183ff4f3f] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit:ns]

Eshell V15.0 (press Ctrl+G to abort, type help(). for help)
1> ssl:start().
ok
2> ssl:connect("api.smtp2go.com", 443, [{customize_hostname_check, [{match_fun, public_key:pkix_verify_hostname_match_fun(https)}]}, {signature_algs, ssl:signature_algs(all, 'tlsv1.3')}, {cacerts, public_key:cacerts_get()}]).
{ok,{sslsocket,{gen_tcp,#Port<0.5>,tls_connection,undefined},
[<0.123.0>,<0.122.0>]}}
3> ssl:connect("www.kb.cert.org", 443, [{signature_algs, ssl:signature_algs(all, 'tlsv1.3')}, {cacerts, public_key:cacerts_get()}]).
{ok,{sslsocket,{gen_tcp,#Port<0.6>,tls_connection,undefined},
[<0.128.0>,<0.127.0>]}}
4> ssl:connect("people.bath.ac.uk", 443, [{signature_algs, ssl:signature_algs(all, 'tlsv1.3')}, {cacerts, public_key:cacerts_get()}]).
{ok,{sslsocket,{gen_tcp,#Port<0.7>,tls_connection,undefined},
[<0.133.0>,<0.132.0>]}}
5>

Erlang/OTP 26 [erts-14.2.5] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit:ns]

Eshell V14.2.5 (press Ctrl+G to abort, type help(). for help)
1> ssl:start().
ok
2> ssl:connect("api.smtp2go.com", 443, [{customize_hostname_check, [{match_fun, public_key:pkix_verify_hostname_match_fun(https)}]}, {signature_algs, ssl:signature_algs(all, 'tlsv1.3')}, {cacerts, public_key:cacerts_get()}]).
{ok,{sslsocket,{gen_tcp,#Port<0.5>,tls_connection,undefined},
[<0.124.0>,<0.123.0>]}}
3> ssl:connect("www.kb.cert.org", 443, [{signature_algs, ssl:signature_algs(all, 'tlsv1.3')}, {cacerts, public_key:cacerts_get()}]).
{ok,{sslsocket,{gen_tcp,#Port<0.6>,tls_connection,undefined},
[<0.129.0>,<0.128.0>]}}
4> ssl:connect("people.bath.ac.uk", 443, [{signature_algs, ssl:signature_algs(all, 'tlsv1.3')}, {cacerts, public_key:cacerts_get()}]).
{ok,{sslsocket,{gen_tcp,#Port<0.7>,tls_connection,undefined},
[<0.134.0>,<0.133.0>]}}
5>

Note that you may not want to enable all non default signature algorithms. See User guide for guidance.

@IngelaAndin IngelaAndin added the not a bug Issue is determined as not a bug by OTP label Jun 17, 2024
@1player
Copy link
Author

1player commented Jun 17, 2024 via email

@IngelaAndin
Copy link
Contributor

I do not agree that it is premature not support it by default. It is not deprecated we just want the
user to make the security tradeoff. Come to think of it this is actually a good use for signature_algs_cert option that lets you support different algorithms for cert signing than for other uses of digital signatures in the protocol. And it would be much less serious to choose all algorithms here. But I think we could consider having a default for this one including {sha,rsa} for interoperability reasons it currently has no default value which in practice make it fallbacks to singnature_algs option.

@liamwhite
Copy link

Regarding the need to explicitly add {sha, rsa}, it seems like we cannot express the following constraint as ssl opts:

  • Allow SHA1WithRSA for trusted root certificates in the chain
  • Disallow SHA1WithRSA by default otherwise

This is what browsers and openssl s_client appear to do. Is there a possibility ssl can support this?

@IngelaAndin IngelaAndin self-assigned this Jun 22, 2024
@IngelaAndin IngelaAndin reopened this Jun 22, 2024
@IngelaAndin
Copy link
Contributor

I will make this one open as I we are considering to make some interop change. I will investigate @liamwhite suggestion when back at work next week.

@IngelaAndin
Copy link
Contributor

@liamwhite There is no need to explicitly configure that behavior that you are describing. It will work as

From TLS-1.3 RFC:

The signatures on certificates that are self-signed or certificates
   that are trust anchors are not validated, since they begin a
   certification path (see [[RFC5280], Section 3.2](https://datatracker.ietf.org/doc/html/rfc5280#section-3.2)).  A certificate that
   begins a certification path MAY use a signature algorithm that is not
   advertised as being supported in the "signature_algorithms"
   extension.

@liamwhite
Copy link

Okay. Is that describing the behavior that will be implemented in OTP 28, or how it is supposed to work in OTP 27?

@IngelaAndin
Copy link
Contributor

@liamwhite this already works. The thing that we are considering changing is if there is an intermediate CA that is signed using sha1 we could allow that although not allowing sha1 for TLS protocol signatures, and that that should be default.

@1player
Copy link
Author

1player commented Jun 26, 2024

I do believe that allowing intermediate SHA1 is probably the sanest approach here, given that they're still out there for a few years on a large number of Internet hosts.

IngelaAndin added a commit to IngelaAndin/otp that referenced this issue Jun 28, 2024
Make upgrade path smoother by adding rsa_pkcs1_sha to the
default of signature_algs as the default signature_algs_cert.
Note this is only applicable when signature_algs is not configured,
that is set to the default, that will then become the default of
signature_algs_cert in practice. This will allow certificates to
use rsa_pkcs1_sha algorithm but still disallow it in the TLS protocol.

Also add some missing handling of signature_algs_cert in DTLS.

closes erlang#8588
IngelaAndin added a commit to IngelaAndin/otp that referenced this issue Jun 28, 2024
Make upgrade path smoother by adding rsa_pkcs1_sha to the
default of signature_algs as the default signature_algs_cert.
Note this is only applicable when signature_algs is not configured,
that is set to the default, that will then become the default of
signature_algs_cert in practice. This will allow certificates to
use rsa_pkcs1_sha algorithm but still disallow it in the TLS protocol.

Also add some missing handling of signature_algs_cert in DTLS.

closes erlang#8588
IngelaAndin added a commit to IngelaAndin/otp that referenced this issue Jul 8, 2024
Make upgrade path smoother by adding rsa_pkcs1_sha to the
default of signature_algs as the default signature_algs_cert.
Note this is only applicable when signature_algs is not configured,
that is set to the default, that will then become the default of
signature_algs_cert in practice. This will allow certificates to
use rsa_pkcs1_sha algorithm but still disallow it in the TLS protocol.

Also add some missing handling of signature_algs_cert in DTLS.

closes erlang#8588
IngelaAndin added a commit that referenced this issue Jul 9, 2024
* ingela/ssl/default-cert-sign/GH-8588/OTP-19152:
  ssl: Add signature_algs_cert default when signature_algs default is used
  ssl: Test root cert is allowed to use any signature.
IngelaAndin added a commit that referenced this issue Jul 9, 2024
Make upgrade path smoother by adding rsa_pkcs1_sha to the
default of signature_algs as the default signature_algs_cert.
Note this is only applicable when signature_algs is not configured,
that is set to the default, that will then become the default of
signature_algs_cert in practice. This will allow certificates to
use rsa_pkcs1_sha algorithm but still disallow it in the TLS protocol.

Also add some missing handling of signature_algs_cert in DTLS.

closes #8588
IngelaAndin pushed a commit that referenced this issue Jul 10, 2024
…' into maint-26

* ingela/maint-26/ssl/default-cert-sign/GH-8588/OTP-19152:
  ssl: Add signature_algs_cert default when signature_algs default is used
  ssl: Test root cert is allowed to use any signature.
IngelaAndin pushed a commit that referenced this issue Jul 10, 2024
…int-27

* ingela/ssl/default-cert-sign/GH-8588/OTP-19152:
  ssl: Add signature_algs_cert default when signature_algs default is used
  ssl: Test root cert is allowed to use any signature.

# Conflicts:
#	lib/ssl/test/ssl_api_SUITE.erl
#	lib/ssl/test/ssl_cert_SUITE.erl
jjcarstens added a commit to nerves-hub/nerves_hub_web that referenced this issue Aug 2, 2024
This is needed to support devices using OTP 27.0.1 (using `nerves_system_br`
>= 1.28.2).

OTP 27.0 was missing some handling of `ecdsa` signature algs for certificates
and would only allow `SHA1` for ecdsa certs. Device Certificates connecting
to NervesHub typically use `{:sha256, :ecdsa}` which would not match when
comparing allowed `:signature_algs_cert` option of the incoming client hello.

This was fixed as part of erlang/otp#8588 via
[`e57bfe6d`](erlang/otp@e57bfe6#diff-519ed7d3ffd869a0cf148a8b2fb6136d280147fa1d5c2aa6496a8fd2fc7ad188R1747-R1749)
to support checking mutliple `:ecdsa` options
jjcarstens added a commit to nerves-hub/nerves_hub_web that referenced this issue Aug 2, 2024
This is needed to support devices using OTP 27.0.1 (using `nerves_system_br`
>= 1.28.2).

OTP 27.0 was missing some handling of `ecdsa` signature algs for certificates
and would only allow `SHA1` for ecdsa certs. Device Certificates connecting
to NervesHub typically use `{:sha256, :ecdsa}` which would not match when
comparing allowed `:signature_algs_cert` option of the incoming client hello.

This was fixed as part of erlang/otp#8588 via
[`e57bfe6d`](erlang/otp@e57bfe6#diff-519ed7d3ffd869a0cf148a8b2fb6136d280147fa1d5c2aa6496a8fd2fc7ad188R1747-R1749)
to support checking mutliple `:ecdsa` options
liamwhite added a commit to philomena-dev/philomena that referenced this issue Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is reported as a bug not a bug Issue is determined as not a bug by OTP team:PS Assigned to OTP team PS
Projects
None yet
Development

No branches or pull requests

3 participants