cmd/pomerium: move middleware for all http handlers to global context (#117)

authenticate/handlers.go

@@ -6,7 +6,6 @@ import (
 	"net/http"
 	"net/url"
 	"strings"
-	"time"
 
 	"github.com/pomerium/pomerium/internal/cryptutil"
 	"github.com/pomerium/pomerium/internal/httputil"
@@ -16,56 +15,20 @@ import (
 	"github.com/pomerium/pomerium/internal/version"
 )
 
-// securityHeaders corresponds to HTTP response headers that help to protect
-// against protocol downgrade attacks and cookie hijacking.
-//
-// https://www.owasp.org/index.php/OWASP_Secure_Headers_Project#tab=Headers
-// https://https.cio.gov/hsts/
-var securityHeaders = map[string]string{
-	"Strict-Transport-Security": "max-age=31536000; includeSubDomains; preload",
-	"X-Frame-Options":           "DENY",
-	"X-Content-Type-Options":    "nosniff",
-	"X-XSS-Protection":          "1; mode=block",
-	"Content-Security-Policy": "default-src 'none'; style-src 'self' " +
-		"'sha256-pSTVzZsFAqd2U3QYu+BoBDtuJWaPM/+qMy/dBRrhb5Y='; img-src 'self';",
-	"Referrer-Policy": "Same-origin",
-}
-
 // Handler returns the authenticate service's HTTP request multiplexer, and routes.
 func (a *Authenticate) Handler() http.Handler {
-	// set up our standard middlewares
+	// validation middleware chain
-	stdMiddleware := middleware.NewChain()
+	validate := middleware.NewChain()
-	stdMiddleware = stdMiddleware.Append(middleware.Healthcheck("/ping", version.UserAgent()))
+	validate = validate.Append(middleware.ValidateSignature(a.SharedKey))
-	stdMiddleware = stdMiddleware.Append(middleware.NewHandler(log.Logger))
+	validate = validate.Append(middleware.ValidateRedirectURI(a.RedirectURL))
-	stdMiddleware = stdMiddleware.Append(middleware.AccessHandler(
-		func(r *http.Request, status, size int, duration time.Duration) {
-			middleware.FromRequest(r).Debug().
-				Str("method", r.Method).
-				Str("url", r.URL.String()).
-				Int("status", status).
-				Int("size", size).
-				Dur("duration", duration).
-				Msg("authenticate: request")
-		}))
-	stdMiddleware = stdMiddleware.Append(middleware.SetHeaders(securityHeaders))
-	stdMiddleware = stdMiddleware.Append(middleware.ForwardedAddrHandler("fwd_ip"))
-	stdMiddleware = stdMiddleware.Append(middleware.RemoteAddrHandler("ip"))
-	stdMiddleware = stdMiddleware.Append(middleware.UserAgentHandler("user_agent"))
-	stdMiddleware = stdMiddleware.Append(middleware.RefererHandler("referer"))
-	stdMiddleware = stdMiddleware.Append(middleware.RequestIDHandler("req_id", "Request-Id"))
-	validateSignatureMiddleware := stdMiddleware.Append(
-		middleware.ValidateSignature(a.SharedKey),
-		middleware.ValidateRedirectURI(a.RedirectURL))
-
 	mux := http.NewServeMux()
-	mux.Handle("/robots.txt", stdMiddleware.ThenFunc(a.RobotsTxt))
+	mux.HandleFunc("/robots.txt", a.RobotsTxt)
 	// Identity Provider (IdP) callback endpoints and callbacks
-	mux.Handle("/start", stdMiddleware.ThenFunc(a.OAuthStart))
+	mux.HandleFunc("/start", a.OAuthStart)
-	mux.Handle("/oauth2/callback", stdMiddleware.ThenFunc(a.OAuthCallback))
+	mux.HandleFunc("/oauth2/callback", a.OAuthCallback)
 	// authenticate-server endpoints
-	mux.Handle("/sign_in", validateSignatureMiddleware.ThenFunc(a.SignIn))
+	mux.Handle("/sign_in", validate.ThenFunc(a.SignIn))
-	mux.Handle("/sign_out", validateSignatureMiddleware.ThenFunc(a.SignOut)) // GET POST
+	mux.Handle("/sign_out", validate.ThenFunc(a.SignOut)) // GET POST
-
 	return mux
 }
 

authorize/gprc.go

@@ -5,14 +5,11 @@ import (
 	"context"
 
 	pb "github.com/pomerium/pomerium/proto/authorize"
-
-	"github.com/pomerium/pomerium/internal/log"
 )
 
 // Authorize validates the user identity, device, and context of a request for
 // a given route. Currently only checks identity.
 func (a *Authorize) Authorize(ctx context.Context, in *pb.AuthorizeRequest) (*pb.AuthorizeReply, error) {
 	ok := a.ValidIdentity(in.Route, &Identity{in.User, in.Email, in.Groups})
-	log.Debug().Str("route", in.Route).Strs("groups", in.Groups).Str("email", in.Email).Bool("Valid?", ok).Msg("authorize/grpc")
 	return &pb.AuthorizeReply{IsValid: ok}, nil
 }

cmd/pomerium/main.go

@@ -30,7 +30,7 @@ func main() {
 		fmt.Println(version.FullVersion())
 		os.Exit(0)
 	}
-	opt, err := parseOptions()
+	opt, err := optionsFromEnvConfig()
 	if err != nil {
 		log.Fatal().Err(err).Msg("cmd/pomerium: options")
 	}
@@ -40,10 +40,6 @@ func main() {
 	grpcServer := grpc.NewServer(grpcOpts...)
 
 	mux := http.NewServeMux()
-	mux.HandleFunc("/ping", func(rw http.ResponseWriter, _ *http.Request) {
-		rw.WriteHeader(http.StatusOK)
-		fmt.Fprintf(rw, version.UserAgent())
-	})
 
 	_, err = newAuthenticateService(opt.Services, mux, grpcServer)
 	if err != nil {
@@ -80,7 +76,8 @@ func main() {
 	} else {
 		defer srv.Close()
 	}
-	if err := https.ListenAndServeTLS(httpOpts, mux, grpcServer); err != nil {
+
+	if err := https.ListenAndServeTLS(httpOpts, wrapMiddleware(opt, mux), grpcServer); err != nil {
 		log.Fatal().Err(err).Msg("cmd/pomerium: https server")
 	}
 }
@@ -154,16 +151,31 @@ func newProxyService(s string, mux *http.ServeMux) (*proxy.Proxy, error) {
 	return service, nil
 }
 
-func parseOptions() (*Options, error) {
+func wrapMiddleware(o *Options, mux *http.ServeMux) http.Handler {
-	o, err := optionsFromEnvConfig()
+	c := middleware.NewChain()
-	if err != nil {
+	c = c.Append(middleware.NewHandler(log.Logger))
-		return nil, err
+	c = c.Append(middleware.AccessHandler(func(r *http.Request, status, size int, duration time.Duration) {
+		middleware.FromRequest(r).Debug().
+			Str("service", o.Services).
+			Str("method", r.Method).
+			Str("url", r.URL.String()).
+			Int("status", status).
+			Int("size", size).
+			Dur("duration", duration).
+			Str("user", r.Header.Get(proxy.HeaderUserID)).
+			Str("email", r.Header.Get(proxy.HeaderEmail)).
+			Str("group", r.Header.Get(proxy.HeaderGroups)).
+			// Str("sig", r.Header.Get(proxy.HeaderJWT)).
+			Msg("http-request")
+	}))
+	if o != nil && len(o.Headers) != 0 {
+		c = c.Append(middleware.SetHeaders(o.Headers))
 	}
-	if o.Debug {
+	c = c.Append(middleware.ForwardedAddrHandler("fwd_ip"))
-		log.SetDebugMode()
+	c = c.Append(middleware.RemoteAddrHandler("ip"))
-	}
+	c = c.Append(middleware.UserAgentHandler("user_agent"))
-	if o.LogLevel != "" {
+	c = c.Append(middleware.RefererHandler("referer"))
-		log.SetLevel(o.LogLevel)
+	c = c.Append(middleware.RequestIDHandler("req_id", "Request-Id"))
-	}
+	c = c.Append(middleware.Healthcheck("/ping", version.UserAgent()))
-	return o, nil
+	return c.Then(mux)
 }

cmd/pomerium/main_test.go

@@ -1,10 +1,11 @@
 package main
 
 import (
+	"fmt"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"os"
-	"reflect"
 	"strings"
 	"testing"
 
@@ -166,34 +167,33 @@ func Test_newProxyeService(t *testing.T) {
 	}
 }
 
-func Test_parseOptions(t *testing.T) {
+func Test_wrapMiddleware(t *testing.T) {
-	tests := []struct {
+	o := &Options{
-		name     string
+		Services: "all",
-		envKey   string
+		Headers: map[string]string{
-		envValue string
+			"X-Content-Type-Options":    "nosniff",
+			"X-Frame-Options":           "SAMEORIGIN",
+			"X-XSS-Protection":          "1; mode=block",
+			"Strict-Transport-Security": "max-age=31536000; includeSubDomains; preload",
+			"Content-Security-Policy":   "default-src 'none'; style-src 'self' 'sha256-pSTVzZsFAqd2U3QYu+BoBDtuJWaPM/+qMy/dBRrhb5Y='; img-src 'self';",
+			"Referrer-Policy":           "Same-origin",
+		}}
+	mux := http.NewServeMux()
+	req := httptest.NewRequest(http.MethodGet, "/404", nil)
+	rr := httptest.NewRecorder()
+	h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		w.Header().Set("Content-Type", "application/json")
+		io.WriteString(w, `OK`)
+	})
 
-		want    *Options
+	mux.Handle("/404", h)
-		wantErr bool
+	out := wrapMiddleware(o, mux)
-	}{
+	out.ServeHTTP(rr, req)
-		{"no shared secret", "", "", nil, true},
+	expected := fmt.Sprintf("OK")
-		{"good", "SHARED_SECRET", "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM=", &Options{Services: "all", SharedKey: "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM=", LogLevel: "debug"}, false},
+	body := rr.Body.String()
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			os.Setenv(tt.envKey, tt.envValue)
-			defer os.Unsetenv(tt.envKey)
 
-			got, err := parseOptions()
+	if body != expected {
-			if (err != nil) != tt.wantErr {
+		t.Errorf("handler returned unexpected body: got %v want %v", body, expected)
-				t.Errorf("parseOptions() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
-			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("parseOptions()\n")
-				t.Errorf("got: %+v\n", got)
-				t.Errorf("want: %+v\n", tt.want)
-
-			}
-		})
 	}
 }

cmd/pomerium/options.go

@@ -6,8 +6,12 @@ import (
 	"time"
 
 	"github.com/pomerium/envconfig"
+	"github.com/pomerium/pomerium/internal/log"
 )
 
+// DisableHeaderKey is the key used to check whether to disable setting header
+const DisableHeaderKey = "disable"
+
 // Options are the global environmental flags used to set up pomerium's services.
 // If a base64 encoded certificate and key are not provided as environmental variables,
 // or if a file location is not provided, the server will attempt to find a matching keypair
@@ -45,6 +49,9 @@ type Options struct {
 	// on port 80.  If empty, no redirect server is started.
 	HTTPRedirectAddr string `envconfig:"HTTP_REDIRECT_ADDR"`
 
+	// Headers to set on all proxied requests. Add a 'disable' key map to turn off.
+	Headers map[string]string `envconfig:"HEADERS"`
+
 	// Timeout settings : https://github.com/pomerium/pomerium/issues/40
 	ReadTimeout       time.Duration `envconfig:"TIMEOUT_READ"`
 	WriteTimeout      time.Duration `envconfig:"TIMEOUT_WRITE"`
@@ -56,6 +63,14 @@ var defaultOptions = &Options{
 	Debug:    false,
 	LogLevel: "debug",
 	Services: "all",
+	Headers: map[string]string{
+		"X-Content-Type-Options":    "nosniff",
+		"X-Frame-Options":           "SAMEORIGIN",
+		"X-XSS-Protection":          "1; mode=block",
+		"Strict-Transport-Security": "max-age=31536000; includeSubDomains; preload",
+		"Content-Security-Policy":   "default-src 'none'; style-src 'self' 'sha256-pSTVzZsFAqd2U3QYu+BoBDtuJWaPM/+qMy/dBRrhb5Y='; img-src 'self';",
+		"Referrer-Policy":           "Same-origin",
+	},
 }
 
 // optionsFromEnvConfig builds the main binary's configuration
@@ -71,6 +86,15 @@ func optionsFromEnvConfig() (*Options, error) {
 	if o.SharedKey == "" {
 		return nil, errors.New("shared-key cannot be empty")
 	}
+	if o.Debug {
+		log.SetDebugMode()
+	}
+	if o.LogLevel != "" {
+		log.SetLevel(o.LogLevel)
+	}
+	if _, disable := o.Headers[DisableHeaderKey]; disable {
+		o.Headers = make(map[string]string)
+	}
 	return o, nil
 }
 

proxy/handlers.go

@@ -16,7 +16,6 @@ import (
 	"github.com/pomerium/pomerium/internal/middleware"
 	"github.com/pomerium/pomerium/internal/policy"
 	"github.com/pomerium/pomerium/internal/sessions"
-	"github.com/pomerium/pomerium/internal/version"
 )
 
 var (
@@ -32,7 +31,12 @@ type StateParameter struct {
 
 // Handler returns a http handler for a Proxy
 func (p *Proxy) Handler() http.Handler {
-	// routes
+	// validation middleware chain
+	validate := middleware.NewChain()
+	validate = validate.Append(middleware.ValidateHost(func(host string) bool {
+		_, ok := p.routeConfigs[host]
+		return ok
+	}))
 	mux := http.NewServeMux()
 	mux.HandleFunc("/favicon.ico", p.Favicon)
 	mux.HandleFunc("/robots.txt", p.RobotsTxt)
@@ -40,37 +44,7 @@ func (p *Proxy) Handler() http.Handler {
 	mux.HandleFunc("/.pomerium/callback", p.OAuthCallback)
 	// mux.HandleFunc("/.pomerium/refresh", p.Refresh) //todo(bdd): needs DoS protection before inclusion
 	mux.HandleFunc("/", p.Proxy)
-
+	return validate.Then(mux)
-	// middleware chain
-	c := middleware.NewChain()
-	c = c.Append(middleware.Healthcheck("/ping", version.UserAgent()))
-	c = c.Append(middleware.NewHandler(log.Logger))
-	c = c.Append(middleware.AccessHandler(func(r *http.Request, status, size int, duration time.Duration) {
-		middleware.FromRequest(r).Debug().
-			Str("method", r.Method).
-			Str("url", r.URL.String()).
-			Int("status", status).
-			Int("size", size).
-			Dur("duration", duration).
-			Str("pomerium-user", r.Header.Get(HeaderUserID)).
-			Str("pomerium-email", r.Header.Get(HeaderEmail)).
-			Msg("proxy: request")
-	}))
-	c = c.Append(middleware.SetHeaders(p.headers))
-	c = c.Append(middleware.ForwardedAddrHandler("fwd_ip"))
-	c = c.Append(middleware.RemoteAddrHandler("ip"))
-	c = c.Append(middleware.UserAgentHandler("user_agent"))
-	c = c.Append(middleware.RefererHandler("referer"))
-	c = c.Append(middleware.RequestIDHandler("req_id", "Request-Id"))
-	c = c.Append(middleware.ValidateHost(func(host string) bool {
-		_, ok := p.routeConfigs[host]
-		return ok
-	}))
-
-	// serve the middleware and mux
-	return c.Then(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		mux.ServeHTTP(w, r)
-	}))
 }
 
 // RobotsTxt sets the User-Agent header in the response to be "Disallow"

proxy/handlers_test.go

@@ -13,7 +13,6 @@ import (
 	"time"
 
 	"github.com/pomerium/pomerium/internal/sessions"
-	"github.com/pomerium/pomerium/internal/version"
 	"github.com/pomerium/pomerium/proxy/clients"
 )
 
@@ -206,14 +205,11 @@ func TestProxy_Handler(t *testing.T) {
 	}
 	mux := http.NewServeMux()
 	mux.Handle("/", h)
-	req := httptest.NewRequest("GET", "/ping", nil)
+	req := httptest.NewRequest("GET", "/", nil)
-
 	rr := httptest.NewRecorder()
 	mux.ServeHTTP(rr, req)
-	expected := version.UserAgent()
+	if rr.Code != http.StatusNotFound {
-	body := rr.Body.String()
+		t.Errorf("expected 404 route not found for empty route")
-	if body != expected {
-		t.Errorf("handler returned unexpected body: got %v want %v", body, expected)
 	}
 }
 

proxy/proxy.go

@@ -31,8 +31,6 @@ const (
 	HeaderEmail = "x-pomerium-authenticated-user-email"
 	// HeaderGroups is the header key containing the user's groups.
 	HeaderGroups = "x-pomerium-authenticated-user-groups"
-	// DisableHeaderKey is the key used to check whether to disable setting header
-	DisableHeaderKey = "disable"
 )
 
 // Options represents the configurations available for the proxy service.
@@ -72,9 +70,6 @@ type Options struct {
 	CookieExpire   time.Duration `envconfig:"COOKIE_EXPIRE"`
 	CookieRefresh  time.Duration `envconfig:"COOKIE_REFRESH"`
 
-	// Headers to set on all proxied requests. Add a 'disable' key map to turn off.
-	Headers map[string]string `envconfig:"HEADERS"`
-
 	// Sub-routes
 	Routes                 map[string]string `envconfig:"ROUTES"`
 	DefaultUpstreamTimeout time.Duration     `envconfig:"DEFAULT_UPSTREAM_TIMEOUT"`
@@ -88,12 +83,6 @@ var defaultOptions = &Options{
 	CookieExpire:           time.Duration(14) * time.Hour,
 	CookieRefresh:          time.Duration(30) * time.Minute,
 	DefaultUpstreamTimeout: time.Duration(30) * time.Second,
-	Headers: map[string]string{
-		"X-Content-Type-Options":    "nosniff",
-		"X-Frame-Options":           "SAMEORIGIN",
-		"X-XSS-Protection":          "1; mode=block",
-		"Strict-Transport-Security": "max-age=31536000; includeSubDomains; preload",
-	},
 }
 
 // OptionsFromEnvConfig builds the identity provider service's configuration
@@ -189,7 +178,6 @@ type Proxy struct {
 	redirectURL  *url.URL
 	templates    *template.Template
 	routeConfigs map[string]*routeConfig
-	headers      map[string]string
 }
 
 type routeConfig struct {
@@ -227,12 +215,6 @@ func New(opts *Options) (*Proxy, error) {
 		return nil, err
 	}
 
-	// if the disable key is found in the security header map, clear the map
-	if _, disable := opts.Headers[DisableHeaderKey]; disable {
-		opts.Headers = make(map[string]string)
-	}
-	log.Debug().Interface("headers", opts.Headers).Msg("proxy: security headers")
-
 	p := &Proxy{
 		routeConfigs: make(map[string]*routeConfig),
 		// services
@@ -244,7 +226,6 @@ func New(opts *Options) (*Proxy, error) {
 		SharedKey:    opts.SharedKey,
 		redirectURL:  &url.URL{Path: "/.pomerium/callback"},
 		templates:    templates.New(),
-		headers:      opts.Headers,
 	}
 	var policies []policy.Policy
 	if opts.Policy != "" {

proxy/proxy_test.go

@@ -124,7 +124,6 @@ func testOptions() *Options {
 		SharedKey:       "80ldlrU2d7w+wVpKNfevk6fmb8otEx6CqOfshj2LwhQ=",
 		CookieSecret:    "OromP1gurwGWjQPYb1nNgSxtbVB5NnLzX6z5WOKr0Yw=",
 		CookieName:      "pomerium",
-		Headers:         defaultOptions.Headers,
 	}
 }
 
@@ -206,23 +205,18 @@ func TestNew(t *testing.T) {
 	shortCookieLength.CookieSecret = "gN3xnvfsAwfCXxnJorGLKUG4l2wC8sS8nfLMhcStPg=="
 	badRoutedProxy := testOptions()
 	badRoutedProxy.SigningKey = "YmFkIGtleQo="
-	disableHeaders := testOptions()
-	disableHeaders.Headers = map[string]string{"disable": "true"}
-
 	tests := []struct {
-		name       string
+		name      string
-		opts       *Options
+		opts      *Options
-		wantProxy  bool
+		wantProxy bool
-		numRoutes  int
+		numRoutes int
-		wantErr    bool
+		wantErr   bool
-		numHeaders int
 	}{
-		{"good", good, true, 1, false, len(defaultOptions.Headers)},
+		{"good", good, true, 1, false},
-		{"empty options", &Options{}, false, 0, true, 0},
+		{"empty options", &Options{}, false, 0, true},
-		{"nil options", nil, false, 0, true, 0},
+		{"nil options", nil, false, 0, true},
-		{"short secret/validate sanity check", shortCookieLength, false, 0, true, 0},
+		{"short secret/validate sanity check", shortCookieLength, false, 0, true},
-		{"invalid ec key, valid base64 though", badRoutedProxy, false, 0, true, 0},
+		{"invalid ec key, valid base64 though", badRoutedProxy, false, 0, true},
-		{"test disabled headers", disableHeaders, false, 1, false, 0},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -237,10 +231,6 @@ func TestNew(t *testing.T) {
 			if got != nil && len(got.routeConfigs) != tt.numRoutes {
 				t.Errorf("New() = num routeConfigs \n%+v, want \n%+v", got, tt.numRoutes)
 			}
-			if got != nil && len(got.headers) != tt.numHeaders {
-				t.Errorf("New() = num Headers \n%+v, want \n%+v", got.headers, tt.numHeaders)
-			}
-
 		})
 	}
 }