Skip to content

fix: bug in overlap detection of cert SANs#7234

Merged
rudrakhp merged 1 commit intoenvoyproxy:mainfrom
rudrakhp:san_overlap_detection
Oct 15, 2025
Merged

fix: bug in overlap detection of cert SANs#7234
rudrakhp merged 1 commit intoenvoyproxy:mainfrom
rudrakhp:san_overlap_detection

Conversation

@rudrakhp
Copy link
Copy Markdown
Member

What type of PR is this?

fix: bug in overlap detection of cert SANs

What this PR does / why we need it:
Fix SAN overlap detection

Which issue(s) this PR fixes:

Fixes #7230

Release Notes: Yes

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 14, 2025

Codecov Report

❌ Patch coverage is 88.23529% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.13%. Comparing base (5e41fac) to head (a84a2d4).
⚠️ Report is 32 commits behind head on main.

Files with missing lines Patch % Lines
internal/gatewayapi/listener.go 88.23% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7234      +/-   ##
==========================================
+ Coverage   71.12%   71.13%   +0.01%     
==========================================
  Files         228      228              
  Lines       40703    40709       +6     
==========================================
+ Hits        28949    28959      +10     
+ Misses      10061    10057       -4     
  Partials     1693     1693              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rudrakhp
Copy link
Copy Markdown
Member Author

/retest

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
Copy Markdown
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
Copy Markdown
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)

Copy link
Copy Markdown
Member

@zhaohuabing zhaohuabing left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for fixing this!

@rudrakhp rudrakhp merged commit 3c133bc into envoyproxy:main Oct 15, 2025
73 of 78 checks passed
shawnh2 pushed a commit to shawnh2/gateway that referenced this pull request Oct 15, 2025
arkodg added a commit that referenced this pull request Oct 15, 2025
* fix(xds-server): clear snapshot on stream close (#6618)

* fix(xds-server): clear snapshot on stream close

Signed-off-by: Zachary Vacura <[email protected]>

* check if there are other active connections before clearning the snapshot

Signed-off-by: Zachary Vacura <[email protected]>
Signed-off-by: shawnh2 <[email protected]>

* bug: disable x-envoy-ratelimited by default (#7110)

* bug: disable x-envoy-ratelimited by default

* can be enabled with `enableEnvoyHeaders` in CTP

Relates to #7034

Signed-off-by: Arko Dasgupta <[email protected]>

* fix tests and release note

Signed-off-by: Arko Dasgupta <[email protected]>

* fix testdata

Signed-off-by: Arko Dasgupta <[email protected]>

---------

Signed-off-by: Arko Dasgupta <[email protected]>
Signed-off-by: shawnh2 <[email protected]>

* fix(translator): Fix panic with request mirror + grpcroute (#6875)

Signed-off-by: Andrew Moreland <[email protected]>
Signed-off-by: shawnh2 <[email protected]>

* fix: bug in overlap detection of cert SANs (#7234)

Signed-off-by: shawnh2 <[email protected]>

* bump golang for crypto/x509 reggression (#7236)

Signed-off-by: zirain <[email protected]>
Signed-off-by: shawnh2 <[email protected]>

* fix gen-check

Signed-off-by: shawnh2 <[email protected]>

---------

Signed-off-by: Zachary Vacura <[email protected]>
Signed-off-by: shawnh2 <[email protected]>
Signed-off-by: Arko Dasgupta <[email protected]>
Signed-off-by: Andrew Moreland <[email protected]>
Signed-off-by: zirain <[email protected]>
Co-authored-by: Zach Vacura <[email protected]>
Co-authored-by: Arko Dasgupta <[email protected]>
Co-authored-by: Andrew Moreland <[email protected]>
Co-authored-by: Rudrakh Panigrahi <[email protected]>
Co-authored-by: zirain <[email protected]>
aburan28 pushed a commit to aburan28/gateway that referenced this pull request Oct 18, 2025
zirain pushed a commit to zirain/gateway that referenced this pull request Nov 14, 2025
zirain added a commit that referenced this pull request Nov 14, 2025
* fix: bug in overlap detection of cert SANs (#7234)

Signed-off-by: zirain <[email protected]>

* fix(translator): Fix panic with request mirror + grpcroute (#6875)

Signed-off-by: Andrew Moreland <[email protected]>
Signed-off-by: zirain <[email protected]>

* fix: watch change for the ca cert in the Backend (#7294)

* watch change for the ca cert in the Backend

Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: zirain <[email protected]>

* fix ipFamily not set in UDPListener (#7313)

fix: set ipfamily in udpistener (#7312)

Signed-off-by: cong <[email protected]>
Signed-off-by: zirain <[email protected]>

* coalesce updates to reduce intermediate updates (#7328)

* coalesce updates to reduce redundant processing in subscription handler

Signed-off-by: Huabing Zhao <[email protected]>

* retain order

Signed-off-by: Huabing Zhao <[email protected]>

* keep intermediate delete updates

Signed-off-by: Huabing Zhao <[email protected]>

* minor wording

Signed-off-by: Huabing Zhao <[email protected]>

* treat delete as normal operations

Signed-off-by: Huabing Zhao <[email protected]>

* retain the original order of the last updates for each key

Signed-off-by: Huabing Zhao <[email protected]>

* address comments

Signed-off-by: Huabing Zhao <[email protected]>

* fix test

Signed-off-by: Huabing Zhao <[email protected]>

---------

Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: zirain <[email protected]>

* fix: port typo (#7397)

Signed-off-by: cong <[email protected]>
Signed-off-by: zirain <[email protected]>

* fix: validate EnvoyGateway configuration before reload (#7412)

Signed-off-by: zirain <[email protected]>

* fix: missing  jwt provider when jwt is configured on multiple listeners sharing the same port (#7337)

* fix jwt provider missing when jwt is configured at multiple ir listeners

Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: zirain <[email protected]>

* fix: memory leak (#7429)

Fix memory leak.

Two watchable.Maps were never closed when shutting down the provider:
- GatewayClassStatuses.Close() - missing in GatewayAPIStatuses.Close()
- BackendTrafficPolicyStatuses.Close() - missing in PolicyStatuses.Close()

Each unclosed map leaked 3 goroutines:
1. Internal watchable.Map.coalesce goroutine
2. HandleSubscription goroutine blocked on channel read
3. Error handler goroutine blocked on channel read

Signed-off-by: Gonzalo Serrano <[email protected]>
Signed-off-by: zirain <[email protected]>

* fix gen after cherry-pick

Signed-off-by: zirain <[email protected]>

* fix watchutil test

Signed-off-by: Huabing Zhao <[email protected]>

---------

Signed-off-by: zirain <[email protected]>
Signed-off-by: Andrew Moreland <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: cong <[email protected]>
Signed-off-by: Gonzalo Serrano <[email protected]>
Co-authored-by: Rudrakh Panigrahi <[email protected]>
Co-authored-by: Andrew Moreland <[email protected]>
Co-authored-by: Huabing (Robin) Zhao <[email protected]>
Co-authored-by: 聪 <[email protected]>
Co-authored-by: Gonzalo Serrano <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug in logic detecting overlapping cert SANs

5 participants