| Message ID | 20241125195626.856992-13-jean-philippe@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | arm: Run Arm CCA VMs with KVM | expand |
On Mon, Nov 25, 2024 at 07:56:10PM +0000, Jean-Philippe Brucker wrote: > This option selects which measurement algorithm to use for attestation. > Supported values are SHA256 and SHA512. Default to SHA512 arbitrarily. I'd suggest that defaulting to sha256 is the better choice. sha512 is overkill from a security POV, while doubling the size of hash data that has to be passed around. It is unusual for applications needing hashing to pick 512 over 256 IME. Shorter config param data is a win for QEMU command line parameters. > SHA512 is generally faster on 64-bit architectures. On a few arm64 CPUs > I tested SHA256 is much faster, but that's most likely because they only > support acceleration via FEAT_SHA256 (Armv8.0) and not FEAT_SHA512 > (Armv8.2). Future CPUs supporting RME are likely to also support > FEAT_SHA512. IMHO speed is largely tangential for this use case, as it is a one time hash operation at VM startup. > Cc: Eric Blake <eblake@redhat.com> > Cc: Markus Armbruster <armbru@redhat.com> > Cc: Daniel P. Berrangé <berrange@redhat.com> > Cc: Eduardo Habkost <eduardo@habkost.net> > Acked-by: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > --- > v2->v3: Rename measurement-algorithm > --- > qapi/qom.json | 20 +++++++++++++++++++- > target/arm/kvm-rme.c | 39 ++++++++++++++++++++++++++++++++++++++- > 2 files changed, 57 insertions(+), 2 deletions(-) > > diff --git a/qapi/qom.json b/qapi/qom.json > index f982850bca..901ba67634 100644 > --- a/qapi/qom.json > +++ b/qapi/qom.json > @@ -1068,6 +1068,20 @@ > 'data': { '*cpu-affinity': ['uint16'], > '*node-affinity': ['uint16'] } } > > +## > +# @RmeGuestMeasurementAlgorithm: > +# > +# @sha256: Use the SHA256 algorithm > +# > +# @sha512: Use the SHA512 algorithm > +# > +# Algorithm to use for realm measurements > +# > +# Since: 9.3 > +## > +{ 'enum': 'RmeGuestMeasurementAlgorithm', > + 'data': ['sha256', 'sha512'] } A design question for Markus.... We have a "QCryptoHashAlg" enum that covers both of these strings and more besides. We have a choice of using a dedicated enum strictly limited to just what RME needs, vs using the common enum type, but rejecting unsupported algorithms at runtime. Do we prefer duplication for improve specificity, or removal of duplication ? > + > ## > # @RmeGuestProperties: > # > @@ -1077,10 +1091,14 @@ > # hex string. This optional parameter allows to uniquely identify > # the VM instance during attestation. (default: 0) > # > +# @measurement-algorithm: Realm measurement algorithm > +# (default: sha512) > +# > # Since: 9.3 > ## > { 'struct': 'RmeGuestProperties', > - 'data': { '*personalization-value': 'str' } } > + 'data': { '*personalization-value': 'str', > + '*measurement-algorithm': 'RmeGuestMeasurementAlgorithm' } } > With regards, Daniel
Daniel P. Berrangé <berrange@redhat.com> writes: > On Mon, Nov 25, 2024 at 07:56:10PM +0000, Jean-Philippe Brucker wrote: [...] >> diff --git a/qapi/qom.json b/qapi/qom.json >> index f982850bca..901ba67634 100644 >> --- a/qapi/qom.json >> +++ b/qapi/qom.json >> @@ -1068,6 +1068,20 @@ >> 'data': { '*cpu-affinity': ['uint16'], >> '*node-affinity': ['uint16'] } } >> >> +## >> +# @RmeGuestMeasurementAlgorithm: >> +# >> +# @sha256: Use the SHA256 algorithm >> +# >> +# @sha512: Use the SHA512 algorithm >> +# >> +# Algorithm to use for realm measurements >> +# >> +# Since: 9.3 >> +## >> +{ 'enum': 'RmeGuestMeasurementAlgorithm', >> + 'data': ['sha256', 'sha512'] } > > > A design question for Markus.... > > > We have a "QCryptoHashAlg" enum that covers both of these strings > and more besides. > > We have a choice of using a dedicated enum strictly limited to > just what RME needs, vs using the common enum type, but rejecting > unsupported algorithms at runtime. Do we prefer duplication for > improve specificity, or removal of duplication ? With a dedicated enum, introspection shows precisely the accepted values. If we reuse a wider enum, introspection shows additional, invalid values. Say we later make one of these valid here. If we reuse the wider enum now, nothing changes in introspection then, i.e. introspection cannot tell us whether this particular QEMU supports this additional algorithm. With a dedicated enum, it can. Whether that's needed in practice I find hard to predict. I lean towards lean towards dedicated enum. Questions? [...]
On Tue, Nov 26, 2024 at 04:11:19PM +0100, Markus Armbruster wrote: > Daniel P. Berrangé <berrange@redhat.com> writes: > > > On Mon, Nov 25, 2024 at 07:56:10PM +0000, Jean-Philippe Brucker wrote: > > [...] > > >> diff --git a/qapi/qom.json b/qapi/qom.json > >> index f982850bca..901ba67634 100644 > >> --- a/qapi/qom.json > >> +++ b/qapi/qom.json > >> @@ -1068,6 +1068,20 @@ > >> 'data': { '*cpu-affinity': ['uint16'], > >> '*node-affinity': ['uint16'] } } > >> > >> +## > >> +# @RmeGuestMeasurementAlgorithm: > >> +# > >> +# @sha256: Use the SHA256 algorithm > >> +# > >> +# @sha512: Use the SHA512 algorithm > >> +# > >> +# Algorithm to use for realm measurements > >> +# > >> +# Since: 9.3 > >> +## > >> +{ 'enum': 'RmeGuestMeasurementAlgorithm', > >> + 'data': ['sha256', 'sha512'] } > > > > > > A design question for Markus.... > > > > > > We have a "QCryptoHashAlg" enum that covers both of these strings > > and more besides. > > > > We have a choice of using a dedicated enum strictly limited to > > just what RME needs, vs using the common enum type, but rejecting > > unsupported algorithms at runtime. Do we prefer duplication for > > improve specificity, or removal of duplication ? > > With a dedicated enum, introspection shows precisely the accepted > values. > > If we reuse a wider enum, introspection shows additional, invalid > values. > > Say we later make one of these valid here. If we reuse the wider enum > now, nothing changes in introspection then, i.e. introspection cannot > tell us whether this particular QEMU supports this additional algorithm. > With a dedicated enum, it can. Whether that's needed in practice I find > hard to predict. > > I lean towards lean towards dedicated enum. > > Questions? That's fine with me. Lets stick with the approach in this patch. With regards, Daniel
diff --git a/qapi/qom.json b/qapi/qom.json index f982850bca..901ba67634 100644 --- a/qapi/qom.json +++ b/qapi/qom.json @@ -1068,6 +1068,20 @@ 'data': { '*cpu-affinity': ['uint16'], '*node-affinity': ['uint16'] } } +## +# @RmeGuestMeasurementAlgorithm: +# +# @sha256: Use the SHA256 algorithm +# +# @sha512: Use the SHA512 algorithm +# +# Algorithm to use for realm measurements +# +# Since: 9.3 +## +{ 'enum': 'RmeGuestMeasurementAlgorithm', + 'data': ['sha256', 'sha512'] } + ## # @RmeGuestProperties: # @@ -1077,10 +1091,14 @@ # hex string. This optional parameter allows to uniquely identify # the VM instance during attestation. (default: 0) # +# @measurement-algorithm: Realm measurement algorithm +# (default: sha512) +# # Since: 9.3 ## { 'struct': 'RmeGuestProperties', - 'data': { '*personalization-value': 'str' } } + 'data': { '*personalization-value': 'str', + '*measurement-algorithm': 'RmeGuestMeasurementAlgorithm' } } ## # @ObjectType: diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c index 0be55867ee..bf0bcf9a38 100644 --- a/target/arm/kvm-rme.c +++ b/target/arm/kvm-rme.c @@ -23,7 +23,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(RmeGuest, RME_GUEST) #define RME_PAGE_SIZE qemu_real_host_page_size() -#define RME_MAX_CFG 1 +#define RME_MAX_CFG 2 struct RmeGuest { ConfidentialGuestSupport parent_obj; @@ -31,6 +31,7 @@ struct RmeGuest { GSList *ram_regions; uint8_t *personalization_value; + RmeGuestMeasurementAlgorithm measurement_algo; hwaddr ram_base; size_t ram_size; @@ -63,6 +64,19 @@ static int rme_configure_one(RmeGuest *guest, uint32_t cfg, Error **errp) memcpy(args.rpv, guest->personalization_value, KVM_CAP_ARM_RME_RPV_SIZE); cfg_str = "personalization value"; break; + case KVM_CAP_ARM_RME_CFG_HASH_ALGO: + switch (guest->measurement_algo) { + case RME_GUEST_MEASUREMENT_ALGORITHM_SHA256: + args.hash_algo = KVM_CAP_ARM_RME_MEASUREMENT_ALGO_SHA256; + break; + case RME_GUEST_MEASUREMENT_ALGORITHM_SHA512: + args.hash_algo = KVM_CAP_ARM_RME_MEASUREMENT_ALGO_SHA512; + break; + default: + g_assert_not_reached(); + } + cfg_str = "hash algorithm"; + break; default: g_assert_not_reached(); } @@ -275,12 +289,34 @@ static void rme_set_rpv(Object *obj, const char *value, Error **errp) } } +static int rme_get_measurement_algo(Object *obj, Error **errp) +{ + RmeGuest *guest = RME_GUEST(obj); + + return guest->measurement_algo; +} + +static void rme_set_measurement_algo(Object *obj, int algo, Error **errp) +{ + RmeGuest *guest = RME_GUEST(obj); + + guest->measurement_algo = algo; +} + static void rme_guest_class_init(ObjectClass *oc, void *data) { object_class_property_add_str(oc, "personalization-value", rme_get_rpv, rme_set_rpv); object_class_property_set_description(oc, "personalization-value", "Realm personalization value (512-bit hexadecimal number)"); + + object_class_property_add_enum(oc, "measurement-algorithm", + "RmeGuestMeasurementAlgorithm", + &RmeGuestMeasurementAlgorithm_lookup, + rme_get_measurement_algo, + rme_set_measurement_algo); + object_class_property_set_description(oc, "measurement-algorithm", + "Realm measurement algorithm ('sha256', 'sha512')"); } static void rme_guest_init(Object *obj) @@ -290,6 +326,7 @@ static void rme_guest_init(Object *obj) exit(1); } rme_guest = RME_GUEST(obj); + rme_guest->measurement_algo = RME_GUEST_MEASUREMENT_ALGORITHM_SHA512; } static void rme_guest_finalize(Object *obj)