On 7/2/26 11:26 AM, Zhuoying Cai wrote:
Thanks for the feedback!
On 7/2/26 10:13 AM, Jared Rossi wrote:
On 7/1/26 4:49 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.
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 <[email protected]>
---
docs/system/s390x/secure-ipl.rst | 15 ++
pc-bios/s390-ccw/Makefile | 2 +-
pc-bios/s390-ccw/bootmap.c | 27 +++
pc-bios/s390-ccw/bootmap.h | 9 +
pc-bios/s390-ccw/jump2ipl.c | 7 +
pc-bios/s390-ccw/main.c | 18 +-
pc-bios/s390-ccw/s390-ccw.h | 20 ++
pc-bios/s390-ccw/sclp.c | 27 +++
pc-bios/s390-ccw/sclp.h | 6 +
pc-bios/s390-ccw/secure-ipl.c | 362 +++++++++++++++++++++++++++++++
pc-bios/s390-ccw/secure-ipl.h | 116 ++++++++++
11 files changed, 607 insertions(+), 2 deletions(-)
create mode 100644 pc-bios/s390-ccw/secure-ipl.c
create mode 100644 pc-bios/s390-ccw/secure-ipl.h
[...]
+
+void update_cert_list(IplSignatureCertificateList *cert_list)
+{
+ IplSignatureCertificateEntry *cert_entry;
+ uint8_t *cert_buf;
+
+ cert_buf = (uint8_t *)qipl.ipl_data;
+
+ for_each_rb_entry(cert_entry, cert_list) {
+ memcpy(cert_buf, (uint8_t *)cert_entry->addr, cert_entry->len);
+ cert_entry->addr = (uint64_t)cert_buf;
+ cert_buf += cert_entry->len;
+ }
+}
This looks like the correct idea but I think it needs additional
validation. Firstly, as Eric mentioned, there is no check that the size
of our cert entries don't exceed the qipl.ipl_data space. I know there
are implicit factors such as maximum certificate size and such that
limit it in a way that it shouldn't be possible, but we should still check.
At the beginning of zipl_run_secure(), I added a check to ensure that
there is enough space to store the certificates required for the current
IPL. We store the certificates directly in the qipl.ipl_data area rather
than storing cert entries (IplSignatureCertificateEntry) in qipl.ipl_data.
As discussed previously and documented here
(<https://www.ibm.com/docs/en/linux-on-systems?topic=keys-required-signatures>),
an IPL requires at most two, and possibly three, signed components,
meaning that at most three certificates are needed. Based on that, I
check that MAX_SIGNED_COMP * CERT_BUF_MAX_LEN does not exceed
CERT_BUF_SIZE, ensuring there is sufficient space for the certificates
used by the current IPL. Please let me know if you think any additional
validation is needed.
That check is good. I think it could be a compile time check though
since everything is constant.
I guess the reason this feels fragile is because the size of the
iplb_chain array determines the size of the ipl_data area, and the chain
array size itself is determined by a magic number 7 in hw/ipl.c. We
need to change that line anyway to initialize the chain to zero as Eric
pointed out, so lets also use MAX_BOOT_DEVS - 1 instead of just magic
7. This way we can also add another compile time check:
(sizeof(IplParameterBlock) * (MAX_BOOT_DEVS - 1)) == CERT_BUF_SIZE
which will ensure that any potential changes to the either the cert
buffer size and/or number of allowed boot devices will be kept in sync
for the shared buffer area.
Secondly and more critically, you need to reset the qipl.ipl_data
address to it's initial value before reusing it. I don't think I saw
that done anywhere. The address is incremented after each IPLB is
loaded from the chain (see the load_next_iplb() function in iplb.h), so
if we in theory had started with 7 IPLBs in the chain and loaded all of
them, and if we then copied the maximum allowed size of certificate
entry data, we would exceed the qipl.ipl_data upper limit and probably
clobber the kernel.
I think the easiest way to restore the qipl.ipl_data address to its
initial value is to track how many IPLBs we've used, multiply that by
the size of the IPLB, and then move the address back by that amount.
Thanks for pointing this out. I missed resetting the qipl.ipl_data
address before reusing it. I'll restore it to its initial value before
copying the certificate data. Thanks!
Regards,
Jared Rossi
Thanks,
Jared Rossi
