apache/cloudstack
1
0
Fork 0
mirror of https://github.com/apache/cloudstack.git synced 2025-10-26 08:42:29 +01:00
Code Issues Packages Projects Releases Wiki Activity

server: Enable revocation checking for uploaded certificates (#4065)

Browse Source 
This update turns on certificate revocation checking for uploaded certificates:

- Updated `CertServiceImpl` to be able to enable revocation checking.
- Introduced a new parameter `ENABLED_REVOCATION_CHECK` for `UploadSslCertCmd`.
- Updated `CertServiceTest`.

Even if no CLRs are specified via `PKIXParameters`, the certificates
themselves may still provide info for revocation checking:

- The AIA extension may contains a URL to the OCSP responder.
- The CLRDP extension contains a URL to the CLR.

Those extensions may need to be explicitly enabled by setting the system properties `com.sun.security.enableAIAcaIssuers` and `com.sun.security.enableCRLDP` to true. See [Java PKI Programmer's Guide](https://docs.oracle.com/en/java/javase/11/security/java-pki-programmers-guide.html).

Using a revoked certificate may be dangerous. One of the most common reasons why a certificate authority (CA) revokes a certificate is that the private key has been compromised. For example, the private key might have been stolen by an adversary.

If I understand correctly, the `CertServiceImpl` bean is used for operations with certificates on a load balancer. In particular, it validates a certificate chain without revocation checking while uploading a certificate. If a compromised revoked certificate is then used by the load balancer, then it may result to compromising TLS connections. However, the attacker has to be able to implement man-in-the-middle attack to compromise the connections. So the attacker has to be quite powerful. Therefore, such an attack is definitely not easy to implement. On the other hand, the impact may be significant because of loss of confidentiality.

This has been discussed on security@cloudstack.apache.org
This commit is contained in:
Artem Smotrakov 2020-06-04 04:17:05 +02:00 committed by GitHub
parent 8c4c148718
commit 28a19311f4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 79 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
View File

@ -55,6 +55,7 @@ public class ApiConstants {
public static final String CERTIFICATE_CHAIN = "certchain";
public static final String CERTIFICATE_FINGERPRINT = "fingerprint";
public static final String CERTIFICATE_ID = "certid";
public static final String ENABLED_REVOCATION_CHECK = "enabledrevocationcheck";
public static final String CONTROLLER = "controller";
public static final String CONTROLLER_UNIT = "controllerunit";
public static final String COPY_IMAGE_TAGS = "copyimagetags";

7
api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/UploadSslCertCmd.java
View File

@ -76,6 +76,9 @@ public class UploadSslCertCmd extends BaseCmd {
@Parameter(name = ApiConstants.NAME , type = CommandType.STRING, required = true, description = "Name for the uploaded certificate")
private String name;
@Parameter(name = ApiConstants.ENABLED_REVOCATION_CHECK, type = CommandType.BOOLEAN, description = "Enables revocation checking for certificates", since = "4.15")
private Boolean enabledRevocationCheck = Boolean.TRUE;
/////////////////////////////////////////////////////
/////////////////// Accessors ///////////////////////
/////////////////////////////////////////////////////
@ -112,6 +115,10 @@ public class UploadSslCertCmd extends BaseCmd {
return name;
}
public Boolean getEnabledRevocationCheck() {
return enabledRevocationCheck;
}
/////////////////////////////////////////////////////
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////

10
server/src/main/java/org/apache/cloudstack/network/ssl/CertServiceImpl.java
View File

@ -125,7 +125,7 @@ public class CertServiceImpl implements CertService {
final String chain = certCmd.getChain();
final String name = certCmd.getName();
validate(cert, key, password, chain);
validate(cert, key, password, chain, certCmd.getEnabledRevocationCheck());
s_logger.debug("Certificate Validation succeeded");
final String fingerPrint = CertificateHelper.generateFingerPrint(parseCertificate(cert));
@ -278,7 +278,7 @@ public class CertServiceImpl implements CertService {
return certResponseList;
}
private void validate(final String certInput, final String keyInput, final String password, final String chainInput) {
private void validate(final String certInput, final String keyInput, final String password, final String chainInput, boolean revocationEnabled) {
try {
List<Certificate> chain = null;
final Certificate cert = parseCertificate(certInput);
@ -292,7 +292,7 @@ public class CertServiceImpl implements CertService {
validateKeys(cert.getPublicKey(), key);
if (chainInput != null) {
validateChain(chain, cert);
validateChain(chain, cert, revocationEnabled);
}
} catch (final IOException | CertificateException e) {
throw new IllegalStateException("Parsing certificate/key failed: " + e.getMessage(), e);
@ -388,7 +388,7 @@ public class CertServiceImpl implements CertService {
}
}
private void validateChain(final List<Certificate> chain, final Certificate cert) {
private void validateChain(final List<Certificate> chain, final Certificate cert, boolean revocationEnabled) {
final List<Certificate> certs = new ArrayList<Certificate>();
final Set<TrustAnchor> anchors = new HashSet<TrustAnchor>();
@ -410,7 +410,7 @@ public class CertServiceImpl implements CertService {
PKIXBuilderParameters params = null;
try {
params = new PKIXBuilderParameters(anchors, target);
params.setRevocationEnabled(false);
params.setRevocationEnabled(revocationEnabled);
params.addCertStore(CertStore.getInstance("Collection", new CollectionCertStoreParameters(certs)));
final CertPathBuilder builder = CertPathBuilder.getInstance("PKIX", "BC");
builder.build(params);

65
server/src/test/java/org/apache/cloudstack/network/ssl/CertServiceTest.java
View File

@ -140,9 +140,74 @@ public class CertServiceTest {
chainField.setAccessible(true);
chainField.set(uploadCmd, chain);
final Field enabledRevocationCheckField = klazz.getDeclaredField("enabledRevocationCheck");
enabledRevocationCheckField.setAccessible(true);
enabledRevocationCheckField.set(uploadCmd, Boolean.FALSE);
certService.uploadSslCert(uploadCmd);
}
@Test
/**
* Given a certificate signed by a CA and a valid CA chain, but without any info for revocation checking, upload should fail.
*/
public void runUploadSslCertWithNoRevocationInfo() throws Exception {
Assume.assumeTrue(isOpenJdk() || isJCEInstalled());
TransactionLegacy.open("runUploadSslCertWithCAChain");
final String certFile = URLDecoder.decode(getClass().getResource("/certs/rsa_ca_signed.crt").getFile(),Charset.defaultCharset().name());
final String keyFile = URLDecoder.decode(getClass().getResource("/certs/rsa_ca_signed.key").getFile(),Charset.defaultCharset().name());
final String chainFile = URLDecoder.decode(getClass().getResource("/certs/root_chain.crt").getFile(),Charset.defaultCharset().name());
final String cert = readFileToString(new File(certFile));
final String key = readFileToString(new File(keyFile));
final String chain = readFileToString(new File(chainFile));
final CertServiceImpl certService = new CertServiceImpl();
//setting mock objects
certService._accountMgr = Mockito.mock(AccountManager.class);
final Account account = new AccountVO("testaccount", 1, "networkdomain", (short)0, UUID.randomUUID().toString());
when(certService._accountMgr.getAccount(anyLong())).thenReturn(account);
certService._domainDao = Mockito.mock(DomainDao.class);
final DomainVO domain = new DomainVO("networkdomain", 1L, 1L, "networkdomain");
when(certService._domainDao.findByIdIncludingRemoved(anyLong())).thenReturn(domain);
certService._sslCertDao = Mockito.mock(SslCertDao.class);
when(certService._sslCertDao.persist(Matchers.any(SslCertVO.class))).thenReturn(new SslCertVO());
certService._accountDao = Mockito.mock(AccountDao.class);
when(certService._accountDao.findByIdIncludingRemoved(anyLong())).thenReturn((AccountVO)account);
//creating the command
final UploadSslCertCmd uploadCmd = new UploadSslCertCmdExtn();
final Class<?> klazz = uploadCmd.getClass().getSuperclass();
final Field certField = klazz.getDeclaredField("cert");
certField.setAccessible(true);
certField.set(uploadCmd, cert);
final Field keyField = klazz.getDeclaredField("key");
keyField.setAccessible(true);
keyField.set(uploadCmd, key);
final Field chainField = klazz.getDeclaredField("chain");
chainField.setAccessible(true);
chainField.set(uploadCmd, chain);
try {
certService.uploadSslCert(uploadCmd);
} catch (Exception e) {
Assert.assertTrue(e.getMessage().contains("Invalid certificate chain"));
Throwable cause = e.getCause();
Assert.assertTrue(cause.getMessage().contains("Certification path could not be validated"));
cause = cause.getCause();
Assert.assertTrue(cause.getMessage().contains("No CRLs found"));
}
}
// @Test
/**
* Given a Self-signed Certificate with encrypted key, upload should succeed