On Fri, Jul 11, 2025 at 05:10:45PM -0400, 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.
>
> Signed-off-by: Zhuoying Cai <zy...@linux.ibm.com>
> ---
> include/hw/s390x/ipl/diag320.h | 47 ++++++
> target/s390x/diag.c | 254 ++++++++++++++++++++++++++++++++-
> 2 files changed, 300 insertions(+), 1 deletion(-)
>
> int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
> @@ -191,8 +193,252 @@ out:
> }
> }
>
> +static int diag_320_is_cert_valid(S390IPLCertificate qcert)
> +{
> + int version;
> + int rc;
> + Error *err = NULL;
> +
> + version = qcrypto_x509_get_cert_version(qcert.raw, qcert.size, &err);
> + if (version < 0) {
> + error_report_err(err);
> + return -1;
> + }
IMHO this isn't doing anything worthwhile given qemu accepts
any version at all here.
> +
> + rc = qcrypto_x509_check_cert_times(qcert.raw, qcert.size, &err);
> + if (rc != 0) {
> + error_report_err(err);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int diag_320_get_cert_info(VCEntry *vce, S390IPLCertificate qcert,
> + uint8_t **cert_der, unsigned char
> **key_id_data,
> + void **hash_data)
> +{
> + int algo;
> + int rc;
> + Error *err = NULL;
> +
> + /* key-type */
> + algo = qcrypto_x509_get_pk_algorithm(qcert.raw, qcert.size, &err);
> + if (algo < 0) {
> + error_report_err(err);
> + return -1;
> + }
> + if (algo == QCRYPTO_PK_ALGO_RSA) {
> + vce->key_type = DIAG_320_VCE_KEYTYPE_SELF_DESCRIBING;
> + }
Shouldn't this have an "else" clause to either fill in vce->key_type,
or raise an error
> +
> + /* certificate in DER format */
> + rc = qcrypto_x509_convert_cert_der(qcert.raw, qcert.size,
> + cert_der, &qcert.der_size, &err);
> + if (rc < 0) {
> + error_report_err(err);
> + goto out;
> + }
> +
> + /* VC format */
> + vce->format = DIAG_320_VCE_FORMAT_X509_DER;
> +
> + /* key id and key id len */
> + rc = qcrypto_x509_get_cert_key_id(qcert.raw, qcert.size,
> + QCRYPTO_KEYID_FLAGS_SHA256,
> + key_id_data, &qcert.key_id_size, &err);
> + if (rc < 0) {
> + error_report_err(err);
> + goto out;
> + }
> + vce->keyid_len = cpu_to_be16(qcert.key_id_size);
> +
> + /* hash type */
> + if (qcert.hash_type == QCRYPTO_SIG_ALGO_RSA_SHA256) {
> + vce->hash_type = DIAG_320_VCE_HASHTYPE_SHA2_256;
> + }
Again, an "else" clause to either fill in vce->hash_type,
or raise an error ?
> +
> + /* hash and hash len */
> + *hash_data = g_malloc0(qcert.hash_size);
> + rc = qcrypto_get_x509_cert_fingerprint(qcert.raw, qcert.size,
> + QCRYPTO_HASH_ALGO_SHA256,
> + *hash_data, &qcert.hash_size,
> &err);
> + if (rc < 0) {
> + error_report_err(err);
> + goto out;
> + }
> + vce->hash_len = cpu_to_be16(qcert.hash_size);
> +
> + return 0;
> +
> +out:
> + g_clear_pointer(cert_der, g_free);
> + g_clear_pointer(key_id_data, g_free);
> + g_clear_pointer(hash_data, g_free);
> +
> + return -1;
> +}
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
