From 1ddd2ea48ad5e6e6a891ef969083af2ae7224c3f Mon Sep 17 00:00:00 2001 From: Henry Avetisyan Date: Wed, 30 Jan 2019 10:30:37 -0800 Subject: [PATCH] make ca issuer check optional (#623) * make ca issuer check optional * ca issuer check based on rdn match --- .../com/yahoo/athenz/zpe/AuthZpeClient.java | 100 ++++++++++++++---- .../com/yahoo/athenz/zpe/TestAuthZpe.java | 43 +++++++- 2 files changed, 120 insertions(+), 23 deletions(-) diff --git a/clients/java/zpe/src/main/java/com/yahoo/athenz/zpe/AuthZpeClient.java b/clients/java/zpe/src/main/java/com/yahoo/athenz/zpe/AuthZpeClient.java index 82252fd2617..43a07fe33fc 100644 --- a/clients/java/zpe/src/main/java/com/yahoo/athenz/zpe/AuthZpeClient.java +++ b/clients/java/zpe/src/main/java/com/yahoo/athenz/zpe/AuthZpeClient.java @@ -17,12 +17,11 @@ import java.security.PublicKey; import java.security.cert.X509Certificate; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; +import javax.naming.InvalidNameException; +import javax.naming.ldap.LdapName; +import javax.naming.ldap.Rdn; import javax.security.auth.x500.X500Principal; import org.slf4j.Logger; @@ -66,8 +65,9 @@ public class AuthZpeClient { private static ZpeClient zpeClt = null; private static PublicKeyStore publicKeyStore = null; - private static final Set X509_ISSUERS = new HashSet<>(); - + private static final Set X509_ISSUERS_NAMES = new HashSet<>(); + private static final List> X509_ISSUERS_RDNS = new ArrayList<>(); + private static final String ROLE_SEARCH = ":role."; public enum AccessCheckStatus { @@ -191,16 +191,22 @@ public static void setTokenAllowedOffset(int offset) { * @param issuers list of Athenz CA issuers separated by | */ public static void setX509CAIssuers(final String issuers) { + if (issuers == null || issuers.isEmpty()) { return; } - String[] issuerArray = issuers.replaceAll("\\s+" , "").split("\\|"); - for (String issuer: issuerArray) { + String[] issuerArray = issuers.split("\\|"); + for (String issuer : issuerArray) { if (LOG.isDebugEnabled()) { LOG.debug("x509 issuer: {}", issuer); } - X509_ISSUERS.add(issuer); + X509_ISSUERS_NAMES.add(issuer.replaceAll("\\s+", "")); + try { + X509_ISSUERS_RDNS.add(new LdapName(issuer).getRdns()); + } catch (InvalidNameException ex) { + LOG.error("Invalid issuer: {}, error: {}", issuer, ex.getMessage()); + } } } @@ -273,17 +279,15 @@ public static AccessCheckStatus allowAccess(X509Certificate cert, String angReso LOG.debug("AUTHZPECLT:allowAccess: action=" + action + " resource=" + angResource); } zpeMetric.increment(ZpeConsts.ZPE_METRIC_NAME, DEFAULT_DOMAIN); - // validate the certificate against CAs - X500Principal issuerx500Principal = cert.getIssuerX500Principal(); - String issuer = issuerx500Principal.getName(); - - if (issuer == null || issuer.isEmpty() - || !X509_ISSUERS.contains(issuer.replaceAll("\\s+" , ""))) { - if (LOG.isDebugEnabled()) { - LOG.debug("AUTHZPECLT:allowAccess: missing or mismatch issuer {}", issuer); - } + + // validate the certificate against CAs if the feature + // is configured. if the caller does not specify any + // issuers we're not going to make any checks + + if (!certIssuerMatch(cert)) { return AccessCheckStatus.DENY_CERT_MISMATCH_ISSUER; } + String subject = Crypto.extractX509CertCommonName(cert); if (subject == null || subject.isEmpty()) { if (LOG.isDebugEnabled()) { @@ -952,6 +956,63 @@ static Map> getRoleSpecificDenyPolicies(String domain) { return null; } + static boolean certIssuerMatch(X509Certificate cert) { + + // first check if we have any issuers configured + + if (X509_ISSUERS_NAMES.isEmpty()) { + return true; + } + + X500Principal issuerx500Principal = cert.getIssuerX500Principal(); + String issuer = issuerx500Principal.getName(); + boolean match = issuerMatch(issuer); + + if (!match && LOG.isDebugEnabled()) { + LOG.debug("AUTHZPECLT:certIssuerMatch: missing or mismatch issuer {}", issuer); + } + + return match; + } + + static boolean issuerMatch(final String issuer) { + + // verify we have a valid issuer before any checks + + if (issuer == null || issuer.isEmpty()) { + return false; + } + + // first we're going to check our quick check + // using the issuer as is without any rdn compare + + if (X509_ISSUERS_NAMES.contains(issuer.replaceAll("\\s+" , ""))) { + return true; + } + + // we're going to do more expensive rdn match + + + try { + X500Principal issuerCheck = new X500Principal(issuer); + List issuerRdns = new LdapName(issuerCheck.getName()).getRdns(); + + for (List rdns : X509_ISSUERS_RDNS) { + if (rdns.size() != issuerRdns.size()) { + continue; + } + if (rdns.containsAll(issuerRdns)) { + return true; + } + } + } catch (InvalidNameException ignored) { + // the caller will log the failure + } + + return false; + } + + /// CLOVER:OFF public static void main(String [] args) { if (args.length != 3) { @@ -967,4 +1028,5 @@ public static void main(String [] args) { System.out.println("Authorization Response: " + AuthZpeClient.allowAccess(roleToken, action, resource).toString()); } + /// CLOVER:ON } diff --git a/clients/java/zpe/src/test/java/com/yahoo/athenz/zpe/TestAuthZpe.java b/clients/java/zpe/src/test/java/com/yahoo/athenz/zpe/TestAuthZpe.java index a7edd3b88f8..06882e13e2e 100644 --- a/clients/java/zpe/src/test/java/com/yahoo/athenz/zpe/TestAuthZpe.java +++ b/clients/java/zpe/src/test/java/com/yahoo/athenz/zpe/TestAuthZpe.java @@ -15,10 +15,6 @@ */ package com.yahoo.athenz.zpe; -import static org.testng.Assert.assertEquals; -import static org.testng.Assert.assertNotNull; -import static org.testng.Assert.assertNull; - import java.io.File; import java.io.IOException; import java.nio.file.Files; @@ -48,6 +44,8 @@ import com.yahoo.athenz.zts.SignedPolicyData; import com.yahoo.rdl.JSON; +import static org.testng.Assert.*; + /** * These tests are dependent on a policy file in a local dir. */ @@ -1042,4 +1040,41 @@ public void testX509CertificateReadAllowed(String issuer, String subject, Access AccessCheckStatus status = AuthZpeClient.allowAccess(cert, angResource, action); Assert.assertEquals(status, expectedStatus); } + + @Test + public void testIssuerMatch() { + + // our default set contains the following issuers: + + // C=US, ST=CA, O=Athenz, OU=Testing Domain, CN=angler:role.public + // C=US, ST=CA, O=Athenz, OU=Testing Domain2, CN=angler:role.public + // C=US, ST=CA, O=Athenz, OU=Testing Domain, CN=angler.test:role.public"; + + // passing null or empty list to the set method has no impact + // make sure no exceptions are thrown + + AuthZpeClient.setX509CAIssuers(null); + AuthZpeClient.setX509CAIssuers(""); + + // add a new entry in our list + + AuthZpeClient.setX509CAIssuers("cn=Athenz CA, ou=engineering, o=My Company! Inc., c=us"); + + // passing values in the same order should match with our string set + + assertTrue(AuthZpeClient.issuerMatch("cn=Athenz CA, ou=engineering, o=My Company! Inc., c=us")); + + // passing values in different order should match our rdn check + + assertTrue(AuthZpeClient.issuerMatch("o=My Company! Inc., cn=Athenz CA, c=us, ou=engineering")); + + // passing an extra or less rdn component should fail + + assertFalse(AuthZpeClient.issuerMatch("cn=Athenz CA, ou=engineering, o=My Company! Inc., l=Los Angeles, c=us")); + assertFalse(AuthZpeClient.issuerMatch("cn=Athenz CA, ou=engineering, c=us")); + + // same number of components but different values + + assertFalse(AuthZpeClient.issuerMatch("cn=Athenz CA, ou=engineering, o=My Company Inc., c=us")); + } }