From ca0a5721b962eec59b85099f5f0ce2773aaead67 Mon Sep 17 00:00:00 2001 From: Sanju98 Date: Wed, 13 May 2026 22:34:52 +0530 Subject: [PATCH 1/3] Add SPIFFE v2 URI-SAN based principal extraction (DEPEND-89172) Adds server-side support for SPIFFE-issued client certificates in X509 auth. The principal is the ILM UID (path-after-/v2/), aligned with the LinkedIn ILM v2 design: trust-domain stripped, segment-prefix matching for ACL lookup. Key changes: - X509AuthenticationUtil.matchAndExtractSpiffeSAN extracts the ILM UID from v2 SPIFFE URIs. v1 SPIFFE URIs and user-identity URIs (/v/user/...) fall through to existing URN / Subject-DN extraction. - X509AuthenticationConfig adds spiffe.sanMatchRegex as the operator- controlled trust-domain gate. - ZkClientUriDomainMappingHelper recursively walks the znode subtree below each domain. Only leaf znodes are registered as keys (path joined by '/'), letting multi-segment SPIFFE UIDs be expressed as nested znodes (whose names can't contain '/'). getDomains does exact-match then segment-prefix walk-up. clientUriToDomainNames is volatile with in-method snapshot. - Defense-in-depth: SPIFFE path extraction uses URI.getRawPath() and rejects any path containing '%' to prevent URL-decoding bypass of identity checks. Tests: 29/29 pass (X509AuthTest 13, X509SpiffeAuthIntegrationTest 6, ZkClientUriDomainMappingHelperTest 10; includes end-to-end SPIFFE-cert through X509ZNodeGroupAclProvider with real znode mapping). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../server/auth/X509AuthenticationConfig.java | 40 ++++ .../server/auth/X509AuthenticationUtil.java | 142 ++++++++++- .../ZkClientUriDomainMappingHelper.java | 139 ++++++++--- .../zookeeper/common/SpiffeAuthTestUtil.java | 128 ++++++++++ .../zookeeper/common/X509TestHelpers.java | 36 ++- .../ZkClientUriDomainMappingHelperTest.java | 200 +++++++++++++++- .../apache/zookeeper/test/X509AuthTest.java | 225 +++++++++++++++++- .../test/X509SpiffeAuthIntegrationTest.java | 139 +++++++++++ 8 files changed, 990 insertions(+), 59 deletions(-) create mode 100644 zookeeper-server/src/test/java/org/apache/zookeeper/common/SpiffeAuthTestUtil.java create mode 100644 zookeeper-server/src/test/java/org/apache/zookeeper/test/X509SpiffeAuthIntegrationTest.java diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationConfig.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationConfig.java index 5d4eb9b40b7..ff1b80af44d 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationConfig.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationConfig.java @@ -24,6 +24,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.Set; +import java.util.regex.Pattern; import java.util.stream.Collectors; import org.apache.zookeeper.server.auth.znode.groupacl.X509ZNodeGroupAclProvider; import org.slf4j.Logger; @@ -83,6 +84,24 @@ public static X509AuthenticationConfig getInstance() { public static final String SSL_X509_CLIENT_CERT_ID_SAN_EXTRACT_MATCHER_GROUP_INDEX = SSL_X509_CONFIG_PREFIX + "clientCertIdSanExtractMatcherGroupIndex"; public static final String SUBJECT_ALTERNATIVE_NAME_SHORT = "SAN"; + + /** + * Regex to identify SPIFFE URI SANs (type 6). When set, URI SANs (type 6) matching this regex + * are treated as SPIFFE identities; the principal is the path after {@code /v2/} (the ILM UID). + * v1 SPIFFE URIs and user-identity URIs ({@code /v/user/...}) are structurally rejected and + * fall through to URN/DN extraction regardless of this regex. + * If not set, SPIFFE extraction is disabled. + * + *

Recommended: constrain to a specific trust domain and require v2, e.g. + * {@code ^spiffe://prod\.lipki/v2/.*$}. A permissive regex like {@code ^spiffe://.*$} accepts + * SPIFFE URIs from any trust domain, relying on the upstream TLS trust manager alone to reject + * untrusted issuers; non-v2 URIs accepted by such a regex will still fall through. + * + *

ACL matching downstream is segment-prefix on the extracted UID; see + * {@code X509AuthenticationUtil#matchAndExtractSpiffeSAN}. + */ + public static final String SSL_X509_SPIFFE_SAN_MATCH_REGEX = + SSL_X509_CONFIG_PREFIX + "spiffe.sanMatchRegex"; private static final String DEFAULT_REGEX = ".*"; private String clientCertIdType; private int clientCertIdSanMatchType = -1; @@ -90,6 +109,9 @@ public static X509AuthenticationConfig getInstance() { private String clientCertIdSanExtractRegex; private int clientCertIdSanExtractMatcherGroupIndex = -1; + private Pattern spiffeSanMatchPattern; + private boolean spiffeSanMatchPatternLoaded = false; + // ZooKeeper server-side config properties for ZNode group ACL feature /** @@ -226,6 +248,23 @@ public void setClientCertIdSanExtractMatcherGroupIndex( } } + public void setSpiffeSanMatchRegex(String spiffeSanMatchRegex) { + LOG.debug("{} = {}", SSL_X509_SPIFFE_SAN_MATCH_REGEX, spiffeSanMatchRegex); + this.spiffeSanMatchPattern = spiffeSanMatchRegex == null ? null : Pattern.compile(spiffeSanMatchRegex); + this.spiffeSanMatchPatternLoaded = true; + } + + /** + * Compiled SPIFFE SAN match pattern, or null if SPIFFE extraction is not configured. + * Loaded lazily from system properties on first access. + */ + public Pattern getSpiffeSanMatchPattern() { + if (!spiffeSanMatchPatternLoaded) { + setSpiffeSanMatchRegex(System.getProperty(SSL_X509_SPIFFE_SAN_MATCH_REGEX)); + } + return spiffeSanMatchPattern; + } + // Setters for X509 Znode Group Acl properties public void setX509ClientIdAsAclEnabled(String enabled) { @@ -482,4 +521,5 @@ public static void reset() { instance = null; } } + } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationUtil.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationUtil.java index 88ae5a464b9..0eba3c5d315 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationUtil.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationUtil.java @@ -18,11 +18,13 @@ package org.apache.zookeeper.server.auth; +import java.net.URI; import java.security.cert.CertificateException; import java.security.cert.CertificateParsingException; import java.security.cert.X509Certificate; import java.util.Collection; import java.util.List; +import java.util.Optional; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -48,6 +50,19 @@ public class X509AuthenticationUtil extends X509Util { public static final String SUPERUSER_AUTH_SCHEME = "super"; public static final String X509_SCHEME = "x509"; + // Matches LISPIFFE user-identity paths of the form "/v/user" or "/v/user/". + // User-identity SPIFFE certs (issued to humans, not workloads) must NOT be promoted to a + // service principal, otherwise a user credential would be granted service-level ACL access. + // See LISPIFFE-ID spec: https://github.com/linkedin-multiproduct/gopki/blob/master/LISPIFFE-ID.md + private static final Pattern SPIFFE_USER_IDENTITY_PATH_PATTERN = + Pattern.compile("^/v\\d+/user(/.*)?$"); + + // Matches LISPIFFE v2 workload paths and captures the ILM UID (the path after "/v2/"). + // The canonical ILM v2 principal is the full path-after-v2 (e.g. "application/foo-mp/bar-app"); + // ACL matching downstream is segment-prefix on this UID. v1 SPIFFE URIs are deliberately not + // matched here — they are a deprecated design with app-name collision across MPs. + private static final Pattern SPIFFE_V2_PATH_PATTERN = Pattern.compile("^/v2/(.+)$"); + @Override protected String getConfigPrefix() { return X509AuthenticationConfig.SSL_X509_CONFIG_PREFIX; @@ -134,16 +149,127 @@ public static String getClientId(X509Certificate clientCert) { String clientCertIdType = X509AuthenticationConfig.getInstance().getClientCertIdType(); if (clientCertIdType != null && clientCertIdType .equalsIgnoreCase(X509AuthenticationConfig.SUBJECT_ALTERNATIVE_NAME_SHORT)) { + try { + Optional spiffeId = X509AuthenticationUtil.matchAndExtractSpiffeSAN(clientCert); + if (spiffeId.isPresent()) { + LOG.debug("Extracted SPIFFE identity: {}", spiffeId.get()); + return spiffeId.get(); + } + } catch (Exception e) { + LOG.warn("Failed to extract SPIFFE identity from SAN. Falling through to URN-based extraction.", e); + } try { return X509AuthenticationUtil.matchAndExtractSAN(clientCert); } catch (Exception ce) { LOG.warn("Failed to match and extract a client ID from SAN. Using Subject DN instead.", ce); } } - // return Subject DN by default return clientCert.getSubjectX500Principal().getName(); } + /** + * Attempt to extract a client identity from a LISPIFFE v2 URI SAN. Returns the ILM UID — the + * path segment after {@code /v2/} — e.g. {@code spiffe://prod.lipki/v2/application/foo-mp/bar-app} + * yields {@code application/foo-mp/bar-app}. ACL matching downstream is segment-prefix on this + * UID. + * + *

Returns {@link Optional#empty()} when SPIFFE extraction is disabled, no URI SAN matches the + * configured regex, the matched URI is a v1 SPIFFE identity (deprecated; app-name collides + * across MPs), or the matched URI is a user identity ({@code /v/user/...}, which must never + * be promoted to a service principal). Caller falls through to URN/DN extraction. + * + * @throws IllegalArgumentException if multiple URI SANs match the SPIFFE regex + */ + private static Optional matchAndExtractSpiffeSAN(X509Certificate clientCert) + throws CertificateParsingException { + Pattern matchPattern = X509AuthenticationConfig.getInstance().getSpiffeSanMatchPattern(); + if (matchPattern == null) { + return Optional.empty(); + } + + String spiffeUri = findSingleMatchingSan(clientCert, 6, matchPattern, "SPIFFE"); + if (spiffeUri == null) { + return Optional.empty(); + } + + String path; + try { + // getRawPath() returns the literal (un-percent-decoded) path so the principal we accept is + // exactly what the CA validated in the SAN. getPath() would decode %2F → /, allowing a + // single-segment SAN like /v2/foo%2Fbar to be promoted to a multi-segment principal that + // could collide with an unrelated registered identity. Reject any path containing % to + // also block encoded "user" bypass (e.g. /v2/%75ser/alice). + path = URI.create(spiffeUri).getRawPath(); + } catch (IllegalArgumentException e) { + LOG.debug("Malformed SPIFFE URI '{}'; falling through to URN/DN extraction.", spiffeUri); + return Optional.empty(); + } + if (path == null) { + return Optional.empty(); + } + if (path.indexOf('%') >= 0) { + LOG.debug("Rejecting SPIFFE URI with percent-encoded path '{}'; falling through.", spiffeUri); + return Optional.empty(); + } + if (SPIFFE_USER_IDENTITY_PATH_PATTERN.matcher(path).matches()) { + LOG.debug("Rejecting SPIFFE user identity '{}' for service-principal extraction.", spiffeUri); + return Optional.empty(); + } + Matcher v2Matcher = SPIFFE_V2_PATH_PATTERN.matcher(path); + if (!v2Matcher.matches()) { + LOG.debug("SPIFFE URI '{}' is not a v2 identity; falling through to URN/DN extraction.", + spiffeUri); + return Optional.empty(); + } + return Optional.of(v2Matcher.group(1)); + } + + /** + * Returns the single SAN value of the given type whose value matches the regex, or null if + * there are zero matches. Throws if there are multiple matches (callers always want exactly one). + */ + private static String findSingleMatchingSan(X509Certificate cert, int sanType, Pattern pattern, + String matchKind) throws CertificateParsingException { + String found = null; + Collection> sans = cert.getSubjectAlternativeNames(); + if (sans == null) { + return null; + } + for (List san : sans) { + if (!Integer.valueOf(sanType).equals(san.get(0))) { + continue; + } + String value = san.get(1).toString(); + if (!pattern.matcher(value).find()) { + continue; + } + if (found != null) { + String errStr = "Expected exactly 1 " + matchKind + " SAN but found more than 1. " + + "Please fix the match regex so exactly one match is found."; + LOG.error(errStr); + throw new IllegalArgumentException(errStr); + } + found = value; + } + return found; + } + + /** + * Applies an extract regex to a SAN value and returns the captured group. + * + * @throws IllegalArgumentException if the regex does not match. + */ + private static String applyExtractRegex(Pattern extractPattern, String value, int groupIndex) { + Matcher matcher = extractPattern.matcher(value); + if (!matcher.find()) { + String errStr = "Failed to extract identity from '" + value + + "' using regex '" + extractPattern.pattern() + "'"; + LOG.error(errStr); + throw new IllegalArgumentException(errStr); + } + return matcher.group(groupIndex); + } + /** * Extract the authenticated client Id from the specified server connection object. * @param cnxn Server connection object that contains the certificate. @@ -204,18 +330,8 @@ private static String matchAndExtractSAN(X509Certificate clientCert) throw new IllegalArgumentException(errStr); } - // Extract a substring from the found match using extractRegex - Pattern extractPattern = Pattern.compile(extractRegex); - Matcher matcher = extractPattern.matcher(matched.iterator().next().get(1).toString()); - if (matcher.find()) { - // If extractMatcherGroupIndex is not given, return the 1st index by default - String result = matcher.group(extractMatcherGroupIndex); - LOG.debug("Returning extracted client ID: {} using Matcher group index: {}", result, extractMatcherGroupIndex); - return result; - } - String errStr = "Failed to find an extract substring to determine client ID. Please review the extract regex."; - LOG.error(errStr); - throw new IllegalArgumentException(errStr); + return applyExtractRegex(Pattern.compile(extractRegex), + matched.iterator().next().get(1).toString(), extractMatcherGroupIndex); } /** diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/znode/groupacl/ZkClientUriDomainMappingHelper.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/znode/groupacl/ZkClientUriDomainMappingHelper.java index 9061a76cbce..c282097bb7c 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/znode/groupacl/ZkClientUriDomainMappingHelper.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/znode/groupacl/ZkClientUriDomainMappingHelper.java @@ -18,6 +18,7 @@ package org.apache.zookeeper.server.auth.znode.groupacl; +import com.google.common.annotations.VisibleForTesting; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Collections; import java.util.HashMap; @@ -44,16 +45,28 @@ * be cached inside this helper object. This helper object watches the clientUri-domain ZNodes and * updates the internal Map accordingly. * - * The following illustrates the ZNode hierarchy: - * . (root) - * └── /zookeeper/uri-domain-map (mapping root path) - * ├── bar (application domain) - * │ ├── bar0 (client URI) - * │ └── bar1 (client URI) - * └── foo (application domain) - * ├── foo1 (client URI) - * ├── foo2 (client URI) - * └── foo3 (client URI) + * Each leaf znode below a domain is registered as a client URI; the URI is the + * {@code /}-joined path of znode names from the domain down. Since znode names themselves + * cannot contain {@code /}, multi-segment SPIFFE ILM UIDs + * ({@code application//[/]}) are expressed as a path of nested znodes. Intermediate + * znodes are structural only — they are not registered as keys, which prevents a stray + * single-segment znode (e.g. {@code workload}) from matching every SPIFFE workload identity. + * + *

Example tree: + * 

+ * /zookeeper/uri-domain-map
+ * ├── bar
+ * │   └── urn:li:servicePrincipal(bar;ei4;i001)            → "urn:li:servicePrincipal(bar;ei4;i001)" → bar
+ * └── helix
+ *     └── workload
+ *         └── helix-core
+ *             ├── helix-controller                          → "workload/helix-core/helix-controller" → helix
+ *             └── helix-rest                                → "workload/helix-core/helix-rest"       → helix
+ *
+ * + * To grant an MP-level prefix instead, register the MP node as a leaf (i.e. omit app-level + * children); the segment-prefix walk-up in {@link #getDomains(String)} then matches any UID + * with that prefix. * * Note: It is not expected that there would be too many distinct client URIs so as to overwhelm * heap usage. @@ -65,7 +78,10 @@ public class ZkClientUriDomainMappingHelper implements ClientUriDomainMappingHel private final ZooKeeperServer zks; private final String rootPath; - private Map> clientUriToDomainNames = Collections.emptyMap(); + // volatile to publish the reassignment in parseZNodeMapping (watcher thread) to readers in + // getDomains (request-handler threads); see allowedClientIdAsAclDomains in X509AuthenticationConfig + // for the same pattern. + private volatile Map> clientUriToDomainNames = Collections.emptyMap(); private ConnectionAuthInfoUpdater updater = null; public ZkClientUriDomainMappingHelper(ZooKeeperServer zks) { @@ -116,38 +132,101 @@ private void addWatches() { } /** - * Read ZNodes under the root path and populates clientUriToDomainNames. - * Note: this is not thread-safe nor atomic; however, we do not need such strong guarantee with - * this read operation. - * - * Also, note that this is a purely in-memory operation, so re-parsing the entire tree should not - * be a big overhead considering how infrequently the mapping is supposed to be changed. + * Re-read the entire mapping subtree and swap in a new {@code clientUriToDomainNames}. See + * class Javadoc for the registration rule. Runs on bootstrap and on watcher fire (infrequent), + * purely in-memory. Not thread-safe with itself; the volatile reassignment publishes a + * consistent map to readers. */ private void parseZNodeMapping() { Map> newClientUriToDomainNames = new HashMap<>(); try { List domainNames = zks.getZKDatabase().getChildren(rootPath, null, null); - domainNames.forEach(domainName -> { - try { - List clientUris = - zks.getZKDatabase().getChildren(rootPath + "/" + domainName, null, null); - clientUris.forEach(clientUri -> { - LOG.info("Registering client URI domain mapping: {} --> {}", clientUri, domainName); - newClientUriToDomainNames.computeIfAbsent(clientUri, k -> new HashSet<>()).add(domainName); - }); - } catch (KeeperException.NoNodeException e) { - LOG.warn("No clientUri ZNodes found under domain: {}", domainName); - } - }); + for (String domainName : domainNames) { + collectClientUris(rootPath + "/" + domainName, "", domainName, newClientUriToDomainNames); + } } catch (KeeperException.NoNodeException e) { LOG.warn("No application domain ZNodes found in root path: {}", rootPath); } clientUriToDomainNames = newClientUriToDomainNames; } + private void collectClientUris(String currentPath, String accumulatedUri, String domainName, + Map> map) { + List children; + try { + children = zks.getZKDatabase().getChildren(currentPath, null, null); + } catch (KeeperException.NoNodeException e) { + return; + } + if (children.isEmpty()) { + // Only leaf znodes are registered as client URIs. Intermediate znodes are structural — + // registering them would grant the domain to any client whose UID happens to share that + // prefix segment (e.g. registering a 1-segment "workload" key would match every SPIFFE + // workload identity). Operators express grants by creating leaves at the intended depth. + if (!accumulatedUri.isEmpty()) { + LOG.info("Registering client URI domain mapping: {} --> {}", accumulatedUri, domainName); + map.computeIfAbsent(accumulatedUri, k -> new HashSet<>()).add(domainName); + } + return; + } + for (String child : children) { + String childUri = accumulatedUri.isEmpty() ? child : accumulatedUri + "/" + child; + collectClientUris(currentPath + "/" + child, childUri, domainName, map); + } + } + + @VisibleForTesting + void setClientUriToDomainNames(Map> mapping) { + this.clientUriToDomainNames = mapping; + } + + /** + * Resolve the set of application domains for a given client URI. + * + * Lookup proceeds in two stages: + *
    + *
  1. Exact match: if the URI is registered verbatim in the mapping, its domain set + * is returned as-is. URN-style identifiers (e.g. {@code urn:li:servicePrincipal(...)}) + * contain no {@code '/'} and therefore always resolve here or not at all.
    2. + *
  2. Segment-prefix walk-up: if no exact match exists and the URI contains at least + * one {@code '/'}, split on {@code '/'} and probe each strictly-shorter left prefix + * (anchored at the start, aligned on {@code '/'} boundaries). Domains from every prefix + * that is present in the map are unioned into the result. This supports SPIFFE-style ILM + * UIDs of the form {@code application//[/]}, where registering + * {@code application/} covers all of its apps and tags without wildcards.
    3. + *
+ * A {@code null} URI yields the empty set. + */ @Override public Set getDomains(String clientUri) { - return clientUriToDomainNames.getOrDefault(clientUri, Collections.emptySet()); + if (clientUri == null) { + return Collections.emptySet(); + } + // Snapshot the mapping reference once. parseZNodeMapping reassigns the field on watcher + // fire; without this snapshot, the exact-match and the prefix walk-up below could read + // different map references mid-call and return an inconsistent answer. + Map> map = clientUriToDomainNames; + Set exact = map.get(clientUri); + if (exact != null) { + return exact; + } + if (clientUri.indexOf('/') < 0) { + return Collections.emptySet(); + } + String[] segments = clientUri.split("/"); + Set result = new HashSet<>(); + StringBuilder prefix = new StringBuilder(clientUri.length()); + for (int n = 1; n < segments.length; n++) { + if (n > 1) { + prefix.append('/'); + } + prefix.append(segments[n - 1]); + Set match = map.get(prefix.toString()); + if (match != null) { + result.addAll(match); + } + } + return result.isEmpty() ? Collections.emptySet() : result; } @Override diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/SpiffeAuthTestUtil.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/SpiffeAuthTestUtil.java new file mode 100644 index 00000000000..a41f178bf99 --- /dev/null +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/SpiffeAuthTestUtil.java @@ -0,0 +1,128 @@ +/* + * 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 org.apache.zookeeper.common; + +import java.net.Socket; +import java.security.KeyPair; +import java.security.Principal; +import java.security.PrivateKey; +import java.security.Security; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import javax.net.ssl.X509KeyManager; +import javax.net.ssl.X509TrustManager; +import org.apache.zookeeper.server.auth.X509AuthenticationConfig; +import org.bouncycastle.asn1.x500.X500Name; +import org.bouncycastle.asn1.x509.GeneralName; +import org.bouncycastle.asn1.x509.GeneralNames; +import org.bouncycastle.jce.provider.BouncyCastleProvider; + +/** + * Test fixtures for SPIFFE-based authentication: BouncyCastle bootstrap, system-property + * setup/teardown for the {@code spiffe.sanMatchRegex} config, real X509 client cert builder + * with URI SANs, and stub TLS managers. Shared across SPIFFE auth tests. + */ +public final class SpiffeAuthTestUtil { + + public static final long ONE_DAY_MILLIS = 24L * 60 * 60 * 1000; + /** Match regex that accepts only v2 SPIFFE URIs (any trust domain). */ + public static final String SPIFFE_V2_MATCH_REGEX = "^spiffe://.*/v2/.*$"; + + private SpiffeAuthTestUtil() { + } + + public static void registerBouncyCastle() { + if (Security.getProvider(BouncyCastleProvider.PROVIDER_NAME) == null) { + Security.addProvider(new BouncyCastleProvider()); + } + } + + /** Configures SAN+SPIFFE-v2 extraction in the X509AuthenticationConfig singleton. */ + public static void setSpiffeSystemProperties() { + System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_TYPE, "SAN"); + System.setProperty(X509AuthenticationConfig.SSL_X509_SPIFFE_SAN_MATCH_REGEX, SPIFFE_V2_MATCH_REGEX); + X509AuthenticationConfig.reset(); + } + + public static void clearSpiffeSystemProperties() { + System.clearProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_TYPE); + System.clearProperty(X509AuthenticationConfig.SSL_X509_SPIFFE_SAN_MATCH_REGEX); + X509AuthenticationConfig.reset(); + } + + /** + * Builds a real BouncyCastle-signed X509 client certificate with the given URI SANs. Requires + * {@link #registerBouncyCastle()} to have been called once per JVM. + */ + public static X509Certificate buildClientCertWithUriSans(String... uriSans) throws Exception { + KeyPair caKey = X509TestHelpers.generateRSAKeyPair(); + X509Certificate caCert = X509TestHelpers.newSelfSignedCACert( + new X500Name("CN=Test CA"), caKey, ONE_DAY_MILLIS); + KeyPair clientKey = X509TestHelpers.generateRSAKeyPair(); + GeneralName[] names = new GeneralName[uriSans.length]; + for (int i = 0; i < uriSans.length; i++) { + names[i] = new GeneralName(GeneralName.uniformResourceIdentifier, uriSans[i]); + } + return X509TestHelpers.newCertWithSans(caCert, caKey, + new X500Name("CN=test-client"), clientKey.getPublic(), + new GeneralNames(names), ONE_DAY_MILLIS); + } + + /** Trust manager that accepts any cert; for auth-flow tests that don't exercise trust validation. */ + public static final class AcceptAllTrustManager implements X509TrustManager { + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException { + } + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException { + } + @Override + public X509Certificate[] getAcceptedIssuers() { + return new X509Certificate[0]; + } + } + + /** Key manager that returns null for everything; for tests that don't serve outbound TLS. */ + public static final class NoopKeyManager implements X509KeyManager { + @Override + public String chooseClientAlias(String[] keyType, Principal[] issuers, Socket socket) { + return null; + } + @Override + public String chooseServerAlias(String keyType, Principal[] issuers, Socket socket) { + return null; + } + @Override + public X509Certificate[] getCertificateChain(String alias) { + return null; + } + @Override + public String[] getClientAliases(String keyType, Principal[] issuers) { + return null; + } + @Override + public PrivateKey getPrivateKey(String alias) { + return null; + } + @Override + public String[] getServerAliases(String keyType, Principal[] issuers) { + return null; + } + } +} diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestHelpers.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestHelpers.java index b9f2f6db946..68e2ee372a7 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestHelpers.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestHelpers.java @@ -130,19 +130,37 @@ now, new Date(now.getTime() */ public static X509Certificate newCert( X509Certificate caCert, KeyPair caKeyPair, X500Name certSubject, PublicKey certPublicKey, long expirationMillis) throws IOException, OperatorCreationException, GeneralSecurityException { + return newCertSignedBy(caCert, caKeyPair, certSubject, certPublicKey, + getLocalhostSubjectAltNames(), expirationMillis); + } + + /** + * Variant of {@link #newCert} that installs the supplied SANs instead of the default + * localhost SANs. Used by tests that need certs with specific URI SANs (e.g., SPIFFE URIs). + */ + public static X509Certificate newCertWithSans( + X509Certificate caCert, KeyPair caKeyPair, X500Name certSubject, PublicKey certPublicKey, + GeneralNames sans, long expirationMillis) + throws IOException, OperatorCreationException, GeneralSecurityException { + return newCertSignedBy(caCert, caKeyPair, certSubject, certPublicKey, sans, expirationMillis); + } + + private static X509Certificate newCertSignedBy( + X509Certificate caCert, KeyPair caKeyPair, X500Name certSubject, PublicKey certPublicKey, + GeneralNames sans, long expirationMillis) + throws IOException, OperatorCreationException, GeneralSecurityException { if (!caKeyPair.getPublic().equals(caCert.getPublicKey())) { throw new IllegalArgumentException("CA private key does not match the public key in the CA cert"); } Date now = new Date(); - X509v3CertificateBuilder builder = initCertBuilder(new X500Name(caCert.getIssuerDN().getName()), now, new Date( - now.getTime() - + expirationMillis), certSubject, certPublicKey); - builder.addExtension(Extension.basicConstraints, true, new BasicConstraints(false)); // not a CA - builder.addExtension(Extension.keyUsage, true, new KeyUsage(KeyUsage.digitalSignature - | KeyUsage.keyEncipherment)); - builder.addExtension(Extension.extendedKeyUsage, true, new ExtendedKeyUsage(new KeyPurposeId[]{KeyPurposeId.id_kp_serverAuth, KeyPurposeId.id_kp_clientAuth})); - - builder.addExtension(Extension.subjectAlternativeName, false, getLocalhostSubjectAltNames()); + X509v3CertificateBuilder builder = initCertBuilder(new X500Name(caCert.getIssuerDN().getName()), + now, new Date(now.getTime() + expirationMillis), certSubject, certPublicKey); + builder.addExtension(Extension.basicConstraints, true, new BasicConstraints(false)); + builder.addExtension(Extension.keyUsage, true, + new KeyUsage(KeyUsage.digitalSignature | KeyUsage.keyEncipherment)); + builder.addExtension(Extension.extendedKeyUsage, true, + new ExtendedKeyUsage(new KeyPurposeId[]{KeyPurposeId.id_kp_serverAuth, KeyPurposeId.id_kp_clientAuth})); + builder.addExtension(Extension.subjectAlternativeName, false, sans); return buildAndSignCertificate(caKeyPair.getPrivate(), builder); } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/znode/groupacl/ZkClientUriDomainMappingHelperTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/znode/groupacl/ZkClientUriDomainMappingHelperTest.java index 5e8c8996a6c..4850a37760a 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/znode/groupacl/ZkClientUriDomainMappingHelperTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/znode/groupacl/ZkClientUriDomainMappingHelperTest.java @@ -19,9 +19,13 @@ package org.apache.zookeeper.server.auth.znode.groupacl; import java.io.IOException; +import java.security.cert.X509Certificate; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; +import java.util.Map; +import java.util.Set; import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.DummyWatcher; import org.apache.zookeeper.KeeperException; @@ -29,14 +33,18 @@ import org.apache.zookeeper.ZKTestCase; import org.apache.zookeeper.ZooDefs; import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.common.SpiffeAuthTestUtil; +import org.apache.zookeeper.server.MockServerCnxn; import org.apache.zookeeper.server.ServerCnxn; import org.apache.zookeeper.server.ServerCnxnFactory; import org.apache.zookeeper.server.ZooKeeperServer; +import org.apache.zookeeper.server.auth.ServerAuthenticationProvider; import org.apache.zookeeper.server.watch.WatchesReport; import org.apache.zookeeper.test.ClientBase; import org.junit.After; import org.junit.Assert; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.FixMethodOrder; import org.junit.Test; import org.junit.runners.MethodSorters; @@ -58,13 +66,28 @@ public class ZkClientUriDomainMappingHelperTest extends ZKTestCase { CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/foo", CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/foo/foo1", CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/foo/foo2", - CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/foo/bar1" + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/foo/bar1", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-apps", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-apps/workload", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-apps/workload/helix-core", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-apps/workload/helix-core/helix-controller", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-apps/workload/helix-core/helix-rest", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-mp", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-mp/workload", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-mp/workload/different-mp", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-legacy", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-legacy/urn:li:servicePrincipal(legacy;ei4;i001)" }; private ZooKeeperServer zookeeperServer; private ZooKeeper zookeeperClientConnection; private ServerCnxnFactory serverCnxnFactory; + @BeforeClass + public static void registerBouncyCastle() { + SpiffeAuthTestUtil.registerBouncyCastle(); + } + @Before public void setUp() throws IOException, InterruptedException, KeeperException { LOG.info("Starting Zk..."); @@ -157,6 +180,108 @@ public void testA_ZkClientUriDomainMappingHelper() throws KeeperException, Inter Assert.assertEquals(new HashSet<>(Arrays.asList("bar", "foo")), helper.getDomains("bar1")); } + /** + * Verifies the helper recursively walks multi-level znode subtrees. SPIFFE ILM UIDs include + * {@code /} separators (which can't appear in znode names), so they're encoded as a path of + * nested znodes; only leaves are registered as keys. The mapping tree: + * 
+   * helix-apps/workload/helix-core/helix-controller        → "workload/helix-core/helix-controller" → helix-apps
+   * helix-apps/workload/helix-core/helix-rest              → "workload/helix-core/helix-rest"       → helix-apps
+   * helix-mp/workload/different-mp                         → "workload/different-mp"                → helix-mp
+   * helix-legacy/urn:li:servicePrincipal(legacy;ei4;i001)  → "urn:li:..."                           → helix-legacy
+   *
+ */ + @Test + public void testA2_RecursiveZNodeWalkRegistersMultiLevelClientUris() + throws KeeperException, InterruptedException { + String[] paths = { + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH, + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-apps", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-apps/workload", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-apps/workload/helix-core", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-apps/workload/helix-core/helix-controller", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-apps/workload/helix-core/helix-rest", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-mp", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-mp/workload", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-mp/workload/different-mp", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-legacy", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-legacy/urn:li:servicePrincipal(legacy;ei4;i001)" + }; + for (String path : paths) { + zookeeperClientConnection.create(path, null, ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + } + + ClientUriDomainMappingHelper helper = new ZkClientUriDomainMappingHelper(zookeeperServer); + + // App-level leaf: exact match + Assert.assertEquals(Collections.singleton("helix-apps"), + helper.getDomains("workload/helix-core/helix-controller")); + + // App-level leaf: walk-up from a deeper UID (e.g. app + tag) + Assert.assertEquals(Collections.singleton("helix-apps"), + helper.getDomains("workload/helix-core/helix-rest/ltx1-tag")); + + // MP-level leaf: walk-up grants the domain to any app under that MP + Assert.assertEquals(Collections.singleton("helix-mp"), + helper.getDomains("workload/different-mp/any-app/any-tag")); + + // Legacy URN entry resolves via exact match + Assert.assertEquals(Collections.singleton("helix-legacy"), + helper.getDomains("urn:li:servicePrincipal(legacy;ei4;i001)")); + + // No MP-level grant for helix-core, so an unregistered sibling app does NOT match + Assert.assertEquals(Collections.emptySet(), + helper.getDomains("workload/helix-core/unknown-app")); + + // Intermediate znode ("workload") is structural only — not registered as a 1-segment key + Assert.assertEquals(Collections.emptySet(), + helper.getDomains("workload/unrelated-mp/unrelated-app")); + } + + /** + * End-to-end: a real SPIFFE v2 client certificate flowing through + * {@link X509ZNodeGroupAclProvider#handleAuthentication} resolves to the correct + * {@code (x509, )} authInfo entry via the recursive znode mapping. + */ + @Test + public void testA3_SpiffeCertResolvesThroughZNodeMappingToDomainAuthInfo() throws Exception { + String[] paths = { + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH, + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-apps", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-apps/workload", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-apps/workload/helix-core", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-apps/workload/helix-core/helix-controller" + }; + for (String path : paths) { + zookeeperClientConnection.create(path, null, ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + } + + SpiffeAuthTestUtil.setSpiffeSystemProperties(); + try { + X509Certificate cert = SpiffeAuthTestUtil.buildClientCertWithUriSans( + "spiffe://prod.lipki/v2/workload/helix-core/helix-controller"); + + X509ZNodeGroupAclProvider provider = new X509ZNodeGroupAclProvider( + new SpiffeAuthTestUtil.AcceptAllTrustManager(), new SpiffeAuthTestUtil.NoopKeyManager()); + + MockServerCnxn cnxn = new MockServerCnxn(); + cnxn.clientChain = new X509Certificate[]{cert}; + + KeeperException.Code result = provider.handleAuthentication( + new ServerAuthenticationProvider.ServerObjs(zookeeperServer, cnxn), null); + + Assert.assertEquals(KeeperException.Code.OK, result); + + boolean foundDomain = cnxn.getAuthInfo().stream() + .anyMatch(id -> "x509".equals(id.getScheme()) && "helix-apps".equals(id.getId())); + Assert.assertTrue( + "Expected (x509, helix-apps) in authInfo; actual: " + cnxn.getAuthInfo(), + foundDomain); + } finally { + SpiffeAuthTestUtil.clearSpiffeSystemProperties(); + } + } + @Test /** * Make sure the watcher installed while instantiate ZkClientUriDomainMappingHelper does not break @@ -167,4 +292,77 @@ public void testB_GetWatches() { WatchesReport report = zookeeperServer.getZKDatabase().getDataTree().getWatches(); Assert.assertEquals(1, report.getPaths(0).size()); } + + @Test + public void testC_GetDomainsExactMatch() { + ZkClientUriDomainMappingHelper helper = new ZkClientUriDomainMappingHelper(zookeeperServer); + Map> mapping = new HashMap<>(); + mapping.put("workload/foo-mp/bar-app", + new HashSet<>(Collections.singletonList("foo-domain"))); + setMapping(helper, mapping); + + Assert.assertEquals(Collections.singleton("foo-domain"), + helper.getDomains("workload/foo-mp/bar-app")); + } + + @Test + public void testC_GetDomainsSegmentPrefixWalkUpSingleMatch() { + ZkClientUriDomainMappingHelper helper = new ZkClientUriDomainMappingHelper(zookeeperServer); + Map> mapping = new HashMap<>(); + mapping.put("workload/foo-mp", new HashSet<>(Collections.singletonList("foo-domain"))); + setMapping(helper, mapping); + + Assert.assertEquals(Collections.singleton("foo-domain"), + helper.getDomains("workload/foo-mp/bar-app")); + } + + @Test + public void testC_GetDomainsSegmentPrefixWalkUpMultiSegmentUnion() { + ZkClientUriDomainMappingHelper helper = new ZkClientUriDomainMappingHelper(zookeeperServer); + Map> mapping = new HashMap<>(); + mapping.put("workload/foo-mp", new HashSet<>(Collections.singletonList("mp-domain"))); + mapping.put("workload/foo-mp/bar-app", + new HashSet<>(Collections.singletonList("app-domain"))); + setMapping(helper, mapping); + + Assert.assertEquals(new HashSet<>(Arrays.asList("mp-domain", "app-domain")), + helper.getDomains("workload/foo-mp/bar-app/some-tag")); + } + + @Test + public void testC_GetDomainsSegmentAlignmentIsStrict() { + ZkClientUriDomainMappingHelper helper = new ZkClientUriDomainMappingHelper(zookeeperServer); + Map> mapping = new HashMap<>(); + mapping.put("workload/foo-mp", new HashSet<>(Collections.singletonList("foo-domain"))); + setMapping(helper, mapping); + + Assert.assertEquals(Collections.emptySet(), helper.getDomains("workload/foo-mp-extra")); + } + + @Test + public void testC_GetDomainsUrnStyleNoWalkUp() { + ZkClientUriDomainMappingHelper helper = new ZkClientUriDomainMappingHelper(zookeeperServer); + Map> mapping = new HashMap<>(); + mapping.put("urn:li:servicePrincipal(bar;ei4;i001)", + new HashSet<>(Collections.singletonList("bar-domain"))); + setMapping(helper, mapping); + + Assert.assertEquals(Collections.emptySet(), + helper.getDomains("urn:li:servicePrincipal(foo;ei4;i001)")); + } + + @Test + public void testC_GetDomainsNullClientUri() { + ZkClientUriDomainMappingHelper helper = new ZkClientUriDomainMappingHelper(zookeeperServer); + Map> mapping = new HashMap<>(); + mapping.put("workload/foo-mp", new HashSet<>(Collections.singletonList("foo-domain"))); + setMapping(helper, mapping); + + Assert.assertEquals(Collections.emptySet(), helper.getDomains(null)); + } + + private static void setMapping(ZkClientUriDomainMappingHelper helper, + Map> mapping) { + helper.setClientUriToDomainNames(mapping); + } } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/X509AuthTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/X509AuthTest.java index 618d79a2ae2..92ab8030668 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/X509AuthTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/X509AuthTest.java @@ -46,6 +46,7 @@ import javax.security.auth.x500.X500Principal; import org.apache.zookeeper.KeeperException; import org.apache.zookeeper.ZKTestCase; +import org.apache.zookeeper.common.SpiffeAuthTestUtil; import org.apache.zookeeper.server.MockServerCnxn; import org.apache.zookeeper.server.auth.X509AuthenticationConfig; import org.apache.zookeeper.server.auth.X509AuthenticationProvider; @@ -131,6 +132,210 @@ public void testSANBasedAuth() { X509AuthenticationConfig.reset(); } + // SPIFFE test fixtures + private static final String SPIFFE_V1_URI = "spiffe://prod.lipki/v1/wl/espresso-router"; + private static final String SPIFFE_V2_URI = "spiffe://prod.lipki/v2/application/espresso-router/espresso-router"; + + @Test + public void testSpiffeV1FallsBackToDn() { + SpiffeAuthTestUtil.setSpiffeSystemProperties(); + try { + TestCertificate spiffeCert = new TestCertificate("CLIENT", SPIFFE_V1_URI); + X509AuthenticationProvider provider = createProvider(spiffeCert); + MockServerCnxn cnxn = new MockServerCnxn(); + cnxn.clientChain = new X509Certificate[]{spiffeCert}; + + assertEquals(KeeperException.Code.OK, provider.handleAuthentication(cnxn, null)); + assertEquals("CN=CLIENT", cnxn.getAuthInfo().get(0).getId()); + } finally { + SpiffeAuthTestUtil.clearSpiffeSystemProperties(); + } + } + + @Test + public void testSpiffeV2Auth() { + SpiffeAuthTestUtil.setSpiffeSystemProperties(); + try { + TestCertificate spiffeCert = new TestCertificate("CLIENT", SPIFFE_V2_URI); + X509AuthenticationProvider provider = createProvider(spiffeCert); + MockServerCnxn cnxn = new MockServerCnxn(); + cnxn.clientChain = new X509Certificate[]{spiffeCert}; + + assertEquals(KeeperException.Code.OK, provider.handleAuthentication(cnxn, null)); + assertEquals("application/espresso-router/espresso-router", + cnxn.getAuthInfo().get(0).getId()); + } finally { + SpiffeAuthTestUtil.clearSpiffeSystemProperties(); + } + } + + @Test + public void testSpiffeV2WorkloadAuth() { + SpiffeAuthTestUtil.setSpiffeSystemProperties(); + try { + String spiffeWorkloadUri = "spiffe://prod.lipki/v2/workload/foo-mp/bar-app/some-tag"; + TestCertificate spiffeCert = new TestCertificate("CLIENT", spiffeWorkloadUri); + X509AuthenticationProvider provider = createProvider(spiffeCert); + MockServerCnxn cnxn = new MockServerCnxn(); + cnxn.clientChain = new X509Certificate[]{spiffeCert}; + + assertEquals(KeeperException.Code.OK, provider.handleAuthentication(cnxn, null)); + assertEquals("workload/foo-mp/bar-app/some-tag", cnxn.getAuthInfo().get(0).getId()); + } finally { + SpiffeAuthTestUtil.clearSpiffeSystemProperties(); + } + } + + @Test + public void testSpiffeNotConfiguredFallsBackToUrn() { + // SPIFFE regex NOT set — should fall through to URN-based SAN extraction + String urnSan = "urn:li:servicePrincipal(espresso-router;ei4;i001)"; + System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_TYPE, "SAN"); + System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_MATCH_TYPE, "6"); + System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_MATCH_REGEX, "^.*urn:li:.*$"); + System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_EXTRACT_REGEX, + "^.*urn:li:([a-z]+Principal\\([^;%:]+)"); + System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_EXTRACT_MATCHER_GROUP_INDEX, "1"); + // SSL_X509_SPIFFE_SAN_MATCH_REGEX intentionally NOT set + + try { + TestCertificate urnCert = new TestCertificate("CLIENT", urnSan); + X509AuthenticationProvider provider = createProvider(urnCert); + MockServerCnxn cnxn = new MockServerCnxn(); + cnxn.clientChain = new X509Certificate[]{urnCert}; + + assertEquals(KeeperException.Code.OK, provider.handleAuthentication(cnxn, null)); + assertEquals("servicePrincipal(espresso-router", cnxn.getAuthInfo().get(0).getId()); + } finally { + System.clearProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_TYPE); + System.clearProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_MATCH_TYPE); + System.clearProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_MATCH_REGEX); + System.clearProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_EXTRACT_REGEX); + System.clearProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_EXTRACT_MATCHER_GROUP_INDEX); + X509AuthenticationConfig.reset(); + } + } + + @Test + public void testSpiffeConfiguredButNoSpiffeSanFallsBackToUrn() { + // SPIFFE regex set, but cert has URN SAN (not SPIFFE) → SPIFFE returns empty → falls back to URN + String urnSan = "urn:li:servicePrincipal(espresso-router;ei4;i001)"; + System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_TYPE, "SAN"); + System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_MATCH_TYPE, "6"); + System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_MATCH_REGEX, "^.*urn:li:.*$"); + System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_EXTRACT_REGEX, + "^.*urn:li:([a-z]+Principal\\([^;%:]+)"); + System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_EXTRACT_MATCHER_GROUP_INDEX, "1"); + System.setProperty(X509AuthenticationConfig.SSL_X509_SPIFFE_SAN_MATCH_REGEX, SpiffeAuthTestUtil.SPIFFE_V2_MATCH_REGEX); + + try { + TestCertificate urnCert = new TestCertificate("CLIENT", urnSan); + X509AuthenticationProvider provider = createProvider(urnCert); + MockServerCnxn cnxn = new MockServerCnxn(); + cnxn.clientChain = new X509Certificate[]{urnCert}; + + assertEquals(KeeperException.Code.OK, provider.handleAuthentication(cnxn, null)); + // URN SAN doesn't match spiffe://, so falls through to URN extraction + assertEquals("servicePrincipal(espresso-router", cnxn.getAuthInfo().get(0).getId()); + } finally { + System.clearProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_TYPE); + System.clearProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_MATCH_TYPE); + System.clearProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_MATCH_REGEX); + System.clearProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_EXTRACT_REGEX); + System.clearProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_EXTRACT_MATCHER_GROUP_INDEX); + System.clearProperty(X509AuthenticationConfig.SSL_X509_SPIFFE_SAN_MATCH_REGEX); + X509AuthenticationConfig.reset(); + } + } + + @Test + public void testSpiffeV2UserIdentityRejected() { + // SPIFFE user identity (/v2/user/) must NOT be mapped to a service principal. + // It should be silently rejected by the SPIFFE extractor, fall through to URN (no match), + // then fall back to Subject DN. + String spiffeUserUri = "spiffe://prod.lipki/v2/user/alice"; + SpiffeAuthTestUtil.setSpiffeSystemProperties(); + try { + TestCertificate userCert = new TestCertificate("CLIENT", spiffeUserUri); + X509AuthenticationProvider provider = createProvider(userCert); + MockServerCnxn cnxn = new MockServerCnxn(); + cnxn.clientChain = new X509Certificate[]{userCert}; + + assertEquals(KeeperException.Code.OK, provider.handleAuthentication(cnxn, null)); + assertEquals("CN=CLIENT", cnxn.getAuthInfo().get(0).getId()); + } finally { + SpiffeAuthTestUtil.clearSpiffeSystemProperties(); + } + } + + @Test + public void testSpiffeV1UserIdentityRejected() { + // v1 user-identity now falls back to DN for two reasons: user-identity rejection AND v1 rejection. + String spiffeUserUri = "spiffe://prod.lipki/v1/user/alice"; + SpiffeAuthTestUtil.setSpiffeSystemProperties(); + try { + TestCertificate userCert = new TestCertificate("CLIENT", spiffeUserUri); + X509AuthenticationProvider provider = createProvider(userCert); + MockServerCnxn cnxn = new MockServerCnxn(); + cnxn.clientChain = new X509Certificate[]{userCert}; + + assertEquals(KeeperException.Code.OK, provider.handleAuthentication(cnxn, null)); + assertEquals("CN=CLIENT", cnxn.getAuthInfo().get(0).getId()); + } finally { + SpiffeAuthTestUtil.clearSpiffeSystemProperties(); + } + } + + @Test + public void testSpiffeMultipleSpiffeSansFallsBackToDn() { + // Cert with >1 SPIFFE SAN — extractor throws, caught in getClientId, falls through to URN + // (not configured) then to Subject DN. + SpiffeAuthTestUtil.setSpiffeSystemProperties(); + try { + TestCertificate multiSanCert = new TestCertificate("CLIENT", + Arrays.asList(SPIFFE_V2_URI, "spiffe://prod.lipki/v2/application/another-service/another-service")); + X509AuthenticationProvider provider = createProvider(multiSanCert); + MockServerCnxn cnxn = new MockServerCnxn(); + cnxn.clientChain = new X509Certificate[]{multiSanCert}; + + assertEquals(KeeperException.Code.OK, provider.handleAuthentication(cnxn, null)); + assertEquals("CN=CLIENT", cnxn.getAuthInfo().get(0).getId()); + } finally { + SpiffeAuthTestUtil.clearSpiffeSystemProperties(); + } + } + + @Test + public void testSpiffePrefersSpiffeOverUrnWhenBothPresent() { + // Cert has BOTH a URN-format SAN and a SPIFFE SAN. SPIFFE must win. + String urnSan = "urn:li:servicePrincipal(legacy-app;ei4;i001)"; + System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_TYPE, "SAN"); + System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_MATCH_TYPE, "6"); + System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_MATCH_REGEX, "^.*urn:li:.*$"); + System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_EXTRACT_REGEX, + "^.*urn:li:([a-z]+Principal\\([^;%:]+)"); + System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_EXTRACT_MATCHER_GROUP_INDEX, "1"); + System.setProperty(X509AuthenticationConfig.SSL_X509_SPIFFE_SAN_MATCH_REGEX, SpiffeAuthTestUtil.SPIFFE_V2_MATCH_REGEX); + + try { + TestCertificate mixedCert = new TestCertificate("CLIENT", Arrays.asList(urnSan, SPIFFE_V2_URI)); + X509AuthenticationProvider provider = createProvider(mixedCert); + MockServerCnxn cnxn = new MockServerCnxn(); + cnxn.clientChain = new X509Certificate[]{mixedCert}; + + assertEquals(KeeperException.Code.OK, provider.handleAuthentication(cnxn, null)); + // SPIFFE wins — path-after-/v2/, not the URN-derived legacy-app id. + assertEquals("application/espresso-router/espresso-router", + cnxn.getAuthInfo().get(0).getId()); + } finally { + System.clearProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_MATCH_TYPE); + System.clearProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_MATCH_REGEX); + System.clearProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_EXTRACT_REGEX); + System.clearProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_EXTRACT_MATCHER_GROUP_INDEX); + SpiffeAuthTestUtil.clearSpiffeSystemProperties(); + } + } + protected static class TestPublicKey implements PublicKey { private static final long serialVersionUID = 1L; @@ -155,17 +360,21 @@ public static class TestCertificate extends X509Certificate { private byte[] encoded; private X500Principal principal; private PublicKey publicKey; - private String subjectAlternativeName; + private List subjectAlternativeNames; public TestCertificate(String name) { this(name, TEST_SAN_STR); } public TestCertificate(String name, String sanVal) { + this(name, Collections.singletonList(sanVal)); + } + + public TestCertificate(String name, List sanVals) { encoded = name.getBytes(); principal = new X500Principal("CN=" + name); publicKey = new TestPublicKey(); - subjectAlternativeName = sanVal; + subjectAlternativeNames = sanVals; } @Override public boolean hasUnsupportedCriticalExtension() { @@ -273,10 +482,14 @@ public X500Principal getSubjectX500Principal() { } @Override public Collection> getSubjectAlternativeNames() { - List subjectAlternativeNamePair = new ArrayList<>(); - subjectAlternativeNamePair.add(6); - subjectAlternativeNamePair.add(subjectAlternativeName); - return Collections.singletonList(subjectAlternativeNamePair); + List> result = new ArrayList<>(); + for (String san : subjectAlternativeNames) { + List pair = new ArrayList<>(); + pair.add(6); + pair.add(san); + result.add(pair); + } + return result; } } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/X509SpiffeAuthIntegrationTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/X509SpiffeAuthIntegrationTest.java new file mode 100644 index 00000000000..ca44ab16063 --- /dev/null +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/X509SpiffeAuthIntegrationTest.java @@ -0,0 +1,139 @@ +/* + * 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 org.apache.zookeeper.test; + +import static org.junit.Assert.assertEquals; +import java.security.cert.X509Certificate; +import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.ZKTestCase; +import org.apache.zookeeper.common.SpiffeAuthTestUtil; +import org.apache.zookeeper.server.MockServerCnxn; +import org.apache.zookeeper.server.auth.X509AuthenticationProvider; +import org.junit.After; +import org.junit.BeforeClass; +import org.junit.Test; + +/** + * Integration tests for SPIFFE SAN-based client identity extraction. Unlike + * {@link X509AuthTest}, which uses a hand-rolled mock cert, these tests construct REAL + * {@link X509Certificate} instances (BouncyCastle-signed) with SPIFFE URI SANs and run them + * through the full {@link X509AuthenticationProvider#handleAuthentication} path. This exercises + * the JDK's actual SAN parsing, which the mock cert bypasses. + */ +public class X509SpiffeAuthIntegrationTest extends ZKTestCase { + + @BeforeClass + public static void registerBouncyCastle() { + SpiffeAuthTestUtil.registerBouncyCastle(); + } + + @After + public void tearDown() { + SpiffeAuthTestUtil.clearSpiffeSystemProperties(); + } + + @Test + public void testRealCertWithSpiffeV1UriSanFallsBackToSubjectDn() throws Exception { + SpiffeAuthTestUtil.setSpiffeSystemProperties(); + X509Certificate cert = SpiffeAuthTestUtil.buildClientCertWithUriSans( + "spiffe://prod.lipki/v1/wl/espresso-router"); + + String id = runAuth(cert); + + assertEquals(cert.getSubjectX500Principal().getName(), id); + } + + @Test + public void testRealCertWithSpiffeV2UriSanIsExtracted() throws Exception { + SpiffeAuthTestUtil.setSpiffeSystemProperties(); + X509Certificate cert = SpiffeAuthTestUtil.buildClientCertWithUriSans( + "spiffe://prod.lipki/v2/application/espresso-router/espresso-router"); + + String id = runAuth(cert); + + assertEquals("application/espresso-router/espresso-router", id); + } + + @Test + public void testRealCertWithSpiffeUserUriFallsBackToSubjectDn() throws Exception { + SpiffeAuthTestUtil.setSpiffeSystemProperties(); + X509Certificate cert = SpiffeAuthTestUtil.buildClientCertWithUriSans( + "spiffe://prod.lipki/v2/user/alice"); + + String id = runAuth(cert); + + // User identity is rejected for service-principal extraction; falls through to URN + // (not configured here) and then to Subject DN. + assertEquals(cert.getSubjectX500Principal().getName(), id); + } + + @Test + public void testRealCertWithoutSpiffeSanFallsBackToSubjectDn() throws Exception { + SpiffeAuthTestUtil.setSpiffeSystemProperties(); + // URI SAN that is not a SPIFFE URI — should not match the SPIFFE regex. + X509Certificate cert = SpiffeAuthTestUtil.buildClientCertWithUriSans( + "urn:li:servicePrincipal(legacy-app;ei4;i001)"); + + String id = runAuth(cert); + + // No SPIFFE match, URN extraction not configured here, so falls to Subject DN. + assertEquals(cert.getSubjectX500Principal().getName(), id); + } + + /** + * Defense-in-depth: a SAN whose single literal path segment contains {@code %2F} must not + * be silently promoted to a multi-segment principal via URI decoding (which could collide + * with an unrelated legitimate identity). Falls through to Subject DN. + */ + @Test + public void testRealCertWithPercentEncodedSlashInPathFallsBackToSubjectDn() throws Exception { + SpiffeAuthTestUtil.setSpiffeSystemProperties(); + X509Certificate cert = SpiffeAuthTestUtil.buildClientCertWithUriSans( + "spiffe://prod.lipki/v2/application%2Ffoo-mp%2Fbar-app"); + + String id = runAuth(cert); + + assertEquals(cert.getSubjectX500Principal().getName(), id); + } + + /** + * Defense-in-depth: a percent-encoded "user" segment ({@code %75ser}) must not bypass the + * user-identity rejection. Falls through to Subject DN. + */ + @Test + public void testRealCertWithPercentEncodedUserPathFallsBackToSubjectDn() throws Exception { + SpiffeAuthTestUtil.setSpiffeSystemProperties(); + X509Certificate cert = SpiffeAuthTestUtil.buildClientCertWithUriSans( + "spiffe://prod.lipki/v2/%75ser/alice"); + + String id = runAuth(cert); + + assertEquals(cert.getSubjectX500Principal().getName(), id); + } + + private static String runAuth(X509Certificate cert) { + X509AuthenticationProvider provider = new X509AuthenticationProvider( + new SpiffeAuthTestUtil.AcceptAllTrustManager(), + new SpiffeAuthTestUtil.NoopKeyManager()); + MockServerCnxn cnxn = new MockServerCnxn(); + cnxn.clientChain = new X509Certificate[]{cert}; + assertEquals(KeeperException.Code.OK, provider.handleAuthentication(cnxn, null)); + return cnxn.getAuthInfo().get(0).getId(); + } +} From 45a37760895ec7694b072cb0f210556cbcd0a6c1 Mon Sep 17 00:00:00 2001 From: Sanju98 Date: Thu, 14 May 2026 11:47:39 +0530 Subject: [PATCH 2/3] Address review: thread-safe SPIFFE config + wire prefix walk-up to production path Two fixes addressing the code review on PR #142: 1. X509AuthenticationConfig.getSpiffeSanMatchPattern now uses double-checked locking with volatile fields and a dedicated lock object, matching the pattern already used by allowedClientIdAsAclDomains and other lazy-loaded fields in the same class. The previous lazy-init was a data race on the per-handshake hot path. 2. X509ZNodeGroupAclProvider's setDomainAuthUpdater lambda now calls helper.getDomains(clientId) instead of the raw map's getOrDefault, so the segment-prefix walk-up added to ZkClientUriDomainMappingHelper is reachable from the production authentication path. This is the znode-tree analogue of LinkedIn's documented acl-tool wildcard idiom (`--spiffe "application//*"`); without this fix, MP-level prefix grants documented in the class javadoc would silently no-op. Adds testA4_SpiffeCertResolvesViaPrefixWalkUpToDomainAuthInfo: real SPIFFE cert with a 4-segment principal resolves to an MP-level leaf grant through the full provider->helper.getDomains path. This test would have failed before fix #2 (the exact-match lookup misses the 4-segment principal against a 2-segment registered prefix). All 30 SPIFFE-related tests pass (X509AuthTest 13 + X509SpiffeAuthIntegrationTest 6 + ZkClientUriDomainMappingHelperTest 11). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../server/auth/X509AuthenticationConfig.java | 16 ++++-- .../groupacl/X509ZNodeGroupAclProvider.java | 10 ++-- .../ZkClientUriDomainMappingHelperTest.java | 53 ++++++++++++++++++- 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationConfig.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationConfig.java index ff1b80af44d..82e51e4beeb 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationConfig.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationConfig.java @@ -109,8 +109,8 @@ public static X509AuthenticationConfig getInstance() { private String clientCertIdSanExtractRegex; private int clientCertIdSanExtractMatcherGroupIndex = -1; - private Pattern spiffeSanMatchPattern; - private boolean spiffeSanMatchPatternLoaded = false; + private volatile Pattern spiffeSanMatchPattern; + private volatile boolean spiffeSanMatchPatternLoaded = false; // ZooKeeper server-side config properties for ZNode group ACL feature @@ -196,6 +196,7 @@ public static X509AuthenticationConfig getInstance() { private final Object crossDomainAccessDomainsLock = new Object(); private final Object znodeGroupAclSuperUserIdsLock = new Object(); private final Object allowedClientIdAsAclDomainsLock = new Object(); + private final Object spiffeSanMatchPatternLock = new Object(); // Setters for X509 properties @@ -256,11 +257,18 @@ public void setSpiffeSanMatchRegex(String spiffeSanMatchRegex) { /** * Compiled SPIFFE SAN match pattern, or null if SPIFFE extraction is not configured. - * Loaded lazily from system properties on first access. + * Loaded lazily from system properties on first access using double-checked locking against + * {@code spiffeSanMatchPatternLock}, matching the pattern used by other lazy-loaded fields in + * this class (e.g. {@link #getAllowedClientIdAsAclDomains()}). */ + @SuppressFBWarnings("DC_DOUBLECHECK") public Pattern getSpiffeSanMatchPattern() { if (!spiffeSanMatchPatternLoaded) { - setSpiffeSanMatchRegex(System.getProperty(SSL_X509_SPIFFE_SAN_MATCH_REGEX)); + synchronized (spiffeSanMatchPatternLock) { + if (!spiffeSanMatchPatternLoaded) { + setSpiffeSanMatchRegex(System.getProperty(SSL_X509_SPIFFE_SAN_MATCH_REGEX)); + } + } } return spiffeSanMatchPattern; } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/znode/groupacl/X509ZNodeGroupAclProvider.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/znode/groupacl/X509ZNodeGroupAclProvider.java index 34869b83c4f..688a99479e1 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/znode/groupacl/X509ZNodeGroupAclProvider.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/znode/groupacl/X509ZNodeGroupAclProvider.java @@ -19,7 +19,6 @@ package org.apache.zookeeper.server.auth.znode.groupacl; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -164,11 +163,14 @@ private ClientUriDomainMappingHelper getUriDomainMappingHelper(ZooKeeperServer z // Set up AuthInfo updater to refresh connection AuthInfo on any client domain changes. // TODO Making the anonymous class to a separate updater implementation class if any other Acl provider shares // the same logic. - helper.setDomainAuthUpdater((cnxn, clientUriToDomainNames) -> { + // Route through helper.getDomains(clientId) so SPIFFE multi-segment principals resolve + // via the segment-prefix walk-up (operator can register an MP-level leaf to grant all + // apps under that MP; see ZkClientUriDomainMappingHelper class javadoc). The map passed + // into the lambda is ignored — kept in the interface signature for backward compat. + helper.setDomainAuthUpdater((cnxn, ignoredMap) -> { try { String clientId = X509AuthenticationUtil.getClientId(cnxn, trustManager); - assignAuthInfo(cnxn, clientId, - clientUriToDomainNames.getOrDefault(clientId, Collections.emptySet())); + assignAuthInfo(cnxn, clientId, helper.getDomains(clientId)); } catch (UnsupportedOperationException unsupportedEx) { LOG.info("Cannot update AuthInfo for session 0x{} since the operation is not supported.", Long.toHexString(cnxn.getSessionId())); diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/znode/groupacl/ZkClientUriDomainMappingHelperTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/znode/groupacl/ZkClientUriDomainMappingHelperTest.java index 4850a37760a..bc3997dae2a 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/znode/groupacl/ZkClientUriDomainMappingHelperTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/znode/groupacl/ZkClientUriDomainMappingHelperTest.java @@ -76,7 +76,10 @@ public class ZkClientUriDomainMappingHelperTest extends ZKTestCase { CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-mp/workload", CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-mp/workload/different-mp", CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-legacy", - CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-legacy/urn:li:servicePrincipal(legacy;ei4;i001)" + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-legacy/urn:li:servicePrincipal(legacy;ei4;i001)", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-mp-grant", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-mp-grant/application", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-mp-grant/application/helix-core" }; private ZooKeeperServer zookeeperServer; @@ -282,6 +285,54 @@ public void testA3_SpiffeCertResolvesThroughZNodeMappingToDomainAuthInfo() throw } } + /** + * End-to-end: a SPIFFE v2 client cert whose principal is a multi-segment ILM UID resolves to + * the correct {@code (x509, )} authInfo via the segment-prefix walk-up — the + * operator registered only the MP-level leaf, not the full app-level path. This mirrors the + * canonical LinkedIn ACL idiom (e.g. {@code acl-tool ... --spiffe "application//*"}) and + * ensures the prefix-walk-up is reachable from the production authentication path. + */ + @Test + public void testA4_SpiffeCertResolvesViaPrefixWalkUpToDomainAuthInfo() throws Exception { + String[] paths = { + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH, + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-mp-grant", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-mp-grant/application", + CLIENT_URI_DOMAIN_MAPPING_ROOT_PATH + "/helix-mp-grant/application/helix-core" + }; + for (String path : paths) { + zookeeperClientConnection.create(path, null, ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + } + + SpiffeAuthTestUtil.setSpiffeSystemProperties(); + try { + // Cert's path-after-/v2/ is "application/helix-core/helix-controller/ltx1-tag" (4 segments), + // but the operator only registered the 2-segment leaf "application/helix-core". The walk-up + // must hit that prefix and grant the helix-mp-grant domain. + X509Certificate cert = SpiffeAuthTestUtil.buildClientCertWithUriSans( + "spiffe://prod.lipki/v2/application/helix-core/helix-controller/ltx1-tag"); + + X509ZNodeGroupAclProvider provider = new X509ZNodeGroupAclProvider( + new SpiffeAuthTestUtil.AcceptAllTrustManager(), new SpiffeAuthTestUtil.NoopKeyManager()); + + MockServerCnxn cnxn = new MockServerCnxn(); + cnxn.clientChain = new X509Certificate[]{cert}; + + KeeperException.Code result = provider.handleAuthentication( + new ServerAuthenticationProvider.ServerObjs(zookeeperServer, cnxn), null); + + Assert.assertEquals(KeeperException.Code.OK, result); + + boolean foundDomain = cnxn.getAuthInfo().stream() + .anyMatch(id -> "x509".equals(id.getScheme()) && "helix-mp-grant".equals(id.getId())); + Assert.assertTrue( + "Expected (x509, helix-mp-grant) in authInfo via prefix walk-up; actual: " + cnxn.getAuthInfo(), + foundDomain); + } finally { + SpiffeAuthTestUtil.clearSpiffeSystemProperties(); + } + } + @Test /** * Make sure the watcher installed while instantiate ZkClientUriDomainMappingHelper does not break From 4a0533b95c1220a93c249644c4d44ca240a3f362 Mon Sep 17 00:00:00 2001 From: Sanju98 Date: Fri, 15 May 2026 10:25:56 +0530 Subject: [PATCH 3/3] Address review: accept SPIFFE v1 workload certs alongside v2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per @rgodha's review comment, the extractor now accepts both SPIFFE versions instead of rejecting v1 outright: - v2 (existing): spiffe:///v2/ → principal is the full path after /v2/ (the ILM UID, e.g. "application/foo-mp/bar-app"). - v1 workload (new): spiffe:///v1/wl/ → strip the "wl/" type prefix; principal is just the app-name. This matches how legacy authZ handled v1 identities. Other v1 paths (e.g. v1/wf/ workflow) still fall through to URN/DN. User-identity URIs (/v/user/...) continue to be rejected for both versions — they must never be promoted to a service principal. The test match regex broadens to ^spiffe://.*/v[12]/.*$ so existing test scaffolding exercises both versions. testSpiffeV1FallsBackToDn becomes testSpiffeV1WlAuth (now asserts extraction). The integration test for v1 likewise flips from fall-back-to-DN to v1/wl extraction. LISPIFFE-ID spec reference: https://github.com/linkedin-multiproduct/gopki/blob/master/LISPIFFE-ID.md#2-uri-path Co-Authored-By: Claude Opus 4.7 (1M context) --- .../server/auth/X509AuthenticationConfig.java | 20 +++++---- .../server/auth/X509AuthenticationUtil.java | 44 ++++++++++++------- .../zookeeper/common/SpiffeAuthTestUtil.java | 6 +-- .../apache/zookeeper/test/X509AuthTest.java | 10 +++-- .../test/X509SpiffeAuthIntegrationTest.java | 6 ++- 5 files changed, 54 insertions(+), 32 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationConfig.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationConfig.java index 82e51e4beeb..76a4459e418 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationConfig.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationConfig.java @@ -86,16 +86,20 @@ public static X509AuthenticationConfig getInstance() { public static final String SUBJECT_ALTERNATIVE_NAME_SHORT = "SAN"; /** - * Regex to identify SPIFFE URI SANs (type 6). When set, URI SANs (type 6) matching this regex - * are treated as SPIFFE identities; the principal is the path after {@code /v2/} (the ILM UID). - * v1 SPIFFE URIs and user-identity URIs ({@code /v/user/...}) are structurally rejected and - * fall through to URN/DN extraction regardless of this regex. - * If not set, SPIFFE extraction is disabled. + * Regex to identify SPIFFE URI SANs (type 6). When set, URI SANs matching this regex are + * treated as SPIFFE identities. The extractor accepts both: + *
    + *
  • v2 ({@code /v2/}): principal is the full ILM UID (path-after-{@code /v2/})
    • + *
  • v1 workload ({@code /v1/wl/}): principal is just {@code } + * (the {@code wl/} type prefix is stripped)
    • + *
+ * User-identity URIs ({@code /v/user/...}) and other non-{v1/wl,v2} paths fall through to + * URN/DN extraction regardless of this regex. If not set, SPIFFE extraction is disabled. * - *

Recommended: constrain to a specific trust domain and require v2, e.g. - * {@code ^spiffe://prod\.lipki/v2/.*$}. A permissive regex like {@code ^spiffe://.*$} accepts + *

Recommended: constrain to a specific trust domain, e.g. + * {@code ^spiffe://prod\.lipki/v[12]/.*$}. A permissive regex like {@code ^spiffe://.*$} accepts * SPIFFE URIs from any trust domain, relying on the upstream TLS trust manager alone to reject - * untrusted issuers; non-v2 URIs accepted by such a regex will still fall through. + * untrusted issuers. * *

ACL matching downstream is segment-prefix on the extracted UID; see * {@code X509AuthenticationUtil#matchAndExtractSpiffeSAN}. diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationUtil.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationUtil.java index 0eba3c5d315..63c31f91b78 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationUtil.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationUtil.java @@ -57,12 +57,17 @@ public class X509AuthenticationUtil extends X509Util { private static final Pattern SPIFFE_USER_IDENTITY_PATH_PATTERN = Pattern.compile("^/v\\d+/user(/.*)?$"); - // Matches LISPIFFE v2 workload paths and captures the ILM UID (the path after "/v2/"). + // Matches LISPIFFE v2 paths and captures the ILM UID (the path after "/v2/"). // The canonical ILM v2 principal is the full path-after-v2 (e.g. "application/foo-mp/bar-app"); - // ACL matching downstream is segment-prefix on this UID. v1 SPIFFE URIs are deliberately not - // matched here — they are a deprecated design with app-name collision across MPs. + // ACL matching downstream is segment-prefix on this UID. private static final Pattern SPIFFE_V2_PATH_PATTERN = Pattern.compile("^/v2/(.+)$"); + // Matches LISPIFFE v1 workload paths (only "/v1/wl/..."; not "/v1/wf/..." workflow) and + // captures the app-name. Per LISPIFFE-ID spec, the v1 workload unique-identity is + // "wl/"; we strip the "wl/" type prefix and return just the app-name as the + // principal, matching how legacy authZ systems handled v1 identities. + private static final Pattern SPIFFE_V1_WL_PATH_PATTERN = Pattern.compile("^/v1/wl/(.+)$"); + @Override protected String getConfigPrefix() { return X509AuthenticationConfig.SSL_X509_CONFIG_PREFIX; @@ -168,15 +173,20 @@ public static String getClientId(X509Certificate clientCert) { } /** - * Attempt to extract a client identity from a LISPIFFE v2 URI SAN. Returns the ILM UID — the - * path segment after {@code /v2/} — e.g. {@code spiffe://prod.lipki/v2/application/foo-mp/bar-app} - * yields {@code application/foo-mp/bar-app}. ACL matching downstream is segment-prefix on this - * UID. + * Attempt to extract a client identity from a LISPIFFE URI SAN. Supported forms: + *

    + *
  • v2 ({@code spiffe:///v2/}): principal is the full path-after-{@code /v2/} + * (the ILM UID), e.g. {@code spiffe://prod.lipki/v2/application/foo-mp/bar-app} → + * {@code application/foo-mp/bar-app}. ACL matching downstream is segment-prefix on the UID.
    • + *
  • v1 workload ({@code spiffe:///v1/wl/}): principal is just the + * {@code } (the "wl/" type prefix is stripped, matching how legacy authZ + * handled v1 identities).
    • + *
* *

Returns {@link Optional#empty()} when SPIFFE extraction is disabled, no URI SAN matches the - * configured regex, the matched URI is a v1 SPIFFE identity (deprecated; app-name collides - * across MPs), or the matched URI is a user identity ({@code /v/user/...}, which must never - * be promoted to a service principal). Caller falls through to URN/DN extraction. + * configured regex, the matched URI is a user identity ({@code /v/user/...}, which must + * never be promoted to a service principal), or the matched URI is a non-{v1/wl, v2} path + * (e.g. v1 workflow {@code /v1/wf/...}). Caller falls through to URN/DN extraction. * * @throws IllegalArgumentException if multiple URI SANs match the SPIFFE regex */ @@ -216,12 +226,16 @@ private static Optional matchAndExtractSpiffeSAN(X509Certificate clientC return Optional.empty(); } Matcher v2Matcher = SPIFFE_V2_PATH_PATTERN.matcher(path); - if (!v2Matcher.matches()) { - LOG.debug("SPIFFE URI '{}' is not a v2 identity; falling through to URN/DN extraction.", - spiffeUri); - return Optional.empty(); + if (v2Matcher.matches()) { + return Optional.of(v2Matcher.group(1)); + } + Matcher v1WlMatcher = SPIFFE_V1_WL_PATH_PATTERN.matcher(path); + if (v1WlMatcher.matches()) { + return Optional.of(v1WlMatcher.group(1)); } - return Optional.of(v2Matcher.group(1)); + LOG.debug("SPIFFE URI '{}' is not a v1/wl or v2 identity; falling through to URN/DN extraction.", + spiffeUri); + return Optional.empty(); } /** diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/SpiffeAuthTestUtil.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/SpiffeAuthTestUtil.java index a41f178bf99..4fcb9fe2945 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/common/SpiffeAuthTestUtil.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/SpiffeAuthTestUtil.java @@ -41,8 +41,8 @@ public final class SpiffeAuthTestUtil { public static final long ONE_DAY_MILLIS = 24L * 60 * 60 * 1000; - /** Match regex that accepts only v2 SPIFFE URIs (any trust domain). */ - public static final String SPIFFE_V2_MATCH_REGEX = "^spiffe://.*/v2/.*$"; + /** Match regex that accepts SPIFFE v1 and v2 URIs (any trust domain). */ + public static final String SPIFFE_MATCH_REGEX = "^spiffe://.*/v[12]/.*$"; private SpiffeAuthTestUtil() { } @@ -56,7 +56,7 @@ public static void registerBouncyCastle() { /** Configures SAN+SPIFFE-v2 extraction in the X509AuthenticationConfig singleton. */ public static void setSpiffeSystemProperties() { System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_TYPE, "SAN"); - System.setProperty(X509AuthenticationConfig.SSL_X509_SPIFFE_SAN_MATCH_REGEX, SPIFFE_V2_MATCH_REGEX); + System.setProperty(X509AuthenticationConfig.SSL_X509_SPIFFE_SAN_MATCH_REGEX, SPIFFE_MATCH_REGEX); X509AuthenticationConfig.reset(); } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/X509AuthTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/X509AuthTest.java index 92ab8030668..5355e8f7667 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/X509AuthTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/X509AuthTest.java @@ -137,16 +137,18 @@ public void testSANBasedAuth() { private static final String SPIFFE_V2_URI = "spiffe://prod.lipki/v2/application/espresso-router/espresso-router"; @Test - public void testSpiffeV1FallsBackToDn() { + public void testSpiffeV1WlAuth() { SpiffeAuthTestUtil.setSpiffeSystemProperties(); try { + // SPIFFE_V1_URI = "spiffe://prod.lipki/v1/wl/espresso-router"; the "wl/" type prefix is + // stripped, principal is just the app-name. TestCertificate spiffeCert = new TestCertificate("CLIENT", SPIFFE_V1_URI); X509AuthenticationProvider provider = createProvider(spiffeCert); MockServerCnxn cnxn = new MockServerCnxn(); cnxn.clientChain = new X509Certificate[]{spiffeCert}; assertEquals(KeeperException.Code.OK, provider.handleAuthentication(cnxn, null)); - assertEquals("CN=CLIENT", cnxn.getAuthInfo().get(0).getId()); + assertEquals("espresso-router", cnxn.getAuthInfo().get(0).getId()); } finally { SpiffeAuthTestUtil.clearSpiffeSystemProperties(); } @@ -226,7 +228,7 @@ public void testSpiffeConfiguredButNoSpiffeSanFallsBackToUrn() { System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_EXTRACT_REGEX, "^.*urn:li:([a-z]+Principal\\([^;%:]+)"); System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_EXTRACT_MATCHER_GROUP_INDEX, "1"); - System.setProperty(X509AuthenticationConfig.SSL_X509_SPIFFE_SAN_MATCH_REGEX, SpiffeAuthTestUtil.SPIFFE_V2_MATCH_REGEX); + System.setProperty(X509AuthenticationConfig.SSL_X509_SPIFFE_SAN_MATCH_REGEX, SpiffeAuthTestUtil.SPIFFE_MATCH_REGEX); try { TestCertificate urnCert = new TestCertificate("CLIENT", urnSan); @@ -315,7 +317,7 @@ public void testSpiffePrefersSpiffeOverUrnWhenBothPresent() { System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_EXTRACT_REGEX, "^.*urn:li:([a-z]+Principal\\([^;%:]+)"); System.setProperty(X509AuthenticationConfig.SSL_X509_CLIENT_CERT_ID_SAN_EXTRACT_MATCHER_GROUP_INDEX, "1"); - System.setProperty(X509AuthenticationConfig.SSL_X509_SPIFFE_SAN_MATCH_REGEX, SpiffeAuthTestUtil.SPIFFE_V2_MATCH_REGEX); + System.setProperty(X509AuthenticationConfig.SSL_X509_SPIFFE_SAN_MATCH_REGEX, SpiffeAuthTestUtil.SPIFFE_MATCH_REGEX); try { TestCertificate mixedCert = new TestCertificate("CLIENT", Arrays.asList(urnSan, SPIFFE_V2_URI)); diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/X509SpiffeAuthIntegrationTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/X509SpiffeAuthIntegrationTest.java index ca44ab16063..159d472b296 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/X509SpiffeAuthIntegrationTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/X509SpiffeAuthIntegrationTest.java @@ -49,14 +49,16 @@ public void tearDown() { } @Test - public void testRealCertWithSpiffeV1UriSanFallsBackToSubjectDn() throws Exception { + public void testRealCertWithSpiffeV1WlUriSanIsExtracted() throws Exception { SpiffeAuthTestUtil.setSpiffeSystemProperties(); + // v1 workload path "/v1/wl/"; the "wl/" type prefix is stripped, principal is + // just the app-name. X509Certificate cert = SpiffeAuthTestUtil.buildClientCertWithUriSans( "spiffe://prod.lipki/v1/wl/espresso-router"); String id = runAuth(cert); - assertEquals(cert.getSubjectX500Principal().getName(), id); + assertEquals("espresso-router", id); } @Test