Re: [PATCH v3 11/26] target/arm/kvm-rme: Add measurement algorithm property

["Daniel P. Berrangé" <berrange@redhat.com>]

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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|