Skip to content

Commit

Permalink
make ca issuer check optional (#623)
Browse files Browse the repository at this point in the history 
* make ca issuer check optional

* ca issuer check based on rdn match
  • Loading branch information
@havetisyan
havetisyan authored Jan 30, 2019
1 parent 6a2f844 commit 1ddd2ea
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 23 deletions.
100 changes: 81 additions & 19 deletions clients/java/zpe/src/main/java/com/yahoo/athenz/zpe/AuthZpeClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -66,8 +65,9 @@ public class AuthZpeClient {
private static ZpeClient zpeClt = null;
private static PublicKeyStore publicKeyStore = null;

private static final Set<String> X509_ISSUERS = new HashSet<>();

private static final Set<String> X509_ISSUERS_NAMES = new HashSet<>();
private static final List<List<Rdn>> X509_ISSUERS_RDNS = new ArrayList<>();

private static final String ROLE_SEARCH = ":role.";

public enum AccessCheckStatus {
Expand Down Expand Up @@ -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());
}
}
}

Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -952,6 +956,63 @@ static Map<String, List<Struct>> 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<Rdn> issuerRdns = new LdapName(issuerCheck.getName()).getRdns();

for (List<Rdn> 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) {
Expand All @@ -967,4 +1028,5 @@ public static void main(String [] args) {
System.out.println("Authorization Response: "
+ AuthZpeClient.allowAccess(roleToken, action, resource).toString());
}
/// CLOVER:ON
}
43 changes: 39 additions & 4 deletions clients/java/zpe/src/test/java/com/yahoo/athenz/zpe/TestAuthZpe.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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"));
}
}

0 comments on commit 1ddd2ea

Please sign in to comment.