Skip to content

Commit 12c850e

Browse files
nvazqueznvazquez
authored
KVM: Improvements on upload direct download certificates (#2995)
* Improvements on upload direct download certificates * Move upload direct download certificate logic to KVM plugin * Extend unit test certificate expiration days * Add marvin tests and command to revoke certificates * Review comments * Do not include revoke certificates API
1 parent c9ce3e2 commit 12c850e

8 files changed

Lines changed: 472 additions & 42 deletions

File tree

‎agent/src/com/cloud/agent/Agent.java‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939

4040
import javax.naming.ConfigurationException;
4141

42-
import org.apache.cloudstack.agent.directdownload.SetupDirectDownloadCertificate;
4342
import org.apache.cloudstack.agent.lb.SetupMSListAnswer;
4443
import org.apache.cloudstack.agent.lb.SetupMSListCommand;
4544
import org.apache.cloudstack.ca.PostCertificateRenewalCommand;
@@ -630,8 +629,6 @@ protected void processRequest(final Request request, final Link link) {
630629
if (Host.Type.Routing.equals(_resource.getType())) {
631630
scheduleServicesRestartTask();
632631
}
633-
} else if (cmd instanceof SetupDirectDownloadCertificate) {
634-
answer = setupDirectDownloadCertificate((SetupDirectDownloadCertificate) cmd);
635632
} else if (cmd instanceof SetupMSListCommand) {
636633
answer = setupManagementServerList((SetupMSListCommand) cmd);
637634
} else {
@@ -683,31 +680,6 @@ protected void processRequest(final Request request, final Link link) {
683680
}
684681
}
685682

686-
private Answer setupDirectDownloadCertificate(SetupDirectDownloadCertificate cmd) {
687-
String certificate = cmd.getCertificate();
688-
String certificateName = cmd.getCertificateName();
689-
s_logger.info("Importing certificate " + certificateName + " into keystore");
690-
691-
final File agentFile = PropertiesUtil.findConfigFile("agent.properties");
692-
if (agentFile == null) {
693-
return new Answer(cmd, false, "Failed to find agent.properties file");
694-
}
695-
696-
final String keyStoreFile = agentFile.getParent() + "/" + KeyStoreUtils.KS_FILENAME;
697-
698-
String cerFile = agentFile.getParent() + "/" + certificateName + ".cer";
699-
Script.runSimpleBashScript(String.format("echo '%s' > %s", certificate, cerFile));
700-
701-
String privatePasswordFormat = "sed -n '/keystore.passphrase/p' '%s' 2>/dev/null | sed 's/keystore.passphrase=//g' 2>/dev/null";
702-
String privatePasswordCmd = String.format(privatePasswordFormat, agentFile.getAbsolutePath());
703-
String privatePassword = Script.runSimpleBashScript(privatePasswordCmd);
704-
705-
String importCommandFormat = "keytool -importcert -file %s -keystore %s -alias '%s' -storepass '%s' -noprompt";
706-
String importCmd = String.format(importCommandFormat, cerFile, keyStoreFile, certificateName, privatePassword);
707-
Script.runSimpleBashScript(importCmd);
708-
return new Answer(cmd, true, "Certificate " + certificateName + " imported");
709-
}
710-
711683
public Answer setupAgentKeystore(final SetupKeyStoreCommand cmd) {
712684
final String keyStorePassword = cmd.getKeystorePassword();
713685
final long validityDays = cmd.getValidityDays();

api/src/org/apache/cloudstack/api/command/admin/direct/download/UploadTemplateDirectDownloadCertificate.java renamed to api/src/org/apache/cloudstack/api/command/admin/direct/download/UploadTemplateDirectDownloadCertificateCmd.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,19 +35,19 @@
3535

3636
import javax.inject.Inject;
3737

38-
@APICommand(name = UploadTemplateDirectDownloadCertificate.APINAME,
38+
@APICommand(name = UploadTemplateDirectDownloadCertificateCmd.APINAME,
3939
description = "Upload a certificate for HTTPS direct template download on KVM hosts",
4040
responseObject = SuccessResponse.class,
4141
requestHasSensitiveInfo = true,
4242
responseHasSensitiveInfo = true,
4343
since = "4.11.0",
4444
authorized = {RoleType.Admin})
45-
public class UploadTemplateDirectDownloadCertificate extends BaseCmd {
45+
public class UploadTemplateDirectDownloadCertificateCmd extends BaseCmd {
4646

4747
@Inject
4848
DirectDownloadManager directDownloadManager;
4949

50-
private static final Logger LOG = Logger.getLogger(UploadTemplateDirectDownloadCertificate.class);
50+
private static final Logger LOG = Logger.getLogger(UploadTemplateDirectDownloadCertificateCmd.class);
5151
public static final String APINAME = "uploadTemplateDirectDownloadCertificate";
5252

5353
@Parameter(name = ApiConstants.CERTIFICATE, type = BaseCmd.CommandType.STRING, required = true, length = 65535,

core/src/org/apache/cloudstack/agent/directdownload/SetupDirectDownloadCertificate.java renamed to core/src/org/apache/cloudstack/agent/directdownload/SetupDirectDownloadCertificateCommand.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@
2020

2121
import com.cloud.agent.api.Command;
2222

23-
public class SetupDirectDownloadCertificate extends Command {
23+
public class SetupDirectDownloadCertificateCommand extends Command {
2424

2525
private String certificate;
2626
private String certificateName;
2727

28-
public SetupDirectDownloadCertificate(String certificate, String name) {
28+
public SetupDirectDownloadCertificateCommand(String certificate, String name) {
2929
this.certificate = certificate;
3030
this.certificateName = name;
3131
}

‎plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetupDirectDownloadCertificateCommandWrapper.java‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
//
2+
// Licensed to the Apache Software Foundation (ASF) under one
3+
// or more contributor license agreements. See the NOTICE file
4+
// distributed with this work for additional information
5+
// regarding copyright ownership. The ASF licenses this file
6+
// to you under the Apache License, Version 2.0 (the
7+
// "License"); you may not use this file except in compliance
8+
// with the License. You may obtain a copy of the License at
9+
//
10+
// http://www.apache.org/licenses/LICENSE-2.0
11+
//
12+
// Unless required by applicable law or agreed to in writing,
13+
// software distributed under the License is distributed on an
14+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
// KIND, either express or implied. See the License for the
16+
// specific language governing permissions and limitations
17+
// under the License.
18+
//
19+
package com.cloud.hypervisor.kvm.resource.wrapper;
20+
21+
import com.cloud.agent.api.Answer;
22+
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
23+
import com.cloud.resource.CommandWrapper;
24+
import com.cloud.resource.ResourceWrapper;
25+
import com.cloud.utils.PropertiesUtil;
26+
import com.cloud.utils.exception.CloudRuntimeException;
27+
import com.cloud.utils.script.Script;
28+
import org.apache.cloudstack.agent.directdownload.SetupDirectDownloadCertificateCommand;
29+
import org.apache.cloudstack.utils.security.KeyStoreUtils;
30+
import org.apache.log4j.Logger;
31+
32+
import java.io.File;
33+
import java.io.FileNotFoundException;
34+
import java.io.IOException;
35+
36+
import static org.apache.commons.lang.StringUtils.isBlank;
37+
38+
@ResourceWrapper(handles = SetupDirectDownloadCertificateCommand.class)
39+
public class LibvirtSetupDirectDownloadCertificateCommandWrapper extends CommandWrapper<SetupDirectDownloadCertificateCommand, Answer, LibvirtComputingResource> {
40+
41+
private static final String temporaryCertFilePrefix = "CSCERTIFICATE";
42+
43+
private static final Logger s_logger = Logger.getLogger(LibvirtSetupDirectDownloadCertificateCommandWrapper.class);
44+
45+
/**
46+
* Retrieve agent.properties file
47+
*/
48+
private File getAgentPropertiesFile() throws FileNotFoundException {
49+
final File agentFile = PropertiesUtil.findConfigFile("agent.properties");
50+
if (agentFile == null) {
51+
throw new FileNotFoundException("Failed to find agent.properties file");
52+
}
53+
return agentFile;
54+
}
55+
56+
/**
57+
* Get the property 'keystore.passphrase' value from agent.properties file
58+
*/
59+
private String getKeystorePassword(File agentFile) {
60+
String pass = null;
61+
if (agentFile != null) {
62+
try {
63+
pass = PropertiesUtil.loadFromFile(agentFile).getProperty(KeyStoreUtils.KS_PASSPHRASE_PROPERTY);
64+
} catch (IOException e) {
65+
s_logger.error("Could not get 'keystore.passphrase' property value due to: " + e.getMessage());
66+
}
67+
}
68+
return pass;
69+
}
70+
71+
/**
72+
* Get keystore path
73+
*/
74+
private String getKeyStoreFilePath(File agentFile) {
75+
return agentFile.getParent() + "/" + KeyStoreUtils.KS_FILENAME;
76+
}
77+
78+
/**
79+
* Import certificate from temporary file into keystore
80+
*/
81+
private void importCertificate(String tempCerFilePath, String keyStoreFile, String certificateName, String privatePassword) {
82+
s_logger.debug("Importing certificate from temporary file to keystore");
83+
String importCommandFormat = "keytool -importcert -file %s -keystore %s -alias '%s' -storepass '%s' -noprompt";
84+
String importCmd = String.format(importCommandFormat, tempCerFilePath, keyStoreFile, certificateName, privatePassword);
85+
int result = Script.runSimpleBashScriptForExitValue(importCmd);
86+
if (result != 0) {
87+
s_logger.debug("Certificate " + certificateName + " not imported as it already exist on keystore");
88+
}
89+
}
90+
91+
/**
92+
* Create temporary file and return its path
93+
*/
94+
private String createTemporaryFile(File agentFile, String certificateName, String certificate) {
95+
String tempCerFilePath = String.format("%s/%s-%s",
96+
agentFile.getParent(), temporaryCertFilePrefix, certificateName);
97+
s_logger.debug("Creating temporary certificate file into: " + tempCerFilePath);
98+
int result = Script.runSimpleBashScriptForExitValue(String.format("echo '%s' > %s", certificate, tempCerFilePath));
99+
if (result != 0) {
100+
throw new CloudRuntimeException("Could not create the certificate file on path: " + tempCerFilePath);
101+
}
102+
return tempCerFilePath;
103+
}
104+
105+
/**
106+
* Remove temporary file
107+
*/
108+
private void cleanupTemporaryFile(String temporaryFile) {
109+
s_logger.debug("Cleaning up temporary certificate file");
110+
Script.runSimpleBashScript("rm -f " + temporaryFile);
111+
}
112+
113+
@Override
114+
public Answer execute(SetupDirectDownloadCertificateCommand cmd, LibvirtComputingResource serverResource) {
115+
String certificate = cmd.getCertificate();
116+
String certificateName = cmd.getCertificateName();
117+
118+
try {
119+
File agentFile = getAgentPropertiesFile();
120+
String privatePassword = getKeystorePassword(agentFile);
121+
if (isBlank(privatePassword)) {
122+
return new Answer(cmd, false, "No password found for keystore: " + KeyStoreUtils.KS_FILENAME);
123+
}
124+
125+
final String keyStoreFile = getKeyStoreFilePath(agentFile);
126+
String temporaryFile = createTemporaryFile(agentFile, certificateName, certificate);
127+
importCertificate(temporaryFile, keyStoreFile, certificateName, privatePassword);
128+
cleanupTemporaryFile(temporaryFile);
129+
} catch (FileNotFoundException | CloudRuntimeException e) {
130+
s_logger.error("Error while setting up certificate " + certificateName, e);
131+
return new Answer(cmd, false, e.getMessage());
132+
}
133+
134+
return new Answer(cmd, true, "Certificate " + certificateName + " imported");
135+
}
136+
}

‎server/src/com/cloud/server/ManagementServerImpl.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@
6565
import org.apache.cloudstack.api.command.admin.config.ListHypervisorCapabilitiesCmd;
6666
import org.apache.cloudstack.api.command.admin.config.UpdateCfgCmd;
6767
import org.apache.cloudstack.api.command.admin.config.UpdateHypervisorCapabilitiesCmd;
68-
import org.apache.cloudstack.api.command.admin.direct.download.UploadTemplateDirectDownloadCertificate;
68+
import org.apache.cloudstack.api.command.admin.direct.download.UploadTemplateDirectDownloadCertificateCmd;
6969
import org.apache.cloudstack.api.command.admin.domain.CreateDomainCmd;
7070
import org.apache.cloudstack.api.command.admin.domain.DeleteDomainCmd;
7171
import org.apache.cloudstack.api.command.admin.domain.ListDomainChildrenCmd;
@@ -3051,7 +3051,7 @@ public List<Class<?>> getCommands() {
30513051
cmdList.add(ReleasePodIpCmdByAdmin.class);
30523052
cmdList.add(CreateManagementNetworkIpRangeCmd.class);
30533053
cmdList.add(DeleteManagementNetworkIpRangeCmd.class);
3054-
cmdList.add(UploadTemplateDirectDownloadCertificate.class);
3054+
cmdList.add(UploadTemplateDirectDownloadCertificateCmd.class);
30553055

30563056
// Out-of-band management APIs for admins
30573057
cmdList.add(EnableOutOfBandManagementForHostCmd.class);

‎server/src/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java‎

Lines changed: 69 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@
4141

4242
import java.net.URI;
4343
import java.net.URISyntaxException;
44+
import java.security.cert.Certificate;
45+
import java.security.cert.CertificateException;
46+
import java.security.cert.CertificateExpiredException;
47+
import java.security.cert.CertificateNotYetValidException;
4448
import java.util.ArrayList;
4549
import java.util.HashMap;
4650
import java.util.List;
@@ -50,14 +54,15 @@
5054
import java.util.stream.Collectors;
5155
import javax.inject.Inject;
5256

57+
import com.cloud.utils.security.CertificateHelper;
5358
import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand;
5459
import org.apache.cloudstack.agent.directdownload.DirectDownloadAnswer;
5560
import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand.DownloadProtocol;
5661
import org.apache.cloudstack.agent.directdownload.HttpDirectDownloadCommand;
5762
import org.apache.cloudstack.agent.directdownload.MetalinkDirectDownloadCommand;
5863
import org.apache.cloudstack.agent.directdownload.NfsDirectDownloadCommand;
5964
import org.apache.cloudstack.agent.directdownload.HttpsDirectDownloadCommand;
60-
import org.apache.cloudstack.agent.directdownload.SetupDirectDownloadCertificate;
65+
import org.apache.cloudstack.agent.directdownload.SetupDirectDownloadCertificateCommand;
6166
import org.apache.cloudstack.context.CallContext;
6267
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
6368
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
@@ -69,6 +74,7 @@
6974
import org.apache.commons.collections.CollectionUtils;
7075
import org.apache.commons.collections.MapUtils;
7176
import org.apache.log4j.Logger;
77+
import sun.security.x509.X509CertImpl;
7278

7379
public class DirectDownloadManagerImpl extends ManagerBase implements DirectDownloadManager {
7480

@@ -313,14 +319,66 @@ private List<HostVO> getRunningHostsToUploadCertificate(HypervisorType hyperviso
313319
.collect(Collectors.toList());
314320
}
315321

322+
/**
323+
* Return pretified PEM certificate
324+
*/
325+
protected String getPretifiedCertificate(String certificateCer) {
326+
String cert = certificateCer.replaceAll("(.{64})", "$1\n");
327+
if (!cert.startsWith(BEGIN_CERT) && !cert.endsWith(END_CERT)) {
328+
cert = BEGIN_CERT + LINE_SEPARATOR + cert + LINE_SEPARATOR + END_CERT;
329+
}
330+
return cert;
331+
}
332+
333+
/**
334+
* Generate and return certificate from the string
335+
* @throws CloudRuntimeException if the certificate is not well formed
336+
*/
337+
private Certificate getCertificateFromString(String certificatePem) {
338+
try {
339+
return CertificateHelper.buildCertificate(certificatePem);
340+
} catch (CertificateException e) {
341+
e.printStackTrace();
342+
throw new CloudRuntimeException("Cannot parse the certificate provided, please provide a PEM certificate. Error: " + e.getMessage());
343+
}
344+
}
345+
346+
/**
347+
* Perform sanity of string parsed certificate
348+
*/
349+
protected void certificateSanity(String certificatePem) {
350+
Certificate certificate = getCertificateFromString(certificatePem);
351+
352+
if (certificate instanceof X509CertImpl) {
353+
X509CertImpl x509Cert = (X509CertImpl) certificate;
354+
try {
355+
x509Cert.checkValidity();
356+
} catch (CertificateExpiredException | CertificateNotYetValidException e) {
357+
String msg = "Certificate is invalid. Please provide a valid certificate. Error: " + e.getMessage();
358+
s_logger.error(msg);
359+
throw new CloudRuntimeException(msg);
360+
}
361+
if (x509Cert.getSubjectDN() != null) {
362+
s_logger.debug("Valid certificate for domain name: " + x509Cert.getSubjectDN().getName());
363+
}
364+
}
365+
}
366+
316367
@Override
317-
public boolean uploadCertificateToHosts(String certificateCer, String certificateName, String hypervisor) {
368+
public boolean uploadCertificateToHosts(String certificateCer, String alias, String hypervisor) {
318369
HypervisorType hypervisorType = HypervisorType.getType(hypervisor);
319370
List<HostVO> hosts = getRunningHostsToUploadCertificate(hypervisorType);
371+
372+
String certificatePem = getPretifiedCertificate(certificateCer);
373+
certificateSanity(certificatePem);
374+
375+
s_logger.info("Attempting to upload certificate: " + alias + " to " + hosts.size() + " hosts");
320376
if (CollectionUtils.isNotEmpty(hosts)) {
321377
for (HostVO host : hosts) {
322-
if (!uploadCertificate(certificateCer, certificateName, host.getId())) {
323-
throw new CloudRuntimeException("Uploading certificate " + certificateName + " failed on host: " + host.getId());
378+
if (!uploadCertificate(certificatePem, alias, host.getId())) {
379+
String msg = "Could not upload certificate " + alias + " on host: " + host.getName() + " (" + host.getUuid() + ")";
380+
s_logger.error(msg);
381+
throw new CloudRuntimeException(msg);
324382
}
325383
}
326384
}
@@ -331,14 +389,18 @@ public boolean uploadCertificateToHosts(String certificateCer, String certificat
331389
* Upload and import certificate to hostId on keystore
332390
*/
333391
protected boolean uploadCertificate(String certificate, String certificateName, long hostId) {
334-
String cert = certificate.replaceAll("(.{64})", "$1\n");
335-
final String prettified_cert = BEGIN_CERT + LINE_SEPARATOR + cert + LINE_SEPARATOR + END_CERT;
336-
SetupDirectDownloadCertificate cmd = new SetupDirectDownloadCertificate(prettified_cert, certificateName);
392+
SetupDirectDownloadCertificateCommand cmd = new SetupDirectDownloadCertificateCommand(certificate, certificateName);
337393
Answer answer = agentManager.easySend(hostId, cmd);
338394
if (answer == null || !answer.getResult()) {
395+
String msg = "Certificate " + certificateName + " could not be added to host " + hostId;
396+
if (answer != null) {
397+
msg += " due to: " + answer.getDetails();
398+
}
399+
s_logger.error(msg);
339400
return false;
340401
}
341402
s_logger.info("Certificate " + certificateName + " successfully uploaded to host: " + hostId);
342403
return true;
343404
}
405+
344406
}

0 commit comments

Comments
 (0)