Skip to content

Commit d3b0026

Browse files
committed
Address comments
1 parent 516f1f4 commit d3b0026

6 files changed

Lines changed: 19 additions & 18 deletions

File tree

api/src/main/java/org/apache/cloudstack/ca/CAManager.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,9 @@ public interface CAManager extends CAService, Configurable, PluggableService {
158158
* @param sshConnection active SSH connection to the KVM host
159159
* @param agentIp IP address of the KVM host agent
160160
* @param agentHostname hostname of the KVM host agent
161+
* @param caProvider optional CA provider plugin name (null uses default)
161162
*/
162-
void provisionCertificateViaSsh(Connection sshConnection, String agentIp, String agentHostname);
163+
void provisionCertificateViaSsh(Connection sshConnection, String agentIp, String agentHostname, String caProvider);
163164

164165
/**
165166
* Setups up a new keystore and generates CSR for a host

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,20 +106,20 @@ public final class RootCAProvider extends AdapterBase implements CAProvider, Con
106106
private static ConfigKey<String> rootCAPrivateKey = new ConfigKey<>("Hidden", String.class,
107107
"ca.plugin.root.private.key",
108108
null,
109-
"The ROOT CA private key in PEM format (PKCS#8: must start with '-----BEGIN PRIVATE KEY-----'). " +
109+
"The ROOT CA private key in PEM format (PKCS#8: must start with 'BEGIN PRIVATE KEY'). " +
110110
"When set along with the public key and certificate, CloudStack uses this custom CA instead of auto-generating one. " +
111111
"All three ca.plugin.root.* keys must be set together. Restart management server(s) when changed.", true);
112112

113113
private static ConfigKey<String> rootCAPublicKey = new ConfigKey<>("Hidden", String.class,
114114
"ca.plugin.root.public.key",
115115
null,
116-
"The ROOT CA public key in PEM format (X.509/SPKI: must start with '-----BEGIN PUBLIC KEY-----'). " +
116+
"The ROOT CA public key in PEM format (X.509/SPKI: must start with 'BEGIN PUBLIC KEY'). " +
117117
"Required when providing a custom CA. Restart management server(s) when changed.", true);
118118

119119
private static ConfigKey<String> rootCACertificate = new ConfigKey<>("Hidden", String.class,
120120
"ca.plugin.root.ca.certificate",
121121
null,
122-
"The ROOT CA X.509 certificate in PEM format (must start with '-----BEGIN CERTIFICATE-----'). " +
122+
"The ROOT CA X.509 certificate in PEM format (must start with 'BEGIN CERTIFICATE'). " +
123123
"Required when providing a custom CA. Restart management server(s) when changed.", true);
124124

125125
private static ConfigKey<String> rootCAIssuerDN = new ConfigKey<>("Advanced", String.class,
@@ -431,7 +431,7 @@ private boolean setupCA() {
431431
logger.error("Failed to load user-provided CA keys from configuration. " +
432432
"Check that ca.plugin.root.private.key, ca.plugin.root.public.key, and " +
433433
"ca.plugin.root.ca.certificate are all set and in the correct PEM format " +
434-
"(private key must be PKCS#8: '-----BEGIN PRIVATE KEY-----'). " +
434+
"(private key must be PKCS#8: 'BEGIN PRIVATE KEY'). " +
435435
"Overwriting with auto-generated keys.");
436436
}
437437
if (!saveNewRootCAKeypair()) {

scripts/util/keystore-cert-import

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ elif [ ! -f "$CACERT_FILE" ]; then
7070
fi
7171

7272
# Import cacerts into the keystore
73-
awk 'BEGIN{n=0} /-----BEGIN CERTIFICATE-----/{n++}{print > "cloudca." n }' "$CACERT_FILE"
73+
awk 'BEGIN{n=0} /-----BEGIN CERTIFICATE-----/{n++} n>0{print > "cloudca." n }' "$CACERT_FILE"
7474
for caChain in $(ls cloudca.* 2>/dev/null); do
7575
keytool -delete -noprompt -alias "$caChain" -keystore "$KS_FILE" -storepass "$KS_PASS" > /dev/null 2>&1 || true
7676
keytool -import -noprompt -storepass "$KS_PASS" -trustcacerts -alias "$caChain" -file "$caChain" -keystore "$KS_FILE" > /dev/null 2>&1
@@ -140,10 +140,10 @@ if [ -f "$SYSTEM_FILE" ]; then
140140
# Import CA cert(s) into realhostip.keystore so the SSVM JVM
141141
# (which overrides the truststore via -Djavax.net.ssl.trustStore in _run.sh)
142142
# can trust servers signed by the CloudStack CA
143-
REALHOSTIP_KS_FILE="$(dirname $(dirname $PROPS_FILE))/certs/realhostip.keystore"
143+
REALHOSTIP_KS_FILE="$(dirname "$(dirname "$PROPS_FILE")")/certs/realhostip.keystore"
144144
REALHOSTIP_PASS="vmops.com"
145145
if [ -f "$REALHOSTIP_KS_FILE" ]; then
146-
awk 'BEGIN{n=0} /-----BEGIN CERTIFICATE-----/{n++}{print > "cloudca." n }' "$CACERT_FILE"
146+
awk 'BEGIN{n=0} /-----BEGIN CERTIFICATE-----/{n++} n>0{print > "cloudca." n }' "$CACERT_FILE"
147147
for caChain in $(ls cloudca.* 2>/dev/null); do
148148
keytool -delete -noprompt -alias "$caChain" -keystore "$REALHOSTIP_KS_FILE" \
149149
-storepass "$REALHOSTIP_PASS" > /dev/null 2>&1 || true

server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ private void setupAgentSecurity(final Connection sshConnection, final String age
169169
throw new CloudRuntimeException("Cannot secure agent communication because SSH connection is invalid for host IP=" + agentIp);
170170
}
171171

172-
caManager.provisionCertificateViaSsh(sshConnection, agentIp, agentHostname);
172+
caManager.provisionCertificateViaSsh(sshConnection, agentIp, agentHostname, null);
173173

174174
if (logger.isDebugEnabled()) {
175175
logger.debug("Succeeded to import certificate in the keystore for agent on the KVM host: " + agentIp + ". Agent secured and trusted.");

server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -230,15 +230,15 @@ public boolean provisionCertificate(final Host host, final Boolean reconnect, fi
230230

231231
private boolean provisionCertificateForced(Host host, Boolean reconnect, String caProvider) {
232232
if (host.getType() == Host.Type.Routing && host.getHypervisorType() == com.cloud.hypervisor.Hypervisor.HypervisorType.KVM) {
233-
return provisionKvmHostViaSsh(host);
233+
return provisionKvmHostViaSsh(host, caProvider);
234234
} else if (host.getType() == Host.Type.ConsoleProxy || host.getType() == Host.Type.SecondaryStorageVM) {
235-
return provisionSystemVmViaSsh(host, reconnect);
235+
return provisionSystemVmViaSsh(host, reconnect, caProvider);
236236
}
237237
throw new CloudRuntimeException("Forced certificate provisioning is only supported for KVM hosts and SystemVMs.");
238238
}
239239

240240
@Override
241-
public void provisionCertificateViaSsh(final Connection sshConnection, final String agentIp, final String agentHostname) {
241+
public void provisionCertificateViaSsh(final Connection sshConnection, final String agentIp, final String agentHostname, final String caProvider) {
242242
Integer validityPeriod = CAManager.CertValidityPeriod.value();
243243
if (validityPeriod < 1) {
244244
validityPeriod = 1;
@@ -266,7 +266,7 @@ public void provisionCertificateViaSsh(final Connection sshConnection, final Str
266266
// 2. Issue Certificate based on returned CSR
267267
final String csr = keystoreSetupResult.getStdOut();
268268
final Certificate certificate = issueCertificate(csr, Arrays.asList(agentHostname, agentIp),
269-
Collections.singletonList(agentIp), null, null);
269+
Collections.singletonList(agentIp), null, caProvider);
270270

271271
if (certificate == null || certificate.getClientCertificate() == null) {
272272
throw new CloudRuntimeException("Failed to issue certificates for host: " + agentIp);
@@ -297,7 +297,7 @@ public void provisionCertificateViaSsh(final Connection sshConnection, final Str
297297
}
298298
}
299299

300-
private boolean provisionKvmHostViaSsh(Host host) {
300+
private boolean provisionKvmHostViaSsh(Host host, String caProvider) {
301301
final HostVO hostVO = (HostVO) host;
302302
hostDao.loadDetails(hostVO);
303303
String username = hostVO.getDetail(ApiConstants.USERNAME);
@@ -321,7 +321,7 @@ private boolean provisionKvmHostViaSsh(Host host) {
321321
}
322322
}
323323

324-
provisionCertificateViaSsh(sshConnection, hostIp, host.getName());
324+
provisionCertificateViaSsh(sshConnection, hostIp, host.getName(), caProvider);
325325

326326
SSHCmdHelper.sshExecuteCmd(sshConnection, "sudo service cloudstack-agent restart");
327327
return true;
@@ -335,7 +335,7 @@ private boolean provisionKvmHostViaSsh(Host host) {
335335
}
336336
}
337337

338-
private boolean provisionSystemVmViaSsh(Host host, Boolean reconnect) {
338+
private boolean provisionSystemVmViaSsh(Host host, Boolean reconnect, String caProvider) {
339339
VMInstanceVO vm = vmInstanceDao.findVMByInstanceName(host.getName());
340340
if (vm == null) {
341341
throw new CloudRuntimeException("Cannot find underlying VM for host: " + host.getName());
@@ -352,7 +352,7 @@ private boolean provisionSystemVmViaSsh(Host host, Boolean reconnect) {
352352
}
353353

354354
final Certificate certificate = issueCertificate(null, Arrays.asList(vm.getHostName(), vm.getInstanceName()),
355-
new ArrayList<>(ipAddressDetails.values()), CertValidityPeriod.value(), null);
355+
new ArrayList<>(ipAddressDetails.values()), CertValidityPeriod.value(), caProvider);
356356
return deployCertificate(hypervisorHost, certificate, reconnect, sshAccessDetails);
357357
} catch (Exception e) {
358358
logger.error("Failed to provision system VM " + host.getName() + " via hypervisor SSH proxy. Ensure the hypervisor host is connected.", e);

systemvm/patch-sysvms.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ patch_systemvm() {
139139
CACERT_FILE="/usr/local/share/ca-certificates/cloudstack/ca.crt"
140140

141141
if [ -f "$CACERT_FILE" ] && [ -f "$REALHOSTIP_KS_FILE" ]; then
142-
awk 'BEGIN{n=0} /-----BEGIN CERTIFICATE-----/{n++}{print > "cloudca." n }' "$CACERT_FILE"
142+
awk 'BEGIN{n=0} /-----BEGIN CERTIFICATE-----/{n++} n>0{print > "cloudca." n }' "$CACERT_FILE"
143143
for caChain in $(ls cloudca.* 2>/dev/null); do
144144
keytool -delete -noprompt -alias "$caChain" -keystore "$REALHOSTIP_KS_FILE" \
145145
-storepass "$REALHOSTIP_PASS" > /dev/null 2>&1 || true

0 commit comments

Comments
 (0)