-
Notifications
You must be signed in to change notification settings - Fork 82
Introducing configurable setting for min tls version #199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -150,6 +150,25 @@ func (s *Server) startHTTPServers() error { | |
| return err | ||
| } | ||
| s.httpsListener = httpsListener | ||
| tlsConfig := &tls.Config{ | ||
| NextProtos: []string{"h2", "http/1.1", acme.ALPNProto}, | ||
| GetCertificate: s.router.GetCertificate, | ||
| } | ||
| switch s.config.MinTLS { | ||
| case "tls1_0": | ||
| tlsConfig.MinVersion = tls.VersionTLS10 | ||
| case "tls1_1": | ||
| tlsConfig.MinVersion = tls.VersionTLS11 | ||
| case "tls1_2": | ||
| tlsConfig.MinVersion = tls.VersionTLS12 | ||
| case "tls1_3": | ||
| tlsConfig.MinVersion = tls.VersionTLS13 | ||
| case "": | ||
| // No minimum TLS version set | ||
| default: | ||
| return fmt.Errorf("unsupported minimum TLS version: %q (use tls1_0, tls1_1, tls1_2, or tls1_3)", s.config.MinTLS) | ||
| } | ||
|
pebanb marked this conversation as resolved.
Comment on lines
+168
to
+170
|
||
|
|
||
| s.httpsServer = &http.Server{ | ||
| Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| if s.config.HTTP3Enabled { | ||
|
|
@@ -158,10 +177,7 @@ func (s *Server) startHTTPServers() error { | |
|
|
||
| handler.ServeHTTP(w, r) | ||
| }), | ||
| TLSConfig: &tls.Config{ | ||
| NextProtos: []string{"h2", "http/1.1", acme.ALPNProto}, | ||
| GetCertificate: s.router.GetCertificate, | ||
| }, | ||
| TLSConfig: tlsConfig, | ||
| } | ||
|
|
||
| go s.httpServer.Serve(s.httpListener) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,7 +14,7 @@ import ( | |
|
|
||
| func TestServer_Deploying(t *testing.T) { | ||
| target := testTarget(t, func(w http.ResponseWriter, r *http.Request) {}) | ||
| server := testServer(t, true) | ||
| server := testServer(t, false, "") | ||
|
|
||
| testDeployTarget(t, target, server, defaultServiceOptions) | ||
|
|
||
|
|
@@ -24,9 +24,9 @@ func TestServer_Deploying(t *testing.T) { | |
| } | ||
|
|
||
| func TestServer_DeployingHTTPS(t *testing.T) { | ||
| startDeployment := func(http3Enabled bool) *Server { | ||
| startDeployment := func(http3Enabled bool, minTLS string) *Server { | ||
| target := testTarget(t, func(w http.ResponseWriter, r *http.Request) {}) | ||
| server := testServer(t, http3Enabled) | ||
| server := testServer(t, http3Enabled, minTLS) | ||
|
|
||
| certPath, keyPath := prepareTestCertificateFiles(t) | ||
| serviceOptions := defaultServiceOptions | ||
|
|
@@ -40,7 +40,7 @@ func TestServer_DeployingHTTPS(t *testing.T) { | |
| } | ||
|
|
||
| t.Run("with HTTP/3 enabled", func(t *testing.T) { | ||
| server := startDeployment(true) | ||
| server := startDeployment(true, "") | ||
|
|
||
| t.Run("http/1.1", func(t *testing.T) { | ||
| resp, err := testRequestUsingHTTP11(t, server) | ||
|
|
@@ -72,8 +72,56 @@ func TestServer_DeployingHTTPS(t *testing.T) { | |
| }) | ||
| }) | ||
|
|
||
| t.Run("with min TLS 1.0", func(t *testing.T) { | ||
| server := startDeployment(false, "tls1_0") | ||
|
|
||
| t.Run("http/1.1", func(t *testing.T) { | ||
| resp, err := testRequestUsingHTTP11(t, server) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, http.StatusOK, resp.StatusCode) | ||
| assert.Equal(t, "HTTP/1.1", resp.Proto) | ||
| assert.GreaterOrEqual(t, resp.TLS.Version, uint16(tls.VersionTLS10)) | ||
| }) | ||
|
Comment on lines
+80
to
+84
|
||
| }) | ||
|
|
||
| t.Run("with min TLS 1.1", func(t *testing.T) { | ||
| server := startDeployment(false, "tls1_1") | ||
|
|
||
| t.Run("http/1.1", func(t *testing.T) { | ||
| resp, err := testRequestUsingHTTP11(t, server) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, http.StatusOK, resp.StatusCode) | ||
| assert.Equal(t, "HTTP/1.1", resp.Proto) | ||
| assert.GreaterOrEqual(t, resp.TLS.Version, uint16(tls.VersionTLS11)) | ||
| }) | ||
| }) | ||
|
|
||
| t.Run("with min TLS 1.2", func(t *testing.T) { | ||
| server := startDeployment(false, "tls1_2") | ||
|
|
||
| t.Run("http/1.1", func(t *testing.T) { | ||
| resp, err := testRequestUsingHTTP11(t, server) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, http.StatusOK, resp.StatusCode) | ||
| assert.Equal(t, "HTTP/1.1", resp.Proto) | ||
| assert.GreaterOrEqual(t, resp.TLS.Version, uint16(tls.VersionTLS12)) | ||
| }) | ||
| }) | ||
|
|
||
| t.Run("with min TLS 1.3", func(t *testing.T) { | ||
| server := startDeployment(false, "tls1_3") | ||
|
|
||
| t.Run("http/1.1", func(t *testing.T) { | ||
| resp, err := testRequestUsingHTTP11(t, server) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, http.StatusOK, resp.StatusCode) | ||
| assert.Equal(t, "HTTP/1.1", resp.Proto) | ||
| assert.Equal(t, uint16(tls.VersionTLS13), resp.TLS.Version) | ||
| }) | ||
| }) | ||
|
|
||
| t.Run("with HTTP/3 disabled", func(t *testing.T) { | ||
| server := startDeployment(false) | ||
| server := startDeployment(false, "") | ||
|
|
||
| t.Run("http/1.1", func(t *testing.T) { | ||
| resp, err := testRequestUsingHTTP11(t, server) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When MinTLS is empty, the HTTPS server TLS config leaves MinVersion unset. That means the effective minimum is Go's runtime default, which can be changed by environment/runtime settings and may not meet the PR's stated goal of hardening the server. If the intent is to be secure by default, consider setting an explicit default minimum (e.g., TLS 1.2) when MinTLS is not provided, and reserve an explicit opt-out value if you need to preserve legacy behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default and empty case should use tls 1.2 because Go does it implicitly at the moment.