Skip to content

vTPM: support KVM and VMware #10543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 17 commits into
base: 4.20
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions api/src/main/java/com/cloud/vm/VmDetailConstants.java
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,13 @@ public interface VmDetailConstants {
String VMWARE_HOST_NAME = String.format("%s-host", VMWARE_TO_KVM_PREFIX);
String VMWARE_DISK = String.format("%s-disk", VMWARE_TO_KVM_PREFIX);
String VMWARE_MAC_ADDRESSES = String.format("%s-mac-addresses", VMWARE_TO_KVM_PREFIX);

// TPM
String VIRTUAL_TPM_ENABLED = "virtual.tpm.enabled";
String VIRTUAL_TPM_MODEL = "virtual.tpm.model";
String VIRTUAL_TPM_VERSION = "virtual.tpm.version";

// CPU mode and model, ADMIN only
String GUEST_CPU_MODE = "guest.cpu.mode";
String GUEST_CPU_MODEL = "guest.cpu.model";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can non-admin user deploy vTPM enabled instance without these settings? any other way for the normal user to provide these options, from service offering, etc?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently no.
I am thinking of adding global/domain/account settings for both.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, wait, these settings are also available for templates. But again, only available for admin. I will test it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, these two settings are available for templates.
but only root admin can add

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
// under the License.
package org.apache.cloudstack.query;

import java.util.Arrays;
import java.util.List;

import org.apache.cloudstack.affinity.AffinityGroupResponse;
Expand Down Expand Up @@ -97,13 +98,16 @@
import org.apache.cloudstack.framework.config.ConfigKey;

import com.cloud.exception.PermissionDeniedException;
import com.cloud.vm.VmDetailConstants;

/**
* Service used for list api query.
*
*/
public interface QueryService {

List<String> RootAdminOnlyVmSettings = Arrays.asList(VmDetailConstants.GUEST_CPU_MODE, VmDetailConstants.GUEST_CPU_MODEL);

// Config keys
ConfigKey<Boolean> AllowUserViewDestroyedVM = new ConfigKey<>("Advanced", Boolean.class, "allow.user.view.destroyed.vm", "false",
"Determines whether users can view their destroyed or expunging vm ", true, ConfigKey.Scope.Account);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
-- Schema upgrade from 4.20.0.0 to 4.20.1.0
--;

-- Delete user vm details for guest CPU mode/model which are root admin only
DELETE FROM `cloud`.`user_vm_details` WHERE `name` IN ('guest.cpu.mode','guest.cpu.model');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weizhouapache will this impact any VMs in existing deployments with these settings?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two settings have no impact in older versions, but are only available for root admin with this PR (because I think the host CPU is sensitive information).

I think it is better to remove them during upgrade. Otherwise user can add the settings before upgrade, and get the Host CPU after upgrade.


-- Add column api_key_access to user and account tables
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.user', 'api_key_access', 'boolean DEFAULT NULL COMMENT "is api key access allowed for the user" AFTER `secret_key`');
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.account', 'api_key_access', 'boolean DEFAULT NULL COMMENT "is api key access allowed for the account" ');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.SCSIDef;
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.SerialDef;
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.TermPolicy;
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.TpmDef;
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.VideoDef;
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.WatchDogDef;
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.WatchDogDef.WatchDogAction;
Expand Down Expand Up @@ -2660,6 +2661,11 @@
devices.addDevice(createGraphicDef(vmTO));
devices.addDevice(createTabletInputDef());

TpmDef tpmDef = createTpmDef(vmTO);
if (tpmDef != null) {
devices.addDevice(tpmDef);

Check warning on line 2666 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java#L2666

Added line #L2666 was not covered by tests
}

if (isGuestAarch64()) {
createArm64UsbDef(devices);
}
Expand Down Expand Up @@ -2840,8 +2846,11 @@

private CpuModeDef createCpuModeDef(VirtualMachineTO vmTO, int vcpus) {
final CpuModeDef cmd = new CpuModeDef();
cmd.setMode(guestCpuMode);
cmd.setModel(guestCpuModel);
Map<String, String> details = vmTO.getDetails();
String cpuMode = MapUtils.isNotEmpty(details) && details.get(VmDetailConstants.GUEST_CPU_MODE) != null ? details.get(VmDetailConstants.GUEST_CPU_MODE) : guestCpuMode;
String cpuModel = MapUtils.isNotEmpty(details) && details.get(VmDetailConstants.GUEST_CPU_MODEL) != null ? details.get(VmDetailConstants.GUEST_CPU_MODEL) : guestCpuModel;
cmd.setMode(cpuMode);
cmd.setModel(cpuModel);
if (VirtualMachine.Type.User.equals(vmTO.getType())) {
cmd.setFeatures(cpuFeatures);
}
Expand All @@ -2850,6 +2859,19 @@
return cmd;
}

protected TpmDef createTpmDef(VirtualMachineTO vmTO) {
Map<String, String> details = vmTO.getDetails();
if (MapUtils.isEmpty(details)) {
return null;
}
String tpmModel = details.get(VmDetailConstants.VIRTUAL_TPM_MODEL);
if (tpmModel == null) {
return null;

Check warning on line 2869 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java#L2869

Added line #L2869 was not covered by tests
}
String tpmVersion = details.get(VmDetailConstants.VIRTUAL_TPM_VERSION);
return new TpmDef(tpmModel, tpmVersion);
}

private void configureGuestIfUefiEnabled(boolean isSecureBoot, String bootMode, GuestDef guest) {
setGuestLoader(bootMode, SECURE, guest, GuestDef.GUEST_LOADER_SECURE);
setGuestLoader(bootMode, LEGACY, guest, GuestDef.GUEST_LOADER_LEGACY);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.io.File;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -2358,6 +2359,82 @@
}
}

public static class TpmDef {
enum TpmModel {
TIS("tpm-tis"), // TPM Interface Specification (TIS)
CRB("tpm-crb"); // Command-Response Buffer (CRB)

final String model;

TpmModel(String model) {
this.model = model;
}

@Override
public String toString() {
return model;
}
}

enum TpmVersion {
V1_2("1.2"), // 1.2
V2_0("2.0"); // 2.0. Default version. The CRB model is only supported with version 2.0.

final String version;

TpmVersion(String version) {
this.version = version;
}

@Override
public String toString() {
return version;
}
}

private TpmModel model;
private TpmVersion version = TpmVersion.V2_0;

public TpmDef(TpmModel model, TpmVersion version) {
this.model = model;

Check warning on line 2399 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java#L2398-L2399

Added lines #L2398 - L2399 were not covered by tests
if (version != null) {
this.version = version;

Check warning on line 2401 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java#L2401

Added line #L2401 was not covered by tests
}
}

Check warning on line 2403 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java#L2403

Added line #L2403 was not covered by tests

public TpmDef(String model, String version) {
this.model = Arrays.stream(TpmModel.values())
.filter(tpmModel -> tpmModel.toString().equals(model))
.findFirst()
.orElse(null);
if (version != null) {
this.version = Arrays.stream(TpmVersion.values())
.filter(tpmVersion -> tpmVersion.toString().equals(version))
.findFirst()
.orElse(this.version);;
}
}

public TpmModel getModel() {
return model;
}

public TpmVersion getVersion() {
return version;
}

@Override
public String toString() {
StringBuilder tpmBuidler = new StringBuilder();
if (model != null) {
tpmBuidler.append("<tpm model='").append(model).append("'>\n");
tpmBuidler.append("<backend type='emulator' version='").append(version).append("'/>\n");
tpmBuidler.append("</tpm>\n");
}
return tpmBuidler.toString();
}
}

public void setHvsType(String hvs) {
_hvsType = hvs;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6541,4 +6541,28 @@ public void testGetDiskModelFromVMDetailVirtioBlk() {
DiskDef.DiskBus diskBus = libvirtComputingResourceSpy.getDiskModelFromVMDetail(virtualMachineTO);
assertEquals(DiskDef.DiskBus.VIRTIOBLK, diskBus);
}

@Test
public void testCreateTpmDef() {
VirtualMachineTO virtualMachineTO = Mockito.mock(VirtualMachineTO.class);
Map<String, String> details = new HashMap<>();
details.put(VmDetailConstants.VIRTUAL_TPM_MODEL, "tpm-tis");
details.put(VmDetailConstants.VIRTUAL_TPM_VERSION, "2.0");
Mockito.when(virtualMachineTO.getDetails()).thenReturn(details);
LibvirtVMDef.TpmDef tpmDef = libvirtComputingResourceSpy.createTpmDef(virtualMachineTO);
assertEquals(LibvirtVMDef.TpmDef.TpmModel.TIS, tpmDef.getModel());
assertEquals(LibvirtVMDef.TpmDef.TpmVersion.V2_0, tpmDef.getVersion());
}

@Test
public void testCreateTpmDefWithInvalidVersion() {
VirtualMachineTO virtualMachineTO = Mockito.mock(VirtualMachineTO.class);
Map<String, String> details = new HashMap<>();
details.put(VmDetailConstants.VIRTUAL_TPM_MODEL, "tpm-crb");
details.put(VmDetailConstants.VIRTUAL_TPM_VERSION, "3.0");
Mockito.when(virtualMachineTO.getDetails()).thenReturn(details);
LibvirtVMDef.TpmDef tpmDef = libvirtComputingResourceSpy.createTpmDef(virtualMachineTO);
assertEquals(LibvirtVMDef.TpmDef.TpmModel.CRB, tpmDef.getModel());
assertEquals(LibvirtVMDef.TpmDef.TpmVersion.V2_0, tpmDef.getVersion());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -566,9 +566,20 @@ public void testTopology() {
assertEquals("<cpu><topology sockets='4' cores='2' threads='1' /></cpu>", cpuModeDef.toString());
}

@Test
public void testTopologyNoInfo() {
LibvirtVMDef.CpuModeDef cpuModeDef = new LibvirtVMDef.CpuModeDef();
cpuModeDef.setTopology(-1, -1, 4);
assertEquals("<cpu></cpu>", cpuModeDef.toString());
}

@Test
public void testTpmModel() {
LibvirtVMDef.TpmDef tpmDef = new LibvirtVMDef.TpmDef("tpm-tis", "2.0");
assertEquals(LibvirtVMDef.TpmDef.TpmModel.TIS, tpmDef.getModel());
assertEquals(LibvirtVMDef.TpmDef.TpmVersion.V2_0, tpmDef.getVersion());
assertEquals("<tpm model='tpm-tis'>\n" +
"<backend type='emulator' version='2.0'/>\n" +
"</tpm>\n", tpmDef.toString());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,15 @@

import com.cloud.capacity.CapacityManager;
import com.cloud.hypervisor.vmware.mo.HostDatastoreBrowserMO;
import com.vmware.vim25.Description;
import com.vmware.vim25.FileInfo;
import com.vmware.vim25.FileQueryFlags;
import com.vmware.vim25.FolderFileInfo;
import com.vmware.vim25.HostDatastoreBrowserSearchResults;
import com.vmware.vim25.HostDatastoreBrowserSearchSpec;
import com.vmware.vim25.VirtualCdromIsoBackingInfo;
import com.vmware.vim25.VirtualMachineConfigSummary;
import com.vmware.vim25.VirtualTPM;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.backup.PrepareForBackupRestorationCommand;
import org.apache.cloudstack.storage.command.CopyCommand;
Expand Down Expand Up @@ -2597,12 +2599,16 @@

setBootOptions(vmSpec, bootMode, vmConfigSpec);

// Config vTPM
configureVirtualTPM(vmMo, vmSpec, vmConfigSpec);

Check warning on line 2603 in plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java#L2603

Added line #L2603 was not covered by tests

if (StringUtils.isNotEmpty(vmStoragePolicyId)) {
vmConfigSpec.getVmProfile().add(vmProfileSpec);
if (logger.isTraceEnabled()) {
logger.trace(String.format("Configuring the VM %s with storage policy: %s", vmInternalCSName, vmStoragePolicyId));
}
}

//
// Configure VM
//
Expand Down Expand Up @@ -3203,6 +3209,57 @@
vmConfigSpec.getDeviceChange().add(arrayVideoCardConfigSpecs);
}

/**
* Add or Remove virtual TPM module
*
* @param vmMo virtual machine mo
* @param vmSpec virtual machine specs
* @param vmConfigSpec virtual machine config spec
* @throws Exception exception
*/
protected void configureVirtualTPM(VirtualMachineMO vmMo, VirtualMachineTO vmSpec, VirtualMachineConfigSpec vmConfigSpec) throws Exception {
String virtualTPMEnabled = vmSpec.getDetails().getOrDefault(VmDetailConstants.VIRTUAL_TPM_ENABLED, null);
if (Boolean.parseBoolean(virtualTPMEnabled)) {
for (VirtualDevice device : vmMo.getAllDeviceList()) {
if (device instanceof VirtualTPM) {
logger.debug(String.format("Virtual TPM device has already been added to VM , returning", vmMo.getVmName()));
return;

Check warning on line 3226 in plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java#L3225-L3226

Added lines #L3225 - L3226 were not covered by tests
}
}

Check warning on line 3228 in plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java#L3228

Added line #L3228 was not covered by tests
logger.debug("Adding Virtual TPM device");
addVirtualTPMDevice(vmConfigSpec);
} else if (virtualTPMEnabled == null) {
logger.debug(String.format("Virtual TPM device is neither enabled nor disabled for VM %s, skipping", vmMo.getVmName()));

Check warning on line 3232 in plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java#L3232

Added line #L3232 was not covered by tests
} else {
logger.debug(String.format("Virtual TPM device is disabled for VM %s", vmMo.getVmName()));
for (VirtualDevice device : vmMo.getAllDeviceList()) {
if (device instanceof VirtualTPM) {
logger.debug(String.format("Removing Virtual TPM device from VM %s as it is disabled", vmMo.getVmName()));
removeVirtualTPMDevice(vmConfigSpec, (VirtualTPM) device);
}
}
}
}

protected void addVirtualTPMDevice(VirtualMachineConfigSpec vmConfigSpec) {
Description description = new Description();
description.setSummary("Trusted Platform Module");
description.setLabel("Trusted Platform Module");
VirtualTPM virtualTPM = new VirtualTPM();
virtualTPM.setDeviceInfo(description);
VirtualDeviceConfigSpec deviceConfigSpec = new VirtualDeviceConfigSpec();
deviceConfigSpec.setDevice(virtualTPM);
deviceConfigSpec.setOperation(VirtualDeviceConfigSpecOperation.ADD);
vmConfigSpec.getDeviceChange().add(deviceConfigSpec);
}

protected void removeVirtualTPMDevice(VirtualMachineConfigSpec vmConfigSpec, VirtualTPM virtualTPM) {
VirtualDeviceConfigSpec virtualDeviceConfigSpec = new VirtualDeviceConfigSpec();
virtualDeviceConfigSpec.setDevice(virtualTPM);
virtualDeviceConfigSpec.setOperation(VirtualDeviceConfigSpecOperation.REMOVE);
vmConfigSpec.getDeviceChange().add(virtualDeviceConfigSpec);
}

private void tearDownVm(VirtualMachineMO vmMo) throws Exception {

if (vmMo == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@
import com.vmware.vim25.FileInfo;
import com.vmware.vim25.HostDatastoreBrowserSearchResults;
import com.vmware.vim25.HostDatastoreBrowserSearchSpec;
import com.vmware.vim25.VirtualTPM;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.storage.command.CopyCommand;
import org.apache.cloudstack.storage.command.browser.ListDataStoreObjectsAnswer;
import org.apache.cloudstack.storage.command.browser.ListDataStoreObjectsCommand;
Expand Down Expand Up @@ -842,4 +844,41 @@ public void testExecuteWithEmptyPath() throws Exception {
assertEquals(Collections.singletonList(1L), answer.getSizes());
assertEquals(Collections.singletonList(date.getTime()), answer.getLastModified());
}

@Test
public void testAddVirtualTPMDevice() throws Exception {
VirtualMachineMO vmMo = Mockito.mock(VirtualMachineMO.class);
VirtualMachineTO vmSpec = Mockito.mock(VirtualMachineTO.class);
VirtualMachineConfigSpec vmConfigSpec = Mockito.mock(VirtualMachineConfigSpec.class);
Map<String, String> details = new HashMap<>();
details.put(ApiConstants.BootType.UEFI.toString(), "SECURE");
details.put(VmDetailConstants.VIRTUAL_TPM_ENABLED, "true");
when(vmSpec.getDetails()).thenReturn(details);
when(vmMo.getAllDeviceList()).thenReturn(new ArrayList<>());
List<VirtualDeviceConfigSpec> deviceChanges = Mockito.mock(List.class);
when(vmConfigSpec.getDeviceChange()).thenReturn(deviceChanges);

vmwareResource.configureVirtualTPM(vmMo, vmSpec, vmConfigSpec);
Mockito.verify(vmwareResource, Mockito.times(1)).addVirtualTPMDevice(vmConfigSpec);
Mockito.verify(deviceChanges, Mockito.times(1)).add(any(VirtualDeviceConfigSpec.class));
}

@Test
public void testRemoveVirtualTPMDevice() throws Exception {
VirtualMachineMO vmMo = Mockito.mock(VirtualMachineMO.class);
VirtualMachineTO vmSpec = Mockito.mock(VirtualMachineTO.class);
VirtualMachineConfigSpec vmConfigSpec = Mockito.mock(VirtualMachineConfigSpec.class);
Map<String, String> details = new HashMap<>();
details.put(ApiConstants.BootType.UEFI.toString(), "SECURE");
details.put(VmDetailConstants.VIRTUAL_TPM_ENABLED, "false");
when(vmSpec.getDetails()).thenReturn(details);
VirtualTPM tpm = new VirtualTPM();
when(vmMo.getAllDeviceList()).thenReturn(List.of(tpm));
List<VirtualDeviceConfigSpec> deviceChanges = Mockito.mock(List.class);
when(vmConfigSpec.getDeviceChange()).thenReturn(deviceChanges);

vmwareResource.configureVirtualTPM(vmMo, vmSpec, vmConfigSpec);
Mockito.verify(vmwareResource, Mockito.times(1)).removeVirtualTPMDevice(vmConfigSpec, tpm);
Mockito.verify(deviceChanges, Mockito.times(1)).add(any(VirtualDeviceConfigSpec.class));
}
}
Loading
Loading