[gddo] FIX: Missing HSTS-header
Gerrit Bot (Gerrit)
Gerrit Bot has uploaded this change for review.
FIX: Missing HSTS-header
ADD HSTS header to godoc.org
Please review my changes
# Related
https://github.com/golang/go/issues/26162
Change-Id: I0680e5be76450c5b3e047b7a22169d0a09935e48
GitHub-Last-Rev: d00fa25781702672c65d1c50860cb325b490c6e3
GitHub-Pull-Request: golang/gddo#562
---
A gddo-server/debug.test
M gddo-server/main.go
M gddo-server/main_test.go
3 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/gddo-server/debug.test b/gddo-server/debug.test
new file mode 100755
index 0000000..333bcc3
--- /dev/null
+++ b/gddo-server/debug.test
Binary files differ
diff --git a/gddo-server/main.go b/gddo-server/main.go
index 2137cb9..55fa664 100644
--- a/gddo-server/main.go
+++ b/gddo-server/main.go
@@ -989,6 +989,18 @@
s.root.ServeHTTP(w, r)
}
+const HSTSHeaderKey = "Strict-Transport-Security"
+
+func middlewareHSTS(next http.Handler) http.Handler {
+ return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ // This enforces the use of HTTPS for 1 year, including present and future subdomains.
+ // Chrome and Mozilla Firefox maintain an HSTS preload list
+ // that automatically informs the browser that the website can only be accessed through HTTPS.
+ w.Header().Set(HSTSHeaderKey, "max-age=31536000; includeSubDomains; preload")
+ next.ServeHTTP(w, r)
+ })
+}
+
func main() {
ctx := context.Background()
v, err := loadConfig(ctx, os.Args)
@@ -1016,8 +1028,9 @@
}
}
}()
- http.Handle("/", s)
- log.Fatal(http.ListenAndServe(s.v.GetString(ConfigBindAddress), s))
+ ss := middlewareHSTS(s)
+ http.Handle("/", ss)
+ log.Fatal(http.ListenAndServe(s.v.GetString(ConfigBindAddress), ss))
}
// removeInternal removes the internal packages from the given package
diff --git a/gddo-server/main_test.go b/gddo-server/main_test.go
index 6d42906..4d2232c 100644
--- a/gddo-server/main_test.go
+++ b/gddo-server/main_test.go
@@ -7,11 +7,15 @@
package main
import (
+ "io"
+ "net/http"
+ "net/http/httptest"
"reflect"
"testing"
"github.com/golang/gddo/database"
"github.com/golang/gddo/doc"
+ "github.com/stretchr/testify/require"
)
var robotTests = []string{
@@ -111,3 +115,15 @@
})
}
}
+
+func TestMiddlewareHSTS(t *testing.T) {
+ require := require.New(t)
+ req := httptest.NewRequest(http.MethodGet, "/", nil)
+ respRecorder := httptest.NewRecorder()
+ handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ io.WriteString(w, "")
+ })
+ handlerWithMiddlewareHSTS := middlewareHSTS(handler)
+ handlerWithMiddlewareHSTS.ServeHTTP(respRecorder, req)
+ require.Equal("max-age=31536000; includeSubDomains; preload", respRecorder.Header().Get(HSTSHeaderKey))
+}
To view, visit change 122195. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
Congratulations on opening your first change. Thank you for your contribution!
Next steps:
Within the next week or so, a maintainer will review your change and provide
feedback. See https://golang.org/doc/contribute.html#review for more info and
tips to get your patch through code review.
Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.
During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11, it means that this CL will be reviewed as part of the next development
cycle. See https://golang.org/s/release for more details.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #2 to this change.
FIX: Missing HSTS-header
ADD HSTS header to godoc.org
Please review my changes
# Related
https://github.com/golang/go/issues/26162
Change-Id: I0680e5be76450c5b3e047b7a22169d0a09935e48
GitHub-Last-Rev: 5dc784b1ce3c6c2191b7823a8f93a6a21a388553
GitHub-Pull-Request: golang/gddo#562
---
A gddo-server/debug.test
M gddo-server/main.go
M gddo-server/main_test.go
A vendor/github.com/stretchr/testify
4 files changed, 32 insertions(+), 2 deletions(-)
To view, visit change 122195. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #3 to this change.
FIX: Missing HSTS-header
ADD HSTS header to godoc.org
Please review my changes
# Related
https://github.com/golang/go/issues/26162
Change-Id: I0680e5be76450c5b3e047b7a22169d0a09935e48
GitHub-Last-Rev: 4104bd14fce765b5e525dfcc2a8f13663ea02a33
GitHub-Pull-Request: golang/gddo#562
---
A gddo-server/debug.test
M gddo-server/main.go
M gddo-server/main_test.go
A vendor/github.com/stretchr/testify
4 files changed, 35 insertions(+), 2 deletions(-)
To view, visit change 122195. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #4 to this change.
FIX: Missing HSTS-header
ADD HSTS header to godoc.org
Please review my changes
# Related
https://github.com/golang/go/issues/26162
Change-Id: I0680e5be76450c5b3e047b7a22169d0a09935e48
GitHub-Last-Rev: 86c197da8cffa2858b4362e33a911cf3d4d96b8b
GitHub-Pull-Request: golang/gddo#562
---
A gddo-server/debug.test
M gddo-server/main.go
M gddo-server/main_test.go
3 files changed, 33 insertions(+), 2 deletions(-)
To view, visit change 122195. To unsubscribe, or for help writing mail filters, visit settings.
Akhil Indurti (Gerrit)
2 comments:
Patch Set #4, Line 994:
func middlewareHSTS(next http.Handler) http.Handler {Maybe declare this in gddo/httputil instead:
func hsts(h http.Handler) http.Handler.
Patch Set #4, Line 999:
HSTSHeaderKeyMinor nitpick, but this const header declaration isn't necessary. Follow the convention in the rest of the file:
w.Header().Add("Strict-Transport-Security", "max-age=31536000; includeSubDomains; preload")
To view, visit change 122195. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #5 to this change.
FIX: Missing HSTS-header
ADD HSTS header to godoc.org
Please review my changes
# Related
https://github.com/golang/go/issues/26162
Change-Id: I0680e5be76450c5b3e047b7a22169d0a09935e48
GitHub-Last-Rev: f9314757afd2be14ad6ed0793522ec8cb41440a9
GitHub-Pull-Request: golang/gddo#562
---
A gddo-server/debug.test
M gddo-server/main.go
A httputil/middleware.go
A httputil/middleware_test.go
4 files changed, 40 insertions(+), 2 deletions(-)
To view, visit change 122195. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #6 to this change.
FIX: Missing HSTS-header
ADD HSTS header to godoc.org
Please review my changes
# Related
https://github.com/golang/go/issues/26162
Change-Id: I0680e5be76450c5b3e047b7a22169d0a09935e48
GitHub-Last-Rev: 989725ac16f4846c15033cf661c078d9ebf68dfa
GitHub-Pull-Request: golang/gddo#562
---
M gddo-server/main.go
A httputil/middleware.go
A httputil/middleware_test.go
3 files changed, 40 insertions(+), 2 deletions(-)
To view, visit change 122195. To unsubscribe, or for help writing mail filters, visit settings.
Alpha WONG (Gerrit)
Patch Set 4:
(2 comments)
I change it according to your review
Akhil Indurti (Gerrit)
LGTM, but I haven't reviewed for godoc.org before so I'll ping Brad.
2 comments:
// This enforces the use of HTTPS for 1 year, including present and future subdomains.
// Chrome and Mozilla Firefox maintain an HSTS preload list
// that automatically informs the browser that the website can only be accessed through HTTPS.
This comment should be moved up to be documentation for the exported function.
Patch Set #6, Line 10:
// issue : https://github.com/golang/go/issues/26162This comment is not related to the HSTS function, but the gddo-server's usage of httputil.HSTS. Move it to gddo-server/main.go where httputil.HSTS is called.
Also if you reference an issue number, try to use golang.org/issue/26162 or #26162 to make the issue agnostic to the tracker (github).
To view, visit change 122195. To unsubscribe, or for help writing mail filters, visit settings.
Alpha WONG (Gerrit)
Patch Set 6:
(2 comments)
LGTM, but I haven't reviewed for godoc.org before so I'll ping Brad.
Sure, let me clear it up.
thx
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #7 to this change.
FIX: Missing HSTS-header
ADD HSTS header to godoc.org
Please review my changes
# Related
https://github.com/golang/go/issues/26162
Change-Id: I0680e5be76450c5b3e047b7a22169d0a09935e48
GitHub-Last-Rev: c80d1b2c865074821f631c1ab38070d0ad6f0a49
GitHub-Pull-Request: golang/gddo#562
---
M gddo-server/main.go
A httputil/middleware.go
A httputil/middleware_test.go
3 files changed, 40 insertions(+), 2 deletions(-)
To view, visit change 122195. To unsubscribe, or for help writing mail filters, visit settings.
Martin Tournoij (Gerrit)
1 comment:
What if I am running a local gddo instance and don't want HSTS enabled, or enable it for a shorter period?
One of the downsides of HSTS is that you can't just disable it after you made a mistake, and browsers don't allow users to easily override wrong settings. Enabling it by default can be quite annoying. I think it may be worth considering making an option for this, which should perhaps even default to off.
To view, visit change 122195. To unsubscribe, or for help writing mail filters, visit settings.