Skip to content

Commit 6189035

Browse files
EmyrkEmyrk
authored
feat: Add option to enable hsts header (#6147)
* feat: Add option to enable hsts header * Update golden files
1 parent 77afdf7 commit 6189035

File tree

13 files changed

+287
-1
lines changed

13 files changed

+287
-1
lines changed

‎cli/deployment/config.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,20 @@ func newConfig() *codersdk.DeploymentConfig {
374374
Usage: "Controls if the 'Secure' property is set on browser session cookies.",
375375
Flag: "secure-auth-cookie",
376376
},
377+
StrictTransportSecurity: &codersdk.DeploymentConfigField[int]{
378+
Name: "Strict-Transport-Security",
379+
Usage: "Controls if the 'Strict-Transport-Security' header is set on all static file responses. " +
380+
"This header should only be set if the server is accessed via HTTPS. This value is the MaxAge in seconds of " +
381+
"the header.",
382+
Default: 0,
383+
Flag: "strict-transport-security",
384+
},
385+
StrictTransportSecurityOptions: &codersdk.DeploymentConfigField[[]string]{
386+
Name: "Strict-Transport-Security Options",
387+
Usage: "Two optional fields can be set in the Strict-Transport-Security header; 'includeSubDomains' and 'preload'. " +
388+
"The 'strict-transport-security' flag must be set to a non-zero value for these options to be used.",
389+
Flag: "strict-transport-security-options",
390+
},
377391
SSHKeygenAlgorithm: &codersdk.DeploymentConfigField[string]{
378392
Name: "SSH Keygen Algorithm",
379393
Usage: "The algorithm to use for generating ssh keys. Accepted values are \"ed25519\", \"ecdsa\", or \"rsa4096\".",

‎cli/server.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,13 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
485485
options.TLSCertificates = tlsConfig.Certificates
486486
}
487487

488+
if cfg.StrictTransportSecurity.Value > 0 {
489+
options.StrictTransportSecurityCfg, err = httpmw.HSTSConfigOptions(cfg.StrictTransportSecurity.Value, cfg.StrictTransportSecurityOptions.Value)
490+
if err != nil {
491+
return xerrors.Errorf("coderd: setting hsts header failed (options: %v): %w", cfg.StrictTransportSecurityOptions.Value, err)
492+
}
493+
}
494+
488495
if cfg.UpdateCheck.Value {
489496
options.UpdateCheckOptions = &updatecheck.Options{
490497
// Avoid spamming GitHub API checking for updates.

‎cli/testdata/coder_server_--help.golden

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,23 @@ Flags:
280280
"ed25519", "ecdsa", or "rsa4096".
281281
Consumes $CODER_SSH_KEYGEN_ALGORITHM
282282
(default "ed25519")
283+
--strict-transport-security int Controls if the
284+
'Strict-Transport-Security' header
285+
is set on all static file responses.
286+
This header should only be set if
287+
the server is accessed via HTTPS.
288+
This value is the MaxAge in seconds
289+
of the header.
290+
Consumes $CODER_STRICT_TRANSPORT_SECURITY
291+
--strict-transport-security-options strings Two optional fields can be set in
292+
the Strict-Transport-Security
293+
header; 'includeSubDomains' and
294+
'preload'. The
295+
'strict-transport-security' flag
296+
must be set to a non-zero value for
297+
these options to be used.
298+
Consumes
299+
$CODER_STRICT_TRANSPORT_SECURITY_OPTIONS
283300
--swagger-enable Expose the swagger endpoint via
284301
/swagger.
285302
Consumes $CODER_SWAGGER_ENABLE

‎coderd/apidoc/docs.go

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/coderd.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ type Options struct {
103103
OIDCConfig *OIDCConfig
104104
PrometheusRegistry *prometheus.Registry
105105
SecureAuthCookie bool
106+
StrictTransportSecurityCfg httpmw.HSTSConfig
106107
SSHKeygenAlgorithm gitsshkey.Algorithm
107108
Telemetry telemetry.Reporter
108109
TracerProvider trace.TracerProvider
@@ -222,12 +223,18 @@ func New(options *Options) *API {
222223
options.MetricsCacheRefreshInterval,
223224
)
224225

226+
staticHandler := site.Handler(site.FS(), binFS, binHashes)
227+
// Static file handler must be wrapped with HSTS handler if the
228+
// StrictTransportSecurityAge is set. We only need to set this header on
229+
// static files since it only affects browsers.
230+
staticHandler = httpmw.HSTS(staticHandler, options.StrictTransportSecurityCfg)
231+
225232
r := chi.NewRouter()
226233
api := &API{
227234
ID: uuid.New(),
228235
Options: options,
229236
RootHandler: r,
230-
siteHandler: site.Handler(site.FS(), binFS, binHashes),
237+
siteHandler: staticHandler,
231238
HTTPAuth: &HTTPAuthorizer{
232239
Authorizer: options.Authorizer,
233240
Logger: options.Logger,

‎coderd/httpmw/hsts.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package httpmw
2+
3+
import (
4+
"fmt"
5+
"net/http"
6+
"strings"
7+
8+
"golang.org/x/xerrors"
9+
)
10+
11+
const (
12+
hstsHeader = "Strict-Transport-Security"
13+
)
14+
15+
type HSTSConfig struct {
16+
// HeaderValue is an empty string if hsts header is disabled.
17+
HeaderValue string
18+
}
19+
20+
func HSTSConfigOptions(maxAge int, options []string) (HSTSConfig, error) {
21+
if maxAge <= 0 {
22+
// No header, so no need to build the header string.
23+
return HSTSConfig{}, nil
24+
}
25+
26+
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Strict-Transport-Security
27+
var str strings.Builder
28+
_, err := str.WriteString(fmt.Sprintf("max-age=%d", maxAge))
29+
if err != nil {
30+
return HSTSConfig{}, xerrors.Errorf("hsts: write max-age: %w", err)
31+
}
32+
33+
for _, option := range options {
34+
switch {
35+
// Only allow valid options and fix any casing mistakes
36+
case strings.EqualFold(option, "includeSubDomains"):
37+
option = "includeSubDomains"
38+
case strings.EqualFold(option, "preload"):
39+
option = "preload"
40+
default:
41+
return HSTSConfig{}, xerrors.Errorf("hsts: invalid option: %q. Must be 'preload' and/or 'includeSubDomains'", option)
42+
}
43+
_, err = str.WriteString("; " + option)
44+
if err != nil {
45+
return HSTSConfig{}, xerrors.Errorf("hsts: write option: %w", err)
46+
}
47+
}
48+
return HSTSConfig{
49+
HeaderValue: str.String(),
50+
}, nil
51+
}
52+
53+
// HSTS will add the strict-transport-security header if enabled. This header
54+
// forces a browser to always use https for the domain after it loads https once.
55+
// Meaning: On first load of product.coder.com, they are redirected to https. On
56+
// all subsequent loads, the client's local browser forces https. This prevents
57+
// man in the middle.
58+
//
59+
// This header only makes sense if the app is using tls.
60+
//
61+
// Full header example:
62+
// Strict-Transport-Security: max-age=63072000; includeSubDomains; preload
63+
func HSTS(next http.Handler, cfg HSTSConfig) http.Handler {
64+
if cfg.HeaderValue == "" {
65+
// No header, so no need to wrap the handler.
66+
return next
67+
}
68+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
69+
w.Header().Set(hstsHeader, cfg.HeaderValue)
70+
next.ServeHTTP(w, r)
71+
})
72+
}

‎coderd/httpmw/hsts_test.go

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
package httpmw_test
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"testing"
7+
8+
"github.com/stretchr/testify/require"
9+
10+
"github.com/coder/coder/coderd/httpmw"
11+
)
12+
13+
func TestHSTS(t *testing.T) {
14+
t.Parallel()
15+
16+
tests := []struct {
17+
Name string
18+
MaxAge int
19+
Options []string
20+
21+
wantErr bool
22+
expectHeader string
23+
}{
24+
{
25+
Name: "Empty",
26+
MaxAge: 0,
27+
Options: nil,
28+
},
29+
{
30+
Name: "NoAge",
31+
MaxAge: 0,
32+
Options: []string{"includeSubDomains"},
33+
},
34+
{
35+
Name: "NegativeAge",
36+
MaxAge: -100,
37+
Options: []string{"includeSubDomains"},
38+
},
39+
{
40+
Name: "Age",
41+
MaxAge: 1000,
42+
Options: []string{},
43+
expectHeader: "max-age=1000",
44+
},
45+
{
46+
Name: "AgeSubDomains",
47+
MaxAge: 1000,
48+
// Mess with casing
49+
Options: []string{"INCLUDESUBDOMAINS"},
50+
expectHeader: "max-age=1000; includeSubDomains",
51+
},
52+
{
53+
Name: "AgePreload",
54+
MaxAge: 1000,
55+
Options: []string{"Preload"},
56+
expectHeader: "max-age=1000; preload",
57+
},
58+
{
59+
Name: "AllOptions",
60+
MaxAge: 1000,
61+
Options: []string{"preload", "includeSubDomains"},
62+
expectHeader: "max-age=1000; preload; includeSubDomains",
63+
},
64+
65+
// Error values
66+
{
67+
Name: "BadOption",
68+
MaxAge: 100,
69+
Options: []string{"not-valid"},
70+
wantErr: true,
71+
},
72+
{
73+
Name: "BadOptions",
74+
MaxAge: 100,
75+
Options: []string{"includeSubDomains", "not-valid", "still-not-valid"},
76+
wantErr: true,
77+
},
78+
}
79+
for _, tt := range tests {
80+
tt := tt
81+
t.Run(tt.Name, func(t *testing.T) {
82+
t.Parallel()
83+
84+
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
85+
w.WriteHeader(http.StatusOK)
86+
})
87+
88+
cfg, err := httpmw.HSTSConfigOptions(tt.MaxAge, tt.Options)
89+
if tt.wantErr {
90+
require.Error(t, err, "Expect error, HSTS(%v, %v)", tt.MaxAge, tt.Options)
91+
return
92+
}
93+
require.NoError(t, err, "Expect no error, HSTS(%v, %v)", tt.MaxAge, tt.Options)
94+
95+
got := httpmw.HSTS(handler, cfg)
96+
req := httptest.NewRequest("GET", "/", nil)
97+
res := httptest.NewRecorder()
98+
got.ServeHTTP(res, req)
99+
100+
require.Equal(t, tt.expectHeader, res.Header().Get("Strict-Transport-Security"), "expected header value")
101+
})
102+
}
103+
}

‎codersdk/deployment.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,8 @@ type DeploymentConfig struct {
126126
TLS *TLSConfig `json:"tls" typescript:",notnull"`
127127
Trace *TraceConfig `json:"trace" typescript:",notnull"`
128128
SecureAuthCookie *DeploymentConfigField[bool] `json:"secure_auth_cookie" typescript:",notnull"`
129+
StrictTransportSecurity *DeploymentConfigField[int] `json:"strict_transport_security" typescript:",notnull"`
130+
StrictTransportSecurityOptions *DeploymentConfigField[[]string] `json:"strict_transport_security_options" typescript:",notnull"`
129131
SSHKeygenAlgorithm *DeploymentConfigField[string] `json:"ssh_keygen_algorithm" typescript:",notnull"`
130132
MetricsCacheRefreshInterval *DeploymentConfigField[time.Duration] `json:"metrics_cache_refresh_interval" typescript:",notnull"`
131133
AgentStatRefreshInterval *DeploymentConfigField[time.Duration] `json:"agent_stat_refresh_interval" typescript:",notnull"`

‎docs/api/general.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -857,6 +857,28 @@ curl -X GET http://coder-server:8080/api/v2/config/deployment \
857857
"usage": "string",
858858
"value": "string"
859859
},
860+
"strict_transport_security": {
861+
"default": 0,
862+
"enterprise": true,
863+
"flag": "string",
864+
"hidden": true,
865+
"name": "string",
866+
"secret": true,
867+
"shorthand": "string",
868+
"usage": "string",
869+
"value": 0
870+
},
871+
"strict_transport_security_options": {
872+
"default": ["string"],
873+
"enterprise": true,
874+
"flag": "string",
875+
"hidden": true,
876+
"name": "string",
877+
"secret": true,
878+
"shorthand": "string",
879+
"usage": "string",
880+
"value": ["string"]
881+
},
860882
"swagger": {
861883
"enable": {
862884
"default": true,

0 commit comments

Comments
 (0)