Re: [PATCH v1 02/24] hw/s390x/ipl: Create certificate store

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Apr 08, 2025 at 11:55:04AM -0400, Zhuoying Cai wrote:
> Create a certificate store for boot certificates used for secure IPL.
> 
> Load certificates from the -boot-certificate option into the cert store.
> 
> Currently, only x509 certificates in DER format and uses SHA-256 hashing
> algorithm are supported, as these are the types required for secure boot
> on s390.
> 
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>  hw/s390x/cert-store.c       | 249 ++++++++++++++++++++++++++++++++++++
>  hw/s390x/cert-store.h       |  50 ++++++++
>  hw/s390x/ipl.c              |   9 ++
>  hw/s390x/ipl.h              |   3 +
>  hw/s390x/meson.build        |   1 +
>  include/hw/s390x/ipl/qipl.h |   3 +
>  6 files changed, 315 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..1aa8aea040
> --- /dev/null
> +++ b/hw/s390x/cert-store.c
> @@ -0,0 +1,249 @@
> +/*
> + * 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 "qemu/error-report.h"
> +#include "qemu/option.h"
> +#include "qemu/config-file.h"
> +#include "hw/s390x/ebcdic.h"
> +#include "qemu/cutils.h"
> +#include "cert-store.h"
> +
> +#ifdef CONFIG_GNUTLS
> +#include <gnutls/x509.h>
> +#include <gnutls/gnutls.h>
> +#endif /* #define CONFIG_GNUTLS */

It is bad practice to directly use GNUTLS in any QEMU code except
for under the 'crypto/' directory (and its test suites). We must
define internal APIs for accessing the info we need and then call
those instead of gnutls.


> +
> +static const char *s390_get_boot_certificates(void)
> +{
> +    QemuOpts *opts;
> +    const char *path;
> +
> +    opts = qemu_find_opts_singleton("boot-certificates");
> +    path = qemu_opt_get(opts, "boot-certificates");
> +
> +    return path;
> +}
> +
> +static size_t cert2buf(char *path, size_t max_size, char **cert_buf)
> +{
> +    size_t size;
> +    g_autofree char *buf;
> +    buf = g_malloc(max_size);
> +
> +    if (!g_file_get_contents(path, &buf, &size, NULL) ||
> +        size == 0 || size > max_size) {
> +        return 0;
> +    }
> +
> +    *cert_buf = g_steal_pointer(&buf);
> +
> +    return size;
> +}
> +
> +#ifdef CONFIG_GNUTLS
> +int g_init_cert(uint8_t *raw_cert, size_t cert_size, gnutls_x509_crt_t *g_cert)
> +{
> +    int rc;
> +
> +    if (gnutls_x509_crt_init(g_cert) < 0) {
> +        return -1;
> +    }
> +
> +    gnutls_datum_t datum_cert = {raw_cert, cert_size};
> +    rc = gnutls_x509_crt_import(*g_cert, &datum_cert, GNUTLS_X509_FMT_DER);
> +    if (rc) {
> +        gnutls_x509_crt_deinit(*g_cert);
> +        return rc;
> +    }
> +
> +    return 0;
> +}
> +#endif /* CONFIG_GNUTLS */
> +
> +static int init_cert_x509_der(size_t size, char *raw, S390IPLCertificate **qcert)
> +{
> +#ifdef CONFIG_GNUTLS
> +    gnutls_x509_crt_t g_cert = NULL;
> +    g_autofree S390IPLCertificate *q_cert;
> +    size_t key_id_size;
> +    size_t hash_size;
> +    int rc;
> +
> +    rc = g_init_cert((uint8_t *)raw, size, &g_cert);
> +    if (rc) {
> +        if (rc == GNUTLS_E_ASN1_TAG_ERROR) {
> +            error_report("The certificate is not in DER format");
> +        }
> +        return -1;
> +    }
> +
> +    rc = gnutls_x509_crt_get_key_id(g_cert, GNUTLS_KEYID_USE_SHA256, NULL, &key_id_size);
> +    if (rc != GNUTLS_E_SHORT_MEMORY_BUFFER) {
> +        error_report("Failed to get certificate key ID size");
> +        goto out;
> +    }
> +
> +    rc = gnutls_x509_crt_get_fingerprint(g_cert, GNUTLS_DIG_SHA256, NULL, &hash_size);
> +    if (rc != GNUTLS_E_SHORT_MEMORY_BUFFER) {
> +        error_report("Failed to get certificate hash size");
> +        goto out;
> +    }


We already have qcrypto_get_x509_cert_fingerprint() to avoid direct use
of gnutls. That API could be extended to optionally also report the
key id.

> +
> +    q_cert = g_malloc(sizeof(*q_cert));
> +    q_cert->size = size;
> +    q_cert->key_id_size = key_id_size;
> +    q_cert->hash_size = hash_size;
> +    q_cert->raw = raw;
> +    q_cert->format = GNUTLS_X509_FMT_DER;
> +    *qcert = g_steal_pointer(&q_cert);
> +
> +    gnutls_x509_crt_deinit(g_cert);
> +

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|