On 3/5/26 17:41, Zhuoying Cai wrote:
> DIAG 320 subcode 2 provides verification-certificates (VCs) that are in the
> certificate store. Only X509 certificates in DER format and SHA-256 hash
> type are recognized.
>
> The subcode value is denoted by setting the second-left-most bit
> of an 8-byte field.
>
> The Verification Certificate Block (VCB) contains the output data
> when the operation completes successfully. It includes a common
> header followed by zero or more Verification Certificate Entries (VCEs),
> depending on the VCB input length and the VC range (from the first VC
> index to the last VC index) in the certificate store.
>
> Each VCE contains information about a certificate retrieved from
> the S390IPLCertificateStore, such as the certificate name, key type,
> key ID length, hash length, and the raw certificate data.
> The key ID and hash are extracted from the raw certificate by the crypto API.
>
> Note: SHA2-256 VC hash type is required for retrieving the hash
> (fingerprint) of the certificate.
>
> Signed-off-by: Zhuoying Cai <[email protected]>
Functionally, this looks fine. I've noted a few nits below, but I don't
think they're worth dragging things out.
The important thing to address:
- documentation rewording
- CERT_NAME_MAX_LEN change
The rest are inconsequential.
> ---
> docs/specs/s390x-secure-ipl.rst | 22 ++
> hw/s390x/cert-store.h | 3 +-
> include/hw/s390x/ipl/diag320.h | 55 +++++
> target/s390x/diag.c | 343 +++++++++++++++++++++++++++++++-
> 4 files changed, 420 insertions(+), 3 deletions(-)
>
> diff --git a/docs/specs/s390x-secure-ipl.rst b/docs/specs/s390x-secure-ipl.rst
> index 52661fab00..708253ac91 100644
> --- a/docs/specs/s390x-secure-ipl.rst
> +++ b/docs/specs/s390x-secure-ipl.rst
> @@ -38,3 +38,25 @@ Subcode 1 - query verification certificate storage
> information
> The output is returned in the verification-certificate-storage-size block
> (VCSSB). A VCSSB length of 4 indicates that no certificates are available
> in the CS.
> +
> +Subcode 2 - store verification certificates
> + Provides VCs that are in the certificate store.
> +
> + The output is provided in a VCB, which includes a common header followed
> by
> + zero or more verification-certificate entries (VCEs).
> +
> + The instruction expects the cert store to
> + maintain an origin of 1 for the index (i.e. a retrieval of the first
> + certificate in the store should be denoted by setting first-VC to 1).
Nit: early newline after "...cert store to". There is space for a few
more characters on that line.
> +
> + The first-VC index and last-VC index fields of VCB specify the range of
> VCs
> + to be stored by subcode 2. Stored count and remained count fields specify
> + the number of VCs stored and could not be stored in the VCB due to
> + insufficient storage specified in the VCB input length field.
> +
This might read better. It makes it more clear how they're used.
"""
The first-VC and last-VC fields of the VCB specify the index range of
VCs to be stored in the VCB. Certs are stored sequentially, starting
with first-VC index. As each cert is stored, a "stored count" is
incremented. If there is not enough space to store all certs requested
by the index range, a "remaining count" will be recorded and no more
certificates will be stored.
"""
> + Each VCE contains a header followed by information extracted from a
> + certificate within the certificate store. The information includes:
> + key-id, hash, and certificate data. This information is stored
> + contiguously in a VCE (with zero-padding). Following the header, the
> + key-id is immediately stored. The hash and certificate data follow and
> + may be accessed via the respective offset fields stored in the VCE.
> diff --git a/hw/s390x/cert-store.h b/hw/s390x/cert-store.h
> index 7fc9503cb9..6f5ee63177 100644
> --- a/hw/s390x/cert-store.h
> +++ b/hw/s390x/cert-store.h
> @@ -11,10 +11,9 @@
> #define HW_S390_CERT_STORE_H
>
> #include "hw/s390x/ipl/qipl.h"
> +#include "hw/s390x/ipl/diag320.h"
> #include "crypto/x509-utils.h"
>
> -#define CERT_NAME_MAX_LEN 64
> -
Is there a reason why this was moved to diag320.h?
(comments below this point are inconsequential, only consider if a v10
is required).
> #define CERT_KEY_ID_LEN QCRYPTO_HASH_DIGEST_LEN_SHA256
> #define CERT_HASH_LEN QCRYPTO_HASH_DIGEST_LEN_SHA256
>
> diff --git a/include/hw/s390x/ipl/diag320.h b/include/hw/s390x/ipl/diag320.h
> index 6e4779c699..bfd6385b40 100644
> --- a/include/hw/s390x/ipl/diag320.h
> +++ b/include/hw/s390x/ipl/diag320.h
> @@ -12,19 +12,37 @@
>
> #define DIAG_320_SUBC_QUERY_ISM 0
> #define DIAG_320_SUBC_QUERY_VCSI 1
> +#define DIAG_320_SUBC_STORE_VC 2
>
> #define DIAG_320_RC_OK 0x0001
> #define DIAG_320_RC_NOT_SUPPORTED 0x0102
> #define DIAG_320_RC_INVAL_VCSSB_LEN 0x0202
> +#define DIAG_320_RC_INVAL_VCB_LEN 0x0204
> +#define DIAG_320_RC_BAD_RANGE 0x0302
>
> #define DIAG_320_ISM_QUERY_SUBCODES 0x80000000
> #define DIAG_320_ISM_QUERY_VCSI 0x40000000
> +#define DIAG_320_ISM_STORE_VC 0x20000000
>
> #define VCSSB_NO_VC 4
> #define VCSSB_MIN_LEN 128
> #define VCE_HEADER_LEN 128
> +/*
> + * If the VCE flags indicate an invalid certificate,
> + * the VCE length is set to 72, containing only the
> + * first five fields of VCEntry.
> + */
> +#define VCE_INVALID_LEN 72
> #define VCB_HEADER_LEN 64
>
> +#define CERT_NAME_MAX_LEN 64
> +
> +#define DIAG_320_VCE_FLAGS_VALID 0x80
> +#define DIAG_320_VCE_KEYTYPE_SELF_DESCRIBING 0
> +#define DIAG_320_VCE_KEYTYPE_ECDSA_P521 1
> +#define DIAG_320_VCE_FORMAT_X509_DER 1
> +#define DIAG_320_VCE_HASHTYPE_SHA2_256 1
> +
> struct VCStorageSizeBlock {
> uint32_t length;
> uint8_t reserved0[3];
> @@ -39,4 +57,41 @@ struct VCStorageSizeBlock {
> };
> typedef struct VCStorageSizeBlock VCStorageSizeBlock;
>
> +struct VCBlock {
> + uint32_t in_len;
> + uint32_t reserved0;
> + uint16_t first_vc_index;
> + uint16_t last_vc_index;
> + uint32_t reserved1[5];
> + uint32_t out_len;
> + uint8_t reserved2[4];
> + uint16_t stored_ct;
> + uint16_t remain_ct;
> + uint32_t reserved3[5];
> + uint8_t vce_buf[];
> +};
> +typedef struct VCBlock VCBlock;
> +
> +struct VCEntry {
> + uint32_t len;
> + uint8_t flags;
> + uint8_t key_type;
> + uint16_t cert_idx;
> + uint8_t name[CERT_NAME_MAX_LEN];
> + uint8_t format;
> + uint8_t reserved0;
> + uint16_t keyid_len;
> + uint8_t reserved1;
> + uint8_t hash_type;
> + uint16_t hash_len;
> + uint32_t reserved2;
> + uint32_t cert_len;
> + uint32_t reserved3[2];
> + uint16_t hash_offset;
> + uint16_t cert_offset;
> + uint32_t reserved4[7];
> + uint8_t cert_buf[];
> +};
> +typedef struct VCEntry VCEntry;
> +
> #endif
> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
> index c44624e1e6..5326522fda 100644
> --- a/target/s390x/diag.c
> +++ b/target/s390x/diag.c
> @@ -17,13 +17,16 @@
> #include "s390x-internal.h"
> #include "hw/watchdog/wdt_diag288.h"
> #include "system/cpus.h"
> +#include "hw/s390x/cert-store.h"
> #include "hw/s390x/ipl.h"
> #include "hw/s390x/ipl/diag320.h"
> #include "hw/s390x/s390-virtio-ccw.h"
> #include "system/kvm.h"
> #include "kvm/kvm_s390x.h"
> #include "target/s390x/kvm/pv.h"
> +#include "qapi/error.h"
> #include "qemu/error-report.h"
> +#include "crypto/x509-utils.h"
>
>
> static inline bool diag_parm_addr_valid(uint64_t addr, size_t size, bool
> write)
> @@ -236,8 +239,333 @@ static int handle_diag320_query_vcsi(S390CPU *cpu,
> uint64_t addr, uint64_t r1,
> return DIAG_320_RC_OK;
> }
>
> +static bool is_cert_valid(const S390IPLCertificate *cert)
> +{
> + int rc;
> + Error *err = NULL;
> +
> + rc = qcrypto_x509_check_cert_times(cert->raw, cert->size, &err);
> + if (rc != 0) {
> + error_report_err(err);
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static int handle_key_id(VCEntry *vce, const S390IPLCertificate *cert)
> +{
> + int rc;
> + g_autofree unsigned char *key_id_data = NULL;
> + size_t key_id_len;
> + Error *err = NULL;
> +
> + rc = qcrypto_x509_get_cert_key_id(cert->raw, cert->size,
> + QCRYPTO_HASH_ALGO_SHA256,
> + &key_id_data, &key_id_len, &err);
> + if (rc < 0) {
> + error_report_err(err);
> + return -1;
> + }
> +
> + if (VCE_HEADER_LEN + key_id_len > be32_to_cpu(vce->len)) {
> + error_report("Unable to write key ID: exceeds buffer bounds");
> + return -1;
> + }
> +
> + vce->keyid_len = cpu_to_be16(key_id_len);
> +
> + memcpy(vce->cert_buf, key_id_data, key_id_len);
> +
> + return 0;
> +}
> +
> +static int handle_hash(VCEntry *vce, const S390IPLCertificate *cert,
> + uint16_t keyid_field_len)
> +{
> + int rc;
> + uint16_t hash_offset;
> + g_autofree void *hash_data = NULL;
> + size_t hash_len;
> + Error *err = NULL;
> +
> + hash_len = CERT_HASH_LEN;
> + hash_data = g_malloc0(hash_len);
> + rc = qcrypto_get_x509_cert_fingerprint(cert->raw, cert->size,
> + QCRYPTO_HASH_ALGO_SHA256,
> + hash_data, &hash_len, &err);
> + if (rc < 0) {
> + error_report_err(err);
> + return -1;
> + }
> +
> + hash_offset = VCE_HEADER_LEN + keyid_field_len;
> + if (hash_offset + hash_len > be32_to_cpu(vce->len)) {
> + error_report("Unable to write hash: exceeds buffer bounds");
> + return -1;
> + }
> +
> + vce->hash_len = cpu_to_be16(hash_len);
> + vce->hash_type = DIAG_320_VCE_HASHTYPE_SHA2_256;
> + vce->hash_offset = cpu_to_be16(hash_offset);
> +
> + memcpy((uint8_t *)vce + hash_offset, hash_data, hash_len);
> +
> + return 0;
> +}
> +
> +static int handle_cert(VCEntry *vce, const S390IPLCertificate *cert,
> + uint16_t hash_field_len)
> +{
> + int rc;
> + uint16_t cert_offset;
> + g_autofree uint8_t *cert_der = NULL;
> + size_t der_size;
> + Error *err = NULL;
> +
> + rc = qcrypto_x509_convert_cert_der(cert->raw, cert->size,
> + &cert_der, &der_size, &err);
> + if (rc < 0) {
> + error_report_err(err);
> + return -1;
> + }
> +
> + cert_offset = be16_to_cpu(vce->hash_offset) + hash_field_len;
> + if (cert_offset + der_size > be32_to_cpu(vce->len)) {
> + error_report("Unable to write certificate: exceeds buffer bounds");
> + return -1;
> + }
> +
> + vce->format = DIAG_320_VCE_FORMAT_X509_DER;
> + vce->cert_len = cpu_to_be32(der_size);
> + vce->cert_offset = cpu_to_be16(cert_offset);
> +
> + memcpy((uint8_t *)vce + cert_offset, cert_der, der_size);
> +
> + return 0;
> +}
> +
> +static int get_key_type(const S390IPLCertificate *cert)
> +{
> + int rc;
> + Error *err = NULL;
> +
> + rc = qcrypto_x509_check_ecc_curve_p521(cert->raw, cert->size, &err);
> + if (rc == -1) {
> + error_report_err(err);
> + return -1;
> + }
> +
> + return (rc == 1) ? DIAG_320_VCE_KEYTYPE_ECDSA_P521 :
> + DIAG_320_VCE_KEYTYPE_SELF_DESCRIBING;
> +}
> +
> +static int build_vce_header(VCEntry *vce, const S390IPLCertificate *cert,
> int idx)
> +{
> + int key_type;
> +
> + vce->len = cpu_to_be32(VCE_HEADER_LEN);
> + vce->cert_idx = cpu_to_be16(idx + 1);
> + memcpy(vce->name, cert->name, CERT_NAME_MAX_LEN);
> +
> + key_type = get_key_type(cert);
> + if (key_type == -1) {
> + return -1;
> + }
> + vce->key_type = key_type;
> +
> + return 0;
> +}
> +
> +static int build_vce_data(VCEntry *vce, const S390IPLCertificate *cert)
> +{
> + uint16_t keyid_field_len;
> + uint16_t hash_field_len;
> + uint32_t cert_field_len;
> + uint32_t vce_len;
> + int rc;
> +
> + rc = handle_key_id(vce, cert);
> + if (rc) {
> + return -1;
> + }
> + keyid_field_len = ROUND_UP(be16_to_cpu(vce->keyid_len), 4);
> +
> + rc = handle_hash(vce, cert, keyid_field_len);
> + if (rc) {
> + return -1;
> + }
> + hash_field_len = ROUND_UP(be16_to_cpu(vce->hash_len), 4);
> +
> + rc = handle_cert(vce, cert, hash_field_len);
> + if (rc || !is_cert_valid(cert)) {
> + return -1;
> + }
> + cert_field_len = ROUND_UP(be32_to_cpu(vce->cert_len), 4);
> +
> + vce_len = VCE_HEADER_LEN + keyid_field_len + hash_field_len +
> cert_field_len;
> + if (vce_len > be32_to_cpu(vce->len)) {
> + return -1;
> + }
> +
> + vce->flags |= DIAG_320_VCE_FLAGS_VALID;
> +
> + /* Update vce length to reflect the actual size used by vce */
> + vce->len = cpu_to_be32(vce_len);
> +
> + return 0;
> +}
> +
> +static VCEntry *diag_320_build_vce(const S390IPLCertificate *cert, int idx)
> +{
> + g_autofree VCEntry *vce = NULL;
> + uint32_t vce_max_size;
> + int rc;
> +
> + /*
> + * Each field of the VCE is word-aligned.
> + * Allocate enough space for the largest possible size for this VCE.
> + * As the certificate fields (key-id, hash, data) are parsed, the
> + * VCE's length field will be updated accordingly.
> + */
> + vce_max_size = VCE_HEADER_LEN +
> + ROUND_UP(CERT_KEY_ID_LEN, 4) +
> + ROUND_UP(CERT_HASH_LEN, 4) +
> + ROUND_UP(cert->der_size, 4);
> +
> + vce = g_malloc0(vce_max_size);
> + rc = build_vce_header(vce, cert, idx);
> + if (rc) {
> + /*
> + * Error occurs - VCE does not contain a valid certificate.
> + * Bit 0 of the VCE flags is 0 and the VCE length is set.
> + */
> + vce->len = cpu_to_be32(VCE_INVALID_LEN);
> + goto out;
> + }
> +
> + vce->len = cpu_to_be32(vce_max_size);
> + rc = build_vce_data(vce, cert);
> + if (rc) {
> + vce->len = cpu_to_be32(VCE_INVALID_LEN);
> + }
> +
> +out:
> + return g_steal_pointer(&vce);
> +}
> +
> +static int handle_diag320_store_vc(S390CPU *cpu, uint64_t addr, uint64_t r1,
> uintptr_t ra,
> + S390IPLCertificateStore *cs)
> +{
> + g_autofree VCBlock *vcb = NULL;
> + size_t entry_offset;
> + size_t remaining_space;
> + uint32_t vce_len;
> + uint16_t first_vc_index;
> + uint16_t last_vc_index;
> + int cs_start_index;
> + int cs_end_index;
> + uint32_t in_len;
> +
> + vcb = g_new0(VCBlock, 1);
> + if (s390_cpu_virt_mem_read(cpu, addr, r1, vcb, sizeof(*vcb))) {
> + s390_cpu_virt_mem_handle_exc(cpu, ra);
> + return -1;
> + }
> +
> + in_len = be32_to_cpu(vcb->in_len);
> + first_vc_index = be16_to_cpu(vcb->first_vc_index);
> + last_vc_index = be16_to_cpu(vcb->last_vc_index);
> +
> + if (in_len % TARGET_PAGE_SIZE != 0) {
> + return DIAG_320_RC_INVAL_VCB_LEN;
> + }
> +
> + if (first_vc_index > last_vc_index) {
> + return DIAG_320_RC_BAD_RANGE;
> + }
> +
> + vcb->out_len = VCB_HEADER_LEN;
> +
> + /*
> + * DIAG 320 subcode 2 expects to query a certificate store that
> + * maintains an index origin of 1. However, the S390IPLCertificateStore
> + * maintains an index origin of 0. Thus, the indices must be adjusted
> + * for correct access into the cert store. A couple of special cases
> + * must also be accounted for.
> + */
> +
> + /* Both indices are 0; return header with no certs */
> + if (first_vc_index == 0 && last_vc_index == 0) {
> + goto out;
> + }
> +
> + /* Normalize indices */
> + cs_start_index = (first_vc_index == 0) ? 0 : first_vc_index - 1;
> + cs_end_index = last_vc_index - 1;
> +
> + /* Requested range is outside the cert store; return header with no
> certs */
> + if (cs_start_index >= cs->count || cs_end_index >= cs->count) {
> + goto out;
> + }
> +
> + entry_offset = VCB_HEADER_LEN;
> + remaining_space = in_len - VCB_HEADER_LEN;
> +
> + for (int i = cs_start_index; i <= cs_end_index; i++) {
> + VCEntry *vce;
> + const S390IPLCertificate *cert = &cs->certs[i];
> +
> + /*
> + * Bit 0 of the VCE flags indicates whether the certificate is valid.
> + * The caller of DIAG320 subcode 2 is responsible for verifying that
> + * the VCE contains a valid certificate.
> + */
> + vce = diag_320_build_vce(cert, i);
> + vce_len = be32_to_cpu(vce->len);
Why not just use vce->len? It would seem that diag_320_build_vce will
set the len to one of:
- max len
- actual len
- invalid len (72)
In all cases, endianness is accounted for.
> +
> + /*
> + * If there is no more space to store the cert,
> + * set the remaining verification cert count and
> + * break early.
> + */
> + if (remaining_space < vce_len) {
> + vcb->remain_ct = cpu_to_be16(last_vc_index - i);
> + g_free(vce);
> + break;
> + }
> +
> + /* Write VCE */
> + if (s390_cpu_virt_mem_write(cpu, addr + entry_offset, r1, vce,
> vce_len)) {
> + s390_cpu_virt_mem_handle_exc(cpu, ra);
> + g_free(vce);
> + return -1;
> + }
> +
> + entry_offset += vce_len;
entry_offset seems to be serving the same purpose as vcb->out_len. I
think it makes sense to drop the entry_offset variable and use
vcb->out_len in its place. Then there is one less thing to keep track of.
> + vcb->out_len += vce_len;
> + remaining_space -= vce_len;
> + vcb->stored_ct++;
> +
> + g_free(vce);
> + }
> + vcb->stored_ct = cpu_to_be16(vcb->stored_ct);
> +
> +out:
> + vcb->out_len = cpu_to_be32(vcb->out_len);
> +
> + if (s390_cpu_virt_mem_write(cpu, addr, r1, vcb, VCB_HEADER_LEN)) {
> + s390_cpu_virt_mem_handle_exc(cpu, ra);
> + return -1;
> + }
> +
> + return DIAG_320_RC_OK;
> +}
> +
> QEMU_BUILD_BUG_MSG(sizeof(VCStorageSizeBlock) != VCSSB_MIN_LEN,
> "size of VCStorageSizeBlock is wrong");
> +QEMU_BUILD_BUG_MSG(sizeof(VCBlock) != VCB_HEADER_LEN, "size of VCBlock is
> wrong");
> +QEMU_BUILD_BUG_MSG(sizeof(VCEntry) != VCE_HEADER_LEN, "size of VCEntry is
> wrong");
>
> void handle_diag_320(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t
> ra)
> {
> @@ -268,7 +596,8 @@ void handle_diag_320(CPUS390XState *env, uint64_t r1,
> uint64_t r3, uintptr_t ra)
> * for now.
> */
> uint32_t ism_word0 = cpu_to_be32(DIAG_320_ISM_QUERY_SUBCODES |
> - DIAG_320_ISM_QUERY_VCSI);
> + DIAG_320_ISM_QUERY_VCSI |
> + DIAG_320_ISM_STORE_VC);
>
> if (s390_cpu_virt_mem_write(cpu, addr, r1, &ism_word0,
> sizeof(ism_word0))) {
> s390_cpu_virt_mem_handle_exc(cpu, ra);
> @@ -294,6 +623,18 @@ void handle_diag_320(CPUS390XState *env, uint64_t r1,
> uint64_t r3, uintptr_t ra)
> }
> env->regs[r1 + 1] = rc;
> break;
> + case DIAG_320_SUBC_STORE_VC:
> + if (addr & ~TARGET_PAGE_MASK) {
> + s390_program_interrupt(env, PGM_SPECIFICATION, ra);
> + return;
> + }
> +
> + rc = handle_diag320_store_vc(cpu, addr, r1, ra, cs);
> + if (rc == -1) {
> + return;
> + }
> + env->regs[r1 + 1] = rc;
> + break;
> default:
> env->regs[r1 + 1] = DIAG_320_RC_NOT_SUPPORTED;
> break;
--
Regards,
Collin
