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 <[email protected]>
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
+}
