* Re: [PATCH v2 1/2] sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux boot
2021-06-21 19:05 ` [PATCH v2 1/2] sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux boot Dov Murik
@ 2021-06-21 20:32 ` Philippe Mathieu-Daudé
2021-06-22 9:44 ` Dov Murik
2021-06-22 8:28 ` Dov Murik
2021-06-22 21:15 ` Connor Kuehl
2 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-21 20:32 UTC (permalink / raw)
To: Dov Murik, qemu-devel
Cc: Tom Lendacky, Ashish Kalra, Brijesh Singh, Eduardo Habkost,
Michael S. Tsirkin, Connor Kuehl, James Bottomley,
Richard Henderson, Dr. David Alan Gilbert,
Tobin Feldman-Fitzthum, Jim Cadden, Paolo Bonzini, Laszlo Ersek
Hi Dov,
Minor comments inlined.
On 6/21/21 9:05 PM, Dov Murik wrote:
> Add the sev_add_kernel_loader_hashes function to calculate the hashes of
> the kernel/initrd/cmdline and fill a designated OVMF encrypted hash
> table area. For this to work, OVMF must support an encrypted area to
> place the data which is advertised via a special GUID in the OVMF reset
> table.
>
> The hashes of each of the files is calculated (or the string in the case
> of the cmdline with trailing '\0' included). Each entry in the hashes
> table is GUID identified and since they're passed through the
> sev_encrypt_flash interface, the hashes will be accumulated by the PSP
> measurement (SEV_LAUNCH_MEASURE).
>
> Co-developed-by: James Bottomley <jejb@linux.ibm.com>
> Signed-off-by: James Bottomley <jejb@linux.ibm.com>
> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
> ---
> target/i386/sev-stub.c | 5 ++
> target/i386/sev.c | 121 +++++++++++++++++++++++++++++++++++++++++
> target/i386/sev_i386.h | 12 ++++
> 3 files changed, 138 insertions(+)
>
> diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
> index 0227cb5177..2b5e42d644 100644
> --- a/target/i386/sev-stub.c
> +++ b/target/i386/sev-stub.c
> @@ -81,3 +81,8 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
> error_setg(errp, "SEV is not available in this QEMU");
> return NULL;
> }
> +
> +bool sev_add_kernel_loader_hashes(KernelLoaderContext *ctx, Error **errp)
> +{
> + return false;
Can't happen, so:
g_assert_not_reached();
> +}
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 83df8c09f6..8e3f601bb6 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -23,6 +23,7 @@
> #include "qemu/base64.h"
> #include "qemu/module.h"
> #include "qemu/uuid.h"
> +#include "crypto/hash.h"
> #include "sysemu/kvm.h"
> #include "sev_i386.h"
> #include "sysemu/sysemu.h"
> @@ -83,6 +84,29 @@ typedef struct __attribute__((__packed__)) SevInfoBlock {
> uint32_t reset_addr;
> } SevInfoBlock;
>
> +#define SEV_HASH_TABLE_RV_GUID "7255371f-3a3b-4b04-927b-1da6efa8d454"
> +typedef struct __attribute__((__packed__))
The codebase used to use QEMU_PACKED (see "qemu/compiler.h" but
apparently it isn't enforced.
SevHashTableDescriptor {
> + /* SEV hash table area guest address */
> + uint32_t base;
> + /* SEV hash table area size (in bytes) */
> + uint32_t size;
> +} SevHashTableDescriptor;
> +
> +/* hard code sha256 digest size */
> +#define HASH_SIZE 32
> +
> +typedef struct __attribute__((__packed__)) SevHashTableEntry {
> + uint8_t guid[16];
What about using QemuUUID?
> + uint16_t len;
> + uint8_t hash[HASH_SIZE];
> +} SevHashTableEntry;
> +
> +typedef struct __attribute__((__packed__)) SevHashTable {
> + uint8_t guid[16];
> + uint16_t len;
> + SevHashTableEntry entries[];
> +} SevHashTable;
> +
> static SevGuestState *sev_guest;
> static Error *sev_mig_blocker;
>
> @@ -1077,6 +1101,103 @@ int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size)
> return 0;
> }
>
> +static const uint8_t sev_hash_table_header_guid[] =
> + UUID_LE(0x9438d606, 0x4f22, 0x4cc9, 0xb4, 0x79, 0xa7, 0x93,
> + 0xd4, 0x11, 0xfd, 0x21);
Personally I'd have used:
static const QemuUUID sev_hash_table_header_guid = {
.data = UUID_LE(...);
};
and added qemu_uuid_copy() to complete the API, but that's fine.
> +
> +static const uint8_t sev_kernel_entry_guid[] =
> + UUID_LE(0x4de79437, 0xabd2, 0x427f, 0xb8, 0x35, 0xd5, 0xb1,
> + 0x72, 0xd2, 0x04, 0x5b);
> +static const uint8_t sev_initrd_entry_guid[] =
> + UUID_LE(0x44baf731, 0x3a2f, 0x4bd7, 0x9a, 0xf1, 0x41, 0xe2,
> + 0x91, 0x69, 0x78, 0x1d);
> +static const uint8_t sev_cmdline_entry_guid[] =
> + UUID_LE(0x97d02dd8, 0xbd20, 0x4c94, 0xaa, 0x78, 0xe7, 0x71,
> + 0x4d, 0x36, 0xab, 0x2a);
> +
> +static void fill_sev_hash_table_entry(SevHashTableEntry *e, const uint8_t *guid,
> + const uint8_t *hash, size_t hash_len)
> +{
> + memcpy(e->guid, guid, sizeof(e->guid));
> + e->len = sizeof(*e);
> + memcpy(e->hash, hash, hash_len);
> +}
> +
> +/*
> + * Add the hashes of the linux kernel/initrd/cmdline to an encrypted guest page
> + * which is included in SEV's initial memory measurement.
> + */
> +bool sev_add_kernel_loader_hashes(KernelLoaderContext *ctx, Error **errp)
> +{
> + uint8_t *data;
> + SevHashTableDescriptor *area;
> + SevHashTable *ht;
> + SevHashTableEntry *e;
> + uint8_t hash_buf[HASH_SIZE];
> + uint8_t *hash = hash_buf;
> + size_t hash_len = sizeof(hash_buf);
> + int ht_index = 0;
> + int aligned_len;
> +
> + if (!pc_system_ovmf_table_find(SEV_HASH_TABLE_RV_GUID, &data, NULL)) {
If we never use the data_len argument, can we simplify the prototype?
> + error_setg(errp, "SEV: kernel specified but OVMF has no hash table guid");
> + return false;
> + }
> + area = (SevHashTableDescriptor *)data;
> +
> + ht = qemu_map_ram_ptr(NULL, area->base);
> +
> + /* Populate the hashes table header */
> + memcpy(ht->guid, sev_hash_table_header_guid, sizeof(ht->guid));
> + ht->len = sizeof(*ht);
> +
> + /* Calculate hash of kernel command-line */
> + if (qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA256, ctx->cmdline_data,
> + ctx->cmdline_size,
> + &hash, &hash_len, errp) < 0) {
> + return false;
> + }
Maybe move the qcrypto_hash_bytes() call before filling ht?
> + e = &ht->entries[ht_index++];
> + fill_sev_hash_table_entry(e, sev_cmdline_entry_guid, hash, hash_len);
> +
> + /* Calculate hash of initrd */
> + if (ctx->initrd_data) {
> + if (qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA256, ctx->initrd_data,
> + ctx->initrd_size, &hash, &hash_len, errp) < 0) {
> + return false;
> + }
Ah, now I see the pattern. Hmm OK then.
> + e = &ht->entries[ht_index++];
> + fill_sev_hash_table_entry(e, sev_initrd_entry_guid, hash, hash_len);
> + }
> +
> + /* Calculate hash of the kernel */
> + struct iovec iov[2] = {
> + { .iov_base = ctx->setup_data, .iov_len = ctx->setup_size },
> + { .iov_base = ctx->kernel_data, .iov_len = ctx->kernel_size }
> + };
> + if (qcrypto_hash_bytesv(QCRYPTO_HASH_ALG_SHA256, iov, 2,
> + &hash, &hash_len, errp) < 0) {
> + return false;
> + }
> + e = &ht->entries[ht_index++];
> + fill_sev_hash_table_entry(e, sev_kernel_entry_guid, hash, hash_len);
> +
> + /* now we have all the possible entries, finalize the hashes table */
> + ht->len += ht_index * sizeof(*e);
> + /* SEV len has to be 16 byte aligned */
> + aligned_len = ROUND_UP(ht->len, 16);
> + if (aligned_len != ht->len) {
> + /* zero the excess data so the measurement can be reliably calculated */
> + memset(&ht->entries[ht_index], 0, aligned_len - ht->len);
> + }
> +
> + if (sev_encrypt_flash((uint8_t *)ht, aligned_len, errp) < 0) {
> + return false;
> + }
> +
> + return true;
> +}
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 1/2] sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux boot
2021-06-21 20:32 ` Philippe Mathieu-Daudé
@ 2021-06-22 9:44 ` Dov Murik
2021-06-22 9:49 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 15+ messages in thread
From: Dov Murik @ 2021-06-22 9:44 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Tom Lendacky, Ashish Kalra, Brijesh Singh, Eduardo Habkost,
Michael S. Tsirkin, Connor Kuehl, James Bottomley,
Richard Henderson, Dr. David Alan Gilbert, dovmurik,
Tobin Feldman-Fitzthum, Jim Cadden, Paolo Bonzini, Laszlo Ersek
Hi Philippe,
On 21/06/2021 23:32, Philippe Mathieu-Daudé wrote:
> Hi Dov,
>
> Minor comments inlined.
>
> On 6/21/21 9:05 PM, Dov Murik wrote:
>> Add the sev_add_kernel_loader_hashes function to calculate the hashes of
>> the kernel/initrd/cmdline and fill a designated OVMF encrypted hash
>> table area. For this to work, OVMF must support an encrypted area to
>> place the data which is advertised via a special GUID in the OVMF reset
>> table.
>>
>> The hashes of each of the files is calculated (or the string in the case
>> of the cmdline with trailing '\0' included). Each entry in the hashes
>> table is GUID identified and since they're passed through the
>> sev_encrypt_flash interface, the hashes will be accumulated by the PSP
>> measurement (SEV_LAUNCH_MEASURE).
>>
>> Co-developed-by: James Bottomley <jejb@linux.ibm.com>
>> Signed-off-by: James Bottomley <jejb@linux.ibm.com>
>> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
>> ---
>> target/i386/sev-stub.c | 5 ++
>> target/i386/sev.c | 121 +++++++++++++++++++++++++++++++++++++++++
>> target/i386/sev_i386.h | 12 ++++
>> 3 files changed, 138 insertions(+)
>>
>> diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
>> index 0227cb5177..2b5e42d644 100644
>> --- a/target/i386/sev-stub.c
>> +++ b/target/i386/sev-stub.c
>> @@ -81,3 +81,8 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
>> error_setg(errp, "SEV is not available in this QEMU");
>> return NULL;
>> }
>> +
>> +bool sev_add_kernel_loader_hashes(KernelLoaderContext *ctx, Error **errp)
>> +{
>> + return false;
>
> Can't happen, so:
>
> g_assert_not_reached();
>
OK, I'll use it.
I guess the comment is relevant to other functions in that file as well
(e.g., sev_encrypt_flash), but I'll leave that to your SEV-housekeeping
series.
>> +}
>> diff --git a/target/i386/sev.c b/target/i386/sev.c
>> index 83df8c09f6..8e3f601bb6 100644
>> --- a/target/i386/sev.c
>> +++ b/target/i386/sev.c
>> @@ -23,6 +23,7 @@
>> #include "qemu/base64.h"
>> #include "qemu/module.h"
>> #include "qemu/uuid.h"
>> +#include "crypto/hash.h"
>> #include "sysemu/kvm.h"
>> #include "sev_i386.h"
>> #include "sysemu/sysemu.h"
>> @@ -83,6 +84,29 @@ typedef struct __attribute__((__packed__)) SevInfoBlock {
>> uint32_t reset_addr;
>> } SevInfoBlock;
>>
>> +#define SEV_HASH_TABLE_RV_GUID "7255371f-3a3b-4b04-927b-1da6efa8d454"
>> +typedef struct __attribute__((__packed__))
>
> The codebase used to use QEMU_PACKED (see "qemu/compiler.h" but
> apparently it isn't enforced.
>
I can use it.
> SevHashTableDescriptor {
>> + /* SEV hash table area guest address */
>> + uint32_t base;
>> + /* SEV hash table area size (in bytes) */
>> + uint32_t size;
>> +} SevHashTableDescriptor;
>> +
>> +/* hard code sha256 digest size */
>> +#define HASH_SIZE 32
>> +
>> +typedef struct __attribute__((__packed__)) SevHashTableEntry {
>> + uint8_t guid[16];
>
> What about using QemuUUID?
>
I agree. I'll use it, coupled with your .data init below.
>> + uint16_t len;
>> + uint8_t hash[HASH_SIZE];
>> +} SevHashTableEntry;
>> +
>> +typedef struct __attribute__((__packed__)) SevHashTable {
>> + uint8_t guid[16];
>> + uint16_t len;
>> + SevHashTableEntry entries[];
>> +} SevHashTable;
>> +
>> static SevGuestState *sev_guest;
>> static Error *sev_mig_blocker;
>>
>> @@ -1077,6 +1101,103 @@ int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size)
>> return 0;
>> }
>>
>> +static const uint8_t sev_hash_table_header_guid[] =
>> + UUID_LE(0x9438d606, 0x4f22, 0x4cc9, 0xb4, 0x79, 0xa7, 0x93,
>> + 0xd4, 0x11, 0xfd, 0x21);
>
> Personally I'd have used:
>
> static const QemuUUID sev_hash_table_header_guid = {
> .data = UUID_LE(...);
> };
Yes, I'll use this.
>
> and added qemu_uuid_copy() to complete the API, but that's fine.
I think simple C assignment works for structs (and hence QemuUUID),
something like:
SevHashTable *ht = ...;
ht->guid = sev_hash_table_header_guid;
(where both ht->guid and sev_hash_table_header_guid are QemuUUID.)
>
>> +
>> +static const uint8_t sev_kernel_entry_guid[] =
>> + UUID_LE(0x4de79437, 0xabd2, 0x427f, 0xb8, 0x35, 0xd5, 0xb1,
>> + 0x72, 0xd2, 0x04, 0x5b);
>> +static const uint8_t sev_initrd_entry_guid[] =
>> + UUID_LE(0x44baf731, 0x3a2f, 0x4bd7, 0x9a, 0xf1, 0x41, 0xe2,
>> + 0x91, 0x69, 0x78, 0x1d);
>> +static const uint8_t sev_cmdline_entry_guid[] =
>> + UUID_LE(0x97d02dd8, 0xbd20, 0x4c94, 0xaa, 0x78, 0xe7, 0x71,
>> + 0x4d, 0x36, 0xab, 0x2a);
>> +
>> +static void fill_sev_hash_table_entry(SevHashTableEntry *e, const uint8_t *guid,
>> + const uint8_t *hash, size_t hash_len)
>> +{
>> + memcpy(e->guid, guid, sizeof(e->guid));
>> + e->len = sizeof(*e);
>> + memcpy(e->hash, hash, hash_len);
>> +}
>> +
>> +/*
>> + * Add the hashes of the linux kernel/initrd/cmdline to an encrypted guest page
>> + * which is included in SEV's initial memory measurement.
>> + */
>> +bool sev_add_kernel_loader_hashes(KernelLoaderContext *ctx, Error **errp)
>> +{
>> + uint8_t *data;
>> + SevHashTableDescriptor *area;
>> + SevHashTable *ht;
>> + SevHashTableEntry *e;
>> + uint8_t hash_buf[HASH_SIZE];
>> + uint8_t *hash = hash_buf;
>> + size_t hash_len = sizeof(hash_buf);
>> + int ht_index = 0;
>> + int aligned_len;
>> +
>> + if (!pc_system_ovmf_table_find(SEV_HASH_TABLE_RV_GUID, &data, NULL)) {
>
> If we never use the data_len argument, can we simplify the prototype?
The current uses for the OVMF reset vector GUIDed table is for simple
structs with known length (secret injection page address, SEV-ES reset
address, SEV table of hashes address). But keeping the length there
allows adding variable-sized entries such as strings/blobs.
>
>> + error_setg(errp, "SEV: kernel specified but OVMF has no hash table guid");
>> + return false;
>> + }
>> + area = (SevHashTableDescriptor *)data;
>> +
>> + ht = qemu_map_ram_ptr(NULL, area->base);
>> +
>> + /* Populate the hashes table header */
>> + memcpy(ht->guid, sev_hash_table_header_guid, sizeof(ht->guid));
>> + ht->len = sizeof(*ht);
>> +
>> + /* Calculate hash of kernel command-line */
>> + if (qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA256, ctx->cmdline_data,
>> + ctx->cmdline_size,
>> + &hash, &hash_len, errp) < 0) {
>> + return false;
>> + }
>
> Maybe move the qcrypto_hash_bytes() call before filling ht?
(below)
>
>> + e = &ht->entries[ht_index++];
>> + fill_sev_hash_table_entry(e, sev_cmdline_entry_guid, hash, hash_len);
>> +
>> + /* Calculate hash of initrd */
>> + if (ctx->initrd_data) {
>> + if (qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA256, ctx->initrd_data,
>> + ctx->initrd_size, &hash, &hash_len, errp) < 0) {
>> + return false;
>> + }
>
> Ah, now I see the pattern. Hmm OK then.
>
But this might change if initrd_hash is no longer optional (see separate
self-reply to this patch). In such a case I'll probably first calculate
all the three hashes, and then fill in the SevHashTable struct.
-Dov
>> + e = &ht->entries[ht_index++];
>> + fill_sev_hash_table_entry(e, sev_initrd_entry_guid, hash, hash_len);
>> + }
>> +
>> + /* Calculate hash of the kernel */
>> + struct iovec iov[2] = {
>> + { .iov_base = ctx->setup_data, .iov_len = ctx->setup_size },
>> + { .iov_base = ctx->kernel_data, .iov_len = ctx->kernel_size }
>> + };
>> + if (qcrypto_hash_bytesv(QCRYPTO_HASH_ALG_SHA256, iov, 2,
>> + &hash, &hash_len, errp) < 0) {
>> + return false;
>> + }
>> + e = &ht->entries[ht_index++];
>> + fill_sev_hash_table_entry(e, sev_kernel_entry_guid, hash, hash_len);
>> +
>> + /* now we have all the possible entries, finalize the hashes table */
>> + ht->len += ht_index * sizeof(*e);
>> + /* SEV len has to be 16 byte aligned */
>> + aligned_len = ROUND_UP(ht->len, 16);
>> + if (aligned_len != ht->len) {
>> + /* zero the excess data so the measurement can be reliably calculated */
>> + memset(&ht->entries[ht_index], 0, aligned_len - ht->len);
>> + }
>> +
>> + if (sev_encrypt_flash((uint8_t *)ht, aligned_len, errp) < 0) {
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 1/2] sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux boot
2021-06-22 9:44 ` Dov Murik
@ 2021-06-22 9:49 ` Philippe Mathieu-Daudé
2021-06-22 10:26 ` Dov Murik
0 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-22 9:49 UTC (permalink / raw)
To: Dov Murik, qemu-devel
Cc: Tom Lendacky, Ashish Kalra, Brijesh Singh, Eduardo Habkost,
Michael S. Tsirkin, Connor Kuehl, James Bottomley,
Richard Henderson, Dr. David Alan Gilbert,
Tobin Feldman-Fitzthum, Jim Cadden, Paolo Bonzini, Laszlo Ersek
On 6/22/21 11:44 AM, Dov Murik wrote:
> On 21/06/2021 23:32, Philippe Mathieu-Daudé wrote:
>> and added qemu_uuid_copy() to complete the API, but that's fine.
>
> I think simple C assignment works for structs (and hence QemuUUID),
> something like:
>
> SevHashTable *ht = ...;
> ht->guid = sev_hash_table_header_guid;
>
> (where both ht->guid and sev_hash_table_header_guid are QemuUUID.)
OK.
>>> + if (!pc_system_ovmf_table_find(SEV_HASH_TABLE_RV_GUID, &data, NULL)) {
>>
>> If we never use the data_len argument, can we simplify the prototype?
>
> The current uses for the OVMF reset vector GUIDed table is for simple
> structs with known length (secret injection page address, SEV-ES reset
> address, SEV table of hashes address). But keeping the length there
> allows adding variable-sized entries such as strings/blobs.
OK. Good opportunity to document the prototype declaration ;)
>
>>
>>> + error_setg(errp, "SEV: kernel specified but OVMF has no hash table guid");
>>> + return false;
>>> + }
>>> + area = (SevHashTableDescriptor *)data;
>>> +
>>> + ht = qemu_map_ram_ptr(NULL, area->base);
>>> +
>>> + /* Populate the hashes table header */
>>> + memcpy(ht->guid, sev_hash_table_header_guid, sizeof(ht->guid));
>>> + ht->len = sizeof(*ht);
>>> +
>>> + /* Calculate hash of kernel command-line */
>>> + if (qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA256, ctx->cmdline_data,
>>> + ctx->cmdline_size,
>>> + &hash, &hash_len, errp) < 0) {
>>> + return false;
>>> + }
>>
>> Maybe move the qcrypto_hash_bytes() call before filling ht?
>
> (below)
>
>>
>>> + e = &ht->entries[ht_index++];
>>> + fill_sev_hash_table_entry(e, sev_cmdline_entry_guid, hash, hash_len);
>>> +
>>> + /* Calculate hash of initrd */
>>> + if (ctx->initrd_data) {
>>> + if (qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA256, ctx->initrd_data,
>>> + ctx->initrd_size, &hash, &hash_len, errp) < 0) {
>>> + return false;
>>> + }
>>
>> Ah, now I see the pattern. Hmm OK then.
>>
>
> But this might change if initrd_hash is no longer optional (see separate
> self-reply to this patch). In such a case I'll probably first calculate
> all the three hashes, and then fill in the SevHashTable struct.
Yes, sounds simpler.
Regards,
Phil.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 1/2] sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux boot
2021-06-22 9:49 ` Philippe Mathieu-Daudé
@ 2021-06-22 10:26 ` Dov Murik
2021-06-22 11:10 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 15+ messages in thread
From: Dov Murik @ 2021-06-22 10:26 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Tom Lendacky, Ashish Kalra, Brijesh Singh, Eduardo Habkost,
Michael S. Tsirkin, Connor Kuehl, James Bottomley,
Richard Henderson, Dr. David Alan Gilbert,
Tobin Feldman-Fitzthum, Jim Cadden, Paolo Bonzini, Laszlo Ersek
On 22/06/2021 12:49, Philippe Mathieu-Daudé wrote:
> On 6/22/21 11:44 AM, Dov Murik wrote:
>> On 21/06/2021 23:32, Philippe Mathieu-Daudé wrote:
>
...
>
>>>> + if (!pc_system_ovmf_table_find(SEV_HASH_TABLE_RV_GUID, &data, NULL)) {
>>>
>>> If we never use the data_len argument, can we simplify the prototype?
>>
>> The current uses for the OVMF reset vector GUIDed table is for simple
>> structs with known length (secret injection page address, SEV-ES reset
>> address, SEV table of hashes address). But keeping the length there
>> allows adding variable-sized entries such as strings/blobs.
>
> OK. Good opportunity to document the prototype declaration ;)
Yep. I'll send as a separate standalone patch.
P.S.
In a previous version you mentioned you ran into issues with a qemu
build with SEV disabled. I tried that by modifying
default-configs/devices/i386-softmmu.mak and uncommenting CONFIG_SEV=n
there. Is there a friendlier way to create such a build?
I'm currently building with:
cd build
../configure --target-list=x86_64-softmmu
make
Thanks,
-Dov
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 1/2] sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux boot
2021-06-22 10:26 ` Dov Murik
@ 2021-06-22 11:10 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-22 11:10 UTC (permalink / raw)
To: Dov Murik, qemu-devel
Cc: Tom Lendacky, Ashish Kalra, Brijesh Singh, Eduardo Habkost,
Michael S. Tsirkin, Connor Kuehl, James Bottomley,
Richard Henderson, Dr. David Alan Gilbert,
Tobin Feldman-Fitzthum, Jim Cadden, Paolo Bonzini, Laszlo Ersek
On 6/22/21 12:26 PM, Dov Murik wrote:
>
>
> On 22/06/2021 12:49, Philippe Mathieu-Daudé wrote:
>> On 6/22/21 11:44 AM, Dov Murik wrote:
>>> On 21/06/2021 23:32, Philippe Mathieu-Daudé wrote:
>>
>
> ...
>
>>
>>>>> + if (!pc_system_ovmf_table_find(SEV_HASH_TABLE_RV_GUID, &data, NULL)) {
>>>>
>>>> If we never use the data_len argument, can we simplify the prototype?
>>>
>>> The current uses for the OVMF reset vector GUIDed table is for simple
>>> structs with known length (secret injection page address, SEV-ES reset
>>> address, SEV table of hashes address). But keeping the length there
>>> allows adding variable-sized entries such as strings/blobs.
>>
>> OK. Good opportunity to document the prototype declaration ;)
>
> Yep. I'll send as a separate standalone patch.
Sure.
> P.S.
> In a previous version you mentioned you ran into issues with a qemu
> build with SEV disabled. I tried that by modifying
> default-configs/devices/i386-softmmu.mak and uncommenting CONFIG_SEV=n
> there. Is there a friendlier way to create such a build?
Unfortunately not yet in mainstream (distributions tune their builds,
often disabling all recent features, and enable them on a case by case
basis once it is well tested).
Thankfully Alex posted a series to add the possibility:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg817710.html
>
> I'm currently building with:
>
> cd build
> ../configure --target-list=x86_64-softmmu
> make
>
>
> Thanks,
> -Dov
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux boot
2021-06-21 19:05 ` [PATCH v2 1/2] sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux boot Dov Murik
2021-06-21 20:32 ` Philippe Mathieu-Daudé
@ 2021-06-22 8:28 ` Dov Murik
2021-06-22 21:15 ` Connor Kuehl
2 siblings, 0 replies; 15+ messages in thread
From: Dov Murik @ 2021-06-22 8:28 UTC (permalink / raw)
To: qemu-devel, James Bottomley
Cc: Tom Lendacky, Ashish Kalra, Brijesh Singh, Eduardo Habkost,
Michael S. Tsirkin, Connor Kuehl, Laszlo Ersek,
Richard Henderson, Dr. David Alan Gilbert,
Tobin Feldman-Fitzthum, Jim Cadden, Paolo Bonzini,
Philippe Mathieu-Daudé
I found an issue with this patch when no -initrd is passed, see below.
On 21/06/2021 22:05, Dov Murik wrote:
> Add the sev_add_kernel_loader_hashes function to calculate the hashes of
> the kernel/initrd/cmdline and fill a designated OVMF encrypted hash
> table area. For this to work, OVMF must support an encrypted area to
> place the data which is advertised via a special GUID in the OVMF reset
> table.
>
> The hashes of each of the files is calculated (or the string in the case
> of the cmdline with trailing '\0' included). Each entry in the hashes
> table is GUID identified and since they're passed through the
> sev_encrypt_flash interface, the hashes will be accumulated by the PSP
> measurement (SEV_LAUNCH_MEASURE).
>
> Co-developed-by: James Bottomley <jejb@linux.ibm.com>
> Signed-off-by: James Bottomley <jejb@linux.ibm.com>
> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
> ---
> target/i386/sev-stub.c | 5 ++
> target/i386/sev.c | 121 +++++++++++++++++++++++++++++++++++++++++
> target/i386/sev_i386.h | 12 ++++
> 3 files changed, 138 insertions(+)
>
> diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
> index 0227cb5177..2b5e42d644 100644
> --- a/target/i386/sev-stub.c
> +++ b/target/i386/sev-stub.c
> @@ -81,3 +81,8 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
> error_setg(errp, "SEV is not available in this QEMU");
> return NULL;
> }
> +
> +bool sev_add_kernel_loader_hashes(KernelLoaderContext *ctx, Error **errp)
> +{
> + return false;
> +}
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 83df8c09f6..8e3f601bb6 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -23,6 +23,7 @@
> #include "qemu/base64.h"
> #include "qemu/module.h"
> #include "qemu/uuid.h"
> +#include "crypto/hash.h"
> #include "sysemu/kvm.h"
> #include "sev_i386.h"
> #include "sysemu/sysemu.h"
> @@ -83,6 +84,29 @@ typedef struct __attribute__((__packed__)) SevInfoBlock {
> uint32_t reset_addr;
> } SevInfoBlock;
>
> +#define SEV_HASH_TABLE_RV_GUID "7255371f-3a3b-4b04-927b-1da6efa8d454"
> +typedef struct __attribute__((__packed__)) SevHashTableDescriptor {
> + /* SEV hash table area guest address */
> + uint32_t base;
> + /* SEV hash table area size (in bytes) */
> + uint32_t size;
> +} SevHashTableDescriptor;
> +
> +/* hard code sha256 digest size */
> +#define HASH_SIZE 32
> +
> +typedef struct __attribute__((__packed__)) SevHashTableEntry {
> + uint8_t guid[16];
> + uint16_t len;
> + uint8_t hash[HASH_SIZE];
> +} SevHashTableEntry;
> +
> +typedef struct __attribute__((__packed__)) SevHashTable {
> + uint8_t guid[16];
> + uint16_t len;
> + SevHashTableEntry entries[];
> +} SevHashTable;
> +
> static SevGuestState *sev_guest;
> static Error *sev_mig_blocker;
>
> @@ -1077,6 +1101,103 @@ int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size)
> return 0;
> }
>
> +static const uint8_t sev_hash_table_header_guid[] =
> + UUID_LE(0x9438d606, 0x4f22, 0x4cc9, 0xb4, 0x79, 0xa7, 0x93,
> + 0xd4, 0x11, 0xfd, 0x21);
> +
> +static const uint8_t sev_kernel_entry_guid[] =
> + UUID_LE(0x4de79437, 0xabd2, 0x427f, 0xb8, 0x35, 0xd5, 0xb1,
> + 0x72, 0xd2, 0x04, 0x5b);
> +static const uint8_t sev_initrd_entry_guid[] =
> + UUID_LE(0x44baf731, 0x3a2f, 0x4bd7, 0x9a, 0xf1, 0x41, 0xe2,
> + 0x91, 0x69, 0x78, 0x1d);
> +static const uint8_t sev_cmdline_entry_guid[] =
> + UUID_LE(0x97d02dd8, 0xbd20, 0x4c94, 0xaa, 0x78, 0xe7, 0x71,
> + 0x4d, 0x36, 0xab, 0x2a);
> +
> +static void fill_sev_hash_table_entry(SevHashTableEntry *e, const uint8_t *guid,
> + const uint8_t *hash, size_t hash_len)
> +{
> + memcpy(e->guid, guid, sizeof(e->guid));
> + e->len = sizeof(*e);
> + memcpy(e->hash, hash, hash_len);
> +}
> +
> +/*
> + * Add the hashes of the linux kernel/initrd/cmdline to an encrypted guest page
> + * which is included in SEV's initial memory measurement.
> + */
> +bool sev_add_kernel_loader_hashes(KernelLoaderContext *ctx, Error **errp)
> +{
> + uint8_t *data;
> + SevHashTableDescriptor *area;
> + SevHashTable *ht;
> + SevHashTableEntry *e;
> + uint8_t hash_buf[HASH_SIZE];
> + uint8_t *hash = hash_buf;
> + size_t hash_len = sizeof(hash_buf);
> + int ht_index = 0;
> + int aligned_len;
> +
> + if (!pc_system_ovmf_table_find(SEV_HASH_TABLE_RV_GUID, &data, NULL)) {
> + error_setg(errp, "SEV: kernel specified but OVMF has no hash table guid");
> + return false;
> + }
> + area = (SevHashTableDescriptor *)data;
> +
> + ht = qemu_map_ram_ptr(NULL, area->base);
> +
> + /* Populate the hashes table header */
> + memcpy(ht->guid, sev_hash_table_header_guid, sizeof(ht->guid));
> + ht->len = sizeof(*ht);
> +
> + /* Calculate hash of kernel command-line */
> + if (qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA256, ctx->cmdline_data,
> + ctx->cmdline_size,
> + &hash, &hash_len, errp) < 0) {
> + return false;
> + }
> + e = &ht->entries[ht_index++];
> + fill_sev_hash_table_entry(e, sev_cmdline_entry_guid, hash, hash_len);
> +
> + /* Calculate hash of initrd */
> + if (ctx->initrd_data) {
> + if (qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA256, ctx->initrd_data,
> + ctx->initrd_size, &hash, &hash_len, errp) < 0) {
> + return false;
> + }
> + e = &ht->entries[ht_index++];
> + fill_sev_hash_table_entry(e, sev_initrd_entry_guid, hash, hash_len);
> + }
As can be seen, we conditionally add the hash table entry for initrd,
depending on whether the user passed the -initrd switch to QEMU.
However, in the OVMF side, initrd hash is *always* verified; in this
case it will be verified against a the hash of an empty buffer (sha256
starts with e3b0c44298fc...). Since QEMU (this patch) doesn't add that
entry in the hashes table, we get an an access denied error in OVMF.
I think OVMF is doing the correct thing: the Guest Owner wishes to have
no initrd, and OVMF should verify that the initrd it reads is indeed
empty. Therefore the measured SEV hashes table should include an entry
for initrd with the sha256 of an empty buffer.
(James - do you agree?)
I think we should remove this condition, and always fill the three
entries in the hashes table: cmdline, initrd, and kernel. Here's the
treatment for missing arguments:
* No -cmdline: use the hash of the 1-byte "\0" string (this is already
the case, no code changes needed).
* No -initrd: use the hash of the empty buffer.
* No -kernel: this code will not be reached at all (this is already the
case, no code changes needed).
This leads to another possible modification: instead of array of
SevHashTableEntry entries[] in SevHashTable, we can have:
typedef struct __attribute__((__packed__)) SevHashTable {
uint8_t guid[16];
uint16_t len;
SevHashTableEntry cmdline;
SevHashTableEntry initrd;
SevHashTableEntry kernel;
uint8_t zero_padding[]; /* padded to 16-byte boundary */
} SevHashTable;
This could clear up things here; keeping the GUID+length in each entry
leaves the future option for a more modular table.
It will also make the order of entries clear, which is required for
calculating the expected measurement on the Guest Owner side.
This also means that the total length of the entire table is known in
advance (168 bytes; aligned to 176 bytes with zero padding). Not sure
if this helps.
-Dov
> +
> + /* Calculate hash of the kernel */
> + struct iovec iov[2] = {
> + { .iov_base = ctx->setup_data, .iov_len = ctx->setup_size },
> + { .iov_base = ctx->kernel_data, .iov_len = ctx->kernel_size }
> + };
> + if (qcrypto_hash_bytesv(QCRYPTO_HASH_ALG_SHA256, iov, 2,
> + &hash, &hash_len, errp) < 0) {
> + return false;
> + }
> + e = &ht->entries[ht_index++];
> + fill_sev_hash_table_entry(e, sev_kernel_entry_guid, hash, hash_len);
> +
> + /* now we have all the possible entries, finalize the hashes table */
> + ht->len += ht_index * sizeof(*e);
> + /* SEV len has to be 16 byte aligned */
> + aligned_len = ROUND_UP(ht->len, 16);
> + if (aligned_len != ht->len) {
> + /* zero the excess data so the measurement can be reliably calculated */
> + memset(&ht->entries[ht_index], 0, aligned_len - ht->len);
> + }
> +
> + if (sev_encrypt_flash((uint8_t *)ht, aligned_len, errp) < 0) {
> + return false;
> + }
> +
> + return true;
> +}
> +
> static void
> sev_register_types(void)
> {
> diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
> index ae6d840478..deb3eec409 100644
> --- a/target/i386/sev_i386.h
> +++ b/target/i386/sev_i386.h
> @@ -28,6 +28,17 @@
> #define SEV_POLICY_DOMAIN 0x10
> #define SEV_POLICY_SEV 0x20
>
> +typedef struct KernelLoaderContext {
> + char *setup_data;
> + size_t setup_size;
> + char *kernel_data;
> + size_t kernel_size;
> + char *initrd_data;
> + size_t initrd_size;
> + char *cmdline_data;
> + size_t cmdline_size;
> +} KernelLoaderContext;
> +
> extern bool sev_es_enabled(void);
> extern uint64_t sev_get_me_mask(void);
> extern SevInfo *sev_get_info(void);
> @@ -37,5 +48,6 @@ extern char *sev_get_launch_measurement(void);
> extern SevCapability *sev_get_capabilities(Error **errp);
> extern SevAttestationReport *
> sev_get_attestation_report(const char *mnonce, Error **errp);
> +extern bool sev_add_kernel_loader_hashes(KernelLoaderContext *ctx, Error **errp);
>
> #endif
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 1/2] sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux boot
2021-06-21 19:05 ` [PATCH v2 1/2] sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux boot Dov Murik
2021-06-21 20:32 ` Philippe Mathieu-Daudé
2021-06-22 8:28 ` Dov Murik
@ 2021-06-22 21:15 ` Connor Kuehl
2021-06-23 8:41 ` Dov Murik
2 siblings, 1 reply; 15+ messages in thread
From: Connor Kuehl @ 2021-06-22 21:15 UTC (permalink / raw)
To: Dov Murik, qemu-devel
Cc: Tom Lendacky, Ashish Kalra, Brijesh Singh, Eduardo Habkost,
Michael S. Tsirkin, Laszlo Ersek, James Bottomley,
Richard Henderson, Dr. David Alan Gilbert,
Tobin Feldman-Fitzthum, Jim Cadden, Paolo Bonzini,
Philippe Mathieu-Daudé
On 6/21/21 2:05 PM, Dov Murik wrote:
> +static void fill_sev_hash_table_entry(SevHashTableEntry *e, const uint8_t *guid,
> + const uint8_t *hash, size_t hash_len)
> +{
> + memcpy(e->guid, guid, sizeof(e->guid));
> + e->len = sizeof(*e);
> + memcpy(e->hash, hash, hash_len);
Should this memcpy be constrained to MIN(sizeof(e->hash), hash_len)? Or
perhaps an assert statement since I see below that this function's
caller sets this to HASH_SIZE which is currently == sizeof(e->hash).
Actually, the assert statement would be easier to debug if the input
to this function is ever unexpected, especially since it avoids an
outcome where the input is silently truncated; which is a pitfall that
that the memcpy clamping would fall into.
Connor
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 1/2] sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux boot
2021-06-22 21:15 ` Connor Kuehl
@ 2021-06-23 8:41 ` Dov Murik
2021-06-23 8:49 ` Daniel P. Berrangé
0 siblings, 1 reply; 15+ messages in thread
From: Dov Murik @ 2021-06-23 8:41 UTC (permalink / raw)
To: Connor Kuehl, qemu-devel
Cc: Tom Lendacky, Ashish Kalra, Brijesh Singh, Eduardo Habkost,
Michael S. Tsirkin, Laszlo Ersek, James Bottomley,
Richard Henderson, Dr. David Alan Gilbert,
Tobin Feldman-Fitzthum, Jim Cadden, Daniel P. Berrange,
Paolo Bonzini, Philippe Mathieu-Daudé
Hi Connor,
+cc: Daniel
On 23/06/2021 0:15, Connor Kuehl wrote:
> On 6/21/21 2:05 PM, Dov Murik wrote:
>> +static void fill_sev_hash_table_entry(SevHashTableEntry *e, const uint8_t *guid,
>> + const uint8_t *hash, size_t hash_len)
>> +{
>> + memcpy(e->guid, guid, sizeof(e->guid));
>> + e->len = sizeof(*e);
>> + memcpy(e->hash, hash, hash_len);
>
> Should this memcpy be constrained to MIN(sizeof(e->hash), hash_len)? Or
> perhaps an assert statement since I see below that this function's
> caller sets this to HASH_SIZE which is currently == sizeof(e->hash).
>
> Actually, the assert statement would be easier to debug if the input
> to this function is ever unexpected, especially since it avoids an
> outcome where the input is silently truncated; which is a pitfall that
> that the memcpy clamping would fall into.
I agree. I'll change it to:
assert(hash_len == sizeof(e->hash));
memcpy(e->hash, hash, sizeof(e->hash));
This way also the memcpy length is known at compile-time.
Related: I wondered if I could replace HASH_SIZE in:
/* hard code sha256 digest size */
#define HASH_SIZE 32
typedef struct QEMU_PACKED SevHashTableEntry {
QemuUUID guid;
uint16_t len;
uint8_t hash[HASH_SIZE];
} SevHashTableEntry;
with some SHA256-related constant from crypto/hash.h, but I only found
the qcrypto_hash_digest_len(QCRYPTO_HASH_ALG_SHA256) function which
doesn't work for setting sizes of arrays at compile-time.
Daniel: do you know what would be the proper way?
Thanks,
-Dov
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 1/2] sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux boot
2021-06-23 8:41 ` Dov Murik
@ 2021-06-23 8:49 ` Daniel P. Berrangé
2021-06-23 9:28 ` Dov Murik
0 siblings, 1 reply; 15+ messages in thread
From: Daniel P. Berrangé @ 2021-06-23 8:49 UTC (permalink / raw)
To: Dov Murik
Cc: Tom Lendacky, Ashish Kalra, Brijesh Singh, Eduardo Habkost,
Michael S. Tsirkin, Connor Kuehl, Laszlo Ersek, James Bottomley,
Richard Henderson, qemu-devel, Dr. David Alan Gilbert,
Tobin Feldman-Fitzthum, Jim Cadden, Paolo Bonzini,
Philippe Mathieu-Daudé
On Wed, Jun 23, 2021 at 11:41:56AM +0300, Dov Murik wrote:
> Hi Connor,
>
> +cc: Daniel
>
> On 23/06/2021 0:15, Connor Kuehl wrote:
> > On 6/21/21 2:05 PM, Dov Murik wrote:
> >> +static void fill_sev_hash_table_entry(SevHashTableEntry *e, const uint8_t *guid,
> >> + const uint8_t *hash, size_t hash_len)
> >> +{
> >> + memcpy(e->guid, guid, sizeof(e->guid));
> >> + e->len = sizeof(*e);
> >> + memcpy(e->hash, hash, hash_len);
> >
> > Should this memcpy be constrained to MIN(sizeof(e->hash), hash_len)? Or
> > perhaps an assert statement since I see below that this function's
> > caller sets this to HASH_SIZE which is currently == sizeof(e->hash).
> >
> > Actually, the assert statement would be easier to debug if the input
> > to this function is ever unexpected, especially since it avoids an
> > outcome where the input is silently truncated; which is a pitfall that
> > that the memcpy clamping would fall into.
>
> I agree. I'll change it to:
>
> assert(hash_len == sizeof(e->hash));
> memcpy(e->hash, hash, sizeof(e->hash));
>
> This way also the memcpy length is known at compile-time.
>
>
>
> Related: I wondered if I could replace HASH_SIZE in:
>
>
> /* hard code sha256 digest size */
> #define HASH_SIZE 32
>
> typedef struct QEMU_PACKED SevHashTableEntry {
> QemuUUID guid;
> uint16_t len;
> uint8_t hash[HASH_SIZE];
> } SevHashTableEntry;
>
>
> with some SHA256-related constant from crypto/hash.h, but I only found
> the qcrypto_hash_digest_len(QCRYPTO_HASH_ALG_SHA256) function which
> doesn't work for setting sizes of arrays at compile-time.
>
> Daniel: do you know what would be the proper way?
We don't have any public constants right now - they're just hardcoded
in hash.c struct. We could define public constants, and use those in
the struct instead, as well as in other callers.
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 :|
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 1/2] sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux boot
2021-06-23 8:49 ` Daniel P. Berrangé
@ 2021-06-23 9:28 ` Dov Murik
0 siblings, 0 replies; 15+ messages in thread
From: Dov Murik @ 2021-06-23 9:28 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Tom Lendacky, Ashish Kalra, Brijesh Singh, Eduardo Habkost,
Michael S. Tsirkin, Connor Kuehl, Laszlo Ersek, James Bottomley,
Richard Henderson, qemu-devel, Dr. David Alan Gilbert, dovmurik,
Tobin Feldman-Fitzthum, Jim Cadden, Paolo Bonzini,
Philippe Mathieu-Daudé
On 23/06/2021 11:49, Daniel P. Berrangé wrote:
> On Wed, Jun 23, 2021 at 11:41:56AM +0300, Dov Murik wrote:
...
>>
>> Related: I wondered if I could replace HASH_SIZE in:
>>
>>
>> /* hard code sha256 digest size */
>> #define HASH_SIZE 32
>>
>> typedef struct QEMU_PACKED SevHashTableEntry {
>> QemuUUID guid;
>> uint16_t len;
>> uint8_t hash[HASH_SIZE];
>> } SevHashTableEntry;
>>
>>
>> with some SHA256-related constant from crypto/hash.h, but I only found
>> the qcrypto_hash_digest_len(QCRYPTO_HASH_ALG_SHA256) function which
>> doesn't work for setting sizes of arrays at compile-time.
>>
>> Daniel: do you know what would be the proper way?
>
> We don't have any public constants right now - they're just hardcoded
> in hash.c struct. We could define public constants, and use those in
> the struct instead, as well as in other callers.
>
Thanks Daniel.
I see the exact same pattern in block/quorom.c (see HASH_LENGTH there).
I'll leave this change for a separate series.
-Dov
^ permalink raw reply [flat|nested] 15+ messages in thread