| Message ID | 147377822450.11859.5845767550630184079.stgit@brijesh-build-machine (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Sep 13, 2016 at 10:50:24AM -0400, Brijesh Singh wrote: > In SEV-enabled mode we need to reload the BIOS image on loader reset, this > will ensure that BIOS image gets encrypted and included as part of launch > meausrement on guest reset. > > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> Could you pls explain what does measurement mean in this context, and how is it helpful to reload rom on every boot as opposed to first boot after migration? > --- > hw/core/loader.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/hw/core/loader.c b/hw/core/loader.c > index 53e0e41..9b03bfe 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 "sysemu/kvm.h" > > #include <zlib.h> > > @@ -1011,7 +1012,11 @@ static void rom_reset(void *unused) > cpu_physical_memory_write_rom(&address_space_memory, > rom->addr, rom->data, rom->datasize); > } > - if (rom->isrom) { > + > + /* reload the rom on SEV-enabled guest so that it gets included into > + * guest memory measurement on system reset. > + */ > + if (!kvm_sev_enabled() && rom->isrom) { > /* rom needs to be written only once */ > g_free(rom->data); > rom->data = NULL;
On 13/09/2016 16:50, Brijesh Singh wrote: > In SEV-enabled mode we need to reload the BIOS image on loader reset, this > will ensure that BIOS image gets encrypted and included as part of launch > meausrement on guest reset. Just to check if I understand correctly, the secure processor cannot split the encryption and measuring, which is why you need to redo the copy on every reset. Does the guest have to check the measured data (e.g. with a hash) too, to check that it hasn't been tampered with outside the secure processor's control? Of course this would result in garbage written to the modified page, but that might be a valid attack vector. Paolo
On Wed, Sep 14, 2016 at 12:59:32AM +0200, Paolo Bonzini wrote: > > > On 13/09/2016 16:50, Brijesh Singh wrote: > > In SEV-enabled mode we need to reload the BIOS image on loader reset, this > > will ensure that BIOS image gets encrypted and included as part of launch > > meausrement on guest reset. > > Just to check if I understand correctly, the secure processor cannot > split the encryption and measuring, which is why you need to redo the > copy on every reset. > > Does the guest have to check the measured data (e.g. with a hash) too, > to check that it hasn't been tampered with outside the secure > processor's control? I don't know what does measurement mean in this context as this patchset was supposed to be about protecting memory so you don't get to steal guest secrets just because you get to steal hypervisor secrets. Guest checking anything at all seems highly unlikely to help in this context, as they probably already happened by the time guest is checking things. > Of course this would result in garbage written to > the modified page, but that might be a valid attack vector. > > Paolo
On 09/13/2016 05:59 PM, Paolo Bonzini wrote: > > > On 13/09/2016 16:50, Brijesh Singh wrote: >> In SEV-enabled mode we need to reload the BIOS image on loader reset, this >> will ensure that BIOS image gets encrypted and included as part of launch >> meausrement on guest reset. > > Just to check if I understand correctly, the secure processor cannot > split the encryption and measuring, which is why you need to redo the > copy on every reset. > That is right, after LAUNCH_FINISH is called the secure processor cleanup the LAUNCH_START context so that hypervisor can not call LAUNCH_UPDATE to inject a new data into guest memory. After LAUNCH_FINISH only thing we can call is SEV_DEBUG_* or SEV_RECEIVE_* commands. > Does the guest have to check the measured data (e.g. with a hash) too, > to check that it hasn't been tampered with outside the secure > processor's control? Of course this would result in garbage written to > the modified page, but that might be a valid attack vector. > Guest does not need to check the measurement.
On 14/09/2016 22:29, Brijesh Singh wrote: >> Does the guest have to check the measured data (e.g. with a hash) too, >> to check that it hasn't been tampered with outside the secure >> processor's control? Of course this would result in garbage written to >> the modified page, but that might be a valid attack vector. > > Guest does not need to check the measurement. Can you explain why not? Paolo
On Wed, Sep 14, 2016 at 10:38:58PM +0200, Paolo Bonzini wrote: > > > On 14/09/2016 22:29, Brijesh Singh wrote: > >> Does the guest have to check the measured data (e.g. with a hash) too, > >> to check that it hasn't been tampered with outside the secure > >> processor's control? Of course this would result in garbage written to > >> the modified page, but that might be a valid attack vector. > > > > Guest does not need to check the measurement. > > Can you explain why not? > > Paolo For example, guest can boot in a secure environment and then be migrated to cloud. In fact that seems much easier to manage than all the hash based stuff.
On 14/09/2016 23:09, Michael S. Tsirkin wrote: > > > > Does the guest have to check the measured data (e.g. with a hash) too, > > > > to check that it hasn't been tampered with outside the secure > > > > processor's control? Of course this would result in garbage written to > > > > the modified page, but that might be a valid attack vector. > > > > > > Guest does not need to check the measurement. > > > > Can you explain why not? > > For example, guest can boot in a secure environment and then be migrated > to cloud. In fact that seems much easier to manage than all the hash > based stuff. This is not what I was asking. My question was: assuming that the guest is interested in checking the measurement, does it also have to recompute it independently, and if not why? Paolo
On 09/14/2016 03:38 PM, Paolo Bonzini wrote: > > > On 14/09/2016 22:29, Brijesh Singh wrote: >>> Does the guest have to check the measured data (e.g. with a hash) too, >>> to check that it hasn't been tampered with outside the secure >>> processor's control? Of course this would result in garbage written to >>> the modified page, but that might be a valid attack vector. >> >> Guest does not need to check the measurement. > > Can you explain why not? > Paolo, this is good question, I will check this internally and come back to you. > Paolo >
diff --git a/hw/core/loader.c b/hw/core/loader.c index 53e0e41..9b03bfe 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 "sysemu/kvm.h" #include <zlib.h> @@ -1011,7 +1012,11 @@ static void rom_reset(void *unused) cpu_physical_memory_write_rom(&address_space_memory, rom->addr, rom->data, rom->datasize); } - if (rom->isrom) { + + /* reload the rom on SEV-enabled guest so that it gets included into + * guest memory measurement on system reset. + */ + if (!kvm_sev_enabled() && rom->isrom) { /* rom needs to be written only once */ g_free(rom->data); rom->data = NULL;
In SEV-enabled mode we need to reload the BIOS image on loader reset, this will ensure that BIOS image gets encrypted and included as part of launch meausrement on guest reset. Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- hw/core/loader.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)