Re: Bug#959469: buster-pu: package openssl/1.1.1g-1
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 <sebastian@breakpoint.cc> Wed, 06 Jan 2021 21:04:15 +0100
+ -- Sebastian Andrzej Siewior <sebastian@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.Oheimb@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 <tmraz@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.Oheimb@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 <tmraz@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 to: