From d32b6b985835e28335c4fdc82278cf105ebd1e3e Mon Sep 17 00:00:00 2001 From: Jeff Hodges Date: Sat, 8 Jul 2023 23:13:56 -0700 Subject: [PATCH] move common headers to tlsMux MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- howsmyssl.go | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/howsmyssl.go b/howsmyssl.go index 9f527974..c307f90a 100644 --- a/howsmyssl.go +++ b/howsmyssl.go @@ -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 { @@ -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 { @@ -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", "*") } @@ -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"