Skip to content

Commit 2962c86

Browse files
committed
Add some basic headers to OSIN provided pages
Use restrictive defaults for basic security hygiene. Signed-off-by: Simo Sorce <[email protected]>
1 parent c0f2665 commit 2962c86

File tree

4 files changed

+121
-1
lines changed

4 files changed

+121
-1
lines changed

pkg/auth/server/grant/grant.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/golang/glog"
1616
"github.com/openshift/origin/pkg/auth/authenticator"
1717
"github.com/openshift/origin/pkg/auth/server/csrf"
18+
"github.com/openshift/origin/pkg/auth/server/headers"
1819
scopeauthorizer "github.com/openshift/origin/pkg/authorization/authorizer/scope"
1920
oapi "github.com/openshift/origin/pkg/oauth/apis/oauth"
2021
oauthclient "github.com/openshift/origin/pkg/oauth/generated/internalclientset/typed/oauth/internalversion"
@@ -106,12 +107,13 @@ func (l *Grant) Install(mux Mux, paths ...string) {
106107
}
107108

108109
func (l *Grant) ServeHTTP(w http.ResponseWriter, req *http.Request) {
110+
headers.SetStandardHeaders(w)
111+
109112
user, ok, err := l.auth.AuthenticateRequest(req)
110113
if err != nil || !ok {
111114
l.redirect("You must reauthenticate before continuing", w, req)
112115
return
113116
}
114-
115117
switch req.Method {
116118
case "GET":
117119
l.handleForm(user, w, req)

pkg/auth/server/headers/headers.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package headers
2+
3+
import (
4+
"net/http"
5+
)
6+
7+
func SetStandardHeaders(w http.ResponseWriter) {
8+
9+
// We cannot set HSTS by default, it has too many drawbacks in environments
10+
// that use self-signed certs
11+
standardHeaders := map[string]string{
12+
// Turn off caching, it never makes sense for authorization pages
13+
"Cache-Control": "no-cache, no-store",
14+
"Pragma": "no-cache",
15+
"Expires": "0",
16+
// Use a reasonably strict Referer policy by default
17+
"Referrer-Policy": "strict-origin-when-cross-origin",
18+
// Do not allow embedding as that can lead to clickjacking attacks
19+
"X-Frame-Options": "DENY",
20+
// Add other basic scurity hygiene headers
21+
"X-Content-Type-Options": "nosniff",
22+
"X-DNS-Prefetch-Control": "off",
23+
"X-XSS-Protection": "1; mode=block",
24+
}
25+
26+
for key, val := range standardHeaders {
27+
w.Header().Set(key, val)
28+
}
29+
}

pkg/auth/server/login/login.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/openshift/origin/pkg/auth/prometheus"
1818
"github.com/openshift/origin/pkg/auth/server/csrf"
1919
"github.com/openshift/origin/pkg/auth/server/errorpage"
20+
"github.com/openshift/origin/pkg/auth/server/headers"
2021
)
2122

2223
const (
@@ -95,6 +96,7 @@ func (l *Login) Install(mux Mux, paths ...string) {
9596
}
9697

9798
func (l *Login) ServeHTTP(w http.ResponseWriter, req *http.Request) {
99+
headers.SetStandardHeaders(w)
98100
switch req.Method {
99101
case "GET":
100102
l.handleLoginForm(w, req)
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
package integration
2+
3+
import (
4+
"net/http"
5+
"net/url"
6+
"testing"
7+
8+
restclient "k8s.io/client-go/rest"
9+
10+
testutil "github.com/openshift/origin/test/util"
11+
testserver "github.com/openshift/origin/test/util/server"
12+
)
13+
14+
// TestOAuthServerHeaders tests that the Oauth Server pages that return
15+
// browser relevant stuff (HTML) are served with appropriate headers
16+
func TestOAuthServerHeaders(t *testing.T) {
17+
// Set up server
18+
masterOptions, err := testserver.DefaultMasterOptions()
19+
if err != nil {
20+
t.Fatalf("unexpected error: %v", err)
21+
}
22+
23+
defer testserver.CleanupMasterEtcd(t, masterOptions)
24+
25+
clusterAdminKubeConfig, err := testserver.StartConfiguredMaster(masterOptions)
26+
if err != nil {
27+
t.Fatalf("unexpected error: %v", err)
28+
}
29+
30+
clientConfig, err := testutil.GetClusterAdminClientConfig(clusterAdminKubeConfig)
31+
if err != nil {
32+
t.Fatalf("unexpected error: %v", err)
33+
}
34+
35+
anonConfig := restclient.AnonymousClientConfig(clientConfig)
36+
transport, err := restclient.TransportFor(anonConfig)
37+
if err != nil {
38+
t.Fatalf("unexpected error: %v", err)
39+
}
40+
41+
baseURL, err := url.Parse(clientConfig.Host)
42+
if err != nil {
43+
t.Fatalf("unexpected error: %v", err)
44+
}
45+
46+
// Hit the login URL
47+
loginURL := &url.URL{}
48+
*loginURL = *baseURL
49+
loginURL.Path = "/login"
50+
checkNewReqHeaders(t, transport, loginURL.String())
51+
52+
// Hit the grant URL
53+
grantURL := &url.URL{}
54+
*grantURL = *baseURL
55+
grantURL.Path = "/oauth/authorize/approve"
56+
checkNewReqHeaders(t, transport, grantURL.String())
57+
58+
}
59+
60+
func checkNewReqHeaders(t *testing.T, rt http.RoundTripper, check_url string) {
61+
req, err := http.NewRequest("GET", check_url, nil)
62+
if err != nil {
63+
t.Fatalf("unexpected error %v", err)
64+
}
65+
66+
req.Header.Set("Accept", "text/html; charset=UTF-8")
67+
68+
resp, err := rt.RoundTrip(req)
69+
if err != nil {
70+
t.Fatalf("unexpected error %v", err)
71+
}
72+
73+
checkImportantHeaders := map[string]string{
74+
"Referrer-Policy": "strict-origin-when-cross-origin",
75+
"X-Frame-Options": "DENY",
76+
"X-Content-Type-Options": "nosniff",
77+
"X-DNS-Prefetch-Control": "off",
78+
"X-XSS-Protection": "1; mode=block",
79+
}
80+
81+
for key, val := range checkImportantHeaders {
82+
header := resp.Header.Get(key)
83+
if header != val {
84+
t.Fatalf("While probing %s expected header %s: %s, got {%v}", check_url, key, val, header)
85+
}
86+
}
87+
}

0 commit comments

Comments
 (0)