[PATCH v5 04/29] hw/s390x/ipl: Create certificate store

Zhuoying Cai posted 29 patches 2 months, 4 weeks ago
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Jared Rossi <jrossi@linux.ibm.com>, Zhuoying Cai <zycai@linux.ibm.com>, Jason Herne <jjherne@linux.ibm.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
[PATCH v5 04/29] hw/s390x/ipl: Create certificate store
Posted by Zhuoying Cai 2 months, 4 weeks ago
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 <zycai@linux.ibm.com>
---
 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 <zycai@linux.ibm.com>
+ *
+ * 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;
+}
+
+static S390IPLCertificate *init_cert_x509(size_t size, uint8_t *raw, Error **errp)
+{
+    S390IPLCertificate *q_cert = NULL;
+    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;
+    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);
+            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 <zycai@linux.ibm.com>
+ *
+ * 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;
+    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
-- 
2.50.1
Re: [PATCH v5 04/29] hw/s390x/ipl: Create certificate store
Posted by Collin Walling 2 months, 1 week ago
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 <zycai@linux.ibm.com>
> ---
>  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 <zycai@linux.ibm.com>
> + *
> + * 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 <zycai@linux.ibm.com>
> + *
> + * 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
Re: [PATCH v5 04/29] hw/s390x/ipl: Create certificate store
Posted by Zhuoying Cai 2 months ago
On 9/8/25 8:54 PM, Collin Walling wrote:
> 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 <zycai@linux.ibm.com>
>> ---
>>  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 <zycai@linux.ibm.com>
>> + *
>> + * 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.
> 

Thanks for the feedback! I agree that these two fields can be removed
from S390IPLCertificate.

However, the values themselves are still needed. Since both the key ID
and fingerprint (hash value) of the certificate are SHA-256 outputs
(with the fingerprint specifically requiring it), their lengths should
always be 32 bytes.

These values are also used in this patch when calculating the
certificate data length (key ID, fingerprint, and certificate), and in
patch 9 when computing the VCE length.

Given that, it might be clearer to define them as constants rather than
storing them in the struct.

>> +    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 <zycai@linux.ibm.com>
>> + *
>> + * 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
> 
>
Re: [PATCH v5 04/29] hw/s390x/ipl: Create certificate store
Posted by Farhan Ali 2 months, 2 weeks ago
On 8/18/2025 2:42 PM, 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 <zycai@linux.ibm.com>
> ---
>   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 <zycai@linux.ibm.com>
> + *
> + * 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;
> +}
> +
> +static S390IPLCertificate *init_cert_x509(size_t size, uint8_t *raw, Error **errp)
> +{
> +    S390IPLCertificate *q_cert = NULL;
> +    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;
> +    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++;
> +}
> +

Do we need to free qcert here after we copied the contents to 
cert_store->certs[cert_store->count]? Also AFAIU we copy the certificate 
file contents into QEMU memory, but don't free it. Just wanted to 
clarify, do we need the file contents in QEMU memory for ccw-bios and 
guest kernel use? Thanks Farhan


> +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);
> +            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 <zycai@linux.ibm.com>
> + *
> + * 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;
> +    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
Re: [PATCH v5 04/29] hw/s390x/ipl: Create certificate store
Posted by Zhuoying Cai 2 months, 2 weeks ago
On 8/27/25 7:14 PM, Farhan Ali wrote:
> 
> On 8/18/2025 2:42 PM, 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 <zycai@linux.ibm.com>
>> ---
>>   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 <zycai@linux.ibm.com>
>> + *
>> + * 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;
>> +}
>> +
>> +static S390IPLCertificate *init_cert_x509(size_t size, uint8_t *raw, Error **errp)
>> +{
>> +    S390IPLCertificate *q_cert = NULL;
>> +    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;
>> +    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++;
>> +}
>> +
> 
> Do we need to free qcert here after we copied the contents to 
> cert_store->certs[cert_store->count]? Also AFAIU we copy the certificate 
> file contents into QEMU memory, but don't free it. Just wanted to 
> clarify, do we need the file contents in QEMU memory for ccw-bios and 
> guest kernel use? Thanks Farhan
> 
> 

Yes, the file contents need to remain in QEMU memory for both ccw-bios
and guest kernel use.

* ccw-bios: The certificate used to verify the component is retrieved in
the BIOS via DIAG320, with its address stored in the IPL Information
Report Block.

* guest kernel: Certificates can also be retrieved via DIAG320 and made
available to the guest kernel keyring.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8cf57d7217c32133d25615324c0ab4aaacf4d9c4

>> +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);
>> +            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 <zycai@linux.ibm.com>
>> + *
>> + * 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;
>> +    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
Re: [PATCH v5 04/29] hw/s390x/ipl: Create certificate store
Posted by Jared Rossi 2 months, 1 week ago

On 8/28/25 10:46 AM, Zhuoying Cai wrote:
> On 8/27/25 7:14 PM, Farhan Ali wrote:
>> [snip...]
>>
>> +
>> +static S390IPLCertificate *init_cert_x509(size_t size, uint8_t *raw, Error **errp)
>> +{
>> +    S390IPLCertificate *q_cert = NULL;
>> +    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;
>> +    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++;
>> +}
>> +
>> Do we need to free qcert here after we copied the contents to
>> cert_store->certs[cert_store->count]? Also AFAIU we copy the certificate
>> file contents into QEMU memory, but don't free it. Just wanted to
>> clarify, do we need the file contents in QEMU memory for ccw-bios and
>> guest kernel use? Thanks Farhan
>>
>>
> Yes, the file contents need to remain in QEMU memory for both ccw-bios
> and guest kernel use.
>
> * ccw-bios: The certificate used to verify the component is retrieved in
> the BIOS via DIAG320, with its address stored in the IPL Information
> Report Block.
>
> * guest kernel: Certificates can also be retrieved via DIAG320 and made
> available to the guest kernel keyring.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8cf57d7217c32133d25615324c0ab4aaacf4d9c4
Something still doesn't seem right to me.  As far as I can tell the cert 
store will be reinitialized each time the guest reboots, which sounds 
like it will cause problems if there is no corresponding free somewhere.

What is the expected behavior during a reboot?  Should the guest A)  
parse the cert paths again and reinitialize, or B)  read the entries in 
the previously created cert store?

If A, then the cert store needs to be cleared/freed in some way each time.

If B, then the cert store should not be reinitialized.

> [snip...]
Regards,
Jared Rossi

Re: [PATCH v5 04/29] hw/s390x/ipl: Create certificate store
Posted by Zhuoying Cai 2 months, 1 week ago
On 9/2/25 11:15 AM, Jared Rossi wrote:
> 
> 
> On 8/28/25 10:46 AM, Zhuoying Cai wrote:
>> On 8/27/25 7:14 PM, Farhan Ali wrote:
>>> [snip...]
>>>
>>> +
>>> +static S390IPLCertificate *init_cert_x509(size_t size, uint8_t *raw, Error **errp)
>>> +{
>>> +    S390IPLCertificate *q_cert = NULL;
>>> +    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;
>>> +    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++;
>>> +}
>>> +
>>> Do we need to free qcert here after we copied the contents to
>>> cert_store->certs[cert_store->count]? Also AFAIU we copy the certificate
>>> file contents into QEMU memory, but don't free it. Just wanted to
>>> clarify, do we need the file contents in QEMU memory for ccw-bios and
>>> guest kernel use? Thanks Farhan
>>>
>>>
>> Yes, the file contents need to remain in QEMU memory for both ccw-bios
>> and guest kernel use.
>>
>> * ccw-bios: The certificate used to verify the component is retrieved in
>> the BIOS via DIAG320, with its address stored in the IPL Information
>> Report Block.
>>
>> * guest kernel: Certificates can also be retrieved via DIAG320 and made
>> available to the guest kernel keyring.
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8cf57d7217c32133d25615324c0ab4aaacf4d9c4
> Something still doesn't seem right to me.  As far as I can tell the cert 
> store will be reinitialized each time the guest reboots, which sounds 
> like it will cause problems if there is no corresponding free somewhere.
> 
> What is the expected behavior during a reboot?  Should the guest A)  
> parse the cert paths again and reinitialize, or B)  read the entries in 
> the previously created cert store?
> 
> If A, then the cert store needs to be cleared/freed in some way each time.
> 
> If B, then the cert store should not be reinitialized.
> 
>> [snip...]
> Regards,
> Jared Rossi

I think option B sounds more accurate. Since the cert store can't be
modified during reboot, it's reasonable to read from the previously
created cert store.


Re: [PATCH v5 04/29] hw/s390x/ipl: Create certificate store
Posted by Jared Rossi 2 months, 2 weeks ago

On 8/18/25 5:42 PM, 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 <zycai@linux.ibm.com>
> ---
>   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 <zycai@linux.ibm.com>
> + *
> + * 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;
> +}
> +
> +static S390IPLCertificate *init_cert_x509(size_t size, uint8_t *raw, Error **errp)
> +{
> +    S390IPLCertificate *q_cert = NULL;
> +    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;
> +    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);
Missing free?

> +
> +    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);
This looks like it should return an error?

> +        }
> +    }
> +
> +    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);
> +            exit(1);
> +        }
Why not do this check before the loop, using cert_path_builder->len 
directly?

Also, I think there is a missing free in this error case?

> +
> +        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 <zycai@linux.ibm.com>
> + *
> + * 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;
> +    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,
Jared Rossi
Re: [PATCH v5 04/29] hw/s390x/ipl: Create certificate store
Posted by Zhuoying Cai 2 months, 2 weeks ago
On 8/26/25 9:40 AM, Jared Rossi wrote:
> 
> 
> On 8/18/25 5:42 PM, 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 <zycai@linux.ibm.com>
>> ---
>>   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 <zycai@linux.ibm.com>
>> + *
>> + * 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;
>> +}
>> +
>> +static S390IPLCertificate *init_cert_x509(size_t size, uint8_t *raw, Error **errp)
>> +{
>> +    S390IPLCertificate *q_cert = NULL;
>> +    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;
>> +    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);
> Missing free?
> 

Since the ownership of the buf pointer is transferred to qcert after
init_cert_x509() completes successfully, free() is not necessary here.

I'll add comments here to make things clearer.

>> +
>> +    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);
> This looks like it should return an error?
> 
>> +        }
>> +    }
>> +
>> +    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);
>> +            exit(1);
>> +        }
> Why not do this check before the loop, using cert_path_builder->len 
> directly?
> 
> Also, I think there is a missing free in this error case?
> 

Thanks for the feedback. I'll address them in the next version.

>> +
>> +        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 <zycai@linux.ibm.com>
>> + *
>> + * 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;
>> +    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,
> Jared Rossi