From 7d6210e4b1fd5ed16a671a07aaa14a98a9f7c33c Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Thu, 4 Jun 2020 16:50:22 +0900 Subject: [PATCH 01/16] sbsign: allow for adding intermediate certificates SignedData can have multiple certificates, but the current implementation of sbsign only allows a single one (as a signer). With this patch, "-addcert" options will be available on command line to specify a file in which any number of intermediate certificates in PEM format can be concatenated. $ sign --key --cert --addcert [...] image_file Background: I'm working on implementing UEFI secure boot on U-Boot and want to test my code against PE images with intermediate certificates in certificate chain. As far as I know, the only tool that supports it in signing is Microsoft's signtool.exe. So I'd like to have some corresponding tool on linux. Signed-off-by: AKASHI Takahiro Signed-off-by: James Bottomley --- src/sbsign.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/src/sbsign.c b/src/sbsign.c index ff1fdfd..92607a7 100644 --- a/src/sbsign.c +++ b/src/sbsign.c @@ -49,6 +49,8 @@ #include #include #include +#include +#include #include @@ -75,6 +77,7 @@ static struct option options[] = { { "help", no_argument, NULL, 'h' }, { "version", no_argument, NULL, 'V' }, { "engine", required_argument, NULL, 'e'}, + { "addcert", required_argument, NULL, 'a'}, { NULL, 0, NULL, 0 }, }; @@ -88,6 +91,7 @@ static void usage(void) "\t--key signing key (PEM-encoded RSA " "private key)\n" "\t--cert certificate (x509 certificate)\n" + "\t--addcert additional intermediate certificates in a file\n" "\t--detached write a detached signature, instead of\n" "\t a signed binary\n" "\t--output write signed data to \n" @@ -112,9 +116,43 @@ static void set_default_outfilename(struct sign_context *ctx) ctx->infilename, extension); } +static int add_intermediate_certs(PKCS7 *p7, const char *filename) +{ + STACK_OF(X509_INFO) *certs; + X509_INFO *cert; + BIO *bio = NULL; + int i; + + bio = BIO_new(BIO_s_file()); + if (!bio || BIO_read_filename(bio, filename) <=0) { + fprintf(stderr, + "error in reading intermediate certificates file\n"); + ERR_print_errors_fp(stderr); + return -1; + } + + certs = PEM_X509_INFO_read_bio(bio, NULL, NULL, NULL); + if (!certs) { + fprintf(stderr, + "error in parsing intermediate certificates file\n"); + ERR_print_errors_fp(stderr); + return -1; + } + + for (i = 0; i < sk_X509_INFO_num(certs); i++) { + cert = sk_X509_INFO_value(certs, i); + PKCS7_add_certificate(p7, cert->x509); + } + + sk_X509_INFO_pop_free(certs, X509_INFO_free); + BIO_free_all(bio); + + return 0; +} + int main(int argc, char **argv) { - const char *keyfilename, *certfilename, *engine; + const char *keyfilename, *certfilename, *addcertfilename, *engine; struct sign_context *ctx; uint8_t *buf, *tmp; int rc, c, sigsize; @@ -124,11 +162,12 @@ int main(int argc, char **argv) keyfilename = NULL; certfilename = NULL; + addcertfilename = NULL; engine = NULL; for (;;) { int idx; - c = getopt_long(argc, argv, "o:c:k:dvVhe:", options, &idx); + c = getopt_long(argc, argv, "o:c:k:dvVhe:a:", options, &idx); if (c == -1) break; @@ -157,6 +196,9 @@ int main(int argc, char **argv) case 'e': engine = optarg; break; + case 'a': + addcertfilename = optarg; + break; } } @@ -189,6 +231,7 @@ int main(int argc, char **argv) talloc_steal(ctx, ctx->image); ERR_load_crypto_strings(); + ERR_load_BIO_strings(); OpenSSL_add_all_digests(); OpenSSL_add_all_ciphers(); OPENSSL_config(NULL); @@ -228,6 +271,9 @@ int main(int argc, char **argv) if (rc) return EXIT_FAILURE; + if (addcertfilename && add_intermediate_certs(p7, addcertfilename)) + return EXIT_FAILURE; + sigsize = i2d_PKCS7(p7, NULL); tmp = buf = talloc_array(ctx->image, uint8_t, sigsize); i2d_PKCS7(p7, &tmp); From df27a417b92ebdcf4161fd115fc61a204ff7c202 Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Fri, 5 Jun 2020 18:29:07 -0700 Subject: [PATCH 02/16] sbverify: fix verification with intermediate certificates sbverify is currently failing if an intermediate certificate is added on signing but the binary is verified with the singing certificate. It fails with X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY. This is happening because the x509_STORE only contains the signing certificate but the pkcs7 bundle in the binary contains the issuer certificate as well. Fix this by unconditionally approving any locally missing certificates on verify. Signed-off-by: James Bottomley --- src/sbverify.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sbverify.c b/src/sbverify.c index 3920d91..4dddecc 100644 --- a/src/sbverify.c +++ b/src/sbverify.c @@ -210,8 +210,7 @@ static int x509_verify_cb(int status, X509_STORE_CTX *ctx) == XKU_CODE_SIGN) status = 1; - else if (err == X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY || - err == X509_V_ERR_CERT_UNTRUSTED || + else if (err == X509_V_ERR_CERT_UNTRUSTED || err == X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT || err == X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE) { /* all certs given with the --cert argument are trusted */ @@ -221,6 +220,7 @@ static int x509_verify_cb(int status, X509_STORE_CTX *ctx) } else if (err == X509_V_ERR_CERT_HAS_EXPIRED || err == X509_V_ERR_ERROR_IN_CERT_NOT_BEFORE_FIELD || err == X509_V_ERR_CERT_NOT_YET_VALID || + err == X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY || err == X509_V_ERR_ERROR_IN_CERT_NOT_AFTER_FIELD) /* UEFI explicitly allows expired certificates */ status = 1; From 6c2b07fa1c5a2cffffd76a0a0703d2de93cfad06 Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Fri, 5 Jun 2020 18:34:55 -0700 Subject: [PATCH 03/16] Tests: Add intermediate certificate tests to the sign-verify cases Signed-off-by: James Bottomley --- tests/Makefile.am | 27 +++++++++++++++++++++++---- tests/sign-attach-verify.sh | 20 ++++++++++++++++---- tests/sign-verify-detached.sh | 15 +++++++++++++-- tests/sign-verify.sh | 15 +++++++++++++-- tests/test-wrapper.sh | 6 +++++- 5 files changed, 70 insertions(+), 13 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index a6606f0..93f46e2 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -3,6 +3,10 @@ AUTOMAKE_OPTIONS = parallel-tests test_key = private-key.rsa test_cert = public-cert.pem +ca_key = ca-key.ec +ca_cert = ca-cert.pem +int_key = int-key.ec +int_cert = int-cert.pem test_arches = $(EFI_ARCH) check_PROGRAMS = test.pecoff @@ -31,11 +35,25 @@ check_SCRIPTS = test-wrapper.sh AM_CFLAGS=-fpic -I/usr/include/efi -I/usr/include/efi/$(EFI_ARCH) -$(test_key): Makefile +%.rsa: Makefile openssl genrsa -out $@ 2048 -$(test_cert): $(test_key) Makefile - openssl req -x509 -sha256 -subj '/' -new -key $< -out $@ +%.ec: Makefile + openssl genpkey -algorithm ec -pkeyopt ec_paramgen_curve:prime256v1 -out $@ + +$(ca_cert): $(ca_key) Makefile + openssl req -x509 -days 1 -sha256 -subj '/CN=CA Key/' -new -key $< -out $@ + +$(int_cert): $(int_key) $(ca_cert) Makefile + openssl req -new -subj '/CN=Intermediate Certificate/' -key $< -out tmp.req + echo -e "[ca]\nbasicConstraints = critical, CA:true\n" > ca.cnf + openssl x509 -req -sha256 -CA $(ca_cert) -CAkey $(ca_key) -in tmp.req -set_serial 1 -days 1 -extfile ca.cnf -extensions ca -out $@ + -rm -f tmp.req ca.cnf + +$(test_cert): $(test_key) $(int_cert) Makefile + openssl req -new -subj '/CN=Signer Certificate/' -key $< -out tmp.req + openssl x509 -req -sha256 -CA $(int_cert) -CAkey $(int_key) -in tmp.req -set_serial 1 -days 1 -out $@ + -rm -f tmp.req TESTS = sign-verify.sh \ sign-verify-detached.sh \ @@ -65,4 +83,5 @@ AM_TESTS_ENVIRONMENT = TEST_ARCHES='$(test_arches)'; export TEST_ARCHES; SH_LOG_COMPILER = $(srcdir)/test-wrapper.sh EXTRA_DIST = test.S $(TESTS) $(check_SCRIPTS) -CLEANFILES = $(test_key) $(test_cert) +CLEANFILES = $(test_key) $(test_cert) $(int_key) $(int_cert) $(ca_key) \ + $(ca_cert) diff --git a/tests/sign-attach-verify.sh b/tests/sign-attach-verify.sh index 2ae6e70..21ed6db 100755 --- a/tests/sign-attach-verify.sh +++ b/tests/sign-attach-verify.sh @@ -3,7 +3,19 @@ sig="test.sig" signed="test.signed" -"$sbsign" --cert "$cert" --key "$key" --detached --output "$sig" "$image" -cp "$image" "$signed" -"$sbattach" --attach "$sig" "$signed" -"$sbverify" --cert "$cert" "$signed" +"$sbsign" --cert "$cert" --key "$key" --detached --output "$sig" "$image" || exit 1 +cp "$image" "$signed" || exit 1 +"$sbattach" --attach "$sig" "$signed" || exit 1 +"$sbverify" --cert "$cert" "$signed" || exit 1 +"$sbverify" --cert "$intcert" "$signed" || exit 1 +# there's no intermediate cert in the image so it can't chain to the ca which +# is why this should fail +"$sbverify" --cert "$cacert" "$signed" && exit 1 + +# now add intermediates +"$sbsign" --cert "$cert" --key "$key" --addcert "$intcert" --detached --output "$sig" "$image" || exit 1 +cp "$image" "$signed" || exit 1 +"$sbattach" --attach "$sig" "$signed" || exit 1 +"$sbverify" --cert "$cert" "$signed" || exit 1 +"$sbverify" --cert "$intcert" "$signed" || exit 1 +"$sbverify" --cert "$cacert" "$signed" || exit 1 diff --git a/tests/sign-verify-detached.sh b/tests/sign-verify-detached.sh index 7b045e4..d2959be 100755 --- a/tests/sign-verify-detached.sh +++ b/tests/sign-verify-detached.sh @@ -2,5 +2,16 @@ sig="test.sig" -"$sbsign" --cert "$cert" --key "$key" --detached --output $sig "$image" -"$sbverify" --cert "$cert" --detached $sig "$image" +"$sbsign" --cert "$cert" --key "$key" --detached --output $sig "$image" || exit 1 +"$sbverify" --cert "$cert" --detached $sig "$image" || exit 1 +"$sbverify" --cert "$intcert" --detached $sig "$image" || exit 1 +# should fail because no intermediate +"$sbverify" --cert "$cacert" --detached $sig "$image" && exit 1 + +# now make sure everything succeeds with the intermediate added +"$sbsign" --cert "$cert" --key "$key" --addcert "$intcert" --detached --output $sig "$image" || exit 1 +"$sbverify" --cert "$cert" --detached $sig "$image" || exit 1 +"$sbverify" --cert "$intcert" --detached $sig "$image" || exit 1 +"$sbverify" --cert "$cacert" --detached $sig "$image" || exit 1 + +exit 0 diff --git a/tests/sign-verify.sh b/tests/sign-verify.sh index cf493f3..a61aff8 100755 --- a/tests/sign-verify.sh +++ b/tests/sign-verify.sh @@ -2,5 +2,16 @@ signed="test.signed" -"$sbsign" --cert "$cert" --key "$key" --output "$signed" "$image" -"$sbverify" --cert "$cert" "$signed" +"$sbsign" --cert "$cert" --key "$key" --output "$signed" "$image" || exit 1 +"$sbverify" --cert "$cert" "$signed" || exit 1 +"$sbverify" --cert "$intcert" "$signed" || exit 1 +# there's no intermediate cert in the image so it can't chain to the ca which +# is why this should fail +"$sbverify" --cert "$cacert" "$signed" && exit 1 + +# now add the intermediates and each level should succeed +"$sbsign" --cert "$cert" --addcert "$intcert" --key "$key" --output "$signed" "$image" || exit 1 +"$sbverify" --cert "$cert" "$signed" || exit 1 +"$sbverify" --cert "$intcert" "$signed" || exit 1 +"$sbverify" --cert "$cacert" "$signed" || exit 1 + diff --git a/tests/test-wrapper.sh b/tests/test-wrapper.sh index b9c6cf1..4ef6710 100755 --- a/tests/test-wrapper.sh +++ b/tests/test-wrapper.sh @@ -11,8 +11,12 @@ sbattach=$bindir/sbattach key="$datadir/private-key.rsa" cert="$datadir/public-cert.pem" +intkey="$datadir/int-key.ec" +intcert="$datadir/int-cert.pem" +cakey="$datadir/ca-key.ec" +cacert="$datadir/ca-cert.pem" -export basedir datadir bindir sbsign sbverify sbattach key cert +export basedir datadir bindir sbsign sbverify sbattach key cert intkey intcert cakey cacert # 'test' needs to be an absolute path, as we will cd to a temporary # directory before running the test From 311d6c2b9c1129114834f4df9b12a195a66dc4bc Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Sat, 6 Jun 2020 14:44:54 -0700 Subject: [PATCH 04/16] Fix some openssl 1.1.0 deprecated functions replace OPENSSL_config with OPENSSL_init_crypto and ASN1_STRING_data with ASN1_STRING_get0_data Signed-off-by: James Bottomley --- src/idc.c | 8 ++++++++ src/sbattach.c | 4 ++++ src/sbkeysync.c | 8 ++++++++ src/sbsign.c | 4 ++++ src/sbvarsign.c | 4 ++++ src/sbverify.c | 4 ++++ 6 files changed, 32 insertions(+) diff --git a/src/idc.c b/src/idc.c index 236cefd..6d87bd4 100644 --- a/src/idc.c +++ b/src/idc.c @@ -238,7 +238,11 @@ struct idc *IDC_get(PKCS7 *p7, BIO *bio) /* extract the idc from the signed PKCS7 'other' data */ str = p7->d.sign->contents->d.other->value.asn1_string; +#if OPENSSL_VERSION_NUMBER < 0x10100000L idcbuf = buf = ASN1_STRING_data(str); +#else + idcbuf = buf = ASN1_STRING_get0_data(str); +#endif idc = d2i_IDC(NULL, &buf, ASN1_STRING_length(str)); /* If we were passed a BIO, write the idc data, minus type and length, @@ -289,7 +293,11 @@ int IDC_check_hash(struct idc *idc, struct image *image) } /* check hash against the one we calculated from the image */ +#if OPENSSL_VERSION_NUMBER < 0x10100000L buf = ASN1_STRING_data(str); +#else + buf = ASN1_STRING_get0_data(str); +#endif if (memcmp(buf, sha, sizeof(sha))) { fprintf(stderr, "Hash doesn't match image\n"); fprintf(stderr, " got: %s\n", sha256_str(buf)); diff --git a/src/sbattach.c b/src/sbattach.c index a0c01b8..809e24c 100644 --- a/src/sbattach.c +++ b/src/sbattach.c @@ -233,7 +233,11 @@ int main(int argc, char **argv) ERR_load_crypto_strings(); OpenSSL_add_all_digests(); +#if OPENSSL_VERSION_NUMBER < 0x10100000L OPENSSL_config(NULL); +#else + OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CONFIG, NULL); +#endif /* here we may get highly unlikely failures or we'll get a * complaint about FIPS signatures (usually becuase the FIPS * module isn't present). In either case ignore the errors diff --git a/src/sbkeysync.c b/src/sbkeysync.c index 7b17f40..1f37118 100644 --- a/src/sbkeysync.c +++ b/src/sbkeysync.c @@ -208,7 +208,11 @@ static int x509_key_parse(struct key *key, uint8_t *data, size_t len) goto out; key->id_len = ASN1_STRING_length(serial); +#if OPENSSL_VERSION_NUMBER < 0x10100000L key->id = talloc_memdup(key, ASN1_STRING_data(serial), key->id_len); +#else + key->id = talloc_memdup(key, ASN1_STRING_get0_data(serial), key->id_len); +#endif key->description = talloc_array(key, char, description_len); X509_NAME_oneline(X509_get_subject_name(x509), @@ -930,7 +934,11 @@ int main(int argc, char **argv) ERR_load_crypto_strings(); OpenSSL_add_all_digests(); OpenSSL_add_all_ciphers(); +#if OPENSSL_VERSION_NUMBER < 0x10100000L OPENSSL_config(NULL); +#else + OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CONFIG, NULL); +#endif /* here we may get highly unlikely failures or we'll get a * complaint about FIPS signatures (usually becuase the FIPS * module isn't present). In either case ignore the errors diff --git a/src/sbsign.c b/src/sbsign.c index 92607a7..898fe66 100644 --- a/src/sbsign.c +++ b/src/sbsign.c @@ -234,7 +234,11 @@ int main(int argc, char **argv) ERR_load_BIO_strings(); OpenSSL_add_all_digests(); OpenSSL_add_all_ciphers(); +#if OPENSSL_VERSION_NUMBER < 0x10100000L OPENSSL_config(NULL); +#else + OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CONFIG, NULL); +#endif /* here we may get highly unlikely failures or we'll get a * complaint about FIPS signatures (usually becuase the FIPS * module isn't present). In either case ignore the errors diff --git a/src/sbvarsign.c b/src/sbvarsign.c index 273fd0d..92475a2 100644 --- a/src/sbvarsign.c +++ b/src/sbvarsign.c @@ -513,7 +513,11 @@ int main(int argc, char **argv) OpenSSL_add_all_digests(); OpenSSL_add_all_ciphers(); ERR_load_crypto_strings(); +#if OPENSSL_VERSION_NUMBER < 0x10100000L OPENSSL_config(NULL); +#else + OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CONFIG, NULL); +#endif /* here we may get highly unlikely failures or we'll get a * complaint about FIPS signatures (usually becuase the FIPS * module isn't present). In either case ignore the errors diff --git a/src/sbverify.c b/src/sbverify.c index 4dddecc..ac6705e 100644 --- a/src/sbverify.c +++ b/src/sbverify.c @@ -252,7 +252,11 @@ int main(int argc, char **argv) OpenSSL_add_all_digests(); ERR_load_crypto_strings(); +#if OPENSSL_VERSION_NUMBER < 0x10100000L OPENSSL_config(NULL); +#else + OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CONFIG, NULL); +#endif /* here we may get highly unlikely failures or we'll get a * complaint about FIPS signatures (usually becuase the FIPS * module isn't present). In either case ignore the errors From 6b7d5ccb288509512ff1e36357685262e6d4645c Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Sat, 6 Jun 2020 14:50:33 -0700 Subject: [PATCH 05/16] sbvarsign: remove unused global variable Signed-off-by: James Bottomley --- src/sbvarsign.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sbvarsign.c b/src/sbvarsign.c index 92475a2..bd1fc17 100644 --- a/src/sbvarsign.c +++ b/src/sbvarsign.c @@ -105,7 +105,6 @@ static uint32_t default_attrs = EFI_VARIABLE_NON_VOLATILE | static uint32_t attr_invalid = 0xffffffffu; static const char *attr_prefix = "EFI_VARIABLE_"; -static const EFI_GUID default_guid = EFI_GLOBAL_VARIABLE; static const EFI_GUID cert_pkcs7_guid = EFI_CERT_TYPE_PKCS7_GUID; static void set_default_outfilename(struct varsign_context *ctx) From 5aeb513916c7b1d37304e28571ad3daeda9e3cb0 Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Sat, 6 Jun 2020 14:50:51 -0700 Subject: [PATCH 06/16] sbverify: refer to unused function The function print_certificate_store_certs() is currently commented out leading to an unused function warning. Make verbose a level and call this function for levels > 1 (meaning you have to specify -v -v to see it). Signed-off-by: James Bottomley --- src/sbverify.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/sbverify.c b/src/sbverify.c index ac6705e..8f14f35 100644 --- a/src/sbverify.c +++ b/src/sbverify.c @@ -239,7 +239,7 @@ int main(int argc, char **argv) uint8_t *sig_buf; size_t sig_size; struct idc *idc; - bool verbose; + int verbose; BIO *idcbio; PKCS7 *p7; int sig_count = 0; @@ -247,7 +247,7 @@ int main(int argc, char **argv) status = VERIFY_FAIL; certs = X509_STORE_new(); list = 0; - verbose = false; + verbose = 0; detached_sig_filename = NULL; OpenSSL_add_all_digests(); @@ -282,7 +282,7 @@ int main(int argc, char **argv) list = 1; break; case 'v': - verbose = true; + verbose++; break; case 'V': version(); @@ -337,7 +337,8 @@ int main(int argc, char **argv) if (verbose || list) { print_signature_info(p7); - //print_certificate_store_certs(certs); + if (verbose > 1) + print_certificate_store_certs(certs); } if (list) From e3f7d2754147e76302d6b4e7f52f09bb9a2fed47 Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Sat, 6 Jun 2020 15:34:02 -0700 Subject: [PATCH 07/16] Fix errors on 32 bit print format and signed conversion due to big hex types Signed-off-by: James Bottomley --- src/sbkeysync.c | 6 ++++-- src/sbvarsign.c | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/sbkeysync.c b/src/sbkeysync.c index 1f37118..e51f177 100644 --- a/src/sbkeysync.c +++ b/src/sbkeysync.c @@ -54,9 +54,11 @@ #include "fileio.h" #include "efivars.h" +static struct statfs statfstype; + #define EFIVARS_MOUNTPOINT "/sys/firmware/efi/efivars" -#define PSTORE_FSTYPE 0x6165676C -#define EFIVARS_FSTYPE 0xde5e81e4 +#define PSTORE_FSTYPE ((typeof(statfstype.f_type))0x6165676C) +#define EFIVARS_FSTYPE ((typeof(statfstype.f_type))0xde5e81e4) #define EFI_IMAGE_SECURITY_DATABASE_GUID \ { 0xd719b2cb, 0x3d3a, 0x4596, \ diff --git a/src/sbvarsign.c b/src/sbvarsign.c index bd1fc17..15dfe8b 100644 --- a/src/sbvarsign.c +++ b/src/sbvarsign.c @@ -332,7 +332,7 @@ int write_signed(struct varsign_context *ctx, int include_attrs) printf("Wrote signed data:\n"); if (include_attrs) { i = sizeof(ctx->var_attrs); - printf(" [%04zx:%04zx] attrs\n", 0l, i); + printf(" [%04lx:%04zx] attrs\n", 0l, i); } printf(" [%04zx:%04x] authentication descriptor\n", From ff96a590466a1881d2203988fcc39b51af11c519 Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Sat, 6 Jun 2020 15:08:01 -0700 Subject: [PATCH 08/16] Enable -Werror for builds Now that all the build warnings are eliminated, make sure they don't come back Signed-off-by: James Bottomley --- src/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Makefile.am b/src/Makefile.am index 19a7766..e3f039b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -7,7 +7,7 @@ AM_CFLAGS = -Wall -Wextra --std=gnu99 common_SOURCES = idc.c idc.h image.c image.h fileio.c fileio.h \ efivars.h $(coff_headers) common_LDADD = ../lib/ccan/libccan.a $(libcrypto_LIBS) -common_CFLAGS = -I$(top_srcdir)/lib/ccan/ +common_CFLAGS = -I$(top_srcdir)/lib/ccan/ -Werror sbsign_SOURCES = sbsign.c $(common_SOURCES) sbsign_LDADD = $(common_LDADD) From e17dc20591236d21f086f16294ed691544cb6fc2 Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Sat, 6 Jun 2020 15:21:11 -0700 Subject: [PATCH 09/16] docs: add man page for sbkeysync Signed-off-by: James Bottomley --- docs/Makefile.am | 5 +++-- docs/sbkeysync.1.in | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) create mode 100644 docs/sbkeysync.1.in diff --git a/docs/Makefile.am b/docs/Makefile.am index 1b5a588..89ed110 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -1,8 +1,9 @@ -man1_MANS = sbsign.1 sbverify.1 sbattach.1 sbvarsign.1 sbsiglist.1 +man1_MANS = sbsign.1 sbverify.1 sbattach.1 sbvarsign.1 sbsiglist.1 \ + sbkeysync.1 EXTRA_DIST = sbsign.1.in sbverify.1.in sbattach.1.in \ - sbvarsign.1.in sbsiglist.1.in + sbvarsign.1.in sbsiglist.1.in sbkeysync.1.in CLEANFILES = $(man1_MANS) $(builddir)/%.1: $(srcdir)/%.1.in $(top_builddir)/src/% diff --git a/docs/sbkeysync.1.in b/docs/sbkeysync.1.in new file mode 100644 index 0000000..00aa509 --- /dev/null +++ b/docs/sbkeysync.1.in @@ -0,0 +1,2 @@ +[name] +sbkeysync - UEFI secure boot key synchronization tool From d52f7bbb73401aab8a1d59e8d0d686ad9641035e Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Thu, 11 Jun 2020 16:32:13 -0700 Subject: [PATCH 10/16] Version 0.9.4 AKASHI Takahiro (1): sbsign: allow for adding intermediate certificates James Bottomley (8): sbverify: fix verification with intermediate certificates Tests: Add intermediate certificate tests to the sign-verify cases Fix some openssl 1.1.0 deprecated functions sbvarsign: remove unused global variable sbverify: refer to unused function Fix errors on 32 bit Enable -Werror for builds docs: add man page for sbkeysync Signed-off-by: James Bottomley --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 29927a7..4ffb68f 100644 --- a/configure.ac +++ b/configure.ac @@ -1,4 +1,4 @@ -AC_INIT([sbsigntool], [0.9.3], [James.Bottomley@HansenPartnership.com]) +AC_INIT([sbsigntool], [0.9.4], [James.Bottomley@HansenPartnership.com]) AM_INIT_AUTOMAKE() From f12484869c9590682ac3253d583bf59b890bb826 Mon Sep 17 00:00:00 2001 From: dann frazier Date: Wed, 12 Aug 2020 15:27:08 -0600 Subject: [PATCH 11/16] sbkeysync: Don't ignore errors from insert_new_keys() If insert_new_keys() fails, say due to a full variable store, we currently still exit(0). This can make it difficult to know something is wrong. For example, Debian and Ubuntu implement a secureboot-db systemd service to update the DB and DBX, which calls: ExecStart=/usr/bin/sbkeysync --no-default-keystores --keystore /usr/share/secureboot/updates --verbose But although this seemed to succeed on my system, looking at the logs shows a different story: Inserting key update /usr/share/secureboot/updates/dbx/dbxupdate_x64.bin into dbx Error writing key update: Invalid argument Error syncing keystore file /usr/share/secureboot/updates/dbx/dbxupdate_x64.bin Signed-off-by: dann frazier Signed-off-by: James Bottomley --- src/sbkeysync.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/sbkeysync.c b/src/sbkeysync.c index e51f177..7748990 100644 --- a/src/sbkeysync.c +++ b/src/sbkeysync.c @@ -889,10 +889,12 @@ int main(int argc, char **argv) { bool use_default_keystore_dirs; struct sync_context *ctx; + int rc; use_default_keystore_dirs = true; ctx = talloc_zero(NULL, struct sync_context); list_head_init(&ctx->new_keys); + rc = EXIT_SUCCESS; for (;;) { int idx, c; @@ -985,10 +987,10 @@ int main(int argc, char **argv) if (ctx->verbose) print_new_keys(ctx); - if (!ctx->dry_run) - insert_new_keys(ctx); + if (!ctx->dry_run && insert_new_keys(ctx)) + rc = EXIT_FAILURE; talloc_free(ctx); - return EXIT_SUCCESS; + return rc; } From 4b8fc118774a3dfce41cd21fea3e7cda6bbd0a47 Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Mon, 21 Feb 2022 11:22:47 +1100 Subject: [PATCH 12/16] sbvarsign: do not include PKCS#7 attributes The UEFI spec (8.2.2 Using the EFI_VARIABLE_AUTHENTICATION_2 descriptor) includes the following information about constructing the PKCS#7 message for the authentication descriptor under point 4(g): SignedData.signerInfos shall be constructed as: ... - SignerInfo.authenticatedAttributes shall not be present. sbvarsign does not currently honour this, and generates a PKCS#7 message containing authenticated attributes. This is a snippet from OpenSSL's printout of a message I reconstructed from an auth file: signedAttrs: object: contentType (1.2.840.113549.1.9.3) set: OBJECT:pkcs7-data (1.2.840.113549.1.7.1) object: signingTime (1.2.840.113549.1.9.5) set: UTCTIME:Mar 2 11:20:21 2021 GMT object: messageDigest (1.2.840.113549.1.9.4) set: OCTET STRING: 0000 - 99 58 87 86 82 82 b6 4b-c4 6a e4 e5 6b .X.....K.j..k 000d - 51 39 ac c3 b8 21 24 30-0c 28 e6 e3 aa Q9...!$0.(... 001a - 5c 33 c1 80 3f d1 \3..?. Tell OpenSSL to stop adding attributes. This also brings sbvarsign in to line with sign-efi-sig-list. Signed-off-by: Daniel Axtens Signed-off-by: James Bottomley --- src/sbvarsign.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sbvarsign.c b/src/sbvarsign.c index 15dfe8b..58031ec 100644 --- a/src/sbvarsign.c +++ b/src/sbvarsign.c @@ -251,7 +251,7 @@ static int add_auth_descriptor(struct varsign_context *ctx) md = EVP_get_digestbyname("SHA256"); p7 = PKCS7_new(); - flags = PKCS7_BINARY | PKCS7_DETACHED | PKCS7_NOSMIMECAP;; + flags = PKCS7_BINARY | PKCS7_DETACHED | PKCS7_NOSMIMECAP | PKCS7_NOATTR;; PKCS7_set_type(p7, NID_pkcs7_signed); PKCS7_content_new(p7, NID_pkcs7_data); From 25af2eb5e39b5d54703d4489182a6b9d0af58b76 Mon Sep 17 00:00:00 2001 From: Andreas Schwab Date: Fri, 4 Jun 2021 14:36:17 +0200 Subject: [PATCH 13/16] sbsigntool: add support for RISC-V 64-bit PE/COFF images Signed-off-by: Andreas Schwab Signed-off-by: James Bottomley --- configure.ac | 2 +- src/coff/pe.h | 1 + src/image.c | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 4ffb68f..23dbc54 100644 --- a/configure.ac +++ b/configure.ac @@ -65,7 +65,7 @@ PKG_CHECK_MODULES(uuid, uuid, dnl gnu-efi headers require extra include dirs EFI_ARCH=$(uname -m | sed 's/i.86/ia32/;s/arm.*/arm/') -AM_CONDITIONAL(TEST_BINARY_FORMAT, [ test "$EFI_ARCH" = "arm" -o "$EFI_ARCH" = "aarch64" ]) +AM_CONDITIONAL(TEST_BINARY_FORMAT, [ test "$EFI_ARCH" = "arm" -o "$EFI_ARCH" = "aarch64" -o "$EFI_ARCH" = riscv64 ]) ## # no consistent view of where gnu-efi should dump the efi stuff, so find it diff --git a/src/coff/pe.h b/src/coff/pe.h index 0d1036e..198f23d 100644 --- a/src/coff/pe.h +++ b/src/coff/pe.h @@ -152,6 +152,7 @@ #define IMAGE_FILE_MACHINE_TRICORE 0x0520 #define IMAGE_FILE_MACHINE_WCEMIPSV2 0x0169 #define IMAGE_FILE_MACHINE_AARCH64 0xaa64 +#define IMAGE_FILE_MACHINE_RISCV64 0x5064 #define IMAGE_SUBSYSTEM_UNKNOWN 0 #define IMAGE_SUBSYSTEM_NATIVE 1 diff --git a/src/image.c b/src/image.c index 3ada37b..a828b5a 100644 --- a/src/image.c +++ b/src/image.c @@ -239,6 +239,7 @@ static int image_pecoff_parse(struct image *image) switch (magic) { case IMAGE_FILE_MACHINE_AMD64: case IMAGE_FILE_MACHINE_AARCH64: + case IMAGE_FILE_MACHINE_RISCV64: rc = image_pecoff_parse_64(image); break; case IMAGE_FILE_MACHINE_I386: From d6e4bff8f1ec1118282bea9413d74a94388108ac Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Mon, 13 Jun 2022 16:32:45 -0400 Subject: [PATCH 14/16] Add support for openssl-3 We're currently using a raft of APIs which trigger deprecation warnings, so add OPENSSL_API_COMPAT to the command line for openssl-3 to cause them not to break the build. Signed-off-by: James Bottomley --- configure.ac | 9 ++++++--- src/Makefile.am | 4 ++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 23dbc54..23c17f4 100644 --- a/configure.ac +++ b/configure.ac @@ -55,9 +55,12 @@ AC_DEFINE_UNQUOTED(HAVE_LITTLE_ENDIAN, $little_endian, [Little-endian system]) AC_DEFINE_UNQUOTED(HAVE_BIG_ENDIAN, $big_endian, [Big-endian system]) PKG_PROG_PKG_CONFIG() -PKG_CHECK_MODULES(libcrypto, libcrypto, - [], - AC_MSG_ERROR([libcrypto (from the OpenSSL package) is required])) +PKG_CHECK_MODULES(libcrypto, [libcrypto >= 3.0.0], + [ac_have_openssl3=1], + [PKG_CHECK_MODULES(libcrypto, libcrypto, + [], + AC_MSG_ERROR([libcrypto (from the OpenSSL package) is required]))]) +AM_CONDITIONAL(HAVE_OPENSSL3, test "$ac_have_openssl3" = "1") PKG_CHECK_MODULES(uuid, uuid, [], diff --git a/src/Makefile.am b/src/Makefile.am index e3f039b..38f93ff 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -4,6 +4,10 @@ bin_PROGRAMS = sbsign sbverify sbattach sbvarsign sbsiglist sbkeysync coff_headers = coff/external.h coff/pe.h AM_CFLAGS = -Wall -Wextra --std=gnu99 +if HAVE_OPENSSL3 +AM_CFLAGS += -DOPENSSL_API_COMPAT=0x10100000L +endif + common_SOURCES = idc.c idc.h image.c image.h fileio.c fileio.h \ efivars.h $(coff_headers) common_LDADD = ../lib/ccan/libccan.a $(libcrypto_LIBS) From 75d8405eca50bc62da4d6115f09f9a42969092e1 Mon Sep 17 00:00:00 2001 From: Jeremi Piotrowski Date: Mon, 27 Sep 2021 17:27:27 +0200 Subject: [PATCH 15/16] Fix openssl-3.0 issue involving ASN1 xxx_it Use ASN1_ITEM_rptr() instead of taking the address of IDC_PEID_it. openssl-3.0 changed the type of TYPE_it from `const ASN1_ITEM TYPE_it` to `const ASN1_ITEM *TYPE_it(void)`. This was previously hidden behind OPENSSL_EXPORT_VAR_AS_FUNCTION but in 3.0 only the function version is available. This change should have been transparent to the application, but only if the `ASN1_ITEM_rptr()` macro is used. This change passes `make check` with both openssl 1.1 and 3.0. Signed-off-by: Jeremi Piotrowski Signed-off-by: James Bottomley --- src/idc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/idc.c b/src/idc.c index 6d87bd4..0a82218 100644 --- a/src/idc.c +++ b/src/idc.c @@ -189,7 +189,7 @@ int IDC_set(PKCS7 *p7, PKCS7_SIGNER_INFO *si, struct image *image) idc->data->type = OBJ_nid2obj(peid_nid); idc->data->value = ASN1_TYPE_new(); - type_set_sequence(image, idc->data->value, peid, &IDC_PEID_it); + type_set_sequence(image, idc->data->value, peid, ASN1_ITEM_rptr(IDC_PEID)); idc->digest->alg->parameter = ASN1_TYPE_new(); idc->digest->alg->algorithm = OBJ_nid2obj(NID_sha256); From 9cfca9fe7aa7a8e29b92fe33ce8433e212c9a8ba Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Sun, 19 Mar 2023 17:07:59 -0400 Subject: [PATCH 16/16] Version 0.9.5 Andreas Schwab (1): sbsigntool: add support for RISC-V 64-bit PE/COFF images Daniel Axtens (1): sbvarsign: do not include PKCS#7 attributes James Bottomley (1): Add support for openssl-3 Jeremi Piotrowski (1): Fix openssl-3.0 issue involving ASN1 xxx_it dann frazier (1): sbkeysync: Don't ignore errors from insert_new_keys() Signed-off-by: James Bottomley --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 23c17f4..8a5340a 100644 --- a/configure.ac +++ b/configure.ac @@ -1,4 +1,4 @@ -AC_INIT([sbsigntool], [0.9.4], [James.Bottomley@HansenPartnership.com]) +AC_INIT([sbsigntool], [0.9.5], [James.Bottomley@HansenPartnership.com]) AM_INIT_AUTOMAKE()