From 61fb909aadd289a3d9f4fd828c33e228b1adfa11 Mon Sep 17 00:00:00 2001 From: David Mihalcik Date: Thu, 14 Nov 2024 15:36:42 -0500 Subject: [PATCH 1/3] fix(core): Updates dpop check for connect - Threads through Dpop token and REST service path from gRPC gateway - This is needed when DPoP is enabled (required) OR when the IdP requests it --- service/internal/auth/authn.go | 52 +++++++++++++++++++++-------- service/internal/auth/authn_test.go | 4 +-- service/internal/server/server.go | 28 +++++++++++++++- 3 files changed, 68 insertions(+), 16 deletions(-) diff --git a/service/internal/auth/authn.go b/service/internal/auth/authn.go index 31f9991b3..ba32698ac 100644 --- a/service/internal/auth/authn.go +++ b/service/internal/auth/authn.go @@ -181,10 +181,10 @@ func NewAuthenticator(ctx context.Context, cfg Config, logger *logger.Logger, we } type receiverInfo struct { - // The URI of the request - u string - // The HTTP method of the request - m string + // Acceptable URIs of the request + u []string + // Allowed HTTP methods of the request + m []string } func normalizeURL(o string, u *url.URL) string { @@ -209,6 +209,8 @@ func (a Authentication) MuxHandler(handler http.Handler) http.Handler { return } + dp := r.Header.Values("Dpop") + // Verify the token header := r.Header["Authorization"] if len(header) < 1 { @@ -225,10 +227,11 @@ func (a Authentication) MuxHandler(handler http.Handler) http.Handler { } } accessTok, ctxWithJWK, err := a.checkToken(r.Context(), header, receiverInfo{ - u: normalizeURL(origin, r.URL), - m: r.Method, - }, r.Header["Dpop"]) + u: []string{normalizeURL(origin, r.URL)}, + m: []string{r.Method}, + }, dp) if err != nil { + slog.WarnContext(r.Context(), "unauthenticated", "error", err, "dpop", dp, "authorization", header) http.Error(w, "unauthenticated", http.StatusUnauthorized) return } @@ -270,6 +273,32 @@ func (a Authentication) ConnectUnaryServerInterceptor() connect.UnaryInterceptor ctx context.Context, req connect.AnyRequest, ) (connect.AnyResponse, error) { + ri := receiverInfo{ + u: []string{req.Spec().Procedure}, + m: []string{http.MethodPost}, + } + + paths := req.Header()["Pattern"] + if len(paths) == 0 { + paths = allowedPublicEndpoints[:] + } + for _, m := range []string{"Origin", "Grpcgateway-Origin", "Grpcgateway-Referer"} { + origins := req.Header().Values(m) + if len(origins) == 0 { + continue + } + for _, o := range origins { + if strings.HasSuffix(o, ":443") { + o = "https://" + strings.TrimPrefix(strings.TrimSuffix(o, ":443"), "https://") + } else { + o = strings.TrimSuffix(o, ":80") + } + for _, u := range paths { + ri.u = append(ri.u, normalizeURL(o, &url.URL{Path: u})) + } + } + } + // Interceptor Logic // Allow health checks and other public routes to pass through if slices.ContainsFunc(a.publicRoutes, a.isPublicRoute(req.Spec().Procedure)) { //nolint:contextcheck // There is no way to pass a context here @@ -289,10 +318,7 @@ func (a Authentication) ConnectUnaryServerInterceptor() connect.UnaryInterceptor token, newCtx, err := a.checkToken( ctx, header, - receiverInfo{ - u: req.Spec().Procedure, - m: http.MethodPost, - }, + ri, req.Header()["Dpop"], ) if err != nil { @@ -515,7 +541,7 @@ func (a Authentication) validateDPoP(accessToken jwt.Token, acessTokenRaw string return nil, fmt.Errorf("`htm` claim missing in DPoP JWT") } - if htm != dpopInfo.m { + if !slices.Contains(dpopInfo.m, htm.(string)) { return nil, fmt.Errorf("incorrect `htm` claim in DPoP JWT; received [%v], but should match [%v]", htm, dpopInfo.m) } @@ -524,7 +550,7 @@ func (a Authentication) validateDPoP(accessToken jwt.Token, acessTokenRaw string return nil, fmt.Errorf("`htu` claim missing in DPoP JWT") } - if htu != dpopInfo.u { + if !slices.Contains(dpopInfo.u, htu.(string)) { return nil, fmt.Errorf("incorrect `htu` claim in DPoP JWT; received [%v], but should match [%v]", htu, dpopInfo.u) } diff --git a/service/internal/auth/authn_test.go b/service/internal/auth/authn_test.go index 841458cf7..99d572c60 100644 --- a/service/internal/auth/authn_test.go +++ b/service/internal/auth/authn_test.go @@ -408,8 +408,8 @@ func (s *AuthSuite) TestInvalid_DPoP_Cases() { context.Background(), []string{fmt.Sprintf("DPoP %s", string(testCase.accessToken))}, receiverInfo{ - u: "/a/path", - m: http.MethodPost, + u: []string{"/a/path"}, + m: []string{http.MethodPost}, }, []string{dpopToken}, ) diff --git a/service/internal/server/server.go b/service/internal/server/server.go index b1aa79032..d12e90f0a 100644 --- a/service/internal/server/server.go +++ b/service/internal/server/server.go @@ -9,6 +9,7 @@ import ( "net" "net/http" "net/http/pprof" + "net/textproto" "regexp" "strings" "time" @@ -28,6 +29,7 @@ import ( "golang.org/x/net/http2/h2c" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/metadata" ) const ( @@ -165,7 +167,31 @@ func NewOpenTDFServer(config Config, logger *logger.Logger) (*OpenTDFServer, err } // GRPC Gateway Mux - grpcGatewayMux := runtime.NewServeMux() + grpcGatewayMux := runtime.NewServeMux( + runtime.WithIncomingHeaderMatcher( + func(key string) (string, bool) { + if k, ok := runtime.DefaultHeaderMatcher(key); ok { + return k, true + } + if textproto.CanonicalMIMEHeaderKey(key) == "Dpop" { + logger.Info("DPOP KEY FOUUUUND") + return "Dpop", true + } + return "", false + }, + ), + runtime.WithMetadata(func(ctx context.Context, r *http.Request) metadata.MD { + md := make(map[string]string) + if method, ok := runtime.RPCMethod(ctx); ok { + md["method"] = method // /grpc.gateway.examples.internal.proto.examplepb.LoginService/Login + } + if pattern, ok := runtime.HTTPPathPattern(ctx); ok { + md["pattern"] = pattern // /v1/example/login + } + logger.Info("DPOP METADATATTATA", slog.Any("metadata", md)) + return metadata.New(md) + }), + ) // Create http server httpServer, err := newHTTPServer(config, connectRPC.Mux, grpcGatewayMux, authN, logger) From bc486f6917b96c1fffc529e393160bf9193c1277 Mon Sep 17 00:00:00 2001 From: David Mihalcik Date: Thu, 14 Nov 2024 16:15:07 -0500 Subject: [PATCH 2/3] Update server.go --- service/internal/server/server.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/service/internal/server/server.go b/service/internal/server/server.go index d12e90f0a..783c11eea 100644 --- a/service/internal/server/server.go +++ b/service/internal/server/server.go @@ -174,7 +174,6 @@ func NewOpenTDFServer(config Config, logger *logger.Logger) (*OpenTDFServer, err return k, true } if textproto.CanonicalMIMEHeaderKey(key) == "Dpop" { - logger.Info("DPOP KEY FOUUUUND") return "Dpop", true } return "", false @@ -188,7 +187,6 @@ func NewOpenTDFServer(config Config, logger *logger.Logger) (*OpenTDFServer, err if pattern, ok := runtime.HTTPPathPattern(ctx); ok { md["pattern"] = pattern // /v1/example/login } - logger.Info("DPOP METADATATTATA", slog.Any("metadata", md)) return metadata.New(md) }), ) From b8e143afc6538ba1c9183737a74f679a6409648d Mon Sep 17 00:00:00 2001 From: David Mihalcik Date: Thu, 14 Nov 2024 16:27:17 -0500 Subject: [PATCH 3/3] lint fixes --- service/internal/auth/authn.go | 16 ++++++++++++---- service/internal/server/server.go | 2 +- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/service/internal/auth/authn.go b/service/internal/auth/authn.go index ba32698ac..1b922df47 100644 --- a/service/internal/auth/authn.go +++ b/service/internal/auth/authn.go @@ -536,21 +536,29 @@ func (a Authentication) validateDPoP(accessToken jwt.Token, acessTokenRaw string return nil, fmt.Errorf("the DPoP JWT has expired") } - htm, ok := dpopToken.Get("htm") + htma, ok := dpopToken.Get("htm") if !ok { return nil, fmt.Errorf("`htm` claim missing in DPoP JWT") } + htm, ok := htma.(string) + if !ok { + return nil, fmt.Errorf("`htm` claim invalid format in DPoP JWT") + } - if !slices.Contains(dpopInfo.m, htm.(string)) { + if !slices.Contains(dpopInfo.m, htm) { return nil, fmt.Errorf("incorrect `htm` claim in DPoP JWT; received [%v], but should match [%v]", htm, dpopInfo.m) } - htu, ok := dpopToken.Get("htu") + htua, ok := dpopToken.Get("htu") if !ok { return nil, fmt.Errorf("`htu` claim missing in DPoP JWT") } + htu, ok := htua.(string) + if !ok { + return nil, fmt.Errorf("`htu` claim invalid format in DPoP JWT") + } - if !slices.Contains(dpopInfo.u, htu.(string)) { + if !slices.Contains(dpopInfo.u, htu) { return nil, fmt.Errorf("incorrect `htu` claim in DPoP JWT; received [%v], but should match [%v]", htu, dpopInfo.u) } diff --git a/service/internal/server/server.go b/service/internal/server/server.go index 783c11eea..e2f8ef62d 100644 --- a/service/internal/server/server.go +++ b/service/internal/server/server.go @@ -179,7 +179,7 @@ func NewOpenTDFServer(config Config, logger *logger.Logger) (*OpenTDFServer, err return "", false }, ), - runtime.WithMetadata(func(ctx context.Context, r *http.Request) metadata.MD { + runtime.WithMetadata(func(ctx context.Context, _ *http.Request) metadata.MD { md := make(map[string]string) if method, ok := runtime.RPCMethod(ctx); ok { md["method"] = method // /grpc.gateway.examples.internal.proto.examplepb.LoginService/Login