On Fri, Jul 15, 2016 at 4:29 AM, Dr. David Alan Gilbert <dgilb...@redhat.com > wrote:
> * Matthew Garrett (mj...@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 / dgilb...@redhat.com / Manchester, UK >
