From df178da1e7662018b4f3207925ead912533b0c7d Mon Sep 17 00:00:00 2001 From: mhenrixon Date: Wed, 29 Jul 2026 10:54:30 +0200 Subject: [PATCH] feat(server): set the lowest TLS version the listener negotiates with --min-tls Ports basecamp/kamal-proxy#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 #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 #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. #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 #199 keeps working here. Refs #14 --- README.md | 29 +++++ internal/cmd/run.go | 19 ++- internal/cmd/run_test.go | 64 ++++++++++ internal/server/config.go | 6 + internal/server/server.go | 17 +++ internal/server/tls_version.go | 52 +++++++++ internal/server/tls_version_test.go | 173 ++++++++++++++++++++++++++++ 7 files changed, 357 insertions(+), 3 deletions(-) create mode 100644 internal/server/tls_version.go create mode 100644 internal/server/tls_version_test.go diff --git a/README.md b/README.md index 10a32214..33b003fa 100644 --- a/README.md +++ b/README.md @@ -438,6 +438,35 @@ or `--tls-domains-source` — serves every hostname no other service claims, so client CA applies to all of them. +### Minimum TLS version + +The HTTPS listener negotiates TLS 1.2 and above by default. To refuse TLS 1.2 as +well and serve only TLS 1.3, start the proxy with `--min-tls` (or the `MIN_TLS` +environment variable): + + kamal-proxy run --min-tls 1.3 + +Accepted values are `1.2` and `1.3`. TLS 1.0 and 1.1 cannot be enabled — they are +deprecated by [RFC 8996](https://www.rfc-editor.org/rfc/rfc8996) and Go's TLS +stack already declines to serve them, so the proxy refuses to start rather than +pretend the setting took effect. The `tls1_2` / `tls1_3` spellings are accepted +too, so a configuration written against upstream kamal-proxy keeps working. + +This is a listener-wide setting: it applies to every service, including hosts +that require client certificates. The HTTP/3 listener is always TLS 1.3, since +QUIC is only defined over TLS 1.3 ([RFC 9001](https://www.rfc-editor.org/rfc/rfc9001)), +and `--min-tls` neither lowers nor raises it. + +Raising the minimum locks out clients that cannot reach it, and they fail during +the handshake — before the request reaches the HTTP layer, so nothing appears in +the access log. Check what your clients actually negotiate before setting `1.3`. + +Cipher suites are deliberately not configurable. Go does not allow selecting TLS +1.3 cipher suites at all, and its TLS 1.2 defaults already exclude the RC4, 3DES +and static-RSA suites that a hardening baseline asks you to remove — so a cipher +flag could only weaken the proxy, while silently doing nothing on TLS 1.3. + + ### SAN Certificate Batching When started with `--acme-email` (or the `ACME_EMAIL` environment variable), diff --git a/internal/cmd/run.go b/internal/cmd/run.go index 54fd7d9c..5a211d12 100644 --- a/internal/cmd/run.go +++ b/internal/cmd/run.go @@ -25,9 +25,10 @@ type runCommand struct { func newRunCommand() *runCommand { runCommand := &runCommand{} runCommand.cmd = &cobra.Command{ - Use: "run", - Short: "Run the server", - RunE: runCommand.run, + Use: "run", + Short: "Run the server", + PreRunE: runCommand.preRun, + RunE: runCommand.run, } runCommand.cmd.Flags().BoolVar(&runCommand.debugLogsEnabled, "debug", getEnvBool("DEBUG", false), "Include debugging logs") @@ -51,6 +52,7 @@ func newRunCommand() *runCommand { runCommand.cmd.Flags().DurationVar(&globalConfig.ShutdownTimeout, "shutdown-timeout", getEnvDuration("SHUTDOWN_TIMEOUT", server.DefaultShutdownTimeout), "Maximum time to wait for in-flight requests to drain on shutdown") // ACME/TLS configuration + runCommand.cmd.Flags().StringVar(&globalConfig.MinTLS, "min-tls", getEnvString("MIN_TLS", server.DefaultMinTLSVersion), "Lowest TLS version the HTTPS listener will negotiate: 1.2 or 1.3 (TLS 1.0 and 1.1 cannot be enabled; HTTP/3 is always 1.3)") runCommand.cmd.Flags().StringVar(&globalConfig.ACMEEmail, "acme-email", getEnvString("ACME_EMAIL", ""), "Email address for ACME account registration (required for automatic TLS)") runCommand.cmd.Flags().StringVar(&globalConfig.ACMEDirectory, "acme-directory", getEnvString("ACME_DIRECTORY", server.LetsEncryptProduction), "ACME directory URL") runCommand.cmd.Flags().StringVar(&runCommand.acmeDNSProvider, "acme-dns-provider", getEnvString("ACME_DNS_PROVIDER", "auto"), "DNS provider for DNS-01 challenges (cloudflare, route53, digitalocean, gcloud, namecheap, godaddy, hetzner, vultr, auto)") @@ -60,6 +62,17 @@ func newRunCommand() *runCommand { return runCommand } +// preRun rejects a bad --min-tls before the command restores routing state or +// registers an ACME account, so a typo costs a millisecond rather than a round +// trip to Let's Encrypt. The listener parses it again at bind time. +func (c *runCommand) preRun(cmd *cobra.Command, args []string) error { + if _, err := server.ParseMinTLSVersion(globalConfig.MinTLS); err != nil { + return err + } + + return nil +} + func (c *runCommand) run(cmd *cobra.Command, args []string) error { c.setLogger() diff --git a/internal/cmd/run_test.go b/internal/cmd/run_test.go index c082f4c2..850b6f3b 100644 --- a/internal/cmd/run_test.go +++ b/internal/cmd/run_test.go @@ -155,3 +155,67 @@ func TestGetEnvDuration(t *testing.T) { }) } } + +func TestRunCommand_MinTLSFlag(t *testing.T) { + // This feature teaches operators to export MIN_TLS, so a developer running the + // suite is unusually likely to have it set. The prefixed key wins in findEnv, + // so pinning it here makes the default deterministic. Setting MIN_TLS to "" + // would not work: findEnv reports an empty value as present. + t.Setenv("KAMAL_PROXY_MIN_TLS", server.DefaultMinTLSVersion) + + globalConfig = server.Config{} + + cmd := newRunCommand().cmd + + flag := cmd.Flags().Lookup("min-tls") + require.NotNil(t, flag) + assert.Equal(t, server.DefaultMinTLSVersion, flag.DefValue) + + require.NoError(t, cmd.Flags().Parse([]string{"--min-tls=1.3"})) + assert.Equal(t, "1.3", globalConfig.MinTLS) +} + +func TestRunCommand_MinTLSPreRun(t *testing.T) { + tests := []struct { + name string + value string + expectedError string + }{ + // The accepted spellings have to be exercised through the command, not + // just through ParseMinTLSVersion: preRun is what an operator's value + // actually passes through, and the README promises upstream's tls1_2 + // grammar keeps working here. + {name: "default", value: server.DefaultMinTLSVersion}, + {name: "1.3", value: "1.3"}, + {name: "upstream tls1_2 grammar", value: "tls1_2"}, + {name: "upstream tls1_3 grammar", value: "tls1_3"}, + {name: "TLSv prefix", value: "TLSv1.3"}, + + {name: "TLS 1.0", value: "1.0", expectedError: "cannot be enabled"}, + {name: "TLS 1.1", value: "1.1", expectedError: "cannot be enabled"}, + {name: "upstream tls1_0 grammar", value: "tls1_0", expectedError: "cannot be enabled"}, + {name: "not a version", value: "banana", expectedError: "is not a TLS version"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + globalConfig = server.Config{} + + runCommand := newRunCommand() + require.NoError(t, runCommand.cmd.Flags().Parse([]string{"--min-tls=" + tt.value})) + + err := runCommand.preRun(runCommand.cmd, nil) + + if tt.expectedError != "" { + require.ErrorContains(t, err, tt.expectedError) + require.ErrorContains(t, err, "min-tls") + return + } + + require.NoError(t, err) + // The server parses this same string again at bind time, so what + // preRun accepted has to be what reaches the listener. + assert.Equal(t, tt.value, globalConfig.MinTLS) + }) + } +} diff --git a/internal/server/config.go b/internal/server/config.go index 59ba5441..5e564235 100644 --- a/internal/server/config.go +++ b/internal/server/config.go @@ -46,6 +46,12 @@ type Config struct { MetricsPort int HTTP3Enabled bool + // MinTLS is the lowest TLS version the HTTPS listener will negotiate, + // written as "1.2" or "1.3". Empty means 1.2, which is also Go's own + // minimum, so this setting can only ever narrow what the listener accepts - + // ParseMinTLSVersion refuses TLS 1.0 and 1.1 outright. + MinTLS string + // MetricsAllowIPs restricts the metrics endpoint to these addresses and CIDR // ranges. Empty (the default) serves everyone that can reach the port. MetricsAllowIPs []string diff --git a/internal/server/server.go b/internal/server/server.go index 1942cdf5..d549a339 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -209,6 +209,8 @@ func (s *Server) startHTTP3Server(handler http.Handler, httpsAddr string) error } http3Config := &tls.Config{ + // QUIC is defined only over TLS 1.3 (RFC 9001), so --min-tls can + // neither lower this listener nor needs to raise it. MinVersion: tls.VersionTLS13, NextProtos: []string{"h3"}, GetCertificate: s.router.GetCertificate, @@ -234,6 +236,13 @@ func (s *Server) startHTTPServers() error { return err } + // Parsed here too, not just in the run command, so a Config built directly + // cannot start a listener that silently ignores the setting. + minTLSVersion, err := ParseMinTLSVersion(s.config.MinTLS) + if err != nil { + return err + } + httpAddr := fmt.Sprintf("%s:%d", s.config.Bind, s.config.HttpPort) httpsAddr := fmt.Sprintf("%s:%d", s.config.Bind, s.config.HttpsPort) @@ -265,12 +274,20 @@ func (s *Server) startHTTPServers() error { handler.ServeHTTP(w, r) })) httpsConfig := &tls.Config{ + MinVersion: minTLSVersion, NextProtos: []string{"h2", "http/1.1", acme.ALPNProto}, GetCertificate: s.router.GetCertificate, } httpsConfig.GetConfigForClient = s.clientCertificateConfig(httpsConfig) s.httpsServer.TLSConfig = httpsConfig + if minTLSVersion > tls.VersionTLS12 { + // Worth a line in the boot log: it is the operator's only confirmation + // that the narrowing took effect, and clients it locks out fail in the + // handshake with nothing to show for it at the HTTP layer. + slog.Info("Minimum TLS version raised", "min_tls", tls.VersionName(minTLSVersion)) + } + if s.config.ProxyProtocol { slog.Info("PROXY protocol enabled", "allow", s.config.ProxyProtocolAllowIPs) diff --git a/internal/server/tls_version.go b/internal/server/tls_version.go new file mode 100644 index 00000000..81a8ce03 --- /dev/null +++ b/internal/server/tls_version.go @@ -0,0 +1,52 @@ +package server + +import ( + "crypto/tls" + "fmt" + "strings" +) + +// DefaultMinTLSVersion is the minimum version the HTTPS listener negotiates +// when --min-tls is not given. It matches Go's own server-side minimum, so the +// default states an intent rather than changing behavior. +const DefaultMinTLSVersion = "1.2" + +// ParseMinTLSVersion maps a --min-tls value onto a crypto/tls version constant. +// +// Only 1.2 and 1.3 are accepted. The flag exists to narrow what the listener +// will negotiate for a compliance scanner, never to widen it: Go already +// refuses TLS 1.0 and 1.1, and accepting them here would make a hardening flag +// the only way to downgrade the proxy. An empty value means the default, so an +// unset or blank MIN_TLS boots instead of failing. +func ParseMinTLSVersion(value string) (uint16, error) { + if strings.TrimSpace(value) == "" { + return tls.VersionTLS12, nil + } + + switch normalized := normalizeTLSVersion(value); normalized { + case "1.2": + return tls.VersionTLS12, nil + case "1.3": + return tls.VersionTLS13, nil + case "1.0", "1.1": + return 0, fmt.Errorf("min-tls: TLS %s cannot be enabled; the lowest accepted minimum is %s", normalized, DefaultMinTLSVersion) + default: + return 0, fmt.Errorf("min-tls: %q is not a TLS version; use %s or 1.3", value, DefaultMinTLSVersion) + } +} + +// normalizeTLSVersion reduces the spellings operators actually type - and the +// tls1_2 form used by basecamp/kamal-proxy#199 - to a bare "1.2"/"1.3", so a +// config written for upstream keeps working here. +func normalizeTLSVersion(value string) string { + normalized := strings.ToLower(strings.TrimSpace(value)) + + for _, prefix := range []string{"tlsv", "tls"} { + if trimmed, found := strings.CutPrefix(normalized, prefix); found { + normalized = trimmed + break + } + } + + return strings.ReplaceAll(normalized, "_", ".") +} diff --git a/internal/server/tls_version_test.go b/internal/server/tls_version_test.go new file mode 100644 index 00000000..4f48e68f --- /dev/null +++ b/internal/server/tls_version_test.go @@ -0,0 +1,173 @@ +package server + +import ( + "crypto/tls" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseMinTLSVersion(t *testing.T) { + tests := []struct { + value string + expected uint16 + expectedError string + }{ + {value: "", expected: tls.VersionTLS12}, + {value: "1.2", expected: tls.VersionTLS12}, + {value: "1.3", expected: tls.VersionTLS13}, + {value: " 1.3 ", expected: tls.VersionTLS13}, + {value: "TLSv1.3", expected: tls.VersionTLS13}, + // The grammar basecamp/kamal-proxy#199 uses, so an upstream config keeps working. + {value: "tls1_2", expected: tls.VersionTLS12}, + {value: "tls1_3", expected: tls.VersionTLS13}, + + {value: "1.0", expectedError: "TLS 1.0 cannot be enabled"}, + {value: "1.1", expectedError: "TLS 1.1 cannot be enabled"}, + {value: "tls1_0", expectedError: "TLS 1.0 cannot be enabled"}, + {value: "TLSv1.1", expectedError: "TLS 1.1 cannot be enabled"}, + + {value: "1.4", expectedError: `"1.4" is not a TLS version`}, + {value: "tls13", expectedError: `"tls13" is not a TLS version`}, + {value: "yes", expectedError: `"yes" is not a TLS version`}, + {value: "tlsv", expectedError: `"tlsv" is not a TLS version`}, + } + + for _, tt := range tests { + t.Run(tt.value, func(t *testing.T) { + version, err := ParseMinTLSVersion(tt.value) + + if tt.expectedError != "" { + require.ErrorContains(t, err, tt.expectedError) + require.ErrorContains(t, err, "min-tls") + assert.Zero(t, version) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.expected, version) + }) + } +} + +func TestServer_MinTLSVersionIsEnforcedOnTheHTTPSListener(t *testing.T) { + t.Run("the default refuses TLS 1.1 and serves TLS 1.2", func(t *testing.T) { + server := testMinTLSServer(t, "", nil) + + _, err := testRequestWithTLSVersions(t, server, tls.VersionTLS10, tls.VersionTLS11) + require.Error(t, err) + assert.ErrorContains(t, err, "protocol version not supported") + + resp, err := testRequestWithTLSVersions(t, server, tls.VersionTLS12, tls.VersionTLS12) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + }) + + t.Run("1.3 refuses a TLS 1.2 client", func(t *testing.T) { + server := testMinTLSServer(t, "1.3", nil) + + _, err := testRequestWithTLSVersions(t, server, tls.VersionTLS12, tls.VersionTLS12) + require.Error(t, err) + assert.ErrorContains(t, err, "protocol version not supported") + }) + + t.Run("1.3 still serves TLS 1.3", func(t *testing.T) { + server := testMinTLSServer(t, "1.3", nil) + + resp, err := testRequestWithTLSVersions(t, server, tls.VersionTLS13, tls.VersionTLS13) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + require.NotNil(t, resp.TLS) + assert.Equal(t, uint16(tls.VersionTLS13), resp.TLS.Version) + }) +} + +// Hosts requiring a client certificate get a REPLACEMENT tls.Config, and Go +// negotiates the version against that config rather than the listener's. Only +// because clientCertificateConfig clones does the minimum survive; building a +// fresh config there downgrades every mTLS host back to TLS 1.2 with no error +// anywhere. This test is the tripwire for that. +func TestServer_MinTLSVersionAppliesToMutualTLSHosts(t *testing.T) { + ca := generateTestCA(t) + server := testMinTLSServer(t, "1.3", ca) + clientCert := ca.issueClientCertificate(t) + + t.Run("refuses a valid client certificate offered over TLS 1.2", func(t *testing.T) { + _, err := testRequestWithTLSVersions(t, server, tls.VersionTLS12, tls.VersionTLS12, clientCert) + require.Error(t, err) + assert.ErrorContains(t, err, "protocol version not supported") + }) + + t.Run("accepts the same certificate over TLS 1.3", func(t *testing.T) { + resp, err := testRequestWithTLSVersions(t, server, tls.VersionTLS13, tls.VersionTLS13, clientCert) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + require.NotNil(t, resp.TLS) + assert.Equal(t, uint16(tls.VersionTLS13), resp.TLS.Version) + }) +} + +func TestServer_InvalidMinTLSFailsToStart(t *testing.T) { + tests := []string{"1.1", "sslv3"} + + for _, value := range tests { + t.Run(value, func(t *testing.T) { + config := testConfig(t) + config.MinTLS = value + + server := NewServer(config, NewRouter(config.StatePath())) + err := server.Start() + + require.ErrorContains(t, err, "min-tls") + }) + } +} + +// Helpers + +func testMinTLSServer(t *testing.T, minTLS string, clientCA *testCA) *Server { + t.Helper() + + config := testConfig(t) + config.MinTLS = minTLS + server := testServerWithConfig(t, config) + + certPath, keyPath := prepareTestCertificateFiles(t) + serviceOptions := defaultServiceOptions + serviceOptions.Hosts = []string{"localhost"} + serviceOptions.TLSEnabled = true + serviceOptions.TLSCertificatePath = certPath + serviceOptions.TLSPrivateKeyPath = keyPath + if clientCA != nil { + serviceOptions.TLSClientCACertificatePath = clientCA.certPath + } + + target := testTarget(t, func(w http.ResponseWriter, r *http.Request) {}) + testDeployTarget(t, target, server, serviceOptions) + + return server +} + +// testRequestWithTLSVersions drives a request inside an exact TLS version +// window. Both bounds are set on purpose: Go's own client minimum is TLS 1.2, +// so a MaxVersion of 1.1 alone makes the 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. +func testRequestWithTLSVersions(tb testing.TB, server *Server, minVersion, maxVersion uint16, certs ...tls.Certificate) (*http.Response, error) { + tb.Helper() + + transport := &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, + MinVersion: minVersion, + MaxVersion: maxVersion, + Certificates: certs, + }, + ForceAttemptHTTP2: true, + } + tb.Cleanup(transport.CloseIdleConnections) + + return testRequestUsingTransport(server, transport) +}