[
https://issues.apache.org/jira/browse/THRIFT-6184?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer resolved THRIFT-6184.
--------------------------------
Fix Version/s: 0.25.0
Assignee: Jens Geyer
Resolution: Fixed
> 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
> Assignee: Jens Geyer
> Priority: Major
> Fix For: 0.25.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> 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)
