Jens Geyer created THRIFT-6184:
----------------------------------

             Summary: Erlang TLS client cannot connect on OTP 26 and later, and 
does not verify the server certificate on earlier releases
                 Key: THRIFT-6184
                 URL: https://issues.apache.org/jira/browse/THRIFT-6184
             Project: Thrift
          Issue Type: Bug
          Components: Erlang - Library
            Reporter: Jens Geyer


The Erlang TLS client sets no verification options. 
thrift_sslsocket_transport.erl defaults ssloptions to [] and passes it straight 
to ssl:connect/3, so what happens depends entirely on the OTP release 
underneath.

Measured on one self-signed certificate for CN=evil.example, connecting to 
127.0.0.1, with the same code on four OTP majors:

    OTP 25.3  ssl 10.9.1.3   ACCEPTED - handshake completes, no validation
    OTP 26    ssl 11.1.4.13  
{options,incompatible,[{verify,verify_peer},{cacerts,undefined}]}
    OTP 27    ssl 11.2.12.12 same
    OTP 28    ssl 11.6.0.5   same

Two separate problems, one cause.

On OTP 25 and earlier the client accepts any certificate. OTP does emit a 
warning report on every such connection ("Server authenticity is not verified 
since certificate path validation is not enabled"), so this is at least visible 
in the log.

>From OTP 26 on, ssl:connect/3 refuses the option list outright rather than 
>treating it as a weaker setting, so thrift_sslsocket_transport.erl:154 falls 
>into its error branch, logs "error while connecting over ssl" and exits. The 
>default TLS client path cannot open a connection at all. OTP 26 shipped in 
>2023. Nothing caught it because no CI job builds Erlang - both build.yml and 
>sca.yml pass --without-erlang - which is what THRIFT-6171 is for.

The change prepends {verify, verify_peer} and, when the caller has named no CAs 
itself, the system trust store from public_key:cacerts_get/0. Prepended rather 
than appended because ssl:connect/3 honours the last occurrence of a duplicated 
option, so a caller passing {verify, verify_none} still gets it.

Two details that are easy to get wrong and are covered by tests:

- {cacerts, _} and {cacertfile, _} are separate options, not two spellings of 
one. Prepending the system store unconditionally does not lose to a caller's 
cacertfile - it wins over it, and the certificate the caller meant to trust is 
rejected as unknown_ca. So the system store is only added when the caller 
supplied neither.
- public_key:cacerts_get/0 raises on a host with no trust store, and does not 
exist before OTP 25. The official erlang:*-slim images have no trust store, so 
this is a reachable path and not a corner case. It is caught, and the option is 
left out rather than falling back to no verification.

After the change, on OTP 25, 26, 27 and 28 alike: a server whose CA is not 
trusted is refused; a caller that names the CA file connects; a caller that 
asks for verify_none connects.

Compatibility: this is a behaviour change on OTP 25 and earlier, where a client 
that today connects to a server with a self-signed or otherwise untrusted 
certificate will stop doing so. {ssloptions, [{verify, verify_none}]} restores 
the old behaviour. On OTP 26 and later there is nothing to break, because the 
path does not currently work at all. Needs a release note.

Tests: nine cases in a new lib/erl/test/test_thrift_sslsocket_transport.erl. 
Five fail before the change, including one that connects to a real TLS listener 
holding test/keys/server.crt and shows the handshake being accepted. 348 -> 349 
eunit, 0 failures; xref and erlfmt clean.

Out of scope, both worth their own tickets: the server-side ssl:handshake path, 
which is unaffected here; and LANGUAGES.md's Erlang language level of 22.0, 
which is demonstrably stale but should be raised on the back of CI coverage 
rather than on one hand-tested transport path.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to