On 8/27/25 10:55, Jared Rossi wrote: > > On 8/18/25 5:43 PM, Zhuoying Cai wrote: >> From: Collin Walling <wall...@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 Diag508SignatureVerificationBlock, with the CertificateStoreInfo >> fields ignored. 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 CertificateStoreInfo >> portion of the SigVerifBlock. 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 >> >> Signed-off-by: Collin Walling <wall...@linux.ibm.com> >> Signed-off-by: Zhuoying Cai <zy...@linux.ibm.com> >> --- >> docs/specs/s390x-secure-ipl.rst | 5 ++ >> include/hw/s390x/ipl/diag508.h | 23 +++++++ >> target/s390x/diag.c | 112 +++++++++++++++++++++++++++++++- >> 3 files changed, 139 insertions(+), 1 deletion(-) >> >> diff --git a/docs/specs/s390x-secure-ipl.rst >> b/docs/specs/s390x-secure-ipl.rst >> index 6b3249173f..385f8d85a8 100644 >> --- a/docs/specs/s390x-secure-ipl.rst >> +++ b/docs/specs/s390x-secure-ipl.rst >> @@ -64,3 +64,8 @@ that requires assistance from QEMU. >> >> Subcode 0 - query installed subcodes >> Returns a 64-bit mask indicating which subcodes are supported. >> + >> +Subcode 1 - perform signature verification >> + Perform signature-verification on a signed component, using certificates >> + from the certificate store and leveraging qcrypto libraries to perform >> + this operation. >> diff --git a/include/hw/s390x/ipl/diag508.h b/include/hw/s390x/ipl/diag508.h >> index 6281ad8299..c99c6705c0 100644 >> --- a/include/hw/s390x/ipl/diag508.h >> +++ b/include/hw/s390x/ipl/diag508.h >> @@ -11,5 +11,28 @@ >> #define S390X_DIAG508_H >> >> #define DIAG_508_SUBC_QUERY_SUBC 0x0000 >> +#define DIAG_508_SUBC_SIG_VERIF 0x8000 >> + >> +#define DIAG_508_RC_OK 0x0001 >> +#define DIAG_508_RC_NO_CERTS 0x0102 >> +#define DIAG_508_RC_INVAL_COMP_DATA 0x0202 >> +#define DIAG_508_RC_INVAL_PKCS7_SIG 0x0302 >> +#define DIAG_508_RC_FAIL_VERIF 0x0402 >> + >> +struct Diag508CertificateStoreInfo { >> + uint8_t idx; >> + uint8_t reserved[7]; >> + uint64_t len; >> +}; >> +typedef struct Diag508CertificateStoreInfo Diag508CertificateStoreInfo; >> + >> +struct Diag508SignatureVerificationBlock { >> + Diag508CertificateStoreInfo csi; >> + uint64_t comp_len; >> + uint64_t comp_addr; >> + uint64_t sig_len; >> + uint64_t sig_addr; >> +}; >> +typedef struct Diag508SignatureVerificationBlock >> Diag508SignatureVerificationBlock; > Maybe shorten the above structs to just Diag508CSI and Diag508SVB or > similar? >
On another note, I played around with updating the 508 code to make the structures similar to what we'd see in the PoPs (also made other changes based on feedback I left to Joy on other patches). I came up with this design: struct Diag508SigVerifBlock { uint32_t length; uint8_t reserved0[3]; uint8_t version; uint32_t reserved[2]; uint8_t cert_store_index; uint8_t reserved1[7]; uint64_t cert_len; uint64_t comp_len; uint64_t comp_addr; uint64_t sig_len; uint64_t sig_addr; }; typedef struct Diag508SigVerifBlock Diag508SigVerifBlock; This condenses the two structures while also making it 64 bytes. Do you think this is a good idea? >> >> #endif >> diff --git a/target/s390x/diag.c b/target/s390x/diag.c >> index 6519a3cedc..2fe25a2c66 100644 >> --- a/target/s390x/diag.c >> +++ b/target/s390x/diag.c >> @@ -573,9 +573,107 @@ void handle_diag_320(CPUS390XState *env, uint64_t r1, >> uint64_t r3, uintptr_t ra) >> } >> } >> >> +static int diag_508_verify_sig(uint8_t *cert, size_t cert_size, >> + uint8_t *comp, size_t comp_size, >> + uint8_t *sig, size_t sig_size) >> +{ >> + g_autofree uint8_t *sig_pem = NULL; >> + size_t sig_size_pem; >> + int rc; >> + >> + /* >> + * PKCS#7 signature with DER format >> + * Convert to PEM format for signature verification >> + */ >> + rc = qcrypto_pkcs7_convert_sig_pem(sig, sig_size, &sig_pem, >> &sig_size_pem, NULL); >> + if (rc < 0) { >> + return -1; >> + } >> + >> + /* >> + * Ignore errors from signature format convertion and verification, >> + * because currently in the certificate lookup process. >> + * >> + * Any error is treated as a verification failure, >> + * and the final result (verified or not) will be reported later. >> + */ >> + rc = qcrypto_x509_verify_sig(cert, cert_size, >> + comp, comp_size, >> + sig_pem, sig_size_pem, NULL); >> + if (rc < 0) { >> + return -1; >> + } >> + >> + return 0; >> +} >> + >> +static int handle_diag508_sig_verif(uint64_t addr, size_t csi_size, size_t >> svb_size, >> + S390IPLCertificateStore *qcs) >> +{ >> + int rc; >> + int verified; >> + 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 Diag508SignatureVerificationBlock *svb = NULL; >> + >> + if (!qcs || !qcs->count) { >> + return DIAG_508_RC_NO_CERTS; >> + } >> + >> + svb = g_new0(Diag508SignatureVerificationBlock, 1); >> + cpu_physical_memory_read(addr, svb, svb_size); >> + >> + 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; >> + } >> + >> + 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 >> + */ >> + 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->csi.idx = i; >> + svb->csi.len = cpu_to_be64(qcs->certs[i].der_size); >> + cpu_physical_memory_write(addr, &svb->csi, >> be32_to_cpu(csi_size)); >> + rc = DIAG_508_RC_OK; >> + break; >> + } >> + } >> + >> + return rc; >> +} >> + >> +QEMU_BUILD_BUG_MSG(sizeof(Diag508SignatureVerificationBlock) != 48, >> + "size of Diag508SignatureVerificationBlock is wrong"); >> + >> void handle_diag_508(CPUS390XState *env, uint64_t r1, uint64_t r3, >> uintptr_t ra) >> { >> + S390IPLCertificateStore *qcs = s390_ipl_get_certificate_store(); >> uint64_t subcode = env->regs[r3]; >> + uint64_t addr = env->regs[r1]; >> int rc; >> >> if (env->psw.mask & PSW_MASK_PSTATE) { >> @@ -590,7 +688,19 @@ void handle_diag_508(CPUS390XState *env, uint64_t r1, >> uint64_t r3, uintptr_t ra) >> >> switch (subcode) { >> case DIAG_508_SUBC_QUERY_SUBC: >> - rc = 0; >> + rc = DIAG_508_SUBC_SIG_VERIF; >> + break; >> + case DIAG_508_SUBC_SIG_VERIF: >> + size_t csi_size = sizeof(Diag508CertificateStoreInfo); >> + size_t svb_size = sizeof(Diag508SignatureVerificationBlock); >> + >> + if (!diag_parm_addr_valid(addr, svb_size, false) || >> + !diag_parm_addr_valid(addr, csi_size, true)) { >> + s390_program_interrupt(env, PGM_ADDRESSING, ra); >> + return; >> + } >> + >> + rc = handle_diag508_sig_verif(addr, csi_size, svb_size, qcs); >> break; >> default: >> s390_program_interrupt(env, PGM_SPECIFICATION, ra); > Regards, > Jared Rossi -- Regards, Collin