feat(server): set the lowest TLS version the listener negotiates with --min-tls - #56
Merged
Conversation
… --min-tls Ports basecamp#199, open upstream since before April. The HTTPS listener sets no MinVersion today, so it inherits Go's default of TLS 1.2 with no way to require 1.3. Three deliberate departures from basecamp#199: It refuses TLS 1.0 and 1.1 rather than offering them. Go's server default is already 1.2, so those two values in basecamp#199 are pure new downgrade capability -- the only part of that PR that makes a proxy less secure than it is today, and both are deprecated by RFC 8996. --min-tls can only narrow what the listener accepts; a request for 1.0 or 1.1 fails the boot by name instead of quietly serving them. It validates in run's PreRunE as well as at bind time. run had no PreRunE, so a typo previously cost a state restore and an ACME account registration before startHTTPServers rejected it. The listener still parses the same string, so a Config built in code cannot start a listener that ignores the setting. It leaves testServer's signature alone. basecamp#199 threads minTLS through it and rewrites every call site; this fork already had testServerWithConfig. Cipher suites are consciously out of scope, not overlooked: Go ignores CipherSuites for TLS 1.3 and documents PreferServerCipherSuites as having no effect, its default offer set already excludes the insecure suites, and --min-tls 1.3 is what actually removes the CBC suites a scanner flags. The README says so, so the next person sees a decision rather than a gap. Upstream's tls1_2/tls1_3 spellings are accepted, so a config written against basecamp#199 keeps working here. Refs #14
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.
Summary
Adds
--min-tls(envMIN_TLS), the lowest TLS version the HTTPS listener will negotiate. Ports basecamp#199, which has been open upstream since before April 2026.Accepted values are
1.2and1.3. Upstream'stls1_2/tls1_3spellings are accepted too, so a config written against basecamp#199 keeps working here.Wiring:
--min-tls(internal/cmd/run.go, validated in a newpreRun) →Config.MinTLS(internal/server/config.go) →ParseMinTLSVersion→MinVersionon the HTTPS listener'stls.Config(internal/server/server.go). Newinternal/server/tls_version.goholds the parsing.Closes #12
Closes #13
Closes #14
(#12 and #13 were delivered by merged PRs #53 and #50, which said
Refsrather thanClosesand so left them open. Nothing further is needed for either — see the note at the bottom.)Three deliberate departures from upstream basecamp#199
1. The flag cannot weaken the proxy. basecamp#199 accepts
tls1_0andtls1_1. Go's server-side default minimum is already TLS 1.2, so those two values are pure new downgrade capability — the only thing in that PR that makes a proxy less secure than it is today. Both are deprecated by RFC 8996. Here they are refused by name, with an error that says why, and the proxy declines to start rather than pretend the setting took effect.--min-tlscan only ever narrow what the listener accepts.2. Validated at flag-parse time as well as at bind time.
runhad noPreRunE. Without one, a typo'd--min-tlscosts a state restore and an ACME account registration beforestartHTTPServersrejects it.preRunfails in a millisecond instead. The listener parses the same string again, so aConfigbuilt programmatically cannot start a listener that silently ignores the setting.3.
testServer's signature is untouched. basecamp#199 threadsminTLSthrough it and rewrites every call site. This fork already hadtestServerWithConfig(t, config), so the new tests use that and no existing test changes.Cipher suites are deliberately out
The issue title says "Min-TLS version / cipher config". Cipher configuration is consciously excluded, not forgotten, for reasons verified against the Go 1.26.5 source in this toolchain:
Config.CipherSuitesis ignored for TLS 1.3 — so the flag would be inert on exactly the configuration a hardening baseline asks for.PreferServerCipherSuitesis documented as "a legacy field and has no effect".tls.InsecureCipherSuites()(RC4, 3DES, static RSA), so the flag's realistic use is to weaken.--min-tls 1.3already delivers what compliance scanners actually ask for: TLS 1.3 has no CBC suites at all, which is what removes theECDHE_*_CBC_SHAentries a scanner flags.This is documented in the README so the decision is visible to the next person, rather than looking like an oversight.
Test plan
TestParseMinTLSVersion— table over accepted spellings ("",1.2,1.3," 1.3 ",TLSv1.3,tls1_2,tls1_3), refusals (1.0,1.1,tls1_0,TLSv1.1) and non-versions (1.4,tls13,yes,tlsv)TestServer_MinTLSVersionIsEnforcedOnTheHTTPSListener— a real listener refuses a TLS 1.1 client by default, serves 1.2; under1.3it refuses a 1.2 client and serves 1.3TestServer_MinTLSVersionAppliesToMutualTLSHosts— the tripwire for the mTLS interaction: hosts requiring a client certificate get a replacementtls.Config, and Go negotiates the version against that. Only becauseclientCertificateConfigclones does the minimum survive; a fresh config there would downgrade every mTLS host back to 1.2 with no error anywhereTestServer_InvalidMinTLSFailsToStart—Server.Start()refuses a bad valueTestRunCommand_MinTLSFlag/TestRunCommand_MinTLSPreRun— flag registration, and preRun over both accepted and rejected valuesmake testgreen;go vet ./...andgofmt -l internal/ cmd/clean;-racecleanThe version-refusal tests pin both bounds on the client. Go's own client minimum is TLS 1.2, so a
MaxVersion: TLS1.1alone makes the client config self-contradictory and the request fails locally, before a byte reaches the listener — which would pass a refusal test for entirely the wrong reason.Deviations & judgment calls
--min-tls 1.3. Reproduced against a live listener using boulder's actualva/tlsalpn.godial config:peerCerts=1 proto="acme-tls/1" version=0x0304, under both 1.2 and 1.3, including on mTLS hosts.conf.MinVersion = tls.VersionTLS13(internal/handshake/tls_config.go:14), confirmed on the wire.preRunhad no test. Mutation testing proved it: replacingpreRun'sParseMinTLSVersiondelegation with a literalswitchon"", "1.2", "1.3"left the entire suite green while breaking--min-tls tls1_2at boot — the exact upstream-compatibility contract the README in this same diff promises.TestRunCommand_MinTLSPreRunnow carries accept rows and catches that mutation.TestRunCommand_MinTLSFlagfailed wheneverMIN_TLSwas exported. The verifier correctly noted this is a file-wide pre-existing pattern — nineflag.DefValueassertions inrun_test.gobehave the same way, and CI sets noenv:block so exposure there is zero. I fixed only the new test anyway, with onet.Setenv, because this feature is what teaches operators to exportMIN_TLS— a developer with it set is far likelier than one withREUSE_PORTset. Follow-up worth filing: the other eight assertions remain ambient-sensitive; hardening them belongs in its own PR, not a TLS one.--min-tls 1.3is invisible. It fails during the handshake, so no access-log line, no metric, no status code. Nothing in this repo setshttp.Server.ErrorLog, so Go'shttp: TLS handshake errorlines bypass slog. Verified to be pre-existing —newHTTPServeris untouched by this diff and loses the same lines today withMinVersion: 0— so routing handshake errors into slog is its own change. The README warns operators to check what their clients negotiate before setting1.3.DefaultMinTLSVersionpins"1.2"as a string. If a future Go raises its own server default to 1.3, this fork stays at 1.2 while presenting itself as hardened. The alternative — letting unset mean "inherit Go's default" — tracks forward for free but makes the--helpdefault a lie and the behavior untestable. I kept the pin; it is the value Go documents today, and a fleet whose TLS floor moves without a deploy is its own hazard.GetConfigForClientruns beforemutualVersion, so a per-SNI minimum would genuinely be enforced — the mTLS test in this PR proves the mechanism. It is still the wrong scope, because a compliance scanner connects to the IP with no SNI or an unknown one, lands on the listener's own config, and would report an unchanged result against a per-service setting.Refs. Neither has outstanding proxy-side work. Two things I flagged at the time remain genuinely open and are not closed by this: gem-side--tls-client-ca-pathplumbing in../kamal, and the question of whether an mTLS catch-all should be rejected at validation. Say the word and I will file those as their own issues rather than let them disappear with R3: mTLS client certs (--tls-client-ca-path) #12.