On 2021-01-22 16:38:28 [+0000], Adam D. Barratt wrote: > Both would be good, please.
here is the with the two additional patches. Sebastian
diff --git a/debian/changelog b/debian/changelog index 088c914a3dd4a..56a950734f01d 100644 --- a/debian/changelog +++ b/debian/changelog @@ -4,8 +4,9 @@ openssl (1.1.1i-0+deb10u1) buster; urgency=medium - CVE-2019-1551 (Overflow in the x64_64 Montgomery squaring procedure), (Closes: #947949). * Update symbol list. + * Apply two patches from upstream to address x509 related regressions. - -- Sebastian Andrzej Siewior <sebast...@breakpoint.cc> Wed, 06 Jan 2021 21:04:15 +0100 + -- Sebastian Andrzej Siewior <sebast...@breakpoint.cc> Sun, 24 Jan 2021 11:22:16 +0100 openssl (1.1.1d-0+deb10u4) buster-security; urgency=medium diff --git a/debian/patches/X509_cmp-Fix-comparison-in-case-x509v3_cache_extensions-f.patch b/debian/patches/X509_cmp-Fix-comparison-in-case-x509v3_cache_extensions-f.patch new file mode 100644 index 0000000000000..4e6a391da269d --- /dev/null +++ b/debian/patches/X509_cmp-Fix-comparison-in-case-x509v3_cache_extensions-f.patch @@ -0,0 +1,232 @@ +From: "Dr. David von Oheimb" <david.von.ohe...@siemens.com> +Date: Wed, 30 Dec 2020 09:57:49 +0100 +Subject: X509_cmp(): Fix comparison in case x509v3_cache_extensions() failed + to due to invalid cert + +This is the backport of #13755 to v1.1.1. +Fixes #13698 + +Reviewed-by: Tomas Mraz <tm...@fedoraproject.org> +(Merged from https://github.com/openssl/openssl/pull/13756) +--- + crypto/x509/x509_cmp.c | 18 ++++++++++-------- + crypto/x509/x_all.c | 2 +- + crypto/x509v3/v3_purp.c | 3 ++- + doc/man3/X509_get_extension_flags.pod | 9 +++++++-- + include/openssl/x509v3.h | 5 +++-- + test/certs/invalid-cert.pem | 19 +++++++++++++++++++ + test/recipes/80-test_x509aux.t | 13 ++++++++----- + test/x509aux.c | 17 +++++++++++------ + 8 files changed, 61 insertions(+), 25 deletions(-) + create mode 100644 test/certs/invalid-cert.pem + +diff --git a/crypto/x509/x509_cmp.c b/crypto/x509/x509_cmp.c +index ad620af0aff4..c9d89336406f 100644 +--- a/crypto/x509/x509_cmp.c ++++ b/crypto/x509/x509_cmp.c +@@ -133,19 +133,21 @@ unsigned long X509_subject_name_hash_old(X509 *x) + */ + int X509_cmp(const X509 *a, const X509 *b) + { +- int rv; ++ int rv = 0; + + if (a == b) /* for efficiency */ + return 0; +- /* ensure hash is valid */ +- if (X509_check_purpose((X509 *)a, -1, 0) != 1) +- return -2; +- if (X509_check_purpose((X509 *)b, -1, 0) != 1) +- return -2; + +- rv = memcmp(a->sha1_hash, b->sha1_hash, SHA_DIGEST_LENGTH); +- if (rv) ++ /* try to make sure hash is valid */ ++ (void)X509_check_purpose((X509 *)a, -1, 0); ++ (void)X509_check_purpose((X509 *)b, -1, 0); ++ ++ if ((a->ex_flags & EXFLAG_NO_FINGERPRINT) == 0 ++ && (b->ex_flags & EXFLAG_NO_FINGERPRINT) == 0) ++ rv = memcmp(a->sha1_hash, b->sha1_hash, SHA_DIGEST_LENGTH); ++ if (rv != 0) + return rv; ++ + /* Check for match against stored encoding too */ + if (!a->cert_info.enc.modified && !b->cert_info.enc.modified) { + if (a->cert_info.enc.len < b->cert_info.enc.len) +diff --git a/crypto/x509/x_all.c b/crypto/x509/x_all.c +index aa5ccba44899..bec850af5797 100644 +--- a/crypto/x509/x_all.c ++++ b/crypto/x509/x_all.c +@@ -363,7 +363,7 @@ int X509_digest(const X509 *data, const EVP_MD *type, unsigned char *md, + unsigned int *len) + { + if (type == EVP_sha1() && (data->ex_flags & EXFLAG_SET) != 0 +- && (data->ex_flags & EXFLAG_INVALID) == 0) { ++ && (data->ex_flags & EXFLAG_NO_FINGERPRINT) == 0) { + /* Asking for SHA1 and we already computed it. */ + if (len != NULL) + *len = sizeof(data->sha1_hash); +diff --git a/crypto/x509v3/v3_purp.c b/crypto/x509v3/v3_purp.c +index 2b06dba05398..93b5ca4d4283 100644 +--- a/crypto/x509v3/v3_purp.c ++++ b/crypto/x509v3/v3_purp.c +@@ -391,7 +391,8 @@ static void x509v3_cache_extensions(X509 *x) + } + + if (!X509_digest(x, EVP_sha1(), x->sha1_hash, NULL)) +- x->ex_flags |= EXFLAG_INVALID; ++ x->ex_flags |= (EXFLAG_NO_FINGERPRINT | EXFLAG_INVALID); ++ + /* V1 should mean no extensions ... */ + if (!X509_get_version(x)) + x->ex_flags |= EXFLAG_V1; +diff --git a/doc/man3/X509_get_extension_flags.pod b/doc/man3/X509_get_extension_flags.pod +index 43c9c952c6b7..cca72c71fcab 100644 +--- a/doc/man3/X509_get_extension_flags.pod ++++ b/doc/man3/X509_get_extension_flags.pod +@@ -78,12 +78,17 @@ The certificate contains an unhandled critical extension. + + =item B<EXFLAG_INVALID> + +-Some certificate extension values are invalid or inconsistent. The +-certificate should be rejected. ++Some certificate extension values are invalid or inconsistent. ++The certificate should be rejected. + This bit may also be raised after an out-of-memory error while + processing the X509 object, so it may not be related to the processed + ASN1 object itself. + ++=item B<EXFLAG_NO_FINGERPRINT> ++ ++Failed to compute the internal SHA1 hash value of the certificate. ++This may be due to malloc failure or because no SHA1 implementation was found. ++ + =item B<EXFLAG_INVALID_POLICY> + + The NID_certificate_policies certificate extension is invalid or +diff --git a/include/openssl/x509v3.h b/include/openssl/x509v3.h +index 6c6eca38a582..b9a8943273fb 100644 +--- a/include/openssl/x509v3.h ++++ b/include/openssl/x509v3.h +@@ -364,8 +364,9 @@ struct ISSUING_DIST_POINT_st { + + # define EXFLAG_INVALID_POLICY 0x800 + # define EXFLAG_FRESHEST 0x1000 +-/* Self signed */ +-# define EXFLAG_SS 0x2000 ++# define EXFLAG_SS 0x2000 /* cert is apparently self-signed */ ++ ++# define EXFLAG_NO_FINGERPRINT 0x100000 + + # define KU_DIGITAL_SIGNATURE 0x0080 + # define KU_NON_REPUDIATION 0x0040 +diff --git a/test/certs/invalid-cert.pem b/test/certs/invalid-cert.pem +new file mode 100644 +index 000000000000..a8951305a3dc +--- /dev/null ++++ b/test/certs/invalid-cert.pem +@@ -0,0 +1,19 @@ ++-----BEGIN TRUSTED CERTIFICATE----- ++MIIDJTCCAg2gAwIBAgIUEUSW5o7qpgNCWyXic9Fc9tCLS0gwDQYJKoZIhvcNAQEL ++BQAwEzERMA8GA1UEAwwIUGVyc29TaW0wHhcNMjAxMjE2MDY1NjM5WhcNMzAxMjE2 ++MDY1NjM5WjATMREwDwYDVQQDDAhQZXJzb1NpbTCCASIwDQYJKoZIhvcNAQEBBQAD ++ggEPADCCAQoCggEBAMsgRKnnZbQtG9bB9Hn+CoOOsanmnRELSlGq521qi/eBgs2w ++SdHYM6rsJFwY89RvINLGeUZh/pu7c+ODtTafAWE3JkynG01d2Zrvp1V1r97+FGyD ++f+b1hAggxBy70bTRyr1gAoKQTAm74U/1lj13EpWz7zshgXJ/Pn/hUyTmpNW+fTRE ++xaifN0jkl5tZUURGA6w3+BRhVDQtt92vLihqUGaEFpL8yqqFnN44AoQ5+lgMafWi ++UyYMHcK75ZB8WWklq8zjRP3xC1h56k01rT6KJO6i+BxMcADerYsn5qTlcUiKcpRU ++b6RzLvCUwj91t1aX6npDI3BzSP+wBUUANBfuHEMCAwEAAaNxMG8wFwYDVR0OBBA8 ++yBBnvz1Zt6pHm2GwBaRyMBcGA1UdIwQQPMgQZ789WbeqR5thsAWkcjAPBgNVHRMB ++Af8EBTADAQH/MAsGA1UdDwQEAwIChDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYB ++BQUHAwIwDQYJKoZIhvcNAQELBQADggEBAIEzVbttOUc7kK4aY+74TANFZK/qtBQ7 ++94a/P30TGWSRUq2HnDsR8Vo4z8xm5oKeC+SIi6NGzviWYquuzpJ7idcbr0MIuSyD +++Vg6n1sG64DxWNdGO9lR5c4mWFdIajShczS2+4QIRB/lFZCf7GhPMtIcbP1o9ckY ++2vyv5ZAEU9Z5n0PY+abrKsj0XyvJwdycEsUTywa36fuv6hP3UboLtvK6naXLMrTj ++WtSA6PXjHy7h8h0NC8XLk64mc0lcRC4WM+xJ/C+NHglpmBqBxnStpnZykMZYD1Vy ++JJ1wNc+Y3e2uMBDxZviH3dIPIgqP1Vpi2TWfqr3DTBNCRf4dl/wwNU8= ++-----END TRUSTED CERTIFICATE----- +diff --git a/test/recipes/80-test_x509aux.t b/test/recipes/80-test_x509aux.t +index 65ba5fcf5292..30adf252570a 100644 +--- a/test/recipes/80-test_x509aux.t ++++ b/test/recipes/80-test_x509aux.t +@@ -14,14 +14,17 @@ use OpenSSL::Test::Utils; + + setup("test_x509aux"); + ++my @path = qw(test certs); ++ + plan skip_all => "test_dane uses ec which is not supported by this OpenSSL build" + if disabled("ec"); + + plan tests => 1; # The number of tests being performed + + ok(run(test(["x509aux", +- srctop_file("test", "certs", "roots.pem"), +- srctop_file("test", "certs", "root+anyEKU.pem"), +- srctop_file("test", "certs", "root-anyEKU.pem"), +- srctop_file("test", "certs", "root-cert.pem")] +- )), "x509aux tests"); ++ srctop_file(@path, "roots.pem"), ++ srctop_file(@path, "root+anyEKU.pem"), ++ srctop_file(@path, "root-anyEKU.pem"), ++ srctop_file(@path, "root-cert.pem"), ++ srctop_file(@path, "invalid-cert.pem"), ++ ])), "x509aux tests"); +diff --git a/test/x509aux.c b/test/x509aux.c +index e41f1f6809d2..78013f23ae27 100644 +--- a/test/x509aux.c ++++ b/test/x509aux.c +@@ -30,17 +30,16 @@ static int test_certs(int num) + typedef int (*i2d_X509_t)(X509 *, unsigned char **); + int err = 0; + BIO *fp = BIO_new_file(test_get_argument(num), "r"); +- X509 *reuse = NULL; + + if (!TEST_ptr(fp)) + return 0; + + for (c = 0; !err && PEM_read_bio(fp, &name, &header, &data, &len); ++c) { + const int trusted = (strcmp(name, PEM_STRING_X509_TRUSTED) == 0); +- + d2i_X509_t d2i = trusted ? d2i_X509_AUX : d2i_X509; + i2d_X509_t i2d = trusted ? i2d_X509_AUX : i2d_X509; + X509 *cert = NULL; ++ X509 *reuse = NULL; + const unsigned char *p = data; + unsigned char *buf = NULL; + unsigned char *bufp; +@@ -93,9 +92,15 @@ static int test_certs(int num) + goto next; + } + p = buf; +- reuse = d2i(&reuse, &p, enclen); +- if (reuse == NULL || X509_cmp (reuse, cert)) { +- TEST_error("X509_cmp does not work with %s", name); ++ reuse = d2i(NULL, &p, enclen); ++ if (reuse == NULL) { ++ TEST_error("second d2i call failed for %s", name); ++ err = 1; ++ goto next; ++ } ++ err = X509_cmp(reuse, cert); ++ if (err != 0) { ++ TEST_error("X509_cmp for %s resulted in %d", name, err); + err = 1; + goto next; + } +@@ -141,13 +146,13 @@ static int test_certs(int num) + */ + next: + X509_free(cert); ++ X509_free(reuse); + OPENSSL_free(buf); + OPENSSL_free(name); + OPENSSL_free(header); + OPENSSL_free(data); + } + BIO_free(fp); +- X509_free(reuse); + + if (ERR_GET_REASON(ERR_peek_last_error()) == PEM_R_NO_START_LINE) { + /* Reached end of PEM file */ diff --git a/debian/patches/series b/debian/patches/series index 54001c0d7f228..8aa553ea9acd1 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -4,3 +4,5 @@ no-symbolic.patch pic.patch c_rehash-compat.patch Set-systemwide-default-settings-for-libssl-users.patch +x509_vfy.c-Fix-a-regression-in-find_isser.patch +X509_cmp-Fix-comparison-in-case-x509v3_cache_extensions-f.patch diff --git a/debian/patches/x509_vfy.c-Fix-a-regression-in-find_isser.patch b/debian/patches/x509_vfy.c-Fix-a-regression-in-find_isser.patch new file mode 100644 index 0000000000000..b6c79355e1bdb --- /dev/null +++ b/debian/patches/x509_vfy.c-Fix-a-regression-in-find_isser.patch @@ -0,0 +1,144 @@ +From: "Dr. David von Oheimb" <david.von.ohe...@siemens.com> +Date: Mon, 28 Dec 2020 11:25:59 +0100 +Subject: x509_vfy.c: Fix a regression in find_isser() + +...in case the candidate issuer cert is identical to the target cert. + +Fixes #13739 + +Reviewed-by: Tomas Mraz <tm...@fedoraproject.org> +(Merged from https://github.com/openssl/openssl/pull/13749) +--- + crypto/x509/x509_vfy.c | 13 ++++----- + test/recipes/70-test_verify_extra.t | 3 ++- + test/verify_extra_test.c | 53 ++++++++++++++++++++++++++++++++++--- + 3 files changed, 57 insertions(+), 12 deletions(-) + +diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c +index 730a0160ff0a..883c6d7118ac 100644 +--- a/crypto/x509/x509_vfy.c ++++ b/crypto/x509/x509_vfy.c +@@ -323,9 +323,10 @@ static int sk_X509_contains(STACK_OF(X509) *sk, X509 *cert) + } + + /* +- * Find in given STACK_OF(X509) sk a non-expired issuer cert (if any) of given cert x. +- * The issuer must not be the same as x and must not yet be in ctx->chain, where the +- * exceptional case x is self-issued and ctx->chain has just one element is allowed. ++ * Find in given STACK_OF(X509) sk an issuer cert of given cert x. ++ * The issuer must not yet be in ctx->chain, where the exceptional case ++ * that x is self-issued and ctx->chain has just one element is allowed. ++ * Prefer the first one that is not expired, else take the last expired one. + */ + static X509 *find_issuer(X509_STORE_CTX *ctx, STACK_OF(X509) *sk, X509 *x) + { +@@ -334,11 +335,7 @@ static X509 *find_issuer(X509_STORE_CTX *ctx, STACK_OF(X509) *sk, X509 *x) + + for (i = 0; i < sk_X509_num(sk); i++) { + issuer = sk_X509_value(sk, i); +- /* +- * Below check 'issuer != x' is an optimization and safety precaution: +- * Candidate issuer cert cannot be the same as the subject cert 'x'. +- */ +- if (issuer != x && ctx->check_issued(ctx, x, issuer) ++ if (ctx->check_issued(ctx, x, issuer) + && (((x->ex_flags & EXFLAG_SI) != 0 && sk_X509_num(ctx->chain) == 1) + || !sk_X509_contains(ctx->chain, issuer))) { + rv = issuer; +diff --git a/test/recipes/70-test_verify_extra.t b/test/recipes/70-test_verify_extra.t +index 79a33cd01679..e3bdcbaaf9f0 100644 +--- a/test/recipes/70-test_verify_extra.t ++++ b/test/recipes/70-test_verify_extra.t +@@ -16,4 +16,5 @@ plan tests => 1; + ok(run(test(["verify_extra_test", + srctop_file("test", "certs", "roots.pem"), + srctop_file("test", "certs", "untrusted.pem"), +- srctop_file("test", "certs", "bad.pem")]))); ++ srctop_file("test", "certs", "bad.pem"), ++ srctop_file("test", "certs", "rootCA.pem")]))); +diff --git a/test/verify_extra_test.c b/test/verify_extra_test.c +index d9d1498954b1..94faa4c78b31 100644 +--- a/test/verify_extra_test.c ++++ b/test/verify_extra_test.c +@@ -18,6 +18,21 @@ + static const char *roots_f; + static const char *untrusted_f; + static const char *bad_f; ++static const char *good_f; ++ ++static X509 *load_cert_pem(const char *file) ++{ ++ X509 *cert = NULL; ++ BIO *bio = NULL; ++ ++ if (!TEST_ptr(bio = BIO_new(BIO_s_file()))) ++ return NULL; ++ if (TEST_int_gt(BIO_read_filename(bio, file), 0)) ++ (void)TEST_ptr(cert = PEM_read_bio_X509(bio, NULL, NULL, NULL)); ++ ++ BIO_free(bio); ++ return cert; ++} + + static STACK_OF(X509) *load_certs_from_file(const char *filename) + { +@@ -58,7 +73,7 @@ static STACK_OF(X509) *load_certs_from_file(const char *filename) + return certs; + } + +-/* ++/*- + * Test for CVE-2015-1793 (Alternate Chains Certificate Forgery) + * + * Chain is as follows: +@@ -175,16 +190,48 @@ static int test_store_ctx(void) + return testresult; + } + ++static int test_self_signed(const char *filename, int expected) ++{ ++ X509 *cert = load_cert_pem(filename); ++ STACK_OF(X509) *trusted = sk_X509_new_null(); ++ X509_STORE_CTX *ctx = X509_STORE_CTX_new(); ++ int ret; ++ ++ ret = TEST_ptr(cert) ++ && TEST_true(sk_X509_push(trusted, cert)) ++ && TEST_true(X509_STORE_CTX_init(ctx, NULL, cert, NULL)); ++ X509_STORE_CTX_trusted_stack(ctx, trusted); ++ ret = ret && TEST_int_eq(X509_verify_cert(ctx), expected); ++ ++ X509_STORE_CTX_free(ctx); ++ sk_X509_free(trusted); ++ X509_free(cert); ++ return ret; ++} ++ ++static int test_self_signed_good(void) ++{ ++ return test_self_signed(good_f, 1); ++} ++ ++static int test_self_signed_bad(void) ++{ ++ return test_self_signed(bad_f, 0); ++} ++ + int setup_tests(void) + { + if (!TEST_ptr(roots_f = test_get_argument(0)) + || !TEST_ptr(untrusted_f = test_get_argument(1)) +- || !TEST_ptr(bad_f = test_get_argument(2))) { +- TEST_error("usage: verify_extra_test roots.pem untrusted.pem bad.pem\n"); ++ || !TEST_ptr(bad_f = test_get_argument(2)) ++ || !TEST_ptr(good_f = test_get_argument(3))) { ++ TEST_error("usage: verify_extra_test roots.pem untrusted.pem bad.pem good.pem\n"); + return 0; + } + + ADD_TEST(test_alt_chains_cert_forgery); + ADD_TEST(test_store_ctx); ++ ADD_TEST(test_self_signed_good); ++ ADD_TEST(test_self_signed_bad); + return 1; + }