From 99350dede64ad634300ddf15d0d97a81fd75d330 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sun, 23 Aug 2020 15:32:48 +0100 Subject: [PATCH] DANE: Fix 2-rcpt message, diff domins case. Bug 2265 --- doc/doc-docbook/spec.xfpt | 6 +++ src/src/debug.c | 1 + src/src/deliver.c | 3 ++ src/src/macros.h | 1 + src/src/transports/smtp.c | 71 ++++++++++++++++++++++++---- src/src/verify.c | 2 +- test/confs/5801 | 89 ++++++++++++++++++++++++++++++++++++ test/dnszones-src/db.test.ex | 1 + test/log/5801 | 13 ++++++ test/scripts/5800-DANE/5801 | 12 +++++ 10 files changed, 188 insertions(+), 11 deletions(-) create mode 100644 test/confs/5801 create mode 100644 test/log/5801 create mode 100644 test/scripts/5800-DANE/5801 diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt index ab13a427b..9a4e0a1a9 100644 --- a/doc/doc-docbook/spec.xfpt +++ b/doc/doc-docbook/spec.xfpt @@ -28771,6 +28771,12 @@ Some other recently added features may only be available in one or the other. This should be documented with the feature. If the documentation does not explicitly state that the feature is infeasible in the other TLS implementation, then patches are welcome. +.new +.next +The output from "exim -bV" will show which (if any) support was included +in the build. +Also, the macro "_HAVE_OPENSSL" or "_HAVE_GNUTLS" will be defined. +.wen .endlist diff --git a/src/src/debug.c b/src/src/debug.c index 90c48dde4..fee0b7a81 100644 --- a/src/src/debug.c +++ b/src/src/debug.c @@ -31,6 +31,7 @@ const uschar * rc_names[] = { /* Mostly for debug output */ [CANCELLED] = US"CANCELLED", [FAIL_SEND] = US"FAIL_SEND", [FAIL_DROP] = US"FAIL_DROP", + [DANE] = US"DANE", }; const uschar * dns_rc_names[] = { diff --git a/src/src/deliver.c b/src/src/deliver.c index dd922c728..a47440695 100644 --- a/src/src/deliver.c +++ b/src/src/deliver.c @@ -460,6 +460,9 @@ TRUE if the lists refer to the same hosts in the same order, except that This enables Exim to use a single SMTP transaction for sending to two entirely different domains that happen to end up pointing at the same hosts. +We do not try to batch up different A-record host names that refer to the +same IP. + Arguments: one points to the first host list two points to the second host list diff --git a/src/src/macros.h b/src/src/macros.h index 5c3fa06f6..8e2050e22 100644 --- a/src/src/macros.h +++ b/src/src/macros.h @@ -305,6 +305,7 @@ Use rc_names[] for debug strings. */ #define CANCELLED 13 /* Authentication cancelled */ #define FAIL_SEND 14 /* send() failed in authenticator */ #define FAIL_DROP 15 /* Fail and drop connection (used in ACL) */ +#define DANE 16 /* Deferred for domain mismatch (used in transport) */ /* Returns from the deliver_message() function */ diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c index d1deffa6f..447f76a9b 100644 --- a/src/src/transports/smtp.c +++ b/src/src/transports/smtp.c @@ -2017,11 +2017,12 @@ if (!continue_hostname) switch (rc = tlsa_lookup(sx->conn_args.host, &sx->conn_args.tlsa_dnsa, sx->dane_required)) { case OK: sx->conn_args.dane = TRUE; - ob->tls_tempfail_tryclear = FALSE; - ob->tls_sni = sx->addrlist->domain; + ob->tls_tempfail_tryclear = FALSE; /* force TLS */ + ob->tls_sni = sx->first_addr->domain; /* force SNI */ break; case FAIL_FORCED: break; - default: set_errno_nohost(sx->addrlist, ERRNO_DNSDEFER, + default: + set_errno_nohost(sx->addrlist, ERRNO_DNSDEFER, string_sprintf("DANE error: tlsa lookup %s", rc_to_string(rc)), rc, FALSE, &sx->delivery_start); @@ -3442,6 +3443,7 @@ BOOL pass_message = FALSE; uschar *message = NULL; uschar new_message_id[MESSAGE_ID_LENGTH + 1]; smtp_context * sx = store_get(sizeof(*sx), TRUE); /* tainted, for the data buffers */ +BOOL dane_held; *message_defer = FALSE; @@ -3457,13 +3459,36 @@ sx->conn_args.tblock = tblock; gettimeofday(&sx->delivery_start, NULL); sx->sync_addr = sx->first_addr = addrlist; -/* Get the channel set up ready for a message (MAIL FROM being the next -SMTP command to send */ +DANE_DOMAINS: +dane_held = FALSE; + +/* Get the channel set up ready for a message, MAIL FROM being the next +SMTP command to send. */ if ((rc = smtp_setup_conn(sx, suppress_tls)) != OK) { timesince(&addrlist->delivery_time, &sx->delivery_start); - return rc; + yield = rc; + goto TIDYUP; + } + +/*XXX*/ +/* If the connection used DANE, ignore for now any addresses with incompatible +domains. The SNI has to be the domain. Arrange a whole new TCP conn later, +just in case only TLS isn't enough. */ + +if (sx->conn_args.dane) + { + const uschar * dane_domain = sx->first_addr->domain; + + for (address_item * a = sx->first_addr->next; a; a = a->next) + if ( a->transport_return == PENDING_DEFER + && Ustrcmp(dane_domain, a->domain) != 0) + { + DEBUG(D_transport) debug_printf("DANE: holding %s for later\n", a->domain); + dane_held = TRUE; + a->transport_return = DANE; + } } /* If there is a filter command specified for this transport, we can now @@ -4213,7 +4238,7 @@ if (sx->completed_addr && sx->ok && sx->send_quit) if (sx->first_addr != NULL) /* More addresses still to be sent */ - { /* in this run of the transport */ + { /* on this connection */ continue_sequence++; /* Causes * in logging */ pipelining_active = sx->pipelining_used; /* was cleared at DATA */ goto SEND_MESSAGE; @@ -4249,7 +4274,7 @@ if (sx->completed_addr && sx->ok && sx->send_quit) '2', ob->command_timeout); if (sx->ok && f.continue_more) - return yield; /* More addresses for another run */ + goto TIDYUP; /* More addresses for another run */ } else { @@ -4269,7 +4294,7 @@ if (sx->completed_addr && sx->ok && sx->send_quit) else #endif if (f.continue_more) - return yield; /* More addresses for another run */ + goto TIDYUP; /* More addresses for another run */ /* If the socket is successfully passed, we mustn't send QUIT (or indeed anything!) from here. */ @@ -4309,7 +4334,7 @@ propagate it from the initial sx->cctx.sock = -1; continue_transport = NULL; continue_hostname = NULL; - return yield; + goto TIDYUP; } log_write(0, LOG_PANIC_DIE, "fork failed"); } @@ -4384,9 +4409,35 @@ if (sx->send_quit) (void) event_raise(tblock->event_action, US"tcp:close", NULL); #endif +/*XXX*/ +if (dane_held) + { + sx->first_addr = NULL; + for (address_item * a = sx->addrlist->next; a; a = a->next) + if (a->transport_return == DANE) + { + a->transport_return = PENDING_DEFER; + if (!sx->first_addr) + { + /* Remember the new start-point in the addrlist, for smtp_setup_conn() + to get the domain string for SNI */ + + sx->first_addr = a; + DEBUG(D_transport) debug_printf("DANE: go-around for %s\n", a->domain); + } + } + goto DANE_DOMAINS; + } + continue_transport = NULL; continue_hostname = NULL; return yield; + +TIDYUP: +if (dane_held) for (address_item * a = sx->addrlist->next; a; a = a->next) + if (a->transport_return == DANE) + a->transport_return = PENDING_DEFER; +return yield; } diff --git a/src/src/verify.c b/src/src/verify.c index efc05fcf1..a50ac8b7b 100644 --- a/src/src/verify.c +++ b/src/src/verify.c @@ -654,7 +654,7 @@ coding means skipping this whole loop and doing the append separately. */ if (!sx) sx = store_get(sizeof(*sx), TRUE); /* tainted buffers */ memset(sx, 0, sizeof(*sx)); - sx->addrlist = addr; + sx->addrlist = sx->first_addr = addr; sx->conn_args.host = host; sx->conn_args.host_af = host_af, sx->port = port; diff --git a/test/confs/5801 b/test/confs/5801 new file mode 100644 index 000000000..f0f21e20b --- /dev/null +++ b/test/confs/5801 @@ -0,0 +1,89 @@ +# Exim test configuration 5801 +# DANE common + +SERVER= +CONTROL= * + +.include DIR/aux-var/tls_conf_prefix + +primary_hostname = myhost.test.ex + +# ----- Main settings ----- + +.ifndef OPT +acl_smtp_rcpt = accept +.else +acl_smtp_rcpt = accept verify = recipient/callout +.endif + +log_selector = +received_recipients +tls_certificate_verified +tls_sni + +queue_run_in_order + +tls_advertise_hosts = * +.ifdef _HAVE_GNUTLS +# needed to force generation +tls_dhparam = historic +.endif + +# Set certificate only if server +CDIR1 = DIR/aux-fixed/exim-ca/example.net/server1.example.net +CDIR2 = DIR/aux-fixed/exim-ca/example.com/server1.example.com + + +tls_certificate = ${if eq {SERVER}{server} \ + {${if or {{eq {DETAILS}{ta}} {eq {DETAILS}{ca}} {eq {DETAILS}{ee}}} \ + {CDIR2/fullchain.pem}\ + {CDIR1/fullchain.pem}}}\ + fail} + +tls_privatekey = ${if eq {SERVER}{server} \ + {${if or {{eq {DETAILS}{ta}} {eq {DETAILS}{ca}} {eq {DETAILS}{ee}}} \ + {CDIR2/server1.example.com.unlocked.key}\ + {CDIR1/server1.example.net.unlocked.key}}}\ + fail} + +# ----- Routers ----- + +begin routers + +client: + driver = dnslookup + condition = ${if eq {SERVER}{}} + dnssec_request_domains = * + self = send + transport = send_to_server + errors_to = "" + +server: + driver = redirect + data = :blackhole: + + +# ----- Transports ----- + +begin transports + +send_to_server: + driver = smtp + allow_localhost + port = PORT_D + hosts_try_fastopen = : + + hosts_try_dane = CONTROL + hosts_require_dane = HOSTIPV4 + tls_verify_cert_hostnames = ${if eq {OPT}{no_certname} {}{*}} + tls_try_verify_hosts = thishost.test.ex + tls_verify_certificates = ${if eq {DETAILS}{ca} {CDIR2/ca_chain.pem} {}} + + + +# ----- Retry ----- + + +begin retry + +* * F,5d,10s + + +# End diff --git a/test/dnszones-src/db.test.ex b/test/dnszones-src/db.test.ex index 9b6684ac7..f15bf7a7f 100644 --- a/test/dnszones-src/db.test.ex +++ b/test/dnszones-src/db.test.ex @@ -441,6 +441,7 @@ AA a-aa A V4NET.0.0.100 ; | awk '{print $2}' ; DNSSEC mxdane512ee MX 1 dane512ee +DNSSEC mxdane512ee1 MX 1 dane512ee DNSSEC dane512ee A HOSTIPV4 DNSSEC _1225._tcp.dane512ee TLSA 3 1 2 c0c2fc12e9fe1abf0ae7b1f2ad2798a4689668db8cf7f7b771a43bf8a4f1d9741ef103bad470b1201157150fbd6182054b0170e90ce66b944a82a0a9c81281af diff --git a/test/log/5801 b/test/log/5801 new file mode 100644 index 000000000..3cf13694d --- /dev/null +++ b/test/log/5801 @@ -0,0 +1,13 @@ +1999-03-02 09:44:33 10HmaX-0005vi-00 <= CALLER@myhost.test.ex U=CALLER P=local S=sss for t00@mxdane512ee.test.ex t01@mxdane512ee1.test.ex +1999-03-02 09:44:33 10HmaX-0005vi-00 => t00@mxdane512ee.test.ex R=client T=send_to_server H=dane512ee.test.ex [ip4.ip4.ip4.ip4] X=TLS1.x:ke-RSA-AES256-SHAnnn:xxx CV=dane C="250 OK id=10HmaY-0005vi-00" +1999-03-02 09:44:33 10HmaX-0005vi-00 => t01@mxdane512ee1.test.ex R=client T=send_to_server H=dane512ee.test.ex [ip4.ip4.ip4.ip4] X=TLS1.x:ke-RSA-AES256-SHAnnn:xxx CV=dane C="250 OK id=10HmaZ-0005vi-00" +1999-03-02 09:44:33 10HmaX-0005vi-00 Completed + +******** SERVER ******** +1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port PORT_D +1999-03-02 09:44:33 10HmaY-0005vi-00 <= <> H=the.local.host.name (myhost.test.ex) [ip4.ip4.ip4.ip4] P=esmtps X=TLS1.x:ke-RSA-AES256-SHAnnn:xxx CV=no SNI=mxdane512ee.test.ex S=sss id=E10HmaX-0005vi-00@myhost.test.ex for t00@mxdane512ee.test.ex +1999-03-02 09:44:33 10HmaY-0005vi-00 => :blackhole: R=server +1999-03-02 09:44:33 10HmaY-0005vi-00 Completed +1999-03-02 09:44:33 10HmaZ-0005vi-00 <= <> H=the.local.host.name (myhost.test.ex) [ip4.ip4.ip4.ip4] P=esmtps X=TLS1.x:ke-RSA-AES256-SHAnnn:xxx CV=no SNI=mxdane512ee1.test.ex S=sss id=E10HmaX-0005vi-00@myhost.test.ex for t01@mxdane512ee1.test.ex +1999-03-02 09:44:33 10HmaZ-0005vi-00 => :blackhole: R=server +1999-03-02 09:44:33 10HmaZ-0005vi-00 Completed diff --git a/test/scripts/5800-DANE/5801 b/test/scripts/5800-DANE/5801 new file mode 100644 index 000000000..98fa6b18b --- /dev/null +++ b/test/scripts/5800-DANE/5801 @@ -0,0 +1,12 @@ +# DANE client: conflicting domain +# +exim -DSERVER=server -DDETAILS=ee -bd -oX PORT_D +**** +# +# +# A single message with 2 receipients, different domains though same MX host +exim -odf t00@mxdane512ee.test.ex t01@mxdane512ee1.test.ex +**** +# +killdaemon +no_msglog_check -- 2.30.2