Repository: google/go-safeweb
Pull request #36 (2020-07-31 13:21:00)

HSTS plugin skeleton
A plugin which automatically applies HSTS to incoming requests. The `Before` function must be run on incoming requests before they are passed to a handler.

mattiasgrenfeldt: Tests are here
mattiasgrenfeldt: Rebased on top of master, ready to merge

Patch:
diff --git a/safehttp/incoming_request.go b/safehttp/incoming_request.go
index 5d4dd6a..41ffba3 100644
--- a/safehttp/incoming_request.go
+++ b/safehttp/incoming_request.go
@@ -15,8 +15,10 @@
 package safehttp
 
 import (
+	"crypto/tls"
 	"fmt"
 	"net/http"
+	"net/url"
 	"strings"
 	"sync"
 )
@@ -26,10 +28,17 @@ type IncomingRequest struct {
 	req       *http.Request
 	Header    Header
 	parseOnce sync.Once
+	TLS       *tls.ConnectionState
+	URL       *url.URL
 }
 
 func newIncomingRequest(req *http.Request) *IncomingRequest {
-	return &IncomingRequest{req: req, Header: newHeader(req.Header)}
+	return &IncomingRequest{
+		req:    req,
+		Header: newHeader(req.Header),
+		TLS:    req.TLS,
+		URL:    req.URL,
+	}
 }
 
 // PostForm parses the form parameters provided in the body of a POST, PATCH or
diff --git a/safehttp/plugins/hsts/hsts.go b/safehttp/plugins/hsts/hsts.go
new file mode 100644
index 0000000..0382c52
--- /dev/null
+++ b/safehttp/plugins/hsts/hsts.go
@@ -0,0 +1,99 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package hsts
+
+import (
+	"net/url"
+	"strconv"
+	"strings"
+	"time"
+
+	"github.com/google/go-safeweb/safehttp"
+)
+
+// Plugin implements automatic HSTS functionality.
+// See https://tools.ietf.org/html/rfc6797 for more info.
+type Plugin struct {
+	// MaxAge is the duration that the browser should remember
+	// that a site is only to be accessed using HTTPS. MaxAge
+	// must be positive.
+	MaxAge time.Duration
+
+	// DisableIncludeSubDomains disables the includeSubDomains directive.
+	// When DisableIncludeSubDomains is false, all subdomains
+	// of the domain where this service is hosted will also be added
+	// to the browsers HSTS list.
+	DisableIncludeSubDomains bool
+
+	// Preload enables the preload directive.
+	// This should only be enabled if this site should be
+	// added to the browser HSTS preload list, which is supported
+	// by all major browsers. See https://hstspreload.org/ for
+	// more info.
+	Preload bool
+
+	// BehindProxy controls how the plugin should behave with regards
+	// to HTTPS. If this server is behind a proxy that terminates HTTPS
+	// traffic then this should be enabled. If this is enabled
+	// then the plugin will always send the Strict-Transport-Security
+	// header and will not redirect HTTP traffic to HTTPS traffic.
+	BehindProxy bool
+}
+
+// NewPlugin creates a new HSTS plugin with safe defaults.
+// These safe defaults are:
+//  - max-age set to two years.
+//  - includeSubDomains is enabled.
+//  - preload is disabled.
+func NewPlugin() Plugin {
+	return Plugin{MaxAge: 63072000 * time.Second} // two years in seconds
+}
+
+// Before should be executed before the request is sent to the handler.
+// The function redirects HTTP requests to HTTPS. When HTTPS traffic
+// is received the Strict-Transport-Security header is applied to the
+// response.
+func (p *Plugin) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
+	if p.MaxAge < 0 {
+		return w.ServerError(safehttp.StatusInternalServerError, "Internal Server Error")
+	}
+
+	if !p.BehindProxy && r.TLS == nil {
+		u, err := url.Parse(r.URL.String())
+		if err != nil {
+			return w.ServerError(safehttp.StatusInternalServerError, "Internal Server Error")
+		}
+		u.Scheme = "https"
+		return w.Redirect(r, u.String(), safehttp.StatusMovedPermanently)
+	}
+
+	var value strings.Builder
+	value.WriteString("max-age=")
+	value.WriteString(strconv.FormatInt(int64(p.MaxAge.Seconds()), 10))
+	if !p.DisableIncludeSubDomains {
+		value.WriteString("; includeSubDomains")
+	}
+	if p.Preload {
+		value.WriteString("; preload")
+	}
+	h := w.Header()
+	if err := h.Set("Strict-Transport-Security", value.String()); err != nil {
+		// TODO(@mattiasgrenfeldt): Replace the response with an actual saferesponse somehow.
+		return w.ServerError(safehttp.StatusInternalServerError, "Internal Server Error")
+	}
+	// TODO: Implement header claiming.
+	h.MarkImmutable("Strict-Transport-Security")
+	return safehttp.Result{}
+}
diff --git a/safehttp/response_writer.go b/safehttp/response_writer.go
index 92b2acf..57857f8 100644
--- a/safehttp/response_writer.go
+++ b/safehttp/response_writer.go
@@ -58,6 +58,12 @@ func (w *ResponseWriter) ServerError(code StatusCode, resp Response) Result {
 	return Result{}
 }
 
+// Redirect responds with a redirect to a given url, using code as the status code.
+func (w *ResponseWriter) Redirect(r *IncomingRequest, url string, code StatusCode) Result {
+	http.Redirect(w.rw, r.req, url, int(code))
+	return Result{}
+}
+
 // Header returns the collection of headers that will be set
 // on the response. Headers must be set before writing a
 // response (e.g. Write, WriteTemplate).
diff --git a/safehttp/status.go b/safehttp/status.go
index 59c8232..b65ac7f 100644
--- a/safehttp/status.go
+++ b/safehttp/status.go
@@ -14,12 +14,12 @@
 
 package safehttp
 
-// StatusCode TODO
+// StatusCode contains HTTP status codes as registered with IANA.
+// See: https://www.iana.org/assignments/http-status-codes/http-status-codes.xhtml
 type StatusCode int
 
 const (
-	// Status200OK TODO
-	Status200OK StatusCode = 200
-	// Status500InternalServerError TODO
-	Status500InternalServerError = 500
+	StatusOK                  StatusCode = 200 // RFC 7231, 6.3.1
+	StatusMovedPermanently    StatusCode = 301 // RFC 7231, 6.4.2
+	StatusInternalServerError StatusCode = 500 // RFC 7231, 6.6.1
 )


Test patch:
diff --git a/safehttp/plugins/hsts/hsts_test.go b/safehttp/plugins/hsts/hsts_test.go
new file mode 100644
index 0000000..dbc63b8
--- /dev/null
+++ b/safehttp/plugins/hsts/hsts_test.go
@@ -0,0 +1,291 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package hsts_test
+
+import (
+	"html/template"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-safeweb/safehttp"
+	"github.com/google/go-safeweb/safehttp/plugins/hsts"
+	"github.com/google/safehtml"
+)
+
+type testDispatcher struct{}
+
+func (testDispatcher) Write(rw http.ResponseWriter, resp safehttp.Response) error {
+	switch x := resp.(type) {
+	case safehtml.HTML:
+		_, err := rw.Write([]byte(x.String()))
+		return err
+	default:
+		panic("not a safe response type")
+	}
+}
+
+func (testDispatcher) ExecuteTemplate(rw http.ResponseWriter, t safehttp.Template, data interface{}) error {
+	switch x := t.(type) {
+	case *template.Template:
+		return x.Execute(rw, data)
+	default:
+		panic("not a safe response type")
+	}
+}
+
+type responseRecorder struct {
+	header http.Header
+	writer io.Writer
+	status int
+}
+
+func newResponseRecorder(w io.Writer) *responseRecorder {
+	return &responseRecorder{header: http.Header{}, writer: w, status: http.StatusOK}
+}
+
+func (r *responseRecorder) Header() http.Header {
+	return r.header
+}
+
+func (r *responseRecorder) WriteHeader(statusCode int) {
+	r.status = statusCode
+}
+
+func (r *responseRecorder) Write(data []byte) (int, error) {
+	return r.writer.Write(data)
+}
+
+func TestHSTS(t *testing.T) {
+	var test = []struct {
+		name        string
+		plugin      hsts.Plugin
+		req         *http.Request
+		wantStatus  int
+		wantHeaders map[string][]string
+		wantBody    string
+	}{
+		{
+			name:       "HTTPS",
+			plugin:     hsts.NewPlugin(),
+			req:        httptest.NewRequest("GET", "https://localhost/", nil),
+			wantStatus: 200,
+			wantHeaders: map[string][]string{
+				"Strict-Transport-Security": {"max-age=63072000; includeSubDomains"}, // 63072000 seconds is two years
+			},
+			wantBody: "",
+		},
+		{
+			name:       "HTTP",
+			plugin:     hsts.NewPlugin(),
+			req:        httptest.NewRequest("GET", "http://localhost/", nil),
+			wantStatus: 301,
+			wantHeaders: map[string][]string{
+				"Content-Type": {"text/html; charset=utf-8"},
+				"Location":     {"https://localhost/"},
+			},
+			wantBody: "<a href=\"https://localhost/\">Moved Permanently</a>.\n\n",
+		},
+		{
+			name:       "HTTP behind proxy",
+			plugin:     hsts.Plugin{BehindProxy: true},
+			req:        httptest.NewRequest("GET", "http://localhost/", nil),
+			wantStatus: 200,
+			wantHeaders: map[string][]string{
+				// max-age=0 tells the browser to expire the HSTS protection.
+				"Strict-Transport-Security": {"max-age=0; includeSubDomains"},
+			},
+			wantBody: "",
+		},
+		{
+			name:       "Preload",
+			plugin:     hsts.Plugin{Preload: true, DisableIncludeSubDomains: true},
+			req:        httptest.NewRequest("GET", "https://localhost/", nil),
+			wantStatus: 200,
+			wantHeaders: map[string][]string{
+				// max-age=0 tells the browser to expire the HSTS protection.
+				"Strict-Transport-Security": {"max-age=0; preload"},
+			},
+			wantBody: "",
+		},
+		{
+			name:       "Preload and IncludeSubDomains",
+			plugin:     hsts.Plugin{Preload: true},
+			req:        httptest.NewRequest("GET", "https://localhost/", nil),
+			wantStatus: 200,
+			wantHeaders: map[string][]string{
+				// max-age=0 tells the browser to expire the HSTS protection.
+				"Strict-Transport-Security": {"max-age=0; includeSubDomains; preload"},
+			},
+			wantBody: "",
+		},
+		{
+			name:       "No preload and no includeSubDomains",
+			plugin:     hsts.Plugin{DisableIncludeSubDomains: true},
+			req:        httptest.NewRequest("GET", "https://localhost/", nil),
+			wantStatus: 200,
+			wantHeaders: map[string][]string{
+				// max-age=0 tells the browser to expire the HSTS protection.
+				"Strict-Transport-Security": {"max-age=0"},
+			},
+			wantBody: "",
+		},
+		{
+			name:       "Custom maxage",
+			plugin:     hsts.Plugin{MaxAge: 3600 * time.Second},
+			req:        httptest.NewRequest("GET", "https://localhost/", nil),
+			wantStatus: 200,
+			wantHeaders: map[string][]string{
+				"Strict-Transport-Security": {"max-age=3600; includeSubDomains"}, // 3600 seconds is 1 hour
+			},
+			wantBody: "",
+		},
+	}
+
+	for _, tt := range test {
+		t.Run(tt.name, func(t *testing.T) {
+			m := safehttp.NewMachinery(tt.plugin.Before, &testDispatcher{})
+
+			b := &strings.Builder{}
+			rr := newResponseRecorder(b)
+
+			m.HandleRequest(rr, tt.req)
+
+			if rr.status != tt.wantStatus {
+				t.Errorf("status code got: %v want: %v", rr.status, tt.wantStatus)
+			}
+
+			if diff := cmp.Diff(tt.wantHeaders, map[string][]string(rr.Header())); diff != "" {
+				t.Errorf("rr.Header() mismatch (-want +got):\n%s", diff)
+			}
+
+			if got := b.String(); got != tt.wantBody {
+				t.Errorf("response body got: %q want: %q", got, tt.wantBody)
+			}
+		})
+	}
+}
+
+func TestStrictTransportSecurityAlreadyImmutable(t *testing.T) {
+	p := hsts.NewPlugin()
+	m := safehttp.NewMachinery(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
+		w.Header().MarkImmutable("Strict-Transport-Security")
+		return p.Before(w, r)
+	}, &testDispatcher{})
+
+	req := httptest.NewRequest("GET", "https://localhost/", nil)
+
+	// TODO(@mattiasgrenfeldt): Change from current to desired behavior once
+	// safehttp.ResponseWriter.ServerError has changed signature and been
+	// implemented.
+	t.Run("Current Behavior", func(t *testing.T) {
+		b := &strings.Builder{}
+		rr := newResponseRecorder(b)
+
+		m.HandleRequest(rr, req)
+
+		if want := 200; rr.status != want {
+			t.Errorf("status code got: %v want: %v", rr.status, want)
+		}
+
+		wantHeaders := map[string][]string{}
+		if diff := cmp.Diff(wantHeaders, map[string][]string(rr.Header())); diff != "" {
+			t.Errorf("rr.Header() mismatch (-want +got):\n%s", diff)
+		}
+
+		if got, want := b.String(), ""; got != want {
+			t.Errorf("response body got: %q want: %q", got, want)
+		}
+	})
+
+	t.Run("Desired Behavior", func(t *testing.T) {
+		t.Skip()
+		b := &strings.Builder{}
+		rr := newResponseRecorder(b)
+
+		m.HandleRequest(rr, req)
+
+		if want := 500; rr.status != want {
+			t.Errorf("status code got: %v want: %v", rr.status, want)
+		}
+
+		wantHeaders := map[string][]string{}
+		if diff := cmp.Diff(wantHeaders, map[string][]string(rr.Header())); diff != "" {
+			t.Errorf("rr.Header() mismatch (-want +got):\n%s", diff)
+		}
+
+		if got, want := b.String(), "Internal Server Error"; got != want {
+			t.Errorf("response body got: %q want: %q", got, want)
+		}
+	})
+}
+
+func TestNegativeMaxAge(t *testing.T) {
+	p := hsts.Plugin{MaxAge: -1 * time.Second}
+	m := safehttp.NewMachinery(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
+		w.Header().MarkImmutable("Strict-Transport-Security")
+		return p.Before(w, r)
+	}, &testDispatcher{})
+
+	req := httptest.NewRequest("GET", "https://localhost/", nil)
+
+	// TODO(@mattiasgrenfeldt): Change from current to desired behavior once
+	// safehttp.ResponseWriter.ServerError has changed signature and been
+	// implemented.
+	t.Run("Current Behavior", func(t *testing.T) {
+		b := &strings.Builder{}
+		rr := newResponseRecorder(b)
+
+		m.HandleRequest(rr, req)
+
+		if want := 200; rr.status != want {
+			t.Errorf("status code got: %v want: %v", rr.status, want)
+		}
+
+		wantHeaders := map[string][]string{}
+		if diff := cmp.Diff(wantHeaders, map[string][]string(rr.Header())); diff != "" {
+			t.Errorf("rr.Header() mismatch (-want +got):\n%s", diff)
+		}
+
+		if got, want := b.String(), ""; got != want {
+			t.Errorf("response body got: %q want: %q", got, want)
+		}
+	})
+
+	t.Run("Desired Behavior", func(t *testing.T) {
+		t.Skip()
+		b := &strings.Builder{}
+		rr := newResponseRecorder(b)
+
+		m.HandleRequest(rr, req)
+
+		if want := 500; rr.status != want {
+			t.Errorf("status code got: %v want: %v", rr.status, want)
+		}
+
+		wantHeaders := map[string][]string{}
+		if diff := cmp.Diff(wantHeaders, map[string][]string(rr.Header())); diff != "" {
+			t.Errorf("rr.Header() mismatch (-want +got):\n%s", diff)
+		}
+
+		if got, want := b.String(), "Internal Server Error"; got != want {
+			t.Errorf("response body got: %q want: %q", got, want)
+		}
+	})
+}