nic-6443 commented on code in PR #2810:
URL:
https://github.com/apache/apisix-ingress-controller/pull/2810#discussion_r3700913356
##########
internal/webhook/v1/ssl/conflict_detector.go:
##########
@@ -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 &&
Review Comment:
The skip-regex hash collision is fixed, but this diagnostic is still
unchanged: `FormatConflicts` says “different certificate” when only
`ClientConfigHash` differs. Please make the warning generic or report which TLS
component differs.
##########
internal/webhook/v1/ssl/conflict_detector.go:
##########
@@ -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, ","))
Review Comment:
Fixed by the length-prefixed regex encoding and its collision regression
test. This concern is addressed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
