EAI support for notqmail #197

mbhangui · 2020-12-07T18:04:03Z

This adds EAI support to notqmail. Adapted from Arnt Gulbrandsen unicode address
support here.

Use conf-smtputf8 to compile the EAI support
If conf-smtputf8 has -DSMTPUTF8, use tryidn2 to see if idn2_lookup_u8() from libidn2 can be used.
if Yes, create hassmtputf8.h and #define SMTPUTF8
include hassmtputf8.h in qmail-smtpd.c, qmail-remote.c
The utf8read() function has been adapted from utf8received() by
Erwin Hoffman
Following unit tests cases have been added
- smtpcode() - in qmail-remote.c (test 3 digit codes, valid
  and junk codes)
- mailfrom_parms() in qmail-smtpd.c (test SMTPUTF8 in the
  MAIL FROM parameter)
- utf8read() in qmail-remote.c (test utf8, non-utf8)
- get_capability() in qmail-remote.c to test for presence of EHLO
  capability

This adds EAI support to notqmail. Adapted from dapted from Arnt Gulbrandsen unicode address support [here](http://arnt.gulbrandsen.priv.no/qmail/qmail-smtputf8.patch). 1. Use conf-smtputf8 to compile the EAI support 2. If conf-smtputf8 has -DSMTPUTF8, use tryidn2 to see if idn2_lookup_u8() from libidn2 can be used. if Yes, create hassmtputf8.h and #define SMTPUTF8 3. include hassmtputf8.h in qmail-smtpd.c, qmail-remote.c 4. The utf8read() function has been adapted from utf8received() by Erwin Hoffman 5. Following unit tests cases have been added - smtpcode() - in qmail-remote.c (test 3 digit codes, valid and junk codes) - mailfrom_parms() in qmail-smtpd.c (test SMTPUTF8 in the MAIL FROM parameter) - utf8read() in qmail-remote.c (test utf8, non-utf8) - get_capability() in qmail-remote.c to test for presence of EHLO capability

tests/get_function

schmonz · 2020-12-16T20:51:33Z

Thank you in particular for taking care to write tests for this functionality, especially since we've seen bugs in the currently available implementations.

When and how should this PR land? My thoughts: since SMTPUTF8 is a relatively small elaboration of qmail's existing 8-bit cleanliness, I'd like to see it built and enabled by default in some future notqmail release. Since the feature introduces a dependency on libidn2, I'd like to defer on-by-default until some later release that brings its own breaking changes. That leaves us merging EAI initially as off-by-default (and perhaps also autoconfigured-if-found). This can happen whenever the PR is ready, so it could potentially be part of 1.09 but shouldn't block that release if not.

mbhangui · 2020-12-17T15:39:13Z

On Thu, 17 Dec 2020 at 02:21, Amitai Schleier ***@***.***> wrote: When and how should this PR land? My thoughts: since SMTPUTF8 is a relatively small elaboration of qmail's existing 8-bit cleanliness, I'd like to see it built and enabled by default in some future notqmail release. Since the feature introduces a dependency on libidn2, I'd like to defer on-by-default until some later release that brings its own breaking changes. That leaves us merging EAI initially as off-by-default (and perhaps also autoconfigured-if-found). This can happen whenever the PR is ready, so it could potentially be part of 1.09 but shouldn't block that release if not.

It can be disabled by default by commenting out `-DSMTPUTF8` in conf-smtputf8. But the man page will continue to describe the EAI feature.

schmonz · 2020-12-18T11:04:51Z

qmail-remote.c

+#ifdef SMTPUTF8
+#include <idn2.h>
+#include "utf8read.h"
+#include "env.h"
+#endif


I'm seeing a lot of new ifdefs and it makes me a little uneasy about how "glanceable" our code is. We need some ifdefs for EAI because it's compile-time optional, but they can be (1) more isolated and (2) fewer in number. I'll give some examples of changes I'd like to see.

schmonz · 2020-12-18T11:08:13Z

qmail-remote.c

+#ifdef SMTPUTF8
+static stralloc idnhost = { 0 };
+static int      smtputf8 = 0; /*- if remote has SMTPUTF8 capability */
+static char    *enable_utf8 = 0; /*- enable utf8 */
+int             flagutf8;
+#endif
+


These variables don't cost much to have, so we could lose this ifdef. Another function of the ifdef is to document that these are EAI-specific; we could accomplish that by putting them in a new .c file (eai_remote.c?) and linking with the new .o.

schmonz · 2020-12-18T11:09:42Z

qmail-remote.c

@@ -118,36 +131,50 @@ substdio smtpfrom = SUBSTDIO_FDBUF(saferead,-1,smtpfrombuf,sizeof(smtpfrombuf));

 stralloc smtptext = {0};

-static void get(unsigned char *uc)


Haven't checked, just guessing we should probably retain the old name as a synonym in case patches call it.

schmonz · 2020-12-18T11:11:11Z

qmail-remote.c

+#ifdef SMTPUTF8
+  if (enable_utf8 && (flagutf8 || utf8read()))
+    substdio_puts(&smtpto,"> SMTPUTF8\r\n");
+  else
+#endif


It's cheap to check these variables even in the case where we built without EAI.

if we #ifdef enable_utf8 to 0 then this comes for free as the compiler would optimize it away.

schmonz · 2020-12-18T11:11:34Z

qmail-remote.c

+#ifdef SMTPUTF8
+  if (enable_utf8 && header.len) substdio_put(&smtpto, header.s, header.len);
+#endif


Same as previous

schmonz · 2020-12-18T11:11:51Z

qmail-remote.c

+
+#ifdef SMTPUTF8
+  if (enable_utf8 && !flagutf8)
+    flagutf8 = containsutf8(s,str_len(s));
+#endif
+


Same as previous

schmonz · 2020-12-18T11:16:27Z

qmail-remote.c

+#ifdef SMTPUTF8
+  enable_utf8 = env_get("UTF8");
+#endif


Same as previous. Regarding the environment variable, I'm familiar with setting env vars to control my locale settings, but is this an idiomatic way for a sysadmin to control mail server config? Assuming we're built with EAI, I'm wondering whether EAI should be enabled/disabled systemwide, per domain (incoming and outgoing), or some other level of granularity?

And even if all of this is fine this variable name is a bit too generic.

Should SMTPUTF8 be ok instead of just UTF8. We can have a control file to have the setting per domain. We could add a line in getcontrols() to read a control file.

schmonz · 2020-12-18T11:22:36Z

qmail-remote.c

+#ifdef SMTPUTF8
+  } else
+  if (enable_utf8) {
+      char *asciihost = 0;
+      if (!stralloc_0(&host)) temp_nomem();
+      switch (idn2_lookup_u8(host.s,(uint8_t**)&asciihost,IDN2_NFC_INPUT)) {
+        case IDN2_OK:     break;
+        case IDN2_MALLOC: temp_nomem();
+        default:          perm_dns();
+      }
+      if (!stralloc_copys(&idnhost,asciihost)) temp_nomem();
+#endif


This ifdef is a little extra tricky! It's only really needed around the idn2_lookup_u8() logic. That could be moved into eai_remote.c (along with an ifdef), wrapped in a descriptive function name, and called from here. Then we wouldn't need an ifdef here.

schmonz · 2020-12-18T11:23:24Z

qmail-remote.c

+#ifdef SMTPUTF8
+  switch (relayhost ? dns_ip(&ip,&host) : dns_mxip(&ip,enable_utf8 ? &idnhost : &host,random)) {
+#else
  switch (relayhost ? dns_ip(&ip,&host) : dns_mxip(&ip,&host,random)) {
+#endif


Line 472 is enough all by itself :-)

And I would put it on a line distinct from the switch to make the code less convoluted.

schmonz · 2020-12-18T11:24:46Z

qmail-smtpd.c

+#ifdef SMTPUTF8
+int  smtputf8 = 0, flagutf8 = 0;
+#endif


Let's just always define these, in a separate eai_smtpd.c if that helps readability.

And every one on it's own line. Since they are global variables they are also automatically initialized to 0. Adding an explicit initializer is actually less efficient.

I will do away with flagutf8.

schmonz · 2020-12-18T11:32:25Z

qmail-smtpd.c

+#ifdef SMTPUTF8
+stralloc mfparms = {0};
+void mailfrom_parms(const char *arg)
+{
+  int i;
+  int len;
+
+  len = str_len(arg);
+  mfparms.len=0;
+  i = byte_chr(arg,len,'>');
+  if (i > 4 && i < len) {
+    while (len) {
+      arg++; len--;
+      if (*arg == ' ' || *arg == '\0' ) {
+         if (case_starts(mfparms.s,"SMTPUTF8")) smtputf8 = 1;
+         mfparms.len=0;
+      } else
+        if (!stralloc_catb(&mfparms,arg,1)) die_nomem();
+    }
+  }
+}
+#endif
+


So far, it appears the only use of this function (which I recognize from Arnt's patch) is to check whether the peer is trying to request SMTPUTF8 for this delivery. Let's name it accordingly and mark it static. If we see other uses for it later, we can generalize then.

The same function can be used for getting the ESMTP AUTH and SIZE parameters. I will change this to static for the moment.

schmonz · 2020-12-18T11:34:07Z

qmail-smtpd.c

+#ifdef SMTPUTF8
+  if (env_get("UTF8")) {
+    flagutf8 = 1;
+    out("250-SMTPUTF8\r\n");
+  }
+#endif


Cheap to check whether the admin wants to advertise SMTPUTF8 -- let's just always do it.

Then let us remove flagutf8

If I understand what flagutf8 is doing here, I'd keep it as a file global, making sure it gets set in exactly one place (probably in the same place where control files get read). I'd keep all the flagutf8 checks, too -- just expressed in terms of the global boolean rather than the mechanics of how the config gets applied. And then I might call it utf8_available, or something that more precisely expresses this is whether the server offers the capability, as distinct from client-requested states like utf8_requested or utf8_negotiated.

If I don't understand what flagutf8 is doing here, let's fix that first :-)

schmonz · 2020-12-18T11:34:46Z

qmail-smtpd.c

+#ifdef SMTPUTF8
+  if (flagutf8) mailfrom_parms(arg);
+#endif


Boolean checks are cheap

same as previous comment. Let us remove flagutf8

I was trying to say let's keep flagutf8 and get rid of the #ifdef

schmonz · 2020-12-18T11:35:14Z

qmail-smtpd.c

+#ifdef SMTPUTF8
+  received(&qqt,smtputf8 ? "UTF8SMTP" : "SMTP",local,remoteip,remotehost,remoteinfo,fakehelo);
+#else
  received(&qqt,"SMTP",local,remoteip,remotehost,remoteinfo,fakehelo);
+#endif


Line 413 is enough by itself :-)

DerDakon · 2020-12-19T16:29:34Z

qmail-remote.c

+#ifdef SMTPUTF8
+  } else
+  if (enable_utf8) {
+      char *asciihost = 0;


This should be NULL as it's a pointer variable

DerDakon · 2020-12-19T16:30:35Z

qmail-remote.c

+#ifdef SMTPUTF8
+  switch (relayhost ? dns_ip(&ip,&host) : dns_mxip(&ip,enable_utf8 ? &idnhost : &host,random)) {
+#else
  switch (relayhost ? dns_ip(&ip,&host) : dns_mxip(&ip,&host,random)) {
+#endif


And I would put it on a line distinct from the switch to make the code less convoluted.

DerDakon · 2020-12-19T16:32:20Z

qmail-smtpd.c

+#ifdef SMTPUTF8
+int  smtputf8 = 0, flagutf8 = 0;
+#endif


And every one on it's own line. Since they are global variables they are also automatically initialized to 0. Adding an explicit initializer is actually less efficient.

DerDakon · 2020-12-19T16:32:52Z

qmail-smtpd.c

+void mailfrom_parms(const char *arg)
+{
+  int i;
+  int len;


DerDakon · 2020-12-19T16:34:02Z

qmail-smtpd.c

+    while (len) {
+      arg++; len--;
+      if (*arg == ' ' || *arg == '\0' ) {
+         if (case_starts(mfparms.s,"SMTPUTF8")) smtputf8 = 1;


I'm not sure if the case thing is actually right here. I have to reread the RfC, but I would bet this is not case insensitive.

RFC 5321 2.4. General Syntax Principles and Transaction Model

Verbs and argument values (e.g., "TO:" or "to:" in the RCPT command and extension name keywords) are not case sensitive, with the sole exception in this specification of a mailbox local-part.
SMTP Extensions may explicitly specify case-sensitive elements. But RFC 6531 doesn't explicitly mention any case requirement for SMTPUTF8 keyword. So I assume it inherits the definition from RFC 5321 that the keyword should be case insenstive.

josuah · 2021-01-22T22:42:41Z

Should UTF-8 support everywhere really be an option? It is backward compatible with ASCII from the very byte level, so also being able to read UTF-8 might not break anyone's setup...

On the other hand, having the libidn2 dependency optional as we are pulling https://www.gnu.org/software/libunistring/ ?

My ideal would be to provide a tiny idn.c that implements punny code encoding (is that the aim?), but I yet have to provide this.

josuah

Is there really any need to parse the "with UTF8SMTP" along with checking that the other host is supporting UTF8SMTP ?

If we have an email featuring UTF8 names and we need sending it we are screwed and end-up in the same result: the mail is not delivered. Does failing in a different way worth extra code?

And if we have UTF8 in a mail that lacks "with UTF8SMTP" in a Received header, shall we drop the mail, even though it shall also be well-encoded?

UTF8 email (and content in general) does not have problem with software without support for it (excepted for graphical programs that needs to fetch the codepoints), only with those that religiously reject content with the then-be-damned extra bit.

If anyone is having the a reference to some RFC for "with UTF8SMTP", I am highly interested. So far it looks like some indication for debugging which host scrambled the email rather than carrying any useful information.

Hopefully I am not misunderstanding everything.

josuah · 2021-01-22T22:49:55Z

tests/Makefile

+	echo "extern int smtputf8;"; \
+	echo "extern void temp_nomem();"; \
+	./get_function "void mailfrom_parms" \
+	../qmail-smtpd.c|sed s}die_nomem}temp_nomem}g) > mailfrom_parms.c


C code into the makefile and grepping functions from other files looks a bit sudden by looking at the rest of the code.

There is some interesting idea for maintenance, but it would be easier to integrate such changes onto another changes, and globally for all code.

Actually I don't like this. Looks very crude and can easily break when changes are mode to the original c code. But if it breaks, the test will fail during make itself and alert the maintainer to fix it.

josuah · 2021-01-22T22:53:31Z

tests/get_function

+# from the source code
+#
+start_line=`grep -n "^$1" $2|cut -d: -f1`
+sed -n "$start_line,$"p $2 | while IFS= read -r "line"


would sed -n "/^$1/,/}/ p" "$2" not do the same? Also, pointers would not work, but we admittedly have to encounter this case before working around it...

josuah · 2021-01-22T22:54:39Z

tests/unittest_utf8read.c

+  "To: root\n"
+  "\n"
+  "error: can't create transaction lock on /var/lib/rpm/.rpm.lock (Resource temporarily unavailable)\n"
+  "error: /tmp/skype.gpgsig.zgIdKM: key 1 import failed.\n";


Does this count as veeeery indirect advertising for microsoft? ;)

I will remove this. I just cut pasted the latest email in my Maildir/new folder without reliazing the skype thing.

josuah · 2021-01-22T23:05:07Z

utf8read.c

+      if (case_starts(receivedline.s, "Received: from")) received++;  /* found Received header */
+      if (received) {
+        for (i = 0; i < receivedline.len; i++)
+          if (*(receivedline.s + i) != ' ' && *(receivedline.s + i) != '\t')


I actually like this *(variable + i) notation without syntactic sugar for being very clear about what indices are, but the rest of the source goes with variable[i].

Might as well not introduce multiple coding style into a same daemon.

josuah · 2021-01-22T23:25:20Z

utf8read.c

+  }
+  return 0;
+}
+#endif


The implementation from the patch was using a state machine, rather fun and creative, but plain dumb if/for code like yours feels more straightforward IMHO.

Here it is:

+void checkutf8message() +{ + int pos; + int i; + int r; + char ch; + int state; + + if (containsutf8(sender.s, sender.len)) { utf8message = 1; return; } + for (i = 0;i < reciplist.len;++i) + if (containsutf8(reciplist.sa[i].s, reciplist.sa[i].len)) { + utf8message = 1; + return; + } + + state = 0; + pos = 0; + for (;;) { + r = substdio_get(&ssin,&ch,1); + if (r == 0) break; + if (r == -1) temp_read(); + + if (!stralloc_append(&firstpart,&ch)) temp_nomem(); + + if (ch == '\r') + continue; + if (ch == '\t') + ch = ' '; + + switch (state) { + case 6: /* in Received, at LF but before WITH clause */ + if (ch == ' ') { state = 3; pos = 1; continue; } + state = 0; + /* FALL THROUGH */ + + case 0: /* start of header field */ + if (ch == '\n') return; + state = 1; + pos = 0; + /* FALL THROUGH */ + + case 1: /* partway through "Received:" */ + if (ch != "RECEIVED:"[pos] && ch != "received:"[pos]) { state = 2; continue; } + if (++pos == 9) { state = 3; pos = 0; } + continue; + + case 2: /* other header field */ + if (ch == '\n') state = 0; + continue; + + case 3: /* in Received, before WITH clause or partway though " with " */ + if (ch == '\n') { state = 6; continue; } + if (ch != " WITH "[pos] && ch != " with "[pos]) { pos = 0; continue; } + if (++pos == 6) { state = 4; pos = 0; } + continue; + + case 4: /* in Received, having seen with, before the argument */ + if (pos == 0 && (ch == ' ' || ch == '\t')) continue; + if (ch != "UTF8"[pos] && ch != "utf8"[pos]) { state = 5; continue; } + if(++pos == 4) { utf8message = 1; state = 5; continue; } + continue; + + case 5: /* after the RECEIVED WITH argument */ + /* blast() assumes that it copies whole lines */ + if (ch == '\n') return; + state = 1; + pos = 0; + continue; + } + } +}

Cannot most of it be summarized by strncasestr(fieldstart, fieldlen, " with UTF8") ? Case-insensitive searching for the string " with utf8" within the "Received:" header.

josuah · 2021-01-24T17:50:40Z

Thank you @mbhangui for bringing this starting point, that puts on the table everything to do.

mbhangui · 2021-01-25T03:46:12Z

Is there really any need to parse the "with UTF8SMTP" along with checking that the other host is supporting UTF8SMTP ?

If we have an email featuring UTF8 names and we need sending it we are screwed and end-up in the same result: the mail is not delivered. Does failing in a different way worth extra code?

The relevant rfc is RFC 6531 Section 3.6
My understading is that it may be necessary for the smtp client (qmail-remote). But the RFC doesn't make this mandatory.

The MAIL command parameter SMTPUTF8 asserts that a message is an
internationalized message or the message being sent needs the
SMTPUTF8 support. There is still a chance that a message being sent
via the MAIL command with the SMTPUTF8 parameter is not an
internationalized message. An SMTPUTF8-aware SMTP client or server
that requires accurate knowledge of whether a message is
internationalized needs to parse all message header fields and MIME
header fields [RFC2045] in the message body. However, this
specification does not require that the SMTPUTF8-aware SMTP client or
server inspects the message.

And if we have UTF8 in a mail that lacks "with UTF8SMTP" in a Received header, shall we drop the mail, even though it shall also be well-encoded?

We shouldn't be dropping the mail. Rather qmail-remote shouldn't use the SMTPUTF8 extenstion in the MAIL FROM command

UTF8 email (and content in general) does not have problem with software without support for it (excepted for graphical programs that needs to fetch the codepoints), only with those that religiously reject content with the then-be-damned extra bit.

If anyone is having the a reference to some RFC for "with UTF8SMTP", I am highly interested. So far it looks like some indication for debugging which host scrambled the email rather than carrying any useful information.

RFC 6531 (latest) and RFC 5336 (old one)

Hopefully I am not misunderstanding everything.

While going through the commands I realize that the original patch was written as per RFC 5336, hence the header with UTF8SMTP. RFC 5336 has been obsoleted by RFC6531. And the keyword UTF8SMTP should be changed to SMTPUTF8.

4.3.  WITH Protocol Types Sub-Registry of the Mail Transmission Types
      Registry

   IANA has modified or added the following entries in the "WITH
   protocol types" sub-registry under the "Mail Transmission Types"
   registry.

   +--------------+------------------------------+---------------------+
   | WITH         | Description                  | Reference           |
   | protocol     |                              |                     |
   | types        |                              |                     |
   +--------------+------------------------------+---------------------+
   | UTF8SMTP     | ESMTP with SMTPUTF8          | [RFC6531]           |
   | UTF8SMTPA    | ESMTP with SMTPUTF8 and AUTH | [RFC4954] [RFC6531] |
   | UTF8SMTPS    | ESMTP with SMTPUTF8 and      | [RFC3207] [RFC6531] |
   |              | STARTTLS                     |                     |
   | UTF8SMTPSA   | ESMTP with SMTPUTF8 and both | [RFC3207] [RFC4954] |
   |              | STARTTLS and AUTH            | [RFC6531]           |
   | UTF8LMTP     | LMTP with SMTPUTF8           | [RFC6531]           |
   | UTF8LMTPA    | LMTP with SMTPUTF8 and AUTH  | [RFC4954] [RFC6531] |
   | UTF8LMTPS    | LMTP with SMTPUTF8 and       | [RFC3207] [RFC6531] |
   |              | STARTTLS                     |                     |
   | UTF8LMTPSA   | LMTP with SMTPUTF8 and both  | [RFC3207] [RFC4954] |
   |              | STARTTLS and AUTH            | [RFC6531]           |
   +--------------+------------------------------+---------------------+

Good that you asked those questions and I see that there are minor changes to be made.

mbhangui · 2021-01-25T04:06:22Z

While going through the commands I realize that the original patch was written as per RFC 5336, hence the header with UTF8SMTP. RFC 5336 has been obsoleted by RFC6531. And the keyword UTF8SMTP should be changed to SMTPUTF8.

4.3.  WITH Protocol Types Sub-Registry of the Mail Transmission Types
      Registry

   IANA has modified or added the following entries in the "WITH
   protocol types" sub-registry under the "Mail Transmission Types"
   registry.

   +--------------+------------------------------+---------------------+
   | WITH         | Description                  | Reference           |
   | protocol     |                              |                     |
   | types        |                              |                     |
   +--------------+------------------------------+---------------------+
   | UTF8SMTP     | ESMTP with SMTPUTF8          | [RFC6531]           |
   | UTF8SMTPA    | ESMTP with SMTPUTF8 and AUTH | [RFC4954] [RFC6531] |
   | UTF8SMTPS    | ESMTP with SMTPUTF8 and      | [RFC3207] [RFC6531] |
   |              | STARTTLS                     |                     |
   | UTF8SMTPSA   | ESMTP with SMTPUTF8 and both | [RFC3207] [RFC4954] |
   |              | STARTTLS and AUTH            | [RFC6531]           |
   | UTF8LMTP     | LMTP with SMTPUTF8           | [RFC6531]           |
   | UTF8LMTPA    | LMTP with SMTPUTF8 and AUTH  | [RFC4954] [RFC6531] |
   | UTF8LMTPS    | LMTP with SMTPUTF8 and       | [RFC3207] [RFC6531] |
   |              | STARTTLS                     |                     |
   | UTF8LMTPSA   | LMTP with SMTPUTF8 and both  | [RFC3207] [RFC4954] |
   |              | STARTTLS and AUTH            | [RFC6531]           |
   +--------------+------------------------------+---------------------+

I misread the above. The protocol type still remans UTF8SMTP. RFC 5336 had just the description worded differently

   The "Mail Transmission Types" registry under the Mail Parameters
   registry is requested to be updated to include the following new
   entries:

   +---------------+----------------------------+----------------------+
   | WITH protocol | Description                | Reference            |
   | types         |                            |                      |
   +---------------+----------------------------+----------------------+
   | UTF8SMTP      | UTF8SMTP with Service      | [RFC5336]            |
   |               | Extensions                 |                      |
   | UTF8SMTPA     | UTF8SMTP with SMTP AUTH    | [RFC4954] [RFC5336]  |
   | UTF8SMTPS     | UTF8SMTP with STARTTLS     | [RFC3207] [RFC5336]  |
   | UTF8SMTPSA    | UTF8SMTP with both         | [RFC3207] [RFC4954]  |
   |               | STARTTLS and SMTP AUTH     | [RFC5336]            |
   +---------------+----------------------------+----------------------+

mbhangui · 2021-01-25T05:57:11Z

Is there really any need to parse the "with UTF8SMTP" along with checking that the other host is supporting UTF8SMTP ?

I think I understand what you think is a problem. I'll explain below

If we have an email featuring UTF8 names and we need sending it we are screwed and end-up in the same result: the mail is not delivered. Does failing in a different way worth extra code?

And if we have UTF8 in a mail that lacks "with UTF8SMTP" in a Received header, shall we drop the mail, even though it shall also be well-encoded?

I see a problem. Let us say we are running SMTP service for clients to relay emails to external domains. qmail-smtpd advertises SMTPUTF8 and a client uses EAI and uses internationalized domain names in the FROM or the RCPT for a domain example.com (for example). The mail goes into the queue and qmail-remote finds that example.com doesn't support EAI. We should actually be dropping the mail with error like 553 server does not support internationalized email addresses. If we had not advertised SMTPUTF8 in qmail-smtpd, the mail would have gone through.

One can argue that the original sender will get a bounce and then correct the message, but I'm not sure what we should do. Will read the RFC again to clear up this issue

From <https://docs.github.com/en/free-pro-team@latest/github/creating-cloning-and-archiving-repositories/about-code-owners#codeowners-file-location>: "Code owners are automatically requested for review when someone opens a pull request that modifies code that they own." I've not listed everyone in our GitHub org, just the folks who are usually doing the reviewing (and from whom I usually request reviews). No exclusion is intended, just sensible defaults. We can always manually request additional reviewers on any given PR, and anyone who wants to get more generally involved can of course add themselves here.

"Multiplication result may overflow 'int' before it is converted to 'long'." Make n a datetime_sec (synonym for long) to begin with.

DerDakon requested changes Dec 7, 2020

View reviewed changes

tests/get_function Outdated Show resolved Hide resolved

mbhangui added 3 commits December 8, 2020 00:57

replaced echo with printf

a731d82

turn off utf8read test if libidn2 is missing

f802f92

added apt-get install libidn2-0-dev for libidn2

132f09e

mbhangui changed the title ~~EAI support for qmail~~ EAI support for notqmail Dec 7, 2020

mbhangui added 4 commits December 8, 2020 14:31

add conf-smtputf8 as dependency for unittest_utf8read

c6bc474

added libidn2

7ebc35b

added path of include and libs in conf-cc, conf-ld

fbb80e7

build hassmtputf8.h before qmail-smtpd.o

88a2973

mbhangui requested review from schmonz, DerDakon, josuah, jlmuir, leahneukirchen and xenotrope December 8, 2020 09:40

schmonz linked an issue Dec 16, 2020 that may be closed by this pull request

Email Address Internationalization #51

Open

schmonz added the enhancement New feature or request label Dec 16, 2020

schmonz added this to the 1.90 milestone Dec 16, 2020

schmonz reviewed Dec 18, 2020

View reviewed changes

DerDakon requested changes Dec 19, 2020

View reviewed changes

josuah requested changes Jan 22, 2021

View reviewed changes

schmonz and others added 3 commits January 25, 2021 12:23

Fix CodeQL cpp/integer-multiplication-cast-to-long.

4e1ba81

"Multiplication result may overflow 'int' before it is converted to 'long'." Make n a datetime_sec (synonym for long) to begin with.

refactor as per suggested changes

44a00c9

mbhangui force-pushed the mbhangui-smtputf8 branch from 920d49e to 44a00c9 Compare January 25, 2021 06:55

renamed get1() to get()

9764a86

mbhangui force-pushed the mbhangui-smtputf8 branch from 4295ceb to 9764a86 Compare January 25, 2021 07:05

updated mail data

dae3b88

mbhangui force-pushed the mbhangui-smtputf8 branch from 83070f9 to dae3b88 Compare January 25, 2021 07:27

updated qmail-remote code for EAI

6b6f5fe

mbhangui force-pushed the mbhangui-smtputf8 branch from 68c4a37 to 6b6f5fe Compare January 25, 2021 07:56

fix Makefile for Solaris

aac711a

mbhangui force-pushed the mbhangui-smtputf8 branch from 358654a to aac711a Compare January 25, 2021 09:34

Merge branch 'master' into mbhangui-smtputf8

777df37

		@@ -118,36 +131,50 @@ substdio smtpfrom = SUBSTDIO_FDBUF(saferead,-1,smtpfrombuf,sizeof(smtpfrombuf));

		stralloc smtptext = {0};

		static void get(unsigned char *uc)

EAI support for notqmail #197

Are you sure you want to change the base?

EAI support for notqmail #197

Conversation

mbhangui commented Dec 7, 2020 • edited

schmonz commented Dec 16, 2020

mbhangui commented Dec 17, 2020 via email • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbhangui Jan 9, 2021 • edited

Choose a reason for hiding this comment

josuah commented Jan 22, 2021

josuah left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josuah commented Jan 24, 2021

mbhangui commented Jan 25, 2021 • edited

mbhangui commented Jan 25, 2021

mbhangui commented Jan 25, 2021

mbhangui commented Dec 7, 2020 •

edited

mbhangui commented Dec 17, 2020 via email •

edited

mbhangui Jan 9, 2021 •

edited

mbhangui commented Jan 25, 2021 •

edited