Package: release.debian.org Severity: normal Tags: buster User: release.debian....@packages.debian.org Usertags: pu X-Debbugs-Cc: ex...@packages.debian.org, Jorrit Fahlke <jor...@jorrit.de>
Hello, [ Reason ] I would like to fix two issues in buster: #1 Fix use of concurrent TLS connections under GnuTLS. When a callout was done during a receiving connection, and both used TLS, global info was used rather than per-connection info for tracking the state of data queued for transmission. This could result in a connection hang. #2 Fix issues related to certificate checking: a) Cherry-pick a bugfix to get proper hostname checking with CNAMES. Without this patch when connecting to a CNAME the server provided cert is checked against the A record instead of the original cname. #985243 b) Document limitation/extent of server certificate checking that is done by default and how to change it. #985244 and #985344 2a and 2b are documented in the respective bug-reports, 2a actually might warant priority serious. #1 has popped up repeatedly on the exim-user mailing list, I would categorize it as important. cu Andreas -- `What a good friend you are to him, Dr. Maturin. His other friends are so grateful to you.' `I sew his ears on from time to time, sure'
diff -Nru exim4-4.92/debian/changelog exim4-4.92/debian/changelog --- exim4-4.92/debian/changelog 2020-05-13 18:01:31.000000000 +0200 +++ exim4-4.92/debian/changelog 2021-03-18 09:10:15.000000000 +0100 @@ -1,3 +1,23 @@ +exim4 (4.92-8+deb10u5) buster; urgency=medium + + * Fix use of concurrent TLS connections under GnuTLS: + 80_01-GnuTLS-fix-hanging-callout-connections.patch + 80_02-GnuTLS-tls_write-wait-after-uncorking-the-session.patch + 80_03-GnuTLS-Do-not-care-about-corked-data-when-uncorking.patch + (Thanks, Heiko Schlittermann for the backport) + * Pull 82_TLS-use-RFC-6125-rules-for-certifucate-name-checks-w.patch from + upstream git (already included in 4.94), on TLS connections to a CNAME + verify the certificate against the original CNAME instead of against + the A record. Closes: #985243 + * In README.Debian explicitly document the limitation/extent of server + certificate checking (authenticity not enforced) in the default + configuration (Thanks, Jö Fahlke). This Closes: #985244 (improved + documentation and Closes: #985344 (Yes, without required cert + checking MitM attacks are possible, but for a stable update documenting + this is the best compromise.) + + -- Andreas Metzler <ametz...@debian.org> Thu, 18 Mar 2021 09:10:15 +0100 + exim4 (4.92-8+deb10u4) buster-security; urgency=high * Fix authentication bypass in SPA authenticator due to out-of-bound buffer diff -Nru exim4-4.92/debian/patches/80_01-GnuTLS-fix-hanging-callout-connections.patch exim4-4.92/debian/patches/80_01-GnuTLS-fix-hanging-callout-connections.patch --- exim4-4.92/debian/patches/80_01-GnuTLS-fix-hanging-callout-connections.patch 1970-01-01 01:00:00.000000000 +0100 +++ exim4-4.92/debian/patches/80_01-GnuTLS-fix-hanging-callout-connections.patch 2021-03-18 08:51:35.000000000 +0100 @@ -0,0 +1,83 @@ +From 97c5e07c220b55d1c506a1798c9ce3ae3105adea Mon Sep 17 00:00:00 2001 +From: Jeremy Harris <jgh146...@wizmail.org> +Date: Thu, 13 Feb 2020 16:45:38 +0000 +Subject: [PATCH 4/6] GnuTLS: fix hanging callout connections + +Broken-by: 925ac8e4f1 +(cherry picked from commit bd95ffc2ba87fbd3c752df17bc8fd9c01586d45a) +--- + doc/ChangeLog | 81 ++++--------------------------------------- + src/tls-gnu.c | 24 +++++++------ + 2 files changed, 20 insertions(+), 85 deletions(-) + +--- a/doc/ChangeLog ++++ b/doc/ChangeLog +@@ -67,6 +67,11 @@ JH/41 Bug 2571: Fix SPA authenticator. + being used. A malicious client could thus cause an out-of-bounds read and + possibly gain authentication. Fix by adding the check. + ++JH/25 Fix use of concurrent TLS connections under GnuTLS. When a callout was ++ done during a receiving connection, and both used TLS, global info was ++ used rather than per-connection info for tracking the state of data ++ queued for transmission. This could result in a connection hang. ++ + + Exim version 4.92 + ----------------- +--- a/src/tls-gnu.c ++++ b/src/tls-gnu.c +@@ -124,10 +124,17 @@ typedef struct exim_gnutls_state { + enum peer_verify_requirement verify_requirement; + int fd_in; + int fd_out; +- BOOL peer_cert_verified; +- BOOL peer_dane_verified; +- BOOL trigger_sni_changes; +- BOOL have_set_peerdn; ++ ++ BOOL peer_cert_verified:1; ++ BOOL peer_dane_verified:1; ++ BOOL trigger_sni_changes:1; ++ BOOL have_set_peerdn:1; ++ BOOL xfer_eof:1; /*XXX never gets set! */ ++ BOOL xfer_error:1; ++#ifdef SUPPORT_CORK ++ BOOL corked:1; ++#endif ++ + const struct host_item *host; /* NULL if server */ + gnutls_x509_crt_t peercert; + uschar *peerdn; +@@ -160,8 +167,6 @@ typedef struct exim_gnutls_state { + uschar *xfer_buffer; + int xfer_buffer_lwm; + int xfer_buffer_hwm; +- BOOL xfer_eof; /*XXX never gets set! */ +- BOOL xfer_error; + } exim_gnutls_state_st; + + static const exim_gnutls_state_st exim_gnutls_state_init = { +@@ -2790,9 +2795,8 @@ ssize_t outbytes; + size_t left = len; + exim_gnutls_state_st * state = ct_ctx ? ct_ctx : &state_server; + #ifdef SUPPORT_CORK +-static BOOL corked = FALSE; + +-if (more && !corked) gnutls_record_cork(state->session); ++if (more && !state->corked) gnutls_record_cork(state->session); + #endif + + DEBUG(D_tls) debug_printf("%s(%p, " SIZE_T_FMT "%s)\n", __FUNCTION__, +@@ -2833,10 +2837,10 @@ if (len > INT_MAX) + } + + #ifdef SUPPORT_CORK +-if (more != corked) ++if (more != state->corked) + { + if (!more) (void) gnutls_record_uncork(state->session, 0); +- corked = more; ++ state->corked = more; + } + #endif + diff -Nru exim4-4.92/debian/patches/80_02-GnuTLS-tls_write-wait-after-uncorking-the-session.patch exim4-4.92/debian/patches/80_02-GnuTLS-tls_write-wait-after-uncorking-the-session.patch --- exim4-4.92/debian/patches/80_02-GnuTLS-tls_write-wait-after-uncorking-the-session.patch 1970-01-01 01:00:00.000000000 +0100 +++ exim4-4.92/debian/patches/80_02-GnuTLS-tls_write-wait-after-uncorking-the-session.patch 2021-03-18 08:51:35.000000000 +0100 @@ -0,0 +1,73 @@ +From 783cb0301d9ceef2748956c3f91762275b7b45e5 Mon Sep 17 00:00:00 2001 +From: "Heiko Schlittermann (HS12-RIPE)" <h...@schlittermann.de> +Date: Tue, 18 Feb 2020 18:59:49 +0100 +Subject: [PATCH 5/6] GnuTLS: tls_write(): wait after uncorking the session + +(cherry picked from commit 8f9adfd36222d4e9e730734e00dffe874073e5b4) +--- + src/tls-gnu.c | 34 ++++++++++++++++++++++++++++------ + 1 file changed, 28 insertions(+), 6 deletions(-) + +diff --git a/src/tls-gnu.c b/src/tls-gnu.c +index 822ad89c6..94a718673 100644 +--- a/src/tls-gnu.c ++++ b/src/tls-gnu.c +@@ -2835,9 +2835,14 @@ tls_write(void * ct_ctx, const uschar * buff, size_t len, BOOL more) + ssize_t outbytes; + size_t left = len; + exim_gnutls_state_st * state = ct_ctx ? ct_ctx : &state_server; +-#ifdef SUPPORT_CORK + +-if (more && !state->corked) gnutls_record_cork(state->session); ++#ifdef SUPPORT_CORK ++if (more && !state->corked) ++ { ++ DEBUG(D_tls) debug_printf("gnutls_record_cork(session=%p)\n", state->session); ++ gnutls_record_cork(state->session); ++ state->corked = TRUE; ++ } + #endif + + DEBUG(D_tls) debug_printf("%s(%p, " SIZE_T_FMT "%s)\n", __FUNCTION__, +@@ -2853,6 +2858,7 @@ while (left > 0) + while (outbytes == GNUTLS_E_AGAIN); + + DEBUG(D_tls) debug_printf("outbytes=" SSIZE_T_FMT "\n", outbytes); ++ + if (outbytes < 0) + { + DEBUG(D_tls) debug_printf("%s: gnutls_record_send err\n", __FUNCTION__); +@@ -2878,10 +2884,26 @@ if (len > INT_MAX) + } + + #ifdef SUPPORT_CORK +-if (more != state->corked) +- { +- if (!more) (void) gnutls_record_uncork(state->session, 0); +- state->corked = more; ++if (!more && state->corked) ++ { ++ DEBUG(D_tls) debug_printf("gnutls_record_uncork(session=%p)\n", state->session); ++ do { ++ do ++ /* We can't use GNUTLS_RECORD_WAIT here, as it retries on ++ GNUTLS_E_AGAIN || GNUTLS_E_INTR, which would break our timeout set by alarm(). ++ The GNUTLS_E_AGAIN should not happen ever, as our sockets are blocking anyway. ++ But who knows. (That all relies on the fact that GNUTLS_E_INTR and GNUTLS_E_AGAIN ++ match the EINTR and EAGAIN errno values.) */ ++ outbytes = gnutls_record_uncork(state->session, 0); ++ while (outbytes == GNUTLS_E_AGAIN); ++ ++ if (outbytes < 0) ++ { ++ record_io_error(state, len, US"uncork", NULL); ++ return -1; ++ } ++ } while (gnutls_record_check_corked(state->session) > 0); ++ state->corked = FALSE; + } + #endif + +-- +2.28.0 + diff -Nru exim4-4.92/debian/patches/80_03-GnuTLS-Do-not-care-about-corked-data-when-uncorking.patch exim4-4.92/debian/patches/80_03-GnuTLS-Do-not-care-about-corked-data-when-uncorking.patch --- exim4-4.92/debian/patches/80_03-GnuTLS-Do-not-care-about-corked-data-when-uncorking.patch 1970-01-01 01:00:00.000000000 +0100 +++ exim4-4.92/debian/patches/80_03-GnuTLS-Do-not-care-about-corked-data-when-uncorking.patch 2021-03-18 08:51:35.000000000 +0100 @@ -0,0 +1,55 @@ +From 3afb07f2c63fb6dc3983b28e7cdaf11fceb741d1 Mon Sep 17 00:00:00 2001 +From: "Heiko Schlittermann (HS12-RIPE)" <h...@schlittermann.de> +Date: Mon, 2 Mar 2020 22:56:32 +0100 +Subject: [PATCH 6/6] GnuTLS: Do not care about corked data when uncorking + +(cherry picked from commit d8d7e3a4162b52382daf8319f221c085c76c5b8f) +--- + src/tls-gnu.c | 31 +++++++++++++++---------------- + 1 file changed, 15 insertions(+), 16 deletions(-) + +diff --git a/src/tls-gnu.c b/src/tls-gnu.c +index 94a718673..2091e44db 100644 +--- a/src/tls-gnu.c ++++ b/src/tls-gnu.c +@@ -2887,22 +2887,21 @@ if (len > INT_MAX) + if (!more && state->corked) + { + DEBUG(D_tls) debug_printf("gnutls_record_uncork(session=%p)\n", state->session); +- do { +- do +- /* We can't use GNUTLS_RECORD_WAIT here, as it retries on +- GNUTLS_E_AGAIN || GNUTLS_E_INTR, which would break our timeout set by alarm(). +- The GNUTLS_E_AGAIN should not happen ever, as our sockets are blocking anyway. +- But who knows. (That all relies on the fact that GNUTLS_E_INTR and GNUTLS_E_AGAIN +- match the EINTR and EAGAIN errno values.) */ +- outbytes = gnutls_record_uncork(state->session, 0); +- while (outbytes == GNUTLS_E_AGAIN); +- +- if (outbytes < 0) +- { +- record_io_error(state, len, US"uncork", NULL); +- return -1; +- } +- } while (gnutls_record_check_corked(state->session) > 0); ++ do ++ /* We can't use GNUTLS_RECORD_WAIT here, as it retries on ++ GNUTLS_E_AGAIN || GNUTLS_E_INTR, which would break our timeout set by alarm(). ++ The GNUTLS_E_AGAIN should not happen ever, as our sockets are blocking anyway. ++ But who knows. (That all relies on the fact that GNUTLS_E_INTR and GNUTLS_E_AGAIN ++ match the EINTR and EAGAIN errno values.) */ ++ outbytes = gnutls_record_uncork(state->session, 0); ++ while (outbytes == GNUTLS_E_AGAIN); ++ ++ if (outbytes < 0) ++ { ++ record_io_error(state, len, US"uncork", NULL); ++ return -1; ++ } ++ + state->corked = FALSE; + } + #endif +-- +2.28.0 + diff -Nru exim4-4.92/debian/patches/82_TLS-use-RFC-6125-rules-for-certifucate-name-checks-w.patch exim4-4.92/debian/patches/82_TLS-use-RFC-6125-rules-for-certifucate-name-checks-w.patch --- exim4-4.92/debian/patches/82_TLS-use-RFC-6125-rules-for-certifucate-name-checks-w.patch 1970-01-01 01:00:00.000000000 +0100 +++ exim4-4.92/debian/patches/82_TLS-use-RFC-6125-rules-for-certifucate-name-checks-w.patch 2021-03-18 08:51:35.000000000 +0100 @@ -0,0 +1,188 @@ +Description: TLS: use RFC 6125 rules for certificate name checks when + CNAMES are present. Bug 2594 +Origin: upstream https://git.exim.org/exim.git/commit/0851a3bbf4667081d47f5d85b6b3a5cb33cbdba6 +Bug: https://bugs.exim.org/show_bug.cgi?id=2594 +Forwarded: not-needed +Last-Update: 2021-03-02 + +--- a/doc/ChangeLog ++++ b/doc/ChangeLog +@@ -41,10 +41,15 @@ JH/10 OpenSSL: Fix aggregation of messag + + JH/11 Harden plaintext authenticator against a badly misconfigured client-send + string. Previously it was possible to cause undefined behaviour in a + library routine (usually a crash). Found by "zerons". + ++JH/06 Bug 2594: Change the name used for certificate name checks in the smtp ++ transport. Previously it was the name on the DNS A-record; use instead ++ the head of the CNAME chain leading there (if there is one). This seems ++ to align better with RFC 6125. ++ + + JH/18 GnuTLS: fix $tls_out_ocsp under hosts_request_ocsp. Previously the + verification result was not updated unless hosts_require_ocsp applied. + + JH/20 Bug 2389: fix server advertising of usable certificates, under GnuTLS in +--- a/src/host.c ++++ b/src/host.c +@@ -1966,10 +1966,17 @@ host_item *last = NULL; + BOOL temp_error = FALSE; + #if HAVE_IPV6 + int af; + #endif + ++#ifndef DISABLE_TLS ++/* Copy the host name at this point to the value which is used for ++TLS certificate name checking, before anything modifies it. */ ++ ++host->certname = host->name; ++#endif ++ + /* Make sure DNS options are set as required. This appears to be necessary in + some circumstances when the get..byname() function actually calls the DNS. */ + + dns_init((flags & HOST_FIND_QUALIFY_SINGLE) != 0, + (flags & HOST_FIND_SEARCH_PARENTS) != 0, +@@ -2132,10 +2139,13 @@ for (i = 1; i <= times; + + else + { + host_item *next = store_get(sizeof(host_item)); + next->name = host->name; ++#ifndef DISABLE_TLS ++ next->certname = host->certname; ++#endif + next->mx = host->mx; + next->address = text_address; + next->port = PORT_NONE; + next->status = hstatus_unknown; + next->why = hwhy_unknown; +@@ -2150,16 +2160,16 @@ for (i = 1; i <= times; + + /* If no hosts were found, the address field in the original host block will be + NULL. If temp_error is set, at least one of the lookups gave a temporary error, + so we pass that back. */ + +-if (host->address == NULL) ++if (!host->address) + { + uschar *msg = + #ifndef STAND_ALONE +- (message_id[0] == 0 && smtp_in != NULL)? +- string_sprintf("no IP address found for host %s (during %s)", host->name, ++ message_id[0] == 0 && smtp_in ++ ? string_sprintf("no IP address found for host %s (during %s)", host->name, + smtp_get_connection_info()) : + #endif + string_sprintf("no IP address found for host %s", host->name); + + HDEBUG(D_host_lookup) debug_printf("%s\n", msg); +@@ -2277,10 +2287,17 @@ dns_record *rr; + host_item *thishostlast = NULL; /* Indicates not yet filled in anything */ + BOOL v6_find_again = FALSE; + BOOL dnssec_fail = FALSE; + int i; + ++#ifndef DISABLE_TLS ++/* Copy the host name at this point to the value which is used for ++TLS certificate name checking, before any CNAME-following modifies it. */ ++ ++host->certname = host->name; ++#endif ++ + /* If allow_ip is set, a name which is an IP address returns that value + as its address. This is used for MX records when allow_mx_to_ip is set, for + those sites that feel they have to flaunt the RFC rules. */ + + if (allow_ip && string_is_ip_address(host->name, NULL) != 0) +--- a/src/structs.h ++++ b/src/structs.h +@@ -77,18 +77,21 @@ host addresses is done using this struct + + typedef enum {DS_UNK=-1, DS_NO, DS_YES} dnssec_status_t; + + typedef struct host_item { + struct host_item *next; +- const uschar *name; /* Host name */ +- const uschar *address; /* IP address in text form */ +- int port; /* port value in host order (if SRV lookup) */ +- int mx; /* MX value if found via MX records */ +- int sort_key; /* MX*1000 plus random "fraction" */ +- int status; /* Usable, unusable, or unknown */ +- int why; /* Why host is unusable */ +- int last_try; /* Time of last try if known */ ++ const uschar *name; /* Host name */ ++#ifndef DISABLE_TLS ++ const uschar *certname; /* Name used for certificate checks */ ++#endif ++ const uschar *address; /* IP address in text form */ ++ int port; /* port value in host order (if SRV lookup) */ ++ int mx; /* MX value if found via MX records */ ++ int sort_key; /* MX*1000 plus random "fraction" */ ++ int status; /* Usable, unusable, or unknown */ ++ int why; /* Why host is unusable */ ++ int last_try; /* Time of last try if known */ + dnssec_status_t dnssec; + } host_item; + + /* Chain of rewrite rules, read from the rewrite config, or parsed from the + rewrite_headers field of a transport. */ +--- a/src/tls-gnu.c ++++ b/src/tls-gnu.c +@@ -2191,13 +2191,13 @@ tls_client_setup_hostname_checks(host_it + { + if (verify_check_given_host(CUSS &ob->tls_verify_cert_hostnames, host) == OK) + { + state->exp_tls_verify_cert_hostnames = + #ifdef SUPPORT_I18N +- string_domain_utf8_to_alabel(host->name, NULL); ++ string_domain_utf8_to_alabel(host->certname, NULL); + #else +- host->name; ++ host->certname; + #endif + DEBUG(D_tls) + debug_printf("TLS: server cert verification includes hostname: \"%s\".\n", + state->exp_tls_verify_cert_hostnames); + } +--- a/src/tls-openssl.c ++++ b/src/tls-openssl.c +@@ -309,18 +309,18 @@ typedef struct tls_ext_ctx_cb { + X509_STORE *verify_store; /* non-null if status requested */ + BOOL verify_required; + } client; + } u_ocsp; + #endif +- uschar *dhparam; ++ uschar * dhparam; + /* these are cached from first expand */ +- uschar *server_cipher_list; ++ uschar * server_cipher_list; + /* only passed down to tls_error: */ +- host_item *host; ++ host_item * host; + const uschar * verify_cert_hostnames; + #ifndef DISABLE_EVENT +- uschar * event_action; ++ uschar * event_action; + #endif + } tls_ext_ctx_cb; + + /* should figure out a cleanup of API to handle state preserved per + implementation, for various reasons, which can be void * in the APIs. +@@ -2359,13 +2359,13 @@ if ((rc = setup_certs(ctx, ob->tls_verif + + if (verify_check_given_host(CUSS &ob->tls_verify_cert_hostnames, host) == OK) + { + cbinfo->verify_cert_hostnames = + #ifdef SUPPORT_I18N +- string_domain_utf8_to_alabel(host->name, NULL); ++ string_domain_utf8_to_alabel(host->certname, NULL); + #else +- host->name; ++ host->certname; + #endif + DEBUG(D_tls) debug_printf("Cert hostname to check: \"%s\"\n", + cbinfo->verify_cert_hostnames); + } + return OK; diff -Nru exim4-4.92/debian/patches/series exim4-4.92/debian/patches/series --- exim4-4.92/debian/patches/series 2020-05-13 18:01:31.000000000 +0200 +++ exim4-4.92/debian/patches/series 2021-03-18 08:51:35.000000000 +0100 @@ -26,4 +26,8 @@ 78_02-Fix-buffer-overflow-in-string_vformat.-Bug-2449.patch 79_01-Fix-SPA-authenticator-checking-client-supplied-data-.patch 79_02-Rework-SPA-fix-to-avoid-overflows.-Bug-2571.patch +80_01-GnuTLS-fix-hanging-callout-connections.patch +80_02-GnuTLS-tls_write-wait-after-uncorking-the-session.patch +80_03-GnuTLS-Do-not-care-about-corked-data-when-uncorking.patch +82_TLS-use-RFC-6125-rules-for-certifucate-name-checks-w.patch 90_localscan_dlopen.dpatch diff -Nru exim4-4.92/debian/README.Debian.xml exim4-4.92/debian/README.Debian.xml --- exim4-4.92/debian/README.Debian.xml 2020-05-13 18:01:31.000000000 +0200 +++ exim4-4.92/debian/README.Debian.xml 2021-03-18 09:10:15.000000000 +0100 @@ -1084,17 +1084,38 @@ </para> <para> This means that you will not need any special configuration if - you want to use TLS for outgoing mail. However, if your + you want to use TLS for outgoing mail. However, to enforce + TLS and successful certificate verification, a few things + need to be configured. + </para> + <para> + To enforce TLS and prevent fallback to unencrypted + connections, ensure that hosts_require_tls = * is in effect on + the respective transport. For the remote_smtp_smarthost + transport, this setting can be controlled via the + REMOTE_SMTP_SMARTHOST_HOSTS_REQUIRE_TLS macro. + </para> + <para> + The certificate presented by the remote host is checked + against the system CA certificate store + (<filename>/etc/ssl/certs/</filename>) and the verification + result is logged (CV=...). However successful certificate + verification is <emphasis>not enforced</emphasis> by default. + This can be changed by setting tls_verify_hosts = * on the + respective transport. + </para> + <para> + Another possibility would be to use DANE for certificate + verification. This requires support on the server side and + a resolver with DNSSEC support on the client side. + </para> + <para> + If your server setup mandates the use of client certificates, you need to amend your remote_smtp and/or remote_smtp_smarthost transports with a tls_certificate option. This is not commonly needed. </para> - <para> - The certificate - presented by the remote host is not checked unless you - specify a tls_verify_certificate option on the transport. - </para> <para id="tls_client_certicate"> To make exim send a TLS certificate to the remote host set REMOTE_SMTP_TLS_CERTIFICATE/REMOTE_SMTP_PRIVATEKEY or for
signature.asc
Description: PGP signature