From 42a5c4d3bf7166b5e9b3424be8843d2287f2924f Mon Sep 17 00:00:00 2001 From: Kenneth Jenkins <51246568+kenjenkins@users.noreply.github.com> Date: Wed, 16 Apr 2025 16:40:03 -0700 Subject: [PATCH] authorize: refactor logAuthorizeCheck() 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 | 41 ++++++ 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, 323 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 701113fed..20250b211 100644 --- a/authorize/authorize.go +++ b/authorize/authorize.go @@ -41,7 +41,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 79fc5d0d5..e6b0a2a24 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" @@ -158,7 +159,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") @@ -226,7 +227,7 @@ func (a *Authorize) requireLoginResponse( } // always assume https scheme - checkRequestURL := getCheckRequestURL(in) + checkRequestURL := checkrequest.GetURL(in) checkRequestURL.Scheme = "https" var signInURLQuery url.Values @@ -259,7 +260,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..89776435a --- /dev/null +++ b/authorize/checkrequest/checkrequest.go @@ -0,0 +1,41 @@ +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 a [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 converts the HTTP headers from a [CheckRequest] to a Go map. +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 94c172b30..4e226e5ec 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" @@ -84,7 +81,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 } @@ -142,18 +139,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 @@ -174,7 +163,7 @@ func (a *Authorize) getMatchingPolicy(routeID uint64) *config.Policy { 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, @@ -197,57 +186,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()))