On 8/18/25 17:42, Zhuoying Cai wrote:
> Create a certificate store for boot certificates used for secure IPL.
>
> Load certificates from the `boot-certs` parameter of s390-ccw-virtio
> machine type option into the cert store.
>
> Currently, only X.509 certificates in PEM format are supported, as the
> QEMU command line accepts certificates in PEM format only.
>
> Signed-off-by: Zhuoying Cai <[email protected]>
> ---
> hw/s390x/cert-store.c | 201 ++++++++++++++++++++++++++++++++++++
> hw/s390x/cert-store.h | 38 +++++++
> hw/s390x/ipl.c | 9 ++
> hw/s390x/ipl.h | 3 +
> hw/s390x/meson.build | 1 +
> include/hw/s390x/ipl/qipl.h | 2 +
> 6 files changed, 254 insertions(+)
> create mode 100644 hw/s390x/cert-store.c
> create mode 100644 hw/s390x/cert-store.h
>
> diff --git a/hw/s390x/cert-store.c b/hw/s390x/cert-store.c
> new file mode 100644
> index 0000000000..81e748a912
> --- /dev/null
> +++ b/hw/s390x/cert-store.c
> @@ -0,0 +1,201 @@
> +/*
> + * S390 certificate store implementation
> + *
> + * Copyright 2025 IBM Corp.
> + * Author(s): Zhuoying Cai <[email protected]>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "cert-store.h"
> +#include "qapi/error.h"
> +#include "qemu/error-report.h"
> +#include "qemu/option.h"
> +#include "qemu/config-file.h"
> +#include "hw/s390x/ebcdic.h"
> +#include "hw/s390x/s390-virtio-ccw.h"
> +#include "qemu/cutils.h"
> +#include "crypto/x509-utils.h"
> +#include "qapi/qapi-types-machine-s390x.h"
> +
> +static BootCertPathList *s390_get_boot_certs(void)
> +{
> + return S390_CCW_MACHINE(qdev_get_machine())->boot_certs;
> +}
> +
> +static size_t cert2buf(char *path, char **cert_buf)
> +{
> + size_t size;
> +
> + if (!g_file_get_contents(path, cert_buf, &size, NULL) || size == 0) {
> + return 0;
> + }
> +
> + return size;
> +}
Seems redundant to check if size == 0 here. If g_file_get_contents
succeeds and size is set to 0, there's no difference between returning 0
or returning size.
> +
> +static S390IPLCertificate *init_cert_x509(size_t size, uint8_t *raw, Error
> **errp)
> +{
> + S390IPLCertificate *q_cert = NULL;
I think the term "q_cert" is from an older implementation when we called
this a "QEMUCertStore". Since we're doing this for s390 right now, I
think it makes sense to just to call it "cert".
> + g_autofree uint8_t *cert_der = NULL;
> + size_t der_len = size;
> + int rc;
> +
> + rc = qcrypto_x509_convert_cert_der(raw, size, &cert_der, &der_len, errp);
> + if (rc != 0) {
> + return NULL;
> + }
> +
> + q_cert = g_new0(S390IPLCertificate, 1);
> + q_cert->size = size;
> + q_cert->der_size = der_len;
> + q_cert->key_id_size = QCRYPTO_HASH_DIGEST_LEN_SHA256;
> + q_cert->hash_size = QCRYPTO_HASH_DIGEST_LEN_SHA256;
Setting the sizes here doesn't seem right:
1. it's making an assumption about the cert's data, and
2. these fields are never accessed in any subsequent patch
Patch 9 will explicitly use this enum when retrieving the cert and
fingerprint, but this data is neither checked nor extracted from the
S390IPLCertificate var.
I think it makes sense to get rid of the above two lines and potentially
the two S390IPLCertificate fields as well.
> + q_cert->raw = raw;
> +
> + return q_cert;
> +}
> +
> +static S390IPLCertificate *init_cert(char *path)
> +{
> + char *buf;
> + size_t size;
> + char vc_name[VC_NAME_LEN_BYTES];
> + g_autofree gchar *filename = NULL;
> + S390IPLCertificate *qcert = NULL;
> + Error *local_err = NULL;
> +
> + filename = g_path_get_basename(path);
> +
> + size = cert2buf(path, &buf);
> + if (size == 0) {
> + error_report("Failed to load certificate: %s", path);
> + return NULL;
> + }
> +
> + qcert = init_cert_x509(size, (uint8_t *)buf, &local_err);
> + if (qcert == NULL) {
> + error_reportf_err(local_err, "Failed to initialize certificate: %s:
> ", path);
> + g_free(buf);
> + return NULL;
> + }
> +
> + /*
> + * Left justified certificate name with padding on the right with blanks.
> + * Convert certificate name to EBCDIC.
> + */
> + strpadcpy(vc_name, VC_NAME_LEN_BYTES, filename, ' ');
> + ebcdic_put(qcert->vc_name, vc_name, VC_NAME_LEN_BYTES);
> +
> + return qcert;
> +}
> +
> +static void update_cert_store(S390IPLCertificateStore *cert_store,
> + S390IPLCertificate *qcert)
> +{
> + size_t data_buf_size;
> + size_t keyid_buf_size;
> + size_t hash_buf_size;
> + size_t cert_buf_size;
> +
> + /* length field is word aligned for later DIAG use */
> + keyid_buf_size = ROUND_UP(qcert->key_id_size, 4);
> + hash_buf_size = ROUND_UP(qcert->hash_size, 4);
> + cert_buf_size = ROUND_UP(qcert->der_size, 4);
> + data_buf_size = keyid_buf_size + hash_buf_size + cert_buf_size;
> +
> + if (cert_store->max_cert_size < data_buf_size) {
> + cert_store->max_cert_size = data_buf_size;
> + }
> +
> + cert_store->certs[cert_store->count] = *qcert;
> + cert_store->total_bytes += data_buf_size;
> + cert_store->count++;
> +}
> +
> +static GPtrArray *get_cert_paths(void)
> +{
> + BootCertPathList *path_list = NULL;
> + BootCertPathList *list = NULL;
> + gchar *cert_path;
> + GDir *dir = NULL;
> + const gchar *filename;
> + g_autoptr(GError) err = NULL;
> + g_autoptr(GPtrArray) cert_path_builder = g_ptr_array_new_full(0, g_free);
> +
> + path_list = s390_get_boot_certs();
> + if (path_list == NULL) {
> + return g_steal_pointer(&cert_path_builder);
> + }
> +
> + for (list = path_list; list; list = list->next) {
> + cert_path = list->value->path;
> +
> + if (g_strcmp0(cert_path, "") == 0) {
> + error_report("Empty path in certificate path list is not
> allowed");
> + exit(1);
> + }
> +
> + struct stat st;
> + if (stat(cert_path, &st) != 0) {
> + error_report("Failed to stat path '%s': %s", cert_path,
> g_strerror(errno));
> + exit(1);
> + }
> +
> + if (S_ISREG(st.st_mode)) {
> + if (g_str_has_suffix(cert_path, ".pem")) {
> + g_ptr_array_add(cert_path_builder, g_strdup(cert_path));
> + }
> + } else if (S_ISDIR(st.st_mode)) {
> + dir = g_dir_open(cert_path, 0, &err);
> + if (dir == NULL) {
> + error_report("Failed to open directory '%s': %s",
> + cert_path, err->message);
> + exit(1);
> + }
> +
> + while ((filename = g_dir_read_name(dir))) {
> + if (g_str_has_suffix(filename, ".pem")) {
> + g_ptr_array_add(cert_path_builder,
> + g_build_filename(cert_path, filename,
> NULL));
> + }
> + }
> +
> + g_dir_close(dir);
> + } else {
> + error_report("Path '%s' is neither a file nor a directory",
> cert_path);
> + }
> + }
> +
> + qapi_free_BootCertPathList(path_list);
> + return g_steal_pointer(&cert_path_builder);
> +}
> +
> +void s390_ipl_create_cert_store(S390IPLCertificateStore *cert_store)
> +{
> + GPtrArray *cert_path_builder;
> +
> + cert_path_builder = get_cert_paths();
> + if (cert_path_builder->len == 0) {
> + g_ptr_array_free(cert_path_builder, TRUE);
> + return;
> + }
> +
> + cert_store->max_cert_size = 0;
> + cert_store->total_bytes = 0;
> +
> + for (int i = 0; i < cert_path_builder->len; i++) {
> + if (i > MAX_CERTIFICATES - 1) {
> + error_report("Maximum %d certificates are allowed",
> MAX_CERTIFICATES);
nit: reword to "Cert store exceeds maximum of %d certificates"
> + exit(1);
> + }
> +
> + S390IPLCertificate *qcert = init_cert((char *)
> cert_path_builder->pdata[i]);
> + if (qcert) {
> + update_cert_store(cert_store, qcert);
> + }
> + }
> +
> + g_ptr_array_free(cert_path_builder, TRUE);
> +}
> diff --git a/hw/s390x/cert-store.h b/hw/s390x/cert-store.h
> new file mode 100644
> index 0000000000..f030c8846c
> --- /dev/null
> +++ b/hw/s390x/cert-store.h
> @@ -0,0 +1,38 @@
> +/*
> + * S390 certificate store
> + *
> + * Copyright 2025 IBM Corp.
> + * Author(s): Zhuoying Cai <[email protected]>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef HW_S390_CERT_STORE_H
> +#define HW_S390_CERT_STORE_H
> +
> +#include "hw/s390x/ipl/qipl.h"
> +#include "crypto/x509-utils.h"
> +
> +#define VC_NAME_LEN_BYTES 64
> +
> +struct S390IPLCertificate {
> + uint8_t vc_name[VC_NAME_LEN_BYTES];
> + size_t size;
> + size_t der_size;
> + size_t key_id_size;
> + size_t hash_size;
>From above: I don't think these two fields are needed?
> + uint8_t *raw;
> +};
> +typedef struct S390IPLCertificate S390IPLCertificate;
> +
> +struct S390IPLCertificateStore {
> + uint16_t count;
> + size_t max_cert_size;
> + size_t total_bytes;
> + S390IPLCertificate certs[MAX_CERTIFICATES];
> +} QEMU_PACKED;
> +typedef struct S390IPLCertificateStore S390IPLCertificateStore;
> +
> +void s390_ipl_create_cert_store(S390IPLCertificateStore *cert_store);
> +
> +#endif
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 2f082396c7..186be923d7 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -35,6 +35,7 @@
> #include "qemu/option.h"
> #include "qemu/ctype.h"
> #include "standard-headers/linux/virtio_ids.h"
> +#include "cert-store.h"
>
> #define KERN_IMAGE_START 0x010000UL
> #define LINUX_MAGIC_ADDR 0x010008UL
> @@ -422,6 +423,13 @@ void s390_ipl_convert_loadparm(char *ascii_lp, uint8_t
> *ebcdic_lp)
> }
> }
>
> +S390IPLCertificateStore *s390_ipl_get_certificate_store(void)
> +{
> + S390IPLState *ipl = get_ipl_device();
> +
> + return &ipl->cert_store;
> +}
> +
> static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb)
> {
> CcwDevice *ccw_dev = NULL;
> @@ -717,6 +725,7 @@ void s390_ipl_prepare_cpu(S390CPU *cpu)
>
> if (!ipl->kernel || ipl->iplb_valid) {
> cpu->env.psw.addr = ipl->bios_start_addr;
> + s390_ipl_create_cert_store(&ipl->cert_store);
> if (!ipl->iplb_valid) {
> ipl->iplb_valid = s390_init_all_iplbs(ipl);
> } else {
> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> index 8f83c7da29..bee72dfbb3 100644
> --- a/hw/s390x/ipl.h
> +++ b/hw/s390x/ipl.h
> @@ -13,6 +13,7 @@
> #ifndef HW_S390_IPL_H
> #define HW_S390_IPL_H
>
> +#include "cert-store.h"
> #include "cpu.h"
> #include "exec/target_page.h"
> #include "system/address-spaces.h"
> @@ -35,6 +36,7 @@ int s390_ipl_pv_unpack(struct S390PVResponse *pv_resp);
> void s390_ipl_prepare_cpu(S390CPU *cpu);
> IplParameterBlock *s390_ipl_get_iplb(void);
> IplParameterBlock *s390_ipl_get_iplb_pv(void);
> +S390IPLCertificateStore *s390_ipl_get_certificate_store(void);
>
> enum s390_reset {
> /* default is a reset not triggered by a CPU e.g. issued by QMP */
> @@ -64,6 +66,7 @@ struct S390IPLState {
> IplParameterBlock iplb;
> IplParameterBlock iplb_pv;
> QemuIplParameters qipl;
> + S390IPLCertificateStore cert_store;
> uint64_t start_addr;
> uint64_t compat_start_addr;
> uint64_t bios_start_addr;
> diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build
> index 8866012ddc..80d3d4a74d 100644
> --- a/hw/s390x/meson.build
> +++ b/hw/s390x/meson.build
> @@ -17,6 +17,7 @@ s390x_ss.add(files(
> 'sclpcpu.c',
> 'sclpquiesce.c',
> 'tod.c',
> + 'cert-store.c',
> ))
> s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
> 'tod-kvm.c',
> diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
> index 6824391111..e505f44020 100644
> --- a/include/hw/s390x/ipl/qipl.h
> +++ b/include/hw/s390x/ipl/qipl.h
> @@ -20,6 +20,8 @@
> #define LOADPARM_LEN 8
> #define NO_LOADPARM "\0\0\0\0\0\0\0\0"
>
> +#define MAX_CERTIFICATES 64
> +
> /*
> * The QEMU IPL Parameters will be stored at absolute address
> * 204 (0xcc) which means it is 32-bit word aligned but not
--
Regards,
Collin
