Repository navigation
fix(service): only log TLS session keys when SSLKEYLOGFILE is set - #684
Merged
Merged
Conversation
This was referenced Oct 6, 2026
Contributor
Author
This was referenced Oct 6, 2026
The SBI server was given a key-log path next to the configuration file on every start, so Go wrote the TLS session secrets of every SBI connection to sslkey.log for the life of the process. Anyone able to read that file can decrypt all SBI traffic the NF served over TLS. http2_util.NewServer already leaves key logging off for an empty path (webconsole passes ""). Take the path from SSLKEYLOGFILE instead, so it is off unless an operator asks for it, and log a warning naming the file when it is on. With key logging off, server.TLSConfig stays nil; ServeTLS still negotiates h2 from http.Server.Protocols, as before. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Edvin Lindqvist <edvin.lindqvist@forsway.com>
The warning fired whenever SSLKEYLOGFILE was set, before the server scheme was known and before the file was opened. With scheme: http no TLS session keys are written, and if the file cannot be opened no key log is attached, so in both cases the warning claimed an exposure that did not exist. Warn in the https case, and only when the server has a KeyLogWriter. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Edvin Lindqvist <edvin.lindqvist@forsway.com>
gab-arrobo
force-pushed
the
fix/gate-tls-key-logging
branch
from
October 7, 2026 15:43
b265efe to
4b059a2
Compare
gab-arrobo
approved these changes
Oct 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The SBI server writes the TLS session secrets of every connection it serves to a file, on every start, with no way to turn it off.
service/init.gohandshttp2_util.NewServera key-log path next to the configuration file, soKeyLogWriteris always set and Go appends the secrets of each TLS session tosslkey.logfor the life of the process. Anyone who can read that file can decrypt all the SBI traffic the SMF has served over TLS, including traffic captured before they read it.This is one of eight PRs, one per NF (amf, nrf, smf, pcf, udr, ausf, udm, nssf), with the same change in each. They are independent and can merge in any order.
What goes wrong
On a deployment with
scheme: httpsand TLS 1.3, collectingsslkey.logfrom the eight NFs was enough to decrypt the SBI requests captured on the same deployment: 138 of them, across the NFs' restart and one UE's registration, PDU session and deregistration, with no configuration changed. The decrypted traffic includes the subscriber identifiers and the key material that crosses the SBI during authentication. TLS is in place and working; the file is what undoes it.http2_util.NewServeralready leaves key logging off for an empty path, andwebconsolepasses"". The NFs each compute a path unconditionally, so the library's gate is never used.The change
The key-log path comes from
SSLKEYLOGFILE, the variable curl, Firefox and Wireshark's documentation use for this. Go's standard library does not read it, so the NF reads it and passes it on. Unset, which is every normal deployment, the path is""and no file is created. Set, the NF logs a warning atWarnthat names the file and says what it exposes, from thehttpscase and only when the server has aKeyLogWriter, that is, only when session keys will actually be written:No change to
util, to the configuration schema, or to the Helm chart.With key logging off,
server.TLSConfigstays nil. That does not change ALPN: the old config set onlyKeyLogWriter, andServeTLStakesh2fromhttp.Server.Protocolsthe same way for a nil config.Limits
scheme: httpandSSLKEYLOGFILEset, the file is created but stays empty, and no warning is logged.SSLKEYLOGFILEnames a path the SMF cannot create, the SBI server does not start, as already happened when the old path could not be created.-tags debug) with util before v1.9.1 uses a debugNewServerthat opens the path unconditionally and fails on an empty one, so it needsSSLKEYLOGFILEset or its SBI server does not start. util v1.9.1 fixes this; this branch pins v1.8.13.Tests
No unit test: the change is in
Start(), which binds the SBI port and blocks. Verified instead:http2_util.NewServer(addr, "", h)withListenAndServeTLS(util v1.8.13, Go 1.26):TLSConfigstays nil, a client offeringh2getsh2and is served HTTP/2.0, one offering onlyhttp/1.1getshttp/1.1, and no key-log file is created.scheme: https, built from the commit each was deployed at plus this change. WithSSLKEYLOGFILEunset:h2withh2;SSLKEYLOGFILEset on the NRF only, and the NRF at log levelwarn: the file appeared, the warning was logged, and the NRF's own traffic in a new capture could be decrypted again (16 requests, all on the NRF's port, none on the other NFs').SSLKEYLOGFILEthe warning is logged once, from thehttpscase, and the file is written; with/nonexistent/dir/sslkey.logthere is no warning, the open error is logged asinitialize HTTP server: create pre-master-secret log [...] fail, and the NRF still serves TLS 1.3 withh2.Gate
pre-commit run --all-filespassed on b265efe in a Linux container (golang:1.26,GOWORK=off): gitleaks v8.30.1, gci v0.14.0, staticcheck v0.8.1,go test -race ./... -count=20, golangci-lint v2.13.2 (the version CI pins), yamlfmt v0.21.0, pre-commit-hooks v6.0.0 and reuse v6.2.0. An earlier run on the same commit failed once ingo test -race, inTestARunRecordsThatItDisplacedAnother(producer/restoration_internal_test.go), which this change does not reach:producerdoes not importservice. Run alone 100 times on this branch's base without this change, that test failed 12 times, so it is flaky onmainand is not addressed here.🤖 Generated with Claude Code