From bec94709e708f087fe7fa456bec95d4e63edc3ed Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Wed, 24 Jun 2020 00:04:13 +0100 Subject: [PATCH] Handle quoted local_part input to ${srs_encode }. Bug 2607 --- doc/doc-txt/ChangeLog | 5 +++ src/src/expand.c | 88 ++++++++++++++++++++++++++------------ test/confs/4620 | 15 ++++++- test/log/4620 | 27 +++++++++++- test/mail/4620.CALLER | 4 ++ test/mail/4620.fred[ | 60 ++++++++++++++++++++++++++ test/runtest | 2 +- test/scripts/4620-SRS/4620 | 27 ++++++++++++ test/stdout/4620 | 3 ++ 9 files changed, 198 insertions(+), 33 deletions(-) create mode 100644 test/mail/4620.fred[ create mode 100644 test/stdout/4620 diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 6db97eef7..2c0570635 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -58,6 +58,11 @@ JH/11 Bug 2604: Fix request to cutthrough-deliver when a connection is already held open for a verify callout. Previously this wan not accounted for and a corrupt onward SMTP conversation resulted. +JH/12 Bug 2607: Fix the ${srs_encode } expansion to handle quoted local_parts. + Previously they were embedded naively in the constructed address; when + needed, strip the quoting and quote the entire local_part. + Also make the inbound_srs expansion condition handle quoting. + Exim version 4.94 diff --git a/src/src/expand.c b/src/src/expand.c index f7e9e5c25..291db426a 100644 --- a/src/src/expand.c +++ b/src/src/expand.c @@ -3451,7 +3451,7 @@ switch(cond_type = identify_operator(&s, &opname)) uschar * sub[2]; const pcre * re; int ovec[3*(4+1)]; - int n; + int n, quoting = 0; uschar cksum[4]; BOOL boolvalue = FALSE; @@ -3474,10 +3474,20 @@ switch(cond_type = identify_operator(&s, &opname)) goto srs_result; } - /* Side-effect: record the decoded recipient */ + if (sub[0][0] == '"') + quoting = 1; + else for (uschar * s = sub[0]; *s; s++) + if (!isalnum(*s) && Ustrchr(".!#$%&'*+-/=?^_`{|}~", *s) == NULL) + { quoting = 1; break; } + if (quoting) + DEBUG(D_expand) debug_printf_indent("auto-quoting local part\n"); + + /* Record the (quoted, if needed) decoded recipient as $srs_recipient */ - srs_recipient = string_sprintf("%.*S@%.*S", /* lowercased */ + srs_recipient = string_sprintf("%.*s%.*S%.*s@%.*S", /* lowercased */ + quoting, "\"", ovec[9]-ovec[8], sub[0] + ovec[8], /* substring 4 */ + quoting, "\"", ovec[7]-ovec[6], sub[0] + ovec[6]); /* substring 3 */ /* If a zero-length secret was given, we're done. Otherwise carry on @@ -6777,6 +6787,8 @@ while (*s) { uschar * sub[3]; uschar cksum[4]; + gstring * g = NULL; + BOOL quoted = FALSE; switch (read_subs(sub, 3, 3, CUSS &s, skipping, TRUE, name, &resetok)) { @@ -6785,41 +6797,65 @@ while (*s) case 3: goto EXPAND_FAILED; } - yield = string_catn(yield, US"SRS0=", 5); + g = string_catn(g, US"SRS0=", 5); /* ${l_4:${hmac{md5}{SRS_SECRET}{${lc:$return_path}}}}= */ hmac_md5(sub[0], string_copylc(sub[1]), cksum, sizeof(cksum)); - yield = string_catn(yield, cksum, sizeof(cksum)); - yield = string_catn(yield, US"=", 1); + g = string_catn(g, cksum, sizeof(cksum)); + g = string_catn(g, US"=", 1); /* ${base32:${eval:$tod_epoch/86400&0x3ff}}= */ { struct timeval now; unsigned long i; - gstring * g = NULL; + gstring * h = NULL; gettimeofday(&now, NULL); for (unsigned long i = (now.tv_sec / 86400) & 0x3ff; i; i >>= 5) - g = string_catn(g, &base32_chars[i & 0x1f], 1); - if (g) while (g->ptr > 0) - yield = string_catn(yield, &g->s[--g->ptr], 1); + h = string_catn(h, &base32_chars[i & 0x1f], 1); + if (h) while (h->ptr > 0) + g = string_catn(g, &h->s[--h->ptr], 1); } - yield = string_catn(yield, US"=", 1); + g = string_catn(g, US"=", 1); /* ${domain:$return_path}=${local_part:$return_path} */ { int start, end, domain; uschar * t = parse_extract_address(sub[1], &expand_string_message, &start, &end, &domain, FALSE); + uschar * s; + if (!t) goto EXPAND_FAILED; - if (domain > 0) yield = string_cat(yield, t + domain); - yield = string_catn(yield, US"=", 1); - yield = domain > 0 - ? string_catn(yield, t, domain - 1) : string_cat(yield, t); + if (domain > 0) g = string_cat(g, t + domain); + g = string_catn(g, US"=", 1); + + s = domain > 0 ? string_copyn(t, domain - 1) : t; + if ((quoted = Ustrchr(s, '"') != NULL)) + { + gstring * h = NULL; + DEBUG(D_expand) debug_printf_indent("auto-quoting local part\n"); + while (*s) /* de-quote */ + { + while (*s && *s != '"') h = string_catn(h, s++, 1); + if (*s) s++; + while (*s && *s != '"') h = string_catn(h, s++, 1); + if (*s) s++; + } + gstring_release_unused(h); + s = string_from_gstring(h); + } + g = string_cat(g, s); } + /* Assume that if the original local_part had quotes + it was for good reason */ + + if (quoted) yield = string_catn(yield, US"\"", 1); + yield = string_catn(yield, g->s, g->ptr); + if (quoted) yield = string_catn(yield, US"\"", 1); + /* @$original_domain */ yield = string_catn(yield, US"@", 1); yield = string_cat(yield, sub[2]); @@ -7471,24 +7507,20 @@ while (*s) uschar *t = sub - 1; if (c == EOP_QUOTE) - { - while (!needs_quote && *(++t) != 0) + while (!needs_quote && *++t) needs_quote = !isalnum(*t) && !strchr("_-.", *t); - } + else /* EOP_QUOTE_LOCAL_PART */ - { - while (!needs_quote && *(++t) != 0) - needs_quote = !isalnum(*t) && - strchr("!#$%&'*+-/=?^_`{|}~", *t) == NULL && - (*t != '.' || t == sub || t[1] == 0); - } + while (!needs_quote && *++t) + needs_quote = !isalnum(*t) + && strchr("!#$%&'*+-/=?^_`{|}~", *t) == NULL + && (*t != '.' || t == sub || !t[1]); if (needs_quote) { yield = string_catn(yield, US"\"", 1); t = sub - 1; - while (*(++t) != 0) - { + while (*++t) if (*t == '\n') yield = string_catn(yield, US"\\n", 2); else if (*t == '\r') @@ -7499,10 +7531,10 @@ while (*s) yield = string_catn(yield, US"\\", 1); yield = string_catn(yield, t, 1); } - } yield = string_catn(yield, US"\"", 1); } - else yield = string_cat(yield, sub); + else + yield = string_cat(yield, sub); continue; } diff --git a/test/confs/4620 b/test/confs/4620 index 26906e7c7..ad115c962 100644 --- a/test/confs/4620 +++ b/test/confs/4620 @@ -11,19 +11,29 @@ acl_smtp_rcpt = accept domainlist local_domains = test.ex domainlist remotesite_domains = remote.ex -log_selector = +all_parents +received_recipients +log_selector = +all_parents +received_recipients +return_path_on_delivery queue_only # ----- Routers ----- begin routers +.ifdef CONTROL remote_bouncer: driver = redirect condition = ${if eq {$sender_host_address}{127.0.0.1}} data = :fail: account disabled allow_fail +bounce_return: + driver = manualroute + domains = +local_domains + senders = : + route_list = test.ex 127.0.0.1::PORT_S + self = send + transport = to_external +.endif + external: driver = manualroute domains = !+local_domains @@ -32,6 +42,7 @@ external: transport = ${if eq {$local_part@$domain} {$original_local_part@$original_domain} \ {to_external} {forwarded_external}} +.ifndef CONTROL inbound_srs: driver = redirect senders = : @@ -48,7 +59,7 @@ inbound_srs_failure: condition = ${if inbound_srs {$local_part} {}} allow_fail data = :fail: Invalid SRS recipient address - +.endif local_redirect: driver = redirect diff --git a/test/log/4620 b/test/log/4620 index 5e4413a3e..3510a970a 100644 --- a/test/log/4620 +++ b/test/log/4620 @@ -1,5 +1,5 @@ 1999-03-02 09:44:33 10HmaX-0005vi-00 <= CALLER@the.local.host.name U=CALLER P=local S=sss for redirect@test.ex -1999-03-02 09:44:33 10HmaX-0005vi-00 => remote_user@remote.ex R=external T=forwarded_external H=127.0.0.1 [127.0.0.1] C="250 OK id=10HmaY-0005vi-00" +1999-03-02 09:44:33 10HmaX-0005vi-00 => remote_user@remote.ex P= R=external T=forwarded_external H=127.0.0.1 [127.0.0.1] C="250 OK id=10HmaY-0005vi-00" 1999-03-02 09:44:33 10HmaX-0005vi-00 Completed 1999-03-02 09:44:33 Start queue run: pid=pppp 1999-03-02 09:44:33 10HmaY-0005vi-00 ** remote_user@remote.ex R=remote_bouncer: account disabled @@ -7,10 +7,33 @@ 1999-03-02 09:44:33 10HmaY-0005vi-00 Completed 1999-03-02 09:44:33 End queue run: pid=pppp 1999-03-02 09:44:33 Start queue run: pid=pppp -1999-03-02 09:44:33 10HmaZ-0005vi-00 => CALLER R=local T=appendfile +1999-03-02 09:44:33 10HmaZ-0005vi-00 => srs0=12a1=f=the.local.host.name=CALLER@test.ex P=<> R=bounce_return T=to_external H=127.0.0.1 [127.0.0.1] C="250 OK id=10HmbA-0005vi-00" 1999-03-02 09:44:33 10HmaZ-0005vi-00 Completed 1999-03-02 09:44:33 End queue run: pid=pppp +1999-03-02 09:44:33 Start queue run: pid=pppp +1999-03-02 09:44:33 10HmbA-0005vi-00 => CALLER P=<> R=local T=appendfile +1999-03-02 09:44:33 10HmbA-0005vi-00 Completed +1999-03-02 09:44:33 End queue run: pid=pppp +1999-03-02 09:44:33 10HmbB-0005vi-00 <= "fred["@test.ex U=root P=local S=sss for redirect@test.ex +1999-03-02 09:44:33 10HmbB-0005vi-00 => remote_user@remote.ex P=<"SRS0=ZZZZ=YY=test.ex=fred["@test.ex> R=external T=forwarded_external H=127.0.0.1 [127.0.0.1] C="250 OK id=10HmbC-0005vi-00" +1999-03-02 09:44:33 10HmbB-0005vi-00 Completed +1999-03-02 09:44:33 Start queue run: pid=pppp +1999-03-02 09:44:33 10HmbC-0005vi-00 ** remote_user@remote.ex R=remote_bouncer: account disabled +1999-03-02 09:44:33 10HmbD-0005vi-00 <= <> R=10HmbC-0005vi-00 U=EXIMUSER P=local S=sss for "SRS0=ZZZZ=YY=test.ex=fred["@test.ex +1999-03-02 09:44:33 10HmbC-0005vi-00 Completed +1999-03-02 09:44:33 End queue run: pid=pppp +1999-03-02 09:44:33 Start queue run: pid=pppp +1999-03-02 09:44:33 10HmbD-0005vi-00 => srs0=5a75=f=test.ex=fred[@test.ex <"SRS0=ZZZZ=YY=test.ex=fred["@test.ex> P=<> R=bounce_return T=to_external H=127.0.0.1 [127.0.0.1] C="250 OK id=10HmbE-0005vi-00" +1999-03-02 09:44:33 10HmbD-0005vi-00 Completed +1999-03-02 09:44:33 End queue run: pid=pppp +1999-03-02 09:44:33 Start queue run: pid=pppp +1999-03-02 09:44:33 10HmbE-0005vi-00 => fred[ <"SRS0=ZZZZ=YY=test.ex=fred["@test.ex> P=<> R=local T=appendfile +1999-03-02 09:44:33 10HmbE-0005vi-00 Completed +1999-03-02 09:44:33 End queue run: pid=pppp ******** SERVER ******** 1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on [127.0.0.1]:PORT_S 1999-03-02 09:44:33 10HmaY-0005vi-00 <= SRS0=ZZZZ=YY=the.local.host.name=CALLER@test.ex H=localhost (the.local.host.name) [127.0.0.1] P=esmtp S=sss id=E10HmaX-0005vi-00@the.local.host.name for remote_user@remote.ex +1999-03-02 09:44:33 10HmbA-0005vi-00 <= <> H=localhost (the.local.host.name) [127.0.0.1] P=esmtp S=sss id=E10HmaZ-0005vi-00@the.local.host.name for SRS0=ZZZZ=YY=the.local.host.name=CALLER@test.ex +1999-03-02 09:44:33 10HmbC-0005vi-00 <= "SRS0=ZZZZ=YY=test.ex=fred["@test.ex H=localhost (the.local.host.name) [127.0.0.1] P=esmtp S=sss id=E10HmbB-0005vi-00@the.local.host.name for remote_user@remote.ex +1999-03-02 09:44:33 10HmbE-0005vi-00 <= <> H=localhost (the.local.host.name) [127.0.0.1] P=esmtp S=sss id=E10HmbD-0005vi-00@the.local.host.name for "SRS0=ZZZZ=YY=test.ex=fred["@test.ex diff --git a/test/mail/4620.CALLER b/test/mail/4620.CALLER index 8daaeed5f..f42604b3c 100644 --- a/test/mail/4620.CALLER +++ b/test/mail/4620.CALLER @@ -1,4 +1,8 @@ From MAILER-DAEMON Tue Mar 02 09:44:33 1999 +Received: from localhost ([127.0.0.1] helo=the.local.host.name) + by the.local.host.name with esmtp (Exim x.yz) + id 10HmbA-0005vi-00 + for SRS0=ZZZZ=YY=the.local.host.name=CALLER@test.ex; Tue, 2 Mar 1999 09:44:33 +0000 Received: from EXIMUSER by the.local.host.name with local (Exim x.yz) id 10HmaZ-0005vi-00 for SRS0=ZZZZ=YY=the.local.host.name=CALLER@test.ex; Tue, 2 Mar 1999 09:44:33 +0000 diff --git a/test/mail/4620.fred[ b/test/mail/4620.fred[ new file mode 100644 index 000000000..3a6cf1f46 --- /dev/null +++ b/test/mail/4620.fred[ @@ -0,0 +1,60 @@ +From MAILER-DAEMON Tue Mar 02 09:44:33 1999 +Received: from localhost ([127.0.0.1] helo=the.local.host.name) + by the.local.host.name with esmtp (Exim x.yz) + id 10HmbE-0005vi-00 + for "SRS0=ZZZZ=YY=test.ex=fred["@test.ex; Tue, 2 Mar 1999 09:44:33 +0000 +Received: from EXIMUSER by the.local.host.name with local (Exim x.yz) + id 10HmbD-0005vi-00 + for "SRS0=ZZZZ=YY=test.ex=fred["@test.ex; Tue, 2 Mar 1999 09:44:33 +0000 +X-Failed-Recipients: remote_user@remote.ex +Auto-Submitted: auto-replied +From: Mail Delivery System +To: "SRS0=ZZZZ=YY=test.ex=fred["@test.ex +References: +Content-Type: multipart/report; report-type=delivery-status; boundary=NNNNNNNNNN-eximdsn-MMMMMMMMMM +MIME-Version: 1.0 +Subject: Mail delivery failed: returning message to sender +Message-Id: +Date: Tue, 2 Mar 1999 09:44:33 +0000 + +--NNNNNNNNNN-eximdsn-MMMMMMMMMM +Content-type: text/plain; charset=us-ascii + +This message was created automatically by mail delivery software. + +A message that you sent could not be delivered to one or more of its +recipients. This is a permanent error. The following address(es) failed: + + remote_user@remote.ex + account disabled + +--NNNNNNNNNN-eximdsn-MMMMMMMMMM +Content-type: message/delivery-status + +Reporting-MTA: dns; the.local.host.name + +Action: failed +Final-Recipient: rfc822;remote_user@remote.ex +Status: 5.0.0 + +--NNNNNNNNNN-eximdsn-MMMMMMMMMM +Content-type: message/rfc822 + +Return-path: <"SRS0=ZZZZ=YY=test.ex=fred["@test.ex> +Received: from localhost ([127.0.0.1] helo=the.local.host.name) + by the.local.host.name with esmtp (Exim x.yz) + (envelope-from <"SRS0=ZZZZ=YY=test.ex=fred["@test.ex>) + id 10HmbC-0005vi-00 + for remote_user@remote.ex; Tue, 2 Mar 1999 09:44:33 +0000 +Received: from root by the.local.host.name with local (Exim x.yz) + (envelope-from <"fred["@test.ex>) + id 10HmbB-0005vi-00 + for redirect@test.ex; Tue, 2 Mar 1999 09:44:33 +0000 +Message-Id: +From: "fred["@test.ex +Date: Tue, 2 Mar 1999 09:44:33 +0000 + +Message body + +--NNNNNNNNNN-eximdsn-MMMMMMMMMM-- + diff --git a/test/runtest b/test/runtest index d32ea54dd..961b6ec62 100755 --- a/test/runtest +++ b/test/runtest @@ -934,7 +934,7 @@ RESET_AFTER_EXTRA_LINE_READ: # SRS timestamps and signatures vary by hostname and from run to run - s/SRS0=....=.[^=]?=[^=]+=[^@]+\@test.ex/SRS0=ZZZZ=YY=the.local.host.name=CALLER\@test.ex/; + s/SRS0=....=.[^=]?=([^=]+)=([^@]+)\@([^ ]+)/SRS0=ZZZZ=YY=$1=$2\@$3/; # ======== Output from the "fd" program about open descriptors ======== diff --git a/test/scripts/4620-SRS/4620 b/test/scripts/4620-SRS/4620 index 4a126b8b9..f180142a6 100644 --- a/test/scripts/4620-SRS/4620 +++ b/test/scripts/4620-SRS/4620 @@ -7,9 +7,36 @@ exim -odi redirect@test.ex Message body **** # Run the queue for the remote, will generate bounce which is queued +exim -DCONTROL=remote -q +**** +# Run the queue for the remote, will send bounce to origin +exim -DCONTROL=remote -q +**** +# Run the queue for the local, will process the bounce exim -q **** +# +# +# +# +# Sender with quoted local_part: +# +exim -be +${srs_encode {mysecret} {eximtest@lap.dom.ain} {test.ex}} +${srs_encode {mysecret} {"eximtest"@lap.dom.ain} {test.ex}} +**** +# +# Inject a message; will be passed on to remote and queued there +sudo exim -odi -f '"fred["@test.ex' redirect@test.ex +Message body +**** +# Run the queue for the remote, will generate bounce which is queued +exim -DCONTROL=remote -q +**** # Run the queue for the remote, will send bounce to origin +exim -DCONTROL=remote -q +**** +# Run the queue for the local, will process the bounce exim -q **** # diff --git a/test/stdout/4620 b/test/stdout/4620 new file mode 100644 index 000000000..7883638a6 --- /dev/null +++ b/test/stdout/4620 @@ -0,0 +1,3 @@ +> SRS0=ZZZZ=YY=the.local.host.name=CALLER@test.ex +> "SRS0=ZZZZ=YY=the.local.host.name=CALLER"@test.ex +> -- 2.30.2