From 2e7d1c7f12669e8edb1f998d9b76c2bc6e54acaa Mon Sep 17 00:00:00 2001 From: Kenneth Jenkins <51246568+kenjenkins@users.noreply.github.com> Date: Wed, 23 Apr 2025 09:21:52 -0700 Subject: [PATCH] authorize: refactor logAuthorizeCheck() (#5576) Currently, policy evaluation and authorize logging are coupled to the Envoy CheckRequest proto message (part of the ext_authz API). In the context of ssh proxy authentication, we won't have a CheckRequest. Instead, let's make the existing evaluator.Request type the source of truth for the authorize log fields. This way, whether we populate the evaluator.Request struct from an ext_authz request or from an ssh proxy request, we can use the same logAuthorizeCheck() method for logging. Add some additional fields to evaluator.RequestHTTP for the authorize log fields that are not currently represented in this struct. --- authorize/authorize.go | 2 +- authorize/check_response.go | 7 +- authorize/checkrequest/checkrequest.go | 44 ++++++ authorize/checkrequest/checkrequest_test.go | 55 ++++++++ authorize/evaluator/evaluator.go | 69 +++++++-- authorize/evaluator/evaluator_test.go | 130 +++++++++++++++-- authorize/grpc.go | 73 +--------- authorize/grpc_test.go | 146 ++++---------------- authorize/log.go | 25 ++-- authorize/log_test.go | 33 ++--- 10 files changed, 326 insertions(+), 258 deletions(-) create mode 100644 authorize/checkrequest/checkrequest.go create mode 100644 authorize/checkrequest/checkrequest_test.go diff --git a/authorize/authorize.go b/authorize/authorize.go index 059494400..aaab2343b 100644 --- a/authorize/authorize.go +++ b/authorize/authorize.go @@ -38,7 +38,7 @@ func New(ctx context.Context, cfg *config.Config) (*Authorize, error) { tracerProvider := trace.NewTracerProvider(ctx, "Authorize") tracer := tracerProvider.Tracer(trace.PomeriumCoreTracer) a := &Authorize{ - currentConfig: atomicutil.NewValue(&config.Config{Options: new(config.Options)}), + currentConfig: atomicutil.NewValue(cfg), store: store.New(), tracerProvider: tracerProvider, tracer: tracer, diff --git a/authorize/check_response.go b/authorize/check_response.go index f201f7cdd..2d91b0230 100644 --- a/authorize/check_response.go +++ b/authorize/check_response.go @@ -19,6 +19,7 @@ import ( "google.golang.org/genproto/googleapis/rpc/status" "google.golang.org/grpc/codes" + "github.com/pomerium/pomerium/authorize/checkrequest" "github.com/pomerium/pomerium/authorize/evaluator" "github.com/pomerium/pomerium/internal/httputil" "github.com/pomerium/pomerium/internal/log" @@ -161,7 +162,7 @@ func (a *Authorize) deniedResponse( "code": code, // http code }) headers.Set("Content-Type", "application/json") - case getCheckRequestURL(in).Path == "/robots.txt": + case checkrequest.GetURL(in).Path == "/robots.txt": code = 200 respBody = []byte("User-agent: *\nDisallow: /") headers.Set("Content-Type", "text/plain") @@ -229,7 +230,7 @@ func (a *Authorize) requireLoginResponse( } // always assume https scheme - checkRequestURL := getCheckRequestURL(in) + checkRequestURL := checkrequest.GetURL(in) checkRequestURL.Scheme = "https" var signInURLQuery url.Values @@ -262,7 +263,7 @@ func (a *Authorize) requireWebAuthnResponse( state := a.state.Load() // always assume https scheme - checkRequestURL := getCheckRequestURL(in) + checkRequestURL := checkrequest.GetURL(in) checkRequestURL.Scheme = "https" // If we're already on a webauthn route, return OK. diff --git a/authorize/checkrequest/checkrequest.go b/authorize/checkrequest/checkrequest.go new file mode 100644 index 000000000..850c5bfb9 --- /dev/null +++ b/authorize/checkrequest/checkrequest.go @@ -0,0 +1,44 @@ +// Package checkrequest contains helper functions for working with Envoy +// ext_authz CheckRequest messages. +package checkrequest + +import ( + "net/url" + "strings" + + envoy_service_auth_v3 "github.com/envoyproxy/go-control-plane/envoy/service/auth/v3" + + "github.com/pomerium/pomerium/internal/httputil" + "github.com/pomerium/pomerium/internal/urlutil" +) + +// GetURL converts the request URL from an ext_authz CheckRequest to a [url.URL]. +func GetURL(req *envoy_service_auth_v3.CheckRequest) url.URL { + h := req.GetAttributes().GetRequest().GetHttp() + u := url.URL{ + Scheme: h.GetScheme(), + Host: h.GetHost(), + } + u.Host = urlutil.GetDomainsForURL(&u, false)[0] + // envoy sends the query string as part of the path + path := h.GetPath() + if idx := strings.Index(path, "?"); idx != -1 { + u.RawPath, u.RawQuery = path[:idx], path[idx+1:] + u.RawQuery = u.Query().Encode() + } else { + u.RawPath = path + } + u.Path, _ = url.PathUnescape(u.RawPath) + return u +} + +// GetHeaders returns the HTTP headers from an ext_authz CheckRequest, canonicalizing +// the header keys. +func GetHeaders(req *envoy_service_auth_v3.CheckRequest) map[string]string { + hdrs := make(map[string]string) + ch := req.GetAttributes().GetRequest().GetHttp().GetHeaders() + for k, v := range ch { + hdrs[httputil.CanonicalHeaderKey(k)] = v + } + return hdrs +} diff --git a/authorize/checkrequest/checkrequest_test.go b/authorize/checkrequest/checkrequest_test.go new file mode 100644 index 000000000..bbc9572e6 --- /dev/null +++ b/authorize/checkrequest/checkrequest_test.go @@ -0,0 +1,55 @@ +package checkrequest + +import ( + "net/url" + "testing" + + envoy_service_auth_v3 "github.com/envoyproxy/go-control-plane/envoy/service/auth/v3" + "github.com/stretchr/testify/assert" +) + +func TestGetURL(t *testing.T) { + req := &envoy_service_auth_v3.CheckRequest{ + Attributes: &envoy_service_auth_v3.AttributeContext{ + Request: &envoy_service_auth_v3.AttributeContext_Request{ + Http: &envoy_service_auth_v3.AttributeContext_HttpRequest{ + Host: "example.com:80", + Path: "/some/path?a=b", + Scheme: "http", + Method: "GET", + Headers: map[string]string{"X-Request-Id": "CHECK-REQUEST-ID"}, + }, + }, + }, + } + + assert.Equal(t, url.URL{ + Scheme: "http", + Host: "example.com", + Path: "/some/path", + RawPath: "/some/path", + RawQuery: "a=b", + }, GetURL(req)) +} + +func TestGetHeaders(t *testing.T) { + req := &envoy_service_auth_v3.CheckRequest{ + Attributes: &envoy_service_auth_v3.AttributeContext{ + Request: &envoy_service_auth_v3.AttributeContext_Request{ + Http: &envoy_service_auth_v3.AttributeContext_HttpRequest{ + Headers: map[string]string{ + "content-type": "application/www-x-form-urlencoded", + "x-request-id": "CHECK-REQUEST-ID", + ":authority": "example.com", + }, + }, + }, + }, + } + + assert.Equal(t, map[string]string{ + "Content-Type": "application/www-x-form-urlencoded", + "X-Request-Id": "CHECK-REQUEST-ID", + ":authority": "example.com", + }, GetHeaders(req)) +} diff --git a/authorize/evaluator/evaluator.go b/authorize/evaluator/evaluator.go index 491e01c95..e0d5b7026 100644 --- a/authorize/evaluator/evaluator.go +++ b/authorize/evaluator/evaluator.go @@ -4,16 +4,21 @@ package evaluator import ( "context" "encoding/base64" + "encoding/pem" "fmt" "net/http" "net/url" + "strings" "time" + envoy_service_auth_v3 "github.com/envoyproxy/go-control-plane/envoy/service/auth/v3" "github.com/go-jose/go-jose/v3" "github.com/hashicorp/go-set/v3" "github.com/open-policy-agent/opa/rego" "golang.org/x/sync/errgroup" + "google.golang.org/protobuf/types/known/structpb" + "github.com/pomerium/pomerium/authorize/checkrequest" "github.com/pomerium/pomerium/authorize/internal/store" "github.com/pomerium/pomerium/config" "github.com/pomerium/pomerium/internal/errgrouputil" @@ -36,30 +41,37 @@ type Request struct { // RequestHTTP is the HTTP field in the request. type RequestHTTP struct { Method string `json:"method"` + Host string `json:"host"` Hostname string `json:"hostname"` Path string `json:"path"` + RawPath string `json:"raw_path"` + RawQuery string `json:"raw_query"` URL string `json:"url"` Headers map[string]string `json:"headers"` ClientCertificate ClientCertificateInfo `json:"client_certificate"` IP string `json:"ip"` } -// NewRequestHTTP creates a new RequestHTTP. -func NewRequestHTTP( - method string, - requestURL url.URL, - headers map[string]string, - clientCertificate ClientCertificateInfo, - ip string, +// RequestHTTPFromCheckRequest populates a RequestHTTP from an Envoy CheckRequest proto. +func RequestHTTPFromCheckRequest( + ctx context.Context, + in *envoy_service_auth_v3.CheckRequest, ) RequestHTTP { + requestURL := checkrequest.GetURL(in) + rawPath, rawQuery, _ := strings.Cut(in.GetAttributes().GetRequest().GetHttp().GetPath(), "?") + attrs := in.GetAttributes() + clientCertMetadata := attrs.GetMetadataContext().GetFilterMetadata()["com.pomerium.client-certificate-info"] return RequestHTTP{ - Method: method, + Method: attrs.GetRequest().GetHttp().GetMethod(), + Host: attrs.GetRequest().GetHttp().GetHost(), Hostname: requestURL.Hostname(), Path: requestURL.Path, + RawPath: rawPath, + RawQuery: rawQuery, URL: requestURL.String(), - Headers: headers, - ClientCertificate: clientCertificate, - IP: ip, + Headers: checkrequest.GetHeaders(in), + ClientCertificate: getClientCertificateInfo(ctx, clientCertMetadata), + IP: attrs.GetSource().GetAddress().GetSocketAddress().GetAddress(), } } @@ -77,6 +89,41 @@ type ClientCertificateInfo struct { Intermediates string `json:"intermediates,omitempty"` } +// getClientCertificateInfo translates from the client certificate Envoy +// metadata to the ClientCertificateInfo type. +func getClientCertificateInfo( + ctx context.Context, metadata *structpb.Struct, +) ClientCertificateInfo { + var c ClientCertificateInfo + if metadata == nil { + return c + } + c.Presented = metadata.Fields["presented"].GetBoolValue() + escapedChain := metadata.Fields["chain"].GetStringValue() + if escapedChain == "" { + // No validated client certificate. + return c + } + + chain, err := url.QueryUnescape(escapedChain) + if err != nil { + log.Ctx(ctx).Error().Str("chain", escapedChain).Err(err). + Msg(`received unexpected client certificate "chain" value`) + return c + } + + // Split the chain into the leaf and any intermediate certificates. + p, rest := pem.Decode([]byte(chain)) + if p == nil { + log.Ctx(ctx).Error().Str("chain", escapedChain). + Msg(`received unexpected client certificate "chain" value (no PEM block found)`) + return c + } + c.Leaf = string(pem.EncodeToMemory(p)) + c.Intermediates = string(rest) + return c +} + // RequestSession is the session field in the request. type RequestSession struct { ID string `json:"id"` diff --git a/authorize/evaluator/evaluator_test.go b/authorize/evaluator/evaluator_test.go index 4859aad9d..565fc0d90 100644 --- a/authorize/evaluator/evaluator_test.go +++ b/authorize/evaluator/evaluator_test.go @@ -10,10 +10,12 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/structpb" "github.com/pomerium/pomerium/authorize/internal/store" "github.com/pomerium/pomerium/config" "github.com/pomerium/pomerium/internal/httputil" + "github.com/pomerium/pomerium/internal/testutil" "github.com/pomerium/pomerium/pkg/cryptutil" "github.com/pomerium/pomerium/pkg/grpc/session" "github.com/pomerium/pomerium/pkg/grpc/user" @@ -22,6 +24,113 @@ import ( "github.com/pomerium/pomerium/pkg/storage" ) +func Test_getClientCertificateInfo(t *testing.T) { + const leafPEM = `-----BEGIN CERTIFICATE----- +MIIBZTCCAQugAwIBAgICEAEwCgYIKoZIzj0EAwIwGjEYMBYGA1UEAxMPSW50ZXJt +ZWRpYXRlIENBMCIYDzAwMDEwMTAxMDAwMDAwWhgPMDAwMTAxMDEwMDAwMDBaMB8x +HTAbBgNVBAMTFENsaWVudCBjZXJ0aWZpY2F0ZSAxMFkwEwYHKoZIzj0CAQYIKoZI +zj0DAQcDQgAESly1cwEbcxaJBl6qAhrX1k7vejTFNE2dEbrTMpUYMl86GEWdsDYN +KSa/1wZCowPy82gPGjfAU90odkqJOusCQqM4MDYwEwYDVR0lBAwwCgYIKwYBBQUH +AwIwHwYDVR0jBBgwFoAU6Qb7nEl2XHKpf/QLL6PENsHFqbowCgYIKoZIzj0EAwID +SAAwRQIgXREMUz81pYwJCMLGcV0ApaXIUap1V5n1N4VhyAGxGLYCIQC8p/LwoSgu +71H3/nCi5MxsECsvVtsmHIfwXt0wulQ1TA== +-----END CERTIFICATE----- +` + const intermediatePEM = `-----BEGIN CERTIFICATE----- +MIIBYzCCAQigAwIBAgICEAEwCgYIKoZIzj0EAwIwEjEQMA4GA1UEAxMHUm9vdCBD +QTAiGA8wMDAxMDEwMTAwMDAwMFoYDzAwMDEwMTAxMDAwMDAwWjAaMRgwFgYDVQQD +Ew9JbnRlcm1lZGlhdGUgQ0EwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATYaTr9 +uH4LpEp541/2SlKrdQZwNns+NHY/ftm++NhMDUn+izzNbPZ5aPT6VBs4Q6vbgfkK +kDaBpaKzb+uOT+o1o0IwQDAdBgNVHQ4EFgQU6Qb7nEl2XHKpf/QLL6PENsHFqbow +HwYDVR0jBBgwFoAUiQ3r61y+vxDn6PMWZrpISr67HiQwCgYIKoZIzj0EAwIDSQAw +RgIhAMvdURs28uib2QwSMnqJjKasMb30yrSJvTiSU+lcg97/AiEA+6GpioM0c221 +n/XNKVYEkPmeXHRoz9ZuVDnSfXKJoHE= +-----END CERTIFICATE----- +` + const rootPEM = `-----BEGIN CERTIFICATE----- +MIIBNzCB36ADAgECAgIQADAKBggqhkjOPQQDAjASMRAwDgYDVQQDEwdSb290IENB +MCIYDzAwMDEwMTAxMDAwMDAwWhgPMDAwMTAxMDEwMDAwMDBaMBIxEDAOBgNVBAMT +B1Jvb3QgQ0EwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAS6q0mTvm29xasq7Lwk +aRGb2S/LkQFsAwaCXohSNvonCQHRMCRvA1IrQGk/oyBS5qrDoD9/7xkcVYHuTv5D +CbtuoyEwHzAdBgNVHQ4EFgQUiQ3r61y+vxDn6PMWZrpISr67HiQwCgYIKoZIzj0E +AwIDRwAwRAIgF1ux0ridbN+bo0E3TTcNY8Xfva7yquYRMmEkfbGvSb0CIDqK80B+ +fYCZHo3CID0gRSemaQ/jYMgyeBFrHIr6icZh +-----END CERTIFICATE----- +` + + cases := []struct { + label string + presented bool + chain string + expected ClientCertificateInfo + expectedLog string + }{ + { + "not presented", + false, + "", + ClientCertificateInfo{}, + "", + }, + { + "presented", + true, + url.QueryEscape(leafPEM), + ClientCertificateInfo{ + Presented: true, + Leaf: leafPEM, + }, + "", + }, + { + "presented with intermediates", + true, + url.QueryEscape(leafPEM + intermediatePEM + rootPEM), + ClientCertificateInfo{ + Presented: true, + Leaf: leafPEM, + Intermediates: intermediatePEM + rootPEM, + }, + "", + }, + { + "invalid chain URL encoding", + false, + "invalid%URL%encoding", + ClientCertificateInfo{}, + `{"chain":"invalid%URL%encoding","error":"invalid URL escape \"%UR\"","level":"error","message":"received unexpected client certificate \"chain\" value"}`, + }, + { + "invalid chain PEM encoding", + true, + "not valid PEM data", + ClientCertificateInfo{ + Presented: true, + }, + `{"chain":"not valid PEM data","level":"error","message":"received unexpected client certificate \"chain\" value (no PEM block found)"}`, + }, + } + + ctx := context.Background() + for i := range cases { + c := &cases[i] + t.Run(c.label, func(t *testing.T) { + metadata := &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "presented": structpb.NewBoolValue(c.presented), + "chain": structpb.NewStringValue(c.chain), + }, + } + var info ClientCertificateInfo + logOutput := testutil.CaptureLogs(t, func() { + info = getClientCertificateInfo(ctx, metadata) + }) + assert.Equal(t, c.expected, info) + assert.Contains(t, logOutput, c.expectedLog) + }) + } +} + func TestEvaluator(t *testing.T) { signingKey, err := cryptutil.NewSigningKey() require.NoError(t, err) @@ -527,13 +636,9 @@ func TestEvaluator(t *testing.T) { t.Run("http method", func(t *testing.T) { res, err := eval(t, options, []proto.Message{}, &Request{ Policy: policies[8], - HTTP: NewRequestHTTP( - http.MethodGet, - *mustParseURL("https://from.example.com/"), - nil, - ClientCertificateInfo{}, - "", - ), + HTTP: RequestHTTP{ + Method: http.MethodGet, + }, }) require.NoError(t, err) assert.True(t, res.Allow.Value) @@ -541,13 +646,10 @@ func TestEvaluator(t *testing.T) { t.Run("http path", func(t *testing.T) { res, err := eval(t, options, []proto.Message{}, &Request{ Policy: policies[9], - HTTP: NewRequestHTTP( - "POST", - *mustParseURL("https://from.example.com/test"), - nil, - ClientCertificateInfo{}, - "", - ), + HTTP: RequestHTTP{ + Method: "POST", + Path: "/test", + }, }) require.NoError(t, err) assert.True(t, res.Allow.Value) diff --git a/authorize/grpc.go b/authorize/grpc.go index cda2d0e5d..4c1ba653b 100644 --- a/authorize/grpc.go +++ b/authorize/grpc.go @@ -2,26 +2,23 @@ package authorize import ( "context" - "encoding/pem" "errors" "fmt" "io" "net/http" - "net/url" "strings" envoy_service_auth_v3 "github.com/envoyproxy/go-control-plane/envoy/service/auth/v3" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - "google.golang.org/protobuf/types/known/structpb" + "github.com/pomerium/pomerium/authorize/checkrequest" "github.com/pomerium/pomerium/authorize/evaluator" "github.com/pomerium/pomerium/config" "github.com/pomerium/pomerium/config/envoyconfig" "github.com/pomerium/pomerium/internal/httputil" "github.com/pomerium/pomerium/internal/log" "github.com/pomerium/pomerium/internal/sessions" - "github.com/pomerium/pomerium/internal/urlutil" "github.com/pomerium/pomerium/pkg/contextutil" "github.com/pomerium/pomerium/pkg/grpc/databroker" "github.com/pomerium/pomerium/pkg/grpc/user" @@ -80,7 +77,7 @@ func (a *Authorize) Check(ctx context.Context, in *envoy_service_auth_v3.CheckRe if err != nil { log.Ctx(ctx).Error().Err(err).Str("request-id", requestID).Msg("grpc check ext_authz_error") } - a.logAuthorizeCheck(ctx, in, res, s, u) + a.logAuthorizeCheck(ctx, req, res, s, u) return resp, err } @@ -138,18 +135,10 @@ func (a *Authorize) getEvaluatorRequestFromCheckRequest( ctx context.Context, in *envoy_service_auth_v3.CheckRequest, ) (*evaluator.Request, error) { - requestURL := getCheckRequestURL(in) attrs := in.GetAttributes() - clientCertMetadata := attrs.GetMetadataContext().GetFilterMetadata()["com.pomerium.client-certificate-info"] req := &evaluator.Request{ IsInternal: envoyconfig.ExtAuthzContextExtensionsIsInternal(attrs.GetContextExtensions()), - HTTP: evaluator.NewRequestHTTP( - attrs.GetRequest().GetHttp().GetMethod(), - requestURL, - getCheckRequestHeaders(in), - getClientCertificateInfo(ctx, clientCertMetadata), - attrs.GetSource().GetAddress().GetSocketAddress().GetAddress(), - ), + HTTP: evaluator.RequestHTTPFromCheckRequest(ctx, in), } req.Policy = a.getMatchingPolicy(envoyconfig.ExtAuthzContextExtensionsRouteID(attrs.GetContextExtensions())) return req, nil @@ -185,7 +174,7 @@ func (a *Authorize) withQuerierForCheckRequest(ctx context.Context) context.Cont func getHTTPRequestFromCheckRequest(req *envoy_service_auth_v3.CheckRequest) *http.Request { hattrs := req.GetAttributes().GetRequest().GetHttp() - u := getCheckRequestURL(req) + u := checkrequest.GetURL(req) hreq := &http.Request{ Method: hattrs.GetMethod(), URL: &u, @@ -208,57 +197,3 @@ func getCheckRequestHeaders(req *envoy_service_auth_v3.CheckRequest) map[string] } return hdrs } - -func getCheckRequestURL(req *envoy_service_auth_v3.CheckRequest) url.URL { - h := req.GetAttributes().GetRequest().GetHttp() - u := url.URL{ - Scheme: h.GetScheme(), - Host: h.GetHost(), - } - u.Host = urlutil.GetDomainsForURL(&u, false)[0] - // envoy sends the query string as part of the path - path := h.GetPath() - if idx := strings.Index(path, "?"); idx != -1 { - u.RawPath, u.RawQuery = path[:idx], path[idx+1:] - u.RawQuery = u.Query().Encode() - } else { - u.RawPath = path - } - u.Path, _ = url.PathUnescape(u.RawPath) - return u -} - -// getClientCertificateInfo translates from the client certificate Envoy -// metadata to the ClientCertificateInfo type. -func getClientCertificateInfo( - ctx context.Context, metadata *structpb.Struct, -) evaluator.ClientCertificateInfo { - var c evaluator.ClientCertificateInfo - if metadata == nil { - return c - } - c.Presented = metadata.Fields["presented"].GetBoolValue() - escapedChain := metadata.Fields["chain"].GetStringValue() - if escapedChain == "" { - // No validated client certificate. - return c - } - - chain, err := url.QueryUnescape(escapedChain) - if err != nil { - log.Ctx(ctx).Error().Str("chain", escapedChain).Err(err). - Msg(`received unexpected client certificate "chain" value`) - return c - } - - // Split the chain into the leaf and any intermediate certificates. - p, rest := pem.Decode([]byte(chain)) - if p == nil { - log.Ctx(ctx).Error().Str("chain", escapedChain). - Msg(`received unexpected client certificate "chain" value (no PEM block found)`) - return c - } - c.Leaf = string(pem.EncodeToMemory(p)) - c.Intermediates = string(rest) - return c -} diff --git a/authorize/grpc_test.go b/authorize/grpc_test.go index 0a8ed1026..561dff74a 100644 --- a/authorize/grpc_test.go +++ b/authorize/grpc_test.go @@ -18,7 +18,6 @@ import ( "github.com/pomerium/pomerium/authorize/evaluator" "github.com/pomerium/pomerium/config" "github.com/pomerium/pomerium/internal/atomicutil" - "github.com/pomerium/pomerium/internal/testutil" "github.com/pomerium/pomerium/pkg/grpc/databroker" "github.com/pomerium/pomerium/pkg/storage" ) @@ -92,20 +91,25 @@ func Test_getEvaluatorRequest(t *testing.T) { require.NoError(t, err) expect := &evaluator.Request{ Policy: &a.currentConfig.Load().Options.Policies[0], - HTTP: evaluator.NewRequestHTTP( - http.MethodGet, - mustParseURL("http://example.com/some/path?qs=1"), - map[string]string{ + HTTP: evaluator.RequestHTTP{ + Method: http.MethodGet, + Host: "example.com", + Hostname: "example.com", + Path: "/some/path", + RawPath: "/some/path", + RawQuery: "qs=1", + URL: "http://example.com/some/path?qs=1", + Headers: map[string]string{ "Accept": "text/html", "X-Forwarded-Proto": "https", }, - evaluator.ClientCertificateInfo{ + ClientCertificate: evaluator.ClientCertificateInfo{ Presented: true, Leaf: certPEM[1:] + "\n", Intermediates: "", }, - "", - ), + IP: "", + }, } assert.Equal(t, expect, actual) } @@ -145,127 +149,25 @@ func Test_getEvaluatorRequestWithPortInHostHeader(t *testing.T) { expect := &evaluator.Request{ Policy: &a.currentConfig.Load().Options.Policies[0], Session: evaluator.RequestSession{}, - HTTP: evaluator.NewRequestHTTP( - http.MethodGet, - mustParseURL("http://example.com/some/path?qs=1"), - map[string]string{ + HTTP: evaluator.RequestHTTP{ + Method: http.MethodGet, + Host: "example.com:80", + Hostname: "example.com", + Path: "/some/path", + RawPath: "/some/path", + RawQuery: "qs=1", + URL: "http://example.com/some/path?qs=1", + Headers: map[string]string{ "Accept": "text/html", "X-Forwarded-Proto": "https", }, - evaluator.ClientCertificateInfo{}, - "", - ), + ClientCertificate: evaluator.ClientCertificateInfo{}, + IP: "", + }, } assert.Equal(t, expect, actual) } -func Test_getClientCertificateInfo(t *testing.T) { - const leafPEM = `-----BEGIN CERTIFICATE----- -MIIBZTCCAQugAwIBAgICEAEwCgYIKoZIzj0EAwIwGjEYMBYGA1UEAxMPSW50ZXJt -ZWRpYXRlIENBMCIYDzAwMDEwMTAxMDAwMDAwWhgPMDAwMTAxMDEwMDAwMDBaMB8x -HTAbBgNVBAMTFENsaWVudCBjZXJ0aWZpY2F0ZSAxMFkwEwYHKoZIzj0CAQYIKoZI -zj0DAQcDQgAESly1cwEbcxaJBl6qAhrX1k7vejTFNE2dEbrTMpUYMl86GEWdsDYN -KSa/1wZCowPy82gPGjfAU90odkqJOusCQqM4MDYwEwYDVR0lBAwwCgYIKwYBBQUH -AwIwHwYDVR0jBBgwFoAU6Qb7nEl2XHKpf/QLL6PENsHFqbowCgYIKoZIzj0EAwID -SAAwRQIgXREMUz81pYwJCMLGcV0ApaXIUap1V5n1N4VhyAGxGLYCIQC8p/LwoSgu -71H3/nCi5MxsECsvVtsmHIfwXt0wulQ1TA== ------END CERTIFICATE----- -` - const intermediatePEM = `-----BEGIN CERTIFICATE----- -MIIBYzCCAQigAwIBAgICEAEwCgYIKoZIzj0EAwIwEjEQMA4GA1UEAxMHUm9vdCBD -QTAiGA8wMDAxMDEwMTAwMDAwMFoYDzAwMDEwMTAxMDAwMDAwWjAaMRgwFgYDVQQD -Ew9JbnRlcm1lZGlhdGUgQ0EwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATYaTr9 -uH4LpEp541/2SlKrdQZwNns+NHY/ftm++NhMDUn+izzNbPZ5aPT6VBs4Q6vbgfkK -kDaBpaKzb+uOT+o1o0IwQDAdBgNVHQ4EFgQU6Qb7nEl2XHKpf/QLL6PENsHFqbow -HwYDVR0jBBgwFoAUiQ3r61y+vxDn6PMWZrpISr67HiQwCgYIKoZIzj0EAwIDSQAw -RgIhAMvdURs28uib2QwSMnqJjKasMb30yrSJvTiSU+lcg97/AiEA+6GpioM0c221 -n/XNKVYEkPmeXHRoz9ZuVDnSfXKJoHE= ------END CERTIFICATE----- -` - const rootPEM = `-----BEGIN CERTIFICATE----- -MIIBNzCB36ADAgECAgIQADAKBggqhkjOPQQDAjASMRAwDgYDVQQDEwdSb290IENB -MCIYDzAwMDEwMTAxMDAwMDAwWhgPMDAwMTAxMDEwMDAwMDBaMBIxEDAOBgNVBAMT -B1Jvb3QgQ0EwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAS6q0mTvm29xasq7Lwk -aRGb2S/LkQFsAwaCXohSNvonCQHRMCRvA1IrQGk/oyBS5qrDoD9/7xkcVYHuTv5D -CbtuoyEwHzAdBgNVHQ4EFgQUiQ3r61y+vxDn6PMWZrpISr67HiQwCgYIKoZIzj0E -AwIDRwAwRAIgF1ux0ridbN+bo0E3TTcNY8Xfva7yquYRMmEkfbGvSb0CIDqK80B+ -fYCZHo3CID0gRSemaQ/jYMgyeBFrHIr6icZh ------END CERTIFICATE----- -` - - cases := []struct { - label string - presented bool - chain string - expected evaluator.ClientCertificateInfo - expectedLog string - }{ - { - "not presented", - false, - "", - evaluator.ClientCertificateInfo{}, - "", - }, - { - "presented", - true, - url.QueryEscape(leafPEM), - evaluator.ClientCertificateInfo{ - Presented: true, - Leaf: leafPEM, - }, - "", - }, - { - "presented with intermediates", - true, - url.QueryEscape(leafPEM + intermediatePEM + rootPEM), - evaluator.ClientCertificateInfo{ - Presented: true, - Leaf: leafPEM, - Intermediates: intermediatePEM + rootPEM, - }, - "", - }, - { - "invalid chain URL encoding", - false, - "invalid%URL%encoding", - evaluator.ClientCertificateInfo{}, - `{"chain":"invalid%URL%encoding","error":"invalid URL escape \"%UR\"","level":"error","message":"received unexpected client certificate \"chain\" value"}`, - }, - { - "invalid chain PEM encoding", - true, - "not valid PEM data", - evaluator.ClientCertificateInfo{ - Presented: true, - }, - `{"chain":"not valid PEM data","level":"error","message":"received unexpected client certificate \"chain\" value (no PEM block found)"}`, - }, - } - - ctx := context.Background() - for i := range cases { - c := &cases[i] - t.Run(c.label, func(t *testing.T) { - metadata := &structpb.Struct{ - Fields: map[string]*structpb.Value{ - "presented": structpb.NewBoolValue(c.presented), - "chain": structpb.NewStringValue(c.chain), - }, - } - var info evaluator.ClientCertificateInfo - logOutput := testutil.CaptureLogs(t, func() { - info = getClientCertificateInfo(ctx, metadata) - }) - assert.Equal(t, c.expected, info) - assert.Contains(t, logOutput, c.expectedLog) - }) - } -} - type mockDataBrokerServiceClient struct { databroker.DataBrokerServiceClient diff --git a/authorize/log.go b/authorize/log.go index c59cf38ce..b8c44d68e 100644 --- a/authorize/log.go +++ b/authorize/log.go @@ -2,9 +2,7 @@ package authorize import ( "context" - "strings" - envoy_service_auth_v3 "github.com/envoyproxy/go-control-plane/envoy/service/auth/v3" "github.com/go-jose/go-jose/v3/jwt" "github.com/rs/zerolog" "go.opentelemetry.io/otel/attribute" @@ -21,19 +19,19 @@ import ( func (a *Authorize) logAuthorizeCheck( ctx context.Context, - in *envoy_service_auth_v3.CheckRequest, + req *evaluator.Request, res *evaluator.Result, s sessionOrServiceAccount, u *user.User, ) { ctx, span := a.tracer.Start(ctx, "authorize.grpc.LogAuthorizeCheck") defer span.End() - hdrs := getCheckRequestHeaders(in) + hdrs := req.HTTP.Headers impersonateDetails := a.getImpersonateDetails(ctx, s) evt := log.Ctx(ctx).Info().Str("service", "authorize") fields := a.currentConfig.Load().Options.GetAuthorizeLogFields() for _, field := range fields { - evt = populateLogEvent(ctx, field, evt, in, s, u, hdrs, impersonateDetails, res) + evt = populateLogEvent(ctx, field, evt, req, s, u, impersonateDetails, res) } evt = log.HTTPHeaders(evt, fields, hdrs) @@ -134,22 +132,19 @@ func populateLogEvent( ctx context.Context, field log.AuthorizeLogField, evt *zerolog.Event, - in *envoy_service_auth_v3.CheckRequest, + req *evaluator.Request, s sessionOrServiceAccount, u *user.User, - hdrs map[string]string, impersonateDetails *impersonateDetails, res *evaluator.Result, ) *zerolog.Event { - path, query, _ := strings.Cut(in.GetAttributes().GetRequest().GetHttp().GetPath(), "?") - switch field { case log.AuthorizeLogFieldCheckRequestID: - return evt.Str(string(field), hdrs["X-Request-Id"]) + return evt.Str(string(field), req.HTTP.Headers["X-Request-Id"]) case log.AuthorizeLogFieldEmail: return evt.Str(string(field), u.GetEmail()) case log.AuthorizeLogFieldHost: - return evt.Str(string(field), in.GetAttributes().GetRequest().GetHttp().GetHost()) + return evt.Str(string(field), req.HTTP.Host) case log.AuthorizeLogFieldIDToken: if s, ok := s.(*session.Session); ok { evt = evt.Str(string(field), s.GetIdToken().GetRaw()) @@ -180,13 +175,13 @@ func populateLogEvent( } return evt case log.AuthorizeLogFieldIP: - return evt.Str(string(field), in.GetAttributes().GetSource().GetAddress().GetSocketAddress().GetAddress()) + return evt.Str(string(field), req.HTTP.IP) case log.AuthorizeLogFieldMethod: - return evt.Str(string(field), in.GetAttributes().GetRequest().GetHttp().GetMethod()) + return evt.Str(string(field), req.HTTP.Method) case log.AuthorizeLogFieldPath: - return evt.Str(string(field), path) + return evt.Str(string(field), req.HTTP.RawPath) case log.AuthorizeLogFieldQuery: - return evt.Str(string(field), query) + return evt.Str(string(field), req.HTTP.RawQuery) case log.AuthorizeLogFieldRequestID: return evt.Str(string(field), requestid.FromContext(ctx)) case log.AuthorizeLogFieldServiceAccountID: diff --git a/authorize/log_test.go b/authorize/log_test.go index c2e41bb92..27105563d 100644 --- a/authorize/log_test.go +++ b/authorize/log_test.go @@ -6,8 +6,6 @@ import ( "strings" "testing" - envoy_config_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" - envoy_service_auth_v3 "github.com/envoyproxy/go-control-plane/envoy/service/auth/v3" "github.com/rs/zerolog" "github.com/stretchr/testify/assert" @@ -24,27 +22,16 @@ func Test_populateLogEvent(t *testing.T) { ctx := context.Background() ctx = requestid.WithValue(ctx, "REQUEST-ID") - checkRequest := &envoy_service_auth_v3.CheckRequest{ - Attributes: &envoy_service_auth_v3.AttributeContext{ - Request: &envoy_service_auth_v3.AttributeContext_Request{ - Http: &envoy_service_auth_v3.AttributeContext_HttpRequest{ - Host: "HOST", - Path: "https://www.example.com/some/path?a=b", - Method: "GET", - }, - }, - Source: &envoy_service_auth_v3.AttributeContext_Peer{ - Address: &envoy_config_core_v3.Address{ - Address: &envoy_config_core_v3.Address_SocketAddress{ - SocketAddress: &envoy_config_core_v3.SocketAddress{ - Address: "127.0.0.1", - }, - }, - }, - }, + req := &evaluator.Request{ + HTTP: evaluator.RequestHTTP{ + Method: "GET", + Host: "HOST", + RawPath: "/some/path", + RawQuery: "a=b", + Headers: map[string]string{"X-Request-Id": "CHECK-REQUEST-ID"}, + IP: "127.0.0.1", }, } - headers := map[string]string{"X-Request-Id": "CHECK-REQUEST-ID"} s := &session.Session{ Id: "SESSION-ID", IdToken: &session.IDToken{ @@ -86,7 +73,7 @@ func Test_populateLogEvent(t *testing.T) { {log.AuthorizeLogFieldImpersonateUserID, s, `{"impersonate-user-id":"IMPERSONATE-USER-ID"}`}, {log.AuthorizeLogFieldIP, s, `{"ip":"127.0.0.1"}`}, {log.AuthorizeLogFieldMethod, s, `{"method":"GET"}`}, - {log.AuthorizeLogFieldPath, s, `{"path":"https://www.example.com/some/path"}`}, + {log.AuthorizeLogFieldPath, s, `{"path":"/some/path"}`}, {log.AuthorizeLogFieldQuery, s, `{"query":"a=b"}`}, {log.AuthorizeLogFieldRemovedGroupsCount, s, `{"removed-groups-count":42}`}, {log.AuthorizeLogFieldRequestID, s, `{"request-id":"REQUEST-ID"}`}, @@ -102,7 +89,7 @@ func Test_populateLogEvent(t *testing.T) { var buf bytes.Buffer log := zerolog.New(&buf) evt := log.Log() - evt = populateLogEvent(ctx, tc.field, evt, checkRequest, tc.s, u, headers, impersonateDetails, res) + evt = populateLogEvent(ctx, tc.field, evt, req, tc.s, u, impersonateDetails, res) evt.Send() assert.Equal(t, tc.expect, strings.TrimSpace(buf.String()))