Re: [PATCH v6 12/28] s390x/diag: Implement DIAG 508 subcode 1 for signature verification

[Zhuoying Cai <zycai@linux.ibm.com>]

On 10/7/25 6:27 AM, Thomas Huth wrote:
> On 18/09/2025 01.21, Zhuoying Cai wrote:
>> From: Collin Walling <walling@linux.ibm.com>
>>
>> DIAG 508 subcode 1 performs signature-verification on signed components.
>> A signed component may be a Linux kernel image, or any other signed
>> binary. **Verification of initrd is not supported.**
>>
>> The instruction call expects two item-pairs: an address of a device
>> component, an address of the analogous signature file (in PKCS#7 DER format),
>> and their respective lengths. All of this data should be encapsulated
>> within a Diag508SigVerifBlock.
>>
>> The DIAG handler will read from the provided addresses
>> to retrieve the necessary data, parse the signature file, then
>> perform the signature-verification. Because there is no way to
>> correlate a specific certificate to a component, each certificate
>> in the store is tried until either verification succeeds, or all
>> certs have been exhausted.
>>
>> The subcode value is denoted by setting the second-to-left-most bit of
>> a 2-byte field.
>>
>> A return code of 1 indicates success, and the index and length of the
>> corresponding certificate will be set in the Diag508SigVerifBlock.
>> The following values indicate failure:
>>
>> 	0x0102: certificate not available
>> 	0x0202: component data is invalid
>> 	0x0302: signature is not in PKCS#7 format
>> 	0x0402: signature-verification failed
>> 	0x0502: length of Diag508SigVerifBlock is invalid
>>
>> Signed-off-by: Collin Walling <walling@linux.ibm.com>
>> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>

[...]

>> +
>> +static int handle_diag508_sig_verif(uint64_t addr, size_t svb_size,
>> +                                    S390IPLCertificateStore *qcs)
>> +{
>> +    int rc;
>> +    int verified;
>> +    uint32_t svb_len;
>> +    uint64_t comp_len, comp_addr;
>> +    uint64_t sig_len, sig_addr;
>> +    g_autofree uint8_t *svb_comp = NULL;
>> +    g_autofree uint8_t *svb_sig = NULL;
>> +    g_autofree Diag508SigVerifBlock *svb = NULL;
>> +
>> +    if (!qcs || !qcs->count) {
>> +        return DIAG_508_RC_NO_CERTS;
>> +    }
>> +
>> +    svb = g_new0(Diag508SigVerifBlock, 1);
>> +    cpu_physical_memory_read(addr, svb, svb_size);
>> +
>> +    svb_len = be32_to_cpu(svb->length);
>> +    if (svb_len != svb_size) {
>> +        return DIAG_508_RC_INVAL_LEN;
>> +    }
>> +
>> +    comp_len = be64_to_cpu(svb->comp_len);
>> +    comp_addr = be64_to_cpu(svb->comp_addr);
>> +    sig_len = be64_to_cpu(svb->sig_len);
>> +    sig_addr = be64_to_cpu(svb->sig_addr);
>> +
>> +    if (!comp_len || !comp_addr) {
>> +        return DIAG_508_RC_INVAL_COMP_DATA;
>> +    }
>> +
>> +    if (!sig_len || !sig_addr) {
>> +        return DIAG_508_RC_INVAL_PKCS7_SIG;
>> +    }
> 
> I think there should also be something like an upper limit for comp_len and 
> sign_len here. Otherwise a malicious guest could force QEMU into allocating 
> giga- or terabytes of memory here to cause out-of-memory situations in the host.
> 

Thank you for the suggestion. I agree that setting an upper limit would
help prevent unreasonable memory requests. I think it makes sense to
choose a reasonable value so we don't have to adjust it too often, but
I'm not entirely sure how to determine an appropriate upper bound.

Re: sig_len - the signature length can vary depending on the
cryptographic algorithm, and I don't believe there's a strict limit.
(FYI, in a somewhat similar situation, we haven't enforced a maximum
size on certificate files when loading them into memory, since they're
assumed to be trusted, as Daniel previously suggested -
https://lists.gnu.org/archive/html/qemu-s390x/2025-06/msg00049.html).

If we'd like to set an upper limit for sig_len, the largest signature
I've tested is 1165 bytes, signed with an RSA certificate using an
8192-bit key. Would 4096 be a reasonable upper bound?

Re: comp_len - the size of the guest kernel I'm currently using is
14,184,448 (0xD87000). When I built a kernel with make allyesconfig, the
size can reach 261,005,383 (0xF8EA047). Based on this value, would
262,000,000 (0xF9DCD80) an appropriate upper limit?

>> +    svb_comp = g_malloc0(comp_len);
>> +    cpu_physical_memory_read(comp_addr, svb_comp, comp_len);
>> +
>> +    svb_sig = g_malloc0(sig_len);
>> +    cpu_physical_memory_read(sig_addr, svb_sig, sig_len);
>> +
>> +    rc = DIAG_508_RC_FAIL_VERIF;
>> +    /*
>> +     * It is uncertain which certificate contains
>> +     * the analogous key to verify the signed data
>> +     *
>> +     * Ignore errors from signature format convertion and verification,
>> +     * because currently in the certificate lookup process.
> 
> The second half of above sentence looks incomplete?
> 
>> +     *
>> +     * Any error is treated as a verification failure,
>> +     * and the final result (verified or not) will be reported later.
>> +     */
>> +    for (int i = 0; i < qcs->count; i++) {
>> +        verified = diag_508_verify_sig(qcs->certs[i].raw,
>> +                                       qcs->certs[i].size,
>> +                                       svb_comp, comp_len,
>> +                                       svb_sig, sig_len);
>> +        if (verified == 0) {
>> +            svb->cert_store_index = i;
>> +            svb->cert_len = cpu_to_be64(qcs->certs[i].der_size);
>> +            cpu_physical_memory_write(addr, svb, be32_to_cpu(svb_size));
>> +            rc = DIAG_508_RC_OK;
>> +            break;
>> +       }
>> +    }
>> +
>> +    return rc;
>> +}
> 
>   Thomas
>