From c86ca6f76f0e680d74ad7b5ba8619e1329cd419b Mon Sep 17 00:00:00 2001 From: Caleb Doxsey Date: Fri, 16 Dec 2022 10:59:21 -0700 Subject: [PATCH] webauthn: require session when accessing /.pomerium/webauthn (#3814) * webauthn: require session when accessing /.pomerium/webauthn * remove dead code * remove unusued PomeriumDomains field --- authenticate/handlers.go | 6 --- config/envoyconfig/listeners_test.go | 18 +++++++++ config/envoyconfig/routes.go | 1 + config/envoyconfig/routes_test.go | 6 ++- config/policy_ppl_test.go | 11 ++++++ internal/handlers/webauthn/webauthn.go | 53 +------------------------- pkg/policy/criteria/pomerium_routes.go | 44 ++++++++++++++------- proxy/data.go | 6 --- 8 files changed, 67 insertions(+), 78 deletions(-) diff --git a/authenticate/handlers.go b/authenticate/handlers.go index 74d2bed26..1a0980563 100644 --- a/authenticate/handlers.go +++ b/authenticate/handlers.go @@ -649,17 +649,11 @@ func (a *Authenticate) getWebauthnState(r *http.Request) (*webauthn.State, error return nil, err } - pomeriumDomains, err := a.options.Load().GetAllRouteableHTTPDomains() - if err != nil { - return nil, err - } - return &webauthn.State{ AuthenticateURL: authenticateURL, InternalAuthenticateURL: internalAuthenticateURL, SharedKey: state.sharedKey, Client: state.dataBrokerClient, - PomeriumDomains: pomeriumDomains, Session: s, SessionState: ss, SessionStore: state.sessionStore, diff --git a/config/envoyconfig/listeners_test.go b/config/envoyconfig/listeners_test.go index 727a4b12f..b49753c21 100644 --- a/config/envoyconfig/listeners_test.go +++ b/config/envoyconfig/listeners_test.go @@ -253,6 +253,15 @@ func Test_buildMainHTTPConnectionManagerFilter(t *testing.T) { "cluster": "pomerium-control-plane-http" } }, + { + "name": "pomerium-path-/.pomerium/webauthn", + "match": { + "path": "/.pomerium/webauthn" + }, + "route": { + "cluster": "pomerium-control-plane-http" + } + }, { "name": "pomerium-path-/ping", "match": { @@ -394,6 +403,15 @@ func Test_buildMainHTTPConnectionManagerFilter(t *testing.T) { "cluster": "pomerium-control-plane-http" } }, + { + "name": "pomerium-path-/.pomerium/webauthn", + "match": { + "path": "/.pomerium/webauthn" + }, + "route": { + "cluster": "pomerium-control-plane-http" + } + }, { "name": "pomerium-path-/ping", "match": { diff --git a/config/envoyconfig/routes.go b/config/envoyconfig/routes.go index 02b4c0251..e837bdc74 100644 --- a/config/envoyconfig/routes.go +++ b/config/envoyconfig/routes.go @@ -60,6 +60,7 @@ func (b *Builder) buildPomeriumHTTPRoutes(options *config.Options, domain string routes = append(routes, // enable ext_authz b.buildControlPlanePathRoute("/.pomerium/jwt", true), + b.buildControlPlanePathRoute("/.pomerium/webauthn", true), // disable ext_authz and passthrough to proxy handlers b.buildControlPlanePathRoute("/ping", false), b.buildControlPlanePathRoute("/healthz", false), diff --git a/config/envoyconfig/routes_test.go b/config/envoyconfig/routes_test.go index 7604af6f6..246470df8 100644 --- a/config/envoyconfig/routes_test.go +++ b/config/envoyconfig/routes_test.go @@ -88,6 +88,7 @@ func Test_buildPomeriumHTTPRoutes(t *testing.T) { testutil.AssertProtoJSONEqual(t, `[ `+routeString("path", "/.pomerium/jwt", true)+`, + `+routeString("path", "/.pomerium/webauthn", true)+`, `+routeString("path", "/ping", false)+`, `+routeString("path", "/healthz", false)+`, `+routeString("path", "/.pomerium", false)+`, @@ -126,6 +127,7 @@ func Test_buildPomeriumHTTPRoutes(t *testing.T) { testutil.AssertProtoJSONEqual(t, `[ `+routeString("path", "/.pomerium/jwt", true)+`, + `+routeString("path", "/.pomerium/webauthn", true)+`, `+routeString("path", "/ping", false)+`, `+routeString("path", "/healthz", false)+`, `+routeString("path", "/.pomerium", false)+`, @@ -153,6 +155,7 @@ func Test_buildPomeriumHTTPRoutes(t *testing.T) { testutil.AssertProtoJSONEqual(t, `[ `+routeString("path", "/.pomerium/jwt", true)+`, + `+routeString("path", "/.pomerium/webauthn", true)+`, `+routeString("path", "/ping", false)+`, `+routeString("path", "/healthz", false)+`, `+routeString("path", "/.pomerium", false)+`, @@ -249,7 +252,8 @@ func TestTimeouts(t *testing.T) { UpstreamTimeout: getDuration(tc.upstream), IdleTimeout: getDuration(tc.idle), AllowWebsockets: tc.allowWebsockets, - }}, + }, + }, }, "example.com") if !assert.NoError(t, err, "%v", tc) || !assert.Len(t, routes, 1, tc) || !assert.NotNil(t, routes[0].GetRoute(), "%v", tc) { continue diff --git a/config/policy_ppl_test.go b/config/policy_ppl_test.go index 58328a7fa..9c9d99549 100644 --- a/config/policy_ppl_test.go +++ b/config/policy_ppl_test.go @@ -58,8 +58,19 @@ default allow = [false, set()] default deny = [false, set()] pomerium_routes_0 = [true, {"pomerium-route"}] { + session := get_session(input.session.id) + session.id != "" + contains(input.http.url, "/.pomerium/") +} + +else = [true, {"pomerium-route"}] { contains(input.http.url, "/.pomerium/") not contains(input.http.url, "/.pomerium/jwt") + not contains(input.http.url, "/.pomerium/webauthn") +} + +else = [false, {"user-unauthenticated"}] { + contains(input.http.url, "/.pomerium/") } else = [false, {"non-pomerium-route"}] diff --git a/internal/handlers/webauthn/webauthn.go b/internal/handlers/webauthn/webauthn.go index 976d6ff39..6f42e6008 100644 --- a/internal/handlers/webauthn/webauthn.go +++ b/internal/handlers/webauthn/webauthn.go @@ -4,7 +4,6 @@ package webauthn import ( "bytes" "context" - "encoding/base64" "encoding/json" "errors" "fmt" @@ -16,12 +15,10 @@ import ( "google.golang.org/grpc/status" "google.golang.org/protobuf/types/known/timestamppb" - "github.com/pomerium/pomerium/internal/encoding/jws" "github.com/pomerium/pomerium/internal/httputil" "github.com/pomerium/pomerium/internal/middleware" "github.com/pomerium/pomerium/internal/sessions" "github.com/pomerium/pomerium/internal/urlutil" - "github.com/pomerium/pomerium/pkg/cryptutil" "github.com/pomerium/pomerium/pkg/grpc/databroker" "github.com/pomerium/pomerium/pkg/grpc/device" "github.com/pomerium/pomerium/pkg/grpc/session" @@ -49,7 +46,6 @@ type State struct { AuthenticateURL *url.URL InternalAuthenticateURL *url.URL Client databroker.DataBrokerServiceClient - PomeriumDomains []string RelyingParty *webauthn.RelyingParty Session *session.Session SessionState *sessions.State @@ -423,39 +419,7 @@ func (h *Handler) saveSessionAndRedirect(w http.ResponseWriter, r *http.Request, return err } - // if the redirect URL is for a URL we don't control, just do a plain redirect - if !isURLForPomerium(state.PomeriumDomains, rawRedirectURI) { - httputil.Redirect(w, r, rawRedirectURI, http.StatusFound) - return nil - } - - // sign+encrypt the session JWT - encoder, err := jws.NewHS256Signer(state.SharedKey) - if err != nil { - return err - } - - signedJWT, err := encoder.Marshal(state.SessionState) - if err != nil { - return err - } - - cipher, err := cryptutil.NewAEADCipher(state.SharedKey) - if err != nil { - return err - } - - encryptedJWT := cryptutil.Encrypt(cipher, signedJWT, nil) - encodedJWT := base64.URLEncoding.EncodeToString(encryptedJWT) - - // redirect to the proxy callback URL with the session - callbackURL, err := urlutil.GetCallbackURLForRedirectURI(r, encodedJWT, rawRedirectURI) - if err != nil { - return err - } - - signedCallbackURL := urlutil.NewSignedURL(state.SharedKey, callbackURL) - httputil.Redirect(w, r, signedCallbackURL.String(), http.StatusFound) + httputil.Redirect(w, r, rawRedirectURI, http.StatusFound) return nil } @@ -533,18 +497,3 @@ func getOrCreateDeviceEnrollment( } return deviceEnrollment, nil } - -func isURLForPomerium(pomeriumDomains []string, rawURI string) bool { - uri, err := urlutil.ParseAndValidateURL(rawURI) - if err != nil { - return false - } - - for _, domain := range pomeriumDomains { - if urlutil.StripPort(domain) == urlutil.StripPort(uri.Host) { - return true - } - } - - return false -} diff --git a/pkg/policy/criteria/pomerium_routes.go b/pkg/policy/criteria/pomerium_routes.go index 76b91d047..dd150dc8a 100644 --- a/pkg/policy/criteria/pomerium_routes.go +++ b/pkg/policy/criteria/pomerium_routes.go @@ -5,17 +5,9 @@ import ( "github.com/pomerium/pomerium/pkg/policy/generator" "github.com/pomerium/pomerium/pkg/policy/parser" + "github.com/pomerium/pomerium/pkg/policy/rules" ) -var pomeriumRoutesBody = ast.Body{ - ast.MustParseExpr(` - contains(input.http.url, "/.pomerium/") - `), - ast.MustParseExpr(` - not contains(input.http.url, "/.pomerium/jwt") - `), -} - type pomeriumRoutesCriterion struct { g *Generator } @@ -29,11 +21,37 @@ func (pomeriumRoutesCriterion) Name() string { } func (c pomeriumRoutesCriterion) GenerateRule(_ string, _ parser.Value) (*ast.Rule, []*ast.Rule, error) { - rule := NewCriterionRule(c.g, c.Name(), - ReasonPomeriumRoute, ReasonNonPomeriumRoute, - pomeriumRoutesBody) + r1 := c.g.NewRule(c.Name()) + r1.Head.Value = NewCriterionTerm(true, ReasonPomeriumRoute) + r1.Body = ast.Body{ + ast.MustParseExpr(`session := get_session(input.session.id)`), + ast.MustParseExpr(`session.id != ""`), + ast.MustParseExpr(`contains(input.http.url, "/.pomerium/")`), + } - return rule, nil, nil + r2 := c.g.NewRule(c.Name()) + r2.Head.Value = NewCriterionTerm(true, ReasonPomeriumRoute) + r2.Body = ast.Body{ + ast.MustParseExpr(`contains(input.http.url, "/.pomerium/")`), + ast.MustParseExpr(`not contains(input.http.url, "/.pomerium/jwt")`), + ast.MustParseExpr(`not contains(input.http.url, "/.pomerium/webauthn")`), + } + r1.Else = r2 + + r3 := c.g.NewRule(c.Name()) + r3.Head.Value = NewCriterionTerm(false, ReasonUserUnauthenticated) + r3.Body = ast.Body{ + ast.MustParseExpr(`contains(input.http.url, "/.pomerium/")`), + } + r2.Else = r3 + + r4 := c.g.NewRule(c.Name()) + r4.Head.Value = NewCriterionTerm(false, ReasonNonPomeriumRoute) + r3.Else = r4 + + return r1, []*ast.Rule{ + rules.GetSession(), + }, nil } // PomeriumRoutes returns a Criterion on that allows access to pomerium routes. diff --git a/proxy/data.go b/proxy/data.go index cab09fb6d..09cefe583 100644 --- a/proxy/data.go +++ b/proxy/data.go @@ -131,17 +131,11 @@ func (p *Proxy) getWebauthnState(r *http.Request) (*webauthn.State, error) { return nil, err } - pomeriumDomains, err := options.GetAllRouteableHTTPDomains() - if err != nil { - return nil, err - } - return &webauthn.State{ AuthenticateURL: authenticateURL, InternalAuthenticateURL: internalAuthenticateURL, SharedKey: state.sharedKey, Client: state.dataBrokerClient, - PomeriumDomains: pomeriumDomains, Session: s, SessionState: &ss, SessionStore: state.sessionStore,