On 08/12/2025 22.32, 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]>
---
...
> +struct VCEntry {
> + uint32_t len;
> + uint8_t flags;
> + uint8_t key_type;
> + uint16_t cert_idx;
> + uint32_t name[16];
Why is this defined as an array of uint32_t when it is rather a string instead?
> + 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;
...
diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index 0e1897e03d..1498b29a0d 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -17,6 +17,7 @@
#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"
@@ -24,6 +25,7 @@
#include "kvm/kvm_s390x.h"
#include "target/s390x/kvm/pv.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)
@@ -231,8 +233,330 @@ static int handle_diag320_query_vcsi(S390CPU *cpu,
uint64_t addr, uint64_t r1,
return DIAG_320_RC_OK;
}
+static bool is_cert_valid(S390IPLCertificate cert)
You're using call-by-value for all S390IPLCertificate parameters in this
patch ... that's quite cumbersome since the struct has a size of ~80 bytes.
Unless there is a very good reason for doing so, I'd suggest that you switch
the code to use (const) pointers instead of passing around that struct on
the stack.
+{
+ 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 void handle_key_id(VCEntry *vce, S390IPLCertificate cert)
+{
+ int rc;
+ g_autofree unsigned char *key_id_data = NULL;
+ size_t key_id_len;
+ Error *err = NULL;
+
+ key_id_len = CERT_KEY_ID_LEN;
+ /* key id and key id len */
+ 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;
+ }
+
+ if (VCE_HEADER_LEN + key_id_len > be32_to_cpu(vce->len)) {
+ error_report("Unable to write key ID: exceeds buffer bounds");
+ return;
+ }
+
+ vce->keyid_len = cpu_to_be16(key_id_len);
+
+ memcpy(vce->cert_buf, key_id_data, key_id_len);
+}
+
+static int handle_hash(VCEntry *vce, 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 and 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, S390IPLCertificate cert, uint16_t
hash_field_len)
+{
+ int rc;
+ uint16_t cert_offset;
+ g_autofree uint8_t *cert_der = NULL;
+ Error *err = NULL;
+
+ /* certificate in DER format */
+ rc = qcrypto_x509_convert_cert_der(cert.raw, cert.size,
+ &cert_der, &cert.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 + cert.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(cert.der_size);
+ vce->cert_offset = cpu_to_be16(cert_offset);
+
+ memcpy((uint8_t *)vce + cert_offset, cert_der, cert.der_size);
+
+ return 0;
+}
+
+static int get_key_type(S390IPLCertificate cert)
+{
+ int algo;
+ int rc;
+ Error *err = NULL;
+
+ /* public key algorithm */
+ algo = qcrypto_x509_get_pk_algorithm(cert.raw, cert.size, &err);
+ if (algo < 0) {
+ error_report_err(err);
+ return -1;
+ }
+
+ if (algo == QCRYPTO_PK_ALGO_ECDSA) {
+ 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;
+ }
+
+ return DIAG_320_VCE_KEYTYPE_SELF_DESCRIBING;
+}
+
+static int build_vce_header(VCEntry *vce, S390IPLCertificate cert, int idx)
+{
+ int key_type;
+
+ vce->len = cpu_to_be32(VCE_HEADER_LEN);
+ vce->cert_idx = cpu_to_be16(idx + 1);
+ strncpy((char *)vce->name, (char *)cert.vc_name, VC_NAME_LEN_BYTES);
strncpy is often tripping up static analyzers when you use it like this.
Please consider using memcpy(), pstrcpy() or strpadcpy() instead.
(I guess memcpy is the right thing to use here since vc_name has already
been initialized with strpadcpy ?)
+ key_type = get_key_type(cert);
+ if (key_type == -1) {
+ return -1;
+ }
+ vce->key_type = key_type;
+
+ return 0;
+}
Thomas
