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,

‎docs/api/schemas.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2256,6 +2256,28 @@ CreateParameterRequest is a structure used to create a new parameter value for a
22562256
"usage": "string",
22572257
"value": "string"
22582258
},
2259+
"strict_transport_security": {
2260+
"default": 0,
2261+
"enterprise": true,
2262+
"flag": "string",
2263+
"hidden": true,
2264+
"name": "string",
2265+
"secret": true,
2266+
"shorthand": "string",
2267+
"usage": "string",
2268+
"value": 0
2269+
},
2270+
"strict_transport_security_options": {
2271+
"default": ["string"],
2272+
"enterprise": true,
2273+
"flag": "string",
2274+
"hidden": true,
2275+
"name": "string",
2276+
"secret": true,
2277+
"shorthand": "string",
2278+
"usage": "string",
2279+
"value": ["string"]
2280+
},
22592281
"swagger": {
22602282
"enable": {
22612283
"default": true,
@@ -2515,6 +2537,8 @@ CreateParameterRequest is a structure used to create a new parameter value for a
25152537
| `scim_api_key` | [codersdk.DeploymentConfigField-string](#codersdkdeploymentconfigfield-string) | false | | |
25162538
| `secure_auth_cookie` | [codersdk.DeploymentConfigField-bool](#codersdkdeploymentconfigfield-bool) | false | | |
25172539
| `ssh_keygen_algorithm` | [codersdk.DeploymentConfigField-string](#codersdkdeploymentconfigfield-string) | false | | |
2540+
| `strict_transport_security` | [codersdk.DeploymentConfigField-int](#codersdkdeploymentconfigfield-int) | false | | |
2541+
| `strict_transport_security_options` | [codersdk.DeploymentConfigField-array_string](#codersdkdeploymentconfigfield-array_string) | false | | |
25182542
| `swagger` | [codersdk.SwaggerConfig](#codersdkswaggerconfig) | false | | |
25192543
| `telemetry` | [codersdk.TelemetryConfig](#codersdktelemetryconfig) | false | | |
25202544
| `tls` | [codersdk.TLSConfig](#codersdktlsconfig) | false | | |

‎docs/cli/coder_server.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,10 @@ coder server [flags]
118118
Consumes $CODER_MAX_SESSION_EXPIRY (default 24h0m0s)
119119
--ssh-keygen-algorithm string The algorithm to use for generating ssh keys. Accepted values are "ed25519", "ecdsa", or "rsa4096".
120120
Consumes $CODER_SSH_KEYGEN_ALGORITHM (default "ed25519")
121+
--strict-transport-security int Controls if the 'Strict-Transport-Security' header is set on all static file responses. This header should only be set if the server is accessed via HTTPS. This value is the MaxAge in seconds of the header.
122+
Consumes $CODER_STRICT_TRANSPORT_SECURITY
123+
--strict-transport-security-options strings Two optional fields can be set in the Strict-Transport-Security header; 'includeSubDomains' and 'preload'. The 'strict-transport-security' flag must be set to a non-zero value for these options to be used.
124+
Consumes $CODER_STRICT_TRANSPORT_SECURITY_OPTIONS
121125
--swagger-enable Expose the swagger endpoint via /swagger.
122126
Consumes $CODER_SWAGGER_ENABLE
123127
--telemetry Whether telemetry is enabled or not. Coder collects anonymized usage data to help improve our product.

‎site/src/api/typesGenerated.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,8 @@ export interface DeploymentConfig {
309309
readonly tls: TLSConfig
310310
readonly trace: TraceConfig
311311
readonly secure_auth_cookie: DeploymentConfigField<boolean>
312+
readonly strict_transport_security: DeploymentConfigField<number>
313+
readonly strict_transport_security_options: DeploymentConfigField<string[]>
312314
readonly ssh_keygen_algorithm: DeploymentConfigField<string>
313315
readonly metrics_cache_refresh_interval: DeploymentConfigField<number>
314316
readonly agent_stat_refresh_interval: DeploymentConfigField<number>

0 commit comments

Comments
 (0)