From: Farhan Ali <alifm@linux.ibm.com>
To: Zhuoying Cai <zycai@linux.ibm.com>,
thuth@redhat.com, berrange@redhat.com,
richard.henderson@linaro.org, david@redhat.com,
jrossi@linux.ibm.com, qemu-s390x@nongnu.org,
qemu-devel@nongnu.org
Cc: walling@linux.ibm.com, jjherne@linux.ibm.com,
pasic@linux.ibm.com, borntraeger@linux.ibm.com,
farman@linux.ibm.com, mjrosato@linux.ibm.com, iii@linux.ibm.com,
eblake@redhat.com, armbru@redhat.com
Subject: Re: [PATCH v6 19/28] pc-bios/s390-ccw: Add signature verification for secure IPL in audit mode
Date: Tue, 30 Sep 2025 11:42:17 -0700 [thread overview]
Message-ID: <23e3c4f3-7e8c-4d00-8dee-91bf15261cfb@linux.ibm.com> (raw)
In-Reply-To: <20250917232131.495848-20-zycai@linux.ibm.com>
On 9/17/2025 4:21 PM, Zhuoying Cai wrote:
> Enable secure IPL in audit mode, which performs signature verification,
> but any error does not terminate the boot process. Only warnings will be
> logged to the console instead.
>
> Add a comp_len variable to store the length of a segment in
> zipl_load_segment. comp_len variable is necessary to store the
> calculated segment length and is used during signature verification.
> Return the length on success, or a negative return code on failure.
>
> Secure IPL in audit mode requires at least one certificate provided in
> the key store along with necessary facilities (Secure IPL Facility,
> Certificate Store Facility and secure IPL extension support).
>
> Note: Secure IPL in audit mode is implemented for the SCSI scheme of
> virtio-blk/virtio-scsi devices.
>
> Signed-off-by: Zhuoying Cai<zycai@linux.ibm.com>
> ---
> docs/system/s390x/secure-ipl.rst | 36 +++
> pc-bios/s390-ccw/Makefile | 3 +-
> pc-bios/s390-ccw/bootmap.c | 39 +++-
> pc-bios/s390-ccw/bootmap.h | 11 +
> pc-bios/s390-ccw/main.c | 9 +
> pc-bios/s390-ccw/s390-ccw.h | 15 ++
> pc-bios/s390-ccw/sclp.c | 44 ++++
> pc-bios/s390-ccw/sclp.h | 6 +
> pc-bios/s390-ccw/secure-ipl.c | 371 +++++++++++++++++++++++++++++++
> pc-bios/s390-ccw/secure-ipl.h | 99 +++++++++
> 10 files changed, 630 insertions(+), 3 deletions(-)
> create mode 100644 pc-bios/s390-ccw/secure-ipl.c
> create mode 100644 pc-bios/s390-ccw/secure-ipl.h
>
> diff --git a/docs/system/s390x/secure-ipl.rst b/docs/system/s390x/secure-ipl.rst
> index 92c1bb2153..701594b9de 100644
> --- a/docs/system/s390x/secure-ipl.rst
> +++ b/docs/system/s390x/secure-ipl.rst
> @@ -19,3 +19,39 @@ Note: certificate files must have a .pem extension.
> qemu-system-s390x -machine s390-ccw-virtio, \
> boot-certs.0.path=/.../qemu/certs, \
> boot-certs.1.path=/another/path/cert.pem ...
> +
> +
> +IPL Modes
> +=========
> +
> +The concept of IPL Modes are introduced to differentiate between the IPL configurations.
> +These modes are mutually exclusive and enabled based on the ``boot-certs`` option on the
> +QEMU command line.
> +
> +Normal Mode
> +-----------
> +
> +The absence of certificates will attempt to IPL a guest without secure IPL operations.
> +No checks are performed, and no warnings/errors are reported. This is the default mode.
> +
> +Configuration:
> +
> +.. code-block:: shell
> +
> + qemu-system-s390x -machine s390-ccw-virtio ...
> +
> +Audit Mode
> +----------
> +
> +With *only* the presence of certificates in the store, it is assumed that secure
> +boot operations should be performed with errors reported as warnings. As such,
> +the secure IPL operations will be performed, and any errors that stem from these
> +operations will report a warning via the SCLP console.
> +
> +Configuration:
> +
> +.. code-block:: shell
> +
> + qemu-system-s390x -machine s390-ccw-virtio, \
> + boot-certs.0.path=/.../qemu/certs, \
> + boot-certs.1.path=/another/path/cert.pem ...
> diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
> index a0f24c94a8..603761a857 100644
> --- a/pc-bios/s390-ccw/Makefile
> +++ b/pc-bios/s390-ccw/Makefile
> @@ -34,7 +34,8 @@ QEMU_DGFLAGS = -MMD -MP -MT $@ -MF $(@D)/$(*F).d
> .PHONY : all clean build-all distclean
>
> OBJECTS = start.o main.o bootmap.o jump2ipl.o sclp.o menu.o netmain.o \
> - virtio.o virtio-net.o virtio-scsi.o virtio-blkdev.o cio.o dasd-ipl.o
> + virtio.o virtio-net.o virtio-scsi.o virtio-blkdev.o cio.o dasd-ipl.o \
> + secure-ipl.o
>
> SLOF_DIR := $(SRC_PATH)/../../roms/SLOF
>
> diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
> index 4f54c643ff..3922e7cdde 100644
> --- a/pc-bios/s390-ccw/bootmap.c
> +++ b/pc-bios/s390-ccw/bootmap.c
> @@ -15,6 +15,7 @@
> #include "bootmap.h"
> #include "virtio.h"
> #include "bswap.h"
> +#include "secure-ipl.h" #ifdef DEBUG /* #define DEBUG_FALLBACK */ @@ -617,7 +618,7 @@ static
> int ipl_eckd(void) * Returns: length of the segment on sucess, *
> negative value on error. */ -static int
> zipl_load_segment(ComponentEntry *entry, uint64_t address) +int
> zipl_load_segment(ComponentEntry *entry, uint64_t address) { const int
> max_entries = (MAX_SECTOR_SIZE / sizeof(ScsiBlockPtr)); ScsiBlockPtr
> *bprs = (void *)sec; @@ -735,7 +736,19 @@ static int
> zipl_run(ScsiBlockPtr *pte) /* Load image(s) into RAM */ entry =
> (ComponentEntry *)(&header[1]); - if (zipl_run_normal(&entry,
> tmp_sec)) { + switch (boot_mode) { + case ZIPL_BOOT_MODE_SECURE_AUDIT:
> + if (zipl_run_secure(&entry, tmp_sec)) { + return -1; + } + break; +
> case ZIPL_BOOT_MODE_NORMAL: + if (zipl_run_normal(&entry, tmp_sec)) {
> + return -1; + } + break; + default: + puts("Unknown boot mode");
> return -1;
> }
>
> @@ -1101,17 +1114,35 @@ static int zipl_load_vscsi(void)
> * IPL starts here
> */
>
> +ZiplBootMode zipl_mode(uint8_t hdr_flags)
> +{
> + bool sipl_set = hdr_flags & DIAG308_IPIB_FLAGS_SIPL;
> + bool iplir_set = hdr_flags & DIAG308_IPIB_FLAGS_IPLIR;
> +
> + if (!sipl_set && iplir_set) {
> + return ZIPL_BOOT_MODE_SECURE_AUDIT;
> + }
> +
> + return ZIPL_BOOT_MODE_NORMAL;
> +}
> +
> void zipl_load(void)
> {
> VDev *vdev = virtio_get_device();
>
> if (vdev->is_cdrom) {
> + if (boot_mode == ZIPL_BOOT_MODE_SECURE_AUDIT) {
> + panic("Secure boot from ISO image is not supported!");
> + }
> ipl_iso_el_torito();
> puts("Failed to IPL this ISO image!");
> return;
> }
>
> if (virtio_get_device_type() == VIRTIO_ID_NET) {
> + if (boot_mode == ZIPL_BOOT_MODE_SECURE_AUDIT) {
> + panic("Virtio net boot device does not support secure boot!");
> + }
> netmain();
> puts("Failed to IPL from this network!");
> return;
> @@ -1122,6 +1153,10 @@ void zipl_load(void)
> return;
> }
>
> + if (boot_mode == ZIPL_BOOT_MODE_SECURE_AUDIT) {
> + panic("ECKD boot device does not support secure boot!");
> + }
> +
> switch (virtio_get_device_type()) {
> case VIRTIO_ID_BLOCK:
> zipl_load_vblk();
> diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
> index 95943441d3..90fd530256 100644
> --- a/pc-bios/s390-ccw/bootmap.h
> +++ b/pc-bios/s390-ccw/bootmap.h
> @@ -88,9 +88,18 @@ typedef struct BootMapTable {
> BootMapPointer entry[];
> } __attribute__ ((packed)) BootMapTable;
>
> +#define DER_SIGNATURE_FORMAT 1
> +
> +typedef struct SignatureInformation {
> + uint8_t format;
> + uint8_t reserved[3];
> + uint32_t sig_len;
> +} __attribute__((packed)) SignatureInformation;
> +
> typedef union ComponentEntryData {
> uint64_t load_psw;
> uint64_t load_addr;
> + SignatureInformation sig_info;
> } ComponentEntryData;
>
> typedef struct ComponentEntry {
> @@ -113,6 +122,8 @@ typedef struct ScsiMbr {
> ScsiBlockPtr pt; /* block pointer to program table */
> } __attribute__ ((packed)) ScsiMbr;
>
> +int zipl_load_segment(ComponentEntry *entry, uint64_t address);
> +
> #define ZIPL_MAGIC "zIPL"
> #define ZIPL_MAGIC_EBCDIC "\xa9\xc9\xd7\xd3"
> #define IPL1_MAGIC "\xc9\xd7\xd3\xf1" /* == "IPL1" in EBCDIC */
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index c9328f1c51..668660e64d 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -28,6 +28,7 @@ IplParameterBlock *iplb;
> bool have_iplb;
> static uint16_t cutype;
> LowCore *lowcore; /* Yes, this *is* a pointer to address 0 */
> +ZiplBootMode boot_mode;
>
> #define LOADPARM_PROMPT "PROMPT "
> #define LOADPARM_EMPTY " "
> @@ -272,9 +273,17 @@ static int virtio_setup(void)
>
> static void ipl_boot_device(void)
> {
> + if (boot_mode == ZIPL_BOOT_MODE_UNSPECIFIED) {
> + boot_mode = zipl_mode(iplb->hdr_flags);
> + }
> +
> switch (cutype) {
> case CU_TYPE_DASD_3990:
> case CU_TYPE_DASD_2107:
> + if (boot_mode == ZIPL_BOOT_MODE_SECURE_AUDIT) {
> + panic("Passthrough (vfio) device does not support secure boot!");
Nit: We could be more specific here and mention "Passthrough (vfio) CCW
device...". I know we don't support any other vfio device for booting
today but just makes it clear what kind of passthrough device we are
dealing with.
> + }
> +
> dasd_ipl(blk_schid, cutype);
> break;
> case CU_TYPE_VIRTIO:
> diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
> index b1dc35cded..c2ba40d067 100644
> --- a/pc-bios/s390-ccw/s390-ccw.h
> +++ b/pc-bios/s390-ccw/s390-ccw.h
> @@ -39,6 +39,9 @@ typedef unsigned long long u64;
> #define MIN_NON_ZERO(a, b) ((a) == 0 ? (b) : \
> ((b) == 0 ? (a) : (MIN(a, b))))
> #endif
> +#ifndef ROUND_UP
> +#define ROUND_UP(n, d) (((n) + (d) - 1) & -(0 ? (n) : (d)))
> +#endif
>
> #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
>
> @@ -64,6 +67,8 @@ void sclp_print(const char *string);
> void sclp_set_write_mask(uint32_t receive_mask, uint32_t send_mask);
> void sclp_setup(void);
> void sclp_get_loadparm_ascii(char *loadparm);
> +bool sclp_is_diag320_on(void);
> +bool sclp_is_sipl_on(void);
> int sclp_read(char *str, size_t count);
>
> /* virtio.c */
> @@ -76,6 +81,16 @@ int virtio_read(unsigned long sector, void *load_addr);
> /* bootmap.c */
> void zipl_load(void);
>
> +typedef enum ZiplBootMode {
> + ZIPL_BOOT_MODE_UNSPECIFIED = 0,
> + ZIPL_BOOT_MODE_NORMAL = 1,
> + ZIPL_BOOT_MODE_SECURE_AUDIT = 2,
> +} ZiplBootMode;
> +
> +extern ZiplBootMode boot_mode;
> +
> +ZiplBootMode zipl_mode(uint8_t hdr_flags);
> +
> /* jump2ipl.c */
> void write_reset_psw(uint64_t psw);
> int jump_to_IPL_code(uint64_t address);
> diff --git a/pc-bios/s390-ccw/sclp.c b/pc-bios/s390-ccw/sclp.c
> index 4a07de018d..0b03c3164f 100644
> --- a/pc-bios/s390-ccw/sclp.c
> +++ b/pc-bios/s390-ccw/sclp.c
> @@ -113,6 +113,50 @@ void sclp_get_loadparm_ascii(char *loadparm)
> }
> }
>
> +static void sclp_get_fac134(uint8_t *fac134)
> +{
> +
> + ReadInfo *sccb = (void *)_sccb;
> +
> + memset((char *)_sccb, 0, sizeof(ReadInfo));
> + sccb->h.length = SCCB_SIZE;
> + if (!sclp_service_call(SCLP_CMDW_READ_SCP_INFO, sccb)) {
> + *fac134 = sccb->fac134;
> + }
> +}
> +
> +bool sclp_is_diag320_on(void)
> +{
> + uint8_t fac134 = 0;
> +
> + sclp_get_fac134(&fac134);
> + return fac134 & SCCB_FAC134_DIAG320_BIT;
> +}
> +
> +/*
> + * Get fac_ipl (byte 136 and byte 137 of the SCLP Read Info block)
> + * for IPL device facilities.
> + */
> +static void sclp_get_fac_ipl(uint16_t *fac_ipl)
> +{
> +
> + ReadInfo *sccb = (void *)_sccb;
> +
> + memset((char *)_sccb, 0, sizeof(ReadInfo));
> + sccb->h.length = SCCB_SIZE;
> + if (!sclp_service_call(SCLP_CMDW_READ_SCP_INFO, sccb)) {
> + *fac_ipl = sccb->fac_ipl;
> + }
> +}
> +
> +bool sclp_is_sipl_on(void)
> +{
> + uint16_t fac_ipl = 0;
> +
> + sclp_get_fac_ipl(&fac_ipl);
> + return fac_ipl & SCCB_FAC_IPL_SIPL_BIT;
> +}
> +
> int sclp_read(char *str, size_t count)
> {
> ReadEventData *sccb = (void *)_sccb;
> diff --git a/pc-bios/s390-ccw/sclp.h b/pc-bios/s390-ccw/sclp.h
> index 64b53cad29..cf147f4634 100644
> --- a/pc-bios/s390-ccw/sclp.h
> +++ b/pc-bios/s390-ccw/sclp.h
> @@ -50,6 +50,8 @@ typedef struct SCCBHeader {
> } __attribute__((packed)) SCCBHeader;
>
> #define SCCB_DATA_LEN (SCCB_SIZE - sizeof(SCCBHeader))
> +#define SCCB_FAC134_DIAG320_BIT 0x4
> +#define SCCB_FAC_IPL_SIPL_BIT 0x4000
>
> typedef struct ReadInfo {
> SCCBHeader h;
> @@ -57,6 +59,10 @@ typedef struct ReadInfo {
> uint8_t rnsize;
> uint8_t reserved[13];
> uint8_t loadparm[LOADPARM_LEN];
> + uint8_t reserved1[102];
> + uint8_t fac134;
> + uint8_t reserved2;
> + uint16_t fac_ipl;
> } __attribute__((packed)) ReadInfo;
>
> typedef struct SCCB {
> diff --git a/pc-bios/s390-ccw/secure-ipl.c b/pc-bios/s390-ccw/secure-ipl.c
> new file mode 100644
> index 0000000000..8eab19cb09
> --- /dev/null
> +++ b/pc-bios/s390-ccw/secure-ipl.c
> @@ -0,0 +1,371 @@
> +/*
> + * S/390 Secure IPL
> + *
> + * Functions to support IPL in secure boot mode (DIAG 320, DIAG 508,
> + * signature verification, and certificate handling).
> + *
> + * For secure IPL overview: docs/system/s390x/secure-ipl.rst
> + * For secure IPL technical: docs/specs/s390x-secure-ipl.rst
> + *
> + * Copyright 2025 IBM Corp.
> + * Author(s): Zhuoying Cai<zycai@linux.ibm.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include <stdlib.h>
> +#include <string.h>
> +#include <stdio.h>
> +#include "bootmap.h"
> +#include "s390-ccw.h"
> +#include "secure-ipl.h"
> +
> +uint8_t vcssb_data[VCSSB_MIN_LEN] __attribute__((__aligned__(PAGE_SIZE)));
> +
> +VCStorageSizeBlock *zipl_secure_get_vcssb(void)
> +{
> + VCStorageSizeBlock *vcssb;
> + int rc;
> +
> + if (!(sclp_is_diag320_on() && is_cert_store_facility_supported())) {
Should we fail if either diag320 or certificate store is not available?
Might make the check simpler
> + puts("Certificate Store Facility is not supported by the hypervisor!");
> + return NULL;
> + }
> +
> + vcssb = (VCStorageSizeBlock *)vcssb_data;
> + /* avoid retrieving vcssb multiple times */
> + if (vcssb->length >= VCSSB_MIN_LEN) {
> + return vcssb;
> + }
> +
> + vcssb->length = VCSSB_MIN_LEN;
> + rc = diag320(vcssb, DIAG_320_SUBC_QUERY_VCSI);
> + if (rc != DIAG_320_RC_OK) {
> + return NULL;
> + }
> +
> + return vcssb;
> +}
> +
> +static uint32_t get_certs_length(void)
> +{
> + VCStorageSizeBlock *vcssb;
> + uint32_t len;
> +
> + vcssb = zipl_secure_get_vcssb();
> + if (vcssb == NULL) {
> + return 0;
> + }
> +
> + len = vcssb->total_vcb_len - VCB_HEADER_LEN - vcssb->total_vc_ct * VCE_HEADER_LEN;
> +
> + return len;
> +}
> +
> +static uint32_t request_certificate(uint8_t *cert, uint8_t index)
> +{
> + VCStorageSizeBlock *vcssb;
> + VCBlock *vcb;
> + VCEntry *vce;
> + uint64_t rc = 0;
> + uint32_t cert_len = 0;
> +
> + /* Get Verification Certificate Storage Size block with DIAG320 subcode 1 */
> + vcssb = zipl_secure_get_vcssb();
> + if (vcssb == NULL) {
> + return 0;
> + }
> +
> + /*
> + * Request single entry
> + * Fill input fields of single-entry VCB
> + */
> + vcb = malloc(MAX_SECTOR_SIZE * 4);
Why are we using MAX_SECTOR _SIZE? Shouldn't we use max_single_vcb_len
to allocate the memory for vcb?
> + vcb->in_len = ROUND_UP(vcssb->max_single_vcb_len, PAGE_SIZE);
> + vcb->first_vc_index = index + 1;
> + vcb->last_vc_index = index + 1;
> +
> + rc = diag320(vcb, DIAG_320_SUBC_STORE_VC);
> + if (rc == DIAG_320_RC_OK) {
Nit: We could just check if rc != DIAG_320_RC_OK and goto out. This way
we avoid a bigger if block.
> + if (vcb->out_len == VCB_HEADER_LEN) {
> + puts("No certificate entry");
> + goto out;
> + }
> + if (vcb->remain_ct != 0) {
> + puts("Not enough memory to store all requested certificates");
> + goto out;
> + }
> +
> + vce = (VCEntry *)vcb->vce_buf;
> + if (!is_vce_cert_valid(vce->flags, vce->len)) {
> + puts("Invalid certificate");
> + goto out;
> + }
> +
> + cert_len = vce->cert_len;
> + memcpy(cert, (uint8_t *)vce + vce->cert_offset, vce->cert_len);
> + }
> +
> +out:
> + free(vcb);
> + return cert_len;
> +}
> +
> +static void cert_list_add(IplSignatureCertificateList *certs, int cert_index,
> + uint8_t *cert, uint64_t cert_len)
> +{
> + if (cert_index > MAX_CERTIFICATES - 1) {
> + printf("Warning: Ignoring cert entry [%d] because it's over %d entires\n",
> + cert_index + 1, MAX_CERTIFICATES);
> + return;
> + }
> +
> + certs->cert_entries[cert_index].addr = (uint64_t)cert;
> + certs->cert_entries[cert_index].len = cert_len;
> + certs->ipl_info_header.len += sizeof(certs->cert_entries[cert_index]);
> +}
> +
> +static void comp_list_add(IplDeviceComponentList *comps, int comp_index,
> + int cert_index, uint64_t comp_addr,
> + uint64_t comp_len, uint8_t flags)
> +{
> + if (comp_index > MAX_CERTIFICATES - 1) {
> + printf("Warning: Ignoring comp entry [%d] because it's over %d entires\n",
> + comp_index + 1, MAX_CERTIFICATES);
> + return;
> + }
> +
> + comps->device_entries[comp_index].addr = comp_addr;
> + comps->device_entries[comp_index].len = comp_len;
> + comps->device_entries[comp_index].flags = flags;
> + comps->device_entries[comp_index].cert_index = cert_index;
> + comps->ipl_info_header.len += sizeof(comps->device_entries[comp_index]);
> +}
> +
> +static int update_iirb(IplDeviceComponentList *comps, IplSignatureCertificateList *certs)
> +{
> + IplInfoReportBlock *iirb;
> + IplDeviceComponentList *iirb_comps;
> + IplSignatureCertificateList *iirb_certs;
> + uint32_t iirb_hdr_len;
> + uint32_t comps_len;
> + uint32_t certs_len;
> +
> + if (iplb->len % 8 != 0) {
> + panic("IPL parameter block length field value is not multiple of 8 bytes");
> + }
> +
> + iirb_hdr_len = sizeof(IplInfoReportBlockHeader);
> + comps_len = comps->ipl_info_header.len;
> + certs_len = certs->ipl_info_header.len;
> + if ((comps_len + certs_len + iirb_hdr_len) > sizeof(IplInfoReportBlock)) {
> + puts("Not enough space to hold all components and certificates in IIRB");
> + return -1;
> + }
> +
> + /* IIRB immediately follows IPLB */
> + iirb = &ipl_data.iirb;
> + iirb->hdr.len = iirb_hdr_len;
> +
> + /* Copy IPL device component list after IIRB Header */
> + iirb_comps = (IplDeviceComponentList *) iirb->info_blks;
> + memcpy(iirb_comps, comps, comps_len);
> +
> + /* Update IIRB length */
> + iirb->hdr.len += comps_len;
> +
> + /* Copy IPL sig cert list after IPL device component list */
> + iirb_certs = (IplSignatureCertificateList *) (iirb->info_blks +
> + iirb_comps->ipl_info_header.len);
> + memcpy(iirb_certs, certs, certs_len);
> +
> + /* Update IIRB length */
> + iirb->hdr.len += certs_len;
> +
> + return 0;
> +}
> +
> +static bool secure_ipl_supported(void)
> +{
> + if (!sclp_is_sipl_on()) {
> + puts("Secure IPL Facility is not supported by the hypervisor!");
> + return false;
> + }
> +
> + if (!is_secure_ipl_extension_supported()) {
> + puts("Secure IPL extensions are not supported by the hypervisor!");
> + return false;
> + }
> +
> + if (!(sclp_is_diag320_on() && is_cert_store_facility_supported())) {
> + puts("Certificate Store Facility is not supported by the hypervisor!");
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static void init_lists(IplDeviceComponentList *comps, IplSignatureCertificateList *certs)
> +{
> + comps->ipl_info_header.ibt = IPL_IBT_COMPONENTS;
> + comps->ipl_info_header.len = sizeof(comps->ipl_info_header);
> +
> + certs->ipl_info_header.ibt = IPL_IBT_CERTIFICATES;
> + certs->ipl_info_header.len = sizeof(certs->ipl_info_header);
> +}
> +
> +static uint32_t zipl_load_signature(ComponentEntry *entry, uint64_t sig_sec)
> +{
> + uint32_t sig_len;
> +
> + if (zipl_load_segment(entry, sig_sec) < 0) {
> + return -1;
> + }
> +
> + if (entry->compdat.sig_info.format != DER_SIGNATURE_FORMAT) {
> + puts("Signature is not in DER format");
> + return -1;
> + }
> + sig_len = entry->compdat.sig_info.sig_len;
> +
> + return sig_len;
> +}
> +
> +static int handle_certificate(int *cert_table, uint8_t **cert,
> + uint64_t cert_len, uint8_t cert_idx,
> + IplSignatureCertificateList *certs, int cert_index)
> +{
> + bool unused;
> +
> + unused = cert_table[cert_idx] == -1;
> + if (unused) {
> + if (request_certificate(*cert, cert_idx)) {
> + cert_list_add(certs, cert_index, *cert, cert_len);
> + cert_table[cert_idx] = cert_index;
> + *cert += cert_len;
It's hard to understand why we increment *cert in this function by just
looking at the function. But this function is called in the loop in
zipl_run_secure, could we move this entire function in zipl_run_secure?
> + } else {
> + puts("Could not get certificate");
> + return -1;
> + }
> +
> + /* increment cert_index for the next cert entry */
> + return ++cert_index;
> + }
> +
> + return cert_index;
> +}
> +
> +int zipl_run_secure(ComponentEntry **entry_ptr, uint8_t *tmp_sec)
> +{
> + IplDeviceComponentList comps;
> + IplSignatureCertificateList certs;
> + ComponentEntry *entry = *entry_ptr;
> + uint8_t *cert = NULL;
> + uint64_t *sig = NULL;
> + int cert_index = 0;
> + int comp_index = 0;
> + uint64_t comp_addr;
> + int comp_len;
> + uint32_t sig_len = 0;
> + uint64_t cert_len = -1;
> + uint8_t cert_idx = -1;
Why do we have 2 variables (cert_idx, cert_index) that are named
similar? Can we rename cert_index to cert_table_idx?
> + bool verified;
> + uint32_t certs_len;
> + /*
> + * Store indices of cert entry that have already used for signature verification
> + * to prevent allocating the same certificate multiple times.
> + * cert_table index: index of certificate from qemu cert store used for verification
> + * cert_table value: index of cert entry in cert list that contains the certificate
> + */
> + int cert_table[MAX_CERTIFICATES] = { [0 ... MAX_CERTIFICATES - 1] = -1};
> + int signed_count = 0;
> +
> + if (!secure_ipl_supported()) {
> + return -1;
> + }
> +
> + init_lists(&comps, &certs);
> + certs_len = get_certs_length();
> + cert = malloc(certs_len);
> + sig = malloc(MAX_SECTOR_SIZE);
> +
> + while (entry->component_type != ZIPL_COMP_ENTRY_EXEC) {
> + switch (entry->component_type) {
> + case ZIPL_COMP_ENTRY_SIGNATURE:
> + if (sig_len) {
> + goto out;
> + }
> +
> + sig_len = zipl_load_signature(entry, (uint64_t)sig);
> + if (sig_len < 0) {
> + goto out;
> + }
> + break;
> + case ZIPL_COMP_ENTRY_LOAD:
> + comp_addr = entry->compdat.load_addr;
> + comp_len = zipl_load_segment(entry, comp_addr);
> + if (comp_len < 0) {
> + goto out;
> + }
> +
> + if (!sig_len) {
> + break;
> + }
> +
> + verified = verify_signature(comp_len, comp_addr, sig_len, (uint64_t)sig,
> + &cert_len, &cert_idx);
> +
> + if (verified) {
> + cert_index = handle_certificate(cert_table, &cert, cert_len, cert_idx,
> + &certs, cert_index);
> + if (cert_index == -1) {
> + goto out;
> + }
> +
> + puts("Verified component");
> + comp_list_add(&comps, comp_index, cert_table[cert_idx],
> + comp_addr, comp_len,
> + S390_IPL_COMPONENT_FLAG_SC | S390_IPL_COMPONENT_FLAG_CSV);
> + } else {
> + comp_list_add(&comps, comp_index, -1,
> + comp_addr, comp_len,
> + S390_IPL_COMPONENT_FLAG_SC);
> + zipl_secure_handle("Could not verify component");
> + }
> +
> + comp_index++;
> + signed_count += 1;
> + /* After a signature is used another new one can be accepted */
> + sig_len = 0;
> + break;
> + default:
> + puts("Unknown component entry type");
> + return -1;
> + }
> +
> + entry++;
> +
> + if ((uint8_t *)(&entry[1]) > tmp_sec + MAX_SECTOR_SIZE) {
> + puts("Wrong entry value");
> + return -EINVAL;
> + }
> + }
> +
> + if (signed_count == 0) {
> + zipl_secure_handle("Secure boot is on, but components are not signed");
> + }
> +
> + if (update_iirb(&comps, &certs)) {
> + zipl_secure_handle("Failed to write IPL Information Report Block");
> + }
> +
> + *entry_ptr = entry;
> + free(sig);
> +
> + return 0;
> +out:
> + free(cert);
> + free(sig);
> +
> + return -1;
> +}
> diff --git a/pc-bios/s390-ccw/secure-ipl.h b/pc-bios/s390-ccw/secure-ipl.h
> new file mode 100644
> index 0000000000..a264a44349
> --- /dev/null
> +++ b/pc-bios/s390-ccw/secure-ipl.h
> @@ -0,0 +1,99 @@
> +/*
> + * S/390 Secure IPL
> + *
> + * Copyright 2025 IBM Corp.
> + * Author(s): Zhuoying Cai<zycai@linux.ibm.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef _PC_BIOS_S390_CCW_SECURE_IPL_H
> +#define _PC_BIOS_S390_CCW_SECURE_IPL_H
> +
> +#include <diag320.h>
> +#include <diag508.h>
> +
> +VCStorageSizeBlock *zipl_secure_get_vcssb(void);
> +int zipl_run_secure(ComponentEntry **entry_ptr, uint8_t *tmp_sec);
> +
> +static inline void zipl_secure_handle(const char *message)
> +{
> + switch (boot_mode) {
> + case ZIPL_BOOT_MODE_SECURE_AUDIT:
> + IPL_check(false, message);
> + break;
> + default:
> + break;
> + }
> +}
> +
> +static inline uint64_t diag320(void *data, unsigned long subcode)
> +{
> + register unsigned long addr asm("0") = (unsigned long)data;
> + register unsigned long rc asm("1") = 0;
> +
> + asm volatile ("diag %0,%2,0x320\n"
> + : "+d" (addr), "+d" (rc)
> + : "d" (subcode)
> + : "memory", "cc");
> + return rc;
> +}
> +
> +static inline bool is_vce_cert_valid(uint8_t vce_flags, uint32_t vce_len)
> +{
> + return (vce_flags & DIAG_320_VCE_FLAGS_VALID) && (vce_len > VCE_INVALID_LEN);
Shouldn't just checking the vce_flag be enough?
> +}
> +
> +static inline bool is_cert_store_facility_supported(void)
> +{
> + uint32_t d320_ism;
> +
> + diag320(&d320_ism, DIAG_320_SUBC_QUERY_ISM);
> + return (d320_ism & DIAG_320_ISM_QUERY_SUBCODES) &&
> + (d320_ism & DIAG_320_ISM_QUERY_VCSI) &&
> + (d320_ism & DIAG_320_ISM_STORE_VC);
> +}
> +
> +static inline uint64_t _diag508(void *data, unsigned long subcode)
> +{
> + register unsigned long addr asm("0") = (unsigned long)data;
> + register unsigned long rc asm("1") = 0;
> +
> + asm volatile ("diag %0,%2,0x508\n"
> + : "+d" (addr), "+d" (rc)
> + : "d" (subcode)
> + : "memory", "cc");
> + return rc;
> +}
> +
> +static inline bool is_secure_ipl_extension_supported(void)
> +{
> + uint64_t d508_subcodes;
> +
> + d508_subcodes = _diag508(NULL, DIAG_508_SUBC_QUERY_SUBC);
> + return d508_subcodes & DIAG_508_SUBC_SIG_VERIF;
> +}
> +
> +static inline bool verify_signature(uint64_t comp_len, uint64_t comp_addr,
> + uint64_t sig_len, uint64_t sig_addr,
> + uint64_t *cert_len, uint8_t *cert_idx)
> +{
> + Diag508SigVerifBlock svb;
> +
> + svb.length = sizeof(Diag508SigVerifBlock);
> + svb.version = 0;
> + svb.comp_len = comp_len;
> + svb.comp_addr = comp_addr;
> + svb.sig_len = sig_len;
> + svb.sig_addr = sig_addr;
> +
> + if (_diag508(&svb, DIAG_508_SUBC_SIG_VERIF) == DIAG_508_RC_OK) {
> + *cert_len = svb.cert_len;
> + *cert_idx = svb.cert_store_index;
> + return true;
> + }
> +
> + return false;
> +}
> +
> +#endif /* _PC_BIOS_S390_CCW_SECURE_IPL_H */
next prev parent reply other threads:[~2025-09-30 18:44 UTC|newest]
Thread overview: 89+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-17 23:21 [PATCH v6 00/28] Secure IPL Support for SCSI Scheme of virtio-blk/virtio-scsi Devices Zhuoying Cai
2025-09-17 23:21 ` [PATCH v6 01/28] Add boot-certs to s390-ccw-virtio machine type option Zhuoying Cai
2025-09-18 6:56 ` Markus Armbruster
2025-09-18 8:38 ` Daniel P. Berrangé
2025-09-18 8:51 ` Markus Armbruster
2025-09-23 1:31 ` Zhuoying Cai
2025-09-22 23:48 ` Zhuoying Cai
2025-09-29 18:29 ` Collin Walling
2025-10-08 17:49 ` Zhuoying Cai
2025-09-30 9:34 ` Thomas Huth
2025-09-30 9:37 ` Daniel P. Berrangé
2025-09-30 9:43 ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 02/28] crypto/x509-utils: Refactor with GNUTLS fallback Zhuoying Cai
2025-09-18 18:14 ` Farhan Ali
2025-09-30 9:38 ` Thomas Huth
2025-10-02 13:23 ` Daniel P. Berrangé
2025-09-17 23:21 ` [PATCH v6 03/28] crypto/x509-utils: Add helper functions for certificate store Zhuoying Cai
2025-09-18 18:24 ` Farhan Ali
2025-09-30 9:43 ` Thomas Huth
2025-10-02 13:24 ` Daniel P. Berrangé
2025-09-17 23:21 ` [PATCH v6 04/28] hw/s390x/ipl: Create " Zhuoying Cai
2025-09-18 19:46 ` Farhan Ali
2025-09-30 10:26 ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 05/28] s390x/diag: Introduce DIAG 320 for Certificate Store Facility Zhuoying Cai
2025-09-18 20:07 ` Farhan Ali
2025-09-30 13:08 ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 06/28] s390x/diag: Refactor address validation check from diag308_parm_check Zhuoying Cai
2025-09-18 20:38 ` Farhan Ali
2025-09-30 13:13 ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 07/28] s390x/diag: Implement DIAG 320 subcode 1 Zhuoying Cai
2025-09-19 17:20 ` Farhan Ali
2025-09-30 13:30 ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 08/28] crypto/x509-utils: Add helper functions for DIAG 320 subcode 2 Zhuoying Cai
2025-09-19 18:02 ` Farhan Ali
2025-10-07 9:34 ` Thomas Huth
2025-10-07 9:38 ` Daniel P. Berrangé
2025-10-07 9:41 ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 09/28] s390x/diag: Implement " Zhuoying Cai
2025-09-24 21:53 ` Farhan Ali
2025-09-26 13:42 ` Zhuoying Cai
2025-09-17 23:21 ` [PATCH v6 10/28] s390x/diag: Introduce DIAG 508 for secure IPL operations Zhuoying Cai
2025-09-25 20:50 ` Farhan Ali
2025-10-07 9:47 ` Thomas Huth
2025-10-07 19:46 ` Collin Walling
2025-09-17 23:21 ` [PATCH v6 11/28] crypto/x509-utils: Add helper functions for DIAG 508 subcode 1 Zhuoying Cai
2025-10-07 9:58 ` Thomas Huth
2025-10-07 10:10 ` Daniel P. Berrangé
2025-09-17 23:21 ` [PATCH v6 12/28] s390x/diag: Implement DIAG 508 subcode 1 for signature verification Zhuoying Cai
2025-09-25 21:30 ` Farhan Ali
2025-10-07 10:27 ` Thomas Huth
2025-10-10 16:37 ` Zhuoying Cai
2025-10-10 18:08 ` Thomas Huth
2025-10-07 20:22 ` Collin Walling
2025-09-17 23:21 ` [PATCH v6 13/28] pc-bios/s390-ccw: Introduce IPL Information Report Block (IIRB) Zhuoying Cai
2025-09-25 22:02 ` Farhan Ali
2025-09-17 23:21 ` [PATCH v6 14/28] pc-bios/s390-ccw: Define memory for IPLB and convert IPLB to pointers Zhuoying Cai
2025-09-30 5:17 ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 15/28] hw/s390x/ipl: Add IPIB flags to IPL Parameter Block Zhuoying Cai
2025-09-29 21:21 ` Farhan Ali
2025-09-17 23:21 ` [PATCH v6 16/28] s390x: Guest support for Secure-IPL Facility Zhuoying Cai
2025-09-17 23:21 ` [PATCH v6 17/28] pc-bios/s390-ccw: Refactor zipl_run() Zhuoying Cai
2025-09-26 12:51 ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 18/28] pc-bios/s390-ccw: Rework zipl_load_segment function Zhuoying Cai
2025-09-26 13:02 ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 19/28] pc-bios/s390-ccw: Add signature verification for secure IPL in audit mode Zhuoying Cai
2025-09-26 13:10 ` Thomas Huth
2025-09-30 18:42 ` Farhan Ali [this message]
2025-10-10 18:00 ` Zhuoying Cai
2025-10-10 19:37 ` Farhan Ali
2025-09-17 23:21 ` [PATCH v6 20/28] s390x: Guest support for Secure-IPL Code Loading Attributes Facility (SCLAF) Zhuoying Cai
2025-09-29 12:25 ` Thomas Huth
2025-09-30 13:06 ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 21/28] pc-bios/s390-ccw: Add additional security checks for secure boot Zhuoying Cai
2025-09-29 13:30 ` Thomas Huth
2025-09-29 20:43 ` Zhuoying Cai
2025-09-30 5:14 ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 22/28] Add secure-boot to s390-ccw-virtio machine type option Zhuoying Cai
2025-09-29 14:05 ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 23/28] hw/s390x/ipl: Set IPIB flags for secure IPL Zhuoying Cai
2025-09-17 23:21 ` [PATCH v6 24/28] pc-bios/s390-ccw: Handle true secure IPL mode Zhuoying Cai
2025-09-29 15:24 ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 25/28] pc-bios/s390-ccw: Handle secure boot with multiple boot devices Zhuoying Cai
2025-09-29 18:11 ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 26/28] hw/s390x/ipl: Handle secure boot without specifying a boot device Zhuoying Cai
2025-09-17 23:21 ` [PATCH v6 27/28] docs/specs: Add secure IPL documentation Zhuoying Cai
2025-10-07 11:40 ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 28/28] docs/system/s390x: " Zhuoying Cai
2025-09-29 18:23 ` Thomas Huth
2025-09-26 12:38 ` [PATCH v6 00/28] Secure IPL Support for SCSI Scheme of virtio-blk/virtio-scsi Devices Thomas Huth
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=23e3c4f3-7e8c-4d00-8dee-91bf15261cfb@linux.ibm.com \
--to=alifm@linux.ibm.com \
--cc=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=borntraeger@linux.ibm.com \
--cc=david@redhat.com \
--cc=eblake@redhat.com \
--cc=farman@linux.ibm.com \
--cc=iii@linux.ibm.com \
--cc=jjherne@linux.ibm.com \
--cc=jrossi@linux.ibm.com \
--cc=mjrosato@linux.ibm.com \
--cc=pasic@linux.ibm.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-s390x@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=thuth@redhat.com \
--cc=walling@linux.ibm.com \
--cc=zycai@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).