Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -83,13 +84,38 @@ 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 matching this regex are
* treated as SPIFFE identities. The extractor accepts both:
* <ul>
* <li><b>v2</b> ({@code /v2/<path>}): principal is the full ILM UID (path-after-{@code /v2/})</li>
* <li><b>v1 workload</b> ({@code /v1/wl/<app-name>}): principal is just {@code <app-name>}
* (the {@code wl/} type prefix is stripped)</li>
* </ul>
* User-identity URIs ({@code /v<N>/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.
*
* <p><b>Recommended:</b> 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.
*
* <p>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;
private String clientCertIdSanMatchRegex;
private String clientCertIdSanExtractRegex;
private int clientCertIdSanExtractMatcherGroupIndex = -1;

private volatile Pattern spiffeSanMatchPattern;
private volatile boolean spiffeSanMatchPatternLoaded = false;

// ZooKeeper server-side config properties for ZNode group ACL feature

/**
Expand Down Expand Up @@ -174,6 +200,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

Expand Down Expand Up @@ -226,6 +253,30 @@ 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 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) {
synchronized (spiffeSanMatchPatternLock) {
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) {
Expand Down Expand Up @@ -482,4 +533,5 @@ public static void reset() {
instance = null;
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -48,6 +50,24 @@ 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<N>/user" or "/v<N>/user/<rest>".
// 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 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.
private static final Pattern SPIFFE_V2_PATH_PATTERN = Pattern.compile("^/v2/(.+)$");
Comment on lines +54 to +63
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

As mentioned in another comment - do add support for both SPIFFE v1 and v2.

SPIFFE URI format: spiffe://<trust-domain/<version>/<unique-identity>

Examples of SPIFFE v1:
spiffe://trust-domain/v1/wl/<app-name>

Therefore parsing of the workload unique-identity will get wl/<app-name>
Remove the prefix wl to get the .

Examples SPIFFE v2:
User: spiffe://trust-domain/v2/user/<userId>
Online application: spiffe://trust-domain/v2/application/<ILM-unique-id>

Flyte Workflow (Although I think, this is not the usecase for ZK).

More details here: https://github.com/linkedin-multiproduct/gopki/blob/master/LISPIFFE-ID.md#2-uri-path

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks @rgodha — addressed in 4a0533b.

The extractor now accepts both versions:

  • v2 (existing): spiffe://<td>/v2/<path> → principal is the full path-after-/v2/ (the ILM UID, e.g. application/foo-mp/bar-app).
  • v1 workload (new): spiffe://<td>/v1/wl/<app-name> → strip the wl/ type prefix; principal is just <app-name>. Matches how legacy authZ handled v1 identities.

User-identity URIs (/v<N>/user/...) are still rejected for both versions — they must never be promoted to a service principal. Other v1 paths (e.g. v1/wf/ workflow) fall through to URN/DN since they are not in scope for ZK.

Test testRealCertWithSpiffeV1WlUriSanIsExtracted builds a real BouncyCastle-signed cert with a v1/wl SAN and asserts the extracted principal is the bare app-name. All 30 SPIFFE-related tests pass locally.


// 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/<app-name>"; 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;
Expand Down Expand Up @@ -134,16 +154,136 @@ public static String getClientId(X509Certificate clientCert) {
String clientCertIdType = X509AuthenticationConfig.getInstance().getClientCertIdType();
if (clientCertIdType != null && clientCertIdType
.equalsIgnoreCase(X509AuthenticationConfig.SUBJECT_ALTERNATIVE_NAME_SHORT)) {
try {
Optional<String> 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 URI SAN. Supported forms:
* <ul>
* <li><b>v2</b> ({@code spiffe://<td>/v2/<path>}): 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.</li>
* <li><b>v1 workload</b> ({@code spiffe://<td>/v1/wl/<app-name>}): principal is just the
* {@code <app-name>} (the "wl/" type prefix is stripped, matching how legacy authZ
* handled v1 identities).</li>
* </ul>
*
* <p>Returns {@link Optional#empty()} when SPIFFE extraction is disabled, no URI SAN matches the
* configured regex, the matched URI is a user identity ({@code /v<N>/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
*/
private static Optional<String> 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()) {
return Optional.of(v2Matcher.group(1));
}
Matcher v1WlMatcher = SPIFFE_V1_WL_PATH_PATTERN.matcher(path);
if (v1WlMatcher.matches()) {
return Optional.of(v1WlMatcher.group(1));
}
LOG.debug("SPIFFE URI '{}' is not a v1/wl or v2 identity; falling through to URN/DN extraction.",
spiffeUri);
return Optional.empty();
}

/**
* 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<List<?>> 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.
Expand Down Expand Up @@ -204,18 +344,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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()));
Expand Down
Loading
Loading