Re: [PATCH v6 19/28] pc-bios/s390-ccw: Add signature verification for secure IPL in audit mode

[Thomas Huth <thuth@redhat.com>]

On 18/09/2025 01.21, 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.

This is not a commit description, but documentation that persists, so "are 
introduced" sounds wrong here.

Maybe rather something like:

Multiple IPL modes are available to differentiate between the various 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.

"errors ... will report a warning" sounds weird. maybe rather: "errors ... 
will result in a warning" ?

...
> 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;

Everything is naturally aligned, no need to use packed here.

  Thomas