You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ro...@apache.org on 2021/09/01 12:50:01 UTC
[cloudstack] branch main updated: allow cert renewal even if auth
strictness is false (#4852)
This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 76d5ce3 allow cert renewal even if auth strictness is false (#4852)
76d5ce3 is described below
commit 76d5ce310af7fc02eca124c495bf7e205ed7e9f8
Author: Slair1 <sl...@ippathways.com>
AuthorDate: Wed Sep 1 07:49:07 2021 -0500
allow cert renewal even if auth strictness is false (#4852)
includes updated tests and logging
---
.../ca/provider/RootCACustomTrustManager.java | 51 +++++++++++++---------
.../cloudstack/ca/provider/RootCAProvider.java | 9 +++-
.../ca/provider/RootCACustomTrustManagerTest.java | 37 +++++++++++++++-
.../cloudstack/ca/provider/RootCAProviderTest.java | 3 +-
4 files changed, 75 insertions(+), 25 deletions(-)
diff --git a/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCACustomTrustManager.java b/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCACustomTrustManager.java
index 90f6203..0a4df0a 100644
--- a/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCACustomTrustManager.java
+++ b/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCACustomTrustManager.java
@@ -79,13 +79,16 @@ public final class RootCACustomTrustManager implements X509TrustManager {
if (LOG.isDebugEnabled()) {
printCertificateChain(certificates, s);
}
- if (!authStrictness) {
- return;
- }
- if (certificates == null || certificates.length < 1 || certificates[0] == null) {
+
+ final X509Certificate primaryClientCertificate = (certificates != null && certificates.length > 0 && certificates[0] != null) ? certificates[0] : null;
+ String exceptionMsg = "";
+
+ if (authStrictness && primaryClientCertificate == null) {
throw new CertificateException("In strict auth mode, certificate(s) are expected from client:" + clientAddress);
+ } else if (primaryClientCertificate == null) {
+ LOG.info("No certificate was received from client, but continuing since strict auth mode is disabled");
+ return;
}
- final X509Certificate primaryClientCertificate = certificates[0];
// Revocation check
final BigInteger serialNumber = primaryClientCertificate.getSerialNumber();
@@ -93,18 +96,19 @@ public final class RootCACustomTrustManager implements X509TrustManager {
final String errorMsg = String.format("Client is using revoked certificate of serial=%x, subject=%s from address=%s",
primaryClientCertificate.getSerialNumber(), primaryClientCertificate.getSubjectDN(), clientAddress);
LOG.error(errorMsg);
- throw new CertificateException(errorMsg);
+ exceptionMsg = (Strings.isNullOrEmpty(exceptionMsg)) ? errorMsg : (exceptionMsg + ". " + errorMsg);
}
// Validity check
- if (!allowExpiredCertificate) {
- try {
- primaryClientCertificate.checkValidity();
- } catch (final CertificateExpiredException | CertificateNotYetValidException e) {
- final String errorMsg = String.format("Client certificate has expired with serial=%x, subject=%s from address=%s",
- primaryClientCertificate.getSerialNumber(), primaryClientCertificate.getSubjectDN(), clientAddress);
- LOG.error(errorMsg);
- throw new CertificateException(errorMsg); }
+ try {
+ primaryClientCertificate.checkValidity();
+ } catch (final CertificateExpiredException | CertificateNotYetValidException e) {
+ final String errorMsg = String.format("Client certificate has expired with serial=%x, subject=%s from address=%s",
+ primaryClientCertificate.getSerialNumber(), primaryClientCertificate.getSubjectDN(), clientAddress);
+ LOG.error(errorMsg);
+ if (!allowExpiredCertificate) {
+ throw new CertificateException(errorMsg);
+ }
}
// Ownership check
@@ -122,13 +126,21 @@ public final class RootCACustomTrustManager implements X509TrustManager {
if (!certMatchesOwnership) {
final String errorMsg = "Certificate ownership verification failed for client: " + clientAddress;
LOG.error(errorMsg);
- throw new CertificateException(errorMsg);
+ exceptionMsg = (Strings.isNullOrEmpty(exceptionMsg)) ? errorMsg : (exceptionMsg + ". " + errorMsg);
}
- if (activeCertMap != null && !Strings.isNullOrEmpty(clientAddress)) {
- activeCertMap.put(clientAddress, primaryClientCertificate);
+ if (authStrictness && !Strings.isNullOrEmpty(exceptionMsg)) {
+ throw new CertificateException(exceptionMsg);
}
if (LOG.isDebugEnabled()) {
- LOG.debug("Client/agent connection from ip=" + clientAddress + " has been validated and trusted.");
+ if (authStrictness) {
+ LOG.debug("Client/agent connection from ip=" + clientAddress + " has been validated and trusted.");
+ } else {
+ LOG.debug("Client/agent connection from ip=" + clientAddress + " accepted without certificate validation.");
+ }
+ }
+
+ if (primaryClientCertificate != null && activeCertMap != null && !Strings.isNullOrEmpty(clientAddress)) {
+ activeCertMap.put(clientAddress, primaryClientCertificate);
}
}
@@ -138,9 +150,6 @@ public final class RootCACustomTrustManager implements X509TrustManager {
@Override
public X509Certificate[] getAcceptedIssuers() {
- if (!authStrictness) {
- return null;
- }
return new X509Certificate[]{caCertificate};
}
}
diff --git a/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java b/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java
index d7a9985..b0eebd4 100644
--- a/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java
+++ b/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java
@@ -267,9 +267,16 @@ public final class RootCAProvider extends AdapterBase implements CAProvider, Con
final boolean allowExpiredCertificate = rootCAAllowExpiredCert.value();
TrustManager[] tms = new TrustManager[]{new RootCACustomTrustManager(remoteAddress, authStrictness, allowExpiredCertificate, certMap, caCertificate, crlDao)};
+
sslContext.init(kmf.getKeyManagers(), tms, new SecureRandom());
final SSLEngine sslEngine = sslContext.createSSLEngine();
- sslEngine.setNeedClientAuth(authStrictness);
+ // If authStrictness require SSL and validate client cert, otherwise prefer SSL but don't validate client cert
+ if (authStrictness) {
+ sslEngine.setNeedClientAuth(true); // Require SSL and client cert validation
+ } else {
+ sslEngine.setWantClientAuth(true); // Prefer SSL but don't validate client cert
+ }
+
return sslEngine;
}
diff --git a/plugins/ca/root-ca/src/test/java/org/apache/cloudstack/ca/provider/RootCACustomTrustManagerTest.java b/plugins/ca/root-ca/src/test/java/org/apache/cloudstack/ca/provider/RootCACustomTrustManagerTest.java
index 8161d75..87695b8 100644
--- a/plugins/ca/root-ca/src/test/java/org/apache/cloudstack/ca/provider/RootCACustomTrustManagerTest.java
+++ b/plugins/ca/root-ca/src/test/java/org/apache/cloudstack/ca/provider/RootCACustomTrustManagerTest.java
@@ -62,10 +62,43 @@ public class RootCACustomTrustManagerTest {
}
@Test
- public void testAuthNotStrict() throws Exception {
+ public void testAuthNotStrictWithInvalidCert() throws Exception {
final RootCACustomTrustManager trustManager = new RootCACustomTrustManager(clientIp, false, true, certMap, caCertificate, crlDao);
trustManager.checkClientTrusted(null, null);
- Assert.assertNull(trustManager.getAcceptedIssuers());
+ }
+
+ @Test
+ public void testAuthNotStrictWithRevokedCert() throws Exception {
+ Mockito.when(crlDao.findBySerial(Mockito.any(BigInteger.class))).thenReturn(new CrlVO());
+ final RootCACustomTrustManager trustManager = new RootCACustomTrustManager(clientIp, false, true, certMap, caCertificate, crlDao);
+ trustManager.checkClientTrusted(new X509Certificate[]{caCertificate}, "RSA");
+ Assert.assertTrue(certMap.containsKey(clientIp));
+ Assert.assertEquals(certMap.get(clientIp), caCertificate);
+ }
+
+ @Test
+ public void testAuthNotStrictWithInvalidCertOwnership() throws Exception {
+ Mockito.when(crlDao.findBySerial(Mockito.any(BigInteger.class))).thenReturn(null);
+ final RootCACustomTrustManager trustManager = new RootCACustomTrustManager(clientIp, false, true, certMap, caCertificate, crlDao);
+ trustManager.checkClientTrusted(new X509Certificate[]{caCertificate}, "RSA");
+ Assert.assertTrue(certMap.containsKey(clientIp));
+ Assert.assertEquals(certMap.get(clientIp), caCertificate);
+ }
+
+ @Test(expected = CertificateException.class)
+ public void testAuthNotStrictWithDenyExpiredCertAndOwnership() throws Exception {
+ Mockito.when(crlDao.findBySerial(Mockito.any(BigInteger.class))).thenReturn(null);
+ final RootCACustomTrustManager trustManager = new RootCACustomTrustManager(clientIp, false, false, certMap, caCertificate, crlDao);
+ trustManager.checkClientTrusted(new X509Certificate[]{expiredClientCertificate}, "RSA");
+ }
+
+ @Test
+ public void testAuthNotStrictWithAllowExpiredCertAndOwnership() throws Exception {
+ Mockito.when(crlDao.findBySerial(Mockito.any(BigInteger.class))).thenReturn(null);
+ final RootCACustomTrustManager trustManager = new RootCACustomTrustManager(clientIp, false, true, certMap, caCertificate, crlDao);
+ trustManager.checkClientTrusted(new X509Certificate[]{expiredClientCertificate}, "RSA");
+ Assert.assertTrue(certMap.containsKey(clientIp));
+ Assert.assertEquals(certMap.get(clientIp), expiredClientCertificate);
}
@Test(expected = CertificateException.class)
diff --git a/plugins/ca/root-ca/src/test/java/org/apache/cloudstack/ca/provider/RootCAProviderTest.java b/plugins/ca/root-ca/src/test/java/org/apache/cloudstack/ca/provider/RootCAProviderTest.java
index f9fd531..15514b9 100644
--- a/plugins/ca/root-ca/src/test/java/org/apache/cloudstack/ca/provider/RootCAProviderTest.java
+++ b/plugins/ca/root-ca/src/test/java/org/apache/cloudstack/ca/provider/RootCAProviderTest.java
@@ -133,6 +133,7 @@ public class RootCAProviderTest {
provider.rootCAAuthStrictness = Mockito.mock(ConfigKey.class);
Mockito.when(provider.rootCAAuthStrictness.value()).thenReturn(Boolean.FALSE);
final SSLEngine e = provider.createSSLEngine(SSLUtils.getSSLContext(), "/1.2.3.4:5678", null);
+ Assert.assertTrue(e.getWantClientAuth());
Assert.assertFalse(e.getNeedClientAuth());
}
@@ -149,4 +150,4 @@ public class RootCAProviderTest {
Assert.assertEquals(provider.getProviderName(), "root");
}
-}
\ No newline at end of file
+}