fix: harden SSL host/certificate consistency (conflict detector + SAN coverage)#437
Open
shreemaan-abhishek wants to merge 2 commits into
Open
fix: harden SSL host/certificate consistency (conflict detector + SAN coverage)#437shreemaan-abhishek wants to merge 2 commits into
shreemaan-abhishek wants to merge 2 commits into
Conversation
Two correctness gaps in the SSL conflict detector let colliding TLS
configurations be admitted for the same GatewayProxy:
1. Hosts were compared by exact string equality, so a covering wildcard host
and an exact host were never compared against each other ("*.example.com"
and "app.example.com" are indexed under different keys). Because APISIX
resolves an exact SNI ahead of a covering wildcard, two objects whose hosts
overlap only through a wildcard could both be admitted and then collide at
the data plane. Add sslutil.HostsOverlap / ParentWildcard (single-label
wildcard semantics); for an exact host also look up its covering wildcard,
for a wildcard host enumerate TLS resources and filter by overlap (the
exact-key index can't answer a suffix query), and compare mappings with
HostsOverlap instead of string equality.
2. The conflict key used only the server certificate hash, so two objects for
the same host and server cert but different mTLS client config (spec.client)
were treated as non-conflicting, leaving client-verification behavior for
that SNI nondeterministic. Add ClientConfigHash to HostCertMapping (digest
of the CA secret reference, depth and skip_mtls_uri_regex; empty when no
mTLS) and treat a differing client config as a conflict too.
Adds unit tests for the overlap helpers and detector-level tests for both
wildcard/exact overlap and differing mTLS config, with false-positive guards.
📝 WalkthroughWalkthroughTLS processing now validates declared hosts against certificate DNS SANs, supports single-label wildcard hostname matching, and compares mTLS client-verification configuration hashes during TLS conflict detection. Tests cover coverage, overlap, wildcard lookup, and mTLS cases. ChangesSNI Conflict Detection
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TranslateApisixTls
participant Certificate
participant HostCoveredBy
TranslateApisixTls->>Certificate: extract DNS SANs
Certificate-->>TranslateApisixTls: return SAN list
TranslateApisixTls->>HostCoveredBy: check each declared host
HostCoveredBy-->>TranslateApisixTls: return coverage result
sequenceDiagram
participant DetectConflicts
participant TLSResourceIndex
participant ExistingTLSResources
DetectConflicts->>TLSResourceIndex: look up exact and covering wildcard hosts
TLSResourceIndex->>ExistingTLSResources: enumerate wildcard candidates
ExistingTLSResources-->>DetectConflicts: return TLS resources and mappings
DetectConflicts->>DetectConflicts: compare host overlap, certificate hash, and client config hash
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/webhook/v1/ssl/conflict_detector.go (1)
567-585: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
mappingForHostWithCacheshould not stop at the first overlap
BuildGatewayMappingsandBuildIngressMappingscan emit multiple mappings for the same host with different certificate/client-config hashes. Returning the first match letsfindExternalConflictsmiss other conflicting mappings in the same resource.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/webhook/v1/ssl/conflict_detector.go` around lines 567 - 585, The mappingForHostWithCache lookup must account for every overlapping mapping rather than returning only the first one, so findExternalConflicts can detect differing certificate/client-config hashes within the same resource. Update the surrounding conflict-detection flow to collect or compare all HostsOverlap matches while preserving the cache behavior and no-match result.Source: Coding guidelines
🧹 Nitpick comments (1)
internal/webhook/v1/ssl/conflict_detector.go (1)
354-421: 🚀 Performance & Scalability | 🔵 TrivialWildcard candidate enumeration logic looks correct.
Enumerating all TLS resources for wildcard incoming hosts (since the host index can't answer suffix queries) and merging
ParentWildcard+ no-host candidates for exact hosts is sound. One operational note:listAllTLSResourcesperforms an unfiltered cluster-wide list of Gateways/Ingresses/ApisixTls for every wildcard host in the request, which runs synchronously in the admission webhook path — likely fine against a cache-backed client, but worth keeping in mind for very large clusters.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/webhook/v1/ssl/conflict_detector.go` around lines 354 - 421, No code change is required; the wildcard candidate enumeration in findExternalConflicts is correct. Retain the existing listAllTLSResources, ParentWildcard, and no-host candidate behavior, while recognizing that wildcard requests perform a synchronous cluster-wide resource listing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/webhook/v1/ssl/conflict_detector.go`:
- Around line 274-290: Update clientConfigHash to use an injective encoding for
SkipMTLSUriRegex entries instead of strings.Join with commas, such as
length-prefixed elements or another unambiguous representation. Preserve sorting
and ensure distinct regex lists, including entries containing commas, produce
distinct canonical strings and hashes.
- Around line 100-118: Update the intra-resource conflict logic around the seen
map to compare each valid mapping pair using sslutil.HostsOverlap, so wildcard
and covered exact hosts are evaluated consistently with findExternalConflicts.
Preserve conflicts for differing certificate or client-config hashes, and extend
SSLConflict reporting in both this path and findExternalConflicts to surface
which field diverged when the certificate hashes match.
---
Outside diff comments:
In `@internal/webhook/v1/ssl/conflict_detector.go`:
- Around line 567-585: The mappingForHostWithCache lookup must account for every
overlapping mapping rather than returning only the first one, so
findExternalConflicts can detect differing certificate/client-config hashes
within the same resource. Update the surrounding conflict-detection flow to
collect or compare all HostsOverlap matches while preserving the cache behavior
and no-match result.
---
Nitpick comments:
In `@internal/webhook/v1/ssl/conflict_detector.go`:
- Around line 354-421: No code change is required; the wildcard candidate
enumeration in findExternalConflicts is correct. Retain the existing
listAllTLSResources, ParentWildcard, and no-host candidate behavior, while
recognizing that wildcard requests perform a synchronous cluster-wide resource
listing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 09fd8fa6-780c-46d9-911a-6a55b0c2eca1
📒 Files selected for processing (4)
internal/ssl/util.gointernal/ssl/util_test.gointernal/webhook/v1/ssl/conflict_detector.gointernal/webhook/v1/ssl/conflict_detector_test.go
Comment on lines
100
to
118
| // First, check for conflicts within the new resource itself. | ||
| seen := make(map[string]string, len(newMappings)) | ||
| seen := make(map[string]HostCertMapping, len(newMappings)) | ||
| for _, mapping := range newMappings { | ||
| if mapping.Host == "" || mapping.CertificateHash == "" { | ||
| continue | ||
| } | ||
| if prev, ok := seen[mapping.Host]; ok { | ||
| if prev != mapping.CertificateHash { | ||
| if prev.CertificateHash != mapping.CertificateHash || | ||
| prev.ClientConfigHash != mapping.ClientConfigHash { | ||
| conflicts = append(conflicts, SSLConflict{ | ||
| Host: mapping.Host, | ||
| ConflictingResource: mapping.ResourceRef, | ||
| CertificateHash: prev, | ||
| CertificateHash: prev.CertificateHash, | ||
| }) | ||
| } | ||
| continue | ||
| } | ||
| seen[mapping.Host] = mapping.CertificateHash | ||
| seen[mapping.Host] = mapping | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Intra-resource check isn't overlap-aware, unlike the new external check.
seen is keyed by exact mapping.Host string. If a single submitted object produces both a wildcard mapping (e.g. *.example.com) and an exact mapping it covers (e.g. app.example.com) with different CertificateHash/ClientConfigHash (possible for Ingress with multiple spec.tls blocks, or multi-listener Gateways), this loop will never compare them since they hash to different map keys — yet the external path (via sslutil.HostsOverlap) would flag the identical scenario across two different resources. This is an inconsistent enforcement of SNI uniqueness within a single object.
Also note: when a conflict is due only to a ClientConfigHash mismatch (same CertificateHash), the reported SSLConflict.CertificateHash doesn't indicate that the actual divergence was in mTLS config — same limitation applies to the findExternalConflicts conflict construction (Lines 449-453). Worth surfacing the differing field in the conflict result for operator debuggability.
🛠️ Suggested pairwise overlap check
- seen := make(map[string]HostCertMapping, len(newMappings))
- for _, mapping := range newMappings {
- if mapping.Host == "" || mapping.CertificateHash == "" {
- continue
- }
- if prev, ok := seen[mapping.Host]; ok {
- if prev.CertificateHash != mapping.CertificateHash ||
- prev.ClientConfigHash != mapping.ClientConfigHash {
- conflicts = append(conflicts, SSLConflict{
- Host: mapping.Host,
- ConflictingResource: mapping.ResourceRef,
- CertificateHash: prev.CertificateHash,
- })
- }
- continue
- }
- seen[mapping.Host] = mapping
- }
+ seen := make(map[string]HostCertMapping, len(newMappings))
+ valid := make([]HostCertMapping, 0, len(newMappings))
+ for _, mapping := range newMappings {
+ if mapping.Host == "" || mapping.CertificateHash == "" {
+ continue
+ }
+ valid = append(valid, mapping)
+ if _, ok := seen[mapping.Host]; !ok {
+ seen[mapping.Host] = mapping
+ }
+ }
+ for i := 0; i < len(valid); i++ {
+ for j := i + 1; j < len(valid); j++ {
+ if !sslutil.HostsOverlap(valid[i].Host, valid[j].Host) {
+ continue
+ }
+ if valid[i].CertificateHash != valid[j].CertificateHash ||
+ valid[i].ClientConfigHash != valid[j].ClientConfigHash {
+ conflicts = append(conflicts, SSLConflict{
+ Host: valid[j].Host,
+ ConflictingResource: valid[j].ResourceRef,
+ CertificateHash: valid[i].CertificateHash,
+ })
+ }
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/webhook/v1/ssl/conflict_detector.go` around lines 100 - 118, Update
the intra-resource conflict logic around the seen map to compare each valid
mapping pair using sslutil.HostsOverlap, so wildcard and covered exact hosts are
evaluated consistently with findExternalConflicts. Preserve conflicts for
differing certificate or client-config hashes, and extend SSLConflict reporting
in both this path and findExternalConflicts to surface which field diverged when
the certificate hashes match.
Comment on lines
+274
to
+290
| // clientConfigHash digests an ApisixTls mTLS client-verification config into a | ||
| // stable key. Returns "" when no mTLS is configured, so a resource that enforces | ||
| // mTLS and one that doesn't produce different keys for the same host+cert. It | ||
| // keys on the CA secret reference (namespace/name), which uniquely identifies | ||
| // the trust anchor, plus depth and the skip regexes. | ||
| func clientConfigHash(client *apiv2.ApisixMutualTlsClientConfig) string { | ||
| if client == nil { | ||
| return "" | ||
| } | ||
| regexes := append([]string(nil), client.SkipMTLSUriRegex...) | ||
| sort.Strings(regexes) | ||
| canonical := fmt.Sprintf("ca=%s/%s;depth=%d;skip=%s", | ||
| client.CASecret.Namespace, client.CASecret.Name, client.Depth, | ||
| strings.Join(regexes, ",")) | ||
| sum := sha256.Sum256([]byte(canonical)) | ||
| return hex.EncodeToString(sum[:]) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
clientConfigHash canonicalization is not injective — comma-joined regexes can collide.
strings.Join(regexes, ",") makes {"a,b"} and {"a","b"} produce the identical canonical string skip=a,b. Since SkipMTLSUriRegex entries are regexes, a literal comma is a legal regex character (e.g. inside a character class), so two genuinely different skip-regex sets could hash identically and mask a real mTLS conflict.
🛡️ Suggested length-prefixed canonicalization
func clientConfigHash(client *apiv2.ApisixMutualTlsClientConfig) string {
if client == nil {
return ""
}
regexes := append([]string(nil), client.SkipMTLSUriRegex...)
sort.Strings(regexes)
- canonical := fmt.Sprintf("ca=%s/%s;depth=%d;skip=%s",
- client.CASecret.Namespace, client.CASecret.Name, client.Depth,
- strings.Join(regexes, ","))
- sum := sha256.Sum256([]byte(canonical))
- return hex.EncodeToString(sum[:])
+ h := sha256.New()
+ fmt.Fprintf(h, "ca=%s/%s;depth=%d;", client.CASecret.Namespace, client.CASecret.Name, client.Depth)
+ for _, r := range regexes {
+ fmt.Fprintf(h, "%d:%s;", len(r), r)
+ }
+ return hex.EncodeToString(h.Sum(nil))
}As per coding guidelines, **/*.{js,ts,go}: "enforce SNI/domain uniqueness when updating SSL certificates."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // clientConfigHash digests an ApisixTls mTLS client-verification config into a | |
| // stable key. Returns "" when no mTLS is configured, so a resource that enforces | |
| // mTLS and one that doesn't produce different keys for the same host+cert. It | |
| // keys on the CA secret reference (namespace/name), which uniquely identifies | |
| // the trust anchor, plus depth and the skip regexes. | |
| func clientConfigHash(client *apiv2.ApisixMutualTlsClientConfig) string { | |
| if client == nil { | |
| return "" | |
| } | |
| regexes := append([]string(nil), client.SkipMTLSUriRegex...) | |
| sort.Strings(regexes) | |
| canonical := fmt.Sprintf("ca=%s/%s;depth=%d;skip=%s", | |
| client.CASecret.Namespace, client.CASecret.Name, client.Depth, | |
| strings.Join(regexes, ",")) | |
| sum := sha256.Sum256([]byte(canonical)) | |
| return hex.EncodeToString(sum[:]) | |
| } | |
| func clientConfigHash(client *apiv2.ApisixMutualTlsClientConfig) string { | |
| if client == nil { | |
| return "" | |
| } | |
| regexes := append([]string(nil), client.SkipMTLSUriRegex...) | |
| sort.Strings(regexes) | |
| h := sha256.New() | |
| fmt.Fprintf(h, "ca=%s/%s;depth=%d;", client.CASecret.Namespace, client.CASecret.Name, client.Depth) | |
| for _, r := range regexes { | |
| fmt.Fprintf(h, "%d:%s;", len(r), r) | |
| } | |
| return hex.EncodeToString(h.Sum(nil)) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/webhook/v1/ssl/conflict_detector.go` around lines 274 - 290, Update
clientConfigHash to use an injective encoding for SkipMTLSUriRegex entries
instead of strings.Join with commas, such as length-prefixed elements or another
unambiguous representation. Preserve sorting and ensure distinct regex lists,
including entries containing commas, produce distinct canonical strings and
hashes.
Source: Coding guidelines
Contributor
conformance test report - apisix modeapiVersion: gateway.networking.k8s.io/v1
date: "2026-07-19T06:36:36Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
contact: null
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
result: success
statistics:
Failed: 0
Passed: 12
Skipped: 0
name: GATEWAY-GRPC
summary: Core tests succeeded.
- core:
failedTests:
- HTTPRouteInvalidBackendRefUnknownKind
result: failure
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 1
Passed: 31
Skipped: 1
extended:
result: partial
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 0
Passed: 11
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- GatewayHTTPListenerIsolation
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
name: GATEWAY-HTTP
summary: Core tests failed with 1 test failures. Extended tests partially succeeded
with 1 test skips.
- core:
result: partial
skippedTests:
- TLSRouteSimpleSameNamespace
statistics:
Failed: 0
Passed: 10
Skipped: 1
name: GATEWAY-TLS
summary: Core tests partially succeeded with 1 test skips. |
Contributor
conformance test report - apisix-standalone modeapiVersion: gateway.networking.k8s.io/v1
date: "2026-07-19T06:36:08Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
contact: null
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
result: success
statistics:
Failed: 0
Passed: 12
Skipped: 0
name: GATEWAY-GRPC
summary: Core tests succeeded.
- core:
result: partial
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 0
Passed: 32
Skipped: 1
extended:
result: partial
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 0
Passed: 11
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- GatewayHTTPListenerIsolation
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
name: GATEWAY-HTTP
summary: Core tests partially succeeded with 1 test skips. Extended tests partially
succeeded with 1 test skips.
- core:
result: partial
skippedTests:
- TLSRouteSimpleSameNamespace
statistics:
Failed: 0
Passed: 10
Skipped: 1
name: GATEWAY-TLS
summary: Core tests partially succeeded with 1 test skips. |
TranslateApisixTls copied spec.hosts verbatim into the SSL object's SNIs and only checked that the referenced Secret existed and yielded a keypair. A certificate whose SANs don't cover a declared host (e.g. an internal cert bound to a public hostname) was programmed onto the data plane with no error, so external clients would receive a certificate invalid for the requested host. The admission webhook is fail-open, so the reconcile/translate path has to re-validate independently. Add validateSNICoverage: reject translation when a declared host is not covered by any DNS SAN in the certificate, using the wildcard-aware sslutil.HostCoveredBy (directional: a wildcard SAN covers single-label subdomains, an exact SAN covers only itself). Lenient when the cert declares no DNS SANs or can't be parsed, so certs without a DNS identity keep working.
shreemaan-abhishek
changed the title
Contributor
conformance test reportapiVersion: gateway.networking.k8s.io/v1
date: "2026-07-19T06:53:50Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
contact: null
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
failedTests:
- GatewayModifyListeners
- TLSRouteSimpleSameNamespace
result: failure
statistics:
Failed: 2
Passed: 9
Skipped: 0
name: GATEWAY-TLS
summary: Core tests failed with 2 test failures.
- core:
failedTests:
- GatewayModifyListeners
result: failure
statistics:
Failed: 1
Passed: 11
Skipped: 0
name: GATEWAY-GRPC
summary: Core tests failed with 1 test failures.
- core:
failedTests:
- GatewayModifyListeners
result: failure
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 1
Passed: 31
Skipped: 1
extended:
result: partial
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 0
Passed: 11
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- GatewayHTTPListenerIsolation
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
name: GATEWAY-HTTP
summary: Core tests failed with 1 test failures. Extended tests partially succeeded
with 1 test skips. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Ports the SSL host/certificate consistency hardening from apache/apisix-ingress-controller#2810 to the enterprise controller.
1. Wildcard vs. exact host matching in the SSL conflict detector
Hosts were compared by exact string equality, so a covering wildcard and an exact host were never compared (
*.example.comvsapp.example.com, indexed under different keys). APISIX resolves an exact SNI ahead of a covering wildcard, so overlapping objects could both be admitted and serve conflicting certs for the same hostname.sslutil.HostsOverlap/ParentWildcard; exact host also queries its covering wildcard, wildcard host enumerates + filters by overlap; compare withHostsOverlap2. mTLS client config ignored in the conflict key
Keyed only on server cert hash, so same host+cert with different
spec.clientwas non-conflicting.ClientConfigHash(CA secret ref + depth + skip regex; empty when no mTLS); conflict when cert OR client hash differs3. No certificate SAN coverage check in the translator
TranslateApisixTlsprogrammed the SSL object without checking the cert SANs cover the declared hosts; the webhook is fail-open, so the translate path re-validates.sslutil.HostCoveredBy(directional, wildcard-aware); reject translation when a declared host isn't covered by any DNS SAN; lenient when the cert has no DNS SANs / can't be parsedNotes
Tests
HostsOverlap/ParentWildcard/HostCoveredBySummary by CodeRabbit
Bug Fixes
Tests