Re: [PATCH 2/2] arm: socfpga: agilex5: add SSBL verification

["Sascha Hauer" <s.hauer@pengutronix.de>]

On 2026-10-21 22:33, Michael Tretter wrote:
> Altera VAB usually uses a certificate that is appended to the SSBL image
> using the fcs_prepare [0] tool. The certificate contains a sha384
> checksum and is signed using a key chain that ends with the root key
> that is written to the hardware.
> 
> Assume that the certificate is appended to the SSBL image. Read the
> certificate from the same flash partition as the SSBL, pass it to the
> SDM firmware for verification and check against the root key, and
> finally verify the contained checksum in barebox.
> 
> The SDM firmware returns success for the certificate verification if VAB
> is disabled. Create a dummy certificate to check if VAB boot is disabled
> and accept an unsigned SSBL in this case. If there is a valid
> certificate attached, the checksum has to match, even if VAB is
> disabled.
> 
> The implementation is a bit quirky, because
> 
> 1. it needs to deal with the broken SDM firmware behavior, which expects
>    a 32 bit word with value 0 in front of the certificate,
> 
> 2. needs to walk and parse the (still untrusted) key chain to find the
>    actual size of the certificate, and
> 
> 3. I haven't found a better way if VAB is enabled than to check a dummy
>    certificate.
> 
> The VAB code could be moved to a separate module to make it available to
> other SoCFPGA platforms or to barebox proper. Since it's only used for
> SSBL verification for now and is mingled with the SSBL loading, I kept
> it in the xload code.
> 
> [0] https://github.com/altera-fpga/libfcs
> 
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
>  arch/arm/mach-socfpga/agilex5-xload.c | 157 ++++++++++++++++++++++++++++++++++
>  1 file changed, 157 insertions(+)
> 
> diff --git a/arch/arm/mach-socfpga/agilex5-xload.c b/arch/arm/mach-socfpga/agilex5-xload.c
> index 9e55252c04cc..077f67014945 100644
> --- a/arch/arm/mach-socfpga/agilex5-xload.c
> +++ b/arch/arm/mach-socfpga/agilex5-xload.c
> @@ -5,6 +5,11 @@
>  #include <linux/sizes.h>
>  #include <mach/socfpga/generic.h>
>  #include <pbl/bio.h>
> +#include <mach/socfpga/mailbox_s10.h>
> +#include <mach/socfpga/soc64-system-manager.h>
> +
> +#include <crypto/pbl-sha.h>
> +#include <crypto/sha.h>
>  
>  static inline u32 barebox_arm_size(const char *head)
>  {
> @@ -52,9 +57,128 @@ static ssize_t agilex5_bl33_read(struct pbl_bio *bio, void *bl33,
>  	return size;
>  }
>  
> +struct vab_certificate_data {
> +	u32 magic;
> +	u32 flags;
> +	u32 reserved0;
> +	u32 reserved1;
> +	u8 hash[SHA384_DIGEST_SIZE];
> +	u32 measurement;
> +	u32 reserved2;
> +};
> +
> +struct vab_certificate_int {
> +	u32 magic;
> +	u32 size;
> +	u32 version;
> +	u32 type;
> +	struct vab_certificate_data d;
> +	u32 keychain[0];
> +};
> +
> +/*
> + * Use a wrapper struct that starts with a zero word to be able to pass the
> + * certificate to the SDM firmware as it. Let the code that reads the
> + * certificate deal with it to avoid a copy of the certificate.
> + */
> +struct vab_certificate {
> +	u32 zero;
> +	struct vab_certificate_int cert;
> +};
> +
> +static ssize_t vab_get_cert_size(struct vab_certificate_int *cert,
> +				 size_t size)
> +{
> +	struct {
> +		u32 magic;
> +		u32 size;
> +	} *header;
> +	ssize_t offset = 0;
> +
> +	/*
> +	 * Each element in the key chain has a header with some magic and the
> +	 * size of the element. The length of the entire key chain is attached
> +	 * to the end of the key chain. Walk the entire key chain to find the
> +	 * actual size of the key chain.
> +	 *
> +	 * Note that the header is still untrusted!
> +	 */
> +	while (offset < size) {
> +		header = ((void *)cert) + offset;
> +		if (header->magic == offset)
> +			break;
> +		offset += header->size;
> +		if (offset < 0 || offset > size)
> +			return -EINVAL;
> +	}
> +
> +	return offset;
> +}
> +
> +static ssize_t agilex5_vab_read(struct pbl_bio *bio,
> +				struct vab_certificate *cert,
> +				off_t start, size_t size)
> +{
> +	ssize_t ret;
> +	struct vab_certificate_int *tmp = &(cert->cert);
> +	size_t tmp_size = size - sizeof(u32);
> +
> +	cert->zero = 0;
> +	ret = pbl_bio_read(bio, start, tmp, tmp_size);
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	if (tmp->magic != 0x25d04e7f) {
> +		pr_err("vab cert: invalid magic 0x%x\n", tmp->magic);
> +		return -EINVAL;
> +	}
> +	if (tmp->size != sizeof(*tmp)) {
> +		pr_err("vab cert: invalid size %u", tmp->size);
> +		return -EINVAL;
> +	}
> +
> +	ret = vab_get_cert_size(tmp, tmp_size);
> +	if (ret <= 0) {
> +		pr_err("vab cert: invalid certificate chain\n");
> +		return -EINVAL;
> +	}
> +
> +	pr_info("read vab cert to %p from %lx (%zu bytes)\n",
> +		tmp, start, ret);
> +
> +	return ret + sizeof(u32);
> +}
> +
> +static ssize_t agilex5_vab_dummy(struct vab_certificate *cert)
> +{
> +	struct vab_certificate_int *tmp = &(cert->cert);
> +
> +	tmp->magic = 0x25d04e7f;
> +	tmp->size = sizeof(*tmp);
> +	tmp->version = 1;
> +	tmp->type = 6;
> +
> +	return tmp->size;
> +}
> +
> +static int agilex5_vab_verify(const void *data, size_t size,
> +			      struct vab_certificate *cert)
> +{
> +	char computed_hash[SHA384_DIGEST_SIZE];
> +	char *hash = cert->cert.d.hash;
> +
> +	pbl_sha384(data, size, computed_hash);
> +
> +	return memcmp(hash, computed_hash, SHA384_DIGEST_SIZE);
> +}
> +
>  int agilex5_xload_bl33(struct pbl_bio *bio, void *bl33)
>  {
>  	ssize_t size;
> +	off_t cert_offset;
> +	ssize_t cert_size;
> +	struct vab_certificate *cert;
> +	int ret;
>  
>  	if (!bio) {
>  		pr_warn("source device for SSBL is not configured\n");
> @@ -70,5 +194,38 @@ int agilex5_xload_bl33(struct pbl_bio *bio, void *bl33)
>  	if (size < 0)
>  		return size;
>  
> +	/*
> +	 * The certificate is 8 byte aligned and attached to the SSBL binary.
> +	 * The certificate must not be larger than the partition and is
> +	 * usually smaller than 4K.
> +	 */
> +	size = ALIGN(size, 8);
> +	cert_offset = barebox_part->nor_offset + size;
> +	cert_size = min_t(size_t, SZ_4K, barebox_part->nor_size - size);
> +
> +	/* Use the SDRAM behind the bl33 for the certificate */
> +	cert = bl33 + size;
> +	cert_size = agilex5_vab_read(bio, cert, cert_offset, cert_size);
> +	if (cert_size < 0) {
> +		/*
> +		 * There is no valid certificate. Generate a dummy certificate
> +		 * to ask the firmware if authentication bypass is allowed.
> +		 */
> +		cert_size = agilex5_vab_dummy(cert);
> +	}
> +
> +	ret = mbox_vab_src_cert(cert, cert_size);
> +	if (ret)
> +		return ret;
> +
> +	ret = agilex5_vab_verify(bl33, size, cert);
> +	if (ret) {
> +		if (cert_size != sizeof(*cert)) {
> +			pr_err("SSBL hash mismatch\n");
> +			return ret;
> +		}

This is unnecessary hard to read. I assume cert_size != sizeof(*cert)
means "This is not the dummy cert generated above", because a real cert
must always have data besides the header, right?

> +		pr_debug("SSBL hash mismatch: Bypass authentication\n");

The dummy cert never passes agilex5_vab_verify(), so it's not necessary
to check its sha384 sum.

I think you should rather have a dummy_cert boolean and return
successfully in the dummy case right after calling mbox_vab_src_cert().
With this you would also not have to return successfully after a failed
verification, which raises eyebrows.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |