* [PATCH v2 1/8] lib: rsa: export rsa_verify_with_pkey()
2020-06-16 5:26 [PATCH v2 0/8] efi_loader: secure boot: support intermediate certificates in signature AKASHI Takahiro
@ 2020-06-16 5:26 ` AKASHI Takahiro
2020-07-07 10:06 ` Heinrich Schuchardt
2020-06-16 5:26 ` [PATCH v2 2/8] lib: crypto: add public_key_verify_signature() AKASHI Takahiro
` (6 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: AKASHI Takahiro @ 2020-06-16 5:26 UTC (permalink / raw)
To: u-boot
This function will be used to implement public_key_verify_signature()
in a later patch. rsa_verify() is not suitable here because calculation
of message digest is not necessary.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
include/u-boot/rsa.h | 3 +++
lib/rsa/rsa-verify.c | 8 ++++----
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/include/u-boot/rsa.h b/include/u-boot/rsa.h
index 2d3024d8b710..eab6bfafa476 100644
--- a/include/u-boot/rsa.h
+++ b/include/u-boot/rsa.h
@@ -98,6 +98,9 @@ int rsa_verify(struct image_sign_info *info,
const struct image_region region[], int region_count,
uint8_t *sig, uint sig_len);
+int rsa_verify_with_pkey(struct image_sign_info *info,
+ const void *hash, uint8_t *sig, uint sig_len);
+
int padding_pkcs_15_verify(struct image_sign_info *info,
uint8_t *msg, int msg_len,
const uint8_t *hash, int hash_len);
diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c
index 1d55b997e34c..d0e863f9b0f8 100644
--- a/lib/rsa/rsa-verify.c
+++ b/lib/rsa/rsa-verify.c
@@ -374,8 +374,8 @@ static int rsa_verify_key(struct image_sign_info *info,
*
* Return 0 if verified, -ve on error
*/
-static int rsa_verify_with_pkey(struct image_sign_info *info,
- const void *hash, uint8_t *sig, uint sig_len)
+int rsa_verify_with_pkey(struct image_sign_info *info,
+ const void *hash, uint8_t *sig, uint sig_len)
{
struct key_prop *prop;
int ret;
@@ -395,8 +395,8 @@ static int rsa_verify_with_pkey(struct image_sign_info *info,
return ret;
}
#else
-static int rsa_verify_with_pkey(struct image_sign_info *info,
- const void *hash, uint8_t *sig, uint sig_len)
+int rsa_verify_with_pkey(struct image_sign_info *info,
+ const void *hash, uint8_t *sig, uint sig_len)
{
return -EACCES;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 2/8] lib: crypto: add public_key_verify_signature()
2020-06-16 5:26 [PATCH v2 0/8] efi_loader: secure boot: support intermediate certificates in signature AKASHI Takahiro
2020-06-16 5:26 ` [PATCH v2 1/8] lib: rsa: export rsa_verify_with_pkey() AKASHI Takahiro
@ 2020-06-16 5:26 ` AKASHI Takahiro
2020-07-07 10:04 ` Heinrich Schuchardt
2020-06-16 5:26 ` [PATCH v2 3/8] lib: crypto: enable x509_check_for_self_signed() AKASHI Takahiro
` (5 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: AKASHI Takahiro @ 2020-06-16 5:26 UTC (permalink / raw)
To: u-boot
This function will be called from x509_check_for_self_signed() and
pkcs7_verify_one(), which will be imported from linux in a later patch.
While it does exist in linux code and has a similar functionality of
rsa_verify(), it calls further linux-specific interfaces inside.
That could lead to more files being imported from linux.
So simply re-implement it here instead of re-using the code.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
include/crypto/public_key.h | 2 +-
lib/crypto/public_key.c | 63 ++++++++++++++++++++++++++++++++++++-
2 files changed, 63 insertions(+), 2 deletions(-)
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index 436a1ee1ee64..3ba90fcc3483 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -82,9 +82,9 @@ extern int decrypt_blob(struct kernel_pkey_params *, const void *, void *);
extern int create_signature(struct kernel_pkey_params *, const void *, void *);
extern int verify_signature(const struct key *,
const struct public_key_signature *);
+#endif /* __UBOOT__ */
int public_key_verify_signature(const struct public_key *pkey,
const struct public_key_signature *sig);
-#endif /* !__UBOOT__ */
#endif /* _LINUX_PUBLIC_KEY_H */
diff --git a/lib/crypto/public_key.c b/lib/crypto/public_key.c
index e12ebbb3d0c5..013ea71a180f 100644
--- a/lib/crypto/public_key.c
+++ b/lib/crypto/public_key.c
@@ -25,7 +25,10 @@
#include <keys/asymmetric-subtype.h>
#endif
#include <crypto/public_key.h>
-#ifndef __UBOOT__
+#ifdef __UBOOT__
+#include <image.h>
+#include <u-boot/rsa.h>
+#else
#include <crypto/akcipher.h>
#endif
@@ -80,6 +83,64 @@ void public_key_signature_free(struct public_key_signature *sig)
}
EXPORT_SYMBOL_GPL(public_key_signature_free);
+/**
+ * public_key_verify_signature - Verify a signature using a public key.
+ *
+ * @pkey: Public key
+ * @sig: Signature
+ *
+ * Verify a signature, @sig, using a RSA public key, @pkey.
+ *
+ * Return: 0 - verified, non-zero error code - otherwise
+ */
+int public_key_verify_signature(const struct public_key *pkey,
+ const struct public_key_signature *sig)
+{
+ struct image_sign_info info;
+ struct image_region region;
+ int ret;
+
+ pr_devel("==>%s()\n", __func__);
+
+ if (!pkey || !sig)
+ return -EINVAL;
+
+ if (pkey->key_is_private)
+ return -EINVAL;
+
+ memset(&info, '\0', sizeof(info));
+ info.padding = image_get_padding_algo("pkcs-1.5");
+ /*
+ * Note: image_get_[checksum|crypto]_algo takes an string
+ * argument like "<checksum>,<crypto>"
+ * TODO: support other hash algorithms
+ */
+ if (!strcmp(sig->hash_algo, "sha1")) {
+ info.checksum = image_get_checksum_algo("sha1,rsa2048");
+ info.name = "sha1,rsa2048";
+ } else if (!strcmp(sig->hash_algo, "sha256")) {
+ info.checksum = image_get_checksum_algo("sha256,rsa2048");
+ info.name = "sha256,rsa2048";
+ } else {
+ pr_warn("unknown msg digest algo: %s\n", sig->hash_algo);
+ return -ENOPKG;
+ }
+ info.crypto = image_get_crypto_algo(info.name);
+
+ info.key = pkey->key;
+ info.keylen = pkey->keylen;
+
+ region.data = sig->digest;
+ region.size = sig->digest_size;
+
+ if (rsa_verify_with_pkey(&info, sig->digest, sig->s, sig->s_size))
+ ret = -EKEYREJECTED;
+ else
+ ret = 0;
+
+ pr_devel("<==%s() = %d\n", __func__, ret);
+ return ret;
+}
#else
/*
* Destroy a public key algorithm key.
--
2.27.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 2/8] lib: crypto: add public_key_verify_signature()
2020-06-16 5:26 ` [PATCH v2 2/8] lib: crypto: add public_key_verify_signature() AKASHI Takahiro
@ 2020-07-07 10:04 ` Heinrich Schuchardt
2020-07-08 6:21 ` AKASHI Takahiro
0 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2020-07-07 10:04 UTC (permalink / raw)
To: u-boot
On 16.06.20 07:26, AKASHI Takahiro wrote:
> This function will be called from x509_check_for_self_signed() and
> pkcs7_verify_one(), which will be imported from linux in a later patch.
>
> While it does exist in linux code and has a similar functionality of
> rsa_verify(), it calls further linux-specific interfaces inside.
> That could lead to more files being imported from linux.
>
> So simply re-implement it here instead of re-using the code.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> include/crypto/public_key.h | 2 +-
> lib/crypto/public_key.c | 63 ++++++++++++++++++++++++++++++++++++-
> 2 files changed, 63 insertions(+), 2 deletions(-)
>
> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> index 436a1ee1ee64..3ba90fcc3483 100644
> --- a/include/crypto/public_key.h
> +++ b/include/crypto/public_key.h
> @@ -82,9 +82,9 @@ extern int decrypt_blob(struct kernel_pkey_params *, const void *, void *);
> extern int create_signature(struct kernel_pkey_params *, const void *, void *);
> extern int verify_signature(const struct key *,
> const struct public_key_signature *);
> +#endif /* __UBOOT__ */
>
> int public_key_verify_signature(const struct public_key *pkey,
> const struct public_key_signature *sig);
> -#endif /* !__UBOOT__ */
>
> #endif /* _LINUX_PUBLIC_KEY_H */
> diff --git a/lib/crypto/public_key.c b/lib/crypto/public_key.c
> index e12ebbb3d0c5..013ea71a180f 100644
> --- a/lib/crypto/public_key.c
> +++ b/lib/crypto/public_key.c
> @@ -25,7 +25,10 @@
> #include <keys/asymmetric-subtype.h>
> #endif
> #include <crypto/public_key.h>
> -#ifndef __UBOOT__
> +#ifdef __UBOOT__
> +#include <image.h>
> +#include <u-boot/rsa.h>
> +#else
> #include <crypto/akcipher.h>
> #endif
>
> @@ -80,6 +83,64 @@ void public_key_signature_free(struct public_key_signature *sig)
> }
> EXPORT_SYMBOL_GPL(public_key_signature_free);
>
> +/**
> + * public_key_verify_signature - Verify a signature using a public key.
> + *
> + * @pkey: Public key
> + * @sig: Signature
> + *
> + * Verify a signature, @sig, using a RSA public key, @pkey.
> + *
> + * Return: 0 - verified, non-zero error code - otherwise
> + */
> +int public_key_verify_signature(const struct public_key *pkey,
> + const struct public_key_signature *sig)
> +{
> + struct image_sign_info info;
> + struct image_region region;
> + int ret;
> +
> + pr_devel("==>%s()\n", __func__);
> +
> + if (!pkey || !sig)
> + return -EINVAL;
> +
> + if (pkey->key_is_private)
> + return -EINVAL;
> +
> + memset(&info, '\0', sizeof(info));
> + info.padding = image_get_padding_algo("pkcs-1.5");
> + /*
> + * Note: image_get_[checksum|crypto]_algo takes an string
nits
%/an string/a string/g
> + * argument like "<checksum>,<crypto>"
> + * TODO: support other hash algorithms
> + */
> + if (!strcmp(sig->hash_algo, "sha1")) {
In our coding sig->key_algo can have values "rsa" or "ecrdsa". See
lib/crypto/x509_cert_parser.c:471. Please, check the value.
You can use sig->s_size * 8 to get the actual key size.
You should check the return value of image_get_checksum_algo().
if (!strcmp(sig->key_algo, "rsa")) {
char algo[16];
snprintf(algo, sizeof(algo), "%s,rsa%d", sig->hash_algo, 8 * sig->s_size);
info.checksum = image_get_checksum_algo(algo);
}
if (!info.checksum)
return -ENOPKG;
}
> + info.checksum = image_get_checksum_algo("sha1,rsa2048");
> + info.name = "sha1,rsa2048";
> + } else if (!strcmp(sig->hash_algo, "sha256")) {
> + info.checksum = image_get_checksum_algo("sha256,rsa2048");
> + info.name = "sha256,rsa2048";
> + } else {
> + pr_warn("unknown msg digest algo: %s\n", sig->hash_algo);
Please, use log_warning().
> + return -ENOPKG;
> + }
> + info.crypto = image_get_crypto_algo(info.name);
> +
> + info.key = pkey->key;
> + info.keylen = pkey->keylen;
> +
> + region.data = sig->digest;
> + region.size = sig->digest_size;
> +
> + if (rsa_verify_with_pkey(&info, sig->digest, sig->s, sig->s_size))
Why warning above but no debug message here?
> + ret = -EKEYREJECTED;
> + else
> + ret = 0;
> +
> + pr_devel("<==%s() = %d\n", __func__, ret);
log_content()
Best regards
Heinrich
> + return ret;
> +}
> #else
> /*
> * Destroy a public key algorithm key.
>
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 2/8] lib: crypto: add public_key_verify_signature()
2020-07-07 10:04 ` Heinrich Schuchardt
@ 2020-07-08 6:21 ` AKASHI Takahiro
0 siblings, 0 replies; 25+ messages in thread
From: AKASHI Takahiro @ 2020-07-08 6:21 UTC (permalink / raw)
To: u-boot
Heinrich,
On Tue, Jul 07, 2020 at 12:04:29PM +0200, Heinrich Schuchardt wrote:
> On 16.06.20 07:26, AKASHI Takahiro wrote:
> > This function will be called from x509_check_for_self_signed() and
> > pkcs7_verify_one(), which will be imported from linux in a later patch.
> >
> > While it does exist in linux code and has a similar functionality of
> > rsa_verify(), it calls further linux-specific interfaces inside.
> > That could lead to more files being imported from linux.
> >
> > So simply re-implement it here instead of re-using the code.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > include/crypto/public_key.h | 2 +-
> > lib/crypto/public_key.c | 63 ++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 63 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> > index 436a1ee1ee64..3ba90fcc3483 100644
> > --- a/include/crypto/public_key.h
> > +++ b/include/crypto/public_key.h
> > @@ -82,9 +82,9 @@ extern int decrypt_blob(struct kernel_pkey_params *, const void *, void *);
> > extern int create_signature(struct kernel_pkey_params *, const void *, void *);
> > extern int verify_signature(const struct key *,
> > const struct public_key_signature *);
> > +#endif /* __UBOOT__ */
> >
> > int public_key_verify_signature(const struct public_key *pkey,
> > const struct public_key_signature *sig);
> > -#endif /* !__UBOOT__ */
> >
> > #endif /* _LINUX_PUBLIC_KEY_H */
> > diff --git a/lib/crypto/public_key.c b/lib/crypto/public_key.c
> > index e12ebbb3d0c5..013ea71a180f 100644
> > --- a/lib/crypto/public_key.c
> > +++ b/lib/crypto/public_key.c
> > @@ -25,7 +25,10 @@
> > #include <keys/asymmetric-subtype.h>
> > #endif
> > #include <crypto/public_key.h>
> > -#ifndef __UBOOT__
> > +#ifdef __UBOOT__
> > +#include <image.h>
> > +#include <u-boot/rsa.h>
> > +#else
> > #include <crypto/akcipher.h>
> > #endif
> >
> > @@ -80,6 +83,64 @@ void public_key_signature_free(struct public_key_signature *sig)
> > }
> > EXPORT_SYMBOL_GPL(public_key_signature_free);
> >
> > +/**
> > + * public_key_verify_signature - Verify a signature using a public key.
> > + *
> > + * @pkey: Public key
> > + * @sig: Signature
> > + *
> > + * Verify a signature, @sig, using a RSA public key, @pkey.
> > + *
> > + * Return: 0 - verified, non-zero error code - otherwise
> > + */
> > +int public_key_verify_signature(const struct public_key *pkey,
> > + const struct public_key_signature *sig)
> > +{
> > + struct image_sign_info info;
> > + struct image_region region;
> > + int ret;
> > +
> > + pr_devel("==>%s()\n", __func__);
> > +
> > + if (!pkey || !sig)
> > + return -EINVAL;
> > +
> > + if (pkey->key_is_private)
> > + return -EINVAL;
> > +
> > + memset(&info, '\0', sizeof(info));
> > + info.padding = image_get_padding_algo("pkcs-1.5");
> > + /*
> > + * Note: image_get_[checksum|crypto]_algo takes an string
>
> nits
>
> %/an string/a string/g
Fixed.
> > + * argument like "<checksum>,<crypto>"
> > + * TODO: support other hash algorithms
> > + */
> > + if (!strcmp(sig->hash_algo, "sha1")) {
>
> In our coding sig->key_algo can have values "rsa" or "ecrdsa". See
> lib/crypto/x509_cert_parser.c:471. Please, check the value.
>
> You can use sig->s_size * 8 to get the actual key size.
I think that you are mentioning a check against encryption algo.
Yeah, I will add it although it will anyhow fail in rsa_verify_with_pkey().
> You should check the return value of image_get_checksum_algo().
I think it's nothing but an overhead as the arguments here
have "static" values, but will add it if you want.
> if (!strcmp(sig->key_algo, "rsa")) {
> char algo[16];
> snprintf(algo, sizeof(algo), "%s,rsa%d", sig->hash_algo, 8 * sig->s_size);
> info.checksum = image_get_checksum_algo(algo);
> }
> if (!info.checksum)
> return -ENOPKG;
> }
>
> > + info.checksum = image_get_checksum_algo("sha1,rsa2048");
> > + info.name = "sha1,rsa2048";
> > + } else if (!strcmp(sig->hash_algo, "sha256")) {
> > + info.checksum = image_get_checksum_algo("sha256,rsa2048");
> > + info.name = "sha256,rsa2048";
> > + } else {
> > + pr_warn("unknown msg digest algo: %s\n", sig->hash_algo);
>
> Please, use log_warning().
No, I can't agree.
I prefer to call pr_xxx() very much more than log_xxx() as
this code is embedded in a file imported from linux.
Moreover, log_xxx() is seldom used even across U-Boot.
> > + return -ENOPKG;
> > + }
> > + info.crypto = image_get_crypto_algo(info.name);
> > +
> > + info.key = pkey->key;
> > + info.keylen = pkey->keylen;
> > +
> > + region.data = sig->digest;
> > + region.size = sig->digest_size;
> > +
> > + if (rsa_verify_with_pkey(&info, sig->digest, sig->s, sig->s_size))
>
> Why warning above but no debug message here?
Because
* the reason of EKEYREJECTED failure is trivial in this function.
* rsa_verify_With_pkey() outputs a bunch of verbose and useful
messages instead.
> > + ret = -EKEYREJECTED;
> > + else
> > + ret = 0;
> > +
> > + pr_devel("<==%s() = %d\n", __func__, ret);
>
> log_content()
See my opinion & policy above.
-Takahiro Akashi
> Best regards
>
> Heinrich
>
>
> > + return ret;
> > +}
> > #else
> > /*
> > * Destroy a public key algorithm key.
> >
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 3/8] lib: crypto: enable x509_check_for_self_signed()
2020-06-16 5:26 [PATCH v2 0/8] efi_loader: secure boot: support intermediate certificates in signature AKASHI Takahiro
2020-06-16 5:26 ` [PATCH v2 1/8] lib: rsa: export rsa_verify_with_pkey() AKASHI Takahiro
2020-06-16 5:26 ` [PATCH v2 2/8] lib: crypto: add public_key_verify_signature() AKASHI Takahiro
@ 2020-06-16 5:26 ` AKASHI Takahiro
2020-07-07 10:02 ` Heinrich Schuchardt
2020-06-16 5:26 ` [PATCH v2 4/8] lib: crypto: import pkcs7_verify.c from linux AKASHI Takahiro
` (4 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: AKASHI Takahiro @ 2020-06-16 5:26 UTC (permalink / raw)
To: u-boot
When the file, x509_public_key.c, was imported from linux code in
commit b4adf627d5b7 ("lib: crypto: add x509 parser"),
x509_check_for_self_signed() was commented out for simplicity.
Now it need be enabled in order to make pkcs7_verify_one(), which will be
imported in a later patch, functional.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
lib/crypto/x509_cert_parser.c | 2 --
lib/crypto/x509_public_key.c | 33 +++++++++++++++++++++++++--------
2 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/lib/crypto/x509_cert_parser.c b/lib/crypto/x509_cert_parser.c
index 5f984b9dfdae..eb24349460c2 100644
--- a/lib/crypto/x509_cert_parser.c
+++ b/lib/crypto/x509_cert_parser.c
@@ -142,12 +142,10 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
}
cert->id = kid;
-#ifndef __UBOOT__
/* Detect self-signed certificates */
ret = x509_check_for_self_signed(cert);
if (ret < 0)
goto error_decode;
-#endif
kfree(ctx);
return cert;
diff --git a/lib/crypto/x509_public_key.c b/lib/crypto/x509_public_key.c
index 571af9a0adf9..91810a864049 100644
--- a/lib/crypto/x509_public_key.c
+++ b/lib/crypto/x509_public_key.c
@@ -8,6 +8,7 @@
#define pr_fmt(fmt) "X.509: "fmt
#ifdef __UBOOT__
#include <common.h>
+#include <image.h>
#include <dm/devres.h>
#include <linux/compat.h>
#include <linux/err.h>
@@ -18,6 +19,7 @@
#include <linux/kernel.h>
#ifdef __UBOOT__
#include <crypto/x509_parser.h>
+#include <u-boot/rsa-checksum.h>
#else
#include <linux/slab.h>
#include <keys/asymmetric-subtype.h>
@@ -35,7 +37,9 @@
int x509_get_sig_params(struct x509_certificate *cert)
{
struct public_key_signature *sig = cert->sig;
-#ifndef __UBOOT__
+#ifdef __UBOOT__
+ struct image_region region;
+#else
struct crypto_shash *tfm;
struct shash_desc *desc;
size_t desc_size;
@@ -63,12 +67,25 @@ int x509_get_sig_params(struct x509_certificate *cert)
sig->s_size = cert->raw_sig_size;
#ifdef __UBOOT__
- /*
- * Note:
- * This part (filling sig->digest) should be implemented if
- * x509_check_for_self_signed() is enabled x509_cert_parse().
- * Currently, this check won't affect UEFI secure boot.
- */
+ if (!sig->hash_algo)
+ return -ENOPKG;
+ if (!strcmp(sig->hash_algo, "sha256"))
+ sig->digest_size = SHA256_SUM_LEN;
+ else if (!strcmp(sig->hash_algo, "sha1"))
+ sig->digest_size = SHA1_SUM_LEN;
+ else
+ return -ENOPKG;
+
+ sig->digest = calloc(1, sig->digest_size);
+ if (!sig->digest)
+ return -ENOMEM;
+
+ region.data = cert->tbs;
+ region.size = cert->tbs_size;
+ hash_calculate(sig->hash_algo, ®ion, 1, sig->digest);
+
+ /* TODO: is_hash_blacklisted()? */
+
ret = 0;
#else
/* Allocate the hashing algorithm we're going to need and find out how
@@ -118,7 +135,6 @@ error:
return ret;
}
-#ifndef __UBOOT__
/*
* Check for self-signedness in an X.509 cert and if found, check the signature
* immediately if we can.
@@ -175,6 +191,7 @@ not_self_signed:
return 0;
}
+#ifndef __UBOOT__
/*
* Attempt to parse a data blob for a key as an X509 certificate.
*/
--
2.27.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 3/8] lib: crypto: enable x509_check_for_self_signed()
2020-06-16 5:26 ` [PATCH v2 3/8] lib: crypto: enable x509_check_for_self_signed() AKASHI Takahiro
@ 2020-07-07 10:02 ` Heinrich Schuchardt
2020-07-08 6:24 ` AKASHI Takahiro
0 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2020-07-07 10:02 UTC (permalink / raw)
To: u-boot
On 16.06.20 07:26, AKASHI Takahiro wrote:
> When the file, x509_public_key.c, was imported from linux code in
> commit b4adf627d5b7 ("lib: crypto: add x509 parser"),
> x509_check_for_self_signed() was commented out for simplicity.
>
> Now it need be enabled in order to make pkcs7_verify_one(), which will be
> imported in a later patch, functional.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> lib/crypto/x509_cert_parser.c | 2 --
> lib/crypto/x509_public_key.c | 33 +++++++++++++++++++++++++--------
> 2 files changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/lib/crypto/x509_cert_parser.c b/lib/crypto/x509_cert_parser.c
> index 5f984b9dfdae..eb24349460c2 100644
> --- a/lib/crypto/x509_cert_parser.c
> +++ b/lib/crypto/x509_cert_parser.c
> @@ -142,12 +142,10 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
> }
> cert->id = kid;
>
> -#ifndef __UBOOT__
> /* Detect self-signed certificates */
> ret = x509_check_for_self_signed(cert);
> if (ret < 0)
> goto error_decode;
> -#endif
>
> kfree(ctx);
> return cert;
> diff --git a/lib/crypto/x509_public_key.c b/lib/crypto/x509_public_key.c
> index 571af9a0adf9..91810a864049 100644
> --- a/lib/crypto/x509_public_key.c
> +++ b/lib/crypto/x509_public_key.c
> @@ -8,6 +8,7 @@
> #define pr_fmt(fmt) "X.509: "fmt
> #ifdef __UBOOT__
> #include <common.h>
> +#include <image.h>
> #include <dm/devres.h>
> #include <linux/compat.h>
> #include <linux/err.h>
> @@ -18,6 +19,7 @@
> #include <linux/kernel.h>
> #ifdef __UBOOT__
> #include <crypto/x509_parser.h>
> +#include <u-boot/rsa-checksum.h>
> #else
> #include <linux/slab.h>
> #include <keys/asymmetric-subtype.h>
> @@ -35,7 +37,9 @@
> int x509_get_sig_params(struct x509_certificate *cert)
> {
> struct public_key_signature *sig = cert->sig;
> -#ifndef __UBOOT__
> +#ifdef __UBOOT__
> + struct image_region region;
> +#else
> struct crypto_shash *tfm;
> struct shash_desc *desc;
> size_t desc_size;
> @@ -63,12 +67,25 @@ int x509_get_sig_params(struct x509_certificate *cert)
> sig->s_size = cert->raw_sig_size;
>
> #ifdef __UBOOT__
> - /*
> - * Note:
> - * This part (filling sig->digest) should be implemented if
> - * x509_check_for_self_signed() is enabled x509_cert_parse().
> - * Currently, this check won't affect UEFI secure boot.
> - */
> + if (!sig->hash_algo)
> + return -ENOPKG;
> + if (!strcmp(sig->hash_algo, "sha256"))
> + sig->digest_size = SHA256_SUM_LEN;
> + else if (!strcmp(sig->hash_algo, "sha1"))
> + sig->digest_size = SHA1_SUM_LEN;
> + else
> + return -ENOPKG;
It would be preferable to call hash_lookup_algo() instead of hard coding
hash sizes in multiple places.
Best regards
Heinrich
> +
> + sig->digest = calloc(1, sig->digest_size);
> + if (!sig->digest)
> + return -ENOMEM;
> +
> + region.data = cert->tbs;
> + region.size = cert->tbs_size;
> + hash_calculate(sig->hash_algo, ®ion, 1, sig->digest);
> +
> + /* TODO: is_hash_blacklisted()? */
> +
> ret = 0;
> #else
> /* Allocate the hashing algorithm we're going to need and find out how
> @@ -118,7 +135,6 @@ error:
> return ret;
> }
>
> -#ifndef __UBOOT__
> /*
> * Check for self-signedness in an X.509 cert and if found, check the signature
> * immediately if we can.
> @@ -175,6 +191,7 @@ not_self_signed:
> return 0;
> }
>
> +#ifndef __UBOOT__
> /*
> * Attempt to parse a data blob for a key as an X509 certificate.
> */
>
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 3/8] lib: crypto: enable x509_check_for_self_signed()
2020-07-07 10:02 ` Heinrich Schuchardt
@ 2020-07-08 6:24 ` AKASHI Takahiro
0 siblings, 0 replies; 25+ messages in thread
From: AKASHI Takahiro @ 2020-07-08 6:24 UTC (permalink / raw)
To: u-boot
Heinrich,
On Tue, Jul 07, 2020 at 12:02:51PM +0200, Heinrich Schuchardt wrote:
> On 16.06.20 07:26, AKASHI Takahiro wrote:
> > When the file, x509_public_key.c, was imported from linux code in
> > commit b4adf627d5b7 ("lib: crypto: add x509 parser"),
> > x509_check_for_self_signed() was commented out for simplicity.
> >
> > Now it need be enabled in order to make pkcs7_verify_one(), which will be
> > imported in a later patch, functional.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > lib/crypto/x509_cert_parser.c | 2 --
> > lib/crypto/x509_public_key.c | 33 +++++++++++++++++++++++++--------
> > 2 files changed, 25 insertions(+), 10 deletions(-)
> >
> > diff --git a/lib/crypto/x509_cert_parser.c b/lib/crypto/x509_cert_parser.c
> > index 5f984b9dfdae..eb24349460c2 100644
> > --- a/lib/crypto/x509_cert_parser.c
> > +++ b/lib/crypto/x509_cert_parser.c
> > @@ -142,12 +142,10 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
> > }
> > cert->id = kid;
> >
> > -#ifndef __UBOOT__
> > /* Detect self-signed certificates */
> > ret = x509_check_for_self_signed(cert);
> > if (ret < 0)
> > goto error_decode;
> > -#endif
> >
> > kfree(ctx);
> > return cert;
> > diff --git a/lib/crypto/x509_public_key.c b/lib/crypto/x509_public_key.c
> > index 571af9a0adf9..91810a864049 100644
> > --- a/lib/crypto/x509_public_key.c
> > +++ b/lib/crypto/x509_public_key.c
> > @@ -8,6 +8,7 @@
> > #define pr_fmt(fmt) "X.509: "fmt
> > #ifdef __UBOOT__
> > #include <common.h>
> > +#include <image.h>
> > #include <dm/devres.h>
> > #include <linux/compat.h>
> > #include <linux/err.h>
> > @@ -18,6 +19,7 @@
> > #include <linux/kernel.h>
> > #ifdef __UBOOT__
> > #include <crypto/x509_parser.h>
> > +#include <u-boot/rsa-checksum.h>
> > #else
> > #include <linux/slab.h>
> > #include <keys/asymmetric-subtype.h>
> > @@ -35,7 +37,9 @@
> > int x509_get_sig_params(struct x509_certificate *cert)
> > {
> > struct public_key_signature *sig = cert->sig;
> > -#ifndef __UBOOT__
> > +#ifdef __UBOOT__
> > + struct image_region region;
> > +#else
> > struct crypto_shash *tfm;
> > struct shash_desc *desc;
> > size_t desc_size;
> > @@ -63,12 +67,25 @@ int x509_get_sig_params(struct x509_certificate *cert)
> > sig->s_size = cert->raw_sig_size;
> >
> > #ifdef __UBOOT__
> > - /*
> > - * Note:
> > - * This part (filling sig->digest) should be implemented if
> > - * x509_check_for_self_signed() is enabled x509_cert_parse().
> > - * Currently, this check won't affect UEFI secure boot.
> > - */
> > + if (!sig->hash_algo)
> > + return -ENOPKG;
> > + if (!strcmp(sig->hash_algo, "sha256"))
> > + sig->digest_size = SHA256_SUM_LEN;
> > + else if (!strcmp(sig->hash_algo, "sha1"))
> > + sig->digest_size = SHA1_SUM_LEN;
> > + else
> > + return -ENOPKG;
>
> It would be preferable to call hash_lookup_algo() instead of hard coding
> hash sizes in multiple places.
Maybe it's a matter of taste, but I don't see any benefit of
calling hash_lookup_algo() here as we name an algo name
explicitly.
It's nothing but an overhead.
-Takahiro Akashi
> Best regards
>
> Heinrich
>
> > +
> > + sig->digest = calloc(1, sig->digest_size);
> > + if (!sig->digest)
> > + return -ENOMEM;
> > +
> > + region.data = cert->tbs;
> > + region.size = cert->tbs_size;
> > + hash_calculate(sig->hash_algo, ®ion, 1, sig->digest);
> > +
> > + /* TODO: is_hash_blacklisted()? */
> > +
> > ret = 0;
> > #else
> > /* Allocate the hashing algorithm we're going to need and find out how
> > @@ -118,7 +135,6 @@ error:
> > return ret;
> > }
> >
> > -#ifndef __UBOOT__
> > /*
> > * Check for self-signedness in an X.509 cert and if found, check the signature
> > * immediately if we can.
> > @@ -175,6 +191,7 @@ not_self_signed:
> > return 0;
> > }
> >
> > +#ifndef __UBOOT__
> > /*
> > * Attempt to parse a data blob for a key as an X509 certificate.
> > */
> >
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 4/8] lib: crypto: import pkcs7_verify.c from linux
2020-06-16 5:26 [PATCH v2 0/8] efi_loader: secure boot: support intermediate certificates in signature AKASHI Takahiro
` (2 preceding siblings ...)
2020-06-16 5:26 ` [PATCH v2 3/8] lib: crypto: enable x509_check_for_self_signed() AKASHI Takahiro
@ 2020-06-16 5:26 ` AKASHI Takahiro
2020-07-07 10:27 ` Heinrich Schuchardt
2020-06-16 5:26 ` [PATCH v2 5/8] lib: crypto: add pkcs7_digest() AKASHI Takahiro
` (3 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: AKASHI Takahiro @ 2020-06-16 5:26 UTC (permalink / raw)
To: u-boot
The file, pkcs7_verify.c, will now be imported from linux code and
modified to fit into U-Boot environment.
In particular, pkcs7_verify_one() function will be used in a later patch
to rework signature verification logic aiming to support intermediate
certificates in "chain of trust."
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
lib/crypto/Kconfig | 3 +
lib/crypto/Makefile | 1 +
lib/crypto/pkcs7_verify.c | 524 ++++++++++++++++++++++++++++++++++++++
3 files changed, 528 insertions(+)
create mode 100644 lib/crypto/pkcs7_verify.c
diff --git a/lib/crypto/Kconfig b/lib/crypto/Kconfig
index 2b221b915aa6..6369bafac07b 100644
--- a/lib/crypto/Kconfig
+++ b/lib/crypto/Kconfig
@@ -49,4 +49,7 @@ config PKCS7_MESSAGE_PARSER
This option provides support for parsing PKCS#7 format messages for
signature data and provides the ability to verify the signature.
+config PKCS7_VERIFY
+ bool
+
endif # ASYMMETRIC_KEY_TYPE
diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
index 8267fee0a7b8..f3a414525d2a 100644
--- a/lib/crypto/Makefile
+++ b/lib/crypto/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_PKCS7_MESSAGE_PARSER) += pkcs7_message.o
pkcs7_message-y := \
pkcs7.asn1.o \
pkcs7_parser.o
+obj-$(CONFIG_PKCS7_VERIFY) += pkcs7_verify.o
$(obj)/pkcs7_parser.o: $(obj)/pkcs7.asn1.h
$(obj)/pkcs7.asn1.o: $(obj)/pkcs7.asn1.c $(obj)/pkcs7.asn1.h
diff --git a/lib/crypto/pkcs7_verify.c b/lib/crypto/pkcs7_verify.c
new file mode 100644
index 000000000000..1e8600f7faca
--- /dev/null
+++ b/lib/crypto/pkcs7_verify.c
@@ -0,0 +1,524 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Verify the signature on a PKCS#7 message.
+ *
+ * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells at redhat.com)
+ */
+
+#define pr_fmt(fmt) "PKCS7: "fmt
+#ifdef __UBOOT__
+#include <string.h>
+#include <linux/bitops.h>
+#include <linux/compat.h>
+#else
+#include <linux/kernel.h>
+#include <linux/export.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#endif
+#include <linux/asn1.h>
+#ifndef __UBOOT__
+#include <crypto/hash.h>
+#include <crypto/hash_info.h>
+#endif
+#include <crypto/public_key.h>
+#ifdef __UBOOT__
+#include <crypto/pkcs7_parser.h>
+#else
+#include "pkcs7_parser.h"
+#endif
+
+/*
+ * Digest the relevant parts of the PKCS#7 data
+ */
+#ifdef __UBOOT__
+static int pkcs7_digest(struct pkcs7_message *pkcs7,
+ struct pkcs7_signed_info *sinfo)
+{
+ return 0;
+}
+#else
+static int pkcs7_digest(struct pkcs7_message *pkcs7,
+ struct pkcs7_signed_info *sinfo)
+{
+ struct public_key_signature *sig = sinfo->sig;
+ struct crypto_shash *tfm;
+ struct shash_desc *desc;
+ size_t desc_size;
+ int ret;
+
+ kenter(",%u,%s", sinfo->index, sinfo->sig->hash_algo);
+
+ /* The digest was calculated already. */
+ if (sig->digest)
+ return 0;
+
+ if (!sinfo->sig->hash_algo)
+ return -ENOPKG;
+
+ /* Allocate the hashing algorithm we're going to need and find out how
+ * big the hash operational data will be.
+ */
+ tfm = crypto_alloc_shash(sinfo->sig->hash_algo, 0, 0);
+ if (IS_ERR(tfm))
+ return (PTR_ERR(tfm) == -ENOENT) ? -ENOPKG : PTR_ERR(tfm);
+
+ desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
+ sig->digest_size = crypto_shash_digestsize(tfm);
+
+ ret = -ENOMEM;
+ sig->digest = kmalloc(sig->digest_size, GFP_KERNEL);
+ if (!sig->digest)
+ goto error_no_desc;
+
+ desc = kzalloc(desc_size, GFP_KERNEL);
+ if (!desc)
+ goto error_no_desc;
+
+ desc->tfm = tfm;
+
+ /* Digest the message [RFC2315 9.3] */
+ ret = crypto_shash_digest(desc, pkcs7->data, pkcs7->data_len,
+ sig->digest);
+ if (ret < 0)
+ goto error;
+ pr_devel("MsgDigest = [%*ph]\n", 8, sig->digest);
+
+ /* However, if there are authenticated attributes, there must be a
+ * message digest attribute amongst them which corresponds to the
+ * digest we just calculated.
+ */
+ if (sinfo->authattrs) {
+ u8 tag;
+
+ if (!sinfo->msgdigest) {
+ pr_warn("Sig %u: No messageDigest\n", sinfo->index);
+ ret = -EKEYREJECTED;
+ goto error;
+ }
+
+ if (sinfo->msgdigest_len != sig->digest_size) {
+ pr_debug("Sig %u: Invalid digest size (%u)\n",
+ sinfo->index, sinfo->msgdigest_len);
+ ret = -EBADMSG;
+ goto error;
+ }
+
+ if (memcmp(sig->digest, sinfo->msgdigest,
+ sinfo->msgdigest_len) != 0) {
+ pr_debug("Sig %u: Message digest doesn't match\n",
+ sinfo->index);
+ ret = -EKEYREJECTED;
+ goto error;
+ }
+
+ /* We then calculate anew, using the authenticated attributes
+ * as the contents of the digest instead. Note that we need to
+ * convert the attributes from a CONT.0 into a SET before we
+ * hash it.
+ */
+ memset(sig->digest, 0, sig->digest_size);
+
+ ret = crypto_shash_init(desc);
+ if (ret < 0)
+ goto error;
+ tag = ASN1_CONS_BIT | ASN1_SET;
+ ret = crypto_shash_update(desc, &tag, 1);
+ if (ret < 0)
+ goto error;
+ ret = crypto_shash_finup(desc, sinfo->authattrs,
+ sinfo->authattrs_len, sig->digest);
+ if (ret < 0)
+ goto error;
+ pr_devel("AADigest = [%*ph]\n", 8, sig->digest);
+ }
+
+error:
+ kfree(desc);
+error_no_desc:
+ crypto_free_shash(tfm);
+ kleave(" = %d", ret);
+ return ret;
+}
+
+int pkcs7_get_digest(struct pkcs7_message *pkcs7, const u8 **buf, u32 *len,
+ enum hash_algo *hash_algo)
+{
+ struct pkcs7_signed_info *sinfo = pkcs7->signed_infos;
+ int i, ret;
+
+ /*
+ * This function doesn't support messages with more than one signature.
+ */
+ if (sinfo == NULL || sinfo->next != NULL)
+ return -EBADMSG;
+
+ ret = pkcs7_digest(pkcs7, sinfo);
+ if (ret)
+ return ret;
+
+ *buf = sinfo->sig->digest;
+ *len = sinfo->sig->digest_size;
+
+ for (i = 0; i < HASH_ALGO__LAST; i++)
+ if (!strcmp(hash_algo_name[i], sinfo->sig->hash_algo)) {
+ *hash_algo = i;
+ break;
+ }
+
+ return 0;
+}
+#endif /* !__UBOOT__ */
+
+/*
+ * Find the key (X.509 certificate) to use to verify a PKCS#7 message. PKCS#7
+ * uses the issuer's name and the issuing certificate serial number for
+ * matching purposes. These must match the certificate issuer's name (not
+ * subject's name) and the certificate serial number [RFC 2315 6.7].
+ */
+static int pkcs7_find_key(struct pkcs7_message *pkcs7,
+ struct pkcs7_signed_info *sinfo)
+{
+ struct x509_certificate *x509;
+ unsigned certix = 1;
+
+ kenter("%u", sinfo->index);
+
+ for (x509 = pkcs7->certs; x509; x509 = x509->next, certix++) {
+ /* I'm _assuming_ that the generator of the PKCS#7 message will
+ * encode the fields from the X.509 cert in the same way in the
+ * PKCS#7 message - but I can't be 100% sure of that. It's
+ * possible this will need element-by-element comparison.
+ */
+ if (!asymmetric_key_id_same(x509->id, sinfo->sig->auth_ids[0]))
+ continue;
+ pr_devel("Sig %u: Found cert serial match X.509[%u]\n",
+ sinfo->index, certix);
+
+ if (strcmp(x509->pub->pkey_algo, sinfo->sig->pkey_algo) != 0) {
+ pr_warn("Sig %u: X.509 algo and PKCS#7 sig algo don't match\n",
+ sinfo->index);
+ continue;
+ }
+
+ sinfo->signer = x509;
+ return 0;
+ }
+
+ /* The relevant X.509 cert isn't found here, but it might be found in
+ * the trust keyring.
+ */
+ pr_debug("Sig %u: Issuing X.509 cert not found (#%*phN)\n",
+ sinfo->index,
+ sinfo->sig->auth_ids[0]->len, sinfo->sig->auth_ids[0]->data);
+ return 0;
+}
+
+/*
+ * Verify the internal certificate chain as best we can.
+ */
+static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
+ struct pkcs7_signed_info *sinfo)
+{
+ struct public_key_signature *sig;
+ struct x509_certificate *x509 = sinfo->signer, *p;
+ struct asymmetric_key_id *auth;
+ int ret;
+
+ kenter("");
+
+ for (p = pkcs7->certs; p; p = p->next)
+ p->seen = false;
+
+ for (;;) {
+ pr_debug("verify %s: %*phN\n",
+ x509->subject,
+ x509->raw_serial_size, x509->raw_serial);
+ x509->seen = true;
+
+ if (x509->blacklisted) {
+ /* If this cert is blacklisted, then mark everything
+ * that depends on this as blacklisted too.
+ */
+ sinfo->blacklisted = true;
+ for (p = sinfo->signer; p != x509; p = p->signer)
+ p->blacklisted = true;
+ pr_debug("- blacklisted\n");
+ return 0;
+ }
+
+ if (x509->unsupported_key)
+ goto unsupported_crypto_in_x509;
+
+ pr_debug("- issuer %s\n", x509->issuer);
+ sig = x509->sig;
+ if (sig->auth_ids[0])
+ pr_debug("- authkeyid.id %*phN\n",
+ sig->auth_ids[0]->len, sig->auth_ids[0]->data);
+ if (sig->auth_ids[1])
+ pr_debug("- authkeyid.skid %*phN\n",
+ sig->auth_ids[1]->len, sig->auth_ids[1]->data);
+
+ if (x509->self_signed) {
+ /* If there's no authority certificate specified, then
+ * the certificate must be self-signed and is the root
+ * of the chain. Likewise if the cert is its own
+ * authority.
+ */
+ if (x509->unsupported_sig)
+ goto unsupported_crypto_in_x509;
+ x509->signer = x509;
+ pr_debug("- self-signed\n");
+ return 0;
+ }
+
+ /* Look through the X.509 certificates in the PKCS#7 message's
+ * list to see if the next one is there.
+ */
+ auth = sig->auth_ids[0];
+ if (auth) {
+ pr_debug("- want %*phN\n", auth->len, auth->data);
+ for (p = pkcs7->certs; p; p = p->next) {
+ pr_debug("- cmp [%u] %*phN\n",
+ p->index, p->id->len, p->id->data);
+ if (asymmetric_key_id_same(p->id, auth))
+ goto found_issuer_check_skid;
+ }
+ } else if (sig->auth_ids[1]) {
+ auth = sig->auth_ids[1];
+ pr_debug("- want %*phN\n", auth->len, auth->data);
+ for (p = pkcs7->certs; p; p = p->next) {
+ if (!p->skid)
+ continue;
+ pr_debug("- cmp [%u] %*phN\n",
+ p->index, p->skid->len, p->skid->data);
+ if (asymmetric_key_id_same(p->skid, auth))
+ goto found_issuer;
+ }
+ }
+
+ /* We didn't find the root of this chain */
+ pr_debug("- top\n");
+ return 0;
+
+ found_issuer_check_skid:
+ /* We matched issuer + serialNumber, but if there's an
+ * authKeyId.keyId, that must match the CA subjKeyId also.
+ */
+ if (sig->auth_ids[1] &&
+ !asymmetric_key_id_same(p->skid, sig->auth_ids[1])) {
+ pr_warn("Sig %u: X.509 chain contains auth-skid nonmatch (%u->%u)\n",
+ sinfo->index, x509->index, p->index);
+ return -EKEYREJECTED;
+ }
+ found_issuer:
+ pr_debug("- subject %s\n", p->subject);
+ if (p->seen) {
+ pr_warn("Sig %u: X.509 chain contains loop\n",
+ sinfo->index);
+ return 0;
+ }
+ ret = public_key_verify_signature(p->pub, x509->sig);
+ if (ret < 0)
+ return ret;
+ x509->signer = p;
+ if (x509 == p) {
+ pr_debug("- self-signed\n");
+ return 0;
+ }
+ x509 = p;
+#ifndef __UBOOT__
+ might_sleep();
+#endif
+ }
+
+unsupported_crypto_in_x509:
+ /* Just prune the certificate chain at this point if we lack some
+ * crypto module to go further. Note, however, we don't want to set
+ * sinfo->unsupported_crypto as the signed info block may still be
+ * validatable against an X.509 cert lower in the chain that we have a
+ * trusted copy of.
+ */
+ return 0;
+}
+
+/*
+ * Verify one signed information block from a PKCS#7 message.
+ */
+#ifndef __UBOOT__
+static
+#endif
+int pkcs7_verify_one(struct pkcs7_message *pkcs7,
+ struct pkcs7_signed_info *sinfo)
+{
+ int ret;
+
+ kenter(",%u", sinfo->index);
+
+ /* First of all, digest the data in the PKCS#7 message and the
+ * signed information block
+ */
+ ret = pkcs7_digest(pkcs7, sinfo);
+ if (ret < 0)
+ return ret;
+
+ /* Find the key for the signature if there is one */
+ ret = pkcs7_find_key(pkcs7, sinfo);
+ if (ret < 0)
+ return ret;
+
+ if (!sinfo->signer)
+ return 0;
+
+ pr_devel("Using X.509[%u] for sig %u\n",
+ sinfo->signer->index, sinfo->index);
+
+ /* Check that the PKCS#7 signing time is valid according to the X.509
+ * certificate. We can't, however, check against the system clock
+ * since that may not have been set yet and may be wrong.
+ */
+ if (test_bit(sinfo_has_signing_time, &sinfo->aa_set)) {
+ if (sinfo->signing_time < sinfo->signer->valid_from ||
+ sinfo->signing_time > sinfo->signer->valid_to) {
+ pr_warn("Message signed outside of X.509 validity window\n");
+ return -EKEYREJECTED;
+ }
+ }
+
+ /* Verify the PKCS#7 binary against the key */
+ ret = public_key_verify_signature(sinfo->signer->pub, sinfo->sig);
+ if (ret < 0)
+ return ret;
+
+ pr_devel("Verified signature %u\n", sinfo->index);
+
+ /* Verify the internal certificate chain */
+ return pkcs7_verify_sig_chain(pkcs7, sinfo);
+}
+
+#ifndef __UBOOT__
+/**
+ * pkcs7_verify - Verify a PKCS#7 message
+ * @pkcs7: The PKCS#7 message to be verified
+ * @usage: The use to which the key is being put
+ *
+ * Verify a PKCS#7 message is internally consistent - that is, the data digest
+ * matches the digest in the AuthAttrs and any signature in the message or one
+ * of the X.509 certificates it carries that matches another X.509 cert in the
+ * message can be verified.
+ *
+ * This does not look to match the contents of the PKCS#7 message against any
+ * external public keys.
+ *
+ * Returns, in order of descending priority:
+ *
+ * (*) -EKEYREJECTED if a key was selected that had a usage restriction at
+ * odds with the specified usage, or:
+ *
+ * (*) -EKEYREJECTED if a signature failed to match for which we found an
+ * appropriate X.509 certificate, or:
+ *
+ * (*) -EBADMSG if some part of the message was invalid, or:
+ *
+ * (*) 0 if a signature chain passed verification, or:
+ *
+ * (*) -EKEYREJECTED if a blacklisted key was encountered, or:
+ *
+ * (*) -ENOPKG if none of the signature chains are verifiable because suitable
+ * crypto modules couldn't be found.
+ */
+int pkcs7_verify(struct pkcs7_message *pkcs7,
+ enum key_being_used_for usage)
+{
+ struct pkcs7_signed_info *sinfo;
+ int actual_ret = -ENOPKG;
+ int ret;
+
+ kenter("");
+
+ switch (usage) {
+ case VERIFYING_MODULE_SIGNATURE:
+ if (pkcs7->data_type != OID_data) {
+ pr_warn("Invalid module sig (not pkcs7-data)\n");
+ return -EKEYREJECTED;
+ }
+ if (pkcs7->have_authattrs) {
+ pr_warn("Invalid module sig (has authattrs)\n");
+ return -EKEYREJECTED;
+ }
+ break;
+ case VERIFYING_FIRMWARE_SIGNATURE:
+ if (pkcs7->data_type != OID_data) {
+ pr_warn("Invalid firmware sig (not pkcs7-data)\n");
+ return -EKEYREJECTED;
+ }
+ if (!pkcs7->have_authattrs) {
+ pr_warn("Invalid firmware sig (missing authattrs)\n");
+ return -EKEYREJECTED;
+ }
+ break;
+ case VERIFYING_KEXEC_PE_SIGNATURE:
+ if (pkcs7->data_type != OID_msIndirectData) {
+ pr_warn("Invalid kexec sig (not Authenticode)\n");
+ return -EKEYREJECTED;
+ }
+ /* Authattr presence checked in parser */
+ break;
+ case VERIFYING_UNSPECIFIED_SIGNATURE:
+ if (pkcs7->data_type != OID_data) {
+ pr_warn("Invalid unspecified sig (not pkcs7-data)\n");
+ return -EKEYREJECTED;
+ }
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
+ ret = pkcs7_verify_one(pkcs7, sinfo);
+ if (sinfo->blacklisted) {
+ if (actual_ret == -ENOPKG)
+ actual_ret = -EKEYREJECTED;
+ continue;
+ }
+ if (ret < 0) {
+ if (ret == -ENOPKG) {
+ sinfo->unsupported_crypto = true;
+ continue;
+ }
+ kleave(" = %d", ret);
+ return ret;
+ }
+ actual_ret = 0;
+ }
+
+ kleave(" = %d", actual_ret);
+ return actual_ret;
+}
+EXPORT_SYMBOL_GPL(pkcs7_verify);
+
+/**
+ * pkcs7_supply_detached_data - Supply the data needed to verify a PKCS#7 message
+ * @pkcs7: The PKCS#7 message
+ * @data: The data to be verified
+ * @datalen: The amount of data
+ *
+ * Supply the detached data needed to verify a PKCS#7 message. Note that no
+ * attempt to retain/pin the data is made. That is left to the caller. The
+ * data will not be modified by pkcs7_verify() and will not be freed when the
+ * PKCS#7 message is freed.
+ *
+ * Returns -EINVAL if data is already supplied in the message, 0 otherwise.
+ */
+int pkcs7_supply_detached_data(struct pkcs7_message *pkcs7,
+ const void *data, size_t datalen)
+{
+ if (pkcs7->data) {
+ pr_debug("Data already supplied\n");
+ return -EINVAL;
+ }
+ pkcs7->data = data;
+ pkcs7->data_len = datalen;
+ return 0;
+}
+#endif /* __UBOOT__ */
--
2.27.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 4/8] lib: crypto: import pkcs7_verify.c from linux
2020-06-16 5:26 ` [PATCH v2 4/8] lib: crypto: import pkcs7_verify.c from linux AKASHI Takahiro
@ 2020-07-07 10:27 ` Heinrich Schuchardt
2020-07-08 6:30 ` AKASHI Takahiro
0 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2020-07-07 10:27 UTC (permalink / raw)
To: u-boot
On 16.06.20 07:26, AKASHI Takahiro wrote:
> The file, pkcs7_verify.c, will now be imported from linux code and
> modified to fit into U-Boot environment.
>
> In particular, pkcs7_verify_one() function will be used in a later patch
> to rework signature verification logic aiming to support intermediate
> certificates in "chain of trust."
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> lib/crypto/Kconfig | 3 +
> lib/crypto/Makefile | 1 +
> lib/crypto/pkcs7_verify.c | 524 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 528 insertions(+)
> create mode 100644 lib/crypto/pkcs7_verify.c
>
> diff --git a/lib/crypto/Kconfig b/lib/crypto/Kconfig
> index 2b221b915aa6..6369bafac07b 100644
> --- a/lib/crypto/Kconfig
> +++ b/lib/crypto/Kconfig
> @@ -49,4 +49,7 @@ config PKCS7_MESSAGE_PARSER
> This option provides support for parsing PKCS#7 format messages for
> signature data and provides the ability to verify the signature.
>
> +config PKCS7_VERIFY
> + bool
> +
> endif # ASYMMETRIC_KEY_TYPE
> diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
> index 8267fee0a7b8..f3a414525d2a 100644
> --- a/lib/crypto/Makefile
> +++ b/lib/crypto/Makefile
> @@ -44,6 +44,7 @@ obj-$(CONFIG_PKCS7_MESSAGE_PARSER) += pkcs7_message.o
> pkcs7_message-y := \
> pkcs7.asn1.o \
> pkcs7_parser.o
> +obj-$(CONFIG_PKCS7_VERIFY) += pkcs7_verify.o
>
> $(obj)/pkcs7_parser.o: $(obj)/pkcs7.asn1.h
> $(obj)/pkcs7.asn1.o: $(obj)/pkcs7.asn1.c $(obj)/pkcs7.asn1.h
> diff --git a/lib/crypto/pkcs7_verify.c b/lib/crypto/pkcs7_verify.c
> new file mode 100644
> index 000000000000..1e8600f7faca
> --- /dev/null
> +++ b/lib/crypto/pkcs7_verify.c
> @@ -0,0 +1,524 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/* Verify the signature on a PKCS#7 message.
> + *
> + * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells at redhat.com)
Please, describe the source from where you copied here:
Copied from Linux crypto/asymmetric_keys/pkcs7_verify.c.
> + */
> +
> +#define pr_fmt(fmt) "PKCS7: "fmt
This define is really meant to create unreadable code.
> +#ifdef __UBOOT__
Who needs those #ifdefs?
If you really think they are necessary to resync with the Linux code,
minimize them. One #ifdef for all headers should be enough.
> +#include <string.h>
> +#include <linux/bitops.h>
> +#include <linux/compat.h>
> +#else
> +#include <linux/kernel.h>
> +#include <linux/export.h>
> +#include <linux/slab.h>
> +#include <linux/err.h>
> +#endif
> +#include <linux/asn1.h>
> +#ifndef __UBOOT__
> +#include <crypto/hash.h>
> +#include <crypto/hash_info.h>
> +#endif
> +#include <crypto/public_key.h>
> +#ifdef __UBOOT__
> +#include <crypto/pkcs7_parser.h>
> +#else
> +#include "pkcs7_parser.h"
> +#endif
> +
> +/*
> + * Digest the relevant parts of the PKCS#7 data
> + */
> +#ifdef __UBOOT__
> +static int pkcs7_digest(struct pkcs7_message *pkcs7,
> + struct pkcs7_signed_info *sinfo)
> +{
> + return 0;
> +}
> +#else
> +static int pkcs7_digest(struct pkcs7_message *pkcs7,
> + struct pkcs7_signed_info *sinfo)
> +{
> + struct public_key_signature *sig = sinfo->sig;
> + struct crypto_shash *tfm;
> + struct shash_desc *desc;
> + size_t desc_size;
> + int ret;
> +
> + kenter(",%u,%s", sinfo->index, sinfo->sig->hash_algo);
Please, use our logging function: log_content(). At least where you
defined kenter or pr_devel.
Best regards
Heinrich
> +
> + /* The digest was calculated already. */
> + if (sig->digest)
> + return 0;
> +
> + if (!sinfo->sig->hash_algo)
> + return -ENOPKG;
> +
> + /* Allocate the hashing algorithm we're going to need and find out how
> + * big the hash operational data will be.
> + */
> + tfm = crypto_alloc_shash(sinfo->sig->hash_algo, 0, 0);
> + if (IS_ERR(tfm))
> + return (PTR_ERR(tfm) == -ENOENT) ? -ENOPKG : PTR_ERR(tfm);
> +
> + desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
> + sig->digest_size = crypto_shash_digestsize(tfm);
> +
> + ret = -ENOMEM;
> + sig->digest = kmalloc(sig->digest_size, GFP_KERNEL);
> + if (!sig->digest)
> + goto error_no_desc;
> +
> + desc = kzalloc(desc_size, GFP_KERNEL);
> + if (!desc)
> + goto error_no_desc;
> +
> + desc->tfm = tfm;
> +
> + /* Digest the message [RFC2315 9.3] */
> + ret = crypto_shash_digest(desc, pkcs7->data, pkcs7->data_len,
> + sig->digest);
> + if (ret < 0)
> + goto error;
> + pr_devel("MsgDigest = [%*ph]\n", 8, sig->digest);
> +
> + /* However, if there are authenticated attributes, there must be a
> + * message digest attribute amongst them which corresponds to the
> + * digest we just calculated.
> + */
> + if (sinfo->authattrs) {
> + u8 tag;
> +
> + if (!sinfo->msgdigest) {
> + pr_warn("Sig %u: No messageDigest\n", sinfo->index);
> + ret = -EKEYREJECTED;
> + goto error;
> + }
> +
> + if (sinfo->msgdigest_len != sig->digest_size) {
> + pr_debug("Sig %u: Invalid digest size (%u)\n",
> + sinfo->index, sinfo->msgdigest_len);
> + ret = -EBADMSG;
> + goto error;
> + }
> +
> + if (memcmp(sig->digest, sinfo->msgdigest,
> + sinfo->msgdigest_len) != 0) {
> + pr_debug("Sig %u: Message digest doesn't match\n",
> + sinfo->index);
> + ret = -EKEYREJECTED;
> + goto error;
> + }
> +
> + /* We then calculate anew, using the authenticated attributes
> + * as the contents of the digest instead. Note that we need to
> + * convert the attributes from a CONT.0 into a SET before we
> + * hash it.
> + */
> + memset(sig->digest, 0, sig->digest_size);
> +
> + ret = crypto_shash_init(desc);
> + if (ret < 0)
> + goto error;
> + tag = ASN1_CONS_BIT | ASN1_SET;
> + ret = crypto_shash_update(desc, &tag, 1);
> + if (ret < 0)
> + goto error;
> + ret = crypto_shash_finup(desc, sinfo->authattrs,
> + sinfo->authattrs_len, sig->digest);
> + if (ret < 0)
> + goto error;
> + pr_devel("AADigest = [%*ph]\n", 8, sig->digest);
> + }
> +
> +error:
> + kfree(desc);
> +error_no_desc:
> + crypto_free_shash(tfm);
> + kleave(" = %d", ret);
> + return ret;
> +}
> +
> +int pkcs7_get_digest(struct pkcs7_message *pkcs7, const u8 **buf, u32 *len,
> + enum hash_algo *hash_algo)
> +{
> + struct pkcs7_signed_info *sinfo = pkcs7->signed_infos;
> + int i, ret;
> +
> + /*
> + * This function doesn't support messages with more than one signature.
> + */
> + if (sinfo == NULL || sinfo->next != NULL)
> + return -EBADMSG;
> +
> + ret = pkcs7_digest(pkcs7, sinfo);
> + if (ret)
> + return ret;
> +
> + *buf = sinfo->sig->digest;
> + *len = sinfo->sig->digest_size;
> +
> + for (i = 0; i < HASH_ALGO__LAST; i++)
> + if (!strcmp(hash_algo_name[i], sinfo->sig->hash_algo)) {
> + *hash_algo = i;
> + break;
> + }
> +
> + return 0;
> +}
> +#endif /* !__UBOOT__ */
> +
> +/*
> + * Find the key (X.509 certificate) to use to verify a PKCS#7 message. PKCS#7
> + * uses the issuer's name and the issuing certificate serial number for
> + * matching purposes. These must match the certificate issuer's name (not
> + * subject's name) and the certificate serial number [RFC 2315 6.7].
> + */
> +static int pkcs7_find_key(struct pkcs7_message *pkcs7,
> + struct pkcs7_signed_info *sinfo)
> +{
> + struct x509_certificate *x509;
> + unsigned certix = 1;
> +
> + kenter("%u", sinfo->index);
> +
> + for (x509 = pkcs7->certs; x509; x509 = x509->next, certix++) {
> + /* I'm _assuming_ that the generator of the PKCS#7 message will
> + * encode the fields from the X.509 cert in the same way in the
> + * PKCS#7 message - but I can't be 100% sure of that. It's
> + * possible this will need element-by-element comparison.
> + */
> + if (!asymmetric_key_id_same(x509->id, sinfo->sig->auth_ids[0]))
> + continue;
> + pr_devel("Sig %u: Found cert serial match X.509[%u]\n",
> + sinfo->index, certix);
> +
> + if (strcmp(x509->pub->pkey_algo, sinfo->sig->pkey_algo) != 0) {
> + pr_warn("Sig %u: X.509 algo and PKCS#7 sig algo don't match\n",
> + sinfo->index);
> + continue;
> + }
> +
> + sinfo->signer = x509;
> + return 0;
> + }
> +
> + /* The relevant X.509 cert isn't found here, but it might be found in
> + * the trust keyring.
> + */
> + pr_debug("Sig %u: Issuing X.509 cert not found (#%*phN)\n",
> + sinfo->index,
> + sinfo->sig->auth_ids[0]->len, sinfo->sig->auth_ids[0]->data);
> + return 0;
> +}
> +
> +/*
> + * Verify the internal certificate chain as best we can.
> + */
> +static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> + struct pkcs7_signed_info *sinfo)
> +{
> + struct public_key_signature *sig;
> + struct x509_certificate *x509 = sinfo->signer, *p;
> + struct asymmetric_key_id *auth;
> + int ret;
> +
> + kenter("");
> +
> + for (p = pkcs7->certs; p; p = p->next)
> + p->seen = false;
> +
> + for (;;) {
> + pr_debug("verify %s: %*phN\n",
> + x509->subject,
> + x509->raw_serial_size, x509->raw_serial);
> + x509->seen = true;
> +
> + if (x509->blacklisted) {
> + /* If this cert is blacklisted, then mark everything
> + * that depends on this as blacklisted too.
> + */
> + sinfo->blacklisted = true;
> + for (p = sinfo->signer; p != x509; p = p->signer)
> + p->blacklisted = true;
> + pr_debug("- blacklisted\n");
> + return 0;
> + }
> +
> + if (x509->unsupported_key)
> + goto unsupported_crypto_in_x509;
> +
> + pr_debug("- issuer %s\n", x509->issuer);
> + sig = x509->sig;
> + if (sig->auth_ids[0])
> + pr_debug("- authkeyid.id %*phN\n",
> + sig->auth_ids[0]->len, sig->auth_ids[0]->data);
> + if (sig->auth_ids[1])
> + pr_debug("- authkeyid.skid %*phN\n",
> + sig->auth_ids[1]->len, sig->auth_ids[1]->data);
> +
> + if (x509->self_signed) {
> + /* If there's no authority certificate specified, then
> + * the certificate must be self-signed and is the root
> + * of the chain. Likewise if the cert is its own
> + * authority.
> + */
> + if (x509->unsupported_sig)
> + goto unsupported_crypto_in_x509;
> + x509->signer = x509;
> + pr_debug("- self-signed\n");
> + return 0;
> + }
> +
> + /* Look through the X.509 certificates in the PKCS#7 message's
> + * list to see if the next one is there.
> + */
> + auth = sig->auth_ids[0];
> + if (auth) {
> + pr_debug("- want %*phN\n", auth->len, auth->data);
> + for (p = pkcs7->certs; p; p = p->next) {
> + pr_debug("- cmp [%u] %*phN\n",
> + p->index, p->id->len, p->id->data);
> + if (asymmetric_key_id_same(p->id, auth))
> + goto found_issuer_check_skid;
> + }
> + } else if (sig->auth_ids[1]) {
> + auth = sig->auth_ids[1];
> + pr_debug("- want %*phN\n", auth->len, auth->data);
> + for (p = pkcs7->certs; p; p = p->next) {
> + if (!p->skid)
> + continue;
> + pr_debug("- cmp [%u] %*phN\n",
> + p->index, p->skid->len, p->skid->data);
> + if (asymmetric_key_id_same(p->skid, auth))
> + goto found_issuer;
> + }
> + }
> +
> + /* We didn't find the root of this chain */
> + pr_debug("- top\n");
> + return 0;
> +
> + found_issuer_check_skid:
> + /* We matched issuer + serialNumber, but if there's an
> + * authKeyId.keyId, that must match the CA subjKeyId also.
> + */
> + if (sig->auth_ids[1] &&
> + !asymmetric_key_id_same(p->skid, sig->auth_ids[1])) {
> + pr_warn("Sig %u: X.509 chain contains auth-skid nonmatch (%u->%u)\n",
> + sinfo->index, x509->index, p->index);
> + return -EKEYREJECTED;
> + }
> + found_issuer:
> + pr_debug("- subject %s\n", p->subject);
> + if (p->seen) {
> + pr_warn("Sig %u: X.509 chain contains loop\n",
> + sinfo->index);
> + return 0;
> + }
> + ret = public_key_verify_signature(p->pub, x509->sig);
> + if (ret < 0)
> + return ret;
> + x509->signer = p;
> + if (x509 == p) {
> + pr_debug("- self-signed\n");
> + return 0;
> + }
> + x509 = p;
> +#ifndef __UBOOT__
> + might_sleep();
> +#endif
> + }
> +
> +unsupported_crypto_in_x509:
> + /* Just prune the certificate chain at this point if we lack some
> + * crypto module to go further. Note, however, we don't want to set
> + * sinfo->unsupported_crypto as the signed info block may still be
> + * validatable against an X.509 cert lower in the chain that we have a
> + * trusted copy of.
> + */
> + return 0;
> +}
> +
> +/*
> + * Verify one signed information block from a PKCS#7 message.
> + */
> +#ifndef __UBOOT__
> +static
> +#endif
> +int pkcs7_verify_one(struct pkcs7_message *pkcs7,
> + struct pkcs7_signed_info *sinfo)
> +{
> + int ret;
> +
> + kenter(",%u", sinfo->index);
> +
> + /* First of all, digest the data in the PKCS#7 message and the
> + * signed information block
> + */
> + ret = pkcs7_digest(pkcs7, sinfo);
> + if (ret < 0)
> + return ret;
> +
> + /* Find the key for the signature if there is one */
> + ret = pkcs7_find_key(pkcs7, sinfo);
> + if (ret < 0)
> + return ret;
> +
> + if (!sinfo->signer)
> + return 0;
> +
> + pr_devel("Using X.509[%u] for sig %u\n",
> + sinfo->signer->index, sinfo->index);
> +
> + /* Check that the PKCS#7 signing time is valid according to the X.509
> + * certificate. We can't, however, check against the system clock
> + * since that may not have been set yet and may be wrong.
> + */
> + if (test_bit(sinfo_has_signing_time, &sinfo->aa_set)) {
> + if (sinfo->signing_time < sinfo->signer->valid_from ||
> + sinfo->signing_time > sinfo->signer->valid_to) {
> + pr_warn("Message signed outside of X.509 validity window\n");
> + return -EKEYREJECTED;
> + }
> + }
> +
> + /* Verify the PKCS#7 binary against the key */
> + ret = public_key_verify_signature(sinfo->signer->pub, sinfo->sig);
> + if (ret < 0)
> + return ret;
> +
> + pr_devel("Verified signature %u\n", sinfo->index);
> +
> + /* Verify the internal certificate chain */
> + return pkcs7_verify_sig_chain(pkcs7, sinfo);
> +}
> +
> +#ifndef __UBOOT__
> +/**
> + * pkcs7_verify - Verify a PKCS#7 message
> + * @pkcs7: The PKCS#7 message to be verified
> + * @usage: The use to which the key is being put
> + *
> + * Verify a PKCS#7 message is internally consistent - that is, the data digest
> + * matches the digest in the AuthAttrs and any signature in the message or one
> + * of the X.509 certificates it carries that matches another X.509 cert in the
> + * message can be verified.
> + *
> + * This does not look to match the contents of the PKCS#7 message against any
> + * external public keys.
> + *
> + * Returns, in order of descending priority:
> + *
> + * (*) -EKEYREJECTED if a key was selected that had a usage restriction at
> + * odds with the specified usage, or:
> + *
> + * (*) -EKEYREJECTED if a signature failed to match for which we found an
> + * appropriate X.509 certificate, or:
> + *
> + * (*) -EBADMSG if some part of the message was invalid, or:
> + *
> + * (*) 0 if a signature chain passed verification, or:
> + *
> + * (*) -EKEYREJECTED if a blacklisted key was encountered, or:
> + *
> + * (*) -ENOPKG if none of the signature chains are verifiable because suitable
> + * crypto modules couldn't be found.
> + */
> +int pkcs7_verify(struct pkcs7_message *pkcs7,
> + enum key_being_used_for usage)
> +{
> + struct pkcs7_signed_info *sinfo;
> + int actual_ret = -ENOPKG;
> + int ret;
> +
> + kenter("");
> +
> + switch (usage) {
> + case VERIFYING_MODULE_SIGNATURE:
> + if (pkcs7->data_type != OID_data) {
> + pr_warn("Invalid module sig (not pkcs7-data)\n");
> + return -EKEYREJECTED;
> + }
> + if (pkcs7->have_authattrs) {
> + pr_warn("Invalid module sig (has authattrs)\n");
> + return -EKEYREJECTED;
> + }
> + break;
> + case VERIFYING_FIRMWARE_SIGNATURE:
> + if (pkcs7->data_type != OID_data) {
> + pr_warn("Invalid firmware sig (not pkcs7-data)\n");
> + return -EKEYREJECTED;
> + }
> + if (!pkcs7->have_authattrs) {
> + pr_warn("Invalid firmware sig (missing authattrs)\n");
> + return -EKEYREJECTED;
> + }
> + break;
> + case VERIFYING_KEXEC_PE_SIGNATURE:
> + if (pkcs7->data_type != OID_msIndirectData) {
> + pr_warn("Invalid kexec sig (not Authenticode)\n");
> + return -EKEYREJECTED;
> + }
> + /* Authattr presence checked in parser */
> + break;
> + case VERIFYING_UNSPECIFIED_SIGNATURE:
> + if (pkcs7->data_type != OID_data) {
> + pr_warn("Invalid unspecified sig (not pkcs7-data)\n");
> + return -EKEYREJECTED;
> + }
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
> + ret = pkcs7_verify_one(pkcs7, sinfo);
> + if (sinfo->blacklisted) {
> + if (actual_ret == -ENOPKG)
> + actual_ret = -EKEYREJECTED;
> + continue;
> + }
> + if (ret < 0) {
> + if (ret == -ENOPKG) {
> + sinfo->unsupported_crypto = true;
> + continue;
> + }
> + kleave(" = %d", ret);
> + return ret;
> + }
> + actual_ret = 0;
> + }
> +
> + kleave(" = %d", actual_ret);
> + return actual_ret;
> +}
> +EXPORT_SYMBOL_GPL(pkcs7_verify);
> +
> +/**
> + * pkcs7_supply_detached_data - Supply the data needed to verify a PKCS#7 message
> + * @pkcs7: The PKCS#7 message
> + * @data: The data to be verified
> + * @datalen: The amount of data
> + *
> + * Supply the detached data needed to verify a PKCS#7 message. Note that no
> + * attempt to retain/pin the data is made. That is left to the caller. The
> + * data will not be modified by pkcs7_verify() and will not be freed when the
> + * PKCS#7 message is freed.
> + *
> + * Returns -EINVAL if data is already supplied in the message, 0 otherwise.
> + */
> +int pkcs7_supply_detached_data(struct pkcs7_message *pkcs7,
> + const void *data, size_t datalen)
> +{
> + if (pkcs7->data) {
> + pr_debug("Data already supplied\n");
> + return -EINVAL;
> + }
> + pkcs7->data = data;
> + pkcs7->data_len = datalen;
> + return 0;
> +}
> +#endif /* __UBOOT__ */
>
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 4/8] lib: crypto: import pkcs7_verify.c from linux
2020-07-07 10:27 ` Heinrich Schuchardt
@ 2020-07-08 6:30 ` AKASHI Takahiro
0 siblings, 0 replies; 25+ messages in thread
From: AKASHI Takahiro @ 2020-07-08 6:30 UTC (permalink / raw)
To: u-boot
Heinrich,
On Tue, Jul 07, 2020 at 12:27:49PM +0200, Heinrich Schuchardt wrote:
> On 16.06.20 07:26, AKASHI Takahiro wrote:
> > The file, pkcs7_verify.c, will now be imported from linux code and
> > modified to fit into U-Boot environment.
> >
> > In particular, pkcs7_verify_one() function will be used in a later patch
> > to rework signature verification logic aiming to support intermediate
> > certificates in "chain of trust."
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > lib/crypto/Kconfig | 3 +
> > lib/crypto/Makefile | 1 +
> > lib/crypto/pkcs7_verify.c | 524 ++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 528 insertions(+)
> > create mode 100644 lib/crypto/pkcs7_verify.c
> >
> > diff --git a/lib/crypto/Kconfig b/lib/crypto/Kconfig
> > index 2b221b915aa6..6369bafac07b 100644
> > --- a/lib/crypto/Kconfig
> > +++ b/lib/crypto/Kconfig
> > @@ -49,4 +49,7 @@ config PKCS7_MESSAGE_PARSER
> > This option provides support for parsing PKCS#7 format messages for
> > signature data and provides the ability to verify the signature.
> >
> > +config PKCS7_VERIFY
> > + bool
> > +
> > endif # ASYMMETRIC_KEY_TYPE
> > diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
> > index 8267fee0a7b8..f3a414525d2a 100644
> > --- a/lib/crypto/Makefile
> > +++ b/lib/crypto/Makefile
> > @@ -44,6 +44,7 @@ obj-$(CONFIG_PKCS7_MESSAGE_PARSER) += pkcs7_message.o
> > pkcs7_message-y := \
> > pkcs7.asn1.o \
> > pkcs7_parser.o
> > +obj-$(CONFIG_PKCS7_VERIFY) += pkcs7_verify.o
> >
> > $(obj)/pkcs7_parser.o: $(obj)/pkcs7.asn1.h
> > $(obj)/pkcs7.asn1.o: $(obj)/pkcs7.asn1.c $(obj)/pkcs7.asn1.h
> > diff --git a/lib/crypto/pkcs7_verify.c b/lib/crypto/pkcs7_verify.c
> > new file mode 100644
> > index 000000000000..1e8600f7faca
> > --- /dev/null
> > +++ b/lib/crypto/pkcs7_verify.c
> > @@ -0,0 +1,524 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/* Verify the signature on a PKCS#7 message.
> > + *
> > + * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
> > + * Written by David Howells (dhowells at redhat.com)
>
> Please, describe the source from where you copied here:
>
> Copied from Linux crypto/asymmetric_keys/pkcs7_verify.c.
Okay though I've never seen similar comments when other files
under lib/crypto were imported from linux.
> > + */
> > +
> > +#define pr_fmt(fmt) "PKCS7: "fmt
>
> This define is really meant to create unreadable code.
>
> > +#ifdef __UBOOT__
>
> Who needs those #ifdefs?
>
> If you really think they are necessary to resync with the Linux code,
> minimize them. One #ifdef for all headers should be enough.
>
> > +#include <string.h>
> > +#include <linux/bitops.h>
> > +#include <linux/compat.h>
> > +#else
> > +#include <linux/kernel.h>
> > +#include <linux/export.h>
> > +#include <linux/slab.h>
> > +#include <linux/err.h>
> > +#endif
> > +#include <linux/asn1.h>
> > +#ifndef __UBOOT__
> > +#include <crypto/hash.h>
> > +#include <crypto/hash_info.h>
> > +#endif
> > +#include <crypto/public_key.h>
> > +#ifdef __UBOOT__
> > +#include <crypto/pkcs7_parser.h>
> > +#else
> > +#include "pkcs7_parser.h"
> > +#endif
Okay, I will squash them.
> > +
> > +/*
> > + * Digest the relevant parts of the PKCS#7 data
> > + */
> > +#ifdef __UBOOT__
> > +static int pkcs7_digest(struct pkcs7_message *pkcs7,
> > + struct pkcs7_signed_info *sinfo)
> > +{
> > + return 0;
> > +}
> > +#else
> > +static int pkcs7_digest(struct pkcs7_message *pkcs7,
> > + struct pkcs7_signed_info *sinfo)
> > +{
> > + struct public_key_signature *sig = sinfo->sig;
> > + struct crypto_shash *tfm;
> > + struct shash_desc *desc;
> > + size_t desc_size;
> > + int ret;
> > +
> > + kenter(",%u,%s", sinfo->index, sinfo->sig->hash_algo);
>
> Please, use our logging function: log_content(). At least where you
> defined kenter or pr_devel.
Again, no.
Please read my comment in patch 3/8.
-Takahiro Akashi
> Best regards
>
> Heinrich
>
> > +
> > + /* The digest was calculated already. */
> > + if (sig->digest)
> > + return 0;
> > +
> > + if (!sinfo->sig->hash_algo)
> > + return -ENOPKG;
> > +
> > + /* Allocate the hashing algorithm we're going to need and find out how
> > + * big the hash operational data will be.
> > + */
> > + tfm = crypto_alloc_shash(sinfo->sig->hash_algo, 0, 0);
> > + if (IS_ERR(tfm))
> > + return (PTR_ERR(tfm) == -ENOENT) ? -ENOPKG : PTR_ERR(tfm);
> > +
> > + desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
> > + sig->digest_size = crypto_shash_digestsize(tfm);
> > +
> > + ret = -ENOMEM;
> > + sig->digest = kmalloc(sig->digest_size, GFP_KERNEL);
> > + if (!sig->digest)
> > + goto error_no_desc;
> > +
> > + desc = kzalloc(desc_size, GFP_KERNEL);
> > + if (!desc)
> > + goto error_no_desc;
> > +
> > + desc->tfm = tfm;
> > +
> > + /* Digest the message [RFC2315 9.3] */
> > + ret = crypto_shash_digest(desc, pkcs7->data, pkcs7->data_len,
> > + sig->digest);
> > + if (ret < 0)
> > + goto error;
> > + pr_devel("MsgDigest = [%*ph]\n", 8, sig->digest);
> > +
> > + /* However, if there are authenticated attributes, there must be a
> > + * message digest attribute amongst them which corresponds to the
> > + * digest we just calculated.
> > + */
> > + if (sinfo->authattrs) {
> > + u8 tag;
> > +
> > + if (!sinfo->msgdigest) {
> > + pr_warn("Sig %u: No messageDigest\n", sinfo->index);
> > + ret = -EKEYREJECTED;
> > + goto error;
> > + }
> > +
> > + if (sinfo->msgdigest_len != sig->digest_size) {
> > + pr_debug("Sig %u: Invalid digest size (%u)\n",
> > + sinfo->index, sinfo->msgdigest_len);
> > + ret = -EBADMSG;
> > + goto error;
> > + }
> > +
> > + if (memcmp(sig->digest, sinfo->msgdigest,
> > + sinfo->msgdigest_len) != 0) {
> > + pr_debug("Sig %u: Message digest doesn't match\n",
> > + sinfo->index);
> > + ret = -EKEYREJECTED;
> > + goto error;
> > + }
> > +
> > + /* We then calculate anew, using the authenticated attributes
> > + * as the contents of the digest instead. Note that we need to
> > + * convert the attributes from a CONT.0 into a SET before we
> > + * hash it.
> > + */
> > + memset(sig->digest, 0, sig->digest_size);
> > +
> > + ret = crypto_shash_init(desc);
> > + if (ret < 0)
> > + goto error;
> > + tag = ASN1_CONS_BIT | ASN1_SET;
> > + ret = crypto_shash_update(desc, &tag, 1);
> > + if (ret < 0)
> > + goto error;
> > + ret = crypto_shash_finup(desc, sinfo->authattrs,
> > + sinfo->authattrs_len, sig->digest);
> > + if (ret < 0)
> > + goto error;
> > + pr_devel("AADigest = [%*ph]\n", 8, sig->digest);
> > + }
> > +
> > +error:
> > + kfree(desc);
> > +error_no_desc:
> > + crypto_free_shash(tfm);
> > + kleave(" = %d", ret);
> > + return ret;
> > +}
> > +
> > +int pkcs7_get_digest(struct pkcs7_message *pkcs7, const u8 **buf, u32 *len,
> > + enum hash_algo *hash_algo)
> > +{
> > + struct pkcs7_signed_info *sinfo = pkcs7->signed_infos;
> > + int i, ret;
> > +
> > + /*
> > + * This function doesn't support messages with more than one signature.
> > + */
> > + if (sinfo == NULL || sinfo->next != NULL)
> > + return -EBADMSG;
> > +
> > + ret = pkcs7_digest(pkcs7, sinfo);
> > + if (ret)
> > + return ret;
> > +
> > + *buf = sinfo->sig->digest;
> > + *len = sinfo->sig->digest_size;
> > +
> > + for (i = 0; i < HASH_ALGO__LAST; i++)
> > + if (!strcmp(hash_algo_name[i], sinfo->sig->hash_algo)) {
> > + *hash_algo = i;
> > + break;
> > + }
> > +
> > + return 0;
> > +}
> > +#endif /* !__UBOOT__ */
> > +
> > +/*
> > + * Find the key (X.509 certificate) to use to verify a PKCS#7 message. PKCS#7
> > + * uses the issuer's name and the issuing certificate serial number for
> > + * matching purposes. These must match the certificate issuer's name (not
> > + * subject's name) and the certificate serial number [RFC 2315 6.7].
> > + */
> > +static int pkcs7_find_key(struct pkcs7_message *pkcs7,
> > + struct pkcs7_signed_info *sinfo)
> > +{
> > + struct x509_certificate *x509;
> > + unsigned certix = 1;
> > +
> > + kenter("%u", sinfo->index);
> > +
> > + for (x509 = pkcs7->certs; x509; x509 = x509->next, certix++) {
> > + /* I'm _assuming_ that the generator of the PKCS#7 message will
> > + * encode the fields from the X.509 cert in the same way in the
> > + * PKCS#7 message - but I can't be 100% sure of that. It's
> > + * possible this will need element-by-element comparison.
> > + */
> > + if (!asymmetric_key_id_same(x509->id, sinfo->sig->auth_ids[0]))
> > + continue;
> > + pr_devel("Sig %u: Found cert serial match X.509[%u]\n",
> > + sinfo->index, certix);
> > +
> > + if (strcmp(x509->pub->pkey_algo, sinfo->sig->pkey_algo) != 0) {
> > + pr_warn("Sig %u: X.509 algo and PKCS#7 sig algo don't match\n",
> > + sinfo->index);
> > + continue;
> > + }
> > +
> > + sinfo->signer = x509;
> > + return 0;
> > + }
> > +
> > + /* The relevant X.509 cert isn't found here, but it might be found in
> > + * the trust keyring.
> > + */
> > + pr_debug("Sig %u: Issuing X.509 cert not found (#%*phN)\n",
> > + sinfo->index,
> > + sinfo->sig->auth_ids[0]->len, sinfo->sig->auth_ids[0]->data);
> > + return 0;
> > +}
> > +
> > +/*
> > + * Verify the internal certificate chain as best we can.
> > + */
> > +static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> > + struct pkcs7_signed_info *sinfo)
> > +{
> > + struct public_key_signature *sig;
> > + struct x509_certificate *x509 = sinfo->signer, *p;
> > + struct asymmetric_key_id *auth;
> > + int ret;
> > +
> > + kenter("");
> > +
> > + for (p = pkcs7->certs; p; p = p->next)
> > + p->seen = false;
> > +
> > + for (;;) {
> > + pr_debug("verify %s: %*phN\n",
> > + x509->subject,
> > + x509->raw_serial_size, x509->raw_serial);
> > + x509->seen = true;
> > +
> > + if (x509->blacklisted) {
> > + /* If this cert is blacklisted, then mark everything
> > + * that depends on this as blacklisted too.
> > + */
> > + sinfo->blacklisted = true;
> > + for (p = sinfo->signer; p != x509; p = p->signer)
> > + p->blacklisted = true;
> > + pr_debug("- blacklisted\n");
> > + return 0;
> > + }
> > +
> > + if (x509->unsupported_key)
> > + goto unsupported_crypto_in_x509;
> > +
> > + pr_debug("- issuer %s\n", x509->issuer);
> > + sig = x509->sig;
> > + if (sig->auth_ids[0])
> > + pr_debug("- authkeyid.id %*phN\n",
> > + sig->auth_ids[0]->len, sig->auth_ids[0]->data);
> > + if (sig->auth_ids[1])
> > + pr_debug("- authkeyid.skid %*phN\n",
> > + sig->auth_ids[1]->len, sig->auth_ids[1]->data);
> > +
> > + if (x509->self_signed) {
> > + /* If there's no authority certificate specified, then
> > + * the certificate must be self-signed and is the root
> > + * of the chain. Likewise if the cert is its own
> > + * authority.
> > + */
> > + if (x509->unsupported_sig)
> > + goto unsupported_crypto_in_x509;
> > + x509->signer = x509;
> > + pr_debug("- self-signed\n");
> > + return 0;
> > + }
> > +
> > + /* Look through the X.509 certificates in the PKCS#7 message's
> > + * list to see if the next one is there.
> > + */
> > + auth = sig->auth_ids[0];
> > + if (auth) {
> > + pr_debug("- want %*phN\n", auth->len, auth->data);
> > + for (p = pkcs7->certs; p; p = p->next) {
> > + pr_debug("- cmp [%u] %*phN\n",
> > + p->index, p->id->len, p->id->data);
> > + if (asymmetric_key_id_same(p->id, auth))
> > + goto found_issuer_check_skid;
> > + }
> > + } else if (sig->auth_ids[1]) {
> > + auth = sig->auth_ids[1];
> > + pr_debug("- want %*phN\n", auth->len, auth->data);
> > + for (p = pkcs7->certs; p; p = p->next) {
> > + if (!p->skid)
> > + continue;
> > + pr_debug("- cmp [%u] %*phN\n",
> > + p->index, p->skid->len, p->skid->data);
> > + if (asymmetric_key_id_same(p->skid, auth))
> > + goto found_issuer;
> > + }
> > + }
> > +
> > + /* We didn't find the root of this chain */
> > + pr_debug("- top\n");
> > + return 0;
> > +
> > + found_issuer_check_skid:
> > + /* We matched issuer + serialNumber, but if there's an
> > + * authKeyId.keyId, that must match the CA subjKeyId also.
> > + */
> > + if (sig->auth_ids[1] &&
> > + !asymmetric_key_id_same(p->skid, sig->auth_ids[1])) {
> > + pr_warn("Sig %u: X.509 chain contains auth-skid nonmatch (%u->%u)\n",
> > + sinfo->index, x509->index, p->index);
> > + return -EKEYREJECTED;
> > + }
> > + found_issuer:
> > + pr_debug("- subject %s\n", p->subject);
> > + if (p->seen) {
> > + pr_warn("Sig %u: X.509 chain contains loop\n",
> > + sinfo->index);
> > + return 0;
> > + }
> > + ret = public_key_verify_signature(p->pub, x509->sig);
> > + if (ret < 0)
> > + return ret;
> > + x509->signer = p;
> > + if (x509 == p) {
> > + pr_debug("- self-signed\n");
> > + return 0;
> > + }
> > + x509 = p;
> > +#ifndef __UBOOT__
> > + might_sleep();
> > +#endif
> > + }
> > +
> > +unsupported_crypto_in_x509:
> > + /* Just prune the certificate chain at this point if we lack some
> > + * crypto module to go further. Note, however, we don't want to set
> > + * sinfo->unsupported_crypto as the signed info block may still be
> > + * validatable against an X.509 cert lower in the chain that we have a
> > + * trusted copy of.
> > + */
> > + return 0;
> > +}
> > +
> > +/*
> > + * Verify one signed information block from a PKCS#7 message.
> > + */
> > +#ifndef __UBOOT__
> > +static
> > +#endif
> > +int pkcs7_verify_one(struct pkcs7_message *pkcs7,
> > + struct pkcs7_signed_info *sinfo)
> > +{
> > + int ret;
> > +
> > + kenter(",%u", sinfo->index);
> > +
> > + /* First of all, digest the data in the PKCS#7 message and the
> > + * signed information block
> > + */
> > + ret = pkcs7_digest(pkcs7, sinfo);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /* Find the key for the signature if there is one */
> > + ret = pkcs7_find_key(pkcs7, sinfo);
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (!sinfo->signer)
> > + return 0;
> > +
> > + pr_devel("Using X.509[%u] for sig %u\n",
> > + sinfo->signer->index, sinfo->index);
> > +
> > + /* Check that the PKCS#7 signing time is valid according to the X.509
> > + * certificate. We can't, however, check against the system clock
> > + * since that may not have been set yet and may be wrong.
> > + */
> > + if (test_bit(sinfo_has_signing_time, &sinfo->aa_set)) {
> > + if (sinfo->signing_time < sinfo->signer->valid_from ||
> > + sinfo->signing_time > sinfo->signer->valid_to) {
> > + pr_warn("Message signed outside of X.509 validity window\n");
> > + return -EKEYREJECTED;
> > + }
> > + }
> > +
> > + /* Verify the PKCS#7 binary against the key */
> > + ret = public_key_verify_signature(sinfo->signer->pub, sinfo->sig);
> > + if (ret < 0)
> > + return ret;
> > +
> > + pr_devel("Verified signature %u\n", sinfo->index);
> > +
> > + /* Verify the internal certificate chain */
> > + return pkcs7_verify_sig_chain(pkcs7, sinfo);
> > +}
> > +
> > +#ifndef __UBOOT__
> > +/**
> > + * pkcs7_verify - Verify a PKCS#7 message
> > + * @pkcs7: The PKCS#7 message to be verified
> > + * @usage: The use to which the key is being put
> > + *
> > + * Verify a PKCS#7 message is internally consistent - that is, the data digest
> > + * matches the digest in the AuthAttrs and any signature in the message or one
> > + * of the X.509 certificates it carries that matches another X.509 cert in the
> > + * message can be verified.
> > + *
> > + * This does not look to match the contents of the PKCS#7 message against any
> > + * external public keys.
> > + *
> > + * Returns, in order of descending priority:
> > + *
> > + * (*) -EKEYREJECTED if a key was selected that had a usage restriction at
> > + * odds with the specified usage, or:
> > + *
> > + * (*) -EKEYREJECTED if a signature failed to match for which we found an
> > + * appropriate X.509 certificate, or:
> > + *
> > + * (*) -EBADMSG if some part of the message was invalid, or:
> > + *
> > + * (*) 0 if a signature chain passed verification, or:
> > + *
> > + * (*) -EKEYREJECTED if a blacklisted key was encountered, or:
> > + *
> > + * (*) -ENOPKG if none of the signature chains are verifiable because suitable
> > + * crypto modules couldn't be found.
> > + */
> > +int pkcs7_verify(struct pkcs7_message *pkcs7,
> > + enum key_being_used_for usage)
> > +{
> > + struct pkcs7_signed_info *sinfo;
> > + int actual_ret = -ENOPKG;
> > + int ret;
> > +
> > + kenter("");
> > +
> > + switch (usage) {
> > + case VERIFYING_MODULE_SIGNATURE:
> > + if (pkcs7->data_type != OID_data) {
> > + pr_warn("Invalid module sig (not pkcs7-data)\n");
> > + return -EKEYREJECTED;
> > + }
> > + if (pkcs7->have_authattrs) {
> > + pr_warn("Invalid module sig (has authattrs)\n");
> > + return -EKEYREJECTED;
> > + }
> > + break;
> > + case VERIFYING_FIRMWARE_SIGNATURE:
> > + if (pkcs7->data_type != OID_data) {
> > + pr_warn("Invalid firmware sig (not pkcs7-data)\n");
> > + return -EKEYREJECTED;
> > + }
> > + if (!pkcs7->have_authattrs) {
> > + pr_warn("Invalid firmware sig (missing authattrs)\n");
> > + return -EKEYREJECTED;
> > + }
> > + break;
> > + case VERIFYING_KEXEC_PE_SIGNATURE:
> > + if (pkcs7->data_type != OID_msIndirectData) {
> > + pr_warn("Invalid kexec sig (not Authenticode)\n");
> > + return -EKEYREJECTED;
> > + }
> > + /* Authattr presence checked in parser */
> > + break;
> > + case VERIFYING_UNSPECIFIED_SIGNATURE:
> > + if (pkcs7->data_type != OID_data) {
> > + pr_warn("Invalid unspecified sig (not pkcs7-data)\n");
> > + return -EKEYREJECTED;
> > + }
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
> > + ret = pkcs7_verify_one(pkcs7, sinfo);
> > + if (sinfo->blacklisted) {
> > + if (actual_ret == -ENOPKG)
> > + actual_ret = -EKEYREJECTED;
> > + continue;
> > + }
> > + if (ret < 0) {
> > + if (ret == -ENOPKG) {
> > + sinfo->unsupported_crypto = true;
> > + continue;
> > + }
> > + kleave(" = %d", ret);
> > + return ret;
> > + }
> > + actual_ret = 0;
> > + }
> > +
> > + kleave(" = %d", actual_ret);
> > + return actual_ret;
> > +}
> > +EXPORT_SYMBOL_GPL(pkcs7_verify);
> > +
> > +/**
> > + * pkcs7_supply_detached_data - Supply the data needed to verify a PKCS#7 message
> > + * @pkcs7: The PKCS#7 message
> > + * @data: The data to be verified
> > + * @datalen: The amount of data
> > + *
> > + * Supply the detached data needed to verify a PKCS#7 message. Note that no
> > + * attempt to retain/pin the data is made. That is left to the caller. The
> > + * data will not be modified by pkcs7_verify() and will not be freed when the
> > + * PKCS#7 message is freed.
> > + *
> > + * Returns -EINVAL if data is already supplied in the message, 0 otherwise.
> > + */
> > +int pkcs7_supply_detached_data(struct pkcs7_message *pkcs7,
> > + const void *data, size_t datalen)
> > +{
> > + if (pkcs7->data) {
> > + pr_debug("Data already supplied\n");
> > + return -EINVAL;
> > + }
> > + pkcs7->data = data;
> > + pkcs7->data_len = datalen;
> > + return 0;
> > +}
> > +#endif /* __UBOOT__ */
> >
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 5/8] lib: crypto: add pkcs7_digest()
2020-06-16 5:26 [PATCH v2 0/8] efi_loader: secure boot: support intermediate certificates in signature AKASHI Takahiro
` (3 preceding siblings ...)
2020-06-16 5:26 ` [PATCH v2 4/8] lib: crypto: import pkcs7_verify.c from linux AKASHI Takahiro
@ 2020-06-16 5:26 ` AKASHI Takahiro
2020-06-16 5:26 ` [PATCH v2 6/8] lib: crypto: export and enhance pkcs7_verify_one() AKASHI Takahiro
` (2 subsequent siblings)
7 siblings, 0 replies; 25+ messages in thread
From: AKASHI Takahiro @ 2020-06-16 5:26 UTC (permalink / raw)
To: u-boot
This function was nullified when the file, pkcs7_verify.c, was imported
because it calls further linux-specific interfaces inside, hence that
could lead to more files being imported from linux.
We need this function in pkcs7_verify_one() and so simply re-implement it
here instead of re-using the code.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
lib/crypto/pkcs7_verify.c | 95 +++++++++++++++++++++++++++++++++++++--
1 file changed, 91 insertions(+), 4 deletions(-)
diff --git a/lib/crypto/pkcs7_verify.c b/lib/crypto/pkcs7_verify.c
index 1e8600f7faca..9b9030ea4440 100644
--- a/lib/crypto/pkcs7_verify.c
+++ b/lib/crypto/pkcs7_verify.c
@@ -17,7 +17,10 @@
#include <linux/err.h>
#endif
#include <linux/asn1.h>
-#ifndef __UBOOT__
+#ifdef __UBOOT__
+#include <image.h>
+#include <u-boot/rsa-checksum.h>
+#else
#include <crypto/hash.h>
#include <crypto/hash_info.h>
#endif
@@ -29,15 +32,99 @@
#endif
/*
- * Digest the relevant parts of the PKCS#7 data
+ * pkcs7_digest - Digest the relevant parts of the PKCS#7 data
+ * @pkcs7: PKCS7 Signed Data
+ * @sinfo: PKCS7 Signed Info
+ *
+ * Digest the relevant parts of the PKCS#7 data, @pkcs7, using signature
+ * information in @sinfo. But if there are authentication attributes,
+ * i.e. signed image case, the digest must be calculated against
+ * the authentication attributes.
+ *
+ * Return: 0 - on success, non-zero error code - otherwise
*/
#ifdef __UBOOT__
static int pkcs7_digest(struct pkcs7_message *pkcs7,
struct pkcs7_signed_info *sinfo)
{
- return 0;
+ struct public_key_signature *sig = sinfo->sig;
+ struct image_region regions[2];
+ int ret = 0;
+
+ /* The digest was calculated already. */
+ if (sig->digest)
+ return 0;
+
+ if (!sinfo->sig->hash_algo)
+ return -ENOPKG;
+ if (!strcmp(sinfo->sig->hash_algo, "sha256"))
+ sig->digest_size = SHA256_SUM_LEN;
+ else if (!strcmp(sinfo->sig->hash_algo, "sha1"))
+ sig->digest_size = SHA1_SUM_LEN;
+ else
+ return -ENOPKG;
+
+ sig->digest = calloc(1, sig->digest_size);
+ if (!sig->digest) {
+ pr_warn("Sig %u: Out of memory\n", sinfo->index);
+ return -ENOMEM;
+ }
+
+ regions[0].data = pkcs7->data;
+ regions[0].size = pkcs7->data_len;
+
+ /* Digest the message [RFC2315 9.3] */
+ hash_calculate(sinfo->sig->hash_algo, regions, 1, sig->digest);
+
+ /* However, if there are authenticated attributes, there must be a
+ * message digest attribute amongst them which corresponds to the
+ * digest we just calculated.
+ */
+ if (sinfo->authattrs) {
+ u8 tag;
+
+ if (!sinfo->msgdigest) {
+ pr_warn("Sig %u: No messageDigest\n", sinfo->index);
+ ret = -EKEYREJECTED;
+ goto error;
+ }
+
+ if (sinfo->msgdigest_len != sig->digest_size) {
+ pr_debug("Sig %u: Invalid digest size (%u)\n",
+ sinfo->index, sinfo->msgdigest_len);
+ ret = -EBADMSG;
+ goto error;
+ }
+
+ if (memcmp(sig->digest, sinfo->msgdigest,
+ sinfo->msgdigest_len) != 0) {
+ pr_debug("Sig %u: Message digest doesn't match\n",
+ sinfo->index);
+ ret = -EKEYREJECTED;
+ goto error;
+ }
+
+ /* We then calculate anew, using the authenticated attributes
+ * as the contents of the digest instead. Note that we need to
+ * convert the attributes from a CONT.0 into a SET before we
+ * hash it.
+ */
+ memset(sig->digest, 0, sig->digest_size);
+
+ tag = 0x31;
+ regions[0].data = &tag;
+ regions[0].size = 1;
+ regions[1].data = sinfo->authattrs;
+ regions[1].size = sinfo->authattrs_len;
+
+ hash_calculate(sinfo->sig->hash_algo, regions, 2, sig->digest);
+
+ ret = 0;
+ }
+error:
+ return ret;
}
-#else
+#else /* !__UBOOT__ */
static int pkcs7_digest(struct pkcs7_message *pkcs7,
struct pkcs7_signed_info *sinfo)
{
--
2.27.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 6/8] lib: crypto: export and enhance pkcs7_verify_one()
2020-06-16 5:26 [PATCH v2 0/8] efi_loader: secure boot: support intermediate certificates in signature AKASHI Takahiro
` (4 preceding siblings ...)
2020-06-16 5:26 ` [PATCH v2 5/8] lib: crypto: add pkcs7_digest() AKASHI Takahiro
@ 2020-06-16 5:26 ` AKASHI Takahiro
2020-07-07 10:32 ` Heinrich Schuchardt
2020-06-16 5:26 ` [PATCH v2 7/8] efi_loader: signature: rework for intermediate certificates support AKASHI Takahiro
2020-06-16 5:26 ` [PATCH v2 8/8] test/py: efi_secboot: add test for intermediate certificates AKASHI Takahiro
7 siblings, 1 reply; 25+ messages in thread
From: AKASHI Takahiro @ 2020-06-16 5:26 UTC (permalink / raw)
To: u-boot
The function, pkcs7_verify_one(), will be utilized to rework signature
verification logic aiming to support intermediate certificates in
"chain of trust."
To do that, its function interface is expanded, adding an extra argument
which is expected to return the last certificate in trusted chain.
Then, this last one must further be verified with signature database, db
and/or dbx.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
include/crypto/pkcs7.h | 9 +++++-
lib/crypto/pkcs7_verify.c | 61 ++++++++++++++++++++++++++++++++++-----
2 files changed, 62 insertions(+), 8 deletions(-)
diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
index 8f5c8a7ee3b9..ca35df29f6fb 100644
--- a/include/crypto/pkcs7.h
+++ b/include/crypto/pkcs7.h
@@ -27,7 +27,14 @@ extern int pkcs7_get_content_data(const struct pkcs7_message *pkcs7,
const void **_data, size_t *_datalen,
size_t *_headerlen);
-#ifndef __UBOOT__
+#ifdef __UBOOT__
+struct pkcs7_signed_info;
+struct x509_certificate;
+
+int pkcs7_verify_one(struct pkcs7_message *pkcs7,
+ struct pkcs7_signed_info *sinfo,
+ struct x509_certificate **signer);
+#else
/*
* pkcs7_trust.c
*/
diff --git a/lib/crypto/pkcs7_verify.c b/lib/crypto/pkcs7_verify.c
index 9b9030ea4440..dda96ccf57a2 100644
--- a/lib/crypto/pkcs7_verify.c
+++ b/lib/crypto/pkcs7_verify.c
@@ -302,10 +302,27 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
}
/*
- * Verify the internal certificate chain as best we can.
+ * pkcs7_verify_sig_chain - Verify the internal certificate chain as best
+ * as we can.
+ * @pkcs7: PKCS7 Signed Data
+ * @sinfo: PKCS7 Signed Info
+ * @signer: Singer's certificate
+ *
+ * Build up and verify the internal certificate chain against a signature
+ * in @sinfo, using certificates contained in @pkcs7 as best as we can.
+ * If the chain reaches the end, the last certificate will be returned
+ * in @signer.
+ *
+ * Return: 0 - on success, non-zero error code - otherwise
*/
+#ifdef __UBOOT__
+static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
+ struct pkcs7_signed_info *sinfo,
+ struct x509_certificate **signer)
+#else
static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
struct pkcs7_signed_info *sinfo)
+#endif
{
struct public_key_signature *sig;
struct x509_certificate *x509 = sinfo->signer, *p;
@@ -314,6 +331,8 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
kenter("");
+ *signer = NULL;
+
for (p = pkcs7->certs; p; p = p->next)
p->seen = false;
@@ -331,6 +350,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
for (p = sinfo->signer; p != x509; p = p->signer)
p->blacklisted = true;
pr_debug("- blacklisted\n");
+#ifdef __UBOOT__
+ *signer = x509;
+#endif
return 0;
}
@@ -356,6 +378,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
goto unsupported_crypto_in_x509;
x509->signer = x509;
pr_debug("- self-signed\n");
+#ifdef __UBOOT__
+ *signer = x509;
+#endif
return 0;
}
@@ -386,6 +411,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
/* We didn't find the root of this chain */
pr_debug("- top\n");
+#ifdef __UBOOT__
+ *signer = x509;
+#endif
return 0;
found_issuer_check_skid:
@@ -403,6 +431,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
if (p->seen) {
pr_warn("Sig %u: X.509 chain contains loop\n",
sinfo->index);
+#ifdef __UBOOT__
+ *signer = p;
+#endif
return 0;
}
ret = public_key_verify_signature(p->pub, x509->sig);
@@ -411,6 +442,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
x509->signer = p;
if (x509 == p) {
pr_debug("- self-signed\n");
+#ifdef __UBOOT__
+ *signer = p;
+#endif
return 0;
}
x509 = p;
@@ -430,13 +464,26 @@ unsupported_crypto_in_x509:
}
/*
- * Verify one signed information block from a PKCS#7 message.
+ * pkcs7_verify_one - Verify one signed information block from a PKCS#7
+ * message.
+ * @pkcs7: PKCS7 Signed Data
+ * @sinfo: PKCS7 Signed Info
+ * @signer: Signer's certificate
+ *
+ * Verify one signature in @sinfo and follow the certificate chain.
+ * If the chain reaches the end, the last certificate will be returned
+ * in @signer.
+ *
+ * Return: 0 - on success, non-zero error code - otherwise
*/
-#ifndef __UBOOT__
-static
-#endif
+#ifdef __UBOOT__
int pkcs7_verify_one(struct pkcs7_message *pkcs7,
- struct pkcs7_signed_info *sinfo)
+ struct pkcs7_signed_info *sinfo,
+ struct x509_certificate **signer)
+#else
+static int pkcs7_verify_one(struct pkcs7_message *pkcs7,
+ struct pkcs7_signed_info *sinfo)
+#endif
{
int ret;
@@ -480,7 +527,7 @@ int pkcs7_verify_one(struct pkcs7_message *pkcs7,
pr_devel("Verified signature %u\n", sinfo->index);
/* Verify the internal certificate chain */
- return pkcs7_verify_sig_chain(pkcs7, sinfo);
+ return pkcs7_verify_sig_chain(pkcs7, sinfo, signer);
}
#ifndef __UBOOT__
--
2.27.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 6/8] lib: crypto: export and enhance pkcs7_verify_one()
2020-06-16 5:26 ` [PATCH v2 6/8] lib: crypto: export and enhance pkcs7_verify_one() AKASHI Takahiro
@ 2020-07-07 10:32 ` Heinrich Schuchardt
2020-07-08 6:37 ` AKASHI Takahiro
0 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2020-07-07 10:32 UTC (permalink / raw)
To: u-boot
On 16.06.20 07:26, AKASHI Takahiro wrote:
> The function, pkcs7_verify_one(), will be utilized to rework signature
> verification logic aiming to support intermediate certificates in
> "chain of trust."
Is this also copied from Linux's crypto/asymmetric_keys/pkcs7_verify.c?
If so, please, mention it in the commit message.
If everything copied from crypto/asymmetric_keys/pkcs7_verify.c were in
the same commit, the comparison would be easier.
Best regards
Heinrich
>
> To do that, its function interface is expanded, adding an extra argument
> which is expected to return the last certificate in trusted chain.
> Then, this last one must further be verified with signature database, db
> and/or dbx.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> include/crypto/pkcs7.h | 9 +++++-
> lib/crypto/pkcs7_verify.c | 61 ++++++++++++++++++++++++++++++++++-----
> 2 files changed, 62 insertions(+), 8 deletions(-)
>
> diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
> index 8f5c8a7ee3b9..ca35df29f6fb 100644
> --- a/include/crypto/pkcs7.h
> +++ b/include/crypto/pkcs7.h
> @@ -27,7 +27,14 @@ extern int pkcs7_get_content_data(const struct pkcs7_message *pkcs7,
> const void **_data, size_t *_datalen,
> size_t *_headerlen);
>
> -#ifndef __UBOOT__
> +#ifdef __UBOOT__
> +struct pkcs7_signed_info;
> +struct x509_certificate;
> +
> +int pkcs7_verify_one(struct pkcs7_message *pkcs7,
> + struct pkcs7_signed_info *sinfo,
> + struct x509_certificate **signer);
> +#else
> /*
> * pkcs7_trust.c
> */
> diff --git a/lib/crypto/pkcs7_verify.c b/lib/crypto/pkcs7_verify.c
> index 9b9030ea4440..dda96ccf57a2 100644
> --- a/lib/crypto/pkcs7_verify.c
> +++ b/lib/crypto/pkcs7_verify.c
> @@ -302,10 +302,27 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
> }
>
> /*
> - * Verify the internal certificate chain as best we can.
> + * pkcs7_verify_sig_chain - Verify the internal certificate chain as best
> + * as we can.
> + * @pkcs7: PKCS7 Signed Data
> + * @sinfo: PKCS7 Signed Info
> + * @signer: Singer's certificate
> + *
> + * Build up and verify the internal certificate chain against a signature
> + * in @sinfo, using certificates contained in @pkcs7 as best as we can.
> + * If the chain reaches the end, the last certificate will be returned
> + * in @signer.
> + *
> + * Return: 0 - on success, non-zero error code - otherwise
> */
> +#ifdef __UBOOT__
> +static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> + struct pkcs7_signed_info *sinfo,
> + struct x509_certificate **signer)
> +#else
> static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> struct pkcs7_signed_info *sinfo)
> +#endif
> {
> struct public_key_signature *sig;
> struct x509_certificate *x509 = sinfo->signer, *p;
> @@ -314,6 +331,8 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>
> kenter("");
>
> + *signer = NULL;
> +
> for (p = pkcs7->certs; p; p = p->next)
> p->seen = false;
>
> @@ -331,6 +350,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> for (p = sinfo->signer; p != x509; p = p->signer)
> p->blacklisted = true;
> pr_debug("- blacklisted\n");
> +#ifdef __UBOOT__
> + *signer = x509;
> +#endif
> return 0;
> }
>
> @@ -356,6 +378,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> goto unsupported_crypto_in_x509;
> x509->signer = x509;
> pr_debug("- self-signed\n");
> +#ifdef __UBOOT__
> + *signer = x509;
> +#endif
> return 0;
> }
>
> @@ -386,6 +411,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>
> /* We didn't find the root of this chain */
> pr_debug("- top\n");
> +#ifdef __UBOOT__
> + *signer = x509;
> +#endif
> return 0;
>
> found_issuer_check_skid:
> @@ -403,6 +431,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> if (p->seen) {
> pr_warn("Sig %u: X.509 chain contains loop\n",
> sinfo->index);
> +#ifdef __UBOOT__
> + *signer = p;
> +#endif
> return 0;
> }
> ret = public_key_verify_signature(p->pub, x509->sig);
> @@ -411,6 +442,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> x509->signer = p;
> if (x509 == p) {
> pr_debug("- self-signed\n");
> +#ifdef __UBOOT__
> + *signer = p;
> +#endif
> return 0;
> }
> x509 = p;
> @@ -430,13 +464,26 @@ unsupported_crypto_in_x509:
> }
>
> /*
> - * Verify one signed information block from a PKCS#7 message.
> + * pkcs7_verify_one - Verify one signed information block from a PKCS#7
> + * message.
> + * @pkcs7: PKCS7 Signed Data
> + * @sinfo: PKCS7 Signed Info
> + * @signer: Signer's certificate
> + *
> + * Verify one signature in @sinfo and follow the certificate chain.
> + * If the chain reaches the end, the last certificate will be returned
> + * in @signer.
> + *
> + * Return: 0 - on success, non-zero error code - otherwise
> */
> -#ifndef __UBOOT__
> -static
> -#endif
> +#ifdef __UBOOT__
> int pkcs7_verify_one(struct pkcs7_message *pkcs7,
> - struct pkcs7_signed_info *sinfo)
> + struct pkcs7_signed_info *sinfo,
> + struct x509_certificate **signer)
> +#else
> +static int pkcs7_verify_one(struct pkcs7_message *pkcs7,
> + struct pkcs7_signed_info *sinfo)
> +#endif
> {
> int ret;
>
> @@ -480,7 +527,7 @@ int pkcs7_verify_one(struct pkcs7_message *pkcs7,
> pr_devel("Verified signature %u\n", sinfo->index);
>
> /* Verify the internal certificate chain */
> - return pkcs7_verify_sig_chain(pkcs7, sinfo);
> + return pkcs7_verify_sig_chain(pkcs7, sinfo, signer);
> }
>
> #ifndef __UBOOT__
>
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 6/8] lib: crypto: export and enhance pkcs7_verify_one()
2020-07-07 10:32 ` Heinrich Schuchardt
@ 2020-07-08 6:37 ` AKASHI Takahiro
0 siblings, 0 replies; 25+ messages in thread
From: AKASHI Takahiro @ 2020-07-08 6:37 UTC (permalink / raw)
To: u-boot
Heinrich,
On Tue, Jul 07, 2020 at 12:32:02PM +0200, Heinrich Schuchardt wrote:
> On 16.06.20 07:26, AKASHI Takahiro wrote:
> > The function, pkcs7_verify_one(), will be utilized to rework signature
> > verification logic aiming to support intermediate certificates in
> > "chain of trust."
>
> Is this also copied from Linux's crypto/asymmetric_keys/pkcs7_verify.c?
> If so, please, mention it in the commit message.
Please read my comments carefully.
I clearly mentioned that this function was imported from linux
in the previous commit, patch 4/8.
Moreover, in this commit message, I explicitly mentioned
> > To do that, its function interface is expanded, adding an extra argument
> > which is expected to return the last certificate in trusted chain.
What more do you expect?
-Takahiro Akashi
> If everything copied from crypto/asymmetric_keys/pkcs7_verify.c were in
> the same commit, the comparison would be easier.
>
> Best regards
>
> Heinrich
>
>
> >
> > To do that, its function interface is expanded, adding an extra argument
> > which is expected to return the last certificate in trusted chain.
> > Then, this last one must further be verified with signature database, db
> > and/or dbx.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > include/crypto/pkcs7.h | 9 +++++-
> > lib/crypto/pkcs7_verify.c | 61 ++++++++++++++++++++++++++++++++++-----
> > 2 files changed, 62 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
> > index 8f5c8a7ee3b9..ca35df29f6fb 100644
> > --- a/include/crypto/pkcs7.h
> > +++ b/include/crypto/pkcs7.h
> > @@ -27,7 +27,14 @@ extern int pkcs7_get_content_data(const struct pkcs7_message *pkcs7,
> > const void **_data, size_t *_datalen,
> > size_t *_headerlen);
> >
> > -#ifndef __UBOOT__
> > +#ifdef __UBOOT__
> > +struct pkcs7_signed_info;
> > +struct x509_certificate;
> > +
> > +int pkcs7_verify_one(struct pkcs7_message *pkcs7,
> > + struct pkcs7_signed_info *sinfo,
> > + struct x509_certificate **signer);
> > +#else
> > /*
> > * pkcs7_trust.c
> > */
> > diff --git a/lib/crypto/pkcs7_verify.c b/lib/crypto/pkcs7_verify.c
> > index 9b9030ea4440..dda96ccf57a2 100644
> > --- a/lib/crypto/pkcs7_verify.c
> > +++ b/lib/crypto/pkcs7_verify.c
> > @@ -302,10 +302,27 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
> > }
> >
> > /*
> > - * Verify the internal certificate chain as best we can.
> > + * pkcs7_verify_sig_chain - Verify the internal certificate chain as best
> > + * as we can.
> > + * @pkcs7: PKCS7 Signed Data
> > + * @sinfo: PKCS7 Signed Info
> > + * @signer: Singer's certificate
> > + *
> > + * Build up and verify the internal certificate chain against a signature
> > + * in @sinfo, using certificates contained in @pkcs7 as best as we can.
> > + * If the chain reaches the end, the last certificate will be returned
> > + * in @signer.
> > + *
> > + * Return: 0 - on success, non-zero error code - otherwise
> > */
> > +#ifdef __UBOOT__
> > +static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> > + struct pkcs7_signed_info *sinfo,
> > + struct x509_certificate **signer)
> > +#else
> > static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> > struct pkcs7_signed_info *sinfo)
> > +#endif
> > {
> > struct public_key_signature *sig;
> > struct x509_certificate *x509 = sinfo->signer, *p;
> > @@ -314,6 +331,8 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> >
> > kenter("");
> >
> > + *signer = NULL;
> > +
> > for (p = pkcs7->certs; p; p = p->next)
> > p->seen = false;
> >
> > @@ -331,6 +350,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> > for (p = sinfo->signer; p != x509; p = p->signer)
> > p->blacklisted = true;
> > pr_debug("- blacklisted\n");
> > +#ifdef __UBOOT__
> > + *signer = x509;
> > +#endif
> > return 0;
> > }
> >
> > @@ -356,6 +378,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> > goto unsupported_crypto_in_x509;
> > x509->signer = x509;
> > pr_debug("- self-signed\n");
> > +#ifdef __UBOOT__
> > + *signer = x509;
> > +#endif
> > return 0;
> > }
> >
> > @@ -386,6 +411,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> >
> > /* We didn't find the root of this chain */
> > pr_debug("- top\n");
> > +#ifdef __UBOOT__
> > + *signer = x509;
> > +#endif
> > return 0;
> >
> > found_issuer_check_skid:
> > @@ -403,6 +431,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> > if (p->seen) {
> > pr_warn("Sig %u: X.509 chain contains loop\n",
> > sinfo->index);
> > +#ifdef __UBOOT__
> > + *signer = p;
> > +#endif
> > return 0;
> > }
> > ret = public_key_verify_signature(p->pub, x509->sig);
> > @@ -411,6 +442,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> > x509->signer = p;
> > if (x509 == p) {
> > pr_debug("- self-signed\n");
> > +#ifdef __UBOOT__
> > + *signer = p;
> > +#endif
> > return 0;
> > }
> > x509 = p;
> > @@ -430,13 +464,26 @@ unsupported_crypto_in_x509:
> > }
> >
> > /*
> > - * Verify one signed information block from a PKCS#7 message.
> > + * pkcs7_verify_one - Verify one signed information block from a PKCS#7
> > + * message.
> > + * @pkcs7: PKCS7 Signed Data
> > + * @sinfo: PKCS7 Signed Info
> > + * @signer: Signer's certificate
> > + *
> > + * Verify one signature in @sinfo and follow the certificate chain.
> > + * If the chain reaches the end, the last certificate will be returned
> > + * in @signer.
> > + *
> > + * Return: 0 - on success, non-zero error code - otherwise
> > */
> > -#ifndef __UBOOT__
> > -static
> > -#endif
> > +#ifdef __UBOOT__
> > int pkcs7_verify_one(struct pkcs7_message *pkcs7,
> > - struct pkcs7_signed_info *sinfo)
> > + struct pkcs7_signed_info *sinfo,
> > + struct x509_certificate **signer)
> > +#else
> > +static int pkcs7_verify_one(struct pkcs7_message *pkcs7,
> > + struct pkcs7_signed_info *sinfo)
> > +#endif
> > {
> > int ret;
> >
> > @@ -480,7 +527,7 @@ int pkcs7_verify_one(struct pkcs7_message *pkcs7,
> > pr_devel("Verified signature %u\n", sinfo->index);
> >
> > /* Verify the internal certificate chain */
> > - return pkcs7_verify_sig_chain(pkcs7, sinfo);
> > + return pkcs7_verify_sig_chain(pkcs7, sinfo, signer);
> > }
> >
> > #ifndef __UBOOT__
> >
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 7/8] efi_loader: signature: rework for intermediate certificates support
2020-06-16 5:26 [PATCH v2 0/8] efi_loader: secure boot: support intermediate certificates in signature AKASHI Takahiro
` (5 preceding siblings ...)
2020-06-16 5:26 ` [PATCH v2 6/8] lib: crypto: export and enhance pkcs7_verify_one() AKASHI Takahiro
@ 2020-06-16 5:26 ` AKASHI Takahiro
2020-07-07 10:33 ` Heinrich Schuchardt
2020-06-16 5:26 ` [PATCH v2 8/8] test/py: efi_secboot: add test for intermediate certificates AKASHI Takahiro
7 siblings, 1 reply; 25+ messages in thread
From: AKASHI Takahiro @ 2020-06-16 5:26 UTC (permalink / raw)
To: u-boot
In this commit, efi_signature_verify(with_sigdb) will be re-implemented
using pcks7_verify_one() in order to support certificates chain, where
the signer's certificate will be signed by an intermediate CA (certificate
authority) and the latter's certificate will also be signed by another CA
and so on.
What we need to do here is to search for certificates in a signature,
build up a chain of certificates and verify one by one. pkcs7_verify_one()
handles most of these steps except the last one.
pkcs7_verify_one() returns, if succeeded, the last certificate to verify,
which can be either a self-signed one or one that should be signed by one
of certificates in "db". Re-worked efi_signature_verify() will take care
of this step.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
include/efi_loader.h | 8 +-
lib/efi_loader/Kconfig | 1 +
lib/efi_loader/efi_image_loader.c | 2 +-
lib/efi_loader/efi_signature.c | 385 ++++++++++++++----------------
lib/efi_loader/efi_variable.c | 5 +-
5 files changed, 188 insertions(+), 213 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index a95f9b339027..086528c305ef 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -767,10 +767,10 @@ bool efi_signature_lookup_digest(struct efi_image_regions *regs,
bool efi_signature_verify_one(struct efi_image_regions *regs,
struct pkcs7_message *msg,
struct efi_signature_store *db);
-bool efi_signature_verify_with_sigdb(struct efi_image_regions *regs,
- struct pkcs7_message *msg,
- struct efi_signature_store *db,
- struct efi_signature_store *dbx);
+bool efi_signature_verify(struct efi_image_regions *regs,
+ struct pkcs7_message *msg,
+ struct efi_signature_store *db,
+ struct efi_signature_store *dbx);
bool efi_signature_check_signers(struct pkcs7_message *msg,
struct efi_signature_store *dbx);
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index aad37b715505..5fc178180721 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -157,6 +157,7 @@ config EFI_SECURE_BOOT
select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
select X509_CERTIFICATE_PARSER
select PKCS7_MESSAGE_PARSER
+ select PKCS7_VERIFY
default n
help
Select this option to enable EFI secure boot support.
diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
index 584a841116df..029bb035e5ce 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -642,7 +642,7 @@ static bool efi_image_authenticate(void *efi, size_t efi_size)
}
/* try white-list */
- if (efi_signature_verify_with_sigdb(regs, msg, db, dbx))
+ if (efi_signature_verify(regs, msg, db, dbx))
continue;
debug("Signature was not verified by \"db\"\n");
diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
index a87cbe520f56..55d584b20ac0 100644
--- a/lib/efi_loader/efi_signature.c
+++ b/lib/efi_loader/efi_signature.c
@@ -10,7 +10,9 @@
#include <image.h>
#include <hexdump.h>
#include <malloc.h>
+#include <crypto/pkcs7.h>
#include <crypto/pkcs7_parser.h>
+#include <crypto/public_key.h>
#include <crypto/x509_parser.h>
#include <linux/compat.h>
#include <linux/oid_registry.h>
@@ -61,143 +63,6 @@ static bool efi_hash_regions(struct image_region *regs, int count,
return true;
}
-/**
- * efi_hash_msg_content - calculate a hash value of contentInfo
- * @msg: Signature
- * @hash: Pointer to a pointer to buffer holding a hash value
- * @size: Size of buffer to be returned
- *
- * Calculate a sha256 value of contentInfo in @msg and return a value in @hash.
- *
- * Return: true on success, false on error
- */
-static bool efi_hash_msg_content(struct pkcs7_message *msg, void **hash,
- size_t *size)
-{
- struct image_region regtmp;
-
- regtmp.data = msg->data;
- regtmp.size = msg->data_len;
-
- return efi_hash_regions(®tmp, 1, hash, size);
-}
-
-/**
- * efi_signature_verify - verify a signature with a certificate
- * @regs: List of regions to be authenticated
- * @signed_info: Pointer to PKCS7's signed_info
- * @cert: x509 certificate
- *
- * Signature pointed to by @signed_info against image pointed to by @regs
- * is verified by a certificate pointed to by @cert.
- * @signed_info holds a signature, including a message digest which is to be
- * compared with a hash value calculated from @regs.
- *
- * Return: true if signature is verified, false if not
- */
-static bool efi_signature_verify(struct efi_image_regions *regs,
- struct pkcs7_message *msg,
- struct pkcs7_signed_info *ps_info,
- struct x509_certificate *cert)
-{
- struct image_sign_info info;
- struct image_region regtmp[2];
- void *hash;
- size_t size;
- char c;
- bool verified;
-
- EFI_PRINT("%s: Enter, %p, %p, %p(issuer: %s, subject: %s)\n", __func__,
- regs, ps_info, cert, cert->issuer, cert->subject);
-
- verified = false;
-
- memset(&info, '\0', sizeof(info));
- info.padding = image_get_padding_algo("pkcs-1.5");
- /*
- * Note: image_get_[checksum|crypto]_algo takes an string
- * argument like "<checksum>,<crypto>"
- * TODO: support other hash algorithms
- */
- if (!strcmp(ps_info->sig->hash_algo, "sha1")) {
- info.checksum = image_get_checksum_algo("sha1,rsa2048");
- info.name = "sha1,rsa2048";
- } else if (!strcmp(ps_info->sig->hash_algo, "sha256")) {
- info.checksum = image_get_checksum_algo("sha256,rsa2048");
- info.name = "sha256,rsa2048";
- } else {
- EFI_PRINT("unknown msg digest algo: %s\n",
- ps_info->sig->hash_algo);
- goto out;
- }
- info.crypto = image_get_crypto_algo(info.name);
-
- info.key = cert->pub->key;
- info.keylen = cert->pub->keylen;
-
- /* verify signature */
- EFI_PRINT("%s: crypto: %s, signature len:%x\n", __func__,
- info.name, ps_info->sig->s_size);
- if (ps_info->aa_set & (1UL << sinfo_has_message_digest)) {
- EFI_PRINT("%s: RSA verify authentication attribute\n",
- __func__);
- /*
- * NOTE: This path will be executed only for
- * PE image authentication
- */
-
- /* check if hash matches digest first */
- EFI_PRINT("checking msg digest first, len:0x%x\n",
- ps_info->msgdigest_len);
-
-#ifdef DEBUG
- EFI_PRINT("hash in database:\n");
- print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1,
- ps_info->msgdigest, ps_info->msgdigest_len,
- false);
-#endif
- /* against contentInfo first */
- hash = NULL;
- if ((msg->data && efi_hash_msg_content(msg, &hash, &size)) ||
- /* for signed image */
- efi_hash_regions(regs->reg, regs->num, &hash, &size)) {
- /* for authenticated variable */
- if (ps_info->msgdigest_len != size ||
- memcmp(hash, ps_info->msgdigest, size)) {
- EFI_PRINT("Digest doesn't match\n");
- free(hash);
- goto out;
- }
-
- free(hash);
- } else {
- EFI_PRINT("Digesting image failed\n");
- goto out;
- }
-
- /* against digest */
- c = 0x31;
- regtmp[0].data = &c;
- regtmp[0].size = 1;
- regtmp[1].data = ps_info->authattrs;
- regtmp[1].size = ps_info->authattrs_len;
-
- if (!rsa_verify(&info, regtmp, 2,
- ps_info->sig->s, ps_info->sig->s_size))
- verified = true;
- } else {
- EFI_PRINT("%s: RSA verify content data\n", __func__);
- /* against all data */
- if (!rsa_verify(&info, regs->reg, regs->num,
- ps_info->sig->s, ps_info->sig->s_size))
- verified = true;
- }
-
-out:
- EFI_PRINT("%s: Exit, verified: %d\n", __func__, verified);
- return verified;
-}
-
/**
* efi_signature_lookup_digest - search for an image's digest in sigdb
* @regs: List of regions to be authenticated
@@ -261,61 +126,127 @@ out:
}
/**
- * efi_signature_verify_with_list - verify a signature with signature list
- * @regs: List of regions to be authenticated
- * @msg: Signature
- * @signed_info: Pointer to PKCS7's signed_info
- * @siglist: Signature list for certificates
- * @valid_cert: x509 certificate that verifies this signature
+ * efi_lookup_certificate - find a certificate within db
+ * @msg: Signature
+ * @db: Signature database
*
- * Signature pointed to by @signed_info against image pointed to by @regs
- * is verified by signature list pointed to by @siglist.
- * Signature database is a simple concatenation of one or more
- * signature list(s).
+ * Search signature database pointed to by @db and find a certificate
+ * pointed to by @cert.
*
- * Return: true if signature is verified, false if not
+ * Return: true if found, false otherwise.
*/
-static
-bool efi_signature_verify_with_list(struct efi_image_regions *regs,
- struct pkcs7_message *msg,
- struct pkcs7_signed_info *signed_info,
- struct efi_signature_store *siglist,
- struct x509_certificate **valid_cert)
+static bool efi_lookup_certificate(struct x509_certificate *cert,
+ struct efi_signature_store *db)
{
- struct x509_certificate *cert;
+ struct efi_signature_store *siglist;
struct efi_sig_data *sig_data;
- bool verified = false;
+ struct image_region reg[1];
+ void *hash = NULL, *hash_tmp = NULL;
+ size_t size = 0;
+ bool found = false;
- EFI_PRINT("%s: Enter, %p, %p, %p, %p\n", __func__,
- regs, signed_info, siglist, valid_cert);
+ EFI_PRINT("%s: Enter, %p, %p\n", __func__, cert, db);
- if (guidcmp(&siglist->sig_type, &efi_guid_cert_x509)) {
- EFI_PRINT("Signature type is not supported: %pUl\n",
- &siglist->sig_type);
+ if (!cert || !db || !db->sig_data_list)
goto out;
- }
- /* go through the list */
- for (sig_data = siglist->sig_data_list; sig_data;
- sig_data = sig_data->next) {
- /* TODO: support owner check based on policy */
+ /*
+ * TODO: identify a certificate using sha256 digest
+ * Is there any better way?
+ */
+ /* calculate hash of TBSCertificate */
+ reg[0].data = cert->tbs;
+ reg[0].size = cert->tbs_size;
+ if (!efi_hash_regions(reg, 1, &hash, &size))
+ goto out;
- cert = x509_cert_parse(sig_data->data, sig_data->size);
- if (IS_ERR(cert)) {
- EFI_PRINT("Parsing x509 certificate failed\n");
- goto out;
+ EFI_PRINT("%s: searching for %s\n", __func__, cert->subject);
+ for (siglist = db; siglist; siglist = siglist->next) {
+ /* only with x509 certificate */
+ if (guidcmp(&siglist->sig_type, &efi_guid_cert_x509))
+ continue;
+
+ for (sig_data = siglist->sig_data_list; sig_data;
+ sig_data = sig_data->next) {
+ struct x509_certificate *cert_tmp;
+
+ cert_tmp = x509_cert_parse(sig_data->data,
+ sig_data->size);
+ if (!cert)
+ continue;
+
+ reg[0].data = cert_tmp->tbs;
+ reg[0].size = cert_tmp->tbs_size;
+ if (!efi_hash_regions(reg, 1, &hash_tmp, NULL))
+ goto out;
+
+ x509_free_certificate(cert_tmp);
+
+ if (!memcmp(hash, hash_tmp, size)) {
+ found = true;
+ goto out;
+ }
}
+ }
+out:
+ free(hash);
+ free(hash_tmp);
- verified = efi_signature_verify(regs, msg, signed_info, cert);
+ EFI_PRINT("%s: Exit, found: %d\n", __func__, found);
+ return found;
+}
- if (verified) {
- if (valid_cert)
- *valid_cert = cert;
- else
- x509_free_certificate(cert);
- break;
+/**
+ * efi_verify_certificate - verify certificate's signature with database
+ * @signer: Certificate
+ * @db: Signature database
+ * @root: Certificate to verify @signer
+ *
+ * Determine if certificate pointed to by @signer may be verified
+ * by one of certificates in signature database pointed to by @db.
+ *
+ * Return: true if certificate is verified, false otherwise.
+ */
+static bool efi_verify_certificate(struct x509_certificate *signer,
+ struct efi_signature_store *db,
+ struct x509_certificate **root)
+{
+ struct efi_signature_store *siglist;
+ struct efi_sig_data *sig_data;
+ struct x509_certificate *cert;
+ bool verified = false;
+ int ret;
+
+ EFI_PRINT("%s: Enter, %p, %p\n", __func__, signer, db);
+
+ if (!signer || !db || !db->sig_data_list)
+ goto out;
+
+ for (siglist = db; siglist; siglist = siglist->next) {
+ /* only with x509 certificate */
+ if (guidcmp(&siglist->sig_type, &efi_guid_cert_x509))
+ continue;
+
+ for (sig_data = siglist->sig_data_list; sig_data;
+ sig_data = sig_data->next) {
+ cert = x509_cert_parse(sig_data->data, sig_data->size);
+ if (!cert) {
+ EFI_PRINT("Cannot parse x509 certificate\n");
+ continue;
+ }
+
+ ret = public_key_verify_signature(cert->pub,
+ signer->sig);
+ if (!ret) {
+ verified = true;
+ if (root)
+ *root = cert;
+ else
+ x509_free_certificate(cert);
+ goto out;
+ }
+ x509_free_certificate(cert);
}
- x509_free_certificate(cert);
}
out:
@@ -423,9 +354,9 @@ bool efi_signature_verify_one(struct efi_image_regions *regs,
struct efi_signature_store *db)
{
struct pkcs7_signed_info *sinfo;
- struct efi_signature_store *siglist;
- struct x509_certificate *cert;
+ struct x509_certificate *signer;
bool verified = false;
+ int ret;
EFI_PRINT("%s: Enter, %p, %p, %p\n", __func__, regs, msg, db);
@@ -440,13 +371,29 @@ bool efi_signature_verify_one(struct efi_image_regions *regs,
EFI_PRINT("Signed Info: digest algo: %s, pkey algo: %s\n",
sinfo->sig->hash_algo, sinfo->sig->pkey_algo);
- for (siglist = db; siglist; siglist = siglist->next)
- if (efi_signature_verify_with_list(regs, msg, sinfo,
- siglist, &cert)) {
+ EFI_PRINT("Verifying certificate chain\n");
+ signer = NULL;
+ ret = pkcs7_verify_one(msg, sinfo, &signer);
+ if (ret == -ENOPKG)
+ continue;
+
+ if (ret < 0 || !signer)
+ goto out;
+
+ if (sinfo->blacklisted)
+ continue;
+
+ EFI_PRINT("Verifying last certificate in chain\n");
+ if (signer->self_signed) {
+ if (efi_lookup_certificate(signer, db)) {
verified = true;
goto out;
}
- EFI_PRINT("Valid certificate not in \"db\"\n");
+ } else if (efi_verify_certificate(signer, db, NULL)) {
+ verified = true;
+ goto out;
+ }
+ EFI_PRINT("Valid certificate not in db\n");
}
out:
@@ -454,8 +401,8 @@ out:
return verified;
}
-/**
- * efi_signature_verify_with_sigdb - verify signatures with db and dbx
+/*
+ * efi_signature_verify - verify signatures with db and dbx
* @regs: List of regions to be authenticated
* @msg: Signature
* @db: Signature database for trusted certificates
@@ -466,43 +413,71 @@ out:
*
* Return: true if verification for all signatures passed, false otherwise
*/
-bool efi_signature_verify_with_sigdb(struct efi_image_regions *regs,
- struct pkcs7_message *msg,
- struct efi_signature_store *db,
- struct efi_signature_store *dbx)
+bool efi_signature_verify(struct efi_image_regions *regs,
+ struct pkcs7_message *msg,
+ struct efi_signature_store *db,
+ struct efi_signature_store *dbx)
{
- struct pkcs7_signed_info *info;
- struct efi_signature_store *siglist;
- struct x509_certificate *cert;
+ struct pkcs7_signed_info *sinfo;
+ struct x509_certificate *signer, *root;
bool verified = false;
+ int ret;
EFI_PRINT("%s: Enter, %p, %p, %p, %p\n", __func__, regs, msg, db, dbx);
if (!regs || !msg || !db || !db->sig_data_list)
goto out;
- for (info = msg->signed_infos; info; info = info->next) {
+ for (sinfo = msg->signed_infos; sinfo; sinfo = sinfo->next) {
EFI_PRINT("Signed Info: digest algo: %s, pkey algo: %s\n",
- info->sig->hash_algo, info->sig->pkey_algo);
+ sinfo->sig->hash_algo, sinfo->sig->pkey_algo);
- for (siglist = db; siglist; siglist = siglist->next) {
- if (efi_signature_verify_with_list(regs, msg, info,
- siglist, &cert))
- break;
- }
- if (!siglist) {
- EFI_PRINT("Valid certificate not in \"db\"\n");
+ /*
+ * only for authenticated variable.
+ *
+ * If this function is called for image,
+ * hash calculation will be done in
+ * pkcs7_verify_one().
+ */
+ if (!msg->data &&
+ !efi_hash_regions(regs->reg, regs->num,
+ (void **)&sinfo->sig->digest, NULL)) {
+ EFI_PRINT("Digesting an image failed\n");
goto out;
}
- if (!dbx || efi_signature_check_revocation(info, cert, dbx))
+ EFI_PRINT("Verifying certificate chain\n");
+ signer = NULL;
+ ret = pkcs7_verify_one(msg, sinfo, &signer);
+ if (ret == -ENOPKG)
continue;
- EFI_PRINT("Certificate in \"dbx\"\n");
+ if (ret < 0 || !signer)
+ goto out;
+
+ if (sinfo->blacklisted)
+ goto out;
+
+ EFI_PRINT("Verifying last certificate in chain\n");
+ if (signer->self_signed) {
+ if (efi_lookup_certificate(signer, db))
+ if (efi_signature_check_revocation(sinfo,
+ signer, dbx))
+ continue;
+ } else if (efi_verify_certificate(signer, db, &root)) {
+ bool check;
+
+ check = efi_signature_check_revocation(sinfo, root,
+ dbx);
+ x509_free_certificate(root);
+ if (check)
+ continue;
+ }
+
+ EFI_PRINT("Certificate chain didn't reach trusted CA\n");
goto out;
}
verified = true;
-
out:
EFI_PRINT("%s: Exit, verified: %d\n", __func__, verified);
return verified;
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 90d740fbda2c..a2f3020c1847 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -558,12 +558,11 @@ static efi_status_t efi_variable_authenticate(u16 *variable,
}
/* verify signature */
- if (efi_signature_verify_with_sigdb(regs, var_sig, truststore, NULL)) {
+ if (efi_signature_verify(regs, var_sig, truststore, NULL)) {
EFI_PRINT("Verified\n");
} else {
if (truststore2 &&
- efi_signature_verify_with_sigdb(regs, var_sig,
- truststore2, NULL)) {
+ efi_signature_verify(regs, var_sig, truststore2, NULL)) {
EFI_PRINT("Verified\n");
} else {
EFI_PRINT("Verifying variable's signature failed\n");
--
2.27.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 7/8] efi_loader: signature: rework for intermediate certificates support
2020-06-16 5:26 ` [PATCH v2 7/8] efi_loader: signature: rework for intermediate certificates support AKASHI Takahiro
@ 2020-07-07 10:33 ` Heinrich Schuchardt
0 siblings, 0 replies; 25+ messages in thread
From: Heinrich Schuchardt @ 2020-07-07 10:33 UTC (permalink / raw)
To: u-boot
On 16.06.20 07:26, AKASHI Takahiro wrote:
> In this commit, efi_signature_verify(with_sigdb) will be re-implemented
> using pcks7_verify_one() in order to support certificates chain, where
> the signer's certificate will be signed by an intermediate CA (certificate
> authority) and the latter's certificate will also be signed by another CA
> and so on.
>
> What we need to do here is to search for certificates in a signature,
> build up a chain of certificates and verify one by one. pkcs7_verify_one()
> handles most of these steps except the last one.
>
> pkcs7_verify_one() returns, if succeeded, the last certificate to verify,
> which can be either a self-signed one or one that should be signed by one
> of certificates in "db". Re-worked efi_signature_verify() will take care
> of this step.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Sounds reasonable.
Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 8/8] test/py: efi_secboot: add test for intermediate certificates
2020-06-16 5:26 [PATCH v2 0/8] efi_loader: secure boot: support intermediate certificates in signature AKASHI Takahiro
` (6 preceding siblings ...)
2020-06-16 5:26 ` [PATCH v2 7/8] efi_loader: signature: rework for intermediate certificates support AKASHI Takahiro
@ 2020-06-16 5:26 ` AKASHI Takahiro
2020-07-07 10:42 ` Heinrich Schuchardt
7 siblings, 1 reply; 25+ messages in thread
From: AKASHI Takahiro @ 2020-06-16 5:26 UTC (permalink / raw)
To: u-boot
In this test case, an image may have a signature with additional
intermediate certificates. A chain of trust will be followed and all
the certificates in the middle of chain must be verified before loading.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
test/py/tests/test_efi_secboot/conftest.py | 138 +++++++++++++++++-
test/py/tests/test_efi_secboot/defs.py | 11 +-
test/py/tests/test_efi_secboot/openssl.cnf | 48 ++++++
.../test_efi_secboot/test_signed_intca.py | 134 +++++++++++++++++
4 files changed, 328 insertions(+), 3 deletions(-)
create mode 100644 test/py/tests/test_efi_secboot/openssl.cnf
create mode 100644 test/py/tests/test_efi_secboot/test_signed_intca.py
diff --git a/test/py/tests/test_efi_secboot/conftest.py b/test/py/tests/test_efi_secboot/conftest.py
index 34abcd79ae00..e5ac2a2a21b7 100644
--- a/test/py/tests/test_efi_secboot/conftest.py
+++ b/test/py/tests/test_efi_secboot/conftest.py
@@ -37,7 +37,7 @@ def efi_boot_env(request, u_boot_config):
global HELLO_PATH
image_path = u_boot_config.persistent_data_dir
- image_path = image_path + '/' + EFI_SECBOOT_IMAGE_NAME
+ image_path = image_path + '/' + EFI_SECBOOT_IMAGE_NAME + '.img'
image_size = EFI_SECBOOT_IMAGE_SIZE
part_size = EFI_SECBOOT_PART_SIZE
fs_type = EFI_SECBOOT_FS_TYPE
@@ -46,7 +46,7 @@ def efi_boot_env(request, u_boot_config):
HELLO_PATH = u_boot_config.build_dir + '/lib/efi_loader/helloworld.efi'
try:
- mnt_point = u_boot_config.persistent_data_dir + '/mnt_efisecure'
+ mnt_point = u_boot_config.persistent_data_dir + MNTPNT
check_call('mkdir -p {}'.format(mnt_point), shell=True)
# create a disk/partition
@@ -170,3 +170,137 @@ def efi_boot_env(request, u_boot_config):
yield image_path
finally:
call('rm -f %s' % image_path, shell=True)
+
+#
+# Fixture for UEFI secure boot test of intermediate certificates
+#
+ at pytest.fixture(scope='session')
+def efi_boot_env_intca(request, u_boot_config):
+ """Set up a file system to be used in UEFI secure boot test
+ of intermediate certificates.
+
+ Args:
+ request: Pytest request object.
+ u_boot_config: U-boot configuration.
+
+ Return:
+ A path to disk image to be used for testing
+ """
+ global HELLO_PATH
+
+ image_path = u_boot_config.persistent_data_dir
+ image_path = image_path + '/' + EFI_SECBOOT_IMAGE_NAME + '_intca.img'
+ image_size = EFI_SECBOOT_IMAGE_SIZE
+ part_size = EFI_SECBOOT_PART_SIZE
+ fs_type = EFI_SECBOOT_FS_TYPE
+
+ if HELLO_PATH == '':
+ HELLO_PATH = u_boot_config.build_dir + '/lib/efi_loader/helloworld.efi'
+
+ try:
+ mnt_point = u_boot_config.persistent_data_dir + MNTPNT
+ check_call('mkdir -p {}'.format(mnt_point), shell=True)
+
+ # create a disk/partition
+ check_call('dd if=/dev/zero of=%s bs=1MiB count=%d'
+ % (image_path, image_size), shell=True)
+ check_call('sgdisk %s -n 1:0:+%dMiB'
+ % (image_path, part_size), shell=True)
+ # create a file system
+ check_call('dd if=/dev/zero of=%s.tmp bs=1MiB count=%d'
+ % (image_path, part_size), shell=True)
+ check_call('mkfs -t %s %s.tmp' % (fs_type, image_path), shell=True)
+ check_call('dd if=%s.tmp of=%s bs=1MiB seek=1 count=%d conv=notrunc'
+ % (image_path, image_path, 1), shell=True)
+ check_call('rm %s.tmp' % image_path, shell=True)
+ loop_dev = check_output('sudo losetup -o 1MiB --sizelimit %dMiB --show -f %s | tr -d "\n"'
+ % (part_size, image_path), shell=True).decode()
+ check_output('sudo mount -t %s -o umask=000 %s %s'
+ % (fs_type, loop_dev, mnt_point), shell=True)
+
+ # Create signature database
+ ## PK
+ check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_PK/ -keyout PK.key -out PK.crt -nodes -days 365'
+ % mnt_point, shell=True)
+ check_call('cd %s; %scert-to-efi-sig-list -g %s PK.crt PK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key PK PK.esl PK.auth'
+ % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
+ shell=True)
+ ## KEK
+ check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_KEK/ -keyout KEK.key -out KEK.crt -nodes -days 365'
+ % mnt_point, shell=True)
+ check_call('cd %s; %scert-to-efi-sig-list -g %s KEK.crt KEK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key KEK KEK.esl KEK.auth'
+ % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
+ shell=True)
+
+ # We will have three-tier hierarchy of certificates:
+ # TestRoot: Root CA (self-signed)
+ # TestSub: Intermediate CA (signed by Root CA)
+ # TestCert: User certificate (signed by Intermediate CA, and used
+ # for signing an image)
+ #
+ # NOTE:
+ # I consulted the following EDK2 document for certificate options:
+ # BaseTools/Source/Python/Pkcs7Sign/Readme.md
+ # Please not use them as they are in product system. They are
+ # for test purpose only.
+
+ # TestRoot
+ check_call('cp %s/test/py/tests/test_efi_secboot/openssl.cnf %s' % (u_boot_config.source_dir, mnt_point), shell=True)
+ check_call('cd %s; openssl genrsa -out TestRoot.key 2048; openssl req --config openssl.cnf -extensions v3_ca -new -x509 -days 365 -key TestRoot.key -out TestRoot.crt -subj "/CN=TEST_root/"; touch index.txt' % mnt_point, shell=True)
+ # TestSub
+ check_call('cd %s; openssl genrsa -out TestSub.key 2048; openssl req -new -key TestSub.key -out TestSub.csr -subj "/CN=TEST_sub/"; openssl ca --config openssl.cnf -in TestSub.csr -out TestSub.crt -extensions v3_int_ca -days 365 -batch -rand_serial -cert TestRoot.crt -keyfile TestRoot.key' % mnt_point, shell=True)
+ # TestCert
+ check_call('cd %s; openssl genrsa -out TestCert.key 2048; openssl req -new -key TestCert.key -out TestCert.csr -subj "/CN=TEST_cert/"; openssl ca --config openssl.cnf -in TestCert.csr -out TestCert.crt -extensions usr_cert -days 365 -batch -rand_serial -cert TestSub.crt -keyfile TestSub.key' % mnt_point, shell=True)
+ ## db
+ # for TestCert
+ check_call('cd %s; %scert-to-efi-sig-list -g %s TestCert.crt TestCert.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db TestCert.esl db_a.auth'
+ % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
+ shell=True)
+ # for TestSub
+ check_call('cd %s; %scert-to-efi-sig-list -g %s TestSub.crt TestSub.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db TestSub.esl db_b.auth'
+ % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
+ shell=True)
+ # for TestRoot
+ check_call('cd %s; %scert-to-efi-sig-list -g %s TestRoot.crt TestRoot.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db TestRoot.esl db_c.auth'
+ % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
+ shell=True)
+ ## dbx (hash of certificate with revocation time)
+ # for TestCert
+ check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 TestCert.crt TestCert.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx TestCert.crl dbx_a.auth'
+ % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
+ shell=True)
+ # for TestSub
+ check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 TestSub.crt TestSub.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx TestSub.crl dbx_b.auth'
+ % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
+ shell=True)
+ # for TestRoot
+ check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 TestRoot.crt TestRoot.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx TestRoot.crl dbx_c.auth'
+ % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
+ shell=True)
+
+ # Sign image
+ # additional intermediate certificates may be included
+ # in SignedData
+
+ check_call('cp %s %s' % (HELLO_PATH, mnt_point), shell=True)
+ ## signed by TestCert
+ check_call('cd %s; %ssbsign --key TestCert.key --cert TestCert.crt --out helloworld.efi.signed_a helloworld.efi'
+ % (mnt_point, SBSIGN_PATH), shell=True)
+ ## signed by TestCert with TestSub in signature
+ check_call('cd %s; %ssbsign --key TestCert.key --cert TestCert.crt --addcert TestSub.crt --out helloworld.efi.signed_ab helloworld.efi'
+ % (mnt_point, SBSIGN_PATH), shell=True)
+ ## signed by TestCert with TestSub and TestRoot in signature
+ check_call('cd %s; cat TestSub.crt TestRoot.crt > TestSubRoot.crt; %ssbsign --key TestCert.key --cert TestCert.crt --addcert TestSubRoot.crt --out helloworld.efi.signed_abc helloworld.efi'
+ % (mnt_point, SBSIGN_PATH), shell=True)
+
+ # Clean-up
+ check_call('sudo umount %s' % loop_dev, shell=True)
+ check_call('sudo losetup -d %s' % loop_dev, shell=True)
+
+ except CalledProcessError as e:
+ pytest.skip('Setup failed: %s' % e.cmd)
+ return
+ else:
+ yield image_path
+ finally:
+ call('rm -f %s' % image_path, shell=True)
diff --git a/test/py/tests/test_efi_secboot/defs.py b/test/py/tests/test_efi_secboot/defs.py
index 099f453979ff..c61f69a316f8 100644
--- a/test/py/tests/test_efi_secboot/defs.py
+++ b/test/py/tests/test_efi_secboot/defs.py
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0+
# Disk image name
-EFI_SECBOOT_IMAGE_NAME = 'test_efi_secboot.img'
+EFI_SECBOOT_IMAGE_NAME = 'test_efi_secboot'
# Size in MiB
EFI_SECBOOT_IMAGE_SIZE = 16
@@ -10,12 +10,21 @@ EFI_SECBOOT_PART_SIZE = 8
# Partition file system type
EFI_SECBOOT_FS_TYPE = 'vfat'
+# Mount point
+MNTPNT= 'mnt'
+
# Owner guid
GUID = '11111111-2222-3333-4444-123456789abc'
# v1.5.1 or earlier of efitools has a bug in sha256 calculation, and
# you need build a newer version on your own.
+# The path must terminate with '/'.
EFITOOLS_PATH = ''
+# "--addcert" option of sbsign must be available, otherwise
+# you need build a newer version on your own.
+# The path must terminate with '/'.
+SBSIGN_PATH= '/home/akashi/arm/misc/sbsigntools/src/'
+
# Hello World application for sandbox
HELLO_PATH = ''
diff --git a/test/py/tests/test_efi_secboot/openssl.cnf b/test/py/tests/test_efi_secboot/openssl.cnf
new file mode 100644
index 000000000000..f684f1df7e69
--- /dev/null
+++ b/test/py/tests/test_efi_secboot/openssl.cnf
@@ -0,0 +1,48 @@
+[ ca ]
+default_ca = CA_default
+
+[ CA_default ]
+new_certs_dir = .
+database = ./index.txt
+serial = ./serial
+default_md = sha256
+policy = policy_min
+
+[ req ]
+distinguished_name = def_distinguished_name
+
+[def_distinguished_name]
+
+# Extensions
+# -addext " ... = ..."
+#
+[ v3_ca ]
+ # Extensions for a typical Root CA.
+ basicConstraints = critical,CA:TRUE
+ keyUsage = critical, digitalSignature, cRLSign, keyCertSign
+ subjectKeyIdentifier = hash
+ authorityKeyIdentifier = keyid:always,issuer
+
+[ v3_int_ca ]
+ # Extensions for a typical intermediate CA.
+ basicConstraints = critical, CA:TRUE
+ keyUsage = critical, digitalSignature, cRLSign, keyCertSign
+ subjectKeyIdentifier = hash
+ authorityKeyIdentifier = keyid:always,issuer
+
+[ usr_cert ]
+ # Extensions for user end certificates.
+ basicConstraints = CA:FALSE
+ keyUsage = critical, nonRepudiation, digitalSignature, keyEncipherment
+ extendedKeyUsage = clientAuth, emailProtection
+ subjectKeyIdentifier = hash
+ authorityKeyIdentifier = keyid,issuer
+
+[ policy_min ]
+ countryName = optional
+ stateOrProvinceName = optional
+ localityName = optional
+ organizationName = optional
+ organizationalUnitName = optional
+ commonName = supplied
+ emailAddress = optional
diff --git a/test/py/tests/test_efi_secboot/test_signed_intca.py b/test/py/tests/test_efi_secboot/test_signed_intca.py
new file mode 100644
index 000000000000..80c1917a2cd3
--- /dev/null
+++ b/test/py/tests/test_efi_secboot/test_signed_intca.py
@@ -0,0 +1,134 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2020, Linaro Limited
+# Author: AKASHI Takahiro <takahiro.akashi@linaro.org>
+#
+# U-Boot UEFI: Image Authentication Test (signature with certificates chain)
+
+"""
+This test verifies image authentication for a signed image which is signed
+by user certificate and contains additional intermediate certificates in its
+signature.
+"""
+
+import pytest
+
+ at pytest.mark.boardspec('sandbox')
+ at pytest.mark.buildconfigspec('efi_secure_boot')
+ at pytest.mark.buildconfigspec('cmd_efidebug')
+ at pytest.mark.buildconfigspec('cmd_fat')
+ at pytest.mark.buildconfigspec('cmd_nvedit_efi')
+ at pytest.mark.slow
+class TestEfiSignedImageExt(object):
+ def test_efi_signed_image_ext1(self, u_boot_console, efi_boot_env_intca):
+ """
+ Test Case 1 - authenticated by root CA in db
+ """
+ u_boot_console.restart_uboot()
+ disk_img = efi_boot_env_intca
+ with u_boot_console.log.section('Test Case 1a'):
+ # Test Case 1a, with no Int CA and not authenticated by root CA
+ output = u_boot_console.run_command_list([
+ 'host bind 0 %s' % disk_img,
+ 'fatload host 0:1 4000000 db_c.auth',
+ 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
+ 'fatload host 0:1 4000000 KEK.auth',
+ 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
+ 'fatload host 0:1 4000000 PK.auth',
+ 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
+ assert 'Failed to set EFI variable' not in ''.join(output)
+
+ output = u_boot_console.run_command_list([
+ 'efidebug boot add 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""',
+ 'efidebug boot next 1',
+ 'efidebug test bootmgr'])
+ assert '\'HELLO_a\' failed' in ''.join(output)
+ assert 'efi_start_image() returned: 26' in ''.join(output)
+
+ with u_boot_console.log.section('Test Case 1b'):
+ # Test Case 1b, signed and authenticated by root CA
+ output = u_boot_console.run_command_list([
+ 'efidebug boot add 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""',
+ 'efidebug boot next 2',
+ 'bootefi bootmgr'])
+ assert 'Hello, world!' in ''.join(output)
+
+ def test_efi_signed_image_ext2(self, u_boot_console, efi_boot_env_intca):
+ """
+ Test Case 2 - authenticated by root CA in db
+ """
+ u_boot_console.restart_uboot()
+ disk_img = efi_boot_env_intca
+ with u_boot_console.log.section('Test Case 2a'):
+ # Test Case 2a, unsigned and not authenticated by root CA
+ output = u_boot_console.run_command_list([
+ 'host bind 0 %s' % disk_img,
+ 'fatload host 0:1 4000000 KEK.auth',
+ 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
+ 'fatload host 0:1 4000000 PK.auth',
+ 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
+ assert 'Failed to set EFI variable' not in ''.join(output)
+
+ output = u_boot_console.run_command_list([
+ 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
+ 'efidebug boot next 1',
+ 'efidebug test bootmgr'])
+ assert '\'HELLO_abc\' failed' in ''.join(output)
+ assert 'efi_start_image() returned: 26' in ''.join(output)
+
+ with u_boot_console.log.section('Test Case 2b'):
+ # Test Case 2b, signed and authenticated by root CA
+ output = u_boot_console.run_command_list([
+ 'fatload host 0:1 4000000 db_b.auth',
+ 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
+ 'efidebug boot next 1',
+ 'efidebug test bootmgr'])
+ assert '\'HELLO_abc\' failed' in ''.join(output)
+ assert 'efi_start_image() returned: 26' in ''.join(output)
+
+ with u_boot_console.log.section('Test Case 2c'):
+ # Test Case 2c, signed and authenticated by root CA
+ output = u_boot_console.run_command_list([
+ 'fatload host 0:1 4000000 db_c.auth',
+ 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
+ 'efidebug boot next 1',
+ 'efidebug test bootmgr'])
+ assert 'Hello, world!' in ''.join(output)
+
+ def test_efi_signed_image_ext3(self, u_boot_console, efi_boot_env_intca):
+ """
+ Test Case 3 - revoked by dbx
+ """
+ u_boot_console.restart_uboot()
+ disk_img = efi_boot_env_intca
+ with u_boot_console.log.section('Test Case 3a'):
+ # Test Case 3a, revoked by root CA in dbx
+ output = u_boot_console.run_command_list([
+ 'host bind 0 %s' % disk_img,
+ 'fatload host 0:1 4000000 dbx_c.auth',
+ 'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx',
+ 'fatload host 0:1 4000000 db_c.auth',
+ 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
+ 'fatload host 0:1 4000000 KEK.auth',
+ 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
+ 'fatload host 0:1 4000000 PK.auth',
+ 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
+ assert 'Failed to set EFI variable' not in ''.join(output)
+
+ output = u_boot_console.run_command_list([
+ 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
+ 'efidebug boot next 1',
+ 'efidebug test bootmgr'])
+ assert '\'HELLO_abc\' failed' in ''.join(output)
+ assert 'efi_start_image() returned: 26' in ''.join(output)
+
+ with u_boot_console.log.section('Test Case 3b'):
+ # Test Case 3b, revoked by int CA in dbx
+ output = u_boot_console.run_command_list([
+ 'fatload host 0:1 4000000 dbx_b.auth',
+ 'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx',
+ 'efidebug boot next 1',
+ 'efidebug test bootmgr'])
+ assert 'Hello, world!' in ''.join(output)
+ # Or,
+ # assert('\'HELLO_abc\' failed' in ''.join(output))
+ # assert('efi_start_image() returned: 26' in ''.join(output))
--
2.27.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 8/8] test/py: efi_secboot: add test for intermediate certificates
2020-06-16 5:26 ` [PATCH v2 8/8] test/py: efi_secboot: add test for intermediate certificates AKASHI Takahiro
@ 2020-07-07 10:42 ` Heinrich Schuchardt
2020-07-08 6:39 ` AKASHI Takahiro
2020-07-09 0:58 ` using sudo?, " AKASHI Takahiro
0 siblings, 2 replies; 25+ messages in thread
From: Heinrich Schuchardt @ 2020-07-07 10:42 UTC (permalink / raw)
To: u-boot
On 16.06.20 07:26, AKASHI Takahiro wrote:
> In this test case, an image may have a signature with additional
> intermediate certificates. A chain of trust will be followed and all
> the certificates in the middle of chain must be verified before loading.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> test/py/tests/test_efi_secboot/conftest.py | 138 +++++++++++++++++-
> test/py/tests/test_efi_secboot/defs.py | 11 +-
> test/py/tests/test_efi_secboot/openssl.cnf | 48 ++++++
> .../test_efi_secboot/test_signed_intca.py | 134 +++++++++++++++++
> 4 files changed, 328 insertions(+), 3 deletions(-)
> create mode 100644 test/py/tests/test_efi_secboot/openssl.cnf
> create mode 100644 test/py/tests/test_efi_secboot/test_signed_intca.py
>
> diff --git a/test/py/tests/test_efi_secboot/conftest.py b/test/py/tests/test_efi_secboot/conftest.py
> index 34abcd79ae00..e5ac2a2a21b7 100644
> --- a/test/py/tests/test_efi_secboot/conftest.py
> +++ b/test/py/tests/test_efi_secboot/conftest.py
> @@ -37,7 +37,7 @@ def efi_boot_env(request, u_boot_config):
> global HELLO_PATH
>
> image_path = u_boot_config.persistent_data_dir
> - image_path = image_path + '/' + EFI_SECBOOT_IMAGE_NAME
> + image_path = image_path + '/' + EFI_SECBOOT_IMAGE_NAME + '.img'
> image_size = EFI_SECBOOT_IMAGE_SIZE
> part_size = EFI_SECBOOT_PART_SIZE
> fs_type = EFI_SECBOOT_FS_TYPE
> @@ -46,7 +46,7 @@ def efi_boot_env(request, u_boot_config):
> HELLO_PATH = u_boot_config.build_dir + '/lib/efi_loader/helloworld.efi'
>
> try:
> - mnt_point = u_boot_config.persistent_data_dir + '/mnt_efisecure'
> + mnt_point = u_boot_config.persistent_data_dir + MNTPNT
> check_call('mkdir -p {}'.format(mnt_point), shell=True)
>
> # create a disk/partition
> @@ -170,3 +170,137 @@ def efi_boot_env(request, u_boot_config):
> yield image_path
> finally:
> call('rm -f %s' % image_path, shell=True)
> +
> +#
> +# Fixture for UEFI secure boot test of intermediate certificates
Thanks for adding a test.
> +#
> + at pytest.fixture(scope='session')
> +def efi_boot_env_intca(request, u_boot_config):
> + """Set up a file system to be used in UEFI secure boot test
> + of intermediate certificates.
> +
> + Args:
> + request: Pytest request object.
> + u_boot_config: U-boot configuration.
> +
> + Return:
> + A path to disk image to be used for testing
> + """
> + global HELLO_PATH
> +
> + image_path = u_boot_config.persistent_data_dir
> + image_path = image_path + '/' + EFI_SECBOOT_IMAGE_NAME + '_intca.img'
> + image_size = EFI_SECBOOT_IMAGE_SIZE
> + part_size = EFI_SECBOOT_PART_SIZE
> + fs_type = EFI_SECBOOT_FS_TYPE
> +
> + if HELLO_PATH == '':
> + HELLO_PATH = u_boot_config.build_dir + '/lib/efi_loader/helloworld.efi'
> +
> + try:
> + mnt_point = u_boot_config.persistent_data_dir + MNTPNT
> + check_call('mkdir -p {}'.format(mnt_point), shell=True)
> +
> + # create a disk/partition
> + check_call('dd if=/dev/zero of=%s bs=1MiB count=%d'
> + % (image_path, image_size), shell=True)
> + check_call('sgdisk %s -n 1:0:+%dMiB'
> + % (image_path, part_size), shell=True)
> + # create a file system
> + check_call('dd if=/dev/zero of=%s.tmp bs=1MiB count=%d'
> + % (image_path, part_size), shell=True)
> + check_call('mkfs -t %s %s.tmp' % (fs_type, image_path), shell=True)
> + check_call('dd if=%s.tmp of=%s bs=1MiB seek=1 count=%d conv=notrunc'
> + % (image_path, image_path, 1), shell=True)
> + check_call('rm %s.tmp' % image_path, shell=True)
> + loop_dev = check_output('sudo losetup -o 1MiB --sizelimit %dMiB --show -f %s | tr -d "\n"'
> + % (part_size, image_path), shell=True).decode()
> + check_output('sudo mount -t %s -o umask=000 %s %s'
> + % (fs_type, loop_dev, mnt_point), shell=True)
Can we use virt-make-fs to avoid sudo, please. Package libguestfs-tools
has been added to the Docker image for Gitlab recently.
> +
> + # Create signature database
> + ## PK
> + check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_PK/ -keyout PK.key -out PK.crt -nodes -days 365'
> + % mnt_point, shell=True)
> + check_call('cd %s; %scert-to-efi-sig-list -g %s PK.crt PK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key PK PK.esl PK.auth'
> + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> + shell=True)
> + ## KEK
> + check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_KEK/ -keyout KEK.key -out KEK.crt -nodes -days 365'
> + % mnt_point, shell=True)
> + check_call('cd %s; %scert-to-efi-sig-list -g %s KEK.crt KEK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key KEK KEK.esl KEK.auth'
> + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> + shell=True)
> +
> + # We will have three-tier hierarchy of certificates:
> + # TestRoot: Root CA (self-signed)
> + # TestSub: Intermediate CA (signed by Root CA)
> + # TestCert: User certificate (signed by Intermediate CA, and used
> + # for signing an image)
> + #
> + # NOTE:
> + # I consulted the following EDK2 document for certificate options:
> + # BaseTools/Source/Python/Pkcs7Sign/Readme.md
> + # Please not use them as they are in product system. They are
> + # for test purpose only.
> +
> + # TestRoot
> + check_call('cp %s/test/py/tests/test_efi_secboot/openssl.cnf %s' % (u_boot_config.source_dir, mnt_point), shell=True)
> + check_call('cd %s; openssl genrsa -out TestRoot.key 2048; openssl req --config openssl.cnf -extensions v3_ca -new -x509 -days 365 -key TestRoot.key -out TestRoot.crt -subj "/CN=TEST_root/"; touch index.txt' % mnt_point, shell=True)
Please, use the .format() function of the string class.
Best regards
Heinrich
> + # TestSub
> + check_call('cd %s; openssl genrsa -out TestSub.key 2048; openssl req -new -key TestSub.key -out TestSub.csr -subj "/CN=TEST_sub/"; openssl ca --config openssl.cnf -in TestSub.csr -out TestSub.crt -extensions v3_int_ca -days 365 -batch -rand_serial -cert TestRoot.crt -keyfile TestRoot.key' % mnt_point, shell=True)
> + # TestCert
> + check_call('cd %s; openssl genrsa -out TestCert.key 2048; openssl req -new -key TestCert.key -out TestCert.csr -subj "/CN=TEST_cert/"; openssl ca --config openssl.cnf -in TestCert.csr -out TestCert.crt -extensions usr_cert -days 365 -batch -rand_serial -cert TestSub.crt -keyfile TestSub.key' % mnt_point, shell=True)
> + ## db
> + # for TestCert
> + check_call('cd %s; %scert-to-efi-sig-list -g %s TestCert.crt TestCert.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db TestCert.esl db_a.auth'
> + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> + shell=True)
> + # for TestSub
> + check_call('cd %s; %scert-to-efi-sig-list -g %s TestSub.crt TestSub.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db TestSub.esl db_b.auth'
> + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> + shell=True)
> + # for TestRoot
> + check_call('cd %s; %scert-to-efi-sig-list -g %s TestRoot.crt TestRoot.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db TestRoot.esl db_c.auth'
> + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> + shell=True)
> + ## dbx (hash of certificate with revocation time)
> + # for TestCert
> + check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 TestCert.crt TestCert.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx TestCert.crl dbx_a.auth'
> + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> + shell=True)
> + # for TestSub
> + check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 TestSub.crt TestSub.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx TestSub.crl dbx_b.auth'
> + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> + shell=True)
> + # for TestRoot
> + check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 TestRoot.crt TestRoot.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx TestRoot.crl dbx_c.auth'
> + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> + shell=True)
> +
> + # Sign image
> + # additional intermediate certificates may be included
> + # in SignedData
> +
> + check_call('cp %s %s' % (HELLO_PATH, mnt_point), shell=True)
> + ## signed by TestCert
> + check_call('cd %s; %ssbsign --key TestCert.key --cert TestCert.crt --out helloworld.efi.signed_a helloworld.efi'
> + % (mnt_point, SBSIGN_PATH), shell=True)
> + ## signed by TestCert with TestSub in signature
> + check_call('cd %s; %ssbsign --key TestCert.key --cert TestCert.crt --addcert TestSub.crt --out helloworld.efi.signed_ab helloworld.efi'
> + % (mnt_point, SBSIGN_PATH), shell=True)
> + ## signed by TestCert with TestSub and TestRoot in signature
> + check_call('cd %s; cat TestSub.crt TestRoot.crt > TestSubRoot.crt; %ssbsign --key TestCert.key --cert TestCert.crt --addcert TestSubRoot.crt --out helloworld.efi.signed_abc helloworld.efi'
> + % (mnt_point, SBSIGN_PATH), shell=True)
> +
> + # Clean-up
> + check_call('sudo umount %s' % loop_dev, shell=True)
> + check_call('sudo losetup -d %s' % loop_dev, shell=True)
> +
> + except CalledProcessError as e:
> + pytest.skip('Setup failed: %s' % e.cmd)
> + return
> + else:
> + yield image_path
> + finally:
> + call('rm -f %s' % image_path, shell=True)
> diff --git a/test/py/tests/test_efi_secboot/defs.py b/test/py/tests/test_efi_secboot/defs.py
> index 099f453979ff..c61f69a316f8 100644
> --- a/test/py/tests/test_efi_secboot/defs.py
> +++ b/test/py/tests/test_efi_secboot/defs.py
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0+
>
> # Disk image name
> -EFI_SECBOOT_IMAGE_NAME = 'test_efi_secboot.img'
> +EFI_SECBOOT_IMAGE_NAME = 'test_efi_secboot'
>
> # Size in MiB
> EFI_SECBOOT_IMAGE_SIZE = 16
> @@ -10,12 +10,21 @@ EFI_SECBOOT_PART_SIZE = 8
> # Partition file system type
> EFI_SECBOOT_FS_TYPE = 'vfat'
>
> +# Mount point
> +MNTPNT= 'mnt'
> +
> # Owner guid
> GUID = '11111111-2222-3333-4444-123456789abc'
>
> # v1.5.1 or earlier of efitools has a bug in sha256 calculation, and
> # you need build a newer version on your own.
> +# The path must terminate with '/'.
> EFITOOLS_PATH = ''
>
> +# "--addcert" option of sbsign must be available, otherwise
> +# you need build a newer version on your own.
> +# The path must terminate with '/'.
> +SBSIGN_PATH= '/home/akashi/arm/misc/sbsigntools/src/'
> +
> # Hello World application for sandbox
> HELLO_PATH = ''
> diff --git a/test/py/tests/test_efi_secboot/openssl.cnf b/test/py/tests/test_efi_secboot/openssl.cnf
> new file mode 100644
> index 000000000000..f684f1df7e69
> --- /dev/null
> +++ b/test/py/tests/test_efi_secboot/openssl.cnf
> @@ -0,0 +1,48 @@
> +[ ca ]
> +default_ca = CA_default
> +
> +[ CA_default ]
> +new_certs_dir = .
> +database = ./index.txt
> +serial = ./serial
> +default_md = sha256
> +policy = policy_min
> +
> +[ req ]
> +distinguished_name = def_distinguished_name
> +
> +[def_distinguished_name]
> +
> +# Extensions
> +# -addext " ... = ..."
> +#
> +[ v3_ca ]
> + # Extensions for a typical Root CA.
> + basicConstraints = critical,CA:TRUE
> + keyUsage = critical, digitalSignature, cRLSign, keyCertSign
> + subjectKeyIdentifier = hash
> + authorityKeyIdentifier = keyid:always,issuer
> +
> +[ v3_int_ca ]
> + # Extensions for a typical intermediate CA.
> + basicConstraints = critical, CA:TRUE
> + keyUsage = critical, digitalSignature, cRLSign, keyCertSign
> + subjectKeyIdentifier = hash
> + authorityKeyIdentifier = keyid:always,issuer
> +
> +[ usr_cert ]
> + # Extensions for user end certificates.
> + basicConstraints = CA:FALSE
> + keyUsage = critical, nonRepudiation, digitalSignature, keyEncipherment
> + extendedKeyUsage = clientAuth, emailProtection
> + subjectKeyIdentifier = hash
> + authorityKeyIdentifier = keyid,issuer
> +
> +[ policy_min ]
> + countryName = optional
> + stateOrProvinceName = optional
> + localityName = optional
> + organizationName = optional
> + organizationalUnitName = optional
> + commonName = supplied
> + emailAddress = optional
> diff --git a/test/py/tests/test_efi_secboot/test_signed_intca.py b/test/py/tests/test_efi_secboot/test_signed_intca.py
> new file mode 100644
> index 000000000000..80c1917a2cd3
> --- /dev/null
> +++ b/test/py/tests/test_efi_secboot/test_signed_intca.py
> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (c) 2020, Linaro Limited
> +# Author: AKASHI Takahiro <takahiro.akashi@linaro.org>
> +#
> +# U-Boot UEFI: Image Authentication Test (signature with certificates chain)
> +
> +"""
> +This test verifies image authentication for a signed image which is signed
> +by user certificate and contains additional intermediate certificates in its
> +signature.
> +"""
> +
> +import pytest
> +
> + at pytest.mark.boardspec('sandbox')
> + at pytest.mark.buildconfigspec('efi_secure_boot')
> + at pytest.mark.buildconfigspec('cmd_efidebug')
> + at pytest.mark.buildconfigspec('cmd_fat')
> + at pytest.mark.buildconfigspec('cmd_nvedit_efi')
> + at pytest.mark.slow
> +class TestEfiSignedImageExt(object):
> + def test_efi_signed_image_ext1(self, u_boot_console, efi_boot_env_intca):
> + """
> + Test Case 1 - authenticated by root CA in db
> + """
> + u_boot_console.restart_uboot()
> + disk_img = efi_boot_env_intca
> + with u_boot_console.log.section('Test Case 1a'):
> + # Test Case 1a, with no Int CA and not authenticated by root CA
> + output = u_boot_console.run_command_list([
> + 'host bind 0 %s' % disk_img,
> + 'fatload host 0:1 4000000 db_c.auth',
> + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
> + 'fatload host 0:1 4000000 KEK.auth',
> + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> + 'fatload host 0:1 4000000 PK.auth',
> + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
> + assert 'Failed to set EFI variable' not in ''.join(output)
> +
> + output = u_boot_console.run_command_list([
> + 'efidebug boot add 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""',
> + 'efidebug boot next 1',
> + 'efidebug test bootmgr'])
> + assert '\'HELLO_a\' failed' in ''.join(output)
> + assert 'efi_start_image() returned: 26' in ''.join(output)
> +
> + with u_boot_console.log.section('Test Case 1b'):
> + # Test Case 1b, signed and authenticated by root CA
> + output = u_boot_console.run_command_list([
> + 'efidebug boot add 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""',
> + 'efidebug boot next 2',
> + 'bootefi bootmgr'])
> + assert 'Hello, world!' in ''.join(output)
> +
> + def test_efi_signed_image_ext2(self, u_boot_console, efi_boot_env_intca):
> + """
> + Test Case 2 - authenticated by root CA in db
> + """
> + u_boot_console.restart_uboot()
> + disk_img = efi_boot_env_intca
> + with u_boot_console.log.section('Test Case 2a'):
> + # Test Case 2a, unsigned and not authenticated by root CA
> + output = u_boot_console.run_command_list([
> + 'host bind 0 %s' % disk_img,
> + 'fatload host 0:1 4000000 KEK.auth',
> + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> + 'fatload host 0:1 4000000 PK.auth',
> + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
> + assert 'Failed to set EFI variable' not in ''.join(output)
> +
> + output = u_boot_console.run_command_list([
> + 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
> + 'efidebug boot next 1',
> + 'efidebug test bootmgr'])
> + assert '\'HELLO_abc\' failed' in ''.join(output)
> + assert 'efi_start_image() returned: 26' in ''.join(output)
> +
> + with u_boot_console.log.section('Test Case 2b'):
> + # Test Case 2b, signed and authenticated by root CA
> + output = u_boot_console.run_command_list([
> + 'fatload host 0:1 4000000 db_b.auth',
> + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
> + 'efidebug boot next 1',
> + 'efidebug test bootmgr'])
> + assert '\'HELLO_abc\' failed' in ''.join(output)
> + assert 'efi_start_image() returned: 26' in ''.join(output)
> +
> + with u_boot_console.log.section('Test Case 2c'):
> + # Test Case 2c, signed and authenticated by root CA
> + output = u_boot_console.run_command_list([
> + 'fatload host 0:1 4000000 db_c.auth',
> + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
> + 'efidebug boot next 1',
> + 'efidebug test bootmgr'])
> + assert 'Hello, world!' in ''.join(output)
> +
> + def test_efi_signed_image_ext3(self, u_boot_console, efi_boot_env_intca):
> + """
> + Test Case 3 - revoked by dbx
> + """
> + u_boot_console.restart_uboot()
> + disk_img = efi_boot_env_intca
> + with u_boot_console.log.section('Test Case 3a'):
> + # Test Case 3a, revoked by root CA in dbx
> + output = u_boot_console.run_command_list([
> + 'host bind 0 %s' % disk_img,
> + 'fatload host 0:1 4000000 dbx_c.auth',
> + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx',
> + 'fatload host 0:1 4000000 db_c.auth',
> + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
> + 'fatload host 0:1 4000000 KEK.auth',
> + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> + 'fatload host 0:1 4000000 PK.auth',
> + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
> + assert 'Failed to set EFI variable' not in ''.join(output)
> +
> + output = u_boot_console.run_command_list([
> + 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
> + 'efidebug boot next 1',
> + 'efidebug test bootmgr'])
> + assert '\'HELLO_abc\' failed' in ''.join(output)
> + assert 'efi_start_image() returned: 26' in ''.join(output)
> +
> + with u_boot_console.log.section('Test Case 3b'):
> + # Test Case 3b, revoked by int CA in dbx
> + output = u_boot_console.run_command_list([
> + 'fatload host 0:1 4000000 dbx_b.auth',
> + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx',
> + 'efidebug boot next 1',
> + 'efidebug test bootmgr'])
> + assert 'Hello, world!' in ''.join(output)
> + # Or,
> + # assert('\'HELLO_abc\' failed' in ''.join(output))
> + # assert('efi_start_image() returned: 26' in ''.join(output))
>
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 8/8] test/py: efi_secboot: add test for intermediate certificates
2020-07-07 10:42 ` Heinrich Schuchardt
@ 2020-07-08 6:39 ` AKASHI Takahiro
2020-07-09 0:58 ` using sudo?, " AKASHI Takahiro
1 sibling, 0 replies; 25+ messages in thread
From: AKASHI Takahiro @ 2020-07-08 6:39 UTC (permalink / raw)
To: u-boot
On Tue, Jul 07, 2020 at 12:42:35PM +0200, Heinrich Schuchardt wrote:
> On 16.06.20 07:26, AKASHI Takahiro wrote:
> > In this test case, an image may have a signature with additional
> > intermediate certificates. A chain of trust will be followed and all
> > the certificates in the middle of chain must be verified before loading.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > test/py/tests/test_efi_secboot/conftest.py | 138 +++++++++++++++++-
> > test/py/tests/test_efi_secboot/defs.py | 11 +-
> > test/py/tests/test_efi_secboot/openssl.cnf | 48 ++++++
> > .../test_efi_secboot/test_signed_intca.py | 134 +++++++++++++++++
> > 4 files changed, 328 insertions(+), 3 deletions(-)
> > create mode 100644 test/py/tests/test_efi_secboot/openssl.cnf
> > create mode 100644 test/py/tests/test_efi_secboot/test_signed_intca.py
> >
> > diff --git a/test/py/tests/test_efi_secboot/conftest.py b/test/py/tests/test_efi_secboot/conftest.py
> > index 34abcd79ae00..e5ac2a2a21b7 100644
> > --- a/test/py/tests/test_efi_secboot/conftest.py
> > +++ b/test/py/tests/test_efi_secboot/conftest.py
> > @@ -37,7 +37,7 @@ def efi_boot_env(request, u_boot_config):
> > global HELLO_PATH
> >
> > image_path = u_boot_config.persistent_data_dir
> > - image_path = image_path + '/' + EFI_SECBOOT_IMAGE_NAME
> > + image_path = image_path + '/' + EFI_SECBOOT_IMAGE_NAME + '.img'
> > image_size = EFI_SECBOOT_IMAGE_SIZE
> > part_size = EFI_SECBOOT_PART_SIZE
> > fs_type = EFI_SECBOOT_FS_TYPE
> > @@ -46,7 +46,7 @@ def efi_boot_env(request, u_boot_config):
> > HELLO_PATH = u_boot_config.build_dir + '/lib/efi_loader/helloworld.efi'
> >
> > try:
> > - mnt_point = u_boot_config.persistent_data_dir + '/mnt_efisecure'
> > + mnt_point = u_boot_config.persistent_data_dir + MNTPNT
> > check_call('mkdir -p {}'.format(mnt_point), shell=True)
> >
> > # create a disk/partition
> > @@ -170,3 +170,137 @@ def efi_boot_env(request, u_boot_config):
> > yield image_path
> > finally:
> > call('rm -f %s' % image_path, shell=True)
> > +
> > +#
> > +# Fixture for UEFI secure boot test of intermediate certificates
>
> Thanks for adding a test.
>
>
> > +#
> > + at pytest.fixture(scope='session')
> > +def efi_boot_env_intca(request, u_boot_config):
> > + """Set up a file system to be used in UEFI secure boot test
> > + of intermediate certificates.
> > +
> > + Args:
> > + request: Pytest request object.
> > + u_boot_config: U-boot configuration.
> > +
> > + Return:
> > + A path to disk image to be used for testing
> > + """
> > + global HELLO_PATH
> > +
> > + image_path = u_boot_config.persistent_data_dir
> > + image_path = image_path + '/' + EFI_SECBOOT_IMAGE_NAME + '_intca.img'
> > + image_size = EFI_SECBOOT_IMAGE_SIZE
> > + part_size = EFI_SECBOOT_PART_SIZE
> > + fs_type = EFI_SECBOOT_FS_TYPE
> > +
> > + if HELLO_PATH == '':
> > + HELLO_PATH = u_boot_config.build_dir + '/lib/efi_loader/helloworld.efi'
> > +
> > + try:
> > + mnt_point = u_boot_config.persistent_data_dir + MNTPNT
> > + check_call('mkdir -p {}'.format(mnt_point), shell=True)
> > +
> > + # create a disk/partition
> > + check_call('dd if=/dev/zero of=%s bs=1MiB count=%d'
> > + % (image_path, image_size), shell=True)
> > + check_call('sgdisk %s -n 1:0:+%dMiB'
> > + % (image_path, part_size), shell=True)
> > + # create a file system
> > + check_call('dd if=/dev/zero of=%s.tmp bs=1MiB count=%d'
> > + % (image_path, part_size), shell=True)
> > + check_call('mkfs -t %s %s.tmp' % (fs_type, image_path), shell=True)
> > + check_call('dd if=%s.tmp of=%s bs=1MiB seek=1 count=%d conv=notrunc'
> > + % (image_path, image_path, 1), shell=True)
> > + check_call('rm %s.tmp' % image_path, shell=True)
> > + loop_dev = check_output('sudo losetup -o 1MiB --sizelimit %dMiB --show -f %s | tr -d "\n"'
> > + % (part_size, image_path), shell=True).decode()
> > + check_output('sudo mount -t %s -o umask=000 %s %s'
> > + % (fs_type, loop_dev, mnt_point), shell=True)
>
> Can we use virt-make-fs to avoid sudo, please. Package libguestfs-tools
> has been added to the Docker image for Gitlab recently.
I will check.
> > +
> > + # Create signature database
> > + ## PK
> > + check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_PK/ -keyout PK.key -out PK.crt -nodes -days 365'
> > + % mnt_point, shell=True)
> > + check_call('cd %s; %scert-to-efi-sig-list -g %s PK.crt PK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key PK PK.esl PK.auth'
> > + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > + shell=True)
> > + ## KEK
> > + check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_KEK/ -keyout KEK.key -out KEK.crt -nodes -days 365'
> > + % mnt_point, shell=True)
> > + check_call('cd %s; %scert-to-efi-sig-list -g %s KEK.crt KEK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key KEK KEK.esl KEK.auth'
> > + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > + shell=True)
> > +
> > + # We will have three-tier hierarchy of certificates:
> > + # TestRoot: Root CA (self-signed)
> > + # TestSub: Intermediate CA (signed by Root CA)
> > + # TestCert: User certificate (signed by Intermediate CA, and used
> > + # for signing an image)
> > + #
> > + # NOTE:
> > + # I consulted the following EDK2 document for certificate options:
> > + # BaseTools/Source/Python/Pkcs7Sign/Readme.md
> > + # Please not use them as they are in product system. They are
> > + # for test purpose only.
> > +
> > + # TestRoot
> > + check_call('cp %s/test/py/tests/test_efi_secboot/openssl.cnf %s' % (u_boot_config.source_dir, mnt_point), shell=True)
> > + check_call('cd %s; openssl genrsa -out TestRoot.key 2048; openssl req --config openssl.cnf -extensions v3_ca -new -x509 -days 365 -key TestRoot.key -out TestRoot.crt -subj "/CN=TEST_root/"; touch index.txt' % mnt_point, shell=True)
>
> Please, use the .format() function of the string class.
See my previous comment.
-Takahiro Akashi
>
> Best regards
>
> Heinrich
>
> > + # TestSub
> > + check_call('cd %s; openssl genrsa -out TestSub.key 2048; openssl req -new -key TestSub.key -out TestSub.csr -subj "/CN=TEST_sub/"; openssl ca --config openssl.cnf -in TestSub.csr -out TestSub.crt -extensions v3_int_ca -days 365 -batch -rand_serial -cert TestRoot.crt -keyfile TestRoot.key' % mnt_point, shell=True)
> > + # TestCert
> > + check_call('cd %s; openssl genrsa -out TestCert.key 2048; openssl req -new -key TestCert.key -out TestCert.csr -subj "/CN=TEST_cert/"; openssl ca --config openssl.cnf -in TestCert.csr -out TestCert.crt -extensions usr_cert -days 365 -batch -rand_serial -cert TestSub.crt -keyfile TestSub.key' % mnt_point, shell=True)
> > + ## db
> > + # for TestCert
> > + check_call('cd %s; %scert-to-efi-sig-list -g %s TestCert.crt TestCert.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db TestCert.esl db_a.auth'
> > + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > + shell=True)
> > + # for TestSub
> > + check_call('cd %s; %scert-to-efi-sig-list -g %s TestSub.crt TestSub.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db TestSub.esl db_b.auth'
> > + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > + shell=True)
> > + # for TestRoot
> > + check_call('cd %s; %scert-to-efi-sig-list -g %s TestRoot.crt TestRoot.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db TestRoot.esl db_c.auth'
> > + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > + shell=True)
> > + ## dbx (hash of certificate with revocation time)
> > + # for TestCert
> > + check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 TestCert.crt TestCert.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx TestCert.crl dbx_a.auth'
> > + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > + shell=True)
> > + # for TestSub
> > + check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 TestSub.crt TestSub.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx TestSub.crl dbx_b.auth'
> > + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > + shell=True)
> > + # for TestRoot
> > + check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 TestRoot.crt TestRoot.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx TestRoot.crl dbx_c.auth'
> > + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > + shell=True)
> > +
> > + # Sign image
> > + # additional intermediate certificates may be included
> > + # in SignedData
> > +
> > + check_call('cp %s %s' % (HELLO_PATH, mnt_point), shell=True)
> > + ## signed by TestCert
> > + check_call('cd %s; %ssbsign --key TestCert.key --cert TestCert.crt --out helloworld.efi.signed_a helloworld.efi'
> > + % (mnt_point, SBSIGN_PATH), shell=True)
> > + ## signed by TestCert with TestSub in signature
> > + check_call('cd %s; %ssbsign --key TestCert.key --cert TestCert.crt --addcert TestSub.crt --out helloworld.efi.signed_ab helloworld.efi'
> > + % (mnt_point, SBSIGN_PATH), shell=True)
> > + ## signed by TestCert with TestSub and TestRoot in signature
> > + check_call('cd %s; cat TestSub.crt TestRoot.crt > TestSubRoot.crt; %ssbsign --key TestCert.key --cert TestCert.crt --addcert TestSubRoot.crt --out helloworld.efi.signed_abc helloworld.efi'
> > + % (mnt_point, SBSIGN_PATH), shell=True)
> > +
> > + # Clean-up
> > + check_call('sudo umount %s' % loop_dev, shell=True)
> > + check_call('sudo losetup -d %s' % loop_dev, shell=True)
> > +
> > + except CalledProcessError as e:
> > + pytest.skip('Setup failed: %s' % e.cmd)
> > + return
> > + else:
> > + yield image_path
> > + finally:
> > + call('rm -f %s' % image_path, shell=True)
> > diff --git a/test/py/tests/test_efi_secboot/defs.py b/test/py/tests/test_efi_secboot/defs.py
> > index 099f453979ff..c61f69a316f8 100644
> > --- a/test/py/tests/test_efi_secboot/defs.py
> > +++ b/test/py/tests/test_efi_secboot/defs.py
> > @@ -1,7 +1,7 @@
> > # SPDX-License-Identifier: GPL-2.0+
> >
> > # Disk image name
> > -EFI_SECBOOT_IMAGE_NAME = 'test_efi_secboot.img'
> > +EFI_SECBOOT_IMAGE_NAME = 'test_efi_secboot'
> >
> > # Size in MiB
> > EFI_SECBOOT_IMAGE_SIZE = 16
> > @@ -10,12 +10,21 @@ EFI_SECBOOT_PART_SIZE = 8
> > # Partition file system type
> > EFI_SECBOOT_FS_TYPE = 'vfat'
> >
> > +# Mount point
> > +MNTPNT= 'mnt'
> > +
> > # Owner guid
> > GUID = '11111111-2222-3333-4444-123456789abc'
> >
> > # v1.5.1 or earlier of efitools has a bug in sha256 calculation, and
> > # you need build a newer version on your own.
> > +# The path must terminate with '/'.
> > EFITOOLS_PATH = ''
> >
> > +# "--addcert" option of sbsign must be available, otherwise
> > +# you need build a newer version on your own.
> > +# The path must terminate with '/'.
> > +SBSIGN_PATH= '/home/akashi/arm/misc/sbsigntools/src/'
> > +
> > # Hello World application for sandbox
> > HELLO_PATH = ''
> > diff --git a/test/py/tests/test_efi_secboot/openssl.cnf b/test/py/tests/test_efi_secboot/openssl.cnf
> > new file mode 100644
> > index 000000000000..f684f1df7e69
> > --- /dev/null
> > +++ b/test/py/tests/test_efi_secboot/openssl.cnf
> > @@ -0,0 +1,48 @@
> > +[ ca ]
> > +default_ca = CA_default
> > +
> > +[ CA_default ]
> > +new_certs_dir = .
> > +database = ./index.txt
> > +serial = ./serial
> > +default_md = sha256
> > +policy = policy_min
> > +
> > +[ req ]
> > +distinguished_name = def_distinguished_name
> > +
> > +[def_distinguished_name]
> > +
> > +# Extensions
> > +# -addext " ... = ..."
> > +#
> > +[ v3_ca ]
> > + # Extensions for a typical Root CA.
> > + basicConstraints = critical,CA:TRUE
> > + keyUsage = critical, digitalSignature, cRLSign, keyCertSign
> > + subjectKeyIdentifier = hash
> > + authorityKeyIdentifier = keyid:always,issuer
> > +
> > +[ v3_int_ca ]
> > + # Extensions for a typical intermediate CA.
> > + basicConstraints = critical, CA:TRUE
> > + keyUsage = critical, digitalSignature, cRLSign, keyCertSign
> > + subjectKeyIdentifier = hash
> > + authorityKeyIdentifier = keyid:always,issuer
> > +
> > +[ usr_cert ]
> > + # Extensions for user end certificates.
> > + basicConstraints = CA:FALSE
> > + keyUsage = critical, nonRepudiation, digitalSignature, keyEncipherment
> > + extendedKeyUsage = clientAuth, emailProtection
> > + subjectKeyIdentifier = hash
> > + authorityKeyIdentifier = keyid,issuer
> > +
> > +[ policy_min ]
> > + countryName = optional
> > + stateOrProvinceName = optional
> > + localityName = optional
> > + organizationName = optional
> > + organizationalUnitName = optional
> > + commonName = supplied
> > + emailAddress = optional
> > diff --git a/test/py/tests/test_efi_secboot/test_signed_intca.py b/test/py/tests/test_efi_secboot/test_signed_intca.py
> > new file mode 100644
> > index 000000000000..80c1917a2cd3
> > --- /dev/null
> > +++ b/test/py/tests/test_efi_secboot/test_signed_intca.py
> > @@ -0,0 +1,134 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +# Copyright (c) 2020, Linaro Limited
> > +# Author: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > +#
> > +# U-Boot UEFI: Image Authentication Test (signature with certificates chain)
> > +
> > +"""
> > +This test verifies image authentication for a signed image which is signed
> > +by user certificate and contains additional intermediate certificates in its
> > +signature.
> > +"""
> > +
> > +import pytest
> > +
> > + at pytest.mark.boardspec('sandbox')
> > + at pytest.mark.buildconfigspec('efi_secure_boot')
> > + at pytest.mark.buildconfigspec('cmd_efidebug')
> > + at pytest.mark.buildconfigspec('cmd_fat')
> > + at pytest.mark.buildconfigspec('cmd_nvedit_efi')
> > + at pytest.mark.slow
> > +class TestEfiSignedImageExt(object):
> > + def test_efi_signed_image_ext1(self, u_boot_console, efi_boot_env_intca):
> > + """
> > + Test Case 1 - authenticated by root CA in db
> > + """
> > + u_boot_console.restart_uboot()
> > + disk_img = efi_boot_env_intca
> > + with u_boot_console.log.section('Test Case 1a'):
> > + # Test Case 1a, with no Int CA and not authenticated by root CA
> > + output = u_boot_console.run_command_list([
> > + 'host bind 0 %s' % disk_img,
> > + 'fatload host 0:1 4000000 db_c.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
> > + 'fatload host 0:1 4000000 KEK.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> > + 'fatload host 0:1 4000000 PK.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
> > + assert 'Failed to set EFI variable' not in ''.join(output)
> > +
> > + output = u_boot_console.run_command_list([
> > + 'efidebug boot add 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""',
> > + 'efidebug boot next 1',
> > + 'efidebug test bootmgr'])
> > + assert '\'HELLO_a\' failed' in ''.join(output)
> > + assert 'efi_start_image() returned: 26' in ''.join(output)
> > +
> > + with u_boot_console.log.section('Test Case 1b'):
> > + # Test Case 1b, signed and authenticated by root CA
> > + output = u_boot_console.run_command_list([
> > + 'efidebug boot add 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""',
> > + 'efidebug boot next 2',
> > + 'bootefi bootmgr'])
> > + assert 'Hello, world!' in ''.join(output)
> > +
> > + def test_efi_signed_image_ext2(self, u_boot_console, efi_boot_env_intca):
> > + """
> > + Test Case 2 - authenticated by root CA in db
> > + """
> > + u_boot_console.restart_uboot()
> > + disk_img = efi_boot_env_intca
> > + with u_boot_console.log.section('Test Case 2a'):
> > + # Test Case 2a, unsigned and not authenticated by root CA
> > + output = u_boot_console.run_command_list([
> > + 'host bind 0 %s' % disk_img,
> > + 'fatload host 0:1 4000000 KEK.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> > + 'fatload host 0:1 4000000 PK.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
> > + assert 'Failed to set EFI variable' not in ''.join(output)
> > +
> > + output = u_boot_console.run_command_list([
> > + 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
> > + 'efidebug boot next 1',
> > + 'efidebug test bootmgr'])
> > + assert '\'HELLO_abc\' failed' in ''.join(output)
> > + assert 'efi_start_image() returned: 26' in ''.join(output)
> > +
> > + with u_boot_console.log.section('Test Case 2b'):
> > + # Test Case 2b, signed and authenticated by root CA
> > + output = u_boot_console.run_command_list([
> > + 'fatload host 0:1 4000000 db_b.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
> > + 'efidebug boot next 1',
> > + 'efidebug test bootmgr'])
> > + assert '\'HELLO_abc\' failed' in ''.join(output)
> > + assert 'efi_start_image() returned: 26' in ''.join(output)
> > +
> > + with u_boot_console.log.section('Test Case 2c'):
> > + # Test Case 2c, signed and authenticated by root CA
> > + output = u_boot_console.run_command_list([
> > + 'fatload host 0:1 4000000 db_c.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
> > + 'efidebug boot next 1',
> > + 'efidebug test bootmgr'])
> > + assert 'Hello, world!' in ''.join(output)
> > +
> > + def test_efi_signed_image_ext3(self, u_boot_console, efi_boot_env_intca):
> > + """
> > + Test Case 3 - revoked by dbx
> > + """
> > + u_boot_console.restart_uboot()
> > + disk_img = efi_boot_env_intca
> > + with u_boot_console.log.section('Test Case 3a'):
> > + # Test Case 3a, revoked by root CA in dbx
> > + output = u_boot_console.run_command_list([
> > + 'host bind 0 %s' % disk_img,
> > + 'fatload host 0:1 4000000 dbx_c.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx',
> > + 'fatload host 0:1 4000000 db_c.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
> > + 'fatload host 0:1 4000000 KEK.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> > + 'fatload host 0:1 4000000 PK.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
> > + assert 'Failed to set EFI variable' not in ''.join(output)
> > +
> > + output = u_boot_console.run_command_list([
> > + 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
> > + 'efidebug boot next 1',
> > + 'efidebug test bootmgr'])
> > + assert '\'HELLO_abc\' failed' in ''.join(output)
> > + assert 'efi_start_image() returned: 26' in ''.join(output)
> > +
> > + with u_boot_console.log.section('Test Case 3b'):
> > + # Test Case 3b, revoked by int CA in dbx
> > + output = u_boot_console.run_command_list([
> > + 'fatload host 0:1 4000000 dbx_b.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx',
> > + 'efidebug boot next 1',
> > + 'efidebug test bootmgr'])
> > + assert 'Hello, world!' in ''.join(output)
> > + # Or,
> > + # assert('\'HELLO_abc\' failed' in ''.join(output))
> > + # assert('efi_start_image() returned: 26' in ''.join(output))
> >
>
^ permalink raw reply [flat|nested] 25+ messages in thread* using sudo?, Re: [PATCH v2 8/8] test/py: efi_secboot: add test for intermediate certificates
2020-07-07 10:42 ` Heinrich Schuchardt
2020-07-08 6:39 ` AKASHI Takahiro
@ 2020-07-09 0:58 ` AKASHI Takahiro
2020-07-09 3:15 ` Tom Rini
1 sibling, 1 reply; 25+ messages in thread
From: AKASHI Takahiro @ 2020-07-09 0:58 UTC (permalink / raw)
To: u-boot
Hi Tom,
I'd like to make sure of your policy about usage of "sudo" on CI.
Do you think that we should always avoid using "sudo" in testing?
I remember that you had allowed us to run sudo in (python)
test scripts on Travis CI when I requested this (for FAT filesystem?).
-Takahiro Akashi
On Tue, Jul 07, 2020 at 12:42:35PM +0200, Heinrich Schuchardt wrote:
> On 16.06.20 07:26, AKASHI Takahiro wrote:
> > In this test case, an image may have a signature with additional
> > intermediate certificates. A chain of trust will be followed and all
> > the certificates in the middle of chain must be verified before loading.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > test/py/tests/test_efi_secboot/conftest.py | 138 +++++++++++++++++-
> > test/py/tests/test_efi_secboot/defs.py | 11 +-
> > test/py/tests/test_efi_secboot/openssl.cnf | 48 ++++++
> > .../test_efi_secboot/test_signed_intca.py | 134 +++++++++++++++++
> > 4 files changed, 328 insertions(+), 3 deletions(-)
> > create mode 100644 test/py/tests/test_efi_secboot/openssl.cnf
> > create mode 100644 test/py/tests/test_efi_secboot/test_signed_intca.py
> >
> > diff --git a/test/py/tests/test_efi_secboot/conftest.py b/test/py/tests/test_efi_secboot/conftest.py
> > index 34abcd79ae00..e5ac2a2a21b7 100644
> > --- a/test/py/tests/test_efi_secboot/conftest.py
> > +++ b/test/py/tests/test_efi_secboot/conftest.py
> > @@ -37,7 +37,7 @@ def efi_boot_env(request, u_boot_config):
> > global HELLO_PATH
> >
> > image_path = u_boot_config.persistent_data_dir
> > - image_path = image_path + '/' + EFI_SECBOOT_IMAGE_NAME
> > + image_path = image_path + '/' + EFI_SECBOOT_IMAGE_NAME + '.img'
> > image_size = EFI_SECBOOT_IMAGE_SIZE
> > part_size = EFI_SECBOOT_PART_SIZE
> > fs_type = EFI_SECBOOT_FS_TYPE
> > @@ -46,7 +46,7 @@ def efi_boot_env(request, u_boot_config):
> > HELLO_PATH = u_boot_config.build_dir + '/lib/efi_loader/helloworld.efi'
> >
> > try:
> > - mnt_point = u_boot_config.persistent_data_dir + '/mnt_efisecure'
> > + mnt_point = u_boot_config.persistent_data_dir + MNTPNT
> > check_call('mkdir -p {}'.format(mnt_point), shell=True)
> >
> > # create a disk/partition
> > @@ -170,3 +170,137 @@ def efi_boot_env(request, u_boot_config):
> > yield image_path
> > finally:
> > call('rm -f %s' % image_path, shell=True)
> > +
> > +#
> > +# Fixture for UEFI secure boot test of intermediate certificates
>
> Thanks for adding a test.
>
>
> > +#
> > + at pytest.fixture(scope='session')
> > +def efi_boot_env_intca(request, u_boot_config):
> > + """Set up a file system to be used in UEFI secure boot test
> > + of intermediate certificates.
> > +
> > + Args:
> > + request: Pytest request object.
> > + u_boot_config: U-boot configuration.
> > +
> > + Return:
> > + A path to disk image to be used for testing
> > + """
> > + global HELLO_PATH
> > +
> > + image_path = u_boot_config.persistent_data_dir
> > + image_path = image_path + '/' + EFI_SECBOOT_IMAGE_NAME + '_intca.img'
> > + image_size = EFI_SECBOOT_IMAGE_SIZE
> > + part_size = EFI_SECBOOT_PART_SIZE
> > + fs_type = EFI_SECBOOT_FS_TYPE
> > +
> > + if HELLO_PATH == '':
> > + HELLO_PATH = u_boot_config.build_dir + '/lib/efi_loader/helloworld.efi'
> > +
> > + try:
> > + mnt_point = u_boot_config.persistent_data_dir + MNTPNT
> > + check_call('mkdir -p {}'.format(mnt_point), shell=True)
> > +
> > + # create a disk/partition
> > + check_call('dd if=/dev/zero of=%s bs=1MiB count=%d'
> > + % (image_path, image_size), shell=True)
> > + check_call('sgdisk %s -n 1:0:+%dMiB'
> > + % (image_path, part_size), shell=True)
> > + # create a file system
> > + check_call('dd if=/dev/zero of=%s.tmp bs=1MiB count=%d'
> > + % (image_path, part_size), shell=True)
> > + check_call('mkfs -t %s %s.tmp' % (fs_type, image_path), shell=True)
> > + check_call('dd if=%s.tmp of=%s bs=1MiB seek=1 count=%d conv=notrunc'
> > + % (image_path, image_path, 1), shell=True)
> > + check_call('rm %s.tmp' % image_path, shell=True)
> > + loop_dev = check_output('sudo losetup -o 1MiB --sizelimit %dMiB --show -f %s | tr -d "\n"'
> > + % (part_size, image_path), shell=True).decode()
> > + check_output('sudo mount -t %s -o umask=000 %s %s'
> > + % (fs_type, loop_dev, mnt_point), shell=True)
>
> Can we use virt-make-fs to avoid sudo, please. Package libguestfs-tools
> has been added to the Docker image for Gitlab recently.
>
> > +
> > + # Create signature database
> > + ## PK
> > + check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_PK/ -keyout PK.key -out PK.crt -nodes -days 365'
> > + % mnt_point, shell=True)
> > + check_call('cd %s; %scert-to-efi-sig-list -g %s PK.crt PK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key PK PK.esl PK.auth'
> > + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > + shell=True)
> > + ## KEK
> > + check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_KEK/ -keyout KEK.key -out KEK.crt -nodes -days 365'
> > + % mnt_point, shell=True)
> > + check_call('cd %s; %scert-to-efi-sig-list -g %s KEK.crt KEK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key KEK KEK.esl KEK.auth'
> > + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > + shell=True)
> > +
> > + # We will have three-tier hierarchy of certificates:
> > + # TestRoot: Root CA (self-signed)
> > + # TestSub: Intermediate CA (signed by Root CA)
> > + # TestCert: User certificate (signed by Intermediate CA, and used
> > + # for signing an image)
> > + #
> > + # NOTE:
> > + # I consulted the following EDK2 document for certificate options:
> > + # BaseTools/Source/Python/Pkcs7Sign/Readme.md
> > + # Please not use them as they are in product system. They are
> > + # for test purpose only.
> > +
> > + # TestRoot
> > + check_call('cp %s/test/py/tests/test_efi_secboot/openssl.cnf %s' % (u_boot_config.source_dir, mnt_point), shell=True)
> > + check_call('cd %s; openssl genrsa -out TestRoot.key 2048; openssl req --config openssl.cnf -extensions v3_ca -new -x509 -days 365 -key TestRoot.key -out TestRoot.crt -subj "/CN=TEST_root/"; touch index.txt' % mnt_point, shell=True)
>
> Please, use the .format() function of the string class.
>
> Best regards
>
> Heinrich
>
> > + # TestSub
> > + check_call('cd %s; openssl genrsa -out TestSub.key 2048; openssl req -new -key TestSub.key -out TestSub.csr -subj "/CN=TEST_sub/"; openssl ca --config openssl.cnf -in TestSub.csr -out TestSub.crt -extensions v3_int_ca -days 365 -batch -rand_serial -cert TestRoot.crt -keyfile TestRoot.key' % mnt_point, shell=True)
> > + # TestCert
> > + check_call('cd %s; openssl genrsa -out TestCert.key 2048; openssl req -new -key TestCert.key -out TestCert.csr -subj "/CN=TEST_cert/"; openssl ca --config openssl.cnf -in TestCert.csr -out TestCert.crt -extensions usr_cert -days 365 -batch -rand_serial -cert TestSub.crt -keyfile TestSub.key' % mnt_point, shell=True)
> > + ## db
> > + # for TestCert
> > + check_call('cd %s; %scert-to-efi-sig-list -g %s TestCert.crt TestCert.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db TestCert.esl db_a.auth'
> > + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > + shell=True)
> > + # for TestSub
> > + check_call('cd %s; %scert-to-efi-sig-list -g %s TestSub.crt TestSub.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db TestSub.esl db_b.auth'
> > + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > + shell=True)
> > + # for TestRoot
> > + check_call('cd %s; %scert-to-efi-sig-list -g %s TestRoot.crt TestRoot.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db TestRoot.esl db_c.auth'
> > + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > + shell=True)
> > + ## dbx (hash of certificate with revocation time)
> > + # for TestCert
> > + check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 TestCert.crt TestCert.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx TestCert.crl dbx_a.auth'
> > + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > + shell=True)
> > + # for TestSub
> > + check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 TestSub.crt TestSub.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx TestSub.crl dbx_b.auth'
> > + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > + shell=True)
> > + # for TestRoot
> > + check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 TestRoot.crt TestRoot.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx TestRoot.crl dbx_c.auth'
> > + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > + shell=True)
> > +
> > + # Sign image
> > + # additional intermediate certificates may be included
> > + # in SignedData
> > +
> > + check_call('cp %s %s' % (HELLO_PATH, mnt_point), shell=True)
> > + ## signed by TestCert
> > + check_call('cd %s; %ssbsign --key TestCert.key --cert TestCert.crt --out helloworld.efi.signed_a helloworld.efi'
> > + % (mnt_point, SBSIGN_PATH), shell=True)
> > + ## signed by TestCert with TestSub in signature
> > + check_call('cd %s; %ssbsign --key TestCert.key --cert TestCert.crt --addcert TestSub.crt --out helloworld.efi.signed_ab helloworld.efi'
> > + % (mnt_point, SBSIGN_PATH), shell=True)
> > + ## signed by TestCert with TestSub and TestRoot in signature
> > + check_call('cd %s; cat TestSub.crt TestRoot.crt > TestSubRoot.crt; %ssbsign --key TestCert.key --cert TestCert.crt --addcert TestSubRoot.crt --out helloworld.efi.signed_abc helloworld.efi'
> > + % (mnt_point, SBSIGN_PATH), shell=True)
> > +
> > + # Clean-up
> > + check_call('sudo umount %s' % loop_dev, shell=True)
> > + check_call('sudo losetup -d %s' % loop_dev, shell=True)
> > +
> > + except CalledProcessError as e:
> > + pytest.skip('Setup failed: %s' % e.cmd)
> > + return
> > + else:
> > + yield image_path
> > + finally:
> > + call('rm -f %s' % image_path, shell=True)
> > diff --git a/test/py/tests/test_efi_secboot/defs.py b/test/py/tests/test_efi_secboot/defs.py
> > index 099f453979ff..c61f69a316f8 100644
> > --- a/test/py/tests/test_efi_secboot/defs.py
> > +++ b/test/py/tests/test_efi_secboot/defs.py
> > @@ -1,7 +1,7 @@
> > # SPDX-License-Identifier: GPL-2.0+
> >
> > # Disk image name
> > -EFI_SECBOOT_IMAGE_NAME = 'test_efi_secboot.img'
> > +EFI_SECBOOT_IMAGE_NAME = 'test_efi_secboot'
> >
> > # Size in MiB
> > EFI_SECBOOT_IMAGE_SIZE = 16
> > @@ -10,12 +10,21 @@ EFI_SECBOOT_PART_SIZE = 8
> > # Partition file system type
> > EFI_SECBOOT_FS_TYPE = 'vfat'
> >
> > +# Mount point
> > +MNTPNT= 'mnt'
> > +
> > # Owner guid
> > GUID = '11111111-2222-3333-4444-123456789abc'
> >
> > # v1.5.1 or earlier of efitools has a bug in sha256 calculation, and
> > # you need build a newer version on your own.
> > +# The path must terminate with '/'.
> > EFITOOLS_PATH = ''
> >
> > +# "--addcert" option of sbsign must be available, otherwise
> > +# you need build a newer version on your own.
> > +# The path must terminate with '/'.
> > +SBSIGN_PATH= '/home/akashi/arm/misc/sbsigntools/src/'
> > +
> > # Hello World application for sandbox
> > HELLO_PATH = ''
> > diff --git a/test/py/tests/test_efi_secboot/openssl.cnf b/test/py/tests/test_efi_secboot/openssl.cnf
> > new file mode 100644
> > index 000000000000..f684f1df7e69
> > --- /dev/null
> > +++ b/test/py/tests/test_efi_secboot/openssl.cnf
> > @@ -0,0 +1,48 @@
> > +[ ca ]
> > +default_ca = CA_default
> > +
> > +[ CA_default ]
> > +new_certs_dir = .
> > +database = ./index.txt
> > +serial = ./serial
> > +default_md = sha256
> > +policy = policy_min
> > +
> > +[ req ]
> > +distinguished_name = def_distinguished_name
> > +
> > +[def_distinguished_name]
> > +
> > +# Extensions
> > +# -addext " ... = ..."
> > +#
> > +[ v3_ca ]
> > + # Extensions for a typical Root CA.
> > + basicConstraints = critical,CA:TRUE
> > + keyUsage = critical, digitalSignature, cRLSign, keyCertSign
> > + subjectKeyIdentifier = hash
> > + authorityKeyIdentifier = keyid:always,issuer
> > +
> > +[ v3_int_ca ]
> > + # Extensions for a typical intermediate CA.
> > + basicConstraints = critical, CA:TRUE
> > + keyUsage = critical, digitalSignature, cRLSign, keyCertSign
> > + subjectKeyIdentifier = hash
> > + authorityKeyIdentifier = keyid:always,issuer
> > +
> > +[ usr_cert ]
> > + # Extensions for user end certificates.
> > + basicConstraints = CA:FALSE
> > + keyUsage = critical, nonRepudiation, digitalSignature, keyEncipherment
> > + extendedKeyUsage = clientAuth, emailProtection
> > + subjectKeyIdentifier = hash
> > + authorityKeyIdentifier = keyid,issuer
> > +
> > +[ policy_min ]
> > + countryName = optional
> > + stateOrProvinceName = optional
> > + localityName = optional
> > + organizationName = optional
> > + organizationalUnitName = optional
> > + commonName = supplied
> > + emailAddress = optional
> > diff --git a/test/py/tests/test_efi_secboot/test_signed_intca.py b/test/py/tests/test_efi_secboot/test_signed_intca.py
> > new file mode 100644
> > index 000000000000..80c1917a2cd3
> > --- /dev/null
> > +++ b/test/py/tests/test_efi_secboot/test_signed_intca.py
> > @@ -0,0 +1,134 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +# Copyright (c) 2020, Linaro Limited
> > +# Author: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > +#
> > +# U-Boot UEFI: Image Authentication Test (signature with certificates chain)
> > +
> > +"""
> > +This test verifies image authentication for a signed image which is signed
> > +by user certificate and contains additional intermediate certificates in its
> > +signature.
> > +"""
> > +
> > +import pytest
> > +
> > + at pytest.mark.boardspec('sandbox')
> > + at pytest.mark.buildconfigspec('efi_secure_boot')
> > + at pytest.mark.buildconfigspec('cmd_efidebug')
> > + at pytest.mark.buildconfigspec('cmd_fat')
> > + at pytest.mark.buildconfigspec('cmd_nvedit_efi')
> > + at pytest.mark.slow
> > +class TestEfiSignedImageExt(object):
> > + def test_efi_signed_image_ext1(self, u_boot_console, efi_boot_env_intca):
> > + """
> > + Test Case 1 - authenticated by root CA in db
> > + """
> > + u_boot_console.restart_uboot()
> > + disk_img = efi_boot_env_intca
> > + with u_boot_console.log.section('Test Case 1a'):
> > + # Test Case 1a, with no Int CA and not authenticated by root CA
> > + output = u_boot_console.run_command_list([
> > + 'host bind 0 %s' % disk_img,
> > + 'fatload host 0:1 4000000 db_c.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
> > + 'fatload host 0:1 4000000 KEK.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> > + 'fatload host 0:1 4000000 PK.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
> > + assert 'Failed to set EFI variable' not in ''.join(output)
> > +
> > + output = u_boot_console.run_command_list([
> > + 'efidebug boot add 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""',
> > + 'efidebug boot next 1',
> > + 'efidebug test bootmgr'])
> > + assert '\'HELLO_a\' failed' in ''.join(output)
> > + assert 'efi_start_image() returned: 26' in ''.join(output)
> > +
> > + with u_boot_console.log.section('Test Case 1b'):
> > + # Test Case 1b, signed and authenticated by root CA
> > + output = u_boot_console.run_command_list([
> > + 'efidebug boot add 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""',
> > + 'efidebug boot next 2',
> > + 'bootefi bootmgr'])
> > + assert 'Hello, world!' in ''.join(output)
> > +
> > + def test_efi_signed_image_ext2(self, u_boot_console, efi_boot_env_intca):
> > + """
> > + Test Case 2 - authenticated by root CA in db
> > + """
> > + u_boot_console.restart_uboot()
> > + disk_img = efi_boot_env_intca
> > + with u_boot_console.log.section('Test Case 2a'):
> > + # Test Case 2a, unsigned and not authenticated by root CA
> > + output = u_boot_console.run_command_list([
> > + 'host bind 0 %s' % disk_img,
> > + 'fatload host 0:1 4000000 KEK.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> > + 'fatload host 0:1 4000000 PK.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
> > + assert 'Failed to set EFI variable' not in ''.join(output)
> > +
> > + output = u_boot_console.run_command_list([
> > + 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
> > + 'efidebug boot next 1',
> > + 'efidebug test bootmgr'])
> > + assert '\'HELLO_abc\' failed' in ''.join(output)
> > + assert 'efi_start_image() returned: 26' in ''.join(output)
> > +
> > + with u_boot_console.log.section('Test Case 2b'):
> > + # Test Case 2b, signed and authenticated by root CA
> > + output = u_boot_console.run_command_list([
> > + 'fatload host 0:1 4000000 db_b.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
> > + 'efidebug boot next 1',
> > + 'efidebug test bootmgr'])
> > + assert '\'HELLO_abc\' failed' in ''.join(output)
> > + assert 'efi_start_image() returned: 26' in ''.join(output)
> > +
> > + with u_boot_console.log.section('Test Case 2c'):
> > + # Test Case 2c, signed and authenticated by root CA
> > + output = u_boot_console.run_command_list([
> > + 'fatload host 0:1 4000000 db_c.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
> > + 'efidebug boot next 1',
> > + 'efidebug test bootmgr'])
> > + assert 'Hello, world!' in ''.join(output)
> > +
> > + def test_efi_signed_image_ext3(self, u_boot_console, efi_boot_env_intca):
> > + """
> > + Test Case 3 - revoked by dbx
> > + """
> > + u_boot_console.restart_uboot()
> > + disk_img = efi_boot_env_intca
> > + with u_boot_console.log.section('Test Case 3a'):
> > + # Test Case 3a, revoked by root CA in dbx
> > + output = u_boot_console.run_command_list([
> > + 'host bind 0 %s' % disk_img,
> > + 'fatload host 0:1 4000000 dbx_c.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx',
> > + 'fatload host 0:1 4000000 db_c.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
> > + 'fatload host 0:1 4000000 KEK.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> > + 'fatload host 0:1 4000000 PK.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
> > + assert 'Failed to set EFI variable' not in ''.join(output)
> > +
> > + output = u_boot_console.run_command_list([
> > + 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
> > + 'efidebug boot next 1',
> > + 'efidebug test bootmgr'])
> > + assert '\'HELLO_abc\' failed' in ''.join(output)
> > + assert 'efi_start_image() returned: 26' in ''.join(output)
> > +
> > + with u_boot_console.log.section('Test Case 3b'):
> > + # Test Case 3b, revoked by int CA in dbx
> > + output = u_boot_console.run_command_list([
> > + 'fatload host 0:1 4000000 dbx_b.auth',
> > + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx',
> > + 'efidebug boot next 1',
> > + 'efidebug test bootmgr'])
> > + assert 'Hello, world!' in ''.join(output)
> > + # Or,
> > + # assert('\'HELLO_abc\' failed' in ''.join(output))
> > + # assert('efi_start_image() returned: 26' in ''.join(output))
> >
>
^ permalink raw reply [flat|nested] 25+ messages in thread* using sudo?, Re: [PATCH v2 8/8] test/py: efi_secboot: add test for intermediate certificates
2020-07-09 0:58 ` using sudo?, " AKASHI Takahiro
@ 2020-07-09 3:15 ` Tom Rini
2020-07-09 5:33 ` AKASHI Takahiro
0 siblings, 1 reply; 25+ messages in thread
From: Tom Rini @ 2020-07-09 3:15 UTC (permalink / raw)
To: u-boot
On Thu, Jul 09, 2020 at 09:58:03AM +0900, AKASHI Takahiro wrote:
> Hi Tom,
>
> I'd like to make sure of your policy about usage of "sudo" on CI.
> Do you think that we should always avoid using "sudo" in testing?
>
> I remember that you had allowed us to run sudo in (python)
> test scripts on Travis CI when I requested this (for FAT filesystem?).
So, the best practices at this time are to have the code try and use
guestmount (or similar tools) when possible and fall back to sudo, as
Ubuntu breaks guestmount (and similar tools) by default.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200708/80710a54/attachment.sig>
^ permalink raw reply [flat|nested] 25+ messages in thread
* using sudo?, Re: [PATCH v2 8/8] test/py: efi_secboot: add test for intermediate certificates
2020-07-09 3:15 ` Tom Rini
@ 2020-07-09 5:33 ` AKASHI Takahiro
2020-07-09 12:34 ` Tom Rini
0 siblings, 1 reply; 25+ messages in thread
From: AKASHI Takahiro @ 2020-07-09 5:33 UTC (permalink / raw)
To: u-boot
Tom,
On Wed, Jul 08, 2020 at 11:15:26PM -0400, Tom Rini wrote:
> On Thu, Jul 09, 2020 at 09:58:03AM +0900, AKASHI Takahiro wrote:
>
> > Hi Tom,
> >
> > I'd like to make sure of your policy about usage of "sudo" on CI.
> > Do you think that we should always avoid using "sudo" in testing?
> >
> > I remember that you had allowed us to run sudo in (python)
> > test scripts on Travis CI when I requested this (for FAT filesystem?).
>
> So, the best practices at this time are to have the code try and use
> guestmount (or similar tools) when possible and fall back to sudo, as
> Ubuntu breaks guestmount (and similar tools) by default.
See the commands log (on my ubuntu 19.10) below:
===8<===
<< try 1 >>
tmp$ mkdir tmpdir
tmp$ virt-make-fs -t vfat -s +1M --partition=gpt ./tmpdir tmp.img
libguestfs: error: /usr/bin/supermin exited with error status 1.
To see full error messages you may need to enable debugging.
Do:
export LIBGUESTFS_DEBUG=1 LIBGUESTFS_TRACE=1
and run the command again. For further information, read:
http://libguestfs.org/guestfs-faq.1.html#debugging-libguestfs
You can also run 'libguestfs-test-tool' and post the *complete* output
into a bug report or message to the libguestfs mailing list.
<< try 2 >>
tmp$ LIBGUESTFS_DEBUG=1 virt-make-fs -t vfat -s +1M --partition=gpt ./tmpdir tmp.img
...
supermin: kernel: kernel_version 5.3.0-62-generic
supermin: kernel: modpath /lib/modules/5.3.0-62-generic
cp: cannot open '/boot/vmlinuz-5.3.0-62-generic' for reading: Permission denied
supermin: cp -p '/boot/vmlinuz-5.3.0-62-generic' '/var/tmp/.guestfs-1000/appliance.d.op62psoy/kernel': command failed, see earlier errors
libguestfs: error: /usr/bin/supermin exited with error status 1, see debug messages above
...
<< try 3 >>
tmp$ sudo chmod a+rw /boot/vmlinuz-5.3.0-62-generic
tmp$ LIBGUESTFS_DEBUG=1 virt-make-fs -t vfat -s +1M --partition=gpt ./tmpdir tmp.img
...
tmp$ ls -l tmp.img
-rw-r--r-- 1 akashi akashi 1341440 Jul 9 13:50 tmp.img
===>8===
As you can see, virt-make-fs will fail on *standard* ubuntu.
You have to change the permission of the current kernel's binary.
While I can't make sure, we will have the same issue with guestmount
as it will also create a minimum virtual machine before execution.
What does it mean?
You must change the permission every time when you re-install the OS
or re-bump the kernel version. Obviously, I can't do that from my own
test script (without sudo).
So if you don't have any way (or workaround) to deal with it,
libguestfs-tools or guestmount cannot be a solution here.
-Takahiro Akashi
> --
> Tom
^ permalink raw reply [flat|nested] 25+ messages in thread
* using sudo?, Re: [PATCH v2 8/8] test/py: efi_secboot: add test for intermediate certificates
2020-07-09 5:33 ` AKASHI Takahiro
@ 2020-07-09 12:34 ` Tom Rini
0 siblings, 0 replies; 25+ messages in thread
From: Tom Rini @ 2020-07-09 12:34 UTC (permalink / raw)
To: u-boot
On Thu, Jul 09, 2020 at 02:33:49PM +0900, AKASHI Takahiro wrote:
> Tom,
>
> On Wed, Jul 08, 2020 at 11:15:26PM -0400, Tom Rini wrote:
> > On Thu, Jul 09, 2020 at 09:58:03AM +0900, AKASHI Takahiro wrote:
> >
> > > Hi Tom,
> > >
> > > I'd like to make sure of your policy about usage of "sudo" on CI.
> > > Do you think that we should always avoid using "sudo" in testing?
> > >
> > > I remember that you had allowed us to run sudo in (python)
> > > test scripts on Travis CI when I requested this (for FAT filesystem?).
> >
> > So, the best practices at this time are to have the code try and use
> > guestmount (or similar tools) when possible and fall back to sudo, as
> > Ubuntu breaks guestmount (and similar tools) by default.
>
> See the commands log (on my ubuntu 19.10) below:
>
> ===8<===
> << try 1 >>
> tmp$ mkdir tmpdir
> tmp$ virt-make-fs -t vfat -s +1M --partition=gpt ./tmpdir tmp.img
> libguestfs: error: /usr/bin/supermin exited with error status 1.
> To see full error messages you may need to enable debugging.
> Do:
> export LIBGUESTFS_DEBUG=1 LIBGUESTFS_TRACE=1
> and run the command again. For further information, read:
> http://libguestfs.org/guestfs-faq.1.html#debugging-libguestfs
> You can also run 'libguestfs-test-tool' and post the *complete* output
> into a bug report or message to the libguestfs mailing list.
>
> << try 2 >>
> tmp$ LIBGUESTFS_DEBUG=1 virt-make-fs -t vfat -s +1M --partition=gpt ./tmpdir tmp.img
> ...
> supermin: kernel: kernel_version 5.3.0-62-generic
> supermin: kernel: modpath /lib/modules/5.3.0-62-generic
> cp: cannot open '/boot/vmlinuz-5.3.0-62-generic' for reading: Permission denied
> supermin: cp -p '/boot/vmlinuz-5.3.0-62-generic' '/var/tmp/.guestfs-1000/appliance.d.op62psoy/kernel': command failed, see earlier errors
> libguestfs: error: /usr/bin/supermin exited with error status 1, see debug messages above
> ...
>
> << try 3 >>
> tmp$ sudo chmod a+rw /boot/vmlinuz-5.3.0-62-generic
> tmp$ LIBGUESTFS_DEBUG=1 virt-make-fs -t vfat -s +1M --partition=gpt ./tmpdir tmp.img
> ...
> tmp$ ls -l tmp.img
> -rw-r--r-- 1 akashi akashi 1341440 Jul 9 13:50 tmp.img
> ===>8===
>
> As you can see, virt-make-fs will fail on *standard* ubuntu.
> You have to change the permission of the current kernel's binary.
Yes, exactly. This is an intentional behavior in Ubuntu (and not
Debian) and why we cannot rely on the various virt tools working.
I fixed the current tests over in
http://patchwork.ozlabs.org/project/uboot/patch/20200707155309.24770-1-trini at konsulko.com/
but need to follow up and try what Stephen was saying to clean it up
more still.
> While I can't make sure, we will have the same issue with guestmount
> as it will also create a minimum virtual machine before execution.
>
> What does it mean?
> You must change the permission every time when you re-install the OS
> or re-bump the kernel version. Obviously, I can't do that from my own
> test script (without sudo).
> So if you don't have any way (or workaround) to deal with it,
> libguestfs-tools or guestmount cannot be a solution here.
Well, just like the test_fs tests, we try guestmount, if it doesn't work
we fall back to just sudo'ing what we need to run directly. I think
Ubuntu did something very stupid here. I just don't know if moving CI
to be Debian based (and I guess Travis is just working-around the issue
by default for us, given the fs tests run there today) is good enough as
it will leave everyone else's Ubuntu-based setups broken.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200709/ea823a5d/attachment.sig>
^ permalink raw reply [flat|nested] 25+ messages in thread