Skip to content

Commit

Permalink
webauthn: require session when accessing /.pomerium/webauthn (#3814)
Browse files Browse the repository at this point in the history
* webauthn: require session when accessing /.pomerium/webauthn

* remove dead code

* remove unusued PomeriumDomains field
  • Loading branch information
calebdoxsey committed Dec 16, 2022
1 parent 44a5c1b commit c86ca6f
Show file tree
Hide file tree
Showing 8 changed files with 67 additions and 78 deletions.
6 changes: 0 additions & 6 deletions authenticate/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
18 changes: 18 additions & 0 deletions config/envoyconfig/listeners_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down
1 change: 1 addition & 0 deletions config/envoyconfig/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
6 changes: 5 additions & 1 deletion config/envoyconfig/routes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)+`,
Expand Down Expand Up @@ -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)+`,
Expand Down Expand Up @@ -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)+`,
Expand Down Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions config/policy_ppl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}]
Expand Down
53 changes: 1 addition & 52 deletions internal/handlers/webauthn/webauthn.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ package webauthn
import (
"bytes"
"context"
"encoding/base64"
"encoding/json"
"errors"
"fmt"
Expand All @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
44 changes: 31 additions & 13 deletions pkg/policy/criteria/pomerium_routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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/")`),
}

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 rule, nil, nil
return r1, []*ast.Rule{
rules.GetSession(),
}, nil
}

// PomeriumRoutes returns a Criterion on that allows access to pomerium routes.
Expand Down
6 changes: 0 additions & 6 deletions proxy/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit c86ca6f

Please sign in to comment.