mirror of https://github.com/apache/cloudstack.git
kvm: Allow ssvm agent certs to contain host IP for NAT situations (#206)
There are some networking setups where system VM communications are proxied off of the hypervisor host on which the system VM is running. For example, if the KVM management network is a NAT bridge, or the network plugin employs user mode network for system VM management interfaces, then system VM agent comms look as though they come form the hypervisor host. In such a setup, the certificate authentication for agents fails because the source IP is that of the host of the system VM, rather than the system VM itself, and this IP is not in the connecting certificate presented. This PR adds a configuration value that allows the system VM cert to contain the host IP that the system VM is scheduled on. This allows such setups to maintain auth strictness on agent auth. Co-authored-by: Marcus Sorensen <mls@apple.com>
This commit is contained in:
parent
d83c70cd25
commit
89dfb54929
|
|
@ -62,6 +62,11 @@ public interface CAManager extends CAService, Configurable, PluggableService {
|
|||
"true",
|
||||
"Enable automatic renewal and provisioning of certificate to agents as supported by the configured CA plugin.", true, ConfigKey.Scope.Cluster);
|
||||
|
||||
ConfigKey<Boolean> AllowHostIPInSysVMAgentCert = new ConfigKey<>("Advanced", Boolean.class,
|
||||
"ca.framework.cert.systemvm.allow.host.ip",
|
||||
"false",
|
||||
"Allow hypervisor host's IP to be a part of a system VM's agent cert", true, ConfigKey.Scope.Zone);
|
||||
|
||||
ConfigKey<Long> CABackgroundJobDelay = new ConfigKey<>("Advanced", Long.class,
|
||||
"ca.framework.background.task.delay",
|
||||
"3600",
|
||||
|
|
|
|||
|
|
@ -39,6 +39,7 @@ public abstract class NetworkElementCommand extends Command {
|
|||
public static final String VPC_PRIVATE_GATEWAY = "vpc.gateway.private";
|
||||
public static final String FIREWALL_EGRESS_DEFAULT = "firewall.egress.default";
|
||||
public static final String NETWORK_PUB_LAST_IP = "network.public.last.ip";
|
||||
public static final String HYPERVISOR_HOST_PRIVATE_IP = "hypervisor.private.ip";
|
||||
|
||||
private String routerAccessIp;
|
||||
|
||||
|
|
|
|||
|
|
@ -1007,6 +1007,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
|
|||
if (!Strings.isNullOrEmpty(csr)) {
|
||||
final Map<String, String> ipAddressDetails = new HashMap<>(sshAccessDetails);
|
||||
ipAddressDetails.remove(NetworkElementCommand.ROUTER_NAME);
|
||||
addHostIpToCertDetailsIfConfigAllows(vmHost, ipAddressDetails, CAManager.AllowHostIPInSysVMAgentCert);
|
||||
final Certificate certificate = caManager.issueCertificate(csr, Arrays.asList(vm.getHostName(), vm.getInstanceName()),
|
||||
new ArrayList<>(ipAddressDetails.values()), CAManager.CertValidityPeriod.value(), null);
|
||||
final boolean result = caManager.deployCertificate(vmHost, certificate, false, sshAccessDetails);
|
||||
|
|
@ -1018,6 +1019,12 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
|
|||
}
|
||||
}
|
||||
|
||||
protected void addHostIpToCertDetailsIfConfigAllows(Host vmHost, Map<String, String> ipAddressDetails, ConfigKey<Boolean> configKey) {
|
||||
if (configKey.valueIn(vmHost.getDataCenterId())) {
|
||||
ipAddressDetails.put(NetworkElementCommand.HYPERVISOR_HOST_PRIVATE_IP, vmHost.getPrivateIpAddress());
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfile.Param, Object> params, final DeploymentPlan planToDeploy, final DeploymentPlanner planner)
|
||||
throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException {
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@
|
|||
|
||||
package com.cloud.vm;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.mockito.Matchers.any;
|
||||
|
|
@ -31,8 +32,11 @@ import java.util.HashMap;
|
|||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
import com.cloud.agent.api.routing.NetworkElementCommand;
|
||||
import com.cloud.exception.InvalidParameterValueException;
|
||||
import com.cloud.host.Host;
|
||||
import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
|
||||
import org.apache.cloudstack.framework.config.ConfigKey;
|
||||
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
|
||||
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
|
||||
import org.junit.Assert;
|
||||
|
|
@ -149,6 +153,49 @@ public class VirtualMachineManagerImplTest {
|
|||
virtualMachineManagerImpl.setStoragePoolAllocators(storagePoolAllocators);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testaddHostIpToCertDetailsIfConfigAllows() {
|
||||
Host vmHost = mock(Host.class);
|
||||
ConfigKey testConfig = mock(ConfigKey.class);
|
||||
|
||||
Long dataCenterId = 5L;
|
||||
String hostIp = "1.1.1.1";
|
||||
String routerIp = "2.2.2.2";
|
||||
Map<String, String> ipAddresses = new HashMap<>();
|
||||
ipAddresses.put(NetworkElementCommand.ROUTER_IP, routerIp);
|
||||
|
||||
when(testConfig.valueIn(dataCenterId)).thenReturn(true);
|
||||
when(vmHost.getDataCenterId()).thenReturn(dataCenterId);
|
||||
when(vmHost.getPrivateIpAddress()).thenReturn(hostIp);
|
||||
|
||||
virtualMachineManagerImpl.addHostIpToCertDetailsIfConfigAllows(vmHost, ipAddresses, testConfig);
|
||||
assertTrue(ipAddresses.containsKey(NetworkElementCommand.HYPERVISOR_HOST_PRIVATE_IP));
|
||||
assertEquals(hostIp, ipAddresses.get(NetworkElementCommand.HYPERVISOR_HOST_PRIVATE_IP));
|
||||
assertTrue(ipAddresses.containsKey(NetworkElementCommand.ROUTER_IP));
|
||||
assertEquals(routerIp, ipAddresses.get(NetworkElementCommand.ROUTER_IP));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testaddHostIpToCertDetailsIfConfigAllowsWhenConfigFalse() {
|
||||
Host vmHost = mock(Host.class);
|
||||
ConfigKey testConfig = mock(ConfigKey.class);
|
||||
|
||||
Long dataCenterId = 5L;
|
||||
String hostIp = "1.1.1.1";
|
||||
String routerIp = "2.2.2.2";
|
||||
Map<String, String> ipAddresses = new HashMap<>();
|
||||
ipAddresses.put(NetworkElementCommand.ROUTER_IP, routerIp);
|
||||
|
||||
when(testConfig.valueIn(dataCenterId)).thenReturn(false);
|
||||
when(vmHost.getDataCenterId()).thenReturn(dataCenterId);
|
||||
when(vmHost.getPrivateIpAddress()).thenReturn(hostIp);
|
||||
|
||||
virtualMachineManagerImpl.addHostIpToCertDetailsIfConfigAllows(vmHost, ipAddresses, testConfig);
|
||||
assertFalse(ipAddresses.containsKey(NetworkElementCommand.HYPERVISOR_HOST_PRIVATE_IP));
|
||||
assertTrue(ipAddresses.containsKey(NetworkElementCommand.ROUTER_IP));
|
||||
assertEquals(routerIp, ipAddresses.get(NetworkElementCommand.ROUTER_IP));
|
||||
}
|
||||
|
||||
@Test(expected = CloudRuntimeException.class)
|
||||
public void testScaleVM3() throws Exception {
|
||||
when(vmInstanceMock.getHostId()).thenReturn(null);
|
||||
|
|
@ -322,7 +369,7 @@ public class VirtualMachineManagerImplTest {
|
|||
Map<Volume, StoragePool> volumeToPoolObjectMap = virtualMachineManagerImpl.buildMapUsingUserInformation(virtualMachineProfileMock, hostMock, userDefinedVolumeToStoragePoolMap);
|
||||
|
||||
assertFalse(volumeToPoolObjectMap.isEmpty());
|
||||
Assert.assertEquals(storagePoolVoMock, volumeToPoolObjectMap.get(volumeVoMock));
|
||||
assertEquals(storagePoolVoMock, volumeToPoolObjectMap.get(volumeVoMock));
|
||||
|
||||
Mockito.verify(userDefinedVolumeToStoragePoolMap, times(1)).keySet();
|
||||
}
|
||||
|
|
@ -341,8 +388,8 @@ public class VirtualMachineManagerImplTest {
|
|||
Mockito.doReturn(volumesOfVm).when(volumeDaoMock).findUsableVolumesForInstance(vmInstanceVoMockId);
|
||||
List<Volume> volumesNotMapped = virtualMachineManagerImpl.findVolumesThatWereNotMappedByTheUser(virtualMachineProfileMock, volumeToStoragePoolObjectMap);
|
||||
|
||||
Assert.assertEquals(1, volumesNotMapped.size());
|
||||
Assert.assertEquals(volumeVoMock2, volumesNotMapped.get(0));
|
||||
assertEquals(1, volumesNotMapped.size());
|
||||
assertEquals(volumeVoMock2, volumesNotMapped.get(0));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -388,8 +435,8 @@ public class VirtualMachineManagerImplTest {
|
|||
|
||||
List<StoragePool> poolList = virtualMachineManagerImpl.getCandidateStoragePoolsToMigrateLocalVolume(virtualMachineProfileMock, dataCenterDeploymentMock, volumeVoMock);
|
||||
|
||||
Assert.assertEquals(1, poolList.size());
|
||||
Assert.assertEquals(storagePoolVoMock, poolList.get(0));
|
||||
assertEquals(1, poolList.size());
|
||||
assertEquals(storagePoolVoMock, poolList.get(0));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -407,8 +454,8 @@ public class VirtualMachineManagerImplTest {
|
|||
Mockito.doReturn(true).when(virtualMachineManagerImpl).isStorageCrossClusterMigration(hostMockId, storagePoolVoMock);
|
||||
List<StoragePool> poolList = virtualMachineManagerImpl.getCandidateStoragePoolsToMigrateLocalVolume(virtualMachineProfileMock, dataCenterDeploymentMock, volumeVoMock);
|
||||
|
||||
Assert.assertEquals(1, poolList.size());
|
||||
Assert.assertEquals(storagePoolVoMock, poolList.get(0));
|
||||
assertEquals(1, poolList.size());
|
||||
assertEquals(storagePoolVoMock, poolList.get(0));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -506,7 +553,7 @@ public class VirtualMachineManagerImplTest {
|
|||
virtualMachineManagerImpl.createVolumeToStoragePoolMappingIfPossible(virtualMachineProfileMock, dataCenterDeploymentMock, volumeToPoolObjectMap, volumeVoMock, storagePoolVoMock);
|
||||
|
||||
assertFalse(volumeToPoolObjectMap.isEmpty());
|
||||
Assert.assertEquals(storagePoolMockOther, volumeToPoolObjectMap.get(volumeVoMock));
|
||||
assertEquals(storagePoolMockOther, volumeToPoolObjectMap.get(volumeVoMock));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -563,7 +610,7 @@ public class VirtualMachineManagerImplTest {
|
|||
virtualMachineManagerImpl.createStoragePoolMappingsForVolumes(virtualMachineProfileMock, dataCenterDeploymentMock, volumeToPoolObjectMap, allVolumes);
|
||||
|
||||
assertFalse(volumeToPoolObjectMap.isEmpty());
|
||||
Assert.assertEquals(storagePoolVoMock, volumeToPoolObjectMap.get(volumeVoMock));
|
||||
assertEquals(storagePoolVoMock, volumeToPoolObjectMap.get(volumeVoMock));
|
||||
|
||||
Mockito.verify(virtualMachineManagerImpl).executeManagedStorageChecksWhenTargetStoragePoolNotProvided(hostMock, storagePoolVoMock, volumeVoMock);
|
||||
Mockito.verify(virtualMachineManagerImpl).isStorageCrossClusterMigration(hostMockId, storagePoolVoMock);
|
||||
|
|
@ -584,7 +631,7 @@ public class VirtualMachineManagerImplTest {
|
|||
|
||||
Map<Volume, StoragePool> mappingVolumeAndStoragePool = virtualMachineManagerImpl.createMappingVolumeAndStoragePool(virtualMachineProfileMock, hostMock, new HashMap<>());
|
||||
|
||||
Assert.assertEquals(mappingVolumeAndStoragePool, volumeToPoolObjectMap);
|
||||
assertEquals(mappingVolumeAndStoragePool, volumeToPoolObjectMap);
|
||||
|
||||
InOrder inOrder = Mockito.inOrder(virtualMachineManagerImpl);
|
||||
inOrder.verify(virtualMachineManagerImpl).buildMapUsingUserInformation(Mockito.eq(virtualMachineProfileMock), Mockito.eq(hostMock), Mockito.anyMapOf(Long.class, Long.class));
|
||||
|
|
@ -651,7 +698,7 @@ public class VirtualMachineManagerImplTest {
|
|||
|
||||
boolean result = virtualMachineManagerImpl.isRootVolumeOnLocalStorage(0l);
|
||||
|
||||
Assert.assertEquals(expected, result);
|
||||
assertEquals(expected, result);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
|||
|
|
@ -424,6 +424,6 @@ public class CAManagerImpl extends ManagerBase implements CAManager {
|
|||
|
||||
@Override
|
||||
public ConfigKey<?>[] getConfigKeys() {
|
||||
return new ConfigKey<?>[] {CAProviderPlugin, CertKeySize, CertSignatureAlgorithm, CertValidityPeriod, AutomaticCertRenewal, CABackgroundJobDelay, CertExpiryAlertPeriod};
|
||||
return new ConfigKey<?>[] {CAProviderPlugin, CertKeySize, CertSignatureAlgorithm, CertValidityPeriod, AutomaticCertRenewal, AllowHostIPInSysVMAgentCert, CABackgroundJobDelay, CertExpiryAlertPeriod};
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue