Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

EAI support for notqmail #197

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

EAI support for notqmail #197

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
27a8ecb
EAI support for qmail
mbhangui Dec 7, 2020
a731d82
replaced echo with printf
mbhangui Dec 7, 2020
f802f92
turn off utf8read test if libidn2 is missing
mbhangui Dec 7, 2020
132f09e
added apt-get install libidn2-0-dev for libidn2
mbhangui Dec 7, 2020
c6bc474
add conf-smtputf8 as dependency for unittest_utf8read
mbhangui Dec 8, 2020
7ebc35b
added libidn2
mbhangui Dec 8, 2020
fbb80e7
added path of include and libs in conf-cc, conf-ld
mbhangui Dec 8, 2020
88a2973
build hassmtputf8.h before qmail-smtpd.o
mbhangui Dec 8, 2020
97157ba
Define CODEOWNERS with our default reviewers.
schmonz Dec 2, 2020
4e1ba81
Fix CodeQL cpp/integer-multiplication-cast-to-long.
schmonz Dec 2, 2020
44a00c9
refactor as per suggested changes
mbhangui Jan 25, 2021
9764a86
renamed get1() to get()
mbhangui Jan 25, 2021
dae3b88
updated mail data
mbhangui Jan 25, 2021
6b6f5fe
updated qmail-remote code for EAI
mbhangui Jan 25, 2021
aac711a
fix Makefile for Solaris
mbhangui Jan 25, 2021
777df37
Merge branch 'master' into mbhangui-smtputf8
mbhangui Jan 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 26 additions & 7 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1348,16 +1348,35 @@ alloc.h substdio.h datetime.h now.h datetime.h triggerpull.h extra.h \
uidgid.h auto_qmail.h auto_uids.h auto_users.h date822fmt.h fmtqfn.h
./compile qmail-queue.c

hassmtputf8.h: \
tryidn2.c compile load conf-smtputf8
((./compile `grep -h -v "^#" conf-smtputf8` tryidn2.c \
&& ./load tryidn2 -lidn2) >/dev/null 2>&1 \
&& echo \#define SMTPUTF8 1 || echo "WARNING!!! Not compiled with -DSMTPUTF8" 1<&2) > hassmtputf8.h
rm -f tryidn2.o tryidn2

idn2.lib: \
tryidn2.c compile load conf-smtputf8
((./compile `grep -h -v "^#" conf-smtputf8` tryidn2.c \
&& ./load tryidn2 -lidn2) >/dev/null 2>&1 \
&& echo "-lidn2" || echo "WARNING!!! Not linked with libidn2" 1>&2) > idn2.lib
rm -f tryidn2.o tryidn2

utf8read.o: \
compile utf8read.c hassmtputf8.h stralloc.h case.h substdio.h subfd.h
./compile utf8read.c

qmail-remote: \
load qmail-remote.o control.o constmap.o timeoutread.o timeoutwrite.o \
timeoutconn.o tcpto.o dns.o ip.o ipalloc.o ipme.o quote.o \
ndelay.a case.a sig.a open.a lock.a getln.a stralloc.a \
substdio.a error.a str.a fs.a auto_qmail.o dns.lib socket.lib
timeoutconn.o tcpto.o dns.o ip.o ipalloc.o ipme.o quote.o envread.o \
utf8read.o ndelay.a case.a sig.a open.a lock.a getln.a stralloc.a \
substdio.a error.a str.a fs.a auto_qmail.o dns.lib socket.lib \
idn2.lib
./load qmail-remote control.o constmap.o timeoutread.o \
timeoutwrite.o timeoutconn.o tcpto.o dns.o ip.o \
timeoutwrite.o timeoutconn.o tcpto.o dns.o ip.o utf8read.o \
ipalloc.o ipme.o quote.o ndelay.a case.a sig.a open.a \
lock.a getln.a stralloc.a substdio.a error.a \
str.a fs.a auto_qmail.o `cat dns.lib` `cat socket.lib`
envread.o lock.a getln.a stralloc.a substdio.a error.a \
str.a fs.a auto_qmail.o `cat dns.lib socket.lib idn2.lib`

qmail-remote.0: \
qmail-remote.8
Expand All @@ -1368,7 +1387,7 @@ subfd.h substdio.h scan.h case.h error.h auto_qmail.h control.h dns.h \
alloc.h quote.h ip.h ipalloc.h ip.h gen_alloc.h ipme.h ip.h ipalloc.h \
gen_alloc.h gen_allocdefs.h str.h now.h datetime.h exit.h constmap.h \
tcpto.h readwrite.h timeoutconn.h timeoutread.h timeoutwrite.h oflops.h \
error.h
error.h hassmtputf8.h utf8read.h env.h
./compile qmail-remote.c

qmail-rspawn: \
Expand Down
4 changes: 4 additions & 0 deletions TARGETS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,3 +371,7 @@ forgeries.0
setup
qtmp.h
qmail-send.service
trysmtputf8
idn2.lib
hassmtputf8.h
utf8read.o
1 change: 1 addition & 0 deletions conf-smtputf8
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-DSMTPUTF8
8 changes: 8 additions & 0 deletions qmail-remote.8
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ arguments to
The envelope sender address is listed as
.I sender\fP.

If the environment variable UTF8 is defined,
.B qmail-remote
will respect SMTPUTF8 and EAI addresses. If message is utf8,
.B qmail-remote
will use idn2_lookup_u8(3) to perform IDNA2008 lookup string conversion
on
.IR host .

Note that
.B qmail-remote
does not take options
Expand Down
149 changes: 130 additions & 19 deletions qmail-remote.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@
#include "timeoutconn.h"
#include "timeoutread.h"
#include "timeoutwrite.h"
#include "hassmtputf8.h"
#ifdef SMTPUTF8
#include <idn2.h>
#include "utf8read.h"
#include "env.h"
#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


#define HUGESMTPTEXT 5000

Expand All @@ -47,6 +53,13 @@ saa reciplist = {0};

struct ip_address partner;

#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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

void out(s) char *s; { if (substdio_puts(subfdoutsmall,s) == -1) _exit(0); }
void zero() { if (substdio_put(subfdoutsmall,"\0",1) == -1) _exit(0); }
void zerodie() { zero(); substdio_flush(subfdoutsmall); _exit(0); }
Expand Down Expand Up @@ -118,36 +131,50 @@ substdio smtpfrom = SUBSTDIO_FDBUF(saferead,-1,smtpfrombuf,sizeof(smtpfrombuf));

stralloc smtptext = {0};

static void get(unsigned char *uc)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

static void get1(unsigned char *uc)
{
char *ch = (char *)uc;
substdio_get(&smtpfrom,ch,1);
if (*ch != '\r')
if (smtptext.len < HUGESMTPTEXT)
if (!stralloc_append(&smtptext,ch)) temp_nomem();
if (*ch != '\r' && smtptext.len < HUGESMTPTEXT &&
!stralloc_append(&smtptext,ch))
temp_nomem();
}

static unsigned long get3()
{
char str[4];
int i;
unsigned long code;

substdio_get(&smtpfrom,str,3);
str[3] = 0;
for (i = 0;i < 3;i++) {
if (str[i] == '\r') continue;
if (smtptext.len < HUGESMTPTEXT &&
!stralloc_append(&smtptext,str+i))
temp_nomem();
}
scan_ulong(str,&code);
return code;
}

unsigned long smtpcode()
{
unsigned char ch;
unsigned long code;
unsigned char ch;
unsigned long code;
int err = 0;

if (!stralloc_copys(&smtptext,"")) temp_nomem();

get(&ch); code = ch - '0';
get(&ch); code = code * 10 + (ch - '0');
get(&ch); code = code * 10 + (ch - '0');
if ((code = get3()) < 200) err = 1;
for (;;) {
get(&ch);
get1((char *)&ch);
if (ch != ' ' && ch != '-') err = 1;
if (ch != '-') break;
while (ch != '\n') get(&ch);
get(&ch);
get(&ch);
get(&ch);
while (ch != '\n') get1((char *)&ch);
if (get3() != code) err = 1;
}
while (ch != '\n') get(&ch);

return code;
while (ch != '\n') get1((char *)&ch);
return err ? 400 : code;
}

void outsmtptext()
Expand Down Expand Up @@ -211,6 +238,33 @@ void blast()
substdio_flush(&smtpto);
}

#ifdef SMTPUTF8
/*
* this function is general purpose
* we could use it when we add AUTH,
* STARTTLS, SIZE extensions
*/
int get_capability(const char *capa)
{
int i = 0, len;

/* e.g.
* 250-PIPELINING
* 250-8BITMIME
* 250-SIZE 10000000
* 250-ETRN
* 250-STARTTLS
* 250-SMTPUTF8
* 250 HELP
*/
len = str_len(capa);
for (i = 0; i < smtptext.len-len; ++i)
if (case_starts(smtptext.s+i,capa)) return 1;

return 0;
}
#endif

stralloc recip = {0};

void smtp()
Expand All @@ -221,14 +275,44 @@ void smtp()

if (smtpcode() != 220) quit("ZConnected to "," but greeting failed");

#ifdef SMTPUTF8
/*-
* this part of the code is general purpose
* it can be used for
* checking other extensions like
* AUTH, STARTTLS, SIZE, etc
*/
substdio_puts(&smtpto,"EHLO ");
substdio_put(&smtpto,helohost.s,helohost.len);
substdio_puts(&smtpto,"\r\n");
substdio_flush(&smtpto);

if (smtpcode() != 250) { /*- do a HELO if EHLO fails */
substdio_puts(&smtpto,"HELO ");
substdio_put(&smtpto,helohost.s,helohost.len);
substdio_puts(&smtpto,"\r\n");
substdio_flush(&smtpto);

code = smtpcode();
if (code >= 500) quit("DConnected to "," but my name was rejected");
if (code != 250) quit("ZConnected to "," but my name was rejected");
} else /* EHLO succeeded. Let's check SMTPUTF8 capa */
smtputf8 = get_capability("SMTPUTF8"); /*- did the remote server advertize SMTPUTF8 */
#else
substdio_puts(&smtpto,"HELO ");
substdio_put(&smtpto,helohost.s,helohost.len);
substdio_puts(&smtpto,"\r\n");
substdio_flush(&smtpto);
if (smtpcode() != 250) quit("ZConnected to "," but my name was rejected");
#endif

substdio_puts(&smtpto,"MAIL FROM:<");
substdio_put(&smtpto,sender.s,sender.len);
#ifdef SMTPUTF8
if (enable_utf8 && (flagutf8 || utf8read()))
substdio_puts(&smtpto,"> SMTPUTF8\r\n");
else
#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

substdio_puts(&smtpto,">\r\n");
substdio_flush(&smtpto);
code = smtpcode();
Expand Down Expand Up @@ -262,6 +346,9 @@ void smtp()
if (code >= 500) quit("D"," failed on DATA command");
if (code >= 400) quit("Z"," failed on DATA command");

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as previous

blast();
code = smtpcode();
flagcritical = 0;
Expand All @@ -276,7 +363,12 @@ stralloc canonbox = {0};
void addrmangle(stralloc *saout, char *s)
{
int j;


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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as previous

j = str_rchr(s,'@');
if (!s[j]) {
if (!stralloc_copys(saout,s)) temp_nomem();
Expand Down Expand Up @@ -325,6 +417,9 @@ int main(int argc, char **argv)
if (chdir(auto_qmail) == -1) temp_chdir();
getcontrols();

#ifdef SMTPUTF8
enable_utf8 = env_get("UTF8");
#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


if (!stralloc_copys(&host,argv[1])) temp_nomem();

Expand All @@ -342,6 +437,18 @@ int main(int argc, char **argv)
relayhost[i] = 0;
}
if (!stralloc_copys(&host,relayhost)) temp_nomem();
#ifdef SMTPUTF8
} else
if (enable_utf8) {
char *asciihost = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be NULL as it's a pointer variable

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}


Expand All @@ -361,7 +468,11 @@ int main(int argc, char **argv)


random = now() + (getpid() << 16);
#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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 472 is enough all by itself :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

case DNS_MEM: temp_nomem();
case DNS_SOFT: temp_dns();
case DNS_HARD: perm_dns();
Expand Down
9 changes: 9 additions & 0 deletions qmail-smtpd.8
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ must be supplied several environment variables;
see
.BR tcp-environ(5) .

If the environment variable
.B UTF8
is non-empty
.B qmail-smtpd
offers RFC 5336 SMTP Email Address Internationalization support and will advertize the
capability in the EHLO greeting. Since qmail-smtpd is 8 bit clean, setting of UTF8 has no real
consequences except for displaying this setting in the received headers as \fBUTF8(E)SMTP\fR.

.B qmail-smtpd
is responsible for counting hops.
It rejects any message with 100 or more
Expand All @@ -35,6 +43,7 @@ see
.B qmail-smtpd
accepts messages that contain long lines or non-ASCII characters,
even though such messages violate the SMTP protocol.

.SH "CONTROL FILES"
.TP 5
.I badmailfrom
Expand Down
Loading