From b5a960ee5658dcc7bf0353f64e273669c8ea9ef5 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Sun, 19 Jul 2026 11:49:53 +0545 Subject: [PATCH 1/2] fix: match overlapping hosts and mTLS config in SSL conflict detector 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. --- internal/ssl/util.go | 52 +++++ internal/ssl/util_test.go | 59 ++++++ internal/webhook/v1/ssl/conflict_detector.go | 151 +++++++++++--- .../webhook/v1/ssl/conflict_detector_test.go | 189 ++++++++++++++++++ 4 files changed, 425 insertions(+), 26 deletions(-) create mode 100644 internal/ssl/util_test.go diff --git a/internal/ssl/util.go b/internal/ssl/util.go index ad34dd40..ee86fec8 100644 --- a/internal/ssl/util.go +++ b/internal/ssl/util.go @@ -180,6 +180,58 @@ func NormalizeHosts(hosts []string) []string { return normalized } +// HostsOverlap reports whether two SNI host patterns can both match a common +// concrete hostname. It understands single-label wildcards ("*.example.com" +// matches "app.example.com" but not "a.b.example.com"). Two distinct wildcards +// never share a concrete host. +func HostsOverlap(a, b string) bool { + a = strings.ToLower(strings.TrimSpace(a)) + b = strings.ToLower(strings.TrimSpace(b)) + if a == "" || b == "" { + return false + } + if a == b { + return true + } + aWild := strings.HasPrefix(a, "*.") + bWild := strings.HasPrefix(b, "*.") + switch { + case aWild && bWild: + return false + case aWild: + return wildcardCovers(a, b) + case bWild: + return wildcardCovers(b, a) + default: + return false + } +} + +// wildcardCovers reports whether wildcard "*.suffix" matches the exact host, +// i.e. host is exactly ".suffix". +func wildcardCovers(wildcard, host string) bool { + suffix := wildcard[1:] // ".example.com" + if !strings.HasSuffix(host, suffix) { + return false + } + label := host[:len(host)-len(suffix)] + return label != "" && !strings.Contains(label, ".") +} + +// ParentWildcard returns the single-label covering wildcard for an exact host, +// e.g. "app.example.com" -> "*.example.com". Returns "" if host is empty, is +// itself a wildcard, or has no parent label. +func ParentWildcard(host string) string { + if host == "" || strings.HasPrefix(host, "*.") { + return "" + } + i := strings.IndexByte(host, '.') + if i < 0 || i == len(host)-1 { + return "" + } + return "*." + host[i+1:] +} + // CertificateHash returns the SHA-256 hash of the leaf certificate contained in the PEM data. // The hash is calculated from the DER-encoded bytes so that formatting differences (whitespace, // line endings, certificate ordering) do not affect the result. diff --git a/internal/ssl/util_test.go b/internal/ssl/util_test.go new file mode 100644 index 00000000..11d4985d --- /dev/null +++ b/internal/ssl/util_test.go @@ -0,0 +1,59 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ssl + +import "testing" + +func TestHostsOverlap(t *testing.T) { + cases := []struct { + a, b string + want bool + }{ + {"example.com", "example.com", true}, // identical exact + {"*.example.com", "*.example.com", true}, // identical wildcard + {"app.example.com", "*.example.com", true}, // exact covered by wildcard + {"*.example.com", "app.example.com", true}, // wildcard covers exact (inverse) + {"App.Example.com", "*.EXAMPLE.com", true}, // case-insensitive + {"a.b.example.com", "*.example.com", false}, // multi-label not covered + {"example.com", "*.example.com", false}, // apex not covered by its wildcard + {"app.example.com", "app.other.com", false}, // different exacts + {"*.example.com", "*.other.com", false}, // distinct wildcards never overlap + {"*.a.example.com", "*.example.com", false}, // wildcards at different depths + {"", "example.com", false}, // empty + } + for _, c := range cases { + if got := HostsOverlap(c.a, c.b); got != c.want { + t.Errorf("HostsOverlap(%q, %q) = %v, want %v", c.a, c.b, got, c.want) + } + } +} + +func TestParentWildcard(t *testing.T) { + cases := []struct { + host, want string + }{ + {"app.example.com", "*.example.com"}, + {"example.com", "*.com"}, + {"*.example.com", ""}, // already a wildcard + {"localhost", ""}, // no parent label + {"", ""}, + } + for _, c := range cases { + if got := ParentWildcard(c.host); got != c.want { + t.Errorf("ParentWildcard(%q) = %q, want %q", c.host, got, c.want) + } + } +} diff --git a/internal/webhook/v1/ssl/conflict_detector.go b/internal/webhook/v1/ssl/conflict_detector.go index fe5983ea..95db3dce 100644 --- a/internal/webhook/v1/ssl/conflict_detector.go +++ b/internal/webhook/v1/ssl/conflict_detector.go @@ -17,6 +17,8 @@ package ssl import ( "context" + "crypto/sha256" + "encoding/hex" "fmt" "sort" "strings" @@ -42,7 +44,11 @@ var logger = log.Log.WithName("ssl-conflict-detector") type HostCertMapping struct { Host string CertificateHash string - ResourceRef string + // ClientConfigHash digests the mTLS client-verification config (CA ref, depth, + // skip regexes). Empty when the resource enforces no mTLS. Two resources for the + // same host+cert but differing client config are still a conflict. + ClientConfigHash string + ResourceRef string } // SSLConflict exposes the conflict details to the admission webhook for reporting. @@ -92,22 +98,23 @@ func (d *ConflictDetector) DetectConflicts(ctx context.Context, obj client.Objec conflicts := make([]SSLConflict, 0) // 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 } if len(conflicts) > 0 { @@ -251,17 +258,37 @@ func (d *ConflictDetector) BuildApisixTlsMappings(ctx context.Context, tls *apiv // if len(hosts) == 0 { // hosts = info.hosts // } + clientHash := clientConfigHash(tls.Spec.Client) for _, host := range hosts { mappings = append(mappings, HostCertMapping{ - Host: host, - CertificateHash: info.hash, - ResourceRef: fmt.Sprintf("%s/%s/%s", internaltypes.KindApisixTls, tls.Namespace, tls.Name), + Host: host, + CertificateHash: info.hash, + ClientConfigHash: clientHash, + ResourceRef: fmt.Sprintf("%s/%s/%s", internaltypes.KindApisixTls, tls.Namespace, tls.Name), }) } return mappings } +// 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 (d *ConflictDetector) getSecretInfo(ctx context.Context, nn types.NamespacedName) (*secretInfo, error) { if nn.Name == "" || nn.Namespace == "" { return nil, fmt.Errorf("secret namespaced name is incomplete: %s", nn) @@ -324,10 +351,10 @@ func (d *ConflictDetector) resolveGatewayProxy(ctx context.Context, obj client.O } } -func (d *ConflictDetector) findExternalConflicts(ctx context.Context, obj client.Object, gatewayProxy *v1alpha1.GatewayProxy, hosts map[string]string) ([]SSLConflict, error) { +func (d *ConflictDetector) findExternalConflicts(ctx context.Context, obj client.Object, gatewayProxy *v1alpha1.GatewayProxy, newMappings map[string]HostCertMapping) ([]SSLConflict, error) { excludeUID := obj.GetUID() - hostValues := make([]string, 0, len(hosts)) - for host := range hosts { + hostValues := make([]string, 0, len(newMappings)) + for host := range newMappings { hostValues = append(hostValues, host) } sort.Strings(hostValues) @@ -338,24 +365,54 @@ func (d *ConflictDetector) findExternalConflicts(ctx context.Context, obj client var noHostCandidates []client.Object noHostFetched := false + var allTLSResources []client.Object + allFetched := false for _, host := range hostValues { - candidates, err := d.listResourcesByHost(ctx, host) - if err != nil { - logger.Error(err, "failed to list resources by host", "host", host) - return nil, err - } - if host != "" { - if !noHostFetched { - // List resources with empty host. - noHostCandidates, err = d.listResourcesByHost(ctx, "") + var ( + candidates []client.Object + err error + ) + if strings.HasPrefix(host, "*.") { + // A wildcard covers many exact hosts; the host index can't answer a + // suffix query, so enumerate all TLS resources and filter by overlap. + if !allFetched { + allTLSResources, err = d.listAllTLSResources(ctx) + if err != nil { + logger.Error(err, "failed to list TLS resources", "object", objectKey(obj)) + return nil, err + } + allFetched = true + } + candidates = allTLSResources + } else { + candidates, err = d.listResourcesByHost(ctx, host) + if err != nil { + logger.Error(err, "failed to list resources by host", "host", host) + return nil, err + } + // Also look up a covering wildcard, which is indexed under a + // different key (e.g. "*.example.com" for host "app.example.com"). + if parent := sslutil.ParentWildcard(host); parent != "" { + wildcardCandidates, err := d.listResourcesByHost(ctx, parent) if err != nil { - logger.Error(err, "failed to list resources by host", "host", "", "object", objectKey(obj)) + logger.Error(err, "failed to list resources by host", "host", parent) return nil, err } - noHostFetched = true + candidates = mergeCandidateObjects(candidates, wildcardCandidates) + } + if host != "" { + if !noHostFetched { + // List resources with empty host. + noHostCandidates, err = d.listResourcesByHost(ctx, "") + if err != nil { + logger.Error(err, "failed to list resources by host", "host", "", "object", objectKey(obj)) + return nil, err + } + noHostFetched = true + } + candidates = mergeCandidateObjects(candidates, noHostCandidates) } - candidates = mergeCandidateObjects(candidates, noHostCandidates) } for _, candidate := range candidates { if candidate.GetUID() == excludeUID { @@ -376,8 +433,12 @@ func (d *ConflictDetector) findExternalConflicts(ctx context.Context, obj client if !ok { continue } - // same cert hash, no conflict - if mapping.CertificateHash == hosts[host] { + // Same server cert AND same mTLS client config: no conflict. A + // differing client config is still a conflict even when the server + // cert matches. + newMapping := newMappings[host] + if mapping.CertificateHash == newMapping.CertificateHash && + mapping.ClientConfigHash == newMapping.ClientConfigHash { continue } @@ -410,6 +471,39 @@ func (d *ConflictDetector) findExternalConflicts(ctx context.Context, obj client return results, nil } +// listAllTLSResources enumerates every TLS-bearing Gateway, Ingress and +// ApisixTls. Used when the incoming host is a wildcard, whose covered exact +// hosts can't be resolved through the exact-key host index. +func (d *ConflictDetector) listAllTLSResources(ctx context.Context) ([]client.Object, error) { + results := make([]client.Object, 0) + + var gatewayList gatewayv1.GatewayList + if err := d.client.List(ctx, &gatewayList); err != nil { + return nil, err + } + for i := range gatewayList.Items { + results = append(results, gatewayList.Items[i].DeepCopy()) + } + + var ingressList networkingv1.IngressList + if err := d.client.List(ctx, &ingressList); err != nil { + return nil, err + } + for i := range ingressList.Items { + results = append(results, ingressList.Items[i].DeepCopy()) + } + + var tlsList apiv2.ApisixTlsList + if err := d.client.List(ctx, &tlsList); err != nil { + return nil, err + } + for i := range tlsList.Items { + results = append(results, tlsList.Items[i].DeepCopy()) + } + + return results, nil +} + func (d *ConflictDetector) listResourcesByHost(ctx context.Context, host string) ([]client.Object, error) { results := make([]client.Object, 0) @@ -478,7 +572,12 @@ func (d *ConflictDetector) mappingForHostWithCache(ctx context.Context, obj clie } for _, mapping := range mappings { - if mapping.Host == host { + if mapping.Host == "" { + continue + } + // Wildcard-aware: an exact host and a covering wildcard overlap even + // though their index keys differ ("app.example.com" vs "*.example.com"). + if sslutil.HostsOverlap(mapping.Host, host) { return mapping, true } } diff --git a/internal/webhook/v1/ssl/conflict_detector_test.go b/internal/webhook/v1/ssl/conflict_detector_test.go index d9d3063f..c511f034 100644 --- a/internal/webhook/v1/ssl/conflict_detector_test.go +++ b/internal/webhook/v1/ssl/conflict_detector_test.go @@ -31,7 +31,9 @@ import ( networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -386,6 +388,193 @@ func TestConflictDetectorDetectsSelfConflict(t *testing.T) { } } +// newApisixTls builds an ApisixTls bound to the shared testIngressClass. +func newApisixTls(name, uid string, hosts []string, secret *corev1.Secret) *apiv2.ApisixTls { + hostTypes := make([]apiv2.HostType, 0, len(hosts)) + for _, h := range hosts { + hostTypes = append(hostTypes, apiv2.HostType(h)) + } + return &apiv2.ApisixTls{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: testNamespace, + UID: types.UID(uid), + }, + Spec: apiv2.ApisixTlsSpec{ + IngressClassName: testIngressClass, + Hosts: hostTypes, + Secret: apiv2.ApisixSecret{ + Name: secret.Name, + Namespace: secret.Namespace, + }, + }, + } +} + +func buildConflictClient(t *testing.T, objs ...client.Object) client.Client { + t.Helper() + scheme := buildScheme(t) + return fake.NewClientBuilder(). + WithScheme(scheme). + WithIndex(&gatewayv1.Gateway{}, indexer.ParametersRef, indexer.GatewayParametersRefIndexFunc). + WithIndex(&gatewayv1.Gateway{}, indexer.TLSHostIndexRef, indexer.GatewayTLSHostIndexFunc). + WithIndex(&networkingv1.IngressClass{}, indexer.IngressClassParametersRef, indexer.IngressClassParametersRefIndexFunc). + WithIndex(&networkingv1.Ingress{}, indexer.IngressClassRef, indexer.IngressClassRefIndexFunc). + WithIndex(&networkingv1.Ingress{}, indexer.TLSHostIndexRef, indexer.IngressTLSHostIndexFunc). + WithIndex(&apiv2.ApisixTls{}, indexer.IngressClassRef, indexer.ApisixTlsIngressClassIndexFunc). + WithIndex(&apiv2.ApisixTls{}, indexer.TLSHostIndexRef, indexer.ApisixTlsHostIndexFunc). + WithObjects(objs...). + Build() +} + +func sharedIngressClassFixtures() (*v1alpha1.GatewayProxy, *networkingv1.IngressClass) { + gatewayProxy := &v1alpha1.GatewayProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "demo-gp", + Namespace: testNamespace, + UID: "gatewayproxy-uid-wildcard", + }, + } + ingressClass := &networkingv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{Name: testIngressClass}, + Spec: networkingv1.IngressClassSpec{ + Controller: config.ControllerConfig.ControllerName, + Parameters: &networkingv1.IngressClassParametersReference{ + APIGroup: ptr.To(v1alpha1.GroupVersion.Group), + Kind: internaltypes.KindGatewayProxy, + Name: gatewayProxy.Name, + Namespace: ptr.To(testNamespace), + }, + }, + } + return gatewayProxy, ingressClass +} + +// TestConflictDetectorDetectsExactOverlappingWildcard verifies an incoming +// exact host conflicts with an existing covering wildcard, even though the two +// are indexed under different keys. +func TestConflictDetectorDetectsExactOverlappingWildcard(t *testing.T) { + existingSecret := newTLSSecret(t, "existing-cert", []string{"*.example.com"}) + incomingSecret := newTLSSecret(t, "incoming-cert", []string{"app.example.com"}) + gatewayProxy, ingressClass := sharedIngressClassFixtures() + + existing := newApisixTls("existing", "existing-uid", []string{"*.example.com"}, existingSecret) + incoming := newApisixTls("incoming", "incoming-uid", []string{"app.example.com"}, incomingSecret) + + c := buildConflictClient(t, existingSecret, incomingSecret, gatewayProxy, ingressClass, existing) + detector := NewConflictDetector(c) + + conflicts := detector.DetectConflicts(context.Background(), incoming) + if len(conflicts) != 1 { + t.Fatalf("expected 1 conflict, got %d: %+v", len(conflicts), conflicts) + } + if conflicts[0].Host != "app.example.com" { + t.Fatalf("unexpected host: %s", conflicts[0].Host) + } + expectedRef := fmt.Sprintf("ApisixTls/%s/%s", existing.Namespace, existing.Name) + if conflicts[0].ConflictingResource != expectedRef { + t.Fatalf("unexpected conflicting resource: %s", conflicts[0].ConflictingResource) + } +} + +// TestConflictDetectorDetectsWildcardOverlappingExact verifies the inverse: an +// incoming wildcard conflicts with an existing exact host it covers. +func TestConflictDetectorDetectsWildcardOverlappingExact(t *testing.T) { + existingSecret := newTLSSecret(t, "existing-cert", []string{"app.example.com"}) + incomingSecret := newTLSSecret(t, "incoming-cert", []string{"*.example.com"}) + gatewayProxy, ingressClass := sharedIngressClassFixtures() + + existing := newApisixTls("existing", "existing-uid", []string{"app.example.com"}, existingSecret) + incoming := newApisixTls("incoming", "incoming-uid", []string{"*.example.com"}, incomingSecret) + + c := buildConflictClient(t, existingSecret, incomingSecret, gatewayProxy, ingressClass, existing) + detector := NewConflictDetector(c) + + conflicts := detector.DetectConflicts(context.Background(), incoming) + if len(conflicts) != 1 { + t.Fatalf("expected 1 conflict, got %d: %+v", len(conflicts), conflicts) + } + if conflicts[0].Host != "*.example.com" { + t.Fatalf("unexpected host: %s", conflicts[0].Host) + } +} + +// TestConflictDetectorAllowsNonOverlappingWildcard guards against false +// positives: a wildcard and an unrelated exact host must not conflict. +func TestConflictDetectorAllowsNonOverlappingWildcard(t *testing.T) { + existingSecret := newTLSSecret(t, "existing-cert", []string{"a.b.example.com"}) + incomingSecret := newTLSSecret(t, "incoming-cert", []string{"*.example.com"}) + gatewayProxy, ingressClass := sharedIngressClassFixtures() + + existing := newApisixTls("existing", "existing-uid", []string{"a.b.example.com"}, existingSecret) + incoming := newApisixTls("incoming", "incoming-uid", []string{"*.example.com"}, incomingSecret) + + c := buildConflictClient(t, existingSecret, incomingSecret, gatewayProxy, ingressClass, existing) + detector := NewConflictDetector(c) + + conflicts := detector.DetectConflicts(context.Background(), incoming) + if len(conflicts) != 0 { + t.Fatalf("expected no conflict (multi-label host not covered by wildcard), got %+v", conflicts) + } +} + +// newApisixTlsMTLS builds an ApisixTls with an optional mTLS client config. +func newApisixTlsMTLS(name, uid string, hosts []string, secret *corev1.Secret, client *apiv2.ApisixMutualTlsClientConfig) *apiv2.ApisixTls { + tls := newApisixTls(name, uid, hosts, secret) + tls.Spec.Client = client + return tls +} + +// TestConflictDetectorDetectsDifferingMTLS verifies an incoming resource with +// the same host and server cert but no mTLS conflicts with an existing resource +// that enforces mTLS. +func TestConflictDetectorDetectsDifferingMTLS(t *testing.T) { + sharedSecret := newTLSSecret(t, "shared-cert", []string{"api.example.com"}) + gatewayProxy, ingressClass := sharedIngressClassFixtures() + + mtls := &apiv2.ApisixMutualTlsClientConfig{ + CASecret: apiv2.ApisixSecret{Name: "client-ca", Namespace: testNamespace}, + Depth: 1, + } + existing := newApisixTlsMTLS("existing", "existing-uid", []string{"api.example.com"}, sharedSecret, mtls) + incoming := newApisixTlsMTLS("incoming", "incoming-uid", []string{"api.example.com"}, sharedSecret, nil) + + c := buildConflictClient(t, sharedSecret, gatewayProxy, ingressClass, existing) + detector := NewConflictDetector(c) + + conflicts := detector.DetectConflicts(context.Background(), incoming) + if len(conflicts) != 1 { + t.Fatalf("expected 1 conflict (differing mTLS config), got %d: %+v", len(conflicts), conflicts) + } + if conflicts[0].Host != "api.example.com" { + t.Fatalf("unexpected host: %s", conflicts[0].Host) + } +} + +// TestConflictDetectorAllowsIdenticalMTLS guards against false positives: same +// host, same server cert, same mTLS client config must not conflict. +func TestConflictDetectorAllowsIdenticalMTLS(t *testing.T) { + sharedSecret := newTLSSecret(t, "shared-cert", []string{"api.example.com"}) + gatewayProxy, ingressClass := sharedIngressClassFixtures() + + mtls := func() *apiv2.ApisixMutualTlsClientConfig { + return &apiv2.ApisixMutualTlsClientConfig{ + CASecret: apiv2.ApisixSecret{Name: "client-ca", Namespace: testNamespace}, + Depth: 1, + } + } + existing := newApisixTlsMTLS("existing", "existing-uid", []string{"api.example.com"}, sharedSecret, mtls()) + incoming := newApisixTlsMTLS("incoming", "incoming-uid", []string{"api.example.com"}, sharedSecret, mtls()) + + c := buildConflictClient(t, sharedSecret, gatewayProxy, ingressClass, existing) + detector := NewConflictDetector(c) + + conflicts := detector.DetectConflicts(context.Background(), incoming) + if len(conflicts) != 0 { + t.Fatalf("expected no conflict (identical mTLS config), got %+v", conflicts) + } +} + func generateCertificate(t *testing.T, hosts []string) ([]byte, []byte) { t.Helper() priv, err := rsa.GenerateKey(rand.Reader, 2048) From 338395a99f2882713514daaf030ce6cc4422e377 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Sun, 19 Jul 2026 12:06:44 +0545 Subject: [PATCH 2/2] fix: warn when ApisixTls certificate does not cover declared SNI hosts 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 with no signal, so external clients would receive a certificate invalid for the requested host. APISIX serves whatever certificate is configured for an SNI regardless of its SANs (the client validates), so this is advisory rather than fatal: log a warning listing the uncovered hosts and the certificate SANs, using the wildcard-aware sslutil.HostCoveredBy (a wildcard SAN covers single-label subdomains, an exact SAN covers only itself). No warning when the cert declares no DNS SANs or can't be parsed. --- internal/adc/translator/apisixtls.go | 33 ++++++++ internal/adc/translator/apisixtls_test.go | 94 +++++++++++++++++++++++ internal/ssl/util.go | 19 +++++ internal/ssl/util_test.go | 22 ++++++ 4 files changed, 168 insertions(+) create mode 100644 internal/adc/translator/apisixtls_test.go diff --git a/internal/adc/translator/apisixtls.go b/internal/adc/translator/apisixtls.go index ea428711..84783dd8 100644 --- a/internal/adc/translator/apisixtls.go +++ b/internal/adc/translator/apisixtls.go @@ -50,6 +50,14 @@ func (t *Translator) TranslateApisixTls(tctx *provider.TranslateContext, tls *ap return nil, err } + // APISIX serves the cert regardless of SAN, so this is advisory: warn when a + // declared host isn't covered by the cert SANs (clients may reject it). + if uncovered, sans := uncoveredSNIHosts(cert, tls.Spec.Hosts); len(uncovered) > 0 { + t.Log.Info("ApisixTls certificate does not cover all declared SNI hosts", + "namespace", tls.Namespace, "name", tls.Name, + "uncoveredHosts", uncovered, "certificateSANs", sans) + } + // Convert hosts to strings snis := make([]string, len(tls.Spec.Hosts)) for i, host := range tls.Spec.Hosts { @@ -97,3 +105,28 @@ func (t *Translator) TranslateApisixTls(tctx *provider.TranslateContext, tls *ap result.SSL = append(result.SSL, ssl) return result, nil } + +// uncoveredSNIHosts returns the declared hosts not covered by any DNS SAN in the +// certificate (wildcard-aware), plus the cert SANs for logging. Returns nothing +// when the cert declares no DNS SANs or can't be parsed, since coverage can't be +// judged there. +func uncoveredSNIHosts(cert []byte, hosts []apiv2.HostType) (uncovered []string, sans []string) { + sans, err := sslutils.ExtractHostsFromCertificate(cert) + if err != nil || len(sans) == 0 { + return nil, nil + } + for _, h := range hosts { + host := string(h) + covered := false + for _, san := range sans { + if sslutils.HostCoveredBy(host, san) { + covered = true + break + } + } + if !covered { + uncovered = append(uncovered, host) + } + } + return uncovered, sans +} diff --git a/internal/adc/translator/apisixtls_test.go b/internal/adc/translator/apisixtls_test.go new file mode 100644 index 00000000..6e49e36a --- /dev/null +++ b/internal/adc/translator/apisixtls_test.go @@ -0,0 +1,94 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package translator + +import ( + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "slices" + "testing" + "time" + + apiv2 "github.com/apache/apisix-ingress-controller/api/v2" +) + +func genCertWithSANs(t *testing.T, sans []string) []byte { + t.Helper() + priv, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("failed to generate key: %v", err) + } + serial, err := rand.Int(rand.Reader, big.NewInt(1<<62)) + if err != nil { + t.Fatalf("failed to generate serial: %v", err) + } + cn := "test" + if len(sans) > 0 { + cn = sans[0] + } + template := &x509.Certificate{ + SerialNumber: serial, + Subject: pkix.Name{CommonName: cn}, + DNSNames: sans, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + } + der, err := x509.CreateCertificate(rand.Reader, template, template, &priv.PublicKey, priv) + if err != nil { + t.Fatalf("failed to create cert: %v", err) + } + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) +} + +func hostTypes(hosts ...string) []apiv2.HostType { + out := make([]apiv2.HostType, 0, len(hosts)) + for _, h := range hosts { + out = append(out, apiv2.HostType(h)) + } + return out +} + +func TestUncoveredSNIHosts(t *testing.T) { + cases := []struct { + name string + sans []string + hosts []string + wantUncovered []string + }{ + {"exact match", []string{"shop.example.com"}, []string{"shop.example.com"}, nil}, + {"wildcard covers subdomain", []string{"*.example.com"}, []string{"shop.example.com"}, nil}, + {"declared host not in SANs", []string{"internal.corp.local"}, []string{"shop.example.com"}, []string{"shop.example.com"}}, + {"one of many uncovered", []string{"a.example.com"}, []string{"a.example.com", "b.example.com"}, []string{"b.example.com"}}, + {"no DNS SANs is lenient", nil, []string{"shop.example.com"}, nil}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + uncovered, _ := uncoveredSNIHosts(genCertWithSANs(t, c.sans), hostTypes(c.hosts...)) + if !slices.Equal(uncovered, c.wantUncovered) { + t.Fatalf("uncoveredSNIHosts() = %v, want %v", uncovered, c.wantUncovered) + } + }) + } +} diff --git a/internal/ssl/util.go b/internal/ssl/util.go index ee86fec8..be2c8c74 100644 --- a/internal/ssl/util.go +++ b/internal/ssl/util.go @@ -218,6 +218,25 @@ func wildcardCovers(wildcard, host string) bool { return label != "" && !strings.Contains(label, ".") } +// HostCoveredBy reports whether a certificate SAN covers an SNI host. Coverage +// is directional: an exact SAN covers only the identical host; a wildcard SAN +// "*.suffix" covers single-label subdomains "