| Message ID | 1466716148-10655-1-git-send-email-mjg59@coreos.com |
|---|---|
| State | New |
| Headers | show |
* Matthew Garrett (mjg59@coreos.com) wrote: Hi Matthew, (Ccing in Stefan who has been trying to get vTPM in for years and Paolo for any x86ism and especially the ACPIisms, and Daniel for crypto stuff) I'll repeat some of my comments from yesterday's irc chat so you can reply on list. So overall the plus point is it's simple (much smaller than even the interface to the vTPM), the minus is it's very non-standard. > Trusted Boot is based around having a trusted store of measurement data and a > secure communications channel between that store and an attestation target. In > actual hardware, that's a TPM. Since the TPM can only be accessed via the host > system, this in turn requires that the TPM be able to perform reasonably > complicated cryptographic functions in order to demonstrate its trusted state. > > In cloud environments, qemu is inherently trusted and the hypervisor infrastructure > provides a trusted mechanism for extracting information from qemu and providing it > to another system. This means we can skip the crypto and stick with the basic > functionality - ie, providing a trusted store of measurement data. I think the big question for me is what uses this system and in particular how the users can guarantee who they're speaking to; I'd like to understand the cases it works for and those it doesn't; for example: a) (one that works) 'are all the VMs on my hosts running trusted OSs' That works with this just as well as with a vTPM; you ask your hypervisor to give you the measurements for your guests; you trust your hypervisor. Although I think you've somehow got to extract the measurement log from the guest and get it to the hypervisor if it's going to make sense of the measurements. b) (one that doesn't?) I'm connecting to a VM remotely over a network, I want to check the VM really is who it says it is and is running a trusted OS. As a remote entity I don't know which hypervisor is running the VM, the VM itself can't sign anything to give me back because it might just sign a reply for a different VM. On a real TPM the attestation results would be signed using one of the keys in the TPM (I can't remember which). c) (similar to b) 'I paid you to give me a ... VM - can I check it really is that' how do I externally to the cloud show that the measurement came from the same VM I'm asking about. and then I'm not clear which of the existing TPM users could be munged into working with it; can you make an existing trusted-grub or trousers write measurements and log into it? > This driver provides a very small subset of TPM 1.2 functionality in the form of a > bank of registers that can store SHA1 measurements of boot components. Performing a > write to one of these registers will append the new 20 byte hash to the 20 bytes > currently stored within the register, take a SHA1 of this 40 byte value and then > replace the existing register contents with the new value. This ensures that a > given value can only be obtained by performing the same sequence of writes. It also > adds a monitor command to allow an external agent to extract this information from > the running system and provide it over a secure communications channel. Finally, it > measures each of the loaded ROMs into one of the registers at reset time. > > In combination with work in SeaBIOS and the kernel, this permits a fully measured > boot in a virtualised environment without the overhead of a full TPM > implementation. So only SeaBIOS for now? Would it work for EFI in principle? > This version of the implementation depends on port io, but if there's interest I'll > add mmio as well. I guess port IO has the advantage of making it easy to glue into the early parts of the BIOS. Some things I can see are missing: Migration support: You probably want to migrate the current measurements and make sure you don't include the measurements of ROMs on the destination until after reset. (Search for places that use dc->vmsd = .... as examples) You might want to add a measurement to indicate a migration happened; but that's a separate question. QMP support: You should wire it up to the qmp monitor as well. Generally the management tools use qmp rather than hmp, What about hotplug? If I hotplug a NIC should it measure the new ROM? What happens then if I restart the VM from clean with the ROM added. Why do SHA1 based TPM 1.2 now, shouldn't you start with TPM2/newer SHAs? I guess another approach to the same thing would be to bundle this up into something that responded to TPM commands like a vTPM but just had less inside it; then it could attach to the existing TPM interfaces? > Signed-off-by: Matthew Garrett <mjg59@coreos.com> > --- > default-configs/x86_64-softmmu.mak | 1 + > hmp-commands.hx | 13 +++ > hw/core/loader.c | 12 +++ > hw/i386/acpi-build.c | 22 ++++ > hw/misc/Makefile.objs | 1 + > hw/misc/measurements.c | 206 +++++++++++++++++++++++++++++++++++++ > hw/misc/measurements.h | 2 + > include/hw/isa/isa.h | 13 +++ > include/hw/loader.h | 1 + > monitor.c | 1 + > stubs/Makefile.objs | 1 + > stubs/measurements.c | 13 +++ > 12 files changed, 286 insertions(+) > create mode 100644 hw/misc/measurements.c > create mode 100644 hw/misc/measurements.h > create mode 100644 stubs/measurements.c > > diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak > index 6e3b312..6f0fcc3 100644 > --- a/default-configs/x86_64-softmmu.mak > +++ b/default-configs/x86_64-softmmu.mak > @@ -58,3 +58,4 @@ CONFIG_IOH3420=y > CONFIG_I82801B11=y > CONFIG_SMBIOS=y > CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM) > +CONFIG_MEASUREMENTS=y > diff --git a/hmp-commands.hx b/hmp-commands.hx > index 98b4b1a..6a31392 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -1748,6 +1748,19 @@ Set QOM property @var{property} of object at location @var{path} to value @var{v > ETEXI > > { > + .name = "measurements", > + .args_type = "", > + .params = "", > + .help = "Print system measurements", > + .mhandler.cmd = print_measurements, > + }, > +STEXI > +@item measurements > +@findex measurements > +Redirect Print system measurements > +ETEXI > + > + { > .name = "info", > .args_type = "item:s?", > .params = "[subcommand]", > diff --git a/hw/core/loader.c b/hw/core/loader.c > index 53e0e41..e1b7af7 100644 > --- a/hw/core/loader.c > +++ b/hw/core/loader.c > @@ -55,6 +55,7 @@ > #include "exec/address-spaces.h" > #include "hw/boards.h" > #include "qemu/cutils.h" > +#include "hw/misc/measurements.h" > > #include <zlib.h> > > @@ -1026,6 +1027,17 @@ static void rom_reset(void *unused) > } > } > > +void measure_roms(void) > +{ > + Rom *rom; > + QTAILQ_FOREACH(rom, &roms, next) { > + if (rom->data == NULL) { > + continue; > + } > + extend_data(0, rom->data, rom->datasize); > + } > +} Are you making unpredictable assumptions about ROM order? You're explicitly measuring these into PCR 0 - I guess that's probably reasonable but it should be documented. > int rom_check_and_register_reset(void) > { > hwaddr addr = 0; > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 8ca2032..92129d1 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -107,6 +107,7 @@ typedef struct AcpiMiscInfo { > unsigned dsdt_size; > uint16_t pvpanic_port; > uint16_t applesmc_io_base; > + uint16_t measurements_io_base; > } AcpiMiscInfo; > > typedef struct AcpiBuildPciBusHotplugState { > @@ -203,6 +204,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info) > info->tpm_version = tpm_get_version(); > info->pvpanic_port = pvpanic_port(); > info->applesmc_io_base = applesmc_port(); > + info->measurements_io_base = measurements_port(); > } > > /* > @@ -2185,6 +2187,26 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > aml_append(dsdt, scope); > } > > + if (misc->measurements_io_base) { > + scope = aml_scope("\\_SB.PCI0.ISA"); > + dev = aml_device("PCRS"); > + > + aml_append(dev, aml_name_decl("_HID", aml_eisaid("CORE0001"))); > + /* device present, functioning, decoding, not shown in UI */ > + aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); > + > + crs = aml_resource_template(); > + aml_append(crs, > + aml_io(AML_DECODE16, misc->measurements_io_base, > + misc->measurements_io_base, > + 0x01, 2) > + ); > + aml_append(dev, aml_name_decl("_CRS", crs)); > + > + aml_append(scope, dev); > + aml_append(dsdt, scope); > + } > + > sb_scope = aml_scope("\\_SB"); > { > build_memory_devices(sb_scope, nr_mem, pm->mem_hp_io_base, > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs > index ffb49c1..e7a784b 100644 > --- a/hw/misc/Makefile.objs > +++ b/hw/misc/Makefile.objs > @@ -5,6 +5,7 @@ common-obj-$(CONFIG_ISA_DEBUG) += debugexit.o > common-obj-$(CONFIG_SGA) += sga.o > common-obj-$(CONFIG_ISA_TESTDEV) += pc-testdev.o > common-obj-$(CONFIG_PCI_TESTDEV) += pci-testdev.o > +common-obj-$(CONFIG_MEASUREMENTS) += measurements.o > > obj-$(CONFIG_VMPORT) += vmport.o > > diff --git a/hw/misc/measurements.c b/hw/misc/measurements.c > new file mode 100644 > index 0000000..3940d31 > --- /dev/null > +++ b/hw/misc/measurements.c > @@ -0,0 +1,206 @@ > +/* > + * QEMU boot measurement > + * > + * Copyright (c) 2016 CoreOS, Inc <mjg59@coreos.com> > + * > + * Permission is hereby granted, free of charge, to any person obtaining a copy > + * of this software and associated documentation files (the "Software"), to deal > + * in the Software without restriction, including without limitation the rights > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > + * copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > + * THE SOFTWARE. > + */ > +#include "qemu/osdep.h" > +#include "hw/isa/isa.h" > +#include "crypto/hash.h" > +#include "hw/misc/measurements.h" > +#include "monitor/monitor.h" > +#include "hw/loader.h" > + > +#define MEASUREMENT(obj) OBJECT_CHECK(MeasurementState, (obj), TYPE_MEASUREMENTS) > + > +typedef struct MeasurementState MeasurementState; > + > +struct MeasurementState { > + ISADevice parent_obj; > + MemoryRegion io_select; > + MemoryRegion io_value; > + uint16_t iobase; > + uint8_t measurements[24][20]; > + uint8_t tmpmeasurement[20]; Magic numbers; please use #define's somewhere. > + int write_count; > + int read_count; > + uint8_t pcr; > +}; > + > +static void measurement_reset(DeviceState *dev) > +{ > + MeasurementState *s = MEASUREMENT(dev); > + > + memset(s->measurements, 0, sizeof(s->measurements)); > + measure_roms(); If you're assuming that can be none-0 then don't you also want to zero pcr, read_count, write_count? > +} > + > +static void measurement_select(void *opaque, hwaddr addr, uint64_t val, unsigned size) > +{ > + MeasurementState *s = MEASUREMENT(opaque); > + s->pcr = val; > + s->read_count = 0; > + s->write_count = 0; What stops me selecting pcr 25 and overwriting stuff with junk? > +} > + > +static uint64_t measurement_version(void *opaque, hwaddr addr, unsigned size) > +{ > + return 0; > +} > + > +static uint64_t measurement_read(void *opaque, hwaddr addr, unsigned size) > +{ > + MeasurementState *s = MEASUREMENT(opaque); > + > + if (s->read_count == 20) { > + s->read_count = 0; > + } > + return s->measurements[s->pcr][s->read_count++]; > +} > + > +static void extend(MeasurementState *s, int pcrnum, uint8_t *data) > +{ > + uint8_t *result; > + char tmpbuf[40]; > + Error *err; > + size_t resultlen = 0; > + > + memcpy(tmpbuf, s->measurements[pcrnum], 20); > + memcpy(tmpbuf + 20, data, 20); > + qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA1, tmpbuf, 40, &result, &resultlen, &err); I think if you're ignoring any errors then using NULL instead of &err is better. > + memcpy(s->measurements[pcrnum], result, 20); > + g_free(result); > +} > + > +static void measurement_value(void *opaque, hwaddr addr, uint64_t val, unsigned size) > +{ > + MeasurementState *s = opaque; > + > + s->tmpmeasurement[s->write_count++] = val; > + if (s->write_count == 20) { > + extend(s, s->pcr, s->tmpmeasurement); > + s->write_count = 0; > + } > +} > + > +void extend_data(int pcrnum, uint8_t *data, size_t len) > +{ > + uint8_t *result; > + Error *err; > + size_t resultlen = 0; > + int ret; > + Object *obj = object_resolve_path_type("", TYPE_MEASUREMENTS, NULL); > + > + if (!obj) { > + return; > + } > + > + ret = qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA1, (char *)data, len, &result, > + &resultlen, &err); Again probably NULL here for the err unless you actually want to report it. > + if (ret < 0) { > + return; > + } Hmm, what are the rules on freeing result on qcrypto_has_bytes returning an error? > + > + extend(MEASUREMENT(obj), pcrnum, result); > + g_free(result); > +} > + > +static const MemoryRegionOps measurement_select_ops = { > + .write = measurement_select, > + .read = measurement_version, > + .endianness = DEVICE_NATIVE_ENDIAN, > + .impl = { > + .min_access_size = 1, > + .max_access_size = 1, > + }, > +}; > + > +static const MemoryRegionOps measurement_value_ops = { > + .write = measurement_value, > + .read = measurement_read, > + .endianness = DEVICE_NATIVE_ENDIAN, > + .impl = { > + .min_access_size = 1, > + .max_access_size = 1, > + }, > +}; > + > +static void measurement_realize(DeviceState *dev, Error **errp) > +{ > + MeasurementState *s = MEASUREMENT(dev); > + > + memory_region_init_io(&s->io_select, OBJECT(s), &measurement_select_ops, s, > + "measurement-select", 1); > + isa_register_ioport(&s->parent_obj, &s->io_select, s->iobase); > + memory_region_init_io(&s->io_value, OBJECT(s), &measurement_value_ops, s, > + "measurement-value", 1); > + isa_register_ioport(&s->parent_obj, &s->io_value, s->iobase + 1); > + measurement_reset(dev); > +} > + > +static Property measurement_props[] = { > + DEFINE_PROP_UINT16(MEASUREMENTS_PROP_IO_BASE, MeasurementState, iobase, 0x620), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void measurement_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + fprintf(stderr, "CLASS INIT\n"); Oops, fprintf escaped into the wild. You might like to add some trace entries. > + dc->realize = measurement_realize; > + dc->reset = measurement_reset; > + dc->props = measurement_props; > + set_bit(DEVICE_CATEGORY_MISC, dc->categories); > +} > + > +static const TypeInfo measurement = { > + .name = TYPE_MEASUREMENTS, > + .parent = TYPE_ISA_DEVICE, > + .instance_size = sizeof(MeasurementState), > + .class_init = measurement_class_init, > +}; > + > +static void measurement_register_types(void) > +{ > + type_register_static(&measurement); > +} > + > +type_init(measurement_register_types); > + > +void print_measurements(Monitor *mon, const QDict *qdict) > +{ > + int i, j; > + Object *obj = object_resolve_path_type("", TYPE_MEASUREMENTS, NULL); > + MeasurementState *s; > + > + if (!obj) { > + return; > + } > + > + s = MEASUREMENT(obj); > + > + for (i = 0; i < 24; i++) { for (pcr = .... > + monitor_printf(mon, "0x%02x: ", i); > + for (j = 0; j < 20; j++) { > + monitor_printf(mon, "0x%02x ", s->measurements[i][j]); > + } > + monitor_printf(mon, "\n"); > + } > +} > diff --git a/hw/misc/measurements.h b/hw/misc/measurements.h > new file mode 100644 > index 0000000..65ad246 > --- /dev/null > +++ b/hw/misc/measurements.h > @@ -0,0 +1,2 @@ > +void print_measurements(Monitor *mon, const QDict *qdict); > +void extend_data(int pcrnum, uint8_t *data, size_t len); > diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h > index c87fbad..00694fd 100644 > --- a/include/hw/isa/isa.h > +++ b/include/hw/isa/isa.h > @@ -24,6 +24,9 @@ > #define APPLESMC_MAX_DATA_LENGTH 32 > #define APPLESMC_PROP_IO_BASE "iobase" > > +#define TYPE_MEASUREMENTS "measurements" > +#define MEASUREMENTS_PROP_IO_BASE "iobase" > + > static inline uint16_t applesmc_port(void) > { > Object *obj = object_resolve_path_type("", TYPE_APPLE_SMC, NULL); > @@ -34,6 +37,16 @@ static inline uint16_t applesmc_port(void) > return 0; > } > > +static inline uint16_t measurements_port(void) > +{ > + Object *obj = object_resolve_path_type("", TYPE_MEASUREMENTS, NULL); > + > + if (obj) { > + return object_property_get_int(obj, MEASUREMENTS_PROP_IO_BASE, NULL); > + } > + return 0; > +} > + > #define TYPE_ISADMA "isa-dma" > > #define ISADMA_CLASS(klass) \ > diff --git a/include/hw/loader.h b/include/hw/loader.h > index 4879b63..cc3157d 100644 > --- a/include/hw/loader.h > +++ b/include/hw/loader.h > @@ -133,6 +133,7 @@ void rom_reset_order_override(void); > int rom_copy(uint8_t *dest, hwaddr addr, size_t size); > void *rom_ptr(hwaddr addr); > void hmp_info_roms(Monitor *mon, const QDict *qdict); > +void measure_roms(void); > > #define rom_add_file_fixed(_f, _a, _i) \ > rom_add_file(_f, NULL, _a, _i, false, NULL) > diff --git a/monitor.c b/monitor.c > index 6f960f1..6aa7ebc 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -32,6 +32,7 @@ > #include "hw/pci/pci.h" > #include "sysemu/watchdog.h" > #include "hw/loader.h" > +#include "hw/misc/measurements.h" > #include "exec/gdbstub.h" > #include "net/net.h" > #include "net/slirp.h" > diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs > index 4b258a6..2ad7f81 100644 > --- a/stubs/Makefile.objs > +++ b/stubs/Makefile.objs > @@ -41,3 +41,4 @@ stub-obj-y += target-monitor-defs.o > stub-obj-y += target-get-monitor-def.o > stub-obj-y += vhost.o > stub-obj-y += iohandler.o > +stub-obj-y += measurements.o > diff --git a/stubs/measurements.c b/stubs/measurements.c > new file mode 100644 > index 0000000..0485d2e > --- /dev/null > +++ b/stubs/measurements.c > @@ -0,0 +1,13 @@ > +#include "qemu/osdep.h" > +#include "monitor/monitor.h" > +#include "hw/misc/measurements.h" > + > +void print_measurements(Monitor *mon, const QDict *qdict) > +{ > + monitor_printf(mon, "No measurement support\n"); > +} > + > +void extend_data(int pcrnum, uint8_t *data, size_t len) > +{ > + return; > +} > -- > 2.7.4 > > Dave -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote on 07/15/2016 07:29:24 AM: > > * Matthew Garrett (mjg59@coreos.com) wrote: > > Hi Matthew, > (Ccing in Stefan who has been trying to get vTPM in for years and > Paolo for any x86ism and especially the ACPIisms, and Daniel for > crypto stuff) > > I'll repeat some of my comments from yesterday's irc chat so you can > reply on list. > > So overall the plus point is it's simple (much smaller than even theinterface > to the vTPM), the minus is it's very non-standard. > > > Trusted Boot is based around having a trusted store of measurementdata and a > > secure communications channel between that store and an > attestation target. In > > actual hardware, that's a TPM. Since the TPM can only be accessed > via the host > > system, this in turn requires that the TPM be able to perform reasonably > > complicated cryptographic functions in order to demonstrate its > trusted state. > > > > In cloud environments, qemu is inherently trusted and the > hypervisor infrastructure > > provides a trusted mechanism for extracting information from qemu > and providing it > > to another system. This means we can skip the crypto and stick > with the basic > > functionality - ie, providing a trusted store of measurement data. > > I think the big question for me is what uses this system and in > particular how the users > can guarantee who they're speaking to; I'd like to understand the > cases it works > for and those it doesn't; for example: > > a) (one that works) 'are all the VMs on my hosts running trusted OSs' > That works with this just as well as with a vTPM; you ask your > hypervisor to > give you the measurements for your guests; you trust your hypervisor. > Although I think you've somehow got to extract the measurement > log from the > guest and get it to the hypervisor if it's going to make sense of the > measurements. > > b) (one that doesn't?) I'm connecting to a VM remotely over a > network, I want > to check the VM really is who it says it is and is running a trusted OS. > As a remote entity I don't know which hypervisor is running > the VM, the VM > itself can't sign anything to give me back because it might > just sign a reply > for a different VM. On a real TPM the attestation results > would be signed > using one of the keys in the TPM (I can't remember which). Attestation Identity Key (AIK) > > c) (similar to b) 'I paid you to give me a ... VM - can I check > it really is that' > how do I externally to the cloud show that the measurement > came from the same VM > I'm asking about. > > and then I'm not clear which of the existing TPM users could be > munged into working > with it; can you make an existing trusted-grub or trousers write > measurements and log > into it? > > > This driver provides a very small subset of TPM 1.2 functionality > in the form of a > > bank of registers that can store SHA1 measurements of boot > components. Performing a > > write to one of these registers will append the new 20 byte hash > to the 20 bytes > > currently stored within the register, take a SHA1 of this 40 byte > value and then > > replace the existing register contents with the new value. This > ensures that a > > given value can only be obtained by performing the same sequence > of writes. It also > > adds a monitor command to allow an external agent to extract this > information from > > the running system and provide it over a secure communications > channel. Finally, it > > measures each of the loaded ROMs into one of the registers at reset time. Are you also providing a measurement log that goes along with these PCR extensions? Like a measurement log we have in the TCPA ACPI table? Just measurements without knowing what was measured wouldn't be all that helpful. Typically recipients of the measurement list would inspect the individual measurements and replay the extensions to come up with the same state of the PCRs that was quoted (signed) by the TPM. Also, are you going to instrument Linux IMA to use this device? And the list goes on into higher level tools that may work with a measurement list from /sys/kernel/security/{tpm0,ima}/*_measurement_list and assume there's a /dev/tpm0 there that can issue a quote with the AIK. Well, one problem is there's little traction for the vTPM but this device here will require new support in existing tools. Typically the TPM is there for the reason: it is a hardware root of trust that signs the current state of the PCRs that were accumulated by measurements starting early on during BIOS init. Now with this device, apart from exposing this via HMP, how would one be sure that, if the current list of the PCRs is presented to an attesting client, that the kernel or attestation server not just completely fake the state of the PCRs? My assumption here is that the state of this device's PCRs will be exposed to user level application that can then use this in some form of attestation, right? > > > > In combination with work in SeaBIOS and the kernel, this permits a > fully measured > > boot in a virtualised environment without the overhead of a full TPM > > implementation. > > So only SeaBIOS for now? Would it work for EFI in principle? With a measurement list I would hope -- in some form of custom ACPI table or some other buffer? Stefan > > > This version of the implementation depends on port io, but if > there's interest I'll > > add mmio as well. > > I guess port IO has the advantage of making it easy to glue into the > early parts of the BIOS. > > Some things I can see are missing: > Migration support: You probably want to migrate the current > measurements and > make sure you don't include the measurements > of ROMs on the > destination until after reset. > (Search for places that use dc->vmsd = .... asexamples) > You might want to add a measurement to > indicate a migration > happened; but that's a separate question. > > QMP support: You should wire it up to the qmp monitor as well. > Generally the management tools use qmp rather than hmp, > > What about hotplug? If I hotplug a NIC should it measure the new ROM? > What happens then if I restart the VM from clean with > the ROM added. > > Why do SHA1 based TPM 1.2 now, shouldn't you start with TPM2/newer SHAs? > > I guess another approach to the same thing would be to bundle this up into > something that responded to TPM commands like a vTPM but just had less > inside it; then it could attach to the existing TPM interfaces? > > > Signed-off-by: Matthew Garrett <mjg59@coreos.com> > > --- > > default-configs/x86_64-softmmu.mak | 1 + > > hmp-commands.hx | 13 +++ > > hw/core/loader.c | 12 +++ > > hw/i386/acpi-build.c | 22 ++++ > > hw/misc/Makefile.objs | 1 + > > hw/misc/measurements.c | 206 ++++++++++++++++++++++++ > +++++++++++++ > > hw/misc/measurements.h | 2 + > > include/hw/isa/isa.h | 13 +++ > > include/hw/loader.h | 1 + > > monitor.c | 1 + > > stubs/Makefile.objs | 1 + > > stubs/measurements.c | 13 +++ > > 12 files changed, 286 insertions(+) > > create mode 100644 hw/misc/measurements.c > > create mode 100644 hw/misc/measurements.h > > create mode 100644 stubs/measurements.c > > > > diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/ > x86_64-softmmu.mak > > index 6e3b312..6f0fcc3 100644 > > --- a/default-configs/x86_64-softmmu.mak > > +++ b/default-configs/x86_64-softmmu.mak > > @@ -58,3 +58,4 @@ CONFIG_IOH3420=y > > CONFIG_I82801B11=y > > CONFIG_SMBIOS=y > > CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM) > > +CONFIG_MEASUREMENTS=y > > diff --git a/hmp-commands.hx b/hmp-commands.hx > > index 98b4b1a..6a31392 100644 > > --- a/hmp-commands.hx > > +++ b/hmp-commands.hx > > @@ -1748,6 +1748,19 @@ Set QOM property @var{property} of object > at location @var{path} to value @var{v > > ETEXI > > > > { > > + .name = "measurements", > > + .args_type = "", > > + .params = "", > > + .help = "Print system measurements", > > + .mhandler.cmd = print_measurements, > > + }, > > +STEXI > > +@item measurements > > +@findex measurements > > +Redirect Print system measurements > > +ETEXI > > + > > + { > > .name = "info", > > .args_type = "item:s?", > > .params = "[subcommand]", > > diff --git a/hw/core/loader.c b/hw/core/loader.c > > index 53e0e41..e1b7af7 100644 > > --- a/hw/core/loader.c > > +++ b/hw/core/loader.c > > @@ -55,6 +55,7 @@ > > #include "exec/address-spaces.h" > > #include "hw/boards.h" > > #include "qemu/cutils.h" > > +#include "hw/misc/measurements.h" > > > > #include <zlib.h> > > > > @@ -1026,6 +1027,17 @@ static void rom_reset(void *unused) > > } > > } > > > > +void measure_roms(void) > > +{ > > + Rom *rom; > > + QTAILQ_FOREACH(rom, &roms, next) { > > + if (rom->data == NULL) { > > + continue; > > + } > > + extend_data(0, rom->data, rom->datasize); > > + } > > +} > > Are you making unpredictable assumptions about ROM order? > You're explicitly measuring these into PCR 0 - I guess > that's probably reasonable but it should be documented. > > > int rom_check_and_register_reset(void) > > { > > hwaddr addr = 0; > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > index 8ca2032..92129d1 100644 > > --- a/hw/i386/acpi-build.c > > +++ b/hw/i386/acpi-build.c > > @@ -107,6 +107,7 @@ typedef struct AcpiMiscInfo { > > unsigned dsdt_size; > > uint16_t pvpanic_port; > > uint16_t applesmc_io_base; > > + uint16_t measurements_io_base; > > } AcpiMiscInfo; > > > > typedef struct AcpiBuildPciBusHotplugState { > > @@ -203,6 +204,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info) > > info->tpm_version = tpm_get_version(); > > info->pvpanic_port = pvpanic_port(); > > info->applesmc_io_base = applesmc_port(); > > + info->measurements_io_base = measurements_port(); > > } > > > > /* > > @@ -2185,6 +2187,26 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > aml_append(dsdt, scope); > > } > > > > + if (misc->measurements_io_base) { > > + scope = aml_scope("\\_SB.PCI0.ISA"); > > + dev = aml_device("PCRS"); > > + > > + aml_append(dev, aml_name_decl("_HID", aml_eisaid("CORE0001"))); > > + /* device present, functioning, decoding, not shown in UI */ > > + aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); > > + > > + crs = aml_resource_template(); > > + aml_append(crs, > > + aml_io(AML_DECODE16, misc->measurements_io_base, > > + misc->measurements_io_base, > > + 0x01, 2) > > + ); > > + aml_append(dev, aml_name_decl("_CRS", crs)); > > + > > + aml_append(scope, dev); > > + aml_append(dsdt, scope); > > + } > > + > > sb_scope = aml_scope("\\_SB"); > > { > > build_memory_devices(sb_scope, nr_mem, pm->mem_hp_io_base, > > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs > > index ffb49c1..e7a784b 100644 > > --- a/hw/misc/Makefile.objs > > +++ b/hw/misc/Makefile.objs > > @@ -5,6 +5,7 @@ common-obj-$(CONFIG_ISA_DEBUG) += debugexit.o > > common-obj-$(CONFIG_SGA) += sga.o > > common-obj-$(CONFIG_ISA_TESTDEV) += pc-testdev.o > > common-obj-$(CONFIG_PCI_TESTDEV) += pci-testdev.o > > +common-obj-$(CONFIG_MEASUREMENTS) += measurements.o > > > > obj-$(CONFIG_VMPORT) += vmport.o > > > > diff --git a/hw/misc/measurements.c b/hw/misc/measurements.c > > new file mode 100644 > > index 0000000..3940d31 > > --- /dev/null > > +++ b/hw/misc/measurements.c > > @@ -0,0 +1,206 @@ > > +/* > > + * QEMU boot measurement > > + * > > + * Copyright (c) 2016 CoreOS, Inc <mjg59@coreos.com> > > + * > > + * Permission is hereby granted, free of charge, to any person > obtaining a copy > > + * of this software and associated documentation files (the > "Software"), to deal > > + * in the Software without restriction, including without > limitation the rights > > + * to use, copy, modify, merge, publish, distribute, sublicense, > and/or sell > > + * copies of the Software, and to permit persons to whom the Software is > > + * furnished to do so, subject to the following conditions: > > + * > > + * The above copyright notice and this permission notice shall be > included in > > + * all copies or substantial portions of the Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY > KIND, EXPRESS OR > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, > DAMAGES OR OTHER > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR > OTHERWISE, ARISING FROM, > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > DEALINGS IN > > + * THE SOFTWARE. > > + */ > > +#include "qemu/osdep.h" > > +#include "hw/isa/isa.h" > > +#include "crypto/hash.h" > > +#include "hw/misc/measurements.h" > > +#include "monitor/monitor.h" > > +#include "hw/loader.h" > > + > > +#define MEASUREMENT(obj) OBJECT_CHECK(MeasurementState, (obj), > TYPE_MEASUREMENTS) > > + > > +typedef struct MeasurementState MeasurementState; > > + > > +struct MeasurementState { > > + ISADevice parent_obj; > > + MemoryRegion io_select; > > + MemoryRegion io_value; > > + uint16_t iobase; > > + uint8_t measurements[24][20]; > > + uint8_t tmpmeasurement[20]; > > Magic numbers; please use #define's somewhere. > > > + int write_count; > > + int read_count; > > + uint8_t pcr; > > +}; > > + > > +static void measurement_reset(DeviceState *dev) > > +{ > > + MeasurementState *s = MEASUREMENT(dev); > > + > > + memset(s->measurements, 0, sizeof(s->measurements)); > > + measure_roms(); > > If you're assuming that can be none-0 then don't you also > want to zero pcr, read_count, write_count? > > > +} > > + > > +static void measurement_select(void *opaque, hwaddr addr, > uint64_t val, unsigned size) > > +{ > > + MeasurementState *s = MEASUREMENT(opaque); > > + s->pcr = val; > > + s->read_count = 0; > > + s->write_count = 0; > > What stops me selecting pcr 25 and overwriting stuff with junk? > > > +} > > + > > +static uint64_t measurement_version(void *opaque, hwaddr addr, > unsigned size) > > +{ > > + return 0; > > +} > > + > > +static uint64_t measurement_read(void *opaque, hwaddr addr, unsigned size) > > +{ > > + MeasurementState *s = MEASUREMENT(opaque); > > + > > + if (s->read_count == 20) { > > + s->read_count = 0; > > + } > > + return s->measurements[s->pcr][s->read_count++]; > > +} > > + > > +static void extend(MeasurementState *s, int pcrnum, uint8_t *data) > > +{ > > + uint8_t *result; > > + char tmpbuf[40]; > > + Error *err; > > + size_t resultlen = 0; > > + > > + memcpy(tmpbuf, s->measurements[pcrnum], 20); > > + memcpy(tmpbuf + 20, data, 20); > > + qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA1, tmpbuf, 40, > &result, &resultlen, &err); > > I think if you're ignoring any errors then using NULL instead of > &err is better. > > > + memcpy(s->measurements[pcrnum], result, 20); > > + g_free(result); > > +} > > + > > +static void measurement_value(void *opaque, hwaddr addr, uint64_t > val, unsigned size) > > +{ > > + MeasurementState *s = opaque; > > + > > + s->tmpmeasurement[s->write_count++] = val; > > + if (s->write_count == 20) { > > + extend(s, s->pcr, s->tmpmeasurement); > > + s->write_count = 0; > > + } > > +} > > + > > +void extend_data(int pcrnum, uint8_t *data, size_t len) > > +{ > > + uint8_t *result; > > + Error *err; > > + size_t resultlen = 0; > > + int ret; > > + Object *obj = object_resolve_path_type("", TYPE_MEASUREMENTS, NULL); > > + > > + if (!obj) { > > + return; > > + } > > + > > + ret = qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA1, (char *)data, > len, &result, > > + &resultlen, &err); > > Again probably NULL here for the err unless you actually want to report it. > > > + if (ret < 0) { > > + return; > > + } > > Hmm, what are the rules on freeing result on qcrypto_has_bytes returning > an error? > > > + > > + extend(MEASUREMENT(obj), pcrnum, result); > > + g_free(result); > > +} > > + > > +static const MemoryRegionOps measurement_select_ops = { > > + .write = measurement_select, > > + .read = measurement_version, > > + .endianness = DEVICE_NATIVE_ENDIAN, > > + .impl = { > > + .min_access_size = 1, > > + .max_access_size = 1, > > + }, > > +}; > > + > > +static const MemoryRegionOps measurement_value_ops = { > > + .write = measurement_value, > > + .read = measurement_read, > > + .endianness = DEVICE_NATIVE_ENDIAN, > > + .impl = { > > + .min_access_size = 1, > > + .max_access_size = 1, > > + }, > > +}; > > + > > +static void measurement_realize(DeviceState *dev, Error **errp) > > +{ > > + MeasurementState *s = MEASUREMENT(dev); > > + > > + memory_region_init_io(&s->io_select, OBJECT(s), > &measurement_select_ops, s, > > + "measurement-select", 1); > > + isa_register_ioport(&s->parent_obj, &s->io_select, s->iobase); > > + memory_region_init_io(&s->io_value, OBJECT(s), > &measurement_value_ops, s, > > + "measurement-value", 1); > > + isa_register_ioport(&s->parent_obj, &s->io_value, s->iobase + 1); > > + measurement_reset(dev); > > +} > > + > > +static Property measurement_props[] = { > > + DEFINE_PROP_UINT16(MEASUREMENTS_PROP_IO_BASE, > MeasurementState, iobase, 0x620), > > + DEFINE_PROP_END_OF_LIST(), > > +}; > > + > > +static void measurement_class_init(ObjectClass *klass, void *data) > > +{ > > + DeviceClass *dc = DEVICE_CLASS(klass); > > + fprintf(stderr, "CLASS INIT\n"); > > Oops, fprintf escaped into the wild. You might like to add some > trace entries. > > > + dc->realize = measurement_realize; > > + dc->reset = measurement_reset; > > + dc->props = measurement_props; > > + set_bit(DEVICE_CATEGORY_MISC, dc->categories); > > +} > > + > > +static const TypeInfo measurement = { > > + .name = TYPE_MEASUREMENTS, > > + .parent = TYPE_ISA_DEVICE, > > + .instance_size = sizeof(MeasurementState), > > + .class_init = measurement_class_init, > > +}; > > + > > +static void measurement_register_types(void) > > +{ > > + type_register_static(&measurement); > > +} > > + > > +type_init(measurement_register_types); > > + > > +void print_measurements(Monitor *mon, const QDict *qdict) > > +{ > > + int i, j; > > + Object *obj = object_resolve_path_type("", TYPE_MEASUREMENTS, NULL); > > + MeasurementState *s; > > + > > + if (!obj) { > > + return; > > + } > > + > > + s = MEASUREMENT(obj); > > + > > + for (i = 0; i < 24; i++) { > > for (pcr = .... > > > + monitor_printf(mon, "0x%02x: ", i); > > + for (j = 0; j < 20; j++) { > > + monitor_printf(mon, "0x%02x ", s->measurements[i][j]); > > + } > > + monitor_printf(mon, "\n"); > > + } > > +} > > diff --git a/hw/misc/measurements.h b/hw/misc/measurements.h > > new file mode 100644 > > index 0000000..65ad246 > > --- /dev/null > > +++ b/hw/misc/measurements.h > > @@ -0,0 +1,2 @@ > > +void print_measurements(Monitor *mon, const QDict *qdict); > > +void extend_data(int pcrnum, uint8_t *data, size_t len); > > diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h > > index c87fbad..00694fd 100644 > > --- a/include/hw/isa/isa.h > > +++ b/include/hw/isa/isa.h > > @@ -24,6 +24,9 @@ > > #define APPLESMC_MAX_DATA_LENGTH 32 > > #define APPLESMC_PROP_IO_BASE "iobase" > > > > +#define TYPE_MEASUREMENTS "measurements" > > +#define MEASUREMENTS_PROP_IO_BASE "iobase" > > + > > static inline uint16_t applesmc_port(void) > > { > > Object *obj = object_resolve_path_type("", TYPE_APPLE_SMC, NULL); > > @@ -34,6 +37,16 @@ static inline uint16_t applesmc_port(void) > > return 0; > > } > > > > +static inline uint16_t measurements_port(void) > > +{ > > + Object *obj = object_resolve_path_type("", TYPE_MEASUREMENTS, NULL); > > + > > + if (obj) { > > + return object_property_get_int(obj, > MEASUREMENTS_PROP_IO_BASE, NULL); > > + } > > + return 0; > > +} > > + > > #define TYPE_ISADMA "isa-dma" > > > > #define ISADMA_CLASS(klass) \ > > diff --git a/include/hw/loader.h b/include/hw/loader.h > > index 4879b63..cc3157d 100644 > > --- a/include/hw/loader.h > > +++ b/include/hw/loader.h > > @@ -133,6 +133,7 @@ void rom_reset_order_override(void); > > int rom_copy(uint8_t *dest, hwaddr addr, size_t size); > > void *rom_ptr(hwaddr addr); > > void hmp_info_roms(Monitor *mon, const QDict *qdict); > > +void measure_roms(void); > > > > #define rom_add_file_fixed(_f, _a, _i) \ > > rom_add_file(_f, NULL, _a, _i, false, NULL) > > diff --git a/monitor.c b/monitor.c > > index 6f960f1..6aa7ebc 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -32,6 +32,7 @@ > > #include "hw/pci/pci.h" > > #include "sysemu/watchdog.h" > > #include "hw/loader.h" > > +#include "hw/misc/measurements.h" > > #include "exec/gdbstub.h" > > #include "net/net.h" > > #include "net/slirp.h" > > diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs > > index 4b258a6..2ad7f81 100644 > > --- a/stubs/Makefile.objs > > +++ b/stubs/Makefile.objs > > @@ -41,3 +41,4 @@ stub-obj-y += target-monitor-defs.o > > stub-obj-y += target-get-monitor-def.o > > stub-obj-y += vhost.o > > stub-obj-y += iohandler.o > > +stub-obj-y += measurements.o > > diff --git a/stubs/measurements.c b/stubs/measurements.c > > new file mode 100644 > > index 0000000..0485d2e > > --- /dev/null > > +++ b/stubs/measurements.c > > @@ -0,0 +1,13 @@ > > +#include "qemu/osdep.h" > > +#include "monitor/monitor.h" > > +#include "hw/misc/measurements.h" > > + > > +void print_measurements(Monitor *mon, const QDict *qdict) > > +{ > > + monitor_printf(mon, "No measurement support\n"); > > +} > > + > > +void extend_data(int pcrnum, uint8_t *data, size_t len) > > +{ > > + return; > > +} > > -- > > 2.7.4 > > > > > > Dave > > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >
On Fri, Jul 15, 2016 at 4:29 AM, Dr. David Alan Gilbert <dgilbert@redhat.com > wrote: > * Matthew Garrett (mjg59@coreos.com) wrote: > a) (one that works) 'are all the VMs on my hosts running trusted OSs' > That works with this just as well as with a vTPM; you ask your > hypervisor to > give you the measurements for your guests; you trust your hypervisor. > Although I think you've somehow got to extract the measurement log > from the > guest and get it to the hypervisor if it's going to make sense of the > measurements. > The guest can provide the log, but you'd want to obtain the measurements from the hypervisor. The precise implementation of this would probably be somewhat provider-specific - AWS has support for providing signed copies of image metadata, so this could be implemented in a similar way (eg, guest hits the local API endpoint, hypervisor extracts measurement data and signs it, provides that back to guest, guest hands over log and signed measurements to whatever's handling the attestation) > b) (one that doesn't?) I'm connecting to a VM remotely over a network, > I want > to check the VM really is who it says it is and is running a trusted > OS. > As a remote entity I don't know which hypervisor is running the VM, > the VM > itself can't sign anything to give me back because it might just > sign a reply > for a different VM. On a real TPM the attestation results would be > signed > using one of the keys in the TPM (I can't remember which). > You have the same problem with a TPM - the quote you get back has a chain of trust back to the TPM vendor, but unless you've previously established some specific trust with that system you have no way of knowing whether it's the specific TPM you want to talk to or not. In some ways it's easier with a hypervisor, because it has more information about the guest than a TPM does about the OS. For example, the hypervisor can provide a signed measurement alongside signed metadata that includes the guest's IP address. If they're signed with the same key and the IP address matches what you're talking to, you've established that trust in a much more straightforward manner. > c) (similar to b) 'I paid you to give me a ... VM - can I check it > really is that' > how do I externally to the cloud show that the measurement came from > the same VM > I'm asking about. > I think that's covered as above. > and then I'm not clear which of the existing TPM users could be munged > into working > with it; can you make an existing trusted-grub or trousers write > measurements and log > into it? > Yes - my modified SeaBIOS provides the TCG measurement functions and simply stubs them into this rather than the real TPM. Running https://github.com/coreos/grub against that gives me a full set of boot measurements, including log. > > This driver provides a very small subset of TPM 1.2 functionality in the > form of a > > bank of registers that can store SHA1 measurements of boot components. > Performing a > > write to one of these registers will append the new 20 byte hash to the > 20 bytes > > currently stored within the register, take a SHA1 of this 40 byte value > and then > > replace the existing register contents with the new value. This ensures > that a > > given value can only be obtained by performing the same sequence of > writes. It also > > adds a monitor command to allow an external agent to extract this > information from > > the running system and provide it over a secure communications channel. > Finally, it > > measures each of the loaded ROMs into one of the registers at reset time. > > > > In combination with work in SeaBIOS and the kernel, this permits a fully > measured > > boot in a virtualised environment without the overhead of a full TPM > > implementation. > > So only SeaBIOS for now? Would it work for EFI in principle? > For now, because building EFI is tedious. There's nothing BIOS specific, and I'll add support to OVMF once we've nailed down what the interface looks like. > > This version of the implementation depends on port io, but if there's > interest I'll > > add mmio as well. > > I guess port IO has the advantage of making it easy to glue into the early > parts of the BIOS. > Yeah. Not sure whether the right approach is to use port IO where available and MMIO elsewhere, or just suck up the additional implementation work and do MMIO everywhere. > Some things I can see are missing: > Migration support: You probably want to migrate the current > measurements and > make sure you don't include the measurements of ROMs > on the > destination until after reset. > (Search for places that use dc->vmsd = .... as > examples) > You might want to add a measurement to indicate a > migration > happened; but that's a separate question. > Hmm, yes. I think we'd need to carefully consider what the semantics of measurement over migration are - do you think this is necessary for an initial implementation, or could it come later? > QMP support: You should wire it up to the qmp monitor as well. > Generally the management tools use qmp rather than hmp, > Good call. > What about hotplug? If I hotplug a NIC should it measure the new ROM? > What happens then if I restart the VM from clean with > the ROM added. > Mm good question. I *think* my argument would be that, unless we're executing code from that ROM, it shouldn't be measured. That's how it would behave with a real TPM on real hardware. So no to measurement on hotplug, yes to measurement if it's present after reboot. > Why do SHA1 based TPM 1.2 now, shouldn't you start with TPM2/newer SHAs? > There's no TPM2 spec for BIOS, so it'd involve adding an incompatible interface to BIOS to make use of it. However, adding support for additional hashes and then just having the BIOS code always use SHA1 ought to be fine. > I guess another approach to the same thing would be to bundle this up into > something that responded to TPM commands like a vTPM but just had less > inside it; then it could attach to the existing TPM interfaces? > My plan was to abstract this at the firmware interface level, rather than at the hardware level - for the functionality I'm looking at, emulating the TPM command set would add a *lot* of additional complexity. The benefit would be being able to use existing drivers, but I don't even know how much functionality I'd need to implement in order to get, say, Trousers running. > +void measure_roms(void) > +{ > + Rom *rom; > + QTAILQ_FOREACH(rom, &roms, next) { > + if (rom->data == NULL) { > + continue; > + } > + extend_data(0, rom->data, rom->datasize); > + } > +} Are you making unpredictable assumptions about ROM order? > You're explicitly measuring these into PCR 0 - I guess > that's probably reasonable but it should be documented. > Hm. The ordering issue can be basically avoided by having this be logged, which means passing the data over from qemu to the firmware and letting the firmware use that to populate the log. What's the best way to do that likely to be? > +struct MeasurementState { > + ISADevice parent_obj; > + MemoryRegion io_select; > + MemoryRegion io_value; > + uint16_t iobase; > + uint8_t measurements[24][20]; > + uint8_t tmpmeasurement[20]; Magic numbers; please use #define's somewhere. > Ok. > > + int write_count; > > + int read_count; > > + uint8_t pcr; > > +}; > > + > > +static void measurement_reset(DeviceState *dev) > > +{ > > + MeasurementState *s = MEASUREMENT(dev); > > + > > + memset(s->measurements, 0, sizeof(s->measurements)); > > + measure_roms(); > > If you're assuming that can be none-0 then don't you also > want to zero pcr, read_count, write_count? > Hm, yes. > > +} > > + > > +static void measurement_select(void *opaque, hwaddr addr, uint64_t val, > unsigned size) > > +{ > > + MeasurementState *s = MEASUREMENT(opaque); > > + s->pcr = val; > > + s->read_count = 0; > > + s->write_count = 0; > > What stops me selecting pcr 25 and overwriting stuff with junk? > Oops! Yeah uh that's a really rather good point and I am a bad programmer. > + memcpy(tmpbuf, s->measurements[pcrnum], 20); > + memcpy(tmpbuf + 20, data, 20); > + qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA1, tmpbuf, 40, &result, &resultlen, &err); I think if you're ignoring any errors then using NULL instead of &err is > better. > Ok. > > + if (ret < 0) { > > + return; > > + } > > Hmm, what are the rules on freeing result on qcrypto_has_bytes returning > an error? > I'll dig into the code. > + DeviceClass *dc = DEVICE_CLASS(klass); > + fprintf(stderr, "CLASS INIT\n"); Oops, fprintf escaped into the wild. You might like to add some trace > entries. > Heh. Yup. Thanks for the review! -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >
On Fri, Jul 15, 2016 at 11:11 AM, Stefan Berger <stefanb@us.ibm.com> wrote: > > Are you also providing a measurement log that goes along with these PCR extensions? Like a measurement log we have in the TCPA ACPI table? Just measurements without knowing what was measured wouldn't be all that helpful. Typically recipients of the measurement list would inspect the individual measurements and replay the extensions to come up with the same state of the PCRs that was quoted (signed) by the TPM. Also, are you going to instrument Linux IMA to use this device? And the list goes on into higher level tools that may work with a measurement list from /sys/kernel/security/{tpm0,ima}/*_measurement_list and assume there's a /dev/tpm0 there that can issue a quote with the AIK. Well, one problem is there's little traction for the vTPM but this device here will require new support in existing tools. Yes, the firmware will build a log - right now I'm using the TCPA format. However, the initial measurements from qemu aren't being handed over to the firmware, so I need to fix that. There's no fundamental difficulty in adding IMA support, I have a small driver that can register with enough of the TPM layer to allow its extend calls to work. I'm not too worried about the existing tooling not working, to be honest. Making that work would require the hypervisor to be able to provide a signed quote in TPM format, and that seems like a much harder sell. > > Typically the TPM is there for the reason: it is a hardware root of trust that signs the current state of the PCRs that were accumulated by measurements starting early on during BIOS init. Now with this device, apart from exposing this via HMP, how would one be sure that, if the current list of the PCRs is presented to an attesting client, that the kernel or attestation server not just completely fake the state of the PCRs? My assumption here is that the state of this device's PCRs will be exposed to user level application that can then use this in some form of attestation, right? Userspace will be able to grab it, but the idea is that the hypervisor API will allow a copy to be obtained - either a signed copy from the local API endpoint, or directly via a remote API endpoint. The guest won't be able to fake the former case, and isn't involved at all in the latter case.
Matthew Garrett <mjg59@coreos.com> wrote on 07/18/2016 05:26:03 PM: > > On Fri, Jul 15, 2016 at 11:11 AM, Stefan Berger <stefanb@us.ibm.com> wrote: > > > > > > Typically the TPM is there for the reason: it is a hardware root > of trust that signs the current state of the PCRs that were > accumulated by measurements starting early on during BIOS init. Now > with this device, apart from exposing this via HMP, how would one be > sure that, if the current list of the PCRs is presented to an > attesting client, that the kernel or attestation server not just > completely fake the state of the PCRs? My assumption here is that > the state of this device's PCRs will be exposed to user level > application that can then use this in some form of attestation, right? > > > Userspace will be able to grab it, but the idea is that the hypervisor > API will allow a copy to be obtained - either a signed copy from the > local API endpoint, or directly via a remote API endpoint. The guest > won't be able to fake the former case, and isn't involved at all in > the latter case. > The TPM security's model related to logs, the state of the PCRs, and attestation involves the following pieces: - PCRs - measurement log - EK + certificate - platform certificate - AIK + certificate - quotes (signatures) on PCR state with keys that cannot leave the TPM (AIKs) - infrastructure to issue the AIK certificates based on EK + certificate + platform certificate How does the security model of this device and its presumed infrastructure look like? Does the hypervisor then also support IMA measurement lists or is this restricted to firmware? Stefan
On Mon, Jul 18, 2016 at 4:40 PM, Stefan Berger <stefanb@us.ibm.com> wrote: > The TPM security's model related to logs, the state of the PCRs, and > attestation involves the following pieces: > > - PCRs > - measurement log > - EK + certificate > - platform certificate > - AIK + certificate > - quotes (signatures) on PCR state with keys that cannot leave the TPM > (AIKs) > - infrastructure to issue the AIK certificates based on EK + certificate + > platform certificate > > How does the security model of this device and its presumed infrastructure > look like? Does the hypervisor then also support IMA measurement lists or is > this restricted to firmware? The model here is: - PCRs - measurement log - quote on PCR state with key held by hypervisor There's no fundamental reason why additional layers of key can't be introduced, but since all that complexity is on the hypervisor side it's out of scope for the qemu implementation. A mechanism for establishing trust between the hypervisor and the customer is obviously necessary, but there are already examples such as Amazon's Instance Identity Documents ( http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-identity-documents.html ). The OS is free to continue to extend the PCRs after boot, so IMA could certainly be integrated with this.
Matthew Garrett <mjg59@coreos.com> wrote on 07/18/2016 07:52:22 PM: > Subject: Re: [Qemu-devel] [PATCH] hw/misc: Add simple measurement hardware > > On Mon, Jul 18, 2016 at 4:40 PM, Stefan Berger <stefanb@us.ibm.com> wrote: > > The TPM security's model related to logs, the state of the PCRs, and > > attestation involves the following pieces: > > > > - PCRs > > - measurement log > > - EK + certificate > > - platform certificate > > - AIK + certificate > > - quotes (signatures) on PCR state with keys that cannot leave the TPM > > (AIKs) > > - infrastructure to issue the AIK certificates based on EK + certificate + > > platform certificate > > > > How does the security model of this device and its presumed infrastructure > > look like? Does the hypervisor then also support IMA measurement lists or is > > this restricted to firmware? > > The model here is: > - PCRs > - measurement log > - quote on PCR state with key held by hypervisor > > There's no fundamental reason why additional layers of key can't be > introduced, but since all that complexity is on the hypervisor side > it's out of scope for the qemu implementation. A mechanism for > establishing trust between the hypervisor and the customer is > obviously necessary, but there are already examples such as Amazon's > Instance Identity Documents ( > http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance- > identity-documents.html > ). The OS is free to continue to extend the PCRs after boot, so IMA > could certainly be integrated with this. > The point of the TPM is that the device that holds the state of the PCRs provides the signatures over their state rather than some other 'entity' whose trustworthiness wouldn't be clear. Admittedly the device comes with its own set of challenges.
On Jul 18, 2016 17:08, "Stefan Berger" <stefanb@us.ibm.com> wrote:
> The point of the TPM is that the device that holds the state of the PCRs
provides the signatures over their state rather than some other 'entity'
whose trustworthiness wouldn't be clear. Admittedly the device comes with
its own set of challenges.
The hypervisor holds the PCR state and also provides the signature. If the
hypervisor is untrustworthy than the state of the virtualised system can
never be verified, since it could simply have faked the measurements passed
to whatever the root of trust is.
Matthew Garrett <mjg59@coreos.com> wrote on 07/18/2016 08:39:07 PM: > > On Jul 18, 2016 17:08, "Stefan Berger" <stefanb@us.ibm.com> wrote: > > The point of the TPM is that the device that holds the state of > the PCRs provides the signatures over their state rather than some > other 'entity' whose trustworthiness wouldn't be clear. Admittedly > the device comes with its own set of challenges. > The hypervisor holds the PCR state and also provides the signature. > If the hypervisor is untrustworthy than the state of the virtualised > system can never be verified, since it could simply have faked the > measurements passed to whatever the root of trust is. So the hypervisor will have the key for signing and provide the quote ? Stefan
On Jul 18, 2016 17:46, "Stefan Berger" <stefanb@us.ibm.com> wrote: > > > Matthew Garrett <mjg59@coreos.com> wrote on 07/18/2016 08:39:07 PM: > > > > > > On Jul 18, 2016 17:08, "Stefan Berger" <stefanb@us.ibm.com> wrote: > > > The point of the TPM is that the device that holds the state of > > the PCRs provides the signatures over their state rather than some > > other 'entity' whose trustworthiness wouldn't be clear. Admittedly > > the device comes with its own set of challenges. > > > The hypervisor holds the PCR state and also provides the signature. > > If the hypervisor is untrustworthy than the state of the virtualised > > system can never be verified, since it could simply have faked the > > measurements passed to whatever the root of trust is. > > So the hypervisor will have the key for signing and provide the quote ? Either the hypervisor itself or part of the associated platform. This framework is typically inside the same trust boundary.
* Matthew Garrett (mjg59@coreos.com) wrote: > On Fri, Jul 15, 2016 at 4:29 AM, Dr. David Alan Gilbert <dgilbert@redhat.com > > wrote: > > > * Matthew Garrett (mjg59@coreos.com) wrote: > > a) (one that works) 'are all the VMs on my hosts running trusted OSs' > > That works with this just as well as with a vTPM; you ask your > > hypervisor to > > give you the measurements for your guests; you trust your hypervisor. > > Although I think you've somehow got to extract the measurement log > > from the > > guest and get it to the hypervisor if it's going to make sense of the > > measurements. > > > > The guest can provide the log, but you'd want to obtain the measurements > from the hypervisor. The precise implementation of this would probably be > somewhat provider-specific - AWS has support for providing signed copies of > image metadata, so this could be implemented in a similar way (eg, guest > hits the local API endpoint, hypervisor extracts measurement data and signs > it, provides that back to guest, guest hands over log and signed > measurements to whatever's handling the attestation) Yes, this doesn't seem too bad. > > b) (one that doesn't?) I'm connecting to a VM remotely over a network, > > I want > > to check the VM really is who it says it is and is running a trusted > > OS. > > As a remote entity I don't know which hypervisor is running the VM, > > the VM > > itself can't sign anything to give me back because it might just > > sign a reply > > for a different VM. On a real TPM the attestation results would be > > signed > > using one of the keys in the TPM (I can't remember which). > > > > You have the same problem with a TPM - the quote you get back has a chain > of trust back to the TPM vendor, but unless you've previously established > some specific trust with that system you have no way of knowing whether > it's the specific TPM you want to talk to or not. In some ways it's easier > with a hypervisor, because it has more information about the guest than a > TPM does about the OS. For example, the hypervisor can provide a signed > measurement alongside signed metadata that includes the guest's IP address. > If they're signed with the same key and the IP address matches what you're > talking to, you've established that trust in a much more straightforward > manner. Hmm true; with TPM once you have done it once and enrolled it you do know that you're making the measurement against the same device; but in your world that's probably true if the host is involved somewhere with getting the measurement. But I don't see in your description how the guest would get a copy of it's PCRs signed by the host. > > c) (similar to b) 'I paid you to give me a ... VM - can I check it > > really is that' > > how do I externally to the cloud show that the measurement came from > > the same VM > > I'm asking about. > > > > I think that's covered as above. Yep, same. > > and then I'm not clear which of the existing TPM users could be munged > > into working > > with it; can you make an existing trusted-grub or trousers write > > measurements and log > > into it? > > > > Yes - my modified SeaBIOS provides the TCG measurement functions and simply > stubs them into this rather than the real TPM. Running > https://github.com/coreos/grub against that gives me a full set of boot > measurements, including log. Is that reimplementing TPM support in grub? Isn't there a version that already has it? Dealing with those measurements is really difficult in practice - in particular things like knowing whether a kernel command line is sane/safe is pretty difficult, as is knowing the sanity of an initrd; so please make sure those measurements as close to normal TPM behaviour as possible. > > > This driver provides a very small subset of TPM 1.2 functionality in the > > form of a > > > bank of registers that can store SHA1 measurements of boot components. > > Performing a > > > write to one of these registers will append the new 20 byte hash to the > > 20 bytes > > > currently stored within the register, take a SHA1 of this 40 byte value > > and then > > > replace the existing register contents with the new value. This ensures > > that a > > > given value can only be obtained by performing the same sequence of > > writes. It also > > > adds a monitor command to allow an external agent to extract this > > information from > > > the running system and provide it over a secure communications channel. > > Finally, it > > > measures each of the loaded ROMs into one of the registers at reset time. > > > > > > In combination with work in SeaBIOS and the kernel, this permits a fully > > measured > > > boot in a virtualised environment without the overhead of a full TPM > > > implementation. > > > > So only SeaBIOS for now? Would it work for EFI in principle? > > > > For now, because building EFI is tedious. There's nothing BIOS specific, > and I'll add support to OVMF once we've nailed down what the interface > looks like. > > > > > This version of the implementation depends on port io, but if there's > > interest I'll > > > add mmio as well. > > > > I guess port IO has the advantage of making it easy to glue into the early > > parts of the BIOS. > > > > Yeah. Not sure whether the right approach is to use port IO where available > and MMIO elsewhere, or just suck up the additional implementation work and > do MMIO everywhere. > > > > Some things I can see are missing: > > Migration support: You probably want to migrate the current > > measurements and > > make sure you don't include the measurements of ROMs > > on the > > destination until after reset. > > (Search for places that use dc->vmsd = .... as > > examples) > > You might want to add a measurement to indicate a > > migration > > happened; but that's a separate question. > > > > Hmm, yes. I think we'd need to carefully consider what the semantics of > measurement over migration are - do you think this is necessary for an > initial implementation, or could it come later? My main work is migration so of course I'd say you should think about it; but you're right you can probably leave it at first. My assumption would be that you'd just migrate the values as part of the stream (pretty easy see VMState stuff in other drivers). I can't remember the behaviour you'll see after a reset on the destination and whether that means you get the destinations ROMs or the ROMs that were migrated. > > QMP support: You should wire it up to the qmp monitor as well. > > Generally the management tools use qmp rather than hmp, > > > > Good call. > > > > What about hotplug? If I hotplug a NIC should it measure the new ROM? > > What happens then if I restart the VM from clean with > > the ROM added. > > > > Mm good question. I *think* my argument would be that, unless we're > executing code from that ROM, it shouldn't be measured. That's how it would > behave with a real TPM on real hardware. So no to measurement on hotplug, > yes to measurement if it's present after reboot. OK. > > Why do SHA1 based TPM 1.2 now, shouldn't you start with TPM2/newer SHAs? > > > > There's no TPM2 spec for BIOS, so it'd involve adding an incompatible > interface to BIOS to make use of it. However, adding support for additional > hashes and then just having the BIOS code always use SHA1 ought to be fine. Yes, although it's probably messier than that; if I remember correctly the new hashes are longer. > > I guess another approach to the same thing would be to bundle this up into > > something that responded to TPM commands like a vTPM but just had less > > inside it; then it could attach to the existing TPM interfaces? > > > > My plan was to abstract this at the firmware interface level, rather than > at the hardware level - for the functionality I'm looking at, emulating the > TPM command set would add a *lot* of additional complexity. The benefit > would be being able to use existing drivers, but I don't even know how much > functionality I'd need to implement in order to get, say, Trousers running. > > > +void measure_roms(void) > > +{ > > + Rom *rom; > > + QTAILQ_FOREACH(rom, &roms, next) { > > + if (rom->data == NULL) { > > + continue; > > + } > > + extend_data(0, rom->data, rom->datasize); > > + } > > +} > > Are you making unpredictable assumptions about ROM order? > > You're explicitly measuring these into PCR 0 - I guess > > that's probably reasonable but it should be documented. > > > > Hm. The ordering issue can be basically avoided by having this be logged, > which means passing the data over from qemu to the firmware and letting the > firmware use that to populate the log. What's the best way to do that > likely to be? Hmm, not sure. We know this is happening before the guest starts running, so if it somehow made it's way to the code that sets up ACPI perhaps that could do it; but I'm not sure. > > +struct MeasurementState { > > + ISADevice parent_obj; > > + MemoryRegion io_select; > > + MemoryRegion io_value; > > + uint16_t iobase; > > + uint8_t measurements[24][20]; > > + uint8_t tmpmeasurement[20]; > > Magic numbers; please use #define's somewhere. > > > > Ok. > > > > > + int write_count; > > > + int read_count; > > > + uint8_t pcr; > > > +}; > > > + > > > +static void measurement_reset(DeviceState *dev) > > > +{ > > > + MeasurementState *s = MEASUREMENT(dev); > > > + > > > + memset(s->measurements, 0, sizeof(s->measurements)); > > > + measure_roms(); > > > > If you're assuming that can be none-0 then don't you also > > want to zero pcr, read_count, write_count? > > > > Hm, yes. > > > > > +} > > > + > > > +static void measurement_select(void *opaque, hwaddr addr, uint64_t val, > > unsigned size) > > > +{ > > > + MeasurementState *s = MEASUREMENT(opaque); > > > + s->pcr = val; > > > + s->read_count = 0; > > > + s->write_count = 0; > > > > What stops me selecting pcr 25 and overwriting stuff with junk? > > > > Oops! Yeah uh that's a really rather good point and I am a bad programmer. > > > + memcpy(tmpbuf, s->measurements[pcrnum], 20); > > + memcpy(tmpbuf + 20, data, 20); > > + qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA1, tmpbuf, 40, &result, > &resultlen, &err); > > I think if you're ignoring any errors then using NULL instead of &err is > > better. > > > > Ok. > > > > > + if (ret < 0) { > > > + return; > > > + } > > > > Hmm, what are the rules on freeing result on qcrypto_has_bytes returning > > an error? > > > > I'll dig into the code. > > > + DeviceClass *dc = DEVICE_CLASS(klass); > > + fprintf(stderr, "CLASS INIT\n"); > > Oops, fprintf escaped into the wild. You might like to add some trace > > entries. > > > > Heh. Yup. > > Thanks for the review! No problem. Dave > -- > > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak index 6e3b312..6f0fcc3 100644 --- a/default-configs/x86_64-softmmu.mak +++ b/default-configs/x86_64-softmmu.mak @@ -58,3 +58,4 @@ CONFIG_IOH3420=y CONFIG_I82801B11=y CONFIG_SMBIOS=y CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM) +CONFIG_MEASUREMENTS=y diff --git a/hmp-commands.hx b/hmp-commands.hx index 98b4b1a..6a31392 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1748,6 +1748,19 @@ Set QOM property @var{property} of object at location @var{path} to value @var{v ETEXI { + .name = "measurements", + .args_type = "", + .params = "", + .help = "Print system measurements", + .mhandler.cmd = print_measurements, + }, +STEXI +@item measurements +@findex measurements +Redirect Print system measurements +ETEXI + + { .name = "info", .args_type = "item:s?", .params = "[subcommand]", diff --git a/hw/core/loader.c b/hw/core/loader.c index 53e0e41..e1b7af7 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -55,6 +55,7 @@ #include "exec/address-spaces.h" #include "hw/boards.h" #include "qemu/cutils.h" +#include "hw/misc/measurements.h" #include <zlib.h> @@ -1026,6 +1027,17 @@ static void rom_reset(void *unused) } } +void measure_roms(void) +{ + Rom *rom; + QTAILQ_FOREACH(rom, &roms, next) { + if (rom->data == NULL) { + continue; + } + extend_data(0, rom->data, rom->datasize); + } +} + int rom_check_and_register_reset(void) { hwaddr addr = 0; diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 8ca2032..92129d1 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -107,6 +107,7 @@ typedef struct AcpiMiscInfo { unsigned dsdt_size; uint16_t pvpanic_port; uint16_t applesmc_io_base; + uint16_t measurements_io_base; } AcpiMiscInfo; typedef struct AcpiBuildPciBusHotplugState { @@ -203,6 +204,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info) info->tpm_version = tpm_get_version(); info->pvpanic_port = pvpanic_port(); info->applesmc_io_base = applesmc_port(); + info->measurements_io_base = measurements_port(); } /* @@ -2185,6 +2187,26 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, aml_append(dsdt, scope); } + if (misc->measurements_io_base) { + scope = aml_scope("\\_SB.PCI0.ISA"); + dev = aml_device("PCRS"); + + aml_append(dev, aml_name_decl("_HID", aml_eisaid("CORE0001"))); + /* device present, functioning, decoding, not shown in UI */ + aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); + + crs = aml_resource_template(); + aml_append(crs, + aml_io(AML_DECODE16, misc->measurements_io_base, + misc->measurements_io_base, + 0x01, 2) + ); + aml_append(dev, aml_name_decl("_CRS", crs)); + + aml_append(scope, dev); + aml_append(dsdt, scope); + } + sb_scope = aml_scope("\\_SB"); { build_memory_devices(sb_scope, nr_mem, pm->mem_hp_io_base, diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs index ffb49c1..e7a784b 100644 --- a/hw/misc/Makefile.objs +++ b/hw/misc/Makefile.objs @@ -5,6 +5,7 @@ common-obj-$(CONFIG_ISA_DEBUG) += debugexit.o common-obj-$(CONFIG_SGA) += sga.o common-obj-$(CONFIG_ISA_TESTDEV) += pc-testdev.o common-obj-$(CONFIG_PCI_TESTDEV) += pci-testdev.o +common-obj-$(CONFIG_MEASUREMENTS) += measurements.o obj-$(CONFIG_VMPORT) += vmport.o diff --git a/hw/misc/measurements.c b/hw/misc/measurements.c new file mode 100644 index 0000000..3940d31 --- /dev/null +++ b/hw/misc/measurements.c @@ -0,0 +1,206 @@ +/* + * QEMU boot measurement + * + * Copyright (c) 2016 CoreOS, Inc <mjg59@coreos.com> + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +#include "qemu/osdep.h" +#include "hw/isa/isa.h" +#include "crypto/hash.h" +#include "hw/misc/measurements.h" +#include "monitor/monitor.h" +#include "hw/loader.h" + +#define MEASUREMENT(obj) OBJECT_CHECK(MeasurementState, (obj), TYPE_MEASUREMENTS) + +typedef struct MeasurementState MeasurementState; + +struct MeasurementState { + ISADevice parent_obj; + MemoryRegion io_select; + MemoryRegion io_value; + uint16_t iobase; + uint8_t measurements[24][20]; + uint8_t tmpmeasurement[20]; + int write_count; + int read_count; + uint8_t pcr; +}; + +static void measurement_reset(DeviceState *dev) +{ + MeasurementState *s = MEASUREMENT(dev); + + memset(s->measurements, 0, sizeof(s->measurements)); + measure_roms(); +} + +static void measurement_select(void *opaque, hwaddr addr, uint64_t val, unsigned size) +{ + MeasurementState *s = MEASUREMENT(opaque); + s->pcr = val; + s->read_count = 0; + s->write_count = 0; +} + +static uint64_t measurement_version(void *opaque, hwaddr addr, unsigned size) +{ + return 0; +} + +static uint64_t measurement_read(void *opaque, hwaddr addr, unsigned size) +{ + MeasurementState *s = MEASUREMENT(opaque); + + if (s->read_count == 20) { + s->read_count = 0; + } + return s->measurements[s->pcr][s->read_count++]; +} + +static void extend(MeasurementState *s, int pcrnum, uint8_t *data) +{ + uint8_t *result; + char tmpbuf[40]; + Error *err; + size_t resultlen = 0; + + memcpy(tmpbuf, s->measurements[pcrnum], 20); + memcpy(tmpbuf + 20, data, 20); + qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA1, tmpbuf, 40, &result, &resultlen, &err); + memcpy(s->measurements[pcrnum], result, 20); + g_free(result); +} + +static void measurement_value(void *opaque, hwaddr addr, uint64_t val, unsigned size) +{ + MeasurementState *s = opaque; + + s->tmpmeasurement[s->write_count++] = val; + if (s->write_count == 20) { + extend(s, s->pcr, s->tmpmeasurement); + s->write_count = 0; + } +} + +void extend_data(int pcrnum, uint8_t *data, size_t len) +{ + uint8_t *result; + Error *err; + size_t resultlen = 0; + int ret; + Object *obj = object_resolve_path_type("", TYPE_MEASUREMENTS, NULL); + + if (!obj) { + return; + } + + ret = qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA1, (char *)data, len, &result, + &resultlen, &err); + if (ret < 0) { + return; + } + + extend(MEASUREMENT(obj), pcrnum, result); + g_free(result); +} + +static const MemoryRegionOps measurement_select_ops = { + .write = measurement_select, + .read = measurement_version, + .endianness = DEVICE_NATIVE_ENDIAN, + .impl = { + .min_access_size = 1, + .max_access_size = 1, + }, +}; + +static const MemoryRegionOps measurement_value_ops = { + .write = measurement_value, + .read = measurement_read, + .endianness = DEVICE_NATIVE_ENDIAN, + .impl = { + .min_access_size = 1, + .max_access_size = 1, + }, +}; + +static void measurement_realize(DeviceState *dev, Error **errp) +{ + MeasurementState *s = MEASUREMENT(dev); + + memory_region_init_io(&s->io_select, OBJECT(s), &measurement_select_ops, s, + "measurement-select", 1); + isa_register_ioport(&s->parent_obj, &s->io_select, s->iobase); + memory_region_init_io(&s->io_value, OBJECT(s), &measurement_value_ops, s, + "measurement-value", 1); + isa_register_ioport(&s->parent_obj, &s->io_value, s->iobase + 1); + measurement_reset(dev); +} + +static Property measurement_props[] = { + DEFINE_PROP_UINT16(MEASUREMENTS_PROP_IO_BASE, MeasurementState, iobase, 0x620), + DEFINE_PROP_END_OF_LIST(), +}; + +static void measurement_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + fprintf(stderr, "CLASS INIT\n"); + dc->realize = measurement_realize; + dc->reset = measurement_reset; + dc->props = measurement_props; + set_bit(DEVICE_CATEGORY_MISC, dc->categories); +} + +static const TypeInfo measurement = { + .name = TYPE_MEASUREMENTS, + .parent = TYPE_ISA_DEVICE, + .instance_size = sizeof(MeasurementState), + .class_init = measurement_class_init, +}; + +static void measurement_register_types(void) +{ + type_register_static(&measurement); +} + +type_init(measurement_register_types); + +void print_measurements(Monitor *mon, const QDict *qdict) +{ + int i, j; + Object *obj = object_resolve_path_type("", TYPE_MEASUREMENTS, NULL); + MeasurementState *s; + + if (!obj) { + return; + } + + s = MEASUREMENT(obj); + + for (i = 0; i < 24; i++) { + monitor_printf(mon, "0x%02x: ", i); + for (j = 0; j < 20; j++) { + monitor_printf(mon, "0x%02x ", s->measurements[i][j]); + } + monitor_printf(mon, "\n"); + } +} diff --git a/hw/misc/measurements.h b/hw/misc/measurements.h new file mode 100644 index 0000000..65ad246 --- /dev/null +++ b/hw/misc/measurements.h @@ -0,0 +1,2 @@ +void print_measurements(Monitor *mon, const QDict *qdict); +void extend_data(int pcrnum, uint8_t *data, size_t len); diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h index c87fbad..00694fd 100644 --- a/include/hw/isa/isa.h +++ b/include/hw/isa/isa.h @@ -24,6 +24,9 @@ #define APPLESMC_MAX_DATA_LENGTH 32 #define APPLESMC_PROP_IO_BASE "iobase" +#define TYPE_MEASUREMENTS "measurements" +#define MEASUREMENTS_PROP_IO_BASE "iobase" + static inline uint16_t applesmc_port(void) { Object *obj = object_resolve_path_type("", TYPE_APPLE_SMC, NULL); @@ -34,6 +37,16 @@ static inline uint16_t applesmc_port(void) return 0; } +static inline uint16_t measurements_port(void) +{ + Object *obj = object_resolve_path_type("", TYPE_MEASUREMENTS, NULL); + + if (obj) { + return object_property_get_int(obj, MEASUREMENTS_PROP_IO_BASE, NULL); + } + return 0; +} + #define TYPE_ISADMA "isa-dma" #define ISADMA_CLASS(klass) \ diff --git a/include/hw/loader.h b/include/hw/loader.h index 4879b63..cc3157d 100644 --- a/include/hw/loader.h +++ b/include/hw/loader.h @@ -133,6 +133,7 @@ void rom_reset_order_override(void); int rom_copy(uint8_t *dest, hwaddr addr, size_t size); void *rom_ptr(hwaddr addr); void hmp_info_roms(Monitor *mon, const QDict *qdict); +void measure_roms(void); #define rom_add_file_fixed(_f, _a, _i) \ rom_add_file(_f, NULL, _a, _i, false, NULL) diff --git a/monitor.c b/monitor.c index 6f960f1..6aa7ebc 100644 --- a/monitor.c +++ b/monitor.c @@ -32,6 +32,7 @@ #include "hw/pci/pci.h" #include "sysemu/watchdog.h" #include "hw/loader.h" +#include "hw/misc/measurements.h" #include "exec/gdbstub.h" #include "net/net.h" #include "net/slirp.h" diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index 4b258a6..2ad7f81 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -41,3 +41,4 @@ stub-obj-y += target-monitor-defs.o stub-obj-y += target-get-monitor-def.o stub-obj-y += vhost.o stub-obj-y += iohandler.o +stub-obj-y += measurements.o diff --git a/stubs/measurements.c b/stubs/measurements.c new file mode 100644 index 0000000..0485d2e --- /dev/null +++ b/stubs/measurements.c @@ -0,0 +1,13 @@ +#include "qemu/osdep.h" +#include "monitor/monitor.h" +#include "hw/misc/measurements.h" + +void print_measurements(Monitor *mon, const QDict *qdict) +{ + monitor_printf(mon, "No measurement support\n"); +} + +void extend_data(int pcrnum, uint8_t *data, size_t len) +{ + return; +}
Trusted Boot is based around having a trusted store of measurement data and a secure communications channel between that store and an attestation target. In actual hardware, that's a TPM. Since the TPM can only be accessed via the host system, this in turn requires that the TPM be able to perform reasonably complicated cryptographic functions in order to demonstrate its trusted state. In cloud environments, qemu is inherently trusted and the hypervisor infrastructure provides a trusted mechanism for extracting information from qemu and providing it to another system. This means we can skip the crypto and stick with the basic functionality - ie, providing a trusted store of measurement data. This driver provides a very small subset of TPM 1.2 functionality in the form of a bank of registers that can store SHA1 measurements of boot components. Performing a write to one of these registers will append the new 20 byte hash to the 20 bytes currently stored within the register, take a SHA1 of this 40 byte value and then replace the existing register contents with the new value. This ensures that a given value can only be obtained by performing the same sequence of writes. It also adds a monitor command to allow an external agent to extract this information from the running system and provide it over a secure communications channel. Finally, it measures each of the loaded ROMs into one of the registers at reset time. In combination with work in SeaBIOS and the kernel, this permits a fully measured boot in a virtualised environment without the overhead of a full TPM implementation. This version of the implementation depends on port io, but if there's interest I'll add mmio as well. Signed-off-by: Matthew Garrett <mjg59@coreos.com> --- default-configs/x86_64-softmmu.mak | 1 + hmp-commands.hx | 13 +++ hw/core/loader.c | 12 +++ hw/i386/acpi-build.c | 22 ++++ hw/misc/Makefile.objs | 1 + hw/misc/measurements.c | 206 +++++++++++++++++++++++++++++++++++++ hw/misc/measurements.h | 2 + include/hw/isa/isa.h | 13 +++ include/hw/loader.h | 1 + monitor.c | 1 + stubs/Makefile.objs | 1 + stubs/measurements.c | 13 +++ 12 files changed, 286 insertions(+) create mode 100644 hw/misc/measurements.c create mode 100644 hw/misc/measurements.h create mode 100644 stubs/measurements.c