Skip to content
Merged
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
35 changes: 22 additions & 13 deletions internal/gatewayapi/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ func checkOverlappingHostnames(httpsListeners []*ListenerContext) {
if httpsListeners[i].Port != httpsListeners[j].Port {
continue
}
if isOverlappingHostname(httpsListeners[i].Hostname, httpsListeners[j].Hostname) {
if areOverlappingHostnames(httpsListeners[i].Hostname, httpsListeners[j].Hostname) {
// Overlapping listeners can be more than two, we only report the first two for simplicity.
overlappingListeners[i] = &overlappingListener{
gateway1: httpsListeners[i].gateway,
Expand Down Expand Up @@ -398,7 +398,7 @@ type overlappingCertificate struct {
func isOverlappingCertificate(cert1DNSNames, cert2DNSNames []string) *overlappingCertificate {
for _, dns1 := range cert1DNSNames {
for _, dns2 := range cert2DNSNames {
if isOverlappingHostname(ptr.To(gwapiv1.Hostname(dns1)), ptr.To(gwapiv1.Hostname(dns2))) {
if areOverlappingHostnames(ptr.To(gwapiv1.Hostname(dns1)), ptr.To(gwapiv1.Hostname(dns2))) {
return &overlappingCertificate{
san1: dns1,
san2: dns2,
Expand All @@ -409,22 +409,31 @@ func isOverlappingCertificate(cert1DNSNames, cert2DNSNames []string) *overlappin
return nil
}

// isOverlappingHostname checks if two hostnames overlap.
func isOverlappingHostname(hostname1, hostname2 *gwapiv1.Hostname) bool {
if hostname1 == nil || hostname2 == nil {
func areOverlappingHostnames(this, other *gwapiv1.Hostname) bool {
if this == nil || other == nil {
return true
}
domain1 := strings.Replace(string(*hostname1), "*.", "", 1)
domain2 := strings.Replace(string(*hostname2), "*.", "", 1)
return isSubdomain(domain1, domain2) || isSubdomain(domain2, domain1)
return hostnameMatchesWithOther(this, other) || hostnameMatchesWithOther(other, this)
}

// isSubdomain checks if subDomain is a sub-domain of domain
func isSubdomain(subDomain, domain string) bool {
if subDomain == domain {
return true
// hostnameMatchesWithOther returns true if this hostname matches other hostname.
// Assumes that hostnames will either be fully qualified or a wildcard hostname prefixed with a single wildcard.
// E.g. "*.*.example.com" is not valid.
func hostnameMatchesWithOther(this, other *gwapiv1.Hostname) bool {
thisString := string(*this)
otherString := string(*other)
if hasWildcardPrefix(other) && !hasWildcardPrefix(this) {
return strings.HasSuffix(thisString, otherString[1:]) &&
!strings.Contains(strings.TrimSuffix(thisString, otherString[1:]), ".") // not a subdomain
}
return thisString == otherString
}

func hasWildcardPrefix(h *gwapiv1.Hostname) bool {
if h == nil {
return false
}
return strings.HasSuffix(subDomain, fmt.Sprintf(".%s", domain))
return len(string(*h)) > 1 && string(*h)[0] == '*'
}

func buildListenerMetadata(listener *ListenerContext, gateway *GatewayContext) *ir.ResourceMetadata {
Expand Down
29 changes: 17 additions & 12 deletions internal/gatewayapi/listener_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func TestProxySamplingRate(t *testing.T) {
}
}

func TestIsOverlappingHostname(t *testing.T) {
func TestAreOverlappingHostnames(t *testing.T) {
tests := []struct {
name string
hostname1 *gwapiv1.Hostname
Expand All @@ -120,10 +120,10 @@ func TestIsOverlappingHostname(t *testing.T) {
want: true,
},
{
name: "two wildcards matches subdomain",
name: "two wildcards with subdomain does not match",
hostname1: ptr.To(gwapiv1.Hostname("*.example.com")),
hostname2: ptr.To(gwapiv1.Hostname("*.test.example.com")),
want: true,
want: false,
},
{
name: "nil hostname matches all",
Expand All @@ -144,22 +144,22 @@ func TestIsOverlappingHostname(t *testing.T) {
want: true,
},
{
name: "wildcard matches exact",
name: "wildcard matches exactly one level of subdomain",
hostname1: ptr.To(gwapiv1.Hostname("*.example.com")),
hostname2: ptr.To(gwapiv1.Hostname("test.example.com")),
want: true,
},
{
name: "wildcard matches subdomain",
name: "wildcard matches only one level of subdomain",
hostname1: ptr.To(gwapiv1.Hostname("*.example.com")),
hostname2: ptr.To(gwapiv1.Hostname("sub.test.example.com")),
want: true,
want: false,
},
{
name: "wildcard matches empty subdomain",
name: "wildcard does not match empty subdomain",
hostname1: ptr.To(gwapiv1.Hostname("*.example.com")),
hostname2: ptr.To(gwapiv1.Hostname("example.com")),
want: true,
want: false,
},
{
name: "different domains",
Expand All @@ -185,15 +185,21 @@ func TestIsOverlappingHostname(t *testing.T) {
hostname2: ptr.To(gwapiv1.Hostname("testing-api.foo.dev")),
want: false,
},
{
name: "sub domain does not match with parent domain",
hostname1: ptr.To(gwapiv1.Hostname("api.foo.dev")),
hostname2: ptr.To(gwapiv1.Hostname("foo.dev")),
want: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := isOverlappingHostname(tt.hostname1, tt.hostname2); got != tt.want {
if got := areOverlappingHostnames(tt.hostname1, tt.hostname2); got != tt.want {
t.Errorf("isOverlappingHostname(%q, %q) = %v, want %v", ptr.Deref(tt.hostname1, ""), ptr.Deref(tt.hostname2, ""), got, tt.want)
Copy link
Member

Choose a reason for hiding this comment

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

Nit:

Suggested change
t.Errorf("isOverlappingHostname(%q, %q) = %v, want %v", ptr.Deref(tt.hostname1, ""), ptr.Deref(tt.hostname2, ""), got, tt.want)
t.Errorf("areOverlappingHostnames(%q, %q) = %v, want %v", ptr.Deref(tt.hostname1, ""), ptr.Deref(tt.hostname2, ""), got, tt.want)

}
// Test should be symmetric
if got := isOverlappingHostname(tt.hostname2, tt.hostname1); got != tt.want {
if got := areOverlappingHostnames(tt.hostname2, tt.hostname1); got != tt.want {
t.Errorf("isOverlappingHostname(%q, %q) = %v, want %v", ptr.Deref(tt.hostname2, ""), ptr.Deref(tt.hostname1, ""), got, tt.want)
Copy link
Member

Choose a reason for hiding this comment

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

Nit:

Suggested change
t.Errorf("isOverlappingHostname(%q, %q) = %v, want %v", ptr.Deref(tt.hostname2, ""), ptr.Deref(tt.hostname1, ""), got, tt.want)
t.Errorf("areOverlappingHostnames(%q, %q) = %v, want %v", ptr.Deref(tt.hostname2, ""), ptr.Deref(tt.hostname1, ""), got, tt.want)

}
})
Expand Down Expand Up @@ -306,15 +312,14 @@ func TestCheckOverlappingHostnames(t *testing.T) {
Name: "listener-3",
Protocol: gwapiv1.HTTPSProtocolType,
Port: 443,
Hostname: ptr.To(gwapiv1.Hostname("sub.test.example.com")),
Hostname: ptr.To(gwapiv1.Hostname("sub.test.example.com")), // sub domain does not match with parent domain
},
},
},
},
expected: map[int]string{
0: "test.example.com",
1: "*.example.com",
2: "*.example.com",
},
},
{
Expand Down
1 change: 1 addition & 0 deletions release-notes/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ bug fixes: |
Fixed a nil pointer panic in the XDS translator when building API key authentication filter configurations with `sanitize` enabled and no `forwardClientIDHeader` set.
Truncated Gateway API status condition messages to stay within Kubernetes limits and prevent update failures.
Fixed an issue in EnvoyPatchPolicy where it didn't match the target Gateway/GatewayClass due to an incorrect name reference.
Fixed certificate SAN overlap detection in gateway listeners.

# Enhancements that improve performance.
performance improvements: |
Expand Down
Loading