feat: Add option to enable hsts header #6147
Conversation
cli/deployment/config.go
Outdated
| StrictTransportSecurity: &codersdk.DeploymentConfigField[int]{ | ||
| Name: "Strict-Transport-Security", | ||
| Usage: "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. The value should be a whole number in seconds.", |
There was a problem hiding this comment.
I think it would be good to explain that the number sets the maxAge here.
The time, in seconds, that the browser should remember that a site is only to be accessed using HTTPS.
We could also consider splitting this up into an enable bool and explicit maxage option that's required when enabled.
There was a problem hiding this comment.
I added that in the message. If I add another flag, then it's 3 flags to turn this thing fully on :(. I figure anything >0 make sense to enable it.
coderd/coderd.go
Outdated
| // static files since it only affects browsers. | ||
| staticHandler, err = httpmw.HSTS(staticHandler, options.StrictTransportSecurityAge, options.StrictTransportSecurityOptions) | ||
| if err != nil { | ||
| panic(xerrors.Errorf("coderd: setting hsts header failed (options: %v): %w", options.StrictTransportSecurityOptions, err)) |
There was a problem hiding this comment.
The user experience will be poor here if they enter a wrong value into options.
Ideally we should verify this input before we start creating connections/starting the database in cli/server.go so the user gets immediate feedback. But even changing this New function to return an error would be better than panic'ing here IMO (and allow for a clean shutdown).
...
2023-02-10 08:34:44.658 [DEBUG] (devtunnel.stdlib) <./../../../golang.zx2c4.com/wireguard/device/send.go:401> (*Device).RoutineEncryption Routine: encryption worker 8 - stopped
2023-02-10 08:34:44.760 [DEBUG] (postgres.stdlib) <./../../fergusstrange/embedded-postgres/logging.go:47> (*syncedLogger).flush ...
"msg": waiting for server to shut down...2023-02-10 08:34:44.659 UTC [679345] LOG: received fast shutdown request
.2023-02-10 08:34:44.659 UTC [679345] LOG: aborting any active transactions
2023-02-10 08:34:44.661 UTC [679345] LOG: background worker "logical replication launcher" (PID 679352) exited with exit code 1
2023-02-10 08:34:44.661 UTC [679347] LOG: shutting down
2023-02-10 08:34:44.670 UTC [679345] LOG: database system is shut down
done
server stopped
Stopped built-in PostgreSQL
panic: coderd: setting hsts header failed (options: [derp]): hsts: invalid option: "derp". Must be 'preload' and/or 'includeSubDomains'
goroutine 1 [running]:
github.com/coder/coder/coderd.New(0x2915170?)
/home/mafredri/src/coder/coder/coderd/coderd.go:233 +0x1c93
github.com/coder/coder/enterprise/coderd.New({0x2915170, 0xc0006f12c0}, 0xc0004ab5e0)
/home/mafredri/src/coder/coder/enterprise/coderd/coderd.go:54 +0x271
github.com/coder/coder/enterprise/cli.server.func1({0x2915170, 0xc0006f12c0}, 0xc0016dac80)
/home/mafredri/src/coder/coder/enterprise/cli/server.go:76 +0x8d5
github.com/coder/coder/cli.Server.func1(0xc00127ef00, {0x234b98a?, 0x2?, 0x2?})
/home/mafredri/src/coder/coder/cli/server.go:674 +0x58c8
github.com/spf13/cobra.(*Command).execute(0xc00127ef00, {0xc0004d3e20, 0x2, 0x2})
/home/mafredri/.local/go/pkg/mod/github.com/spf13/cobra@v1.6.1/command.go:916 +0x862
github.com/spf13/cobra.(*Command).ExecuteC(0xc00132f800)
/home/mafredri/.local/go/pkg/mod/github.com/spf13/cobra@v1.6.1/command.go:1044 +0x3bd
main.main()
/home/mafredri/src/coder/coder/enterprise/cmd/coder/main.go:19 +0xaf
There was a problem hiding this comment.
I can validate in cli/server.go. It is annoying the New doesn't return an error, so the panic method is how it works :/.
I'll just duplicate the valid check.
There was a problem hiding this comment.
👍. I don’t think we use New in too many places, should be relatively easy to make it return an error?
There was a problem hiding this comment.
Ok I changed it so I validate in server.go.
Coder v0.0.0-devel+8221ab0 - Software development on your infrastucture
Using built-in PostgreSQL (/home/steven/.config/coderv2/postgres)
Started HTTP listener at http://127.0.0.1:3000
Opening tunnel so workspaces can connect to your deployment. For production scenarios, specify an external access URL
View the Web UI: https://fcca5eb9087a599c0cb643b264752ea5.pit-1.try.coder.app
Stopping built-in PostgreSQL...
Stopped built-in PostgreSQL
coderd: setting hsts header failed (options: [asd]): hsts: invalid option: "asd". Must be 'preload' and/or 'includeSubDomains'
Run 'coder server --help' for usage.
| @@ -485,6 +485,13 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co | |||
| options.TLSCertificates = tlsConfig.Certificates | |||
| } | |||
|
|
|||
| if cfg.StrictTransportSecurity.Value > 0 { | |||
| options.StrictTransportSecurityCfg, err = httpmw.HSTSConfigOptions(cfg.StrictTransportSecurity.Value, cfg.StrictTransportSecurityOptions.Value) | |||
Only affects static file serving.
See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Strict-Transport-Security