Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion internal/xds/rbac/matchers.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,16 @@ func (am *authenticatedMatcher) match(data *rpcData) bool {
cert := data.certs[0]
// Use the first non-empty identity source in priority order:
// URI SANs, then DNS SANs, then Subject.
if len(cert.URIs) > 0 {
// Skip degenerate URI entries (empty url.URL{}) that serialize to ""
// to avoid treating them as a present identity source.
hasNonEmptyURI := false
for _, u := range cert.URIs {
if u.String() != "" {
hasNonEmptyURI = true
break
}
}
if hasNonEmptyURI {
for _, uriSAN := range cert.URIs {
if am.stringMatcher.Match(uriSAN.String()) {
return true
Expand Down
101 changes: 101 additions & 0 deletions internal/xds/rbac/rbac_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2035,3 +2035,104 @@ func TestAuthenticatedMatcherIdentitySourcePrecedence(t *testing.T) {
})
}
}

// TestAuthenticatedMatcherEmptyURISAN verifies that a certificate containing
// a degenerate (empty) URI SAN does not prevent fallthrough to DNS SANs.
// An empty url.URL{} serializes to "" via String(), which should not be
// treated as a meaningful identity source.
func TestAuthenticatedMatcherEmptyURISAN(t *testing.T) {
tests := []struct {
name string
uris []*url.URL
dnsNames []string
subjectCN string
matcher *v3matcherpb.StringMatcher
wantMatch bool
}{
{
name: "emptyURISANFallsThroughToDNS",
uris: []*url.URL{{}},
dnsNames: []string{"svc.legitimate.internal"},
matcher: &v3matcherpb.StringMatcher{
MatchPattern: &v3matcherpb.StringMatcher_Contains{
Contains: "svc.legitimate.internal",
},
},
wantMatch: true,
},
{
name: "emptyURISANDoesNotMatchPrefix",
uris: []*url.URL{{}},
matcher: &v3matcherpb.StringMatcher{
MatchPattern: &v3matcherpb.StringMatcher_Prefix{
Prefix: "spiffe://corp.example/",
},
},
wantMatch: false,
},
{
name: "mixedEmptyAndValidURISAN",
uris: []*url.URL{{}, mustParseURL("spiffe://corp.example/svc/admin")},
matcher: &v3matcherpb.StringMatcher{
MatchPattern: &v3matcherpb.StringMatcher_Contains{
Contains: "spiffe://corp.example/svc/admin",
},
},
wantMatch: true,
},
{
name: "mixedEmptyAndValidURISANNoFallthrough",
uris: []*url.URL{{}, mustParseURL("spiffe://corp.example/svc/other")},
dnsNames: []string{"svc.legitimate.internal"},
matcher: &v3matcherpb.StringMatcher{
MatchPattern: &v3matcherpb.StringMatcher_Contains{
Contains: "svc.legitimate.internal",
},
},
wantMatch: false,
},
{
name: "allEmptyURISANsFallThroughToSubject",
uris: []*url.URL{{}, {}},
subjectCN: "svc.legitimate.internal",
matcher: &v3matcherpb.StringMatcher{
MatchPattern: &v3matcherpb.StringMatcher_Contains{
Contains: "svc.legitimate.internal",
},
},
wantMatch: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cert := &x509.Certificate{
URIs: tt.uris,
DNSNames: tt.dnsNames,
Subject: pkix.Name{CommonName: tt.subjectCN},
}
amConfig := &v3rbacpb.Principal_Authenticated{
PrincipalName: tt.matcher,
}
matcher, err := newAuthenticatedMatcher(amConfig)
if err != nil {
t.Fatalf("failed to create matcher: %v", err)
}
rpcData := &rpcData{
authType: "tls",
certs: []*x509.Certificate{cert},
}
if got := matcher.match(rpcData); got != tt.wantMatch {
t.Errorf("match() = %v, want %v", got, tt.wantMatch)
}
})
}
}

func mustParseURL(s string) *url.URL {
u, err := url.Parse(s)
if err != nil {
panic(err)
}
return u
}
Loading