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;
+ }

Reply via email to