refactor to share more authorize check logic

should restore authorize log entries for ssh auth
This commit is contained in:
Kenneth Jenkins 2025-02-27 13:04:44 -08:00
parent 3e6f4464af
commit 1da95d334c
7 changed files with 101 additions and 76 deletions

View file

@ -12,6 +12,8 @@ import (
"github.com/rs/zerolog"
oteltrace "go.opentelemetry.io/otel/trace"
"golang.org/x/sync/errgroup"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"github.com/pomerium/datasource/pkg/directory"
"github.com/pomerium/pomerium/authorize/evaluator"
@ -19,11 +21,16 @@ import (
"github.com/pomerium/pomerium/config"
"github.com/pomerium/pomerium/internal/atomicutil"
"github.com/pomerium/pomerium/internal/log"
"github.com/pomerium/pomerium/internal/sessions"
"github.com/pomerium/pomerium/internal/telemetry/metrics"
"github.com/pomerium/pomerium/internal/telemetry/trace"
"github.com/pomerium/pomerium/pkg/contextutil"
"github.com/pomerium/pomerium/pkg/cryptutil"
"github.com/pomerium/pomerium/pkg/grpc/databroker"
"github.com/pomerium/pomerium/pkg/grpc/user"
"github.com/pomerium/pomerium/pkg/policy/criteria"
"github.com/pomerium/pomerium/pkg/storage"
"github.com/pomerium/pomerium/pkg/telemetry/requestid"
)
// Authorize struct holds
@ -180,3 +187,66 @@ func (a *Authorize) OnConfigChange(ctx context.Context, cfg *config.Config) {
}
}
}
type evaluateResult struct {
// Overall allow/deny result.
Allowed bool
// Reasons for the overall result.
Reasons criteria.Reasons
// Reason detail traces. (Populated only if enabled by the policy.)
Traces []contextutil.PolicyEvaluationTrace
}
func (a *Authorize) evaluate(
ctx context.Context,
req *evaluator.Request,
sessionState *sessions.State,
) (*evaluator.Result, error) {
querier := storage.NewCachingQuerier(
storage.NewQuerier(a.state.Load().dataBrokerClient),
a.globalCache,
)
ctx = storage.WithQuerier(ctx, querier)
requestID := requestid.FromContext(ctx)
state := a.state.Load()
var s sessionOrServiceAccount
var u *user.User
var err error
if sessionState != nil {
s, err = a.getDataBrokerSessionOrServiceAccount(ctx, sessionState.ID, sessionState.DatabrokerRecordVersion)
if status.Code(err) == codes.Unavailable {
log.Ctx(ctx).Debug().Str("request-id", requestID).Err(err).Msg("temporary error checking authorization: data broker unavailable")
return nil, err
} else if err != nil {
log.Ctx(ctx).Info().Err(err).Str("request-id", requestID).Msg("missing or invalid session or service account")
sessionState = nil
}
}
if sessionState != nil && s != nil {
u, _ = a.getDataBrokerUser(ctx, s.GetUserId()) // ignore any missing user error
}
res, err := state.evaluator.Evaluate(ctx, req)
if err != nil {
log.Ctx(ctx).Error().Err(err).Str("request-id", requestID).Msg("error during OPA evaluation")
return nil, err
}
a.logAuthorizeCheck(ctx, req, res, s, u)
/*result := &evaluateResult{
Allowed: res.Allow.Value && !res.Deny.Value,
}
// if show error details is enabled, attach the policy evaluation traces
if req.Policy != nil && req.Policy.ShowErrorDetails {
result.Traces = res.Traces
}*/
return res, nil
}

View file

@ -38,6 +38,7 @@ type RequestHTTP struct {
Method string `json:"method"`
Hostname string `json:"hostname"`
Path string `json:"path"`
Query string `json:"-"`
URL string `json:"url"`
Headers map[string]string `json:"headers"`
ClientCertificate ClientCertificateInfo `json:"client_certificate"`

View file

@ -31,6 +31,10 @@ type PolicyResponse struct {
Traces []contextutil.PolicyEvaluationTrace
}
func (r *PolicyResponse) Allowed() bool {
return r.Allow.Value && !r.Deny.Value
}
// NewPolicyResponse creates a new PolicyResponse.
func NewPolicyResponse() *PolicyResponse {
return &PolicyResponse{

View file

@ -17,12 +17,7 @@ import (
"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/user"
"github.com/pomerium/pomerium/pkg/storage"
"github.com/pomerium/pomerium/pkg/telemetry/requestid"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/structpb"
)
@ -31,12 +26,6 @@ func (a *Authorize) Check(ctx context.Context, in *envoy_service_auth_v3.CheckRe
ctx, span := a.tracer.Start(ctx, "authorize.grpc.Check")
defer span.End()
querier := storage.NewCachingQuerier(
storage.NewQuerier(a.state.Load().dataBrokerClient),
a.globalCache,
)
ctx = storage.WithQuerier(ctx, querier)
state := a.state.Load()
// convert the incoming envoy-style http request into a go-style http request
@ -46,45 +35,21 @@ func (a *Authorize) Check(ctx context.Context, in *envoy_service_auth_v3.CheckRe
sessionState, _ := state.sessionStore.LoadSessionStateAndCheckIDP(hreq)
var s sessionOrServiceAccount
var u *user.User
var err error
if sessionState != nil {
s, err = a.getDataBrokerSessionOrServiceAccount(ctx, sessionState.ID, sessionState.DatabrokerRecordVersion)
if status.Code(err) == codes.Unavailable {
log.Ctx(ctx).Debug().Str("request-id", requestID).Err(err).Msg("temporary error checking authorization: data broker unavailable")
return nil, err
} else if err != nil {
log.Ctx(ctx).Info().Err(err).Str("request-id", requestID).Msg("clearing session due to missing or invalid session or service account")
sessionState = nil
}
}
if sessionState != nil && s != nil {
u, _ = a.getDataBrokerUser(ctx, s.GetUserId()) // ignore any missing user error
}
req, err := a.getEvaluatorRequestFromCheckRequest(ctx, in, sessionState)
if err != nil {
log.Ctx(ctx).Error().Err(err).Str("request-id", requestID).Msg("error building evaluator request")
return nil, err
}
res, err := state.evaluator.Evaluate(ctx, req)
res, err := a.evaluate(ctx, req, sessionState)
if err != nil {
log.Ctx(ctx).Error().Err(err).Str("request-id", requestID).Msg("error during OPA evaluation")
return nil, err
}
// if show error details is enabled, attach the policy evaluation traces
if req.Policy != nil && req.Policy.ShowErrorDetails {
ctx = contextutil.WithPolicyEvaluationTraces(ctx, res.Traces)
}
resp, err := a.handleResult(ctx, in, req, res)
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)
return resp, err
}

View file

@ -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,
in *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 := in.HTTP.Headers
impersonateDetails := a.getImpersonateDetails(ctx, s)
evt := log.Ctx(ctx).Info().Str("service", "authorize")
fields := a.currentOptions.Load().GetAuthorizeLogFields()
for _, field := range fields {
evt = populateLogEvent(ctx, field, evt, in, s, u, hdrs, impersonateDetails, res)
evt = populateLogEvent(ctx, field, evt, in, 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,
in *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), in.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), in.HTTP.Hostname)
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), in.HTTP.IP)
case log.AuthorizeLogFieldMethod:
return evt.Str(string(field), in.GetAttributes().GetRequest().GetHttp().GetMethod())
return evt.Str(string(field), in.HTTP.Method)
case log.AuthorizeLogFieldPath:
return evt.Str(string(field), path)
return evt.Str(string(field), in.HTTP.Path)
case log.AuthorizeLogFieldQuery:
return evt.Str(string(field), query)
return evt.Str(string(field), in.HTTP.Query)
case log.AuthorizeLogFieldRequestID:
return evt.Str(string(field), requestid.FromContext(ctx))
case log.AuthorizeLogFieldServiceAccountID:

View file

@ -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,18 @@ 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",
},
},
},
request := &evaluator.Request{
HTTP: evaluator.RequestHTTP{
Method: "GET",
Hostname: "HOST",
Path: "/some/path",
Query: "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 +75,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"}`},
@ -104,7 +93,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, request, tc.s, u, impersonateDetails, res)
evt.Send()
assert.Equal(t, tc.expect, strings.TrimSpace(buf.String()))

View file

@ -169,7 +169,7 @@ func (a *Authorize) ManageStream(
if err != nil {
return err
}
res, err := a.state.Load().evaluator.Evaluate(ctx, req)
res, err := a.evaluate(ctx, req, &sessions.State{ID: session.Id})
if err != nil {
return err
}
@ -310,7 +310,7 @@ func (a *Authorize) ManageStream(
if err != nil {
return err
}
res, err := a.state.Load().evaluator.Evaluate(ctx, req)
res, err := a.evaluate(ctx, req, sessionState.Load())
if err != nil {
return err
}
@ -406,7 +406,7 @@ func (a *Authorize) getEvaluatorRequestFromSSHAuthRequest(
func handleEvaluatorResponseForSSH(
result *evaluator.Result, state *StreamState,
) *extensions_ssh.ServerMessage {
fmt.Printf(" *** evaluator result: %+v\n", result)
//fmt.Printf(" *** evaluator result: %+v\n", result)
// TODO: ideally there would be a way to keep this in sync with the logic in check_response.go
allow := result.Allow.Value && !result.Deny.Value
@ -440,6 +440,7 @@ func handleEvaluatorResponseForSSH(
// XXX: do we want to send an equivalent to the "show error details" output
// in the case of a deny result?
// XXX: this is not quite right -- needs to exactly match the last list of methods
methods := []string{"publickey"}
if slices.Contains(state.MethodsAuthenticated, "keyboard-interactive") {
methods = append(methods, "keyboard-interactive")