Jens-G opened a new pull request, #3797:
URL: https://github.com/apache/thrift/pull/3797
`thrift_sslsocket_transport` defaults `ssloptions` to `[]` and hands it
straight to `ssl:connect/3`, so what happens depends entirely on the OTP
release underneath. One self-signed certificate for `CN=evil.example`,
connecting to `127.0.0.1`, same code, four 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 problems from one cause.
Up to OTP 25 the client accepts any certificate. OTP does warn on every such
connection, so it is at least visible in the log.
From OTP 26 on, `ssl:connect/3` **rejects the option list outright** rather
than reading it as a weaker setting, so `thrift_sslsocket_transport.erl:154`
falls into its error branch, logs and exits. **The default TLS client path has
not been able to open a connection since 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
Prepend `{verify, verify_peer}`, and the system trust store from
`public_key:cacerts_get/0` when the caller has named no CAs of its own.
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 things here are easy to get wrong, and both have tests:
- **`{cacerts, _}` and `{cacertfile, _}` are separate options, not two
spellings of one.** Prepending the system store does not *lose* to a caller's
`cacertfile` — it *wins over* it, and the certificate the caller meant to trust
comes back as `unknown_ca`. I had this wrong in the first draft and the
cross-version run caught it. The store is now added only 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 rather than a corner case. It is caught, and
the option is left out rather than falling back to no verification.
### Verified across OTP majors
Same server, same client code, with a trust store present:
| | OTP 25 | OTP 26 | OTP 28 |
|---|---|---|---|
| before, `ssloptions = []` | CONNECTED — accepts any cert | cannot even try
| cannot even try |
| after, no caller CA, untrusted-CA server | refused `unknown_ca` | refused
`unknown_ca` | refused `unknown_ca` |
| after, caller names the CA file | **CONNECTED** | **CONNECTED** |
**CONNECTED** |
| after, caller sets `verify_none` | CONNECTED | CONNECTED | CONNECTED |
### Compatibility — worth a reviewer's attention
**Behaviour change on OTP 25 and earlier**, where a client that today
reaches 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 work at all today. Release note needed.
### Tests
Nine cases in a new `lib/erl/test/test_thrift_sslsocket_transport.erl`. Five
fail before the change — including one that stands up a real TLS listener
holding `test/keys/server.crt` and watches the handshake be accepted:
```
Failure/Error: ?assertMatch({'EXIT', _}, connect(Port, []))
expected: = {'EXIT',_}
got: {ok, {t_transport, thrift_buffered_transport, ...}}
```
`348 -> 349` eunit, 0 failures. `xref` clean. `erlfmt` clean on both files —
the one pre-existing warning on `src/thrift_binary_protocol.erl` is on `master`
too and is left alone.
### Not in this PR
- The server-side `ssl:handshake` path, which is unaffected — the listener
handshakes fine on all four majors.
- `LANGUAGES.md` still gives Erlang a language level of 22.0, which is
stale. That should be raised on the back of real CI coverage, not on one
hand-tested transport path.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
