Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xds: E2E Test for Audit Logging #6377

Merged
merged 9 commits into from
Jun 29, 2023
Merged
96 changes: 96 additions & 0 deletions test/xds/xds_server_rbac_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,18 @@ package xds_test

import (
"context"
"encoding/json"
"fmt"
"net"
"strconv"
"strings"
"testing"

v3xdsxdstypepb "github.com/cncf/xds/go/xds/type/v3"
v3routerpb "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/router/v3"
"github.com/google/go-cmp/cmp"
"google.golang.org/grpc"
"google.golang.org/grpc/authz/audit"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/credentials/insecure"
"google.golang.org/grpc/internal"
Expand All @@ -36,6 +40,7 @@ import (
"google.golang.org/grpc/internal/testutils/xds/e2e"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/anypb"
"google.golang.org/protobuf/types/known/structpb"

v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
v3listenerpb "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
Expand Down Expand Up @@ -415,6 +420,8 @@ func (s) TestRBACHTTPFilter(t *testing.T) {
rbacCfg *rpb.RBAC
wantStatusEmptyCall codes.Code
wantStatusUnaryCall codes.Code
wantAuthzOutcomes map[bool]int
eventContent *audit.Event
}{
// This test tests an RBAC HTTP Filter which is configured to allow any RPC.
// Any RPC passing through this RBAC HTTP Filter should proceed as normal.
Expand All @@ -433,10 +440,30 @@ func (s) TestRBACHTTPFilter(t *testing.T) {
},
},
},
AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{
AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_ON_ALLOW,
LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{
{
AuditLogger: &v3corepb.TypedExtensionConfig{
Name: "stat_logger",
TypedConfig: createXDSTypedStruct(t, map[string]interface{}{}, "stat_logger"),
},
IsOptional: false,
},
},
},
},
},
wantStatusEmptyCall: codes.OK,
wantStatusUnaryCall: codes.OK,
wantAuthzOutcomes: map[bool]int{true: 2, false: 0},
// TODO(gtcooke94) add policy name (RBAC filter name) once
// https://github.com/grpc/grpc-go/pull/6327 is merged.
eventContent: &audit.Event{
FullMethodName: "/grpc.testing.TestService/UnaryCall",
rockspore marked this conversation as resolved.
Show resolved Hide resolved
MatchedRule: "anyone",
Authorized: true,
},
},
// This test tests an RBAC HTTP Filter which is configured to allow only
// RPC's with certain paths ("UnaryCall"). Only unary calls passing
Expand Down Expand Up @@ -605,6 +632,12 @@ func (s) TestRBACHTTPFilter(t *testing.T) {
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
func() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not new but do you know why such a anonymous func is needed here?

Copy link
Contributor Author

@gtcooke94 gtcooke94 Jun 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No clue, I'm just working into the existing test - at a glance it does seem unnecessary

lb := &loggerBuilder{
authzDecisionStat: map[bool]int{true: 0, false: 0},
lastEvent: &audit.Event{},
rockspore marked this conversation as resolved.
Show resolved Hide resolved
}
audit.RegisterLoggerBuilder(lb)

managementServer, nodeID, bootstrapContents, resolver, cleanup1 := e2e.SetupManagementServer(t, e2e.ManagementServerOptions{})
defer cleanup1()

Expand Down Expand Up @@ -660,6 +693,17 @@ func (s) TestRBACHTTPFilter(t *testing.T) {
}
// Toggle RBAC back on for next iterations.
envconfig.XDSRBAC = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth considering adding some check to verify audit logging is not active till RBAC is toggled back.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it ends up being somewhat implicit in the test - it makes 2 calls with RBAC on, then 2 calls with RBAC off, and the expected outcome for logging is allowed: 2, denied: 0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree that this isn't the best as it requires holding some test state in your head as you look at it, but I hesitate to make large changes to this existing test


if test.wantAuthzOutcomes != nil {
if diff := cmp.Diff(lb.authzDecisionStat, test.wantAuthzOutcomes); diff != "" {
t.Fatalf("authorization decision do not match\ndiff (-got +want):\n%s", diff)
}
}
if test.eventContent != nil {
if diff := cmp.Diff(lb.lastEvent, test.eventContent); diff != "" {
t.Fatalf("unexpected event\ndiff (-got +want):\n%s", diff)
}
}
}()
})
}
Expand Down Expand Up @@ -895,3 +939,55 @@ func (s) TestRBACToggledOff_WithBadRouteConfiguration(t *testing.T) {
t.Fatalf("UnaryCall() returned err with status: %v, if RBAC is disabled all RPC's should proceed as normal", status.Code(err))
}
}

type statAuditLogger struct {
authzDecisionStat map[bool]int // Map to hold counts of authorization decisions
lastEvent *audit.Event // Field to store last received event
}

func (s *statAuditLogger) Log(event *audit.Event) {
s.authzDecisionStat[event.Authorized]++
*s.lastEvent = *event
}

type loggerBuilder struct {
authzDecisionStat map[bool]int
lastEvent *audit.Event
}

func (loggerBuilder) Name() string {
return "stat_logger"
}

func (lb *loggerBuilder) Build(audit.LoggerConfig) audit.Logger {
return &statAuditLogger{
authzDecisionStat: lb.authzDecisionStat,
lastEvent: lb.lastEvent,
}
}

func (*loggerBuilder) ParseLoggerConfig(config json.RawMessage) (audit.LoggerConfig, error) {
return nil, nil
}

// This is used when converting a custom config from raw JSON to a TypedStruct.
// The TypeURL of the TypeStruct will be "grpc.authz.audit_logging/<name>".
const typeURLPrefix = "grpc.authz.audit_logging/"

// Builds custom configs for audit logger RBAC protos.
func createXDSTypedStruct(t *testing.T, in map[string]interface{}, name string) *anypb.Any {
t.Helper()
pb, err := structpb.NewStruct(in)
if err != nil {
t.Fatalf("createXDSTypedStruct failed during structpb.NewStruct: %v", err)
}
typedStruct := &v3xdsxdstypepb.TypedStruct{
TypeUrl: typeURLPrefix + name,
Value: pb,
}
customConfig, err := anypb.New(typedStruct)
if err != nil {
t.Fatalf("createXDSTypedStruct failed during anypb.New: %v", err)
}
return customConfig
}