Skip to content

Commit

Permalink
move common headers to tlsMux
Browse files Browse the repository at this point in the history
This ensures we add the Strict-Transport-Security header and
`Connection: close` to all HTTPS requests

Previously, these weren't set on the static file HTTP responses but both
are needed.

The `Connection: close` is needed to prevent clients from reusing TLS
connections, requiring our server to keep the memory-heavy version of
`tls.Conn` we've forked. Connection reuse also increases the chance a
client tries to perform TLS renegotiation or resumption and break our
vuln detection. I suspect this latter reason isn't a genuine problem (or
is and this only makes it less likely), but it's been a while since I
thought hard about the patch I made to `crypto/tls`.

Strict-Transport-Security should just always be set on HTTPS requests.

Fixes #525
Fixes #526
  • Loading branch information
jmhodges committed Jul 9, 2023
1 parent 8253806 commit d32b6b9
Showing 1 changed file with 15 additions and 11 deletions.
26 changes: 15 additions & 11 deletions howsmyssl.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ func main() {

func configureHTTPSServer(srv *http.Server) {
// If you add HTTP/2 or HTTP/3 support here, be sure that the Connection:
// close header is being set properly
// close header is being set properly elsewhere
srv.ReadTimeout = 10 * time.Second
srv.WriteTimeout = 15 * time.Second
srv.ConnContext = func(ctx context.Context, c net.Conn) context.Context {
Expand Down Expand Up @@ -311,7 +311,20 @@ func tlsMux(routeHost, redirectHost, acmeRedirectURL string, staticHandler http.
if routeHost != "" {
m.Handle("/", commonRedirect(redirectHost))
}
return protoHandler{logHandler{m}, "https"}
wrapper := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Strict-Transport-Security", hstsHeaderValue)
if r.ProtoMajor == 1 && r.ProtoMinor == 1 {
// We always disconnect folks after their request is done to ensure
// we don't keep our tls.Conn fork with it's extra large memory
// needs (including the `clientHelloMsg`) around for too long. This
// also helps prevent any TLS resumptions from happening and
// breaking our vuln detection. We do this on all requests to avoid
// races.
w.Header().Set("Connection", "close")
}
m.ServeHTTP(w, r)
})
return protoHandler{logHandler{wrapper}, "https"}
}

func plaintextMux(redirectHost string) http.Handler {
Expand Down Expand Up @@ -435,11 +448,6 @@ func hijackHandle(w http.ResponseWriter, r *http.Request, statuses *statusStats,

func defaultResponseHeaders(h http.Header, r *http.Request, contentType string) {
h.Set("Content-Type", contentType)
if r.ProtoMajor == 1 && r.ProtoMinor == 1 {
h.Set("Connection", "close")
}
// TODO(#525): replace with STS applied to all handlers in tlsMux
h.Set("Strict-Transport-Security", hstsHeaderValue)
// Allow CORS requests from any domain, for easy API access
h.Set("Access-Control-Allow-Origin", "*")
}
Expand All @@ -458,10 +466,6 @@ func healthcheck(w http.ResponseWriter, r *http.Request) {
func commonRedirect(redirectHost string) http.Handler {
hf := func(w http.ResponseWriter, r *http.Request) {
commonRedirects.Add(1)
if r.Header.Get(xForwardedProto) == "https" {
// TODO(#525): replace with STS applied to all handlers in tlsMux
w.Header().Set("Strict-Transport-Security", hstsHeaderValue)
}
u := r.URL
// Never set by the Go HTTP library.
u.Scheme = "https"
Expand Down

0 comments on commit d32b6b9

Please sign in to comment.