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

[Zhuoying Cai <zycai@linux.ibm.com>]

On 3/16/26 11:41 PM, Collin Walling wrote:
> On 3/5/26 17:41, 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 |  35 +++
>>  pc-bios/s390-ccw/Makefile        |   3 +-
>>  pc-bios/s390-ccw/bootmap.c       |  36 +++-
>>  pc-bios/s390-ccw/bootmap.h       |  11 +
>>  pc-bios/s390-ccw/main.c          |   6 +
>>  pc-bios/s390-ccw/s390-ccw.h      |  27 +++
>>  pc-bios/s390-ccw/sclp.c          |  38 ++++
>>  pc-bios/s390-ccw/sclp.h          |   6 +
>>  pc-bios/s390-ccw/secure-ipl.c    | 357 +++++++++++++++++++++++++++++++
>>  pc-bios/s390-ccw/secure-ipl.h    | 102 +++++++++
>>  10 files changed, 618 insertions(+), 3 deletions(-)
>>  create mode 100644 pc-bios/s390-ccw/secure-ipl.c
>>  create mode 100644 pc-bios/s390-ccw/secure-ipl.h
>>

[...]

>> +
>> +int zipl_run_secure(ComponentEntry **entry_ptr, uint8_t *tmp_sec)
>> +{
>> +    IplDeviceComponentList comp_list = { 0 };
>> +    IplSignatureCertificateList cert_list = { 0 };
>> +    ComponentEntry *entry = *entry_ptr;
>> +    uint8_t *cert_addr = NULL;
>> +    uint64_t *sig = NULL;
>> +    int cert_entry_idx = 0;
>> +    int comp_entry_idx = 0;
>> +    uint64_t comp_addr;
>> +    int comp_len;
>> +    uint32_t sig_len = 0;
>> +    uint64_t cert_len = -1;
>> +    uint8_t cert_table_idx = -1;
>> +    int cert_index;
>> +    uint8_t flags;
>> +    bool verified;
>> +    /*
>> +     * Keep track of which certificate store indices correspond to the
>> +     * certificate data entries within the IplSignatureCertificateList to
>> +     * prevent allocating space for the same certificate multiple times.
>> +     *
>> +     * The array index corresponds to the certificate's cert-store index.
>> +     *
>> +     * The array value corresponds to the certificate's entry within the
>> +     * IplSignatureCertificateList (with a value of -1 denoting no entry
>> +     * exists for the certificate).
>> +     */
>> +    int cert_list_table[MAX_CERTIFICATES] = { [0 ... MAX_CERTIFICATES - 1] = -1 };
>> +    int signed_count = 0;
>> +
>> +    if (!secure_ipl_supported()) {
>> +        panic("Unable to boot in secure/audit mode");
>> +    }
>> +
>> +    init_lists(&comp_list, &cert_list);
>> +    cert_addr = malloc(get_total_certs_length());
> 
> I'm concerned about the cert_addr being malloc'd here...
> 
>> +    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_table_idx);
>> +
>> +            /* default cert index and flags for unverified component */
>> +            cert_index = -1;
>> +            flags = S390_IPL_DEV_COMP_FLAG_SC;
>> +
>> +            if (verified) {
>> +                if (cert_list_table[cert_table_idx] == -1) {
>> +                    if (!request_certificate(cert_addr, cert_table_idx)) {
>> +                        puts("Could not get certificate");
>> +                        goto out;
>> +                    }
>> +
>> +                    cert_list_table[cert_table_idx] = cert_entry_idx;
>> +                    cert_list_add(&cert_list, cert_entry_idx, cert_addr, cert_len);
> 
> ...then the malloc'd cert is added to the cert list (and this list later
> gets appended to the IIRB).
> 
> Once IPL starts, the data at this address will eventually be
> lost/overwritten. The certificate address needs to point to either:
>  - somewhere on the disk
>  - some dedicated memory space (likely designated by QEMU)
> 
> Though that begs the question: who/what makes use of the IIRB?  It seems
> this information gets lost?
> 

Thanks for raising the concern. I looked into this further.

The IIRB is actively consumed by the kernel, and its contents are
preserved until they are explicitly copied.

It is used for:
* Boot logging: log_component_list() reports what was loaded and verified.
* kexec operations: kexec_file_add_ipl_report() builds the next kernel’s
IPL report from the existing one.
* Keying: load_ipl_certs() installs IPL certificates into the platform
trusted keyring.

In addition, the IPL report block data is not lost during boot (see
arch/s390/boot/ipl_report.c).

Early during boot, read_ipl_report() reserves the entire IPL report
memory region, including the IPL parameter block and the IPL report
itself. Certificate entries reference the actual certificate data via
physical addresses, and all subsequent physical memory allocations are
checked for overlap using ipl_report_certs_intersects(). This ensures
that no allocation can overwrite certificate data before it is saved.

Later, save_ipl_cert_comp_list() allocates permanent storage and copies
the actual certificate data. As a result, the IIRB data is both used and
protected throughout early boot and is not lost.

I also verified, from both the BIOS and the guest kernel, that the
certificate addresses, lengths, and contents are correctly preserved.
For example:

BIOS debug log:
cert_entries = {{addr = 0x3fd6e98c, len = 0x3e7},
                {addr = 0x3fd6ed73, len = 0x3e7}, ...}

Guest kernel debug log:
[    0.060583] boot: copy_certificates_bootdata()
[    0.060584] boot: Certificate data at 0x3fd6e98c (len=0x3e7):
...
[    0.060588] boot: Certificate data at 0x3fd6ed73 (len=0x3e7):
...

>> +
>> +                    /* increment for the next certificate */
>> +                    cert_entry_idx++;
>> +                    cert_addr += cert_len;
>> +                }
>> +
>> +                puts("Verified component");
>> +                cert_index = cert_list_table[cert_table_idx];
>> +                flags |= S390_IPL_DEV_COMP_FLAG_CSV;
>> +            }
>> +
>> +            comp_list_add(&comp_list, comp_entry_idx, cert_index,
>> +                          comp_addr, comp_len, flags);
>> +
>> +            if (!verified) {
>> +                zipl_secure_handle("Could not verify component");
>> +            }
>> +
>> +            comp_entry_idx++;
>> +            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");
>> +    }
>> +
>> +    update_iirb(&comp_list, &cert_list);
>> +
>> +    *entry_ptr = entry;
>> +    free(sig);
>> +
>> +    return 0;
>> +out:
>> +    free(cert_addr);
>> +    free(sig);
>> +
>> +    return -1;
>> +}

[...]