Skip to content

Commit 9161807

Browse files
Slair1Slair1
authored
allow cert renewal even if auth strictness is false (#42)
includes updated tests and logging even w/o strictness
1 parent c874800 commit 9161807

File tree

4 files changed

+83
-29
lines changed

4 files changed

+83
-29
lines changed

‎plugins/ca/root-ca/src/org/apache/cloudstack/ca/provider/RootCACustomTrustManager.java‎

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -79,32 +79,35 @@ public void checkClientTrusted(final X509Certificate[] certificates, final Strin
7979
if (LOG.isDebugEnabled()) {
8080
printCertificateChain(certificates, s);
8181
}
82-
if (!authStrictness) {
83-
return;
84-
}
85-
if (certificates == null || certificates.length < 1 || certificates[0] == null) {
82+
83+
final X509Certificate primaryClientCertificate = (certificates != null && certificates.length > 0 && certificates[0] != null) ? certificates[0] : null;
84+
String exceptionMsg = "";
85+
86+
if (authStrictness && primaryClientCertificate == null) {
8687
throw new CertificateException("In strict auth mode, certificate(s) are expected from client:" + clientAddress);
88+
} else if (primaryClientCertificate == null) {
89+
return;
8790
}
88-
final X509Certificate primaryClientCertificate = certificates[0];
8991

9092
// Revocation check
9193
final BigInteger serialNumber = primaryClientCertificate.getSerialNumber();
9294
if (serialNumber == null || crlDao.findBySerial(serialNumber) != null) {
9395
final String errorMsg = String.format("Client is using revoked certificate of serial=%x, subject=%s from address=%s",
9496
primaryClientCertificate.getSerialNumber(), primaryClientCertificate.getSubjectDN(), clientAddress);
9597
LOG.error(errorMsg);
96-
throw new CertificateException(errorMsg);
98+
exceptionMsg = (Strings.isNullOrEmpty(exceptionMsg)) ? errorMsg : (exceptionMsg + ". " + errorMsg);
9799
}
98100

99101
// Validity check
100-
if (!allowExpiredCertificate) {
101-
try {
102-
primaryClientCertificate.checkValidity();
103-
} catch (final CertificateExpiredException | CertificateNotYetValidException e) {
104-
final String errorMsg = String.format("Client certificate has expired with serial=%x, subject=%s from address=%s",
105-
primaryClientCertificate.getSerialNumber(), primaryClientCertificate.getSubjectDN(), clientAddress);
106-
LOG.error(errorMsg);
107-
throw new CertificateException(errorMsg); }
102+
try {
103+
primaryClientCertificate.checkValidity();
104+
} catch (final CertificateExpiredException | CertificateNotYetValidException e) {
105+
final String errorMsg = String.format("Client certificate has expired with serial=%x, subject=%s from address=%s",
106+
primaryClientCertificate.getSerialNumber(), primaryClientCertificate.getSubjectDN(), clientAddress);
107+
LOG.error(errorMsg);
108+
if (!allowExpiredCertificate) {
109+
throw new CertificateException(errorMsg);
110+
}
108111
}
109112

110113
// Ownership check
@@ -122,13 +125,21 @@ public void checkClientTrusted(final X509Certificate[] certificates, final Strin
122125
if (!certMatchesOwnership) {
123126
final String errorMsg = "Certificate ownership verification failed for client: " + clientAddress;
124127
LOG.error(errorMsg);
125-
throw new CertificateException(errorMsg);
128+
exceptionMsg = (Strings.isNullOrEmpty(exceptionMsg)) ? errorMsg : (exceptionMsg + ". " + errorMsg);
126129
}
127-
if (activeCertMap != null && !Strings.isNullOrEmpty(clientAddress)) {
128-
activeCertMap.put(clientAddress, primaryClientCertificate);
130+
if (authStrictness && !Strings.isNullOrEmpty(exceptionMsg)) {
131+
throw new CertificateException(exceptionMsg);
129132
}
130133
if (LOG.isDebugEnabled()) {
131-
LOG.debug("Client/agent connection from ip=" + clientAddress + " has been validated and trusted.");
134+
if (authStrictness) {
135+
LOG.debug("Client/agent connection from ip=" + clientAddress + " has been validated and trusted.");
136+
} else {
137+
LOG.debug("Client/agent connection from ip=" + clientAddress + " accepted without certificate validation.");
138+
}
139+
}
140+
141+
if (primaryClientCertificate != null && activeCertMap != null && !Strings.isNullOrEmpty(clientAddress)) {
142+
activeCertMap.put(clientAddress, primaryClientCertificate);
132143
}
133144
}
134145

@@ -138,9 +149,6 @@ public void checkServerTrusted(X509Certificate[] x509Certificates, String s) thr
138149

139150
@Override
140151
public X509Certificate[] getAcceptedIssuers() {
141-
if (!authStrictness) {
142-
return null;
143-
}
144152
return new X509Certificate[]{caCertificate};
145153
}
146154
}

‎plugins/ca/root-ca/src/org/apache/cloudstack/ca/provider/RootCAProvider.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,9 +267,16 @@ public SSLEngine createSSLEngine(final SSLContext sslContext, final String remot
267267
final boolean allowExpiredCertificate = rootCAAllowExpiredCert.value();
268268

269269
TrustManager[] tms = new TrustManager[]{new RootCACustomTrustManager(remoteAddress, authStrictness, allowExpiredCertificate, certMap, caCertificate, crlDao)};
270+
270271
sslContext.init(kmf.getKeyManagers(), tms, new SecureRandom());
271272
final SSLEngine sslEngine = sslContext.createSSLEngine();
272-
sslEngine.setNeedClientAuth(authStrictness);
273+
// If authStrictness require SSL and validate client cert, otherwise prefer SSL but don't validate client cert
274+
if (authStrictness) {
275+
sslEngine.setNeedClientAuth(true); // Require SSL and client cert validation
276+
} else {
277+
sslEngine.setWantClientAuth(true); // Prefer SSL but don't validate client cert
278+
}
279+
273280
return sslEngine;
274281
}
275282

‎plugins/ca/root-ca/test/org/apache/cloudstack/ca/provider/RootCACustomTrustManagerTest.java‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,44 @@ public void setUp() throws Exception {
6262
}
6363

6464
@Test
65-
public void testAuthNotStrict() throws Exception {
65+
public void testAuthNotStrictWithInvalidCert() throws Exception {
66+
Mockito.when(crlDao.findBySerial(Mockito.any(BigInteger.class))).thenReturn(new CrlVO());
6667
final RootCACustomTrustManager trustManager = new RootCACustomTrustManager(clientIp, false, true, certMap, caCertificate, crlDao);
6768
trustManager.checkClientTrusted(null, null);
68-
Assert.assertNull(trustManager.getAcceptedIssuers());
69+
}
70+
71+
@Test
72+
public void testAuthNotStrictWithRevokedCert() throws Exception {
73+
Mockito.when(crlDao.findBySerial(Mockito.any(BigInteger.class))).thenReturn(new CrlVO());
74+
final RootCACustomTrustManager trustManager = new RootCACustomTrustManager(clientIp, false, true, certMap, caCertificate, crlDao);
75+
trustManager.checkClientTrusted(new X509Certificate[]{caCertificate}, "RSA");
76+
Assert.assertTrue(certMap.containsKey(clientIp));
77+
Assert.assertEquals(certMap.get(clientIp), caCertificate);
78+
}
79+
80+
@Test
81+
public void testAuthNotStrictWithInvalidCertOwnership() throws Exception {
82+
Mockito.when(crlDao.findBySerial(Mockito.any(BigInteger.class))).thenReturn(null);
83+
final RootCACustomTrustManager trustManager = new RootCACustomTrustManager(clientIp, false, true, certMap, caCertificate, crlDao);
84+
trustManager.checkClientTrusted(new X509Certificate[]{caCertificate}, "RSA");
85+
Assert.assertTrue(certMap.containsKey(clientIp));
86+
Assert.assertEquals(certMap.get(clientIp), caCertificate);
87+
}
88+
89+
@Test(expected = CertificateException.class)
90+
public void testAuthNotStrictWithDenyExpiredCertAndOwnership() throws Exception {
91+
Mockito.when(crlDao.findBySerial(Mockito.any(BigInteger.class))).thenReturn(null);
92+
final RootCACustomTrustManager trustManager = new RootCACustomTrustManager(clientIp, false, false, certMap, caCertificate, crlDao);
93+
trustManager.checkClientTrusted(new X509Certificate[]{expiredClientCertificate}, "RSA");
94+
}
95+
96+
@Test
97+
public void testAuthNotStrictWithAllowExpiredCertAndOwnership() throws Exception {
98+
Mockito.when(crlDao.findBySerial(Mockito.any(BigInteger.class))).thenReturn(null);
99+
final RootCACustomTrustManager trustManager = new RootCACustomTrustManager(clientIp, false, true, certMap, caCertificate, crlDao);
100+
trustManager.checkClientTrusted(new X509Certificate[]{expiredClientCertificate}, "RSA");
101+
Assert.assertTrue(certMap.containsKey(clientIp));
102+
Assert.assertEquals(certMap.get(clientIp), expiredClientCertificate);
69103
}
70104

71105
@Test(expected = CertificateException.class)

‎plugins/ca/root-ca/test/org/apache/cloudstack/ca/provider/RootCAProviderTest.java‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,9 @@
4141
import org.junit.Before;
4242
import org.junit.Test;
4343
import org.junit.runner.RunWith;
44+
4445
import org.mockito.runners.MockitoJUnitRunner;
46+
import org.mockito.Mockito;
4547

4648
@RunWith(MockitoJUnitRunner.class)
4749
public class RootCAProviderTest {
@@ -133,17 +135,20 @@ public void testRevokeCertificate() throws Exception {
133135

134136
@Test
135137
public void testCreateSSLEngineWithoutAuthStrictness() throws Exception {
136-
overrideDefaultConfigValue(RootCAProvider.rootCAAuthStrictness, "_defaultValue", "false");
138+
provider.rootCAAuthStrictness = Mockito.mock(ConfigKey.class);
139+
Mockito.when(provider.rootCAAuthStrictness.value()).thenReturn(Boolean.FALSE);
137140
final SSLEngine e = provider.createSSLEngine(SSLUtils.getSSLContext(), "/1.2.3.4:5678", null);
138-
Assert.assertFalse(e.getUseClientMode());
141+
142+
Assert.assertTrue(e.getWantClientAuth());
139143
Assert.assertFalse(e.getNeedClientAuth());
140144
}
141145

142146
@Test
143147
public void testCreateSSLEngineWithAuthStrictness() throws Exception {
144-
overrideDefaultConfigValue(RootCAProvider.rootCAAuthStrictness, "_defaultValue", "true");
148+
provider.rootCAAuthStrictness = Mockito.mock(ConfigKey.class);
149+
Mockito.when(provider.rootCAAuthStrictness.value()).thenReturn(Boolean.TRUE);
145150
final SSLEngine e = provider.createSSLEngine(SSLUtils.getSSLContext(), "/1.2.3.4:5678", null);
146-
Assert.assertFalse(e.getUseClientMode());
151+
147152
Assert.assertTrue(e.getNeedClientAuth());
148153
}
149154

@@ -152,4 +157,4 @@ public void testGetProviderName() throws Exception {
152157
Assert.assertEquals(provider.getProviderName(), "root");
153158
}
154159

155-
}
160+
}

0 commit comments

Comments
 (0)