Re: [PATCH v7 09/17] target/s390x: Support pckmo encrypt AES subfunctions

[Harald Freudenberger <freude@linux.ibm.com>]

On 2026-06-18 07:50, Finn Callies wrote:
> On 17.06.26 11:48, Harald Freudenberger wrote:
>> Support the subfuctions PCKMO-Encrypt-AES-128-Key,
>> PCKMO-Encrypt-AES-192-Key and PCKMO-Encrypt-AES-256-Key.
>> 
>> These subfunctions derive a protected key from an AES clear key
>> by encrypting it with an internal AES wrapping key. More
>> details can be found in the "z/Architecture Prinziples of
>> Operation" document.
>> 
>> The qemu version provided here is only a fake indented to make
> 
> typo: indeted -> intended
> 
>> protected key available for developing and testing purpose:
>> * The protected key is 'derived' from the clear key by xoring
>>    the fixed pattern 0xAAAA... onto the key value.
>> * The AES Wrapping Key Verification Pattern is a fixed
>>    value of 32 bytes 0xFACEFACE...
>> 
>> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
>> Tested-by: Holger Dengler <dengler@linux.ibm.com>
> 
> With the typo fixed and the other comments at least considered:
> 
> Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
> 
>> ---
>>   target/s390x/gen-features.c      |  3 ++
>>   target/s390x/tcg/cpacf.h         |  2 +
>>   target/s390x/tcg/cpacf_aes.c     | 66 
>> ++++++++++++++++++++++++++++++++
>>   target/s390x/tcg/crypto_helper.c | 21 ++++++++++
>>   4 files changed, 92 insertions(+)
> 
> [ snip ]
> 
>> diff --git a/target/s390x/tcg/cpacf_aes.c 
>> b/target/s390x/tcg/cpacf_aes.c
>> index 0312436c43..5a0a3473d5 100644
>> --- a/target/s390x/tcg/cpacf_aes.c
>> +++ b/target/s390x/tcg/cpacf_aes.c
>> @@ -467,3 +467,69 @@ int cpacf_aes_xts(CPUS390XState *env, const int 
>> mmu_idx, uintptr_t ra,
>>         return !len ? 0 : 3;
>>   }
>> +
>> +/*
>> + * Hard coded pattern xored with the AES clear key
>> + * to 'produce' the protected key.
>> + */
>> +static const uint8_t protkey_xor_pattern[32] = {
> 
> the wrapping key for protected keys is called virtual server master
> key if I recall correctly. Maybe name this accordingly or at least
> something with master or wrapping key in it as this describes the
> purpose better I think.
> 

But honestly I don't want to call this a key - neither a wrapping nor
a virtual server master key. This is a simple xor pattern to be applied
to the clear key value. I don't want to call this a key in any way.
If I would label this a key someone (or AI) would immediately raise
a security finding - hard coded key / weak key / key in code ...

>> +    0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,
>> +    0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,
>> +    0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,
>> +    0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA };
>> +
>> +/*
>> + * Hard coded wkvp ("Wrapping Key Verification Pattern")
>> + */
>> +static const uint8_t protkey_wkvp[32] = {
> 
> Since you call this protkey_wkvp you could call the wrapping key 
> protkey_wk.
> 
>> +    0x0F, 0x0A, 0x0C, 0x0E, 0x0F, 0x0A, 0x0C, 0x0E,
>> +    0x0F, 0x0A, 0x0C, 0x0E, 0x0F, 0x0A, 0x0C, 0x0E,
>> +    0x0F, 0x0A, 0x0C, 0x0E, 0x0F, 0x0A, 0x0C, 0x0E,
>> +    0x0F, 0x0A, 0x0C, 0x0E, 0x0F, 0x0A, 0x0C, 0x0E };
> 
> [ snip ]