Skip to content

Commit

Permalink
Add some basic headers to OSIN provided pages
Browse files Browse the repository at this point in the history
Use restrictive defaults for basic security hygiene.

Signed-off-by: Simo Sorce <simo@redhat.com>
  • Loading branch information
simo5 committed Oct 25, 2017
1 parent c0f2665 commit 24083ea
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 1 deletion.
4 changes: 3 additions & 1 deletion pkg/auth/server/grant/grant.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/golang/glog"
"github.com/openshift/origin/pkg/auth/authenticator"
"github.com/openshift/origin/pkg/auth/server/csrf"
"github.com/openshift/origin/pkg/auth/server/headers"
scopeauthorizer "github.com/openshift/origin/pkg/authorization/authorizer/scope"
oapi "github.com/openshift/origin/pkg/oauth/apis/oauth"
oauthclient "github.com/openshift/origin/pkg/oauth/generated/internalclientset/typed/oauth/internalversion"
Expand Down Expand Up @@ -106,12 +107,13 @@ func (l *Grant) Install(mux Mux, paths ...string) {
}

func (l *Grant) ServeHTTP(w http.ResponseWriter, req *http.Request) {
headers.SetStandardHeaders(w)

user, ok, err := l.auth.AuthenticateRequest(req)
if err != nil || !ok {
l.redirect("You must reauthenticate before continuing", w, req)
return
}

switch req.Method {
case "GET":
l.handleForm(user, w, req)
Expand Down
29 changes: 29 additions & 0 deletions pkg/auth/server/headers/headers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package headers

import (
"net/http"
)

func SetStandardHeaders(w http.ResponseWriter) {

// We cannot set HSTS by default, it has too many drawbacks in environments
// that use self-signed certs
standardHeaders := map[string]string{
// Turn off caching, it never makes sense for authorization pages
"Cache-Control": "no-cache, no-store",
"Pragma": "no-cache",
"Expires": "0",
// Use a reasonably strict Referer policy by default
"Referrer-Policy": "strict-origin-when-cross-origin",
// Do not allow embedding as that can lead to clickjacking attacks
"X-Frame-Options": "DENY",
// Add other basic scurity hygiene headers
"X-Content-Type-Options": "nosniff",
"X-DNS-Prefetch-Control": "off",
"X-XSS-Protection": "1; mode=block",
}

for key, val := range standardHeaders {
w.Header().Set(key, val)
}
}
2 changes: 2 additions & 0 deletions pkg/auth/server/login/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/openshift/origin/pkg/auth/prometheus"
"github.com/openshift/origin/pkg/auth/server/csrf"
"github.com/openshift/origin/pkg/auth/server/errorpage"
"github.com/openshift/origin/pkg/auth/server/headers"
)

const (
Expand Down Expand Up @@ -95,6 +96,7 @@ func (l *Login) Install(mux Mux, paths ...string) {
}

func (l *Login) ServeHTTP(w http.ResponseWriter, req *http.Request) {
headers.SetStandardHeaders(w)
switch req.Method {
case "GET":
l.handleLoginForm(w, req)
Expand Down
85 changes: 85 additions & 0 deletions test/integration/oauth_server_headers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package integration

import (
"net/http"
"net/url"
"testing"

restclient "k8s.io/client-go/rest"

testutil "github.com/openshift/origin/test/util"
testserver "github.com/openshift/origin/test/util/server"
)

// TestOAuthServerHeaders tests that the Oauth Server pages that return
// browser relevant stuff (HTML) are served with appropriate headers
func TestOAuthServerHeaders(t *testing.T) {
// Set up server
masterOptions, err := testserver.DefaultMasterOptions()
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

defer testserver.CleanupMasterEtcd(t, masterOptions)

clusterAdminKubeConfig, err := testserver.StartConfiguredMaster(masterOptions)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

clientConfig, err := testutil.GetClusterAdminClientConfig(clusterAdminKubeConfig)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

anonConfig := restclient.AnonymousClientConfig(clientConfig)
transport, err := restclient.TransportFor(anonConfig)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

baseURL, err := url.Parse(clientConfig.Host)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

// Hit the login URL
loginURL := *baseURL
loginURL.Path = "/login"
checkNewReqHeaders(t, transport, loginURL.String())

// Hit the grant URL
grantURL := *baseURL
grantURL.Path = "/oauth/authorize/approve"
checkNewReqHeaders(t, transport, grantURL.String())

}

func checkNewReqHeaders(t *testing.T, rt http.RoundTripper, check_url string) {
req, err := http.NewRequest("GET", check_url, nil)
if err != nil {
t.Fatalf("unexpected error %v", err)
}

req.Header.Set("Accept", "text/html; charset=UTF-8")

resp, err := rt.RoundTrip(req)
if err != nil {
t.Fatalf("unexpected error %v", err)
}

checkImportantHeaders := map[string]string{
"Referrer-Policy": "strict-origin-when-cross-origin",
"X-Frame-Options": "DENY",
"X-Content-Type-Options": "nosniff",
"X-DNS-Prefetch-Control": "off",
"X-XSS-Protection": "1; mode=block",
}

for key, val := range checkImportantHeaders {
header := resp.Header.Get(key)
if header != val {
t.Errorf("While probing %s expected header %s: %s, got {%v}", check_url, key, val, header)
}
}
}

0 comments on commit 24083ea

Please sign in to comment.