Itamar Heim has posted comments on this change.
Change subject: Trusted Compute Pools - Open Attestation integration with oVirt
engine proposal
......................................................................
Patch Set 2: (9 inline comments)
....................................................
File backend/manager/dbscripts/create_tables.sql
Line 279: migration_support INTEGER NOT NULL default 0,
Line 280: userdefined_properties VARCHAR(4000),
Line 281: predefined_properties VARCHAR(4000),
Line 282: min_allocated_mem INTEGER not null default 0, --indicates how much
memory at least VM need to run, value is in MB
Line 283: trust_lvl VARCHAR(20) default '',
shouldn't this be INTEGER and trust_lvl be an enum?
Line 284: CONSTRAINT PK_vm_static PRIMARY KEY(vm_guid)
Line 285: ) WITH OIDS;
Line 286:
Line 287:
....................................................
File backend/manager/dbscripts/vms_sp.sql
Line 428: v_min_allocated_mem INTEGER,
Line 429: v_quota_id UUID,
Line 430: v_cpu_pinning VARCHAR(4000),
Line 431: v_host_cpu_flags BOOLEAN,
Line 432: v_trust_lvl VARCHAR(20))
i could be wrong, but i suspect more places need to be changed to expose this
field.
Line 433: RETURNS VOID
Line 434: AS $procedure$
Line 435: BEGIN
Line 436: INSERT INTO vm_static(description, mem_size_mb, os, vds_group_id,
vm_guid, VM_NAME,
vmt_guid,domain,creation_date,num_of_monitors,allow_console_reconnect,is_initialized,is_auto_suspend,num_of_sockets,cpu_per_socket,usb_policy,
time_zone,auto_startup,is_stateless,dedicated_vm_for_vds, fail_back,
default_boot_sequence, vm_type, nice_level, default_display_type,
priority,iso_path,origin,initrd_url,kernel_url,kernel_params,migration_support,predefined_properties,userdefined_properties,min_allocated_mem,
entity_type, quota_id, cpu_pinning,
is_smartcard_enabled,is_delete_protected,host_cpu_flags,trust_lvl)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/attestationbroker/AttestationCacheManager.java
Line 5:
Line 6: public class AttestationCacheManager {
Line 7:
Line 8: private static HashMap<String, AttestationValue> attestationValues
= new HashMap<String, AttestationValue>();
Line 9: private static long cacheTimeout = 36000000;//10h
no magic numbers in code please, use a config.
Line 10:
Line 11: public AttestationCacheManager(){
Line 12:
Line 13: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/attestationbroker/AttestationHost.java
Line 39: private static final String HEADER_HOSTS = "hosts";
Line 40: private static final String HEADER_HOST_NAME = "host_name";
Line 41: private static final String HEADER_RESULT = "trust_lvl";
Line 42: private static final String HEADER_VTIME = "vtime";
Line 43: private static final int PORT = 8443;
port (why not all of url) should come from config
Line 44: private static final String CONTENT_TYPE = "application/json";
Line 45:
Line 46:
Line 47: public static AttestationResult validateHostIsTrusted(VDS vds) {
Line 65: public static List<AttestationValue> attestHosts(Set<String>
hosts){
Line 66: String attestationWSURL = Config.<String>
GetValue(ConfigValues.AttestationWebServicesUrl);
Line 67: DefaultHttpClient httpclient = null;
Line 68: List<AttestationValue> values = new
ArrayList<AttestationValue>();
Line 69: httpclient = setupSSLConnection();
i'd enable ssl if config url prefix is https. if url is http, client shouldn't
enable ssl (server may fail it if required of course, but sometime you want to
test/debug/troubleshoot without ssl, and a config change should allow it.
Line 70: HttpPost httpPost = new HttpPost(attestationWSURL +"/"
+POLL_URI);
Line 71: try {
Line 72: httpPost.setEntity(new StringEntity(writeListJson(hosts)));
Line 73: httpPost.setHeader("Accept",CONTENT_TYPE);
Line 100: KeyStore trustStore =
KeyStore.getInstance(KeyStore.getDefaultType());
Line 101: FileInputStream instream = new
FileInputStream(truststoreUrl.getFile());
Line 102: trustStore.load(instream, null);
Line 103: SSLSocketFactory socketFactory = new
SSLSocketFactory(trustStore);
Line 104: Scheme sch = new Scheme("https", PORT, socketFactory);
http/https - should come from url from config. same for port..
I hope httpClient has the logic to get a url and deal with port and ssl-ness.
if not a helper class in utils would be relevant to not dirty logic here for
this.
Line 105:
httpclient.getConnectionManager().getSchemeRegistry().register(sch);
Line 106: return httpclient;
Line 107: }catch (NoSuchAlgorithmException e) {
Line 108: log.error(e.getMessage(), e);
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsSelector.java
Line 518: vmNICs =
getVmNetworkInterfaceDao().getAllForVm(getVm().getId());
Line 519: }
Line 520: return vmNICs;
Line 521: }
Line 522: private static boolean validateHostIsTrusted(VDS curVds) {
wasn't this supposed to move to utils or AttestationBroker (Maybe to a
JsonBroker?)
I'm more puzzled since i see there is a cache manager already.
Line 523: String attestationWSURL, trustStorePath;
Line 524: attestationWSURL = Config.<String>
GetValue(ConfigValues.AttestationWebServicesUrl);
Line 525: trustStorePath=Config.<String>
GetValue(ConfigValues.TrustStore);
Line 526: DefaultHttpClient httpclient = new DefaultHttpClient();
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmStatic.java
Line 78: setNiceLevel(0);
Line 79: setDefaultBootSequence(BootSequence.C);
Line 80: defaultDisplayType = DisplayType.qxl;
Line 81: setVmType(VmType.Desktop);
Line 82: trust_lvl="trusted";
why is that the default?
if for testing, separate to a different patch / add a TODO to remove it before
merging.
Line 83: }
Line 84:
Line 85: public VmStatic(VmStatic vmStatic) {
Line 86: super(vmStatic.getId(),
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
Line 1332: @DefaultValueAttribute("false")
Line 1333: HardwareInfoEnabled(410),
Line 1334:
Line 1335: @TypeConverterAttribute(String.class)
Line 1336:
@DefaultValueAttribute("https://oat-server:8443/AttestationService/resources";)
great, so url has http(s) and port in it - just remove other parts of code
making assumptions rather than using it from the url
Line 1337: AttestationWebServicesUrl(411),
Line 1338:
Line 1339: @TypeConverterAttribute(String.class)
Line 1340: @DefaultValueAttribute("/etc/pki/ovirt-engine/certs")
--
To view, visit http://gerrit.ovirt.org/11237
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4de780cd46069638433255f3f9c994575f752e55
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dave Chen <[email protected]>
Gerrit-Reviewer: Dave Chen <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
