Skip to content

Commit

Permalink
defer accesstoken error logging (#2770)
Browse files Browse the repository at this point in the history 
Signed-off-by: Abhijeet V <[email protected]>
  • Loading branch information
@abvaidya
abvaidya authored Oct 17, 2024
1 parent 659ec5c commit dbab62d
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 27 deletions.
44 changes: 25 additions & 19 deletions libs/java/auth_core/src/main/java/com/yahoo/athenz/auth/token/AccessToken.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,31 +337,31 @@ public boolean confirmMTLSBoundToken(X509Certificate x509Cert, final String x509
// check if the certificate principal matches and the
// creation time for our cert is within our configured
// offset timeouts

if (confirmX509CertPrincipal(x509Cert, cn)) {
StringBuilder errorStore = new StringBuilder();
if (confirmX509CertPrincipal(x509Cert, cn, errorStore)) {
return true;
}

// check if we have authorization details specified for
// proxy access thus we can validate based on that

if (confirmX509CertPrincipalAuthzDetails(x509Cert)) {
if (confirmX509CertPrincipalAuthzDetails(x509Cert, errorStore)) {
return true;
}
// direct comparison of certificate cn and provided hash

return confirmX509ProxyPrincipal(cn, x509CertHash, cnfHash);
return confirmX509ProxyPrincipal(cn, x509CertHash, cnfHash, errorStore);
}

boolean confirmX509CertPrincipalAuthzDetails(X509Certificate cert) {
boolean confirmX509CertPrincipalAuthzDetails(X509Certificate cert, StringBuilder errorStore) {

// make sure we have valid proxy principals specified

List<String> spiffeUris;
try {
spiffeUris = (List<String>) confirm.get(CLAIM_CONFIRM_PROXY_SPIFFE);
} catch (Exception ex) {
LOG.error("Unable to parse proxy principal claim: {}", ex.getMessage());
errorStore.append("Unable to parse proxy principal claim: ").append(ex.getMessage()).append(";");
return false;
}
if (spiffeUris == null) {
Expand All @@ -374,7 +374,8 @@ boolean confirmX509CertPrincipalAuthzDetails(X509Certificate cert) {
return true;
}
}

errorStore.append("confirmX509CertPrincipalAuthzDetails: cert spiffe uri: ").append(certSpiffeUri)
.append(" not found in access token proxy principals: ").append(spiffeUris).append(";");
return false;
}

Expand All @@ -384,34 +385,38 @@ boolean confirmX509CertHash(X509Certificate cert, final String cnfHash) {
return cnfHash.equals(certHash);
}

boolean confirmX509ProxyPrincipal(final String cn, final String certHash, final String cnfHash) {
boolean confirmX509ProxyPrincipal(final String cn, final String certHash, final String cnfHash, StringBuilder errorStore) {

// if the proxy principal set is not null then the client
// has specified some value so we'll enforce it (even if
// the set is empty thus rejecting all requests)

if (ACCESS_TOKEN_PROXY_PRINCIPALS != null && !ACCESS_TOKEN_PROXY_PRINCIPALS.contains(cn)) {
LOG.error("confirmX509ProxyPrincipal: unauthorized proxy principal: {}", cn);
errorStore.append("confirmX509ProxyPrincipal: unauthorized proxy principal: ").append(cn).append(";");
LOG.error(errorStore.toString());
return false;
}

return cnfHash.equals(certHash);
if (!cnfHash.equals(certHash)) {
LOG.error(errorStore.toString());
return false;
}
return true;
}

boolean confirmX509CertPrincipal(X509Certificate cert, final String cn) {
boolean confirmX509CertPrincipal(X509Certificate cert, final String cn, StringBuilder errorStore) {

// if our offset is 0 then the additional confirmation
// check is disabled

if (ACCESS_TOKEN_CERT_OFFSET == 0) {
LOG.error("confirmX509CertPrincipal: check disabled");
errorStore.append("confirmX509CertPrincipal: check disabled;");
return false;
}

// our principal cn must be the client in the token

if (!cn.equals(clientId)) {
LOG.error("confirmX509CertPrincipal: Principal mismatch {} vs {}", cn, clientId);
errorStore.append("confirmX509CertPrincipal: Principal mismatch").append(cn).append(" vs ")
.append(clientId).append(";");
return false;
}

Expand All @@ -431,8 +436,8 @@ boolean confirmX509CertPrincipal(X509Certificate cert, final String cn) {

long certIssueTime = Crypto.extractX509CertIssueTime(cert);
if (certIssueTime < issueTime - 3600) {
LOG.error("confirmX509CertPrincipal: Certificate: {} issued before token: {}",
certIssueTime, issueTime);
errorStore.append("confirmX509CertPrincipal: Certificate ").append(certIssueTime)
.append(" issued before token: ").append(issueTime).append(";");
return false;
}

Expand All @@ -442,8 +447,9 @@ boolean confirmX509CertPrincipal(X509Certificate cert, final String cn) {
// need to take into account that extra hour

if (certIssueTime > issueTime + ACCESS_TOKEN_CERT_OFFSET - 3600) {
LOG.error("confirmX509CertPrincipal: Certificate: {} past configured offset {} for token: {}",
certIssueTime, ACCESS_TOKEN_CERT_OFFSET, issueTime);
errorStore.append("confirmX509CertPrincipal: Certificate ").append(certIssueTime)
.append(" past configured offset: ").append(ACCESS_TOKEN_CERT_OFFSET)
.append(" for token: ").append(issueTime).append(";");
return false;
}

Expand Down
16 changes: 8 additions & 8 deletions libs/java/auth_core/src/test/java/com/yahoo/athenz/auth/token/AccessTokenTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,7 @@ public void testConfirmX509CertPrincipalNullCert() {

long now = System.currentTimeMillis() / 1000;
AccessToken accessToken = createAccessToken(now);
assertFalse(accessToken.confirmX509CertPrincipal(null, "athenz.proxy"));
assertFalse(accessToken.confirmX509CertPrincipal(null, "athenz.proxy", new StringBuilder()));
}

@Test
Expand All @@ -740,7 +740,7 @@ public void testConfirmX509CertPrincipalCertNoCN() throws IOException {
Path path = Paths.get("src/test/resources/no_cn_x509.cert");
String certStr = new String(Files.readAllBytes(path));
X509Certificate cert = Crypto.loadX509Certificate(certStr);
assertFalse(accessToken.confirmX509CertPrincipal(cert, "athenz.proxy"));
assertFalse(accessToken.confirmX509CertPrincipal(cert, "athenz.proxy", new StringBuilder()));
}

@Test
Expand All @@ -752,7 +752,7 @@ public void testConfirmX509CertPrincipalCertCNMismatch() throws IOException {
Path path = Paths.get("src/test/resources/rsa_public_x509.cert");
String certStr = new String(Files.readAllBytes(path));
X509Certificate cert = Crypto.loadX509Certificate(certStr);
assertFalse(accessToken.confirmX509CertPrincipal(cert, "athenz.proxy"));
assertFalse(accessToken.confirmX509CertPrincipal(cert, "athenz.proxy", new StringBuilder()));
}

@Test
Expand Down Expand Up @@ -976,7 +976,7 @@ public void testConfirmX509CertPrincipalCertStartTime() throws IOException {
Path path = Paths.get("src/test/resources/mtls_token2_spec.cert");
String certStr = new String(Files.readAllBytes(path));
X509Certificate cert = Crypto.loadX509Certificate(certStr);
assertFalse(accessToken.confirmX509CertPrincipal(cert, "mtls"));
assertFalse(accessToken.confirmX509CertPrincipal(cert, "mtls", new StringBuilder()));
}

@Test
Expand All @@ -992,7 +992,7 @@ public void testConfirmX509CertPrincipalCertStartTimeCheckDisabled() throws IOEx
Path path = Paths.get("src/test/resources/mtls_token2_spec.cert");
String certStr = new String(Files.readAllBytes(path));
X509Certificate cert = Crypto.loadX509Certificate(certStr);
assertTrue(accessToken.confirmX509CertPrincipal(cert, "mtls"));
assertTrue(accessToken.confirmX509CertPrincipal(cert, "mtls", new StringBuilder()));

AccessToken.setAccessTokenCertOffset(3600);
}
Expand All @@ -1008,7 +1008,7 @@ public void testConfirmX509CertPrincipalCertStartTimePassOffset() throws IOExcep
Path path = Paths.get("src/test/resources/mtls_token2_spec.cert");
String certStr = new String(Files.readAllBytes(path));
X509Certificate cert = Crypto.loadX509Certificate(certStr);
assertFalse(accessToken.confirmX509CertPrincipal(cert, "mtls"));
assertFalse(accessToken.confirmX509CertPrincipal(cert, "mtls", new StringBuilder()));
}

@Test
Expand All @@ -1022,7 +1022,7 @@ public void testConfirmX509CertPrincipal() throws IOException {
Path path = Paths.get("src/test/resources/mtls_token2_spec.cert");
String certStr = new String(Files.readAllBytes(path));
X509Certificate cert = Crypto.loadX509Certificate(certStr);
assertTrue(accessToken.confirmX509CertPrincipal(cert, "mtls"));
assertTrue(accessToken.confirmX509CertPrincipal(cert, "mtls", new StringBuilder()));
}

@Test
Expand All @@ -1038,7 +1038,7 @@ public void testConfirmX509CertPrincipalDisable() throws IOException {
Path path = Paths.get("src/test/resources/mtls_token2_spec.cert");
String certStr = new String(Files.readAllBytes(path));
X509Certificate cert = Crypto.loadX509Certificate(certStr);
assertFalse(accessToken.confirmX509CertPrincipal(cert, "mtls"));
assertFalse(accessToken.confirmX509CertPrincipal(cert, "mtls", new StringBuilder()));

AccessToken.setAccessTokenCertOffset(3600);
}
Expand Down

0 comments on commit dbab62d

Please sign in to comment.