From 34dbda4b991349af93863c9f82e810f2d5a3a2f1 Mon Sep 17 00:00:00 2001 From: micia Date: Tue, 2 Oct 2012 16:23:35 +0200 Subject: [PATCH 01/36] simplify SSL config flag semantics, default is having SSL disabled until crypto.c enables it --- crypto.c | 6 ++---- dma.h | 2 +- net.c | 11 +++-------- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/crypto.c b/crypto.c index 7e8865c..123a2ee 100644 --- a/crypto.c +++ b/crypto.c @@ -111,9 +111,7 @@ smtp_init_crypto(int fd, int feature) */ if (((feature & SECURETRANS) != 0) && (feature & STARTTLS) != 0) { - /* TLS init phase, disable SSL_write */ - config.features |= NOSSL; - + /* TLS init phase */ send_remote_command(fd, "EHLO %s", hostname()); if (read_remote(fd, 0, NULL) == 2) { send_remote_command(fd, "STARTTLS"); @@ -128,7 +126,7 @@ smtp_init_crypto(int fd, int feature) } } /* End of TLS init phase, enable SSL_write/read */ - config.features &= ~NOSSL; + config.features |= USESSL; } config.ssl = SSL_new(ctx); diff --git a/dma.h b/dma.h index 440a7a3..efe06d4 100644 --- a/dma.h +++ b/dma.h @@ -61,7 +61,7 @@ #define STARTTLS 0x002 /* StartTLS support */ #define SECURETRANS 0x004 /* SSL/TLS in general */ -#define NOSSL 0x008 /* Do not use SSL */ +#define USESSL 0x008 /* Use SSL for communication */ #define DEFER 0x010 /* Defer mails */ #define INSECURE 0x020 /* Allow plain login w/o encryption */ #define FULLBOUNCE 0x040 /* Bounce the full message */ diff --git a/net.c b/net.c index fa4a3a4..80ecc71 100644 --- a/net.c +++ b/net.c @@ -92,8 +92,7 @@ send_remote_command(int fd, const char* fmt, ...) strcat(cmd, "\r\n"); len = strlen(cmd); - if (((config.features & SECURETRANS) != 0) && - ((config.features & NOSSL) == 0)) { + if ((config.features & USESSL) != 0) { while ((s = SSL_write(config.ssl, (const char*)cmd, len)) <= 0) { s = SSL_get_error(config.ssl, s); if (s != SSL_ERROR_WANT_READ && @@ -145,8 +144,7 @@ read_remote(int fd, int extbufsize, char *extbuf) memmove(buff, buff + pos, len - pos); len -= pos; pos = 0; - if (((config.features & SECURETRANS) != 0) && - (config.features & NOSSL) == 0) { + if ((config.features & USESSL) != 0) { if ((rlen = SSL_read(config.ssl, buff + len, sizeof(buff) - len)) == -1) { strncpy(neterr, ssl_errstr(), sizeof(neterr)); goto error; @@ -339,7 +337,7 @@ close_connection(int fd) { if (config.ssl != NULL) { if (((config.features & SECURETRANS) != 0) && - ((config.features & NOSSL) == 0)) + ((config.features & USESSL) != 0)) SSL_shutdown(config.ssl); SSL_free(config.ssl); } @@ -381,10 +379,7 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) /* Check first reply from remote host */ if ((config.features & SECURETRANS) == 0 || (config.features & STARTTLS) != 0) { - config.features |= NOSSL; READ_REMOTE_CHECK("connect", 2); - - config.features &= ~NOSSL; } if ((config.features & SECURETRANS) != 0) { From 6574304c6a0ed396cb3baf52cd36e44b50f54c50 Mon Sep 17 00:00:00 2001 From: micia Date: Tue, 2 Oct 2012 17:09:23 +0200 Subject: [PATCH 02/36] simplify read_remote logic and prevent infinite loops due to pos == len, improve external buffer logic to report actual length to the caller, make it return the actual status code rather than status code / 100 --- crypto.c | 29 ++++++--- dma.h | 3 +- net.c | 189 +++++++++++++++++++++++++++---------------------------- 3 files changed, 115 insertions(+), 106 deletions(-) diff --git a/crypto.c b/crypto.c index 123a2ee..d8caa01 100644 --- a/crypto.c +++ b/crypto.c @@ -113,9 +113,9 @@ smtp_init_crypto(int fd, int feature) (feature & STARTTLS) != 0) { /* TLS init phase */ send_remote_command(fd, "EHLO %s", hostname()); - if (read_remote(fd, 0, NULL) == 2) { + if (read_remote(fd, NULL, NULL) == 250) { send_remote_command(fd, "STARTTLS"); - if (read_remote(fd, 0, NULL) != 2) { + if (read_remote(fd, NULL, NULL) != 220) { if ((feature & TLS_OPP) == 0) { syslog(LOG_ERR, "remote delivery deferred: STARTTLS not available: %s", neterr); return (1); @@ -254,22 +254,35 @@ smtp_auth_md5(int fd, char *login, char *password) char buffer[BUF_SIZE], ascii_digest[33]; char *temp; int len, i; + size_t buffsize = sizeof(buffer); static char hextab[] = "0123456789abcdef"; - - temp = calloc(BUF_SIZE, 1); + memset(buffer, 0, sizeof(buffer)); memset(digest, 0, sizeof(digest)); memset(ascii_digest, 0, sizeof(ascii_digest)); /* Send AUTH command according to RFC 2554 */ send_remote_command(fd, "AUTH CRAM-MD5"); - if (read_remote(fd, sizeof(buffer), buffer) != 3) { + if (read_remote(fd, &buffsize, buffer) != 334) { + /* if cram-md5 is not available */ syslog(LOG_DEBUG, "smarthost authentication:" " AUTH cram-md5 not available: %s", neterr); - /* if cram-md5 is not available */ - free(temp); return (-1); } + + if (buffsize > sizeof(buffer)) { + syslog(LOG_DEBUG, "smarthost authentication:" + " oversized response to AUTH CRAM-MD5"); + return (-1); + } + + /* allocate decoding buffer */ + temp = calloc(BUF_SIZE, 1); + if (!temp) { + syslog(LOG_WARNING, "remote delivery deferred:" + " memory allocation failed"); + return (-2); + } /* skip 3 char status + 1 char space */ base64_decode(buffer + 4, temp); @@ -296,7 +309,7 @@ smtp_auth_md5(int fd, char *login, char *password) /* send answer */ send_remote_command(fd, "%s", temp); free(temp); - if (read_remote(fd, 0, NULL) != 2) { + if (read_remote(fd, NULL, NULL) != 220) { syslog(LOG_WARNING, "remote delivery deferred:" " AUTH cram-md5 failed: %s", neterr); return (-2); diff --git a/dma.h b/dma.h index efe06d4..9766877 100644 --- a/dma.h +++ b/dma.h @@ -43,6 +43,7 @@ #include #include #include +#include /*size_t*/ #define VERSION "DragonFly Mail Agent " DMA_VERSION @@ -185,7 +186,7 @@ int dns_get_mx_list(const char *, int, struct mx_hostentry **, int); /* net.c */ char *ssl_errstr(void); -int read_remote(int, int, char *); +int read_remote(int, size_t *, char *); ssize_t send_remote_command(int, const char*, ...) __attribute__((__nonnull__(2), __format__ (__printf__, 2, 3))); int deliver_remote(struct qitem *); diff --git a/net.c b/net.c index 80ecc71..7429781 100644 --- a/net.c +++ b/net.c @@ -116,38 +116,44 @@ send_remote_command(int fd, const char* fmt, ...) } int -read_remote(int fd, int extbufsize, char *extbuf) +read_remote(int fd, size_t *extbufsize, char *extbuf) { ssize_t rlen = 0; - size_t pos, len, copysize; + size_t pos, len, copysize, ebufpos = 0, ebuflen = 0; char buff[BUF_SIZE]; - int done = 0, status = 0, status_running = 0, extbufpos = 0; - enum { parse_status, parse_spacedash, parse_rest } parsestate; + long status = 0; + int done = 0; if (do_timeout(CON_TIMEOUT, 1) != 0) { snprintf(neterr, sizeof(neterr), "Timeout reached"); return (-1); } - + + if (extbuf && extbufsize) { + ebuflen = *extbufsize; + } + /* * Remote reading code from femail.c written by Henning Brauer of * OpenBSD and released under a BSD style license. */ - len = 0; pos = 0; - parsestate = parse_status; + len = 0; neterr[0] = 0; - while (!(done && parsestate == parse_status)) { - rlen = 0; - if (pos == 0 || - (pos > 0 && memchr(buff + pos, '\n', len - pos) == NULL)) { + do { + if (memchr(buff + pos, '\n', len - pos) == NULL) { + rlen = 0; + + /* Move away already parsed data */ memmove(buff, buff + pos, len - pos); len -= pos; pos = 0; + + /* Read the next bytes */ if ((config.features & USESSL) != 0) { if ((rlen = SSL_read(config.ssl, buff + len, sizeof(buff) - len)) == -1) { - strncpy(neterr, ssl_errstr(), sizeof(neterr)); - goto error; + strncpy(neterr, ssl_errstr(), sizeof(neterr)); + goto error; } } else { if ((rlen = read(fd, buff + len, sizeof(buff) - len)) == -1) { @@ -155,87 +161,76 @@ read_remote(int fd, int extbufsize, char *extbuf) goto error; } } - len += rlen; - - copysize = sizeof(neterr) - strlen(neterr) - 1; - if (copysize > len) - copysize = len; - strncat(neterr, buff, copysize); - } - /* - * If there is an external buffer with a size bigger than zero - * and as long as there is space in the external buffer and - * there are new characters read from the mailserver - * copy them to the external buffer - */ - if (extbufpos <= (extbufsize - 1) && rlen > 0 && extbufsize > 0 && extbuf != NULL) { - /* do not write over the bounds of the buffer */ - if(extbufpos + rlen > (extbufsize - 1)) { - rlen = extbufsize - extbufpos; + + if (rlen == 0) { + strncpy(neterr, "unexpected connection end", sizeof(neterr)); + goto error; } - memcpy(extbuf + extbufpos, buff + len - rlen, rlen); - extbufpos += rlen; - } - - if (pos == len) - continue; - - switch (parsestate) { - case parse_status: - for (; pos < len; pos++) { - if (isdigit(buff[pos])) { - status_running = status_running * 10 + (buff[pos] - '0'); - } else { - status = status_running; - status_running = 0; - parsestate = parse_spacedash; - break; + + /* If an external buffer is available, copy data over there */ + if (ebuflen > 0) { + /* Do not write over the buffer bounds */ + ssize_t copysiz = rlen; + + if (ebufpos + copysiz > ebuflen) { + copysiz = ebuflen - ebufpos; + } + + if (copysiz > 0) { + memcpy(extbuf + ebufpos, buff + len, copysiz); } + + /* Add rlen to ebufpos, so the caller + * can know if the provided buffer was too small. + */ + ebufpos += rlen; } - continue; + } - case parse_spacedash: - switch (buff[pos]) { - case ' ': - done = 1; - break; + for (; pos < len && isdigit(buff[pos]); pos++) + ; /* Skip over status number */ - case '-': - /* ignore */ - /* XXX read capabilities */ - break; + if (pos == len) { + /* We need more data from the server */ + continue; + } - default: - strcpy(neterr, "invalid syntax in reply from server"); - goto error; - } + if (buff[pos] == ' ') { + done = 1; + } else if (buff[pos] == '-') { - pos++; - parsestate = parse_rest; - continue; + while (buff[pos++] != '\n') + ; /*skip*/ - case parse_rest: - /* skip up to \n */ - for (; pos < len; pos++) { - if (buff[pos] == '\n') { - pos++; - parsestate = parse_status; - break; - } - } + } else { + strncpy(neterr, "invalid syntax in reply from server", sizeof(neterr)); + goto error; } + + } while (!done); + status = strtol(buff, NULL, 10); + if (status < 100 || status > 999) { + strncpy(neterr, "error reading status, out of range value", sizeof(neterr)); + goto error; } do_timeout(0, 0); - /* chop off trailing newlines */ - while (neterr[0] != 0 && strchr("\r\n", neterr[strlen(neterr) - 1]) != 0) - neterr[strlen(neterr) - 1] = 0; - - return (status/100); + if (extbufsize) + *extbufsize = ebufpos; + + syslog(LOG_DEBUG, "<<< status %d", (int)status); + return ((int)status); error: + /* Ensure neterr '\0' ending byte*/ + neterr[sizeof(neterr) - 1] = '\0'; + + /* Chop off trailing newlines */ + while (neterr[0] != 0 && strchr("\r\n", neterr[strlen(neterr) - 1]) != 0) + neterr[strlen(neterr) - 1] = 0; + do_timeout(0, 0); return (-1); } @@ -264,7 +259,7 @@ smtp_login(int fd, char *login, char* password) (config.features & SECURETRANS) != 0) { /* Send AUTH command according to RFC 2554 */ send_remote_command(fd, "AUTH LOGIN"); - if (read_remote(fd, 0, NULL) != 3) { + if (read_remote(fd, NULL, NULL) != 334) { syslog(LOG_NOTICE, "remote delivery deferred:" " AUTH login not available: %s", neterr); @@ -280,11 +275,11 @@ smtp_login(int fd, char *login, char* password) send_remote_command(fd, "%s", temp); free(temp); - res = read_remote(fd, 0, NULL); - if (res != 3) { + res = read_remote(fd, NULL, NULL); + if (res != 334) { syslog(LOG_NOTICE, "remote delivery %s: AUTH login failed: %s", - res == 5 ? "failed" : "deferred", neterr); - return (res == 5 ? -1 : 1); + res == 503 ? "failed" : "deferred", neterr); + return (res == 503 ? -1 : 1); } len = base64_encode(password, strlen(password), &temp); @@ -293,11 +288,11 @@ smtp_login(int fd, char *login, char* password) send_remote_command(fd, "%s", temp); free(temp); - res = read_remote(fd, 0, NULL); - if (res != 2) { + res = read_remote(fd, NULL, NULL); + if (res != 235) { syslog(LOG_NOTICE, "remote delivery %s: Authentication failed: %s", - res == 5 ? "failed" : "deferred", neterr); - return (res == 5 ? -1 : 1); + res == 503 ? "failed" : "deferred", neterr); + return (res == 503 ? -1 : 1); } } else { syslog(LOG_WARNING, "non-encrypted SMTP login is disabled in config, so skipping it. "); @@ -363,8 +358,8 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) return (1); #define READ_REMOTE_CHECK(c, exp) \ - res = read_remote(fd, 0, NULL); \ - if (res == 5) { \ + res = read_remote(fd, NULL, NULL); \ + if (res == 500 || res == 502) { \ syslog(LOG_ERR, "remote delivery to %s [%s] failed after %s: %s", \ host->host, host->addr, c, neterr); \ snprintf(errmsg, sizeof(errmsg), "%s [%s] did not like our %s:\n%s", \ @@ -379,7 +374,7 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) /* Check first reply from remote host */ if ((config.features & SECURETRANS) == 0 || (config.features & STARTTLS) != 0) { - READ_REMOTE_CHECK("connect", 2); + READ_REMOTE_CHECK("connect", 220); } if ((config.features & SECURETRANS) != 0) { @@ -390,13 +385,13 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) goto out; if ((config.features & STARTTLS) == 0) - READ_REMOTE_CHECK("connect", 2); + READ_REMOTE_CHECK("connect", 250); } /* XXX allow HELO fallback */ /* XXX record ESMTP keywords */ send_remote_command(fd, "EHLO %s", hostname()); - READ_REMOTE_CHECK("EHLO", 2); + READ_REMOTE_CHECK("EHLO", 250); /* * Use SMTP authentication if the user defined an entry for the remote @@ -430,14 +425,14 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) /* XXX send ESMTP ENVID, RET (FULL/HDRS) and 8BITMIME */ send_remote_command(fd, "MAIL FROM:<%s>", it->sender); - READ_REMOTE_CHECK("MAIL FROM", 2); + READ_REMOTE_CHECK("MAIL FROM", 250); /* XXX send ESMTP ORCPT */ send_remote_command(fd, "RCPT TO:<%s>", it->addr); - READ_REMOTE_CHECK("RCPT TO", 2); + READ_REMOTE_CHECK("RCPT TO", 250); send_remote_command(fd, "DATA"); - READ_REMOTE_CHECK("DATA", 3); + READ_REMOTE_CHECK("DATA", 354); error = 0; while (!feof(it->mailf)) { @@ -469,10 +464,10 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) } send_remote_command(fd, "."); - READ_REMOTE_CHECK("final DATA", 2); + READ_REMOTE_CHECK("final DATA", 250); send_remote_command(fd, "QUIT"); - if (read_remote(fd, 0, NULL) != 2) + if (read_remote(fd, NULL, NULL) != 221) syslog(LOG_INFO, "remote delivery succeeded but QUIT failed: %s", neterr); out: From 0ead6c371d7ee551f269b6138bde8c9a081d1723 Mon Sep 17 00:00:00 2001 From: micia Date: Tue, 2 Oct 2012 23:13:05 +0200 Subject: [PATCH 03/36] rework yet again read_remote to be more secure, add HELO fallback and ESMTP aware functions, rework authentication mechanisms accordingly --- conf.c | 2 + crypto.c | 45 +++--- dma.conf | 5 + dma.h | 21 ++- net.c | 461 ++++++++++++++++++++++++++++++++++++++++++------------- 5 files changed, 395 insertions(+), 139 deletions(-) diff --git a/conf.c b/conf.c index 6e4eb25..4931dd6 100644 --- a/conf.c +++ b/conf.c @@ -219,6 +219,8 @@ parse_conf(const char *config_path) config.masquerade_user = user; } else if (strcmp(word, "STARTTLS") == 0 && data == NULL) config.features |= STARTTLS; + else if (strcmp(word, "NOHELO") == 0 && data == NULL) + config.features |= NOHELO; else if (strcmp(word, "OPPORTUNISTIC_TLS") == 0 && data == NULL) config.features |= TLS_OPP; else if (strcmp(word, "SECURETRANSFER") == 0 && data == NULL) diff --git a/crypto.c b/crypto.c index d8caa01..3b2a3bc 100644 --- a/crypto.c +++ b/crypto.c @@ -107,26 +107,17 @@ smtp_init_crypto(int fd, int feature) } /* - * If the user wants STARTTLS, we have to send EHLO here + * If STARTTLS is required, issue it here */ - if (((feature & SECURETRANS) != 0) && - (feature & STARTTLS) != 0) { + if ((feature & STARTTLS) != 0) { /* TLS init phase */ - send_remote_command(fd, "EHLO %s", hostname()); - if (read_remote(fd, NULL, NULL) == 250) { - send_remote_command(fd, "STARTTLS"); - if (read_remote(fd, NULL, NULL) != 220) { - if ((feature & TLS_OPP) == 0) { - syslog(LOG_ERR, "remote delivery deferred: STARTTLS not available: %s", neterr); - return (1); - } else { - syslog(LOG_INFO, "in opportunistic TLS mode, STARTTLS not available: %s", neterr); - return (0); - } - } + send_remote_command(fd, "STARTTLS"); + if (read_remote(fd, NULL, NULL) != 220) { + syslog(LOG_ERR, "remote delivery deferred: STARTTLS failed: %s", neterr); + return (1); } - /* End of TLS init phase, enable SSL_write/read */ - config.features |= USESSL; + + /* End of TLS init phase */ } config.ssl = SSL_new(ctx); @@ -162,7 +153,9 @@ smtp_init_crypto(int fd, int feature) ssl_errstr()); } X509_free(cert); - + + /* At this point we can safely use SSL write/read*/ + config.features |= USESSL; return (0); } @@ -266,14 +259,14 @@ smtp_auth_md5(int fd, char *login, char *password) if (read_remote(fd, &buffsize, buffer) != 334) { /* if cram-md5 is not available */ syslog(LOG_DEBUG, "smarthost authentication:" - " AUTH cram-md5 not available: %s", neterr); - return (-1); + " AUTH CRAM-MD5 failed: %s", neterr); + return (1); } if (buffsize > sizeof(buffer)) { syslog(LOG_DEBUG, "smarthost authentication:" " oversized response to AUTH CRAM-MD5"); - return (-1); + return (1); } /* allocate decoding buffer */ @@ -281,7 +274,7 @@ smtp_auth_md5(int fd, char *login, char *password) if (!temp) { syslog(LOG_WARNING, "remote delivery deferred:" " memory allocation failed"); - return (-2); + return (1); } /* skip 3 char status + 1 char space */ @@ -302,8 +295,8 @@ smtp_auth_md5(int fd, char *login, char *password) /* encode answer */ len = base64_encode(buffer, strlen(buffer), &temp); if (len < 0) { - syslog(LOG_ERR, "can not encode auth reply: %m"); - return (-1); + syslog(LOG_ERR, "cannot encode auth reply: %m"); + return (1); } /* send answer */ @@ -311,8 +304,8 @@ smtp_auth_md5(int fd, char *login, char *password) free(temp); if (read_remote(fd, NULL, NULL) != 220) { syslog(LOG_WARNING, "remote delivery deferred:" - " AUTH cram-md5 failed: %s", neterr); - return (-2); + " AUTH CRAM-MD5 failed: %s", neterr); + return (1); } return (0); diff --git a/dma.conf b/dma.conf index 56fa104..1724c97 100644 --- a/dma.conf +++ b/dma.conf @@ -25,6 +25,11 @@ # SECURETRANSFER) #STARTTLS +# Uncomment if you want to abort when the smarthost doesn't support EHLO, +# by default if EHLO is not supported dma falls back to HELO +# unless secure communication is required. +#NOHELO + # Uncomment if you have specified STARTTLS above and it should be allowed # to fail ("opportunistic TLS", use an encrypted connection when available # but allow an unencrypted one to servers that do not support it) diff --git a/dma.h b/dma.h index 9766877..3643374 100644 --- a/dma.h +++ b/dma.h @@ -48,6 +48,8 @@ #define VERSION "DragonFly Mail Agent " DMA_VERSION #define BUF_SIZE 2048 +#define ESMTPBUF_SIZE 8192 +#define ESMTPTOK_SIZE 1024 #define ERRMSG_SIZE 200 #define USERNAME_SIZE 50 #define MIN_RETRY 300 /* 5 minutes */ @@ -60,13 +62,18 @@ #define SMTP_PORT 25 /* Default SMTP port */ #define CON_TIMEOUT (5*60) /* Connection timeout per RFC5321 */ -#define STARTTLS 0x002 /* StartTLS support */ -#define SECURETRANS 0x004 /* SSL/TLS in general */ -#define USESSL 0x008 /* Use SSL for communication */ -#define DEFER 0x010 /* Defer mails */ -#define INSECURE 0x020 /* Allow plain login w/o encryption */ -#define FULLBOUNCE 0x040 /* Bounce the full message */ -#define TLS_OPP 0x080 /* Opportunistic STARTTLS */ +#define STARTTLS 0x0002 /* StartTLS support required by the user*/ +#define NOHELO 0x0004 /* Don't fallback to HELO if EHLO isn't supported*/ +#define SECURETRANS 0x0008 /* SSL/TLS in general */ +#define USESSL 0x0010 /* Use SSL for communication */ +#define DEFER 0x0020 /* Defer mails */ +#define INSECURE 0x0040 /* Allow plain login w/o encryption */ +#define FULLBOUNCE 0x0080 /* Bounce the full message */ +#define TLS_OPP 0x0100 /* Opportunistic STARTTLS */ +#define HASSTARTTLS 0x0100 /* STARTTLS advertised by the remote host */ +#define AUTHPLAIN 0x0200 /* PLAIN authentication method support */ +#define AUTHLOGIN 0x0400 /* LOGIN authentication method support */ +#define AUTHCRAMMD5 0x0800 /* CRAM MD5 authentication method support */ #ifndef CONF_PATH #error Please define CONF_PATH diff --git a/net.c b/net.c index 7429781..ea09ca6 100644 --- a/net.c +++ b/net.c @@ -92,6 +92,8 @@ send_remote_command(int fd, const char* fmt, ...) strcat(cmd, "\r\n"); len = strlen(cmd); + /*XXX: Verbose mode that logs on LOG_DEBUG sent command*/ + if ((config.features & USESSL) != 0) { while ((s = SSL_write(config.ssl, (const char*)cmd, len)) <= 0) { s = SSL_get_error(config.ssl, s); @@ -118,12 +120,12 @@ send_remote_command(int fd, const char* fmt, ...) int read_remote(int fd, size_t *extbufsize, char *extbuf) { - ssize_t rlen = 0; - size_t pos, len, copysize, ebufpos = 0, ebuflen = 0; + ssize_t pos = 0, len = 0, copysize = 0; + size_t ebufpos = 0, ebuflen = 0; char buff[BUF_SIZE]; - long status = 0; - int done = 0; - + int statnum = 0, currstatnum = 0, done = 0; + enum { PARSE_STATNUM, PARSE_DASH, PARSE_REST } parse = PARSE_STATNUM; + if (do_timeout(CON_TIMEOUT, 1) != 0) { snprintf(neterr, sizeof(neterr), "Timeout reached"); return (-1); @@ -137,100 +139,129 @@ read_remote(int fd, size_t *extbufsize, char *extbuf) * Remote reading code from femail.c written by Henning Brauer of * OpenBSD and released under a BSD style license. */ - pos = 0; - len = 0; neterr[0] = 0; do { - if (memchr(buff + pos, '\n', len - pos) == NULL) { - rlen = 0; - - /* Move away already parsed data */ - memmove(buff, buff + pos, len - pos); - len -= pos; + if (pos == len) { + /* no more data in buffer, read the next chunk */ pos = 0; /* Read the next bytes */ if ((config.features & USESSL) != 0) { - if ((rlen = SSL_read(config.ssl, buff + len, sizeof(buff) - len)) == -1) { + if ((len = SSL_read(config.ssl, buff, sizeof(buff))) == -1) { strncpy(neterr, ssl_errstr(), sizeof(neterr)); goto error; } } else { - if ((rlen = read(fd, buff + len, sizeof(buff) - len)) == -1) { + if ((len = read(fd, buff, sizeof(buff))) == -1) { strncpy(neterr, strerror(errno), sizeof(neterr)); goto error; } } - if (rlen == 0) { + if (len == 0) { strncpy(neterr, "unexpected connection end", sizeof(neterr)); goto error; } - + + /* Copy (at least partially) contents to an error buffer*/ + copysize = sizeof(neterr) - strlen(neterr) - 1; + if (copysize > len) + copysize = len; + + strncat(neterr, buff, copysize); + /* If an external buffer is available, copy data over there */ if (ebuflen > 0) { /* Do not write over the buffer bounds */ - ssize_t copysiz = rlen; + copysize = len; - if (ebufpos + copysiz > ebuflen) { - copysiz = ebuflen - ebufpos; + if (ebufpos + copysize > ebuflen) { + copysize = ebuflen - ebufpos; } - if (copysiz > 0) { - memcpy(extbuf + ebufpos, buff + len, copysiz); + if (copysize > 0) { + memcpy(extbuf + ebufpos, buff, copysize); } - /* Add rlen to ebufpos, so the caller + /* Add len to ebufpos, so the caller * can know if the provided buffer was too small. */ - ebufpos += rlen; + ebufpos += len; } } - - for (; pos < len && isdigit(buff[pos]); pos++) - ; /* Skip over status number */ - - if (pos == len) { - /* We need more data from the server */ - continue; - } - - if (buff[pos] == ' ') { - done = 1; - } else if (buff[pos] == '-') { - - while (buff[pos++] != '\n') - ; /*skip*/ - - } else { - strncpy(neterr, "invalid syntax in reply from server", sizeof(neterr)); + + /* since rlen can't be zero, we're sure that there is at least one character*/ + switch (parse) { + default: + /* shouldn't happen */ + syslog(LOG_CRIT, "reached dead code"); goto error; + case PARSE_STATNUM: + /* parse status number*/ + for (; pos < len; pos++) { + if (isdigit(buff[pos])) { + currstatnum = currstatnum * 10 + (buff[pos] - '0'); + } else { + statnum = currstatnum; + currstatnum = 0; + parse = PARSE_DASH; + break; + } + } + + break; + case PARSE_DASH: + /* parse dash, if space then we're done*/ + if (buff[pos] == ' ') { + done = 1; + } else if (buff[pos] == '-') { + /* ignore */ + /* XXX read capabilities */ + } else { + strncpy(neterr, "invalid syntax in reply from server", sizeof(neterr)); + goto error; + } + + pos++; + parse = PARSE_REST; + break; + case PARSE_REST: + /* parse to newline */ + for (; pos < len; pos++) { + if (buff[pos] == '\n') { + /* Skip the newline and expect a status number */ + pos++; + statnum = 0; + parse = PARSE_STATNUM; + break; + } + } + + break; } } while (!done); - status = strtol(buff, NULL, 10); - if (status < 100 || status > 999) { + if (statnum < 100 || statnum > 999) { strncpy(neterr, "error reading status, out of range value", sizeof(neterr)); goto error; } + + /* Ensure neterr null-termination*/ + neterr[sizeof(neterr) - 1] = 0; + /* Chop off trailing newlines */ + while (neterr[0] != 0 && strchr("\r\n", neterr[strlen(neterr) - 1]) != 0) + neterr[strlen(neterr) - 1] = 0; do_timeout(0, 0); if (extbufsize) *extbufsize = ebufpos; - syslog(LOG_DEBUG, "<<< status %d", (int)status); - return ((int)status); + /* XXX: Verbose mode that logs on LOG_DEBUG returned status */ + return (statnum); error: - /* Ensure neterr '\0' ending byte*/ - neterr[sizeof(neterr) - 1] = '\0'; - - /* Chop off trailing newlines */ - while (neterr[0] != 0 && strchr("\r\n", neterr[strlen(neterr) - 1]) != 0) - neterr[strlen(neterr) - 1] = 0; - do_timeout(0, 0); return (-1); } @@ -244,19 +275,18 @@ smtp_login(int fd, char *login, char* password) char *temp; int len, res = 0; - res = smtp_auth_md5(fd, login, password); - if (res == 0) { - return (0); - } else if (res == -2) { - /* - * If the return code is -2, then then the login attempt failed, - * do not try other login mechanisms - */ + if ((config.features & AUTHCRAMMD5) != 0) { + /* Use CRAM-MD5 authentication if available*/ + return smtp_auth_md5(fd, login, password); + } + + /* Try non-encrypted logins */ + if ((config.features & SECURETRANS) == 0 && (config.features & INSECURE) == 0) { + syslog(LOG_WARNING, "non-encrypted SMTP login is disabled in config, so skipping it"); return (1); } - - if ((config.features & INSECURE) != 0 || - (config.features & SECURETRANS) != 0) { + + if ((config.features & AUTHLOGIN) != 0) { /* Send AUTH command according to RFC 2554 */ send_remote_command(fd, "AUTH LOGIN"); if (read_remote(fd, NULL, NULL) != 334) { @@ -277,8 +307,8 @@ smtp_login(int fd, char *login, char* password) free(temp); res = read_remote(fd, NULL, NULL); if (res != 334) { - syslog(LOG_NOTICE, "remote delivery %s: AUTH login failed: %s", - res == 503 ? "failed" : "deferred", neterr); + syslog(LOG_NOTICE, "remote delivery %s: AUTH LOGIN failed: %s", + res == 503 ? "failed" : "deferred", neterr); return (res == 503 ? -1 : 1); } @@ -294,21 +324,162 @@ smtp_login(int fd, char *login, char* password) res == 503 ? "failed" : "deferred", neterr); return (res == 503 ? -1 : 1); } + + return (0); } else { - syslog(LOG_WARNING, "non-encrypted SMTP login is disabled in config, so skipping it. "); + /*XXX: Support PLAIN login as a fallback*/ + syslog(LOG_ERR, "no supported authentication method for remote host"); return (1); } +} + +static int +esmtp_nextline(char **buff, int skip) +{ + char *line = *buff; + long status; + + if (skip) { + /* Allow skipping to the next line */ + while (!isdigit(*line)) + line++; + } + + status = strtol(line, &line, 10); + if (status != 250) { + /*invalid status*/ + return -1; + } + + if (*line != '-') { + /*invalid syntax*/ + return -1; + } + + line++; + buff = &line; + return 0; +} - return (0); +static char * +esmtp_nexttoken(char **buff) +{ + char *line = *buff; + char *tok = line; + + if (isdigit(*line)) { + /* No more tokens available, + * we are on the next line of the ESMTP response. + */ + return NULL; + } + + /* make the line uppercase to honour RFC + * (tokens are parsed regardless their case) + */ + while (*line != '\n' && *line != ' ') { + *line = toupper(*line); + line++; + } + + /* null terminate and update the parser */ + *line++ = 0; + buff = &line; + return tok; } static int -open_connection(struct mx_hostentry *h) +esmtp_response(int fd) +{ + char buff[ESMTPBUF_SIZE]; + size_t buffsize = sizeof(buff); + char **parse; + char *tok; + int done; + int res; + + res = read_remote(fd, &buffsize, buff); + if (res != 250) { + return res; + } + + if (buffsize > sizeof(buff) || buff[buffsize - 1] != '\n') { + /*oversized or invalid buffer*/ + return -1; + } + + /* parse ESMP */ + parse = &buff; + done = 0; + + do { + if (esmtp_nextline(parse, 0) != 0) { + /* parsing error */ + return -1; + } + + tok = esmtp_nexttoken(parse); + if (!tok) { + /* shouldn't happen */ + return -1; + } + + if (strcmp(tok, "STARTTLS") == 0) { + /* STARTTLS is supported */ + config.features |= HASSTARTTLS; + } else if (strcmp(tok, "AUTH") == 0) { + /* retrieve supported authentication methods */ + while ((tok = esmtp_nexttoken(parse)) != NULL) { + if (strcmp(tok, "CRAM-MD5") == 0) + config.features |= AUTHCRAMMD5; + else if (strcmp(tok, "LOGIN") == 0) + config.features |= AUTHLOGIN; + else if (strcmp(tok, "PLAIN") == 0) + config.features |= AUTHPLAIN; + } + + } else if (strcmp(tok, "OK") == 0) { + /* done parsing */ + done = 1; + } else { + /* unknown or uninteresting feature, skip to newline*/ + if (esmtp_nextline(parse, 0) != 0) { + return -1; + } + } + + } while (!done); + + return res; +} + +static int +expect_response(int fd, const char *when, int exp) +{ + int res = read_remote(fd, NULL, NULL); + + if (res == 500 || res == 502) { + syslog(LOG_NOTICE, "remote delivery deferred: failed after %s: %s", when, neterr); + return (1); + } + + if (res != exp) { + syslog(LOG_ERR, "remote delivery failed after %s: %s", when, neterr); + snprintf(errmsg, sizeof(errmsg), "remote host did not like our %s:\n%s", when, neterr); + return (-1); + } + + return 0; +} + +static int +open_connection(struct mx_hostentry *h, int ehlo) { int fd; + int res; - syslog(LOG_INFO, "trying remote delivery to %s [%s] pref %d", - h->host, h->addr, h->pref); + syslog(LOG_INFO, "trying remote delivery to %s [%s] pref %d using %s", + h->host, h->addr, h->pref, ehlo? "EHLO" : "HELO"); fd = socket(h->ai.ai_family, h->ai.ai_socktype, h->ai.ai_protocol); if (fd < 0) { @@ -323,7 +494,48 @@ open_connection(struct mx_hostentry *h) close(fd); return (-1); } + + /* Check first reply from remote host */ + res = read_remote(fd, NULL, NULL); + if (res != 220) { + switch(res) { + case 421: + syslog(LOG_INFO, "connection rejected temporarily by remote host"); + break; + case 554: + syslog(LOG_INFO, "connection failed, remote host requires QUIT"); + send_remote_command(fd, "QUIT"); + break; + default: + syslog(LOG_INFO, "connection failed, remote host greeted us with %d", res); + break; + } + + close(fd); + return (-2); + } + + syslog(LOG_DEBUG, "connection accepted"); + if (ehlo) { + /* Try EHLO */ + send_remote_command(fd, "EHLO %s", hostname()); + res = esmtp_response(fd); + } else { + send_remote_command(fd, "HELO %s", hostname()); + res = read_remote(fd, NULL, NULL); + } + if (res != 250) { + if (res < 0) { + syslog(LOG_INFO, "connection failed, malformed response by remote host"); + } else { + syslog(LOG_INFO, "connection failed, remote host refused our greeting"); + } + + close(fd); + return -2; + } + return (fd); } @@ -353,46 +565,74 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) return (-1); } - fd = open_connection(host); - if (fd < 0) - return (1); - -#define READ_REMOTE_CHECK(c, exp) \ - res = read_remote(fd, NULL, NULL); \ - if (res == 500 || res == 502) { \ - syslog(LOG_ERR, "remote delivery to %s [%s] failed after %s: %s", \ - host->host, host->addr, c, neterr); \ - snprintf(errmsg, sizeof(errmsg), "%s [%s] did not like our %s:\n%s", \ - host->host, host->addr, c, neterr); \ - return (-1); \ - } else if (res != exp) { \ - syslog(LOG_NOTICE, "remote delivery deferred: %s [%s] failed after %s: %s", \ - host->host, host->addr, c, neterr); \ - return (1); \ + fd = open_connection(host, 1); + if (fd == -2) { + /* fallback to HELO if possible */ + if ((config.features & NOHELO) == 0) { + /* HELO disabled in config file */ + syslog(LOG_NOTICE, "remote delivery deferred:" + " EHLO unsupported by remote host and HELO fallback is disabled"); + return (1); + } + + if ((config.features & SECURETRANS) != 0 && (config.features & INSECURE) == 0) { + /* cannot fallback to HELO if secure connection is required */ + syslog(LOG_NOTICE, "remote delivery deferred:" + " ESMTP unsupported by remote host and secure connection is required"); + return (1); + } + + if ((config.features & STARTTLS) != 0 && (config.features & TLS_OPP) == 0) { + /* cannot fallback to HELO if STARTTLS is mandatory */ + syslog(LOG_NOTICE, "remote delivery deferred:" + " ESMTP unsupported by remote host and STARTTLS is required"); + return (1); + + } + + /*disable any security*/ + config.features &= ~(SECURETRANS | STARTTLS); + fd = open_connection(host, 0); } - - /* Check first reply from remote host */ - if ((config.features & SECURETRANS) == 0 || - (config.features & STARTTLS) != 0) { - READ_REMOTE_CHECK("connect", 220); + + if (fd < 0) { + /*connection failed*/ + return (1); + } + + if ((config.features & HASSTARTTLS) == 0 && (config.features & STARTTLS) != 0) { + if ((config.features & TLS_OPP) != 0) { + /* disable STARTTLS, opportunistic mode */ + syslog(LOG_INFO, "in opportunistic TLS mode, STARTTLS not available"); + config.features &= ~STARTTLS; + } else { + /* remote has no STARTTLS but user required it */ + syslog(LOG_ERR, "remote delivery deferred: STARTTLS not available"); + error = 1; + goto out; + } } if ((config.features & SECURETRANS) != 0) { + /* initialize secure transaction */ error = smtp_init_crypto(fd, config.features); if (error == 0) syslog(LOG_DEBUG, "SSL initialization successful"); else goto out; - - if ((config.features & STARTTLS) == 0) - READ_REMOTE_CHECK("connect", 250); + + /* refresh supported ESMTP features if STARTTLS was used*/ + if ((config.features & STARTTLS) != 0) { + send_remote_command(fd, "EHLO %s", hostname()); + res = esmtp_response(fd); + if (res != 250) { + /* shouldn't happen */ + syslog(LOG_NOTICE, "remote delivery deferred: EHLO after STARTTLS failed: %s", neterr); + error = 1; + goto out; + } + } } - - /* XXX allow HELO fallback */ - /* XXX record ESMTP keywords */ - send_remote_command(fd, "EHLO %s", hostname()); - READ_REMOTE_CHECK("EHLO", 250); - /* * Use SMTP authentication if the user defined an entry for the remote * or smarthost @@ -415,7 +655,7 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) syslog(LOG_ERR, "remote delivery failed:" " SMTP login failed: %m"); snprintf(errmsg, sizeof(errmsg), "SMTP login to %s failed", host->host); - return (-1); + goto out; } /* SMTP login is not available, so try without */ else if (error > 0) { @@ -425,14 +665,20 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) /* XXX send ESMTP ENVID, RET (FULL/HDRS) and 8BITMIME */ send_remote_command(fd, "MAIL FROM:<%s>", it->sender); - READ_REMOTE_CHECK("MAIL FROM", 250); + error = expect_response(fd, "MAIL FROM", 250); + if (error) + goto out; /* XXX send ESMTP ORCPT */ send_remote_command(fd, "RCPT TO:<%s>", it->addr); - READ_REMOTE_CHECK("RCPT TO", 250); + error = expect_response(fd, "RCPT TO", 250); + if (error) + goto out; send_remote_command(fd, "DATA"); - READ_REMOTE_CHECK("DATA", 354); + error = expect_response(fd, "DATA", 354); + if (error) + goto out; error = 0; while (!feof(it->mailf)) { @@ -464,7 +710,9 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) } send_remote_command(fd, "."); - READ_REMOTE_CHECK("final DATA", 250); + error = expect_response(fd, "final DATA", 250); + if (error) + goto out; send_remote_command(fd, "QUIT"); if (read_remote(fd, NULL, NULL) != 221) @@ -472,6 +720,7 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) out: close_connection(fd); + return (error); } From f22e7c54e2c9f985c005cb95603c83535822fd2d Mon Sep 17 00:00:00 2001 From: micia Date: Tue, 2 Oct 2012 23:23:07 +0200 Subject: [PATCH 04/36] add VERBOSE feature to debug network communication --- conf.c | 6 ++++-- dma.conf | 4 ++++ dma.h | 1 + net.c | 13 +++++++++++-- 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/conf.c b/conf.c index 4931dd6..c89c080 100644 --- a/conf.c +++ b/conf.c @@ -186,7 +186,7 @@ parse_conf(const char *config_path) data = strdup(data); else data = NULL; - + if (strcmp(word, "SMARTHOST") == 0 && data != NULL) config.smarthost = data; else if (strcmp(word, "PORT") == 0 && data != NULL) @@ -217,7 +217,9 @@ parse_conf(const char *config_path) user = NULL; config.masquerade_host = host; config.masquerade_user = user; - } else if (strcmp(word, "STARTTLS") == 0 && data == NULL) + } else if (strcmp(word, "VERBOSE") == 0 && data == NULL) + config.features |= VERBOSE; + else if (strcmp(word, "STARTTLS") == 0 && data == NULL) config.features |= STARTTLS; else if (strcmp(word, "NOHELO") == 0 && data == NULL) config.features |= NOHELO; diff --git a/dma.conf b/dma.conf index 1724c97..c3251bb 100644 --- a/dma.conf +++ b/dma.conf @@ -15,6 +15,10 @@ # Path to your spooldir. Just stay with the default. #SPOOLDIR /var/spool/dma +# Verbose output, uncomment if you want development debug output +# to LOG_DEBUG level on syslog +#VERBOSE + # SMTP authentication #AUTHPATH /etc/dma/auth.conf diff --git a/dma.h b/dma.h index 3643374..9a0abb0 100644 --- a/dma.h +++ b/dma.h @@ -62,6 +62,7 @@ #define SMTP_PORT 25 /* Default SMTP port */ #define CON_TIMEOUT (5*60) /* Connection timeout per RFC5321 */ +#define VERBOSE 0x0001 /* Enable debug logging output to LOG_DEBUG */ #define STARTTLS 0x0002 /* StartTLS support required by the user*/ #define NOHELO 0x0004 /* Don't fallback to HELO if EHLO isn't supported*/ #define SECURETRANS 0x0008 /* SSL/TLS in general */ diff --git a/net.c b/net.c index ea09ca6..da8a3f5 100644 --- a/net.c +++ b/net.c @@ -92,7 +92,8 @@ send_remote_command(int fd, const char* fmt, ...) strcat(cmd, "\r\n"); len = strlen(cmd); - /*XXX: Verbose mode that logs on LOG_DEBUG sent command*/ + if (config.features & VERBOSE) + syslog(LOG_DEBUG, ">>> %s", cmd); if ((config.features & USESSL) != 0) { while ((s = SSL_write(config.ssl, (const char*)cmd, len)) <= 0) { @@ -108,8 +109,13 @@ send_remote_command(int fd, const char* fmt, ...) pos = 0; while (pos < len) { n = write(fd, cmd + pos, len - pos); - if (n < 0) + if (n < 0) { + if (errno == EINTR) + continue; + return (-1); + } + pos += n; } } @@ -258,6 +264,9 @@ read_remote(int fd, size_t *extbufsize, char *extbuf) if (extbufsize) *extbufsize = ebufpos; + if (config.features & VERBOSE) + syslog(LOG_DEBUG, "<<< %d", statnum); + /* XXX: Verbose mode that logs on LOG_DEBUG returned status */ return (statnum); From aab5fbc0ef1febddd0d1041004b1e7bbb16b1b03 Mon Sep 17 00:00:00 2001 From: micia Date: Tue, 2 Oct 2012 23:44:13 +0200 Subject: [PATCH 05/36] various bugfixes to VERBOSE logging and ESMTP parsing --- net.c | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/net.c b/net.c index da8a3f5..047a39d 100644 --- a/net.c +++ b/net.c @@ -88,12 +88,12 @@ send_remote_command(int fd, const char* fmt, ...) return (-1); } + if (config.features & VERBOSE) + syslog(LOG_DEBUG, ">>> %s", cmd); + /* We *know* there are at least two more bytes available */ strcat(cmd, "\r\n"); len = strlen(cmd); - - if (config.features & VERBOSE) - syslog(LOG_DEBUG, ">>> %s", cmd); if ((config.features & USESSL) != 0) { while ((s = SSL_write(config.ssl, (const char*)cmd, len)) <= 0) { @@ -186,7 +186,7 @@ read_remote(int fd, size_t *extbufsize, char *extbuf) } if (copysize > 0) { - memcpy(extbuf + ebufpos, buff, copysize); + memcpy(extbuf + ebufpos, buff, copysize); } /* Add len to ebufpos, so the caller @@ -350,7 +350,7 @@ esmtp_nextline(char **buff, int skip) if (skip) { /* Allow skipping to the next line */ - while (!isdigit(*line)) + while (*line != '\0' && !isdigit(*line)) line++; } @@ -366,7 +366,7 @@ esmtp_nextline(char **buff, int skip) } line++; - buff = &line; + *buff = line; return 0; } @@ -386,15 +386,24 @@ esmtp_nexttoken(char **buff) /* make the line uppercase to honour RFC * (tokens are parsed regardless their case) */ - while (*line != '\n' && *line != ' ') { + while (*line != '\0' && *line != '\r' && *line != '\n' && *line != ' ') { *line = toupper(*line); line++; } /* null terminate and update the parser */ - *line++ = 0; - buff = &line; - return tok; + if (*line == '\r') { + *line++ = 0; + } + if (*line == '\n') { + *line++ = 0; + } + if (*line == ' ') { + *line++ = 0; + } + + *buff = line; + return (*line != '\0'? tok : NULL); } static int @@ -403,6 +412,7 @@ esmtp_response(int fd) char buff[ESMTPBUF_SIZE]; size_t buffsize = sizeof(buff); char **parse; + char *esmtp; char *tok; int done; int res; @@ -412,13 +422,14 @@ esmtp_response(int fd) return res; } - if (buffsize > sizeof(buff) || buff[buffsize - 1] != '\n') { + if (buffsize > sizeof(buff)) { /*oversized or invalid buffer*/ return -1; } - + /* parse ESMP */ - parse = &buff; + esmtp = buff; + parse = &esmtp; done = 0; do { From d508bdbc2984e47d321721a10183dc53811d54e0 Mon Sep 17 00:00:00 2001 From: micia Date: Tue, 2 Oct 2012 23:44:44 +0200 Subject: [PATCH 06/36] fix buggy boolean guard --- net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net.c b/net.c index 047a39d..57b1599 100644 --- a/net.c +++ b/net.c @@ -588,7 +588,7 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) fd = open_connection(host, 1); if (fd == -2) { /* fallback to HELO if possible */ - if ((config.features & NOHELO) == 0) { + if ((config.features & NOHELO) != 0) { /* HELO disabled in config file */ syslog(LOG_NOTICE, "remote delivery deferred:" " EHLO unsupported by remote host and HELO fallback is disabled"); From 31fb2f43c30c108ed4bf798ff1cbc268d0611302 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 00:33:09 +0200 Subject: [PATCH 07/36] fix ESMTP parsing --- net.c | 69 +++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 41 insertions(+), 28 deletions(-) diff --git a/net.c b/net.c index 57b1599..7527e41 100644 --- a/net.c +++ b/net.c @@ -349,22 +349,42 @@ esmtp_nextline(char **buff, int skip) long status; if (skip) { - /* Allow skipping to the next line */ - while (*line != '\0' && !isdigit(*line)) + /* Allow skipping to the next line, + * possible scenarios are: + * - the parser is already on a newline + * when it deleted '\r' and is currently over + * a '\n' character. + * - the parser is in a token inside a line, + * happens when it deleted ' ' and is over + * the next token of a line. + */ + while (*line != '\0' && *line != '\n') + line++; + + if (*line == '\n') { line++; + } } + /* now we expect the status number*/ status = strtol(line, &line, 10); if (status != 250) { /*invalid status*/ return -1; } + if (*line == ' ') { + /* signal end of parse with a positive number */ + *buff = line; + return 1; + } + if (*line != '-') { /*invalid syntax*/ return -1; } + /* success */ line++; *buff = line; return 0; @@ -376,7 +396,7 @@ esmtp_nexttoken(char **buff) char *line = *buff; char *tok = line; - if (isdigit(*line)) { + if (*line == '\n' || *line == '\0') { /* No more tokens available, * we are on the next line of the ESMTP response. */ @@ -386,7 +406,7 @@ esmtp_nexttoken(char **buff) /* make the line uppercase to honour RFC * (tokens are parsed regardless their case) */ - while (*line != '\0' && *line != '\r' && *line != '\n' && *line != ' ') { + while (*line != '\0' && *line != '\r' && *line != ' ') { *line = toupper(*line); line++; } @@ -395,15 +415,12 @@ esmtp_nexttoken(char **buff) if (*line == '\r') { *line++ = 0; } - if (*line == '\n') { - *line++ = 0; - } if (*line == ' ') { *line++ = 0; } *buff = line; - return (*line != '\0'? tok : NULL); + return tok; } static int @@ -414,7 +431,7 @@ esmtp_response(int fd) char **parse; char *esmtp; char *tok; - int done; + int error; int res; res = read_remote(fd, &buffsize, buff); @@ -427,23 +444,20 @@ esmtp_response(int fd) return -1; } - /* parse ESMP */ + /* initialize ESMTP parsing */ esmtp = buff; parse = &esmtp; - done = 0; - - do { - if (esmtp_nextline(parse, 0) != 0) { - /* parsing error */ - return -1; - } - + error = esmtp_nextline(parse, 0); + while (error == 0) { tok = esmtp_nexttoken(parse); if (!tok) { - /* shouldn't happen */ + /* shouldn't happen, return parse error */ return -1; } + if ((config.features & VERBOSE) != 0) + syslog(LOG_DEBUG, "ESMTP got %s", tok); + if (strcmp(tok, "STARTTLS") == 0) { /* STARTTLS is supported */ config.features |= HASSTARTTLS; @@ -458,17 +472,16 @@ esmtp_response(int fd) config.features |= AUTHPLAIN; } - } else if (strcmp(tok, "OK") == 0) { - /* done parsing */ - done = 1; - } else { - /* unknown or uninteresting feature, skip to newline*/ - if (esmtp_nextline(parse, 0) != 0) { - return -1; - } } - } while (!done); + /*position over next line*/ + error = esmtp_nextline(parse, 1); + + } + + /*return a negative number on parsing error*/ + if (error < 0) + res = -1; return res; } From e5e6bc96d3420843b1a9f244e30bcb853d2b024f Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 00:39:15 +0200 Subject: [PATCH 08/36] deliver_host simplify and make mail read loop more reliable --- net.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/net.c b/net.c index 7527e41..ded8687 100644 --- a/net.c +++ b/net.c @@ -714,9 +714,7 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) goto out; error = 0; - while (!feof(it->mailf)) { - if (fgets(line, sizeof(line), it->mailf) == NULL) - break; + while (fgets(line, sizeof(line), it->mailf)) { linelen = strlen(line); if (linelen == 0 || line[linelen - 1] != '\n') { syslog(LOG_CRIT, "remote delivery failed: corrupted queue file"); @@ -741,6 +739,12 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) goto out; } } + + if (!feof(it->mailf) || ferror(it->mailf)) { + syslog(LOG_NOTICE, "remote delivery deferred: I/O read error"); + error = 1; + goto out; + } send_remote_command(fd, "."); error = expect_response(fd, "final DATA", 250); From f5cc3fa7d3ce51508a3ff57d05d6512db62628ce Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 00:44:45 +0200 Subject: [PATCH 09/36] add VERBOSE option warning to config file --- dma.conf | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dma.conf b/dma.conf index c3251bb..7f1500d 100644 --- a/dma.conf +++ b/dma.conf @@ -16,7 +16,9 @@ #SPOOLDIR /var/spool/dma # Verbose output, uncomment if you want development debug output -# to LOG_DEBUG level on syslog +# to LOG_DEBUG level on syslog. Use this option with extreme caution, +# it might log sensible data to the system logger, always keep it +# disabled unless your know what you are doing #VERBOSE # SMTP authentication From e8c66f26be265a7e5e9dbeb3e56ad1be6046c18b Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 02:50:01 +0200 Subject: [PATCH 10/36] Improve configuration file routines and port parsing Rework routines to perform basic checking for I/O errors and improve logic to check for meaningful configuration, user defined ports are now validated and stored in unsigned integers for maximum portability. --- conf.c | 46 ++++++++++++++++++++++++++++++++++++---------- net.c | 5 ++--- 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/conf.c b/conf.c index c89c080..df61332 100644 --- a/conf.c +++ b/conf.c @@ -97,6 +97,7 @@ parse_authfile(const char *path) struct authuser *au; FILE *a; char *data; + int error; int lineno = 0; a = fopen(path, "r"); @@ -105,9 +106,7 @@ parse_authfile(const char *path) /* NOTREACHED */ } - while (!feof(a)) { - if (fgets(line, sizeof(line), a) == NULL) - break; + while (fgets(line, sizeof(line), a)) { lineno++; chomp(line); @@ -138,8 +137,14 @@ parse_authfile(const char *path) SLIST_INSERT_HEAD(&authusers, au, next); } - + + error = ferror(a); fclose(a); + + if (error) { + errlog(1, "I/O error while reading file `%s'", path); + /* NOTREACHED */ + } } /* @@ -153,6 +158,7 @@ parse_conf(const char *config_path) char *data; FILE *conf; char line[2048]; + int error; int lineno = 0; conf = fopen(config_path, "r"); @@ -164,9 +170,7 @@ parse_conf(const char *config_path) /* NOTREACHED */ } - while (!feof(conf)) { - if (fgets(line, sizeof(line), conf) == NULL) - break; + while (fgets(line, sizeof(line), conf)) lineno++; chomp(line); @@ -189,9 +193,17 @@ parse_conf(const char *config_path) if (strcmp(word, "SMARTHOST") == 0 && data != NULL) config.smarthost = data; - else if (strcmp(word, "PORT") == 0 && data != NULL) - config.port = atoi(data); - else if (strcmp(word, "ALIASES") == 0 && data != NULL) + else if (strcmp(word, "PORT") == 0 && data != NULL) { + char*check; + long port = strtol(data, &check, 10); + + if (*check != '\0' || port < 0 || port > 0xffff) { + errlogx(1, "invalid value for PORT in %s:%d", config_path, lineno); + /* NOTREACHED */ + } + + config.port = (int)port; + } else if (strcmp(word, "ALIASES") == 0 && data != NULL) config.aliases = data; else if (strcmp(word, "SPOOLDIR") == 0 && data != NULL) config.spooldir = data; @@ -239,5 +251,19 @@ parse_conf(const char *config_path) } } + error = ferror(conf); fclose(conf); + + if (error) { + errlog(1, "I/O error while reading file `%s'", config_path); + /* NOTREACHED */ + } + + /* ensure a meaningful configuration */ + if ((config.features & STARTTLS) != 0) { + if ((config.features & SECURTRANS) == 0) { + syslog(LOG_WARN, "STARTTLS enabled in `%s', implicitly assuming SECURETRANSFER is enabled"); + config.features |= SECURETRANS; + } + } } diff --git a/net.c b/net.c index ded8687..06fe452 100644 --- a/net.c +++ b/net.c @@ -740,7 +740,7 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) } } - if (!feof(it->mailf) || ferror(it->mailf)) { + if (ferror(it->mailf)) { syslog(LOG_NOTICE, "remote delivery deferred: I/O read error"); error = 1; goto out; @@ -757,7 +757,6 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) out: close_connection(fd); - return (error); } @@ -766,7 +765,7 @@ deliver_remote(struct qitem *it) { struct mx_hostentry *hosts, *h; const char *host; - int port; + unsigned int port; int error = 1, smarthost = 0; host = strrchr(it->addr, '@'); From 2fca7236d4a4b033a954a0ef67658ef5163e6967 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 03:11:11 +0200 Subject: [PATCH 11/36] Fix some issues with format and simplify a condition USESSL implies SECURETRANS so the condition is redundant. --- conf.c | 8 ++++---- dma.h | 4 ++-- dns.c | 6 +++--- net.c | 4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/conf.c b/conf.c index df61332..426df31 100644 --- a/conf.c +++ b/conf.c @@ -170,7 +170,7 @@ parse_conf(const char *config_path) /* NOTREACHED */ } - while (fgets(line, sizeof(line), conf)) + while (fgets(line, sizeof(line), conf)) { lineno++; chomp(line); @@ -202,7 +202,7 @@ parse_conf(const char *config_path) /* NOTREACHED */ } - config.port = (int)port; + config.port = (unsigned int)port; } else if (strcmp(word, "ALIASES") == 0 && data != NULL) config.aliases = data; else if (strcmp(word, "SPOOLDIR") == 0 && data != NULL) @@ -261,8 +261,8 @@ parse_conf(const char *config_path) /* ensure a meaningful configuration */ if ((config.features & STARTTLS) != 0) { - if ((config.features & SECURTRANS) == 0) { - syslog(LOG_WARN, "STARTTLS enabled in `%s', implicitly assuming SECURETRANSFER is enabled"); + if ((config.features & SECURETRANS) == 0) { + syslog(LOG_WARNING, "STARTTLS enabled in `%s', implicitly assuming SECURETRANSFER is enabled", config_path); config.features |= SECURETRANS; } } diff --git a/dma.h b/dma.h index 9a0abb0..f1c8dcf 100644 --- a/dma.h +++ b/dma.h @@ -130,7 +130,7 @@ struct queue { struct config { const char *smarthost; - int port; + unsigned int port; /* should be unsigned for maximum compatibility (16 bit ints) */ const char *aliases; const char *spooldir; const char *authpath; @@ -190,7 +190,7 @@ int smtp_auth_md5(int, char *, char *); int smtp_init_crypto(int, int); /* dns.c */ -int dns_get_mx_list(const char *, int, struct mx_hostentry **, int); +int dns_get_mx_list(const char *, unsigned int, struct mx_hostentry **, int); /* net.c */ char *ssl_errstr(void); diff --git a/dns.c b/dns.c index fc5213f..394ea59 100644 --- a/dns.c +++ b/dns.c @@ -61,7 +61,7 @@ sort_pref(const void *a, const void *b) } static int -add_host(int pref, const char *host, int port, struct mx_hostentry **he, size_t *ps) +add_host(int pref, const char *host, unsigned int port, struct mx_hostentry **he, size_t *ps) { struct addrinfo hints, *res, *res0 = NULL; char servname[10]; @@ -74,7 +74,7 @@ add_host(int pref, const char *host, int port, struct mx_hostentry **he, size_t hints.ai_socktype = SOCK_STREAM; hints.ai_protocol = IPPROTO_TCP; - snprintf(servname, sizeof(servname), "%d", port); + snprintf(servname, sizeof(servname), "%u", port); err = getaddrinfo(host, servname, &hints, &res0); if (err) return (err == EAI_AGAIN ? 1 : -1); @@ -111,7 +111,7 @@ add_host(int pref, const char *host, int port, struct mx_hostentry **he, size_t } int -dns_get_mx_list(const char *host, int port, struct mx_hostentry **he, int no_mx) +dns_get_mx_list(const char *host, unsigned int port, struct mx_hostentry **he, int no_mx) { char outname[MAXDNAME]; ns_msg msg; diff --git a/net.c b/net.c index 06fe452..71bcf4d 100644 --- a/net.c +++ b/net.c @@ -576,9 +576,9 @@ static void close_connection(int fd) { if (config.ssl != NULL) { - if (((config.features & SECURETRANS) != 0) && - ((config.features & USESSL) != 0)) + if ((config.features & USESSL) != 0) SSL_shutdown(config.ssl); + SSL_free(config.ssl); } From 7a190860cd8f8df8f5c15f192ad64ea691b1646c Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 03:44:03 +0200 Subject: [PATCH 12/36] Make config file parsing more solid Makes the config file parser ignore trailing and ending whitespaces, leading to the expected results while parsing badly formatted files. Also makes the chomp() function deal with the comments directly, to have consistend behaviour across every configuration file. --- conf.c | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/conf.c b/conf.c index 426df31..f3c014e 100644 --- a/conf.c +++ b/conf.c @@ -38,6 +38,7 @@ #include #include #include +#include #include #include @@ -74,12 +75,34 @@ trim_line(char *line) static void chomp(char *str) { + char *p; + size_t i; size_t len = strlen(str); + /* remove trailing spaces */ + for (i = 0; i < len; i++) { + if (!isspace(str[i])) + break; + } + + memmove(str, str + i, len + 1 - i); + len -= i; if (len == 0) return; - if (str[len - 1] == '\n') - str[len - 1] = 0; + + /* remove ending spaces (also handles ending '\n', if any) */ + while (len-- > 0) { + if (!isspace(str[len])) + break; + } + + str[len + 1] = 0; + + /* remove comments */ + p = strchr(str, '#'); + if (p) { + *p = 0; + } } /* @@ -88,7 +111,7 @@ chomp(char *str) * file format is: * user|host:password * - * A line starting with # is treated as comment and ignored. + * Anything following a # is treated as comment and ignored. */ void parse_authfile(const char *path) @@ -111,9 +134,6 @@ parse_authfile(const char *path) chomp(line); - /* We hit a comment */ - if (*line == '#') - continue; /* Ignore empty lines */ if (*line == 0) continue; @@ -175,10 +195,6 @@ parse_conf(const char *config_path) chomp(line); - /* We hit a comment */ - if (strchr(line, '#')) - *strchr(line, '#') = 0; - data = line; word = strsep(&data, EQS); From 76e082175335bddac445e0a3fd190c75be6bd4d9 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 13:08:51 +0200 Subject: [PATCH 13/36] Improve HELO fallback and ESMTP capabilities refresh Make HELO fallback possible only if no authentication was supplied by the user, also fix a bug in ESMTP flags refreshing (actually drop old features) --- dma.h | 17 +++++++++-------- net.c | 49 +++++++++++++++++++++++++++++-------------------- 2 files changed, 38 insertions(+), 28 deletions(-) diff --git a/dma.h b/dma.h index f1c8dcf..ab619a8 100644 --- a/dma.h +++ b/dma.h @@ -63,18 +63,19 @@ #define CON_TIMEOUT (5*60) /* Connection timeout per RFC5321 */ #define VERBOSE 0x0001 /* Enable debug logging output to LOG_DEBUG */ -#define STARTTLS 0x0002 /* StartTLS support required by the user*/ -#define NOHELO 0x0004 /* Don't fallback to HELO if EHLO isn't supported*/ -#define SECURETRANS 0x0008 /* SSL/TLS in general */ -#define USESSL 0x0010 /* Use SSL for communication */ -#define DEFER 0x0020 /* Defer mails */ -#define INSECURE 0x0040 /* Allow plain login w/o encryption */ -#define FULLBOUNCE 0x0080 /* Bounce the full message */ -#define TLS_OPP 0x0100 /* Opportunistic STARTTLS */ +#define STARTTLS 0x0002 /* StartTLS support required by the user*/ +#define NOHELO 0x0004 /* Don't fallback to HELO if EHLO isn't supported*/ +#define SECURETRANS 0x0008 /* SSL/TLS in general */ +#define USESSL 0x0010 /* Use SSL for communication */ +#define DEFER 0x0020 /* Defer mails */ +#define INSECURE 0x0040 /* Allow plain login w/o encryption */ +#define FULLBOUNCE 0x0080 /* Bounce the full message */ +#define TLS_OPP 0x0100 /* Opportunistic STARTTLS */ #define HASSTARTTLS 0x0100 /* STARTTLS advertised by the remote host */ #define AUTHPLAIN 0x0200 /* PLAIN authentication method support */ #define AUTHLOGIN 0x0400 /* LOGIN authentication method support */ #define AUTHCRAMMD5 0x0800 /* CRAM MD5 authentication method support */ +#define ESMTPMASK (HASSTARTTLS | AUTHPLAIN | AUTHLOGIN | AUTHCRAMMD5) #ifndef CONF_PATH #error Please define CONF_PATH diff --git a/net.c b/net.c index 71bcf4d..3740f2d 100644 --- a/net.c +++ b/net.c @@ -300,7 +300,7 @@ smtp_login(int fd, char *login, char* password) send_remote_command(fd, "AUTH LOGIN"); if (read_remote(fd, NULL, NULL) != 334) { syslog(LOG_NOTICE, "remote delivery deferred:" - " AUTH login not available: %s", + " AUTH LOGIN was refused: %s", neterr); return (1); } @@ -597,6 +597,17 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) snprintf(errmsg, sizeof(errmsg), "can not seek: %s", strerror(errno)); return (-1); } + + /* + * Use SMTP authentication if the user defined an entry for the remote + * or smarthost + */ + SLIST_FOREACH(a, &authusers, next) { + if (strcmp(a->host, host->host) == 0) { + do_auth = 1; + break; + } + } fd = open_connection(host, 1); if (fd == -2) { @@ -608,6 +619,13 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) return (1); } + if (do_auth) { + /* cannot fallback to HELO, authentication is required */ + syslog(LOG_NOTICE, "remote delivery deferred:" + " EHLO unsupported by remote host and authentication is required"); + return (1); + } + if ((config.features & SECURETRANS) != 0 && (config.features & INSECURE) == 0) { /* cannot fallback to HELO if secure connection is required */ syslog(LOG_NOTICE, "remote delivery deferred:" @@ -656,28 +674,19 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) /* refresh supported ESMTP features if STARTTLS was used*/ if ((config.features & STARTTLS) != 0) { - send_remote_command(fd, "EHLO %s", hostname()); - res = esmtp_response(fd); - if (res != 250) { - /* shouldn't happen */ - syslog(LOG_NOTICE, "remote delivery deferred: EHLO after STARTTLS failed: %s", neterr); - error = 1; - goto out; - } - } - } - /* - * Use SMTP authentication if the user defined an entry for the remote - * or smarthost - */ - SLIST_FOREACH(a, &authusers, next) { - if (strcmp(a->host, host->host) == 0) { - do_auth = 1; - break; + config.features &= ~ESMTPMASK; + send_remote_command(fd, "EHLO %s", hostname()); + res = esmtp_response(fd); + if (res != 250) { + /* shouldn't happen */ + syslog(LOG_NOTICE, "remote delivery deferred: EHLO after STARTTLS failed: %s", neterr); + error = 1; + goto out; + } } } - if (do_auth == 1) { + if (do_auth) { /* * Check if the user wants plain text login without using * encryption. From c917cec36c16b02e17d5a5f21b28b57d20e3d0f5 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 13:11:11 +0200 Subject: [PATCH 14/36] Drop entirely the no login fallback If server advertises the login possibility and the user provides us with authentication credential, dma should use them or fail. --- net.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/net.c b/net.c index 3740f2d..b140619 100644 --- a/net.c +++ b/net.c @@ -693,16 +693,12 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) */ syslog(LOG_INFO, "using SMTP authentication for user %s", a->login); error = smtp_login(fd, a->login, a->password); - if (error < 0) { + if (error) { syslog(LOG_ERR, "remote delivery failed:" " SMTP login failed: %m"); snprintf(errmsg, sizeof(errmsg), "SMTP login to %s failed", host->host); goto out; } - /* SMTP login is not available, so try without */ - else if (error > 0) { - syslog(LOG_WARNING, "SMTP login not available. Trying without."); - } } /* XXX send ESMTP ENVID, RET (FULL/HDRS) and 8BITMIME */ From bf5941f737b95fb5828a834a461b8cf1420a5a7b Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 13:23:59 +0200 Subject: [PATCH 15/36] Improve message on I/O error --- net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net.c b/net.c index b140619..2cc1994 100644 --- a/net.c +++ b/net.c @@ -746,7 +746,7 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) } if (ferror(it->mailf)) { - syslog(LOG_NOTICE, "remote delivery deferred: I/O read error"); + syslog(LOG_NOTICE, "remote delivery deferred: I/O read error, %m"); error = 1; goto out; } From 005a00720a14d7134d1317f029e8bc6ffd1fc579 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 13:28:54 +0200 Subject: [PATCH 16/36] Replace SECURETRANS with the more specific USESSL (probably SECURETRANS is redundant) --- net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net.c b/net.c index 2cc1994..19153f5 100644 --- a/net.c +++ b/net.c @@ -290,7 +290,7 @@ smtp_login(int fd, char *login, char* password) } /* Try non-encrypted logins */ - if ((config.features & SECURETRANS) == 0 && (config.features & INSECURE) == 0) { + if ((config.features & USESSL) == 0 && (config.features & INSECURE) == 0) { syslog(LOG_WARNING, "non-encrypted SMTP login is disabled in config, so skipping it"); return (1); } From 1ae3e6bed4967f989b133688b634b809c8f11d35 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 14:08:06 +0200 Subject: [PATCH 17/36] First implementation of the PLAIN authentication method --- net.c | 45 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/net.c b/net.c index 19153f5..8cfa81c 100644 --- a/net.c +++ b/net.c @@ -307,7 +307,6 @@ smtp_login(int fd, char *login, char* password) len = base64_encode(login, strlen(login), &temp); if (len < 0) { -encerr: syslog(LOG_ERR, "can not encode auth reply: %m"); return (1); } @@ -322,21 +321,57 @@ smtp_login(int fd, char *login, char* password) } len = base64_encode(password, strlen(password), &temp); - if (len < 0) - goto encerr; + if (len < 0) { + syslog(LOG_ERR, "can not encode auth reply: %m"); + return (1); + } send_remote_command(fd, "%s", temp); free(temp); res = read_remote(fd, NULL, NULL); if (res != 235) { - syslog(LOG_NOTICE, "remote delivery %s: Authentication failed: %s", + syslog(LOG_NOTICE, "remote delivery %s: authentication failed: %s", res == 503 ? "failed" : "deferred", neterr); return (res == 503 ? -1 : 1); } + return (0); + } else if ((config.features & AUTHPLAIN) != 0) { + /* PLAIN login (single string with authority, authetication and password, + * if no authority is provided the SMTP server will derive it from authentication. + */ + char *buff; + + len = strlen(login) + strlen(password) + 2; + buff = calloc(len, 1); + if (!buff) { + syslog(LOG_NOTICE, "remote delivery deferred: memory allocation failure"); + return (1); + } + + strcpy(buff, login); + strcpy(buff + strlen(login) + 1, password); + + len = base64_encode(buff, len, &temp); + free(buff); + + if (len < 0) { + syslog(LOG_ERR, "can not encode auth reply: %m"); + return (1); + } + + send_remote_command(fd, "%s", temp); + free(temp); + res = read_remote(fd, NULL, NULL); + if (res != 235) { + syslog(LOG_NOTICE, "remote delivery deferred: authentication failed: %s", + neterr); + return (1); + } + return (0); } else { - /*XXX: Support PLAIN login as a fallback*/ + /* No supported authentication method */ syslog(LOG_ERR, "no supported authentication method for remote host"); return (1); } From 460cc2db5d0e763140f3cd386f8c84a57b49e7e7 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 14:47:45 +0200 Subject: [PATCH 18/36] More read_remote improvements, to error handling and parsing Do not let read_remote tolerate invalid status numbers anywhere in the message, also unify error handling and ensure null byte termination for any string. --- net.c | 66 +++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/net.c b/net.c index 8cfa81c..83651c5 100644 --- a/net.c +++ b/net.c @@ -127,18 +127,23 @@ int read_remote(int fd, size_t *extbufsize, char *extbuf) { ssize_t pos = 0, len = 0, copysize = 0; - size_t ebufpos = 0, ebuflen = 0; + size_t ebufpos = 0, ebufmax = 0; char buff[BUF_SIZE]; int statnum = 0, currstatnum = 0, done = 0; enum { PARSE_STATNUM, PARSE_DASH, PARSE_REST } parse = PARSE_STATNUM; if (do_timeout(CON_TIMEOUT, 1) != 0) { - snprintf(neterr, sizeof(neterr), "Timeout reached"); - return (-1); + strncpy(neterr, "timeout reached", sizeof(neterr)); + statnum = -1; + goto timeout; } - if (extbuf && extbufsize) { - ebuflen = *extbufsize; + if (extbufsize) { + /*if no extbuf is provided interpret the call as a "peek" to the size*/ + ebufmax = extbuf? *extbufsize : 0; + /*always leave room for ending null byte*/ + if (ebufmax) + ebufmax--; } /* @@ -166,6 +171,7 @@ read_remote(int fd, size_t *extbufsize, char *extbuf) if (len == 0) { strncpy(neterr, "unexpected connection end", sizeof(neterr)); + statnum = -1; goto error; } @@ -177,23 +183,24 @@ read_remote(int fd, size_t *extbufsize, char *extbuf) strncat(neterr, buff, copysize); /* If an external buffer is available, copy data over there */ - if (ebuflen > 0) { + if (ebufmax > 0) { /* Do not write over the buffer bounds */ copysize = len; - if (ebufpos + copysize > ebuflen) { - copysize = ebuflen - ebufpos; + if (ebufpos + copysize > ebufmax) { + copysize = ebufmax - ebufpos; } if (copysize > 0) { memcpy(extbuf + ebufpos, buff, copysize); + extbuf[ebufpos + copysize] = 0; } - - /* Add len to ebufpos, so the caller - * can know if the provided buffer was too small. - */ - ebufpos += len; } + + /* Add len to ebufpos, so the caller + * can know if the provided buffer was too small. + */ + ebufpos += len; } /* since rlen can't be zero, we're sure that there is at least one character*/ @@ -201,6 +208,7 @@ read_remote(int fd, size_t *extbufsize, char *extbuf) default: /* shouldn't happen */ syslog(LOG_CRIT, "reached dead code"); + statnum = -1; goto error; case PARSE_STATNUM: /* parse status number*/ @@ -208,6 +216,13 @@ read_remote(int fd, size_t *extbufsize, char *extbuf) if (isdigit(buff[pos])) { currstatnum = currstatnum * 10 + (buff[pos] - '0'); } else { + /* verify and store status */ + if (currstatnum < 100 || currstatnum > 999) { + strncpy(neterr, "error reading status, out of range value", sizeof(neterr)); + statnum = -1; + goto error; + } + statnum = currstatnum; currstatnum = 0; parse = PARSE_DASH; @@ -225,6 +240,7 @@ read_remote(int fd, size_t *extbufsize, char *extbuf) /* XXX read capabilities */ } else { strncpy(neterr, "invalid syntax in reply from server", sizeof(neterr)); + statnum = -1; goto error; } @@ -237,7 +253,6 @@ read_remote(int fd, size_t *extbufsize, char *extbuf) if (buff[pos] == '\n') { /* Skip the newline and expect a status number */ pos++; - statnum = 0; parse = PARSE_STATNUM; break; } @@ -247,32 +262,25 @@ read_remote(int fd, size_t *extbufsize, char *extbuf) } } while (!done); - - if (statnum < 100 || statnum > 999) { - strncpy(neterr, "error reading status, out of range value", sizeof(neterr)); - goto error; - } + if (config.features & VERBOSE) + syslog(LOG_DEBUG, "<<< %d", statnum); + +error: + /* Disable timeout */ + do_timeout(0, 0); + +timeout: /* Ensure neterr null-termination*/ neterr[sizeof(neterr) - 1] = 0; /* Chop off trailing newlines */ while (neterr[0] != 0 && strchr("\r\n", neterr[strlen(neterr) - 1]) != 0) neterr[strlen(neterr) - 1] = 0; - do_timeout(0, 0); - if (extbufsize) *extbufsize = ebufpos; - if (config.features & VERBOSE) - syslog(LOG_DEBUG, "<<< %d", statnum); - - /* XXX: Verbose mode that logs on LOG_DEBUG returned status */ return (statnum); - -error: - do_timeout(0, 0); - return (-1); } /* From 1148a3d5fa0194b0527c27c6f16d76a5d1a056c6 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 14:51:27 +0200 Subject: [PATCH 19/36] Minor style fix --- net.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/net.c b/net.c index 83651c5..22d46ca 100644 --- a/net.c +++ b/net.c @@ -140,10 +140,13 @@ read_remote(int fd, size_t *extbufsize, char *extbuf) if (extbufsize) { /*if no extbuf is provided interpret the call as a "peek" to the size*/ - ebufmax = extbuf? *extbufsize : 0; + if (extbuf) { + ebufmax = *extbufsize; + } /*always leave room for ending null byte*/ - if (ebufmax) + if (ebufmax) { ebufmax--; + } } /* From 06621befeffd7601283ac9f059707666a520e5ca Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 15:41:35 +0200 Subject: [PATCH 20/36] Update man page, minor style fix in config file. --- dma.8 | 20 +++++++++++++++++++- dma.conf | 6 +++--- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/dma.8 b/dma.8 index b121cd0..43096e3 100644 --- a/dma.8 +++ b/dma.8 @@ -29,7 +29,7 @@ .\" OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.Dd April 22, 2010 +.Dd October 03, 2012 .Dt DMA 8 .Os .Sh NAME @@ -212,6 +212,16 @@ Just stick with the default. Path to the .Sq auth.conf file. +.It Ic VERBOSE Xo +(boolean, default=commented) +.Xc +Uncomment to turn on verbose development debug output, disabled by default +for security reasons. Use this option with extreme caution, always leave this +option disabled unless you are aware of what you are doing. +When in debug mode +.Nm +is going to log any network communication, including possibly sensitive +data and authentication credentials, to the system logger. .It Ic SECURETRANS Xo (boolean, default=commented) .Xc @@ -222,6 +232,14 @@ Uncomment if you want TLS/SSL secured transfer. Uncomment if you want to use STARTTLS. Only useful together with .Sq SECURETRANS . +.It Ic NOHELO Xo +(boolean, default commented) +.Xc +Uncomment this option to disable HELO fallback when extended +SMTP features are not supported by the remote host. By default +.Nm +falls back to HELO if possible, unless authentication or +secure connection are required. .It Ic OPPORTUNISTIC_TLS Xo (boolean, default=commented) .Xc diff --git a/dma.conf b/dma.conf index 7f1500d..cd00a69 100644 --- a/dma.conf +++ b/dma.conf @@ -15,15 +15,15 @@ # Path to your spooldir. Just stay with the default. #SPOOLDIR /var/spool/dma +# SMTP authentication +#AUTHPATH /etc/dma/auth.conf + # Verbose output, uncomment if you want development debug output # to LOG_DEBUG level on syslog. Use this option with extreme caution, # it might log sensible data to the system logger, always keep it # disabled unless your know what you are doing #VERBOSE -# SMTP authentication -#AUTHPATH /etc/dma/auth.conf - # Uncomment if yout want TLS/SSL support #SECURETRANSFER From c7e813db5bd8107751a440bc637b993178d2a5be Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 15:46:31 +0200 Subject: [PATCH 21/36] Fix typo in man page. --- dma.8 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dma.8 b/dma.8 index 43096e3..194317d 100644 --- a/dma.8 +++ b/dma.8 @@ -233,7 +233,7 @@ Uncomment if you want to use STARTTLS. Only useful together with .Sq SECURETRANS . .It Ic NOHELO Xo -(boolean, default commented) +(boolean, default=commented) .Xc Uncomment this option to disable HELO fallback when extended SMTP features are not supported by the remote host. By default From b7977a86cede6ded4d7370d28ebd5f2f9eb0d610 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 15:57:06 +0200 Subject: [PATCH 22/36] Fix error message errno could be zero leading to a confusing message like: SMTP login failed: success --- net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net.c b/net.c index 22d46ca..3cead26 100644 --- a/net.c +++ b/net.c @@ -741,7 +741,7 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) error = smtp_login(fd, a->login, a->password); if (error) { syslog(LOG_ERR, "remote delivery failed:" - " SMTP login failed: %m"); + " SMTP login failed"); snprintf(errmsg, sizeof(errmsg), "SMTP login to %s failed", host->host); goto out; } From c899ca318cc3e39a9f36f6a695f50341c4011a91 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 16:18:58 +0200 Subject: [PATCH 23/36] Avoid mixed declaration and code. --- util.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/util.c b/util.c index a139b20..09dfcfe 100644 --- a/util.c +++ b/util.c @@ -277,6 +277,9 @@ do_timeout(int timeout, int dojmp) int open_locked(const char *fname, int flags, ...) { +#ifndef O_EXLOCK + int fd, save_errno; +#endif int mode = 0; if (flags & O_CREAT) { @@ -287,8 +290,6 @@ open_locked(const char *fname, int flags, ...) } #ifndef O_EXLOCK - int fd, save_errno; - fd = open(fname, flags, mode); if (fd < 0) return(fd); From faca7524ce785c7346ac047ca939c9625411bce3 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 19:00:51 +0200 Subject: [PATCH 24/36] Make connection structure to avoid modifying global data Remote connection depends on global status, which is unreliable and unpredictable, encapsulate any connection configuration into a struct connection and use only this one for communication, this way any delivery will be easily manageable and the struct config will be read only across the process. --- crypto.c | 55 ++++++++++------ dma.h | 27 ++++---- net.c | 186 +++++++++++++++++++++++++------------------------------ 3 files changed, 137 insertions(+), 131 deletions(-) diff --git a/crypto.c b/crypto.c index 3b2a3bc..59b3bbc 100644 --- a/crypto.c +++ b/crypto.c @@ -77,7 +77,7 @@ init_cert_file(SSL_CTX *ctx, const char *path) } int -smtp_init_crypto(int fd, int feature) +smtp_init_crypto(struct connection *c) { SSL_CTX *ctx = NULL; const SSL_METHOD *meth = NULL; @@ -109,29 +109,44 @@ smtp_init_crypto(int fd, int feature) /* * If STARTTLS is required, issue it here */ - if ((feature & STARTTLS) != 0) { + if ((config.features & STARTTLS) != 0) { /* TLS init phase */ - send_remote_command(fd, "STARTTLS"); - if (read_remote(fd, NULL, NULL) != 220) { - syslog(LOG_ERR, "remote delivery deferred: STARTTLS failed: %s", neterr); - return (1); + if ((c->flags & HASSTARTTLS) != 0) { + send_remote_command(c, "STARTTLS"); + if (read_remote(c, NULL, NULL) != 220) { + /* even in opportunistic TLS, if server marked it as available, an error + * is unexpected + */ + syslog(LOG_ERR, "remote delivery deferred: STARTTLS failed: %s", neterr); + return (1); + } + + /* End of TLS init phase */ + c->flags |= USESTARTTLS; + } else { + if ((config.features & TLS_OPP) == 0) { + /* remote has no STARTTLS but user required it */ + syslog(LOG_ERR, "remote delivery deferred: STARTTLS not available"); + return (1); + } + + /* disable STARTTLS, opportunistic mode */ + syslog(LOG_INFO, "in opportunistic TLS mode, STARTTLS not available"); } - - /* End of TLS init phase */ } - config.ssl = SSL_new(ctx); - if (config.ssl == NULL) { + c->ssl = SSL_new(ctx); + if (c->ssl == NULL) { syslog(LOG_NOTICE, "remote delivery deferred: SSL struct creation failed: %s", ssl_errstr()); return (1); } /* Set ssl to work in client mode */ - SSL_set_connect_state(config.ssl); + SSL_set_connect_state(c->ssl); /* Set fd for SSL in/output */ - error = SSL_set_fd(config.ssl, fd); + error = SSL_set_fd(c->ssl, c->fd); if (error == 0) { syslog(LOG_NOTICE, "remote delivery deferred: SSL set fd failed: %s", ssl_errstr()); @@ -139,7 +154,7 @@ smtp_init_crypto(int fd, int feature) } /* Open SSL connection */ - error = SSL_connect(config.ssl); + error = SSL_connect(c->ssl); if (error < 0) { syslog(LOG_ERR, "remote delivery deferred: SSL handshake failed fatally: %s", ssl_errstr()); @@ -147,7 +162,7 @@ smtp_init_crypto(int fd, int feature) } /* Get peer certificate */ - cert = SSL_get_peer_certificate(config.ssl); + cert = SSL_get_peer_certificate(c->ssl); if (cert == NULL) { syslog(LOG_WARNING, "remote delivery deferred: Peer did not provide certificate: %s", ssl_errstr()); @@ -155,7 +170,7 @@ smtp_init_crypto(int fd, int feature) X509_free(cert); /* At this point we can safely use SSL write/read*/ - config.features |= USESSL; + c->flags |= USESSL; return (0); } @@ -241,7 +256,7 @@ hmac_md5(unsigned char *text, int text_len, unsigned char *key, int key_len, * CRAM-MD5 authentication */ int -smtp_auth_md5(int fd, char *login, char *password) +smtp_auth_md5(struct connection *c, char *login, char *password) { unsigned char digest[BUF_SIZE]; char buffer[BUF_SIZE], ascii_digest[33]; @@ -255,8 +270,8 @@ smtp_auth_md5(int fd, char *login, char *password) memset(ascii_digest, 0, sizeof(ascii_digest)); /* Send AUTH command according to RFC 2554 */ - send_remote_command(fd, "AUTH CRAM-MD5"); - if (read_remote(fd, &buffsize, buffer) != 334) { + send_remote_command(c, "AUTH CRAM-MD5"); + if (read_remote(c, &buffsize, buffer) != 334) { /* if cram-md5 is not available */ syslog(LOG_DEBUG, "smarthost authentication:" " AUTH CRAM-MD5 failed: %s", neterr); @@ -300,9 +315,9 @@ smtp_auth_md5(int fd, char *login, char *password) } /* send answer */ - send_remote_command(fd, "%s", temp); + send_remote_command(c, "%s", temp); free(temp); - if (read_remote(fd, NULL, NULL) != 220) { + if (read_remote(c, NULL, NULL) != 220) { syslog(LOG_WARNING, "remote delivery deferred:" " AUTH CRAM-MD5 failed: %s", neterr); return (1); diff --git a/dma.h b/dma.h index ab619a8..b1ba8a1 100644 --- a/dma.h +++ b/dma.h @@ -66,16 +66,10 @@ #define STARTTLS 0x0002 /* StartTLS support required by the user*/ #define NOHELO 0x0004 /* Don't fallback to HELO if EHLO isn't supported*/ #define SECURETRANS 0x0008 /* SSL/TLS in general */ -#define USESSL 0x0010 /* Use SSL for communication */ #define DEFER 0x0020 /* Defer mails */ #define INSECURE 0x0040 /* Allow plain login w/o encryption */ #define FULLBOUNCE 0x0080 /* Bounce the full message */ #define TLS_OPP 0x0100 /* Opportunistic STARTTLS */ -#define HASSTARTTLS 0x0100 /* STARTTLS advertised by the remote host */ -#define AUTHPLAIN 0x0200 /* PLAIN authentication method support */ -#define AUTHLOGIN 0x0400 /* LOGIN authentication method support */ -#define AUTHCRAMMD5 0x0800 /* CRAM MD5 authentication method support */ -#define ESMTPMASK (HASSTARTTLS | AUTHPLAIN | AUTHLOGIN | AUTHCRAMMD5) #ifndef CONF_PATH #error Please define CONF_PATH @@ -140,9 +134,20 @@ struct config { const char *mailname; const char *masquerade_host; const char *masquerade_user; +}; + +#define USESSL 0x0001 /* Use SSL for communication */ +#define USESTARTTLS 0x0002 /* Has performed STARTTLS */ +#define HASSTARTTLS 0x0004 /* STARTTLS advertised by the remote host */ +#define AUTHPLAIN 0x0008 /* PLAIN authentication method support */ +#define AUTHLOGIN 0x0010 /* LOGIN authentication method support */ +#define AUTHCRAMMD5 0x0020 /* CRAM MD5 authentication method support */ +#define ESMTPMASK (HASSTARTTLS | AUTHPLAIN | AUTHLOGIN | AUTHCRAMMD5) - /* XXX does not belong into config */ +struct connection { + int fd; SSL *ssl; + int flags; }; @@ -187,16 +192,16 @@ void parse_authfile(const char *); /* crypto.c */ void hmac_md5(unsigned char *, int, unsigned char *, int, unsigned char *); -int smtp_auth_md5(int, char *, char *); -int smtp_init_crypto(int, int); +int smtp_auth_md5(struct connection *, char *, char *) __attribute__((__nonnull__(1, 2, 3))); +int smtp_init_crypto(struct connection *) __attribute__((__nonnull__(1))); /* dns.c */ int dns_get_mx_list(const char *, unsigned int, struct mx_hostentry **, int); /* net.c */ char *ssl_errstr(void); -int read_remote(int, size_t *, char *); -ssize_t send_remote_command(int, const char*, ...) __attribute__((__nonnull__(2), __format__ (__printf__, 2, 3))); +int read_remote(struct connection *, size_t *, char *) __attribute__((__nonnull__(1))); +ssize_t send_remote_command(struct connection *, const char*, ...) __attribute__((__nonnull__(1, 2), __format__ (__printf__, 2, 3))); int deliver_remote(struct qitem *); /* base64.c */ diff --git a/net.c b/net.c index 3cead26..64e47ee 100644 --- a/net.c +++ b/net.c @@ -72,7 +72,7 @@ ssl_errstr(void) } ssize_t -send_remote_command(int fd, const char* fmt, ...) +send_remote_command(struct connection *c, const char* fmt, ...) { va_list va; char cmd[4096]; @@ -95,9 +95,9 @@ send_remote_command(int fd, const char* fmt, ...) strcat(cmd, "\r\n"); len = strlen(cmd); - if ((config.features & USESSL) != 0) { - while ((s = SSL_write(config.ssl, (const char*)cmd, len)) <= 0) { - s = SSL_get_error(config.ssl, s); + if ((c->flags & USESSL) != 0) { + while ((s = SSL_write(c->ssl, (const char*)cmd, len)) <= 0) { + s = SSL_get_error(c->ssl, s); if (s != SSL_ERROR_WANT_READ && s != SSL_ERROR_WANT_WRITE) { strncpy(neterr, ssl_errstr(), sizeof(neterr)); @@ -108,7 +108,7 @@ send_remote_command(int fd, const char* fmt, ...) else { pos = 0; while (pos < len) { - n = write(fd, cmd + pos, len - pos); + n = write(c->fd, cmd + pos, len - pos); if (n < 0) { if (errno == EINTR) continue; @@ -124,7 +124,7 @@ send_remote_command(int fd, const char* fmt, ...) } int -read_remote(int fd, size_t *extbufsize, char *extbuf) +read_remote(struct connection *c, size_t *extbufsize, char *extbuf) { ssize_t pos = 0, len = 0, copysize = 0; size_t ebufpos = 0, ebufmax = 0; @@ -160,13 +160,13 @@ read_remote(int fd, size_t *extbufsize, char *extbuf) pos = 0; /* Read the next bytes */ - if ((config.features & USESSL) != 0) { - if ((len = SSL_read(config.ssl, buff, sizeof(buff))) == -1) { + if ((c->flags & USESSL) != 0) { + if ((len = SSL_read(c->ssl, buff, sizeof(buff))) == -1) { strncpy(neterr, ssl_errstr(), sizeof(neterr)); goto error; } } else { - if ((len = read(fd, buff, sizeof(buff))) == -1) { + if ((len = read(c->fd, buff, sizeof(buff))) == -1) { strncpy(neterr, strerror(errno), sizeof(neterr)); goto error; } @@ -290,26 +290,26 @@ read_remote(int fd, size_t *extbufsize, char *extbuf) * Handle SMTP authentication */ static int -smtp_login(int fd, char *login, char* password) +smtp_login(struct connection *c, char *login, char* password) { char *temp; int len, res = 0; - if ((config.features & AUTHCRAMMD5) != 0) { + if ((c->flags & AUTHCRAMMD5) != 0) { /* Use CRAM-MD5 authentication if available*/ - return smtp_auth_md5(fd, login, password); + return smtp_auth_md5(c, login, password); } /* Try non-encrypted logins */ - if ((config.features & USESSL) == 0 && (config.features & INSECURE) == 0) { + if ((c->flags & USESSL) == 0 && (config.features & INSECURE) == 0) { syslog(LOG_WARNING, "non-encrypted SMTP login is disabled in config, so skipping it"); return (1); } - if ((config.features & AUTHLOGIN) != 0) { + if ((c->flags & AUTHLOGIN) != 0) { /* Send AUTH command according to RFC 2554 */ - send_remote_command(fd, "AUTH LOGIN"); - if (read_remote(fd, NULL, NULL) != 334) { + send_remote_command(c, "AUTH LOGIN"); + if (read_remote(c, NULL, NULL) != 334) { syslog(LOG_NOTICE, "remote delivery deferred:" " AUTH LOGIN was refused: %s", neterr); @@ -322,9 +322,9 @@ smtp_login(int fd, char *login, char* password) return (1); } - send_remote_command(fd, "%s", temp); + send_remote_command(c, "%s", temp); free(temp); - res = read_remote(fd, NULL, NULL); + res = read_remote(c, NULL, NULL); if (res != 334) { syslog(LOG_NOTICE, "remote delivery %s: AUTH LOGIN failed: %s", res == 503 ? "failed" : "deferred", neterr); @@ -337,9 +337,9 @@ smtp_login(int fd, char *login, char* password) return (1); } - send_remote_command(fd, "%s", temp); + send_remote_command(c, "%s", temp); free(temp); - res = read_remote(fd, NULL, NULL); + res = read_remote(c, NULL, NULL); if (res != 235) { syslog(LOG_NOTICE, "remote delivery %s: authentication failed: %s", res == 503 ? "failed" : "deferred", neterr); @@ -347,7 +347,7 @@ smtp_login(int fd, char *login, char* password) } return (0); - } else if ((config.features & AUTHPLAIN) != 0) { + } else if ((c->flags & AUTHPLAIN) != 0) { /* PLAIN login (single string with authority, authetication and password, * if no authority is provided the SMTP server will derive it from authentication. */ @@ -371,9 +371,9 @@ smtp_login(int fd, char *login, char* password) return (1); } - send_remote_command(fd, "%s", temp); + send_remote_command(c, "%s", temp); free(temp); - res = read_remote(fd, NULL, NULL); + res = read_remote(c, NULL, NULL); if (res != 235) { syslog(LOG_NOTICE, "remote delivery deferred: authentication failed: %s", neterr); @@ -470,7 +470,7 @@ esmtp_nexttoken(char **buff) } static int -esmtp_response(int fd) +esmtp_response(struct connection *c) { char buff[ESMTPBUF_SIZE]; size_t buffsize = sizeof(buff); @@ -480,7 +480,7 @@ esmtp_response(int fd) int error; int res; - res = read_remote(fd, &buffsize, buff); + res = read_remote(c, &buffsize, buff); if (res != 250) { return res; } @@ -506,23 +506,22 @@ esmtp_response(int fd) if (strcmp(tok, "STARTTLS") == 0) { /* STARTTLS is supported */ - config.features |= HASSTARTTLS; + c->flags |= HASSTARTTLS; } else if (strcmp(tok, "AUTH") == 0) { /* retrieve supported authentication methods */ while ((tok = esmtp_nexttoken(parse)) != NULL) { if (strcmp(tok, "CRAM-MD5") == 0) - config.features |= AUTHCRAMMD5; + c->flags |= AUTHCRAMMD5; else if (strcmp(tok, "LOGIN") == 0) - config.features |= AUTHLOGIN; + c->flags |= AUTHLOGIN; else if (strcmp(tok, "PLAIN") == 0) - config.features |= AUTHPLAIN; + c->flags |= AUTHPLAIN; } } /*position over next line*/ error = esmtp_nextline(parse, 1); - } /*return a negative number on parsing error*/ @@ -533,9 +532,9 @@ esmtp_response(int fd) } static int -expect_response(int fd, const char *when, int exp) +expect_response(struct connection *c, const char *when, int exp) { - int res = read_remote(fd, NULL, NULL); + int res = read_remote(c, NULL, NULL); if (res == 500 || res == 502) { syslog(LOG_NOTICE, "remote delivery deferred: failed after %s: %s", when, neterr); @@ -552,30 +551,30 @@ expect_response(int fd, const char *when, int exp) } static int -open_connection(struct mx_hostentry *h, int ehlo) +open_connection(struct connection*c, struct mx_hostentry *h, int ehlo) { - int fd; int res; syslog(LOG_INFO, "trying remote delivery to %s [%s] pref %d using %s", h->host, h->addr, h->pref, ehlo? "EHLO" : "HELO"); - - fd = socket(h->ai.ai_family, h->ai.ai_socktype, h->ai.ai_protocol); - if (fd < 0) { + + memset(c, 0, sizeof(*c)); + c->fd = socket(h->ai.ai_family, h->ai.ai_socktype, h->ai.ai_protocol); + if (c->fd < 0) { syslog(LOG_INFO, "socket for %s [%s] failed: %m", h->host, h->addr); - return (-1); + return (1); } - if (connect(fd, (struct sockaddr *)&h->sa, h->ai.ai_addrlen) < 0) { + if (connect(c->fd, (struct sockaddr *)&h->sa, h->ai.ai_addrlen) < 0) { syslog(LOG_INFO, "connect to %s [%s] failed: %m", h->host, h->addr); - close(fd); - return (-1); + close(c->fd); + return (1); } /* Check first reply from remote host */ - res = read_remote(fd, NULL, NULL); + res = read_remote(c, NULL, NULL); if (res != 220) { switch(res) { case 421: @@ -583,25 +582,25 @@ open_connection(struct mx_hostentry *h, int ehlo) break; case 554: syslog(LOG_INFO, "connection failed, remote host requires QUIT"); - send_remote_command(fd, "QUIT"); + send_remote_command(c, "QUIT"); break; default: syslog(LOG_INFO, "connection failed, remote host greeted us with %d", res); break; } - close(fd); - return (-2); + close(c->fd); + return (1); } syslog(LOG_DEBUG, "connection accepted"); if (ehlo) { /* Try EHLO */ - send_remote_command(fd, "EHLO %s", hostname()); - res = esmtp_response(fd); + send_remote_command(c, "EHLO %s", hostname()); + res = esmtp_response(c); } else { - send_remote_command(fd, "HELO %s", hostname()); - res = read_remote(fd, NULL, NULL); + send_remote_command(c, "HELO %s", hostname()); + res = read_remote(c, NULL, NULL); } if (res != 250) { @@ -611,34 +610,36 @@ open_connection(struct mx_hostentry *h, int ehlo) syslog(LOG_INFO, "connection failed, remote host refused our greeting"); } - close(fd); - return -2; + close(c->fd); + return -1; } - return (fd); + return 0; } static void -close_connection(int fd) +close_connection(struct connection *c) { - if (config.ssl != NULL) { - if ((config.features & USESSL) != 0) - SSL_shutdown(config.ssl); + if (c->ssl != NULL) { + if ((c->flags & USESSL) != 0) + SSL_shutdown(c->ssl); - SSL_free(config.ssl); + SSL_free(c->ssl); } - close(fd); + close(c->fd); } static int deliver_to_host(struct qitem *it, struct mx_hostentry *host) { + struct connection c; struct authuser *a; char line[1000]; size_t linelen; - int fd, error = 0, do_auth = 0, res = 0; + int error = 0, do_auth = 0, res = 0; + /*initialize read structure*/ if (fseek(it->mailf, 0, SEEK_SET) != 0) { snprintf(errmsg, sizeof(errmsg), "can not seek: %s", strerror(errno)); return (-1); @@ -655,8 +656,8 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) } } - fd = open_connection(host, 1); - if (fd == -2) { + error = open_connection(&c, host, 1); + if (error < 0) { /* fallback to HELO if possible */ if ((config.features & NOHELO) != 0) { /* HELO disabled in config file */ @@ -686,43 +687,28 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) return (1); } - - /*disable any security*/ - config.features &= ~(SECURETRANS | STARTTLS); - fd = open_connection(host, 0); + + error = open_connection(&c, host, 0); } - if (fd < 0) { + if (error) { /*connection failed*/ return (1); } - - if ((config.features & HASSTARTTLS) == 0 && (config.features & STARTTLS) != 0) { - if ((config.features & TLS_OPP) != 0) { - /* disable STARTTLS, opportunistic mode */ - syslog(LOG_INFO, "in opportunistic TLS mode, STARTTLS not available"); - config.features &= ~STARTTLS; - } else { - /* remote has no STARTTLS but user required it */ - syslog(LOG_ERR, "remote delivery deferred: STARTTLS not available"); - error = 1; - goto out; - } - } if ((config.features & SECURETRANS) != 0) { /* initialize secure transaction */ - error = smtp_init_crypto(fd, config.features); - if (error == 0) - syslog(LOG_DEBUG, "SSL initialization successful"); - else + error = smtp_init_crypto(&c); + if (error != 0) { goto out; + } + syslog(LOG_DEBUG, "SSL initialization successful"); /* refresh supported ESMTP features if STARTTLS was used*/ - if ((config.features & STARTTLS) != 0) { - config.features &= ~ESMTPMASK; - send_remote_command(fd, "EHLO %s", hostname()); - res = esmtp_response(fd); + if ((c.flags & USESTARTTLS) != 0) { + c.flags &= ~ESMTPMASK; + send_remote_command(&c, "EHLO %s", hostname()); + res = esmtp_response(&c); if (res != 250) { /* shouldn't happen */ syslog(LOG_NOTICE, "remote delivery deferred: EHLO after STARTTLS failed: %s", neterr); @@ -738,7 +724,7 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) * encryption. */ syslog(LOG_INFO, "using SMTP authentication for user %s", a->login); - error = smtp_login(fd, a->login, a->password); + error = smtp_login(&c, a->login, a->password); if (error) { syslog(LOG_ERR, "remote delivery failed:" " SMTP login failed"); @@ -748,19 +734,19 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) } /* XXX send ESMTP ENVID, RET (FULL/HDRS) and 8BITMIME */ - send_remote_command(fd, "MAIL FROM:<%s>", it->sender); - error = expect_response(fd, "MAIL FROM", 250); + send_remote_command(&c, "MAIL FROM:<%s>", it->sender); + error = expect_response(&c, "MAIL FROM", 250); if (error) goto out; /* XXX send ESMTP ORCPT */ - send_remote_command(fd, "RCPT TO:<%s>", it->addr); - error = expect_response(fd, "RCPT TO", 250); + send_remote_command(&c, "RCPT TO:<%s>", it->addr); + error = expect_response(&c, "RCPT TO", 250); if (error) goto out; - send_remote_command(fd, "DATA"); - error = expect_response(fd, "DATA", 354); + send_remote_command(&c, "DATA"); + error = expect_response(&c, "DATA", 354); if (error) goto out; @@ -784,7 +770,7 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) if (line[0] == '.') linelen++; - if (send_remote_command(fd, "%s", line) != (ssize_t)linelen+1) { + if (send_remote_command(&c, "%s", line) != (ssize_t)linelen+1) { syslog(LOG_NOTICE, "remote delivery deferred: write error"); error = 1; goto out; @@ -797,17 +783,17 @@ deliver_to_host(struct qitem *it, struct mx_hostentry *host) goto out; } - send_remote_command(fd, "."); - error = expect_response(fd, "final DATA", 250); + send_remote_command(&c, "."); + error = expect_response(&c, "final DATA", 250); if (error) goto out; - send_remote_command(fd, "QUIT"); - if (read_remote(fd, NULL, NULL) != 221) + send_remote_command(&c, "QUIT"); + if (read_remote(&c, NULL, NULL) != 221) syslog(LOG_INFO, "remote delivery succeeded but QUIT failed: %s", neterr); out: - close_connection(fd); + close_connection(&c); return (error); } From afe9ead64bdcf71d88743fdae7bb668344e240a4 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 20:11:09 +0200 Subject: [PATCH 25/36] Use memset() to 0 rather than the deprecated bzero() --- local.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/local.c b/local.c index 6a6407e..a66ed15 100644 --- a/local.c +++ b/local.c @@ -42,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -62,7 +63,7 @@ create_mbox(const char *name) /* * We need to enable SIGCHLD temporarily so that waitpid works. */ - bzero(&sa, sizeof(sa)); + memset(&sa, 0, sizeof(sa)); sa.sa_handler = SIG_DFL; sigaction(SIGCHLD, &sa, &osa); From 138ea0315b10b87a3b3f0cd4483cec53de750c95 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 20:14:26 +0200 Subject: [PATCH 26/36] Try a better guess in the unlikely event that _SC_OPEN_MAX is unavailable --- local.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/local.c b/local.c index a66ed15..2546ae7 100644 --- a/local.c +++ b/local.c @@ -75,7 +75,7 @@ create_mbox(const char *name) /* child */ maxfd = sysconf(_SC_OPEN_MAX); if (maxfd == -1) - maxfd = 1024; /* what can we do... */ + maxfd = FOPEN_MAX; /* what can we do... */ for (i = 3; i <= maxfd; ++i) close(i); From d380e953c24b2431e74b5ca0f9b3d7f71acf5d01 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 20:25:26 +0200 Subject: [PATCH 27/36] Make local mail delivery function check for read errors. --- local.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/local.c b/local.c index 2546ae7..5e53a78 100644 --- a/local.c +++ b/local.c @@ -204,9 +204,7 @@ deliver_local(struct qitem *it) if (write(mbox, line, error) != error) goto wrerror; - while (!feof(it->mailf)) { - if (fgets(line, sizeof(line), it->mailf) == NULL) - break; + while (fgets(line, sizeof(line), it->mailf)) { linelen = strlen(line); if (linelen == 0 || line[linelen - 1] != '\n') { syslog(LOG_CRIT, "local delivery failed: corrupted queue file"); @@ -239,6 +237,13 @@ deliver_local(struct qitem *it) if ((size_t)write(mbox, line, linelen) != linelen) goto wrerror; } + + if (ferror(it->mailf)) { + syslog(LOG_ERR, "local delivery failed: I/O error while reading: %m"); + error = 1; + goto chop; + } + close(mbox); return (0); From 1fcb993ddd93a8d10c14016ab458e40ecdfc5be9 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 20:35:53 +0200 Subject: [PATCH 28/36] Fix config file entry parsing Code and both config file and man pages refer that option as SECURETRANS, not SECURETRANSFER. --- conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conf.c b/conf.c index f3c014e..5b628fe 100644 --- a/conf.c +++ b/conf.c @@ -253,7 +253,7 @@ parse_conf(const char *config_path) config.features |= NOHELO; else if (strcmp(word, "OPPORTUNISTIC_TLS") == 0 && data == NULL) config.features |= TLS_OPP; - else if (strcmp(word, "SECURETRANSFER") == 0 && data == NULL) + else if (strcmp(word, "SECURETRANS") == 0 && data == NULL) config.features |= SECURETRANS; else if (strcmp(word, "DEFER") == 0 && data == NULL) config.features |= DEFER; From 5591c0fc5b97362e05567777a52e1d57ec80a544 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 20:46:32 +0200 Subject: [PATCH 29/36] Revert "Fix config file entry parsing" This reverts commit 1fcb993ddd93a8d10c14016ab458e40ecdfc5be9. --- conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conf.c b/conf.c index 5b628fe..f3c014e 100644 --- a/conf.c +++ b/conf.c @@ -253,7 +253,7 @@ parse_conf(const char *config_path) config.features |= NOHELO; else if (strcmp(word, "OPPORTUNISTIC_TLS") == 0 && data == NULL) config.features |= TLS_OPP; - else if (strcmp(word, "SECURETRANS") == 0 && data == NULL) + else if (strcmp(word, "SECURETRANSFER") == 0 && data == NULL) config.features |= SECURETRANS; else if (strcmp(word, "DEFER") == 0 && data == NULL) config.features |= DEFER; From dc4f1a743afc1082517e2166f070274efbf616b3 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 20:49:02 +0200 Subject: [PATCH 30/36] Fix man page entries. SECURETRANS option is actually SECURETRANSFER --- dma.8 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dma.8 b/dma.8 index 194317d..8ac0f34 100644 --- a/dma.8 +++ b/dma.8 @@ -222,7 +222,7 @@ When in debug mode .Nm is going to log any network communication, including possibly sensitive data and authentication credentials, to the system logger. -.It Ic SECURETRANS Xo +.It Ic SECURETRANSFER Xo (boolean, default=commented) .Xc Uncomment if you want TLS/SSL secured transfer. @@ -231,7 +231,7 @@ Uncomment if you want TLS/SSL secured transfer. .Xc Uncomment if you want to use STARTTLS. Only useful together with -.Sq SECURETRANS . +.Sq SECURETRANSFER . .It Ic NOHELO Xo (boolean, default=commented) .Xc @@ -251,7 +251,7 @@ the outside mail exchangers; in opportunistic TLS mode, the connection will be encrypted if the remote server supports STARTTLS, but an unencrypted delivery will still be made if the negotiation fails. Only useful together with -.Sq SECURETRANS +.Sq SECURETRANSFR and .Sq STARTTLS . .It Ic CERTFILE Xo From 87405fe3ff35cfcbffca70dda192a5e915cffa20 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 20:55:45 +0200 Subject: [PATCH 31/36] Simplify logging messages. Be less redundant with hostnames and addresses, the first one should be enough. --- net.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/net.c b/net.c index 64e47ee..821e00a 100644 --- a/net.c +++ b/net.c @@ -555,20 +555,18 @@ open_connection(struct connection*c, struct mx_hostentry *h, int ehlo) { int res; - syslog(LOG_INFO, "trying remote delivery to %s [%s] pref %d using %s", + syslog(LOG_INFO, "connecting to remote host %s [%s] pref %d using %s", h->host, h->addr, h->pref, ehlo? "EHLO" : "HELO"); memset(c, 0, sizeof(*c)); c->fd = socket(h->ai.ai_family, h->ai.ai_socktype, h->ai.ai_protocol); if (c->fd < 0) { - syslog(LOG_INFO, "socket for %s [%s] failed: %m", - h->host, h->addr); + syslog(LOG_INFO, "socket creation failed: %m"); return (1); } if (connect(c->fd, (struct sockaddr *)&h->sa, h->ai.ai_addrlen) < 0) { - syslog(LOG_INFO, "connect to %s [%s] failed: %m", - h->host, h->addr); + syslog(LOG_INFO, "connection failed: %m"); close(c->fd); return (1); } @@ -593,7 +591,6 @@ open_connection(struct connection*c, struct mx_hostentry *h, int ehlo) return (1); } - syslog(LOG_DEBUG, "connection accepted"); if (ehlo) { /* Try EHLO */ send_remote_command(c, "EHLO %s", hostname()); @@ -614,6 +611,7 @@ open_connection(struct connection*c, struct mx_hostentry *h, int ehlo) return -1; } + syslog(LOG_DEBUG, "connection accepted"); return 0; } From 58109b0593d6b83924b20946ca8ddfd547e89c9e Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 21:17:42 +0200 Subject: [PATCH 32/36] Make PLAIN authentication work Actually send AUTH PLAIN and encode credentials properly. --- net.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/net.c b/net.c index 821e00a..1c921eb 100644 --- a/net.c +++ b/net.c @@ -352,16 +352,25 @@ smtp_login(struct connection *c, char *login, char* password) * if no authority is provided the SMTP server will derive it from authentication. */ char *buff; - + + send_remote_command(c, "AUTH PLAIN"); + if (read_remote(c, NULL, NULL) != 334) { + syslog(LOG_NOTICE, "remote delivery deferred:" + " AUTH PLAIN was refused: %s", + neterr); + return (1); + } + len = strlen(login) + strlen(password) + 2; - buff = calloc(len, 1); + buff = calloc(len + 1, 1); if (!buff) { syslog(LOG_NOTICE, "remote delivery deferred: memory allocation failure"); return (1); } - strcpy(buff, login); - strcpy(buff + strlen(login) + 1, password); + /* we want this: '\0'username'\0'password*/ + strcpy(buff + 1, login); + strcpy(buff + 1 + strlen(login) + 1, password); len = base64_encode(buff, len, &temp); free(buff); From ef4681bb3fd0f7b1b0d4d5714c296584e5a58bd0 Mon Sep 17 00:00:00 2001 From: micia Date: Wed, 3 Oct 2012 23:09:23 +0200 Subject: [PATCH 33/36] Some minor fixes Fix a typo in man page, remove unused macro constant, make ESMTP parsing more strict --- dma.8 | 2 +- dma.h | 1 - net.c | 5 +---- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/dma.8 b/dma.8 index 8ac0f34..5f3ef01 100644 --- a/dma.8 +++ b/dma.8 @@ -251,7 +251,7 @@ the outside mail exchangers; in opportunistic TLS mode, the connection will be encrypted if the remote server supports STARTTLS, but an unencrypted delivery will still be made if the negotiation fails. Only useful together with -.Sq SECURETRANSFR +.Sq SECURETRANSFER and .Sq STARTTLS . .It Ic CERTFILE Xo diff --git a/dma.h b/dma.h index b1ba8a1..a8ab52d 100644 --- a/dma.h +++ b/dma.h @@ -49,7 +49,6 @@ #define BUF_SIZE 2048 #define ESMTPBUF_SIZE 8192 -#define ESMTPTOK_SIZE 1024 #define ERRMSG_SIZE 200 #define USERNAME_SIZE 50 #define MIN_RETRY 300 /* 5 minutes */ diff --git a/net.c b/net.c index 1c921eb..8033fcc 100644 --- a/net.c +++ b/net.c @@ -467,12 +467,9 @@ esmtp_nexttoken(char **buff) } /* null terminate and update the parser */ - if (*line == '\r') { + if (*line == '\r' || *line == ' ') { *line++ = 0; } - if (*line == ' ') { - *line++ = 0; - } *buff = line; return tok; From 87090116c3ff5254c756937beef6b97c7521ae90 Mon Sep 17 00:00:00 2001 From: micia Date: Thu, 4 Oct 2012 00:43:35 +0200 Subject: [PATCH 34/36] Consistently replace bcopy() and bzero() with memcpy() and memset() bzero() and bcopy() are deprecated and marked as legacy by POSIX, bcopy() is not even specified anymore by POSIX. --- crypto.c | 9 +++++---- dma.c | 4 ++-- dns.c | 2 +- mail.c | 5 +++-- spool.c | 5 +++-- 5 files changed, 14 insertions(+), 11 deletions(-) diff --git a/crypto.c b/crypto.c index 59b3bbc..6552262 100644 --- a/crypto.c +++ b/crypto.c @@ -40,6 +40,7 @@ #include #include +#include #include #include "dma.h" @@ -223,10 +224,10 @@ hmac_md5(unsigned char *text, int text_len, unsigned char *key, int key_len, */ /* start out by storing key in pads */ - bzero( k_ipad, sizeof k_ipad); - bzero( k_opad, sizeof k_opad); - bcopy( key, k_ipad, key_len); - bcopy( key, k_opad, key_len); + memset(k_ipad, 0, sizeof(k_ipad)); + memset(k_opad, 0, sizeof(k_opad)); + memcpy(k_ipad, key, key_len); + memcpy(k_opad, key, key_len); /* XOR key with ipad and opad values */ for (i=0; i<64; i++) { diff --git a/dma.c b/dma.c index bb5fa19..b2f19e4 100644 --- a/dma.c +++ b/dma.c @@ -240,7 +240,7 @@ go_background(struct queue *queue) } daemonize = 0; - bzero(&sa, sizeof(sa)); + memset(&sa, 0, sizeof(sa)); sa.sa_handler = SIG_IGN; sigaction(SIGCHLD, &sa, NULL); @@ -437,7 +437,7 @@ main(int argc, char **argv) atexit(deltmp); init_random(); - bzero(&queue, sizeof(queue)); + memset(&queue, 0, sizeof(queue)); LIST_INIT(&queue.queue); if (strcmp(argv[0], "mailq") == 0) { diff --git a/dns.c b/dns.c index 394ea59..720359f 100644 --- a/dns.c +++ b/dns.c @@ -92,7 +92,7 @@ add_host(int pref, const char *host, unsigned int port, struct mx_hostentry **he p->pref = pref; p->ai = *res; p->ai.ai_addr = NULL; - bcopy(res->ai_addr, &p->sa, p->ai.ai_addrlen); + memcpy(&p->sa, res->ai_addr, p->ai.ai_addrlen); getnameinfo((struct sockaddr *)&p->sa, p->ai.ai_addrlen, p->addr, sizeof(p->addr), diff --git a/mail.c b/mail.c index 9cbde41..996121c 100644 --- a/mail.c +++ b/mail.c @@ -34,6 +34,7 @@ #include #include +#include #include #include #include @@ -54,7 +55,7 @@ bounce(struct qitem *it, const char *reason) exit(1); } - bzero(&bounceq, sizeof(bounceq)); + memset(&bounceq, 0, sizeof(bounceq)); LIST_INIT(&bounceq.queue); bounceq.sender = ""; if (add_recp(&bounceq, it->sender, EXPAND_WILDCARD) != 0) @@ -170,7 +171,7 @@ parse_addrs(struct parse_state *ps, char *s, struct queue *queue) case START: /* init our data */ - bzero(ps, sizeof(*ps)); + memset(ps, 0, sizeof(*ps)); /* skip over header name */ while (*s != ':') diff --git a/spool.c b/spool.c index fa42654..10866f6 100644 --- a/spool.c +++ b/spool.c @@ -44,6 +44,7 @@ #include #include #include +#include #include #include "dma.h" @@ -159,7 +160,7 @@ readqueuef(struct queue *queue, char *queuefn) char *queueid = NULL, *sender = NULL, *addr = NULL; struct qitem *it = NULL; - bzero(&itmqueue, sizeof(itmqueue)); + memset(&itmqueue, 0, sizeof(itmqueue)); LIST_INIT(&itmqueue.queue); queuef = fopen(queuefn, "r"); @@ -284,7 +285,7 @@ load_queue(struct queue *queue) char *queuefn; char *mailfn; - bzero(queue, sizeof(*queue)); + memset(queue, 0, sizeof(*queue)); LIST_INIT(&queue->queue); spooldir = opendir(config.spooldir); From 8761545ade2cf289154ec141d910b710531c19a6 Mon Sep 17 00:00:00 2001 From: micia Date: Thu, 4 Oct 2012 12:15:40 +0200 Subject: [PATCH 35/36] Clear file error flag Clear FILE stream error flag on retry to allow delivery logic to retry properly on I/O error failure. --- dma.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dma.c b/dma.c index b2f19e4..04de0c0 100644 --- a/dma.c +++ b/dma.c @@ -309,8 +309,12 @@ deliver(struct qitem *it) snprintf(errmsg, sizeof(errmsg), "unknown bounce reason"); retry: + /* clear error and EOF indicator just in case + * last delivery was aborted due to I/O error. + */ + clearerr(it->mailf); syslog(LOG_INFO, "trying delivery"); - + if (it->remote) error = deliver_remote(it); else From 4752047a7d7d5ecbd1dc6f1e676514b13212462d Mon Sep 17 00:00:00 2001 From: micia Date: Thu, 4 Oct 2012 12:26:08 +0200 Subject: [PATCH 36/36] Check for I/O errors even on mail bounces Warn on read errors during bounce creation but still try to deliver the bounced mail. Main delivery routine is responsible to clear FILE stream error indicator to provide a valid stream to the bounce routine. --- dma.c | 1 + mail.c | 10 ++++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/dma.c b/dma.c index 04de0c0..dda398f 100644 --- a/dma.c +++ b/dma.c @@ -359,6 +359,7 @@ deliver(struct qitem *it) } bounce: + clearerr(it->mailf); bounce(it, errmsg); /* NOTREACHED */ } diff --git a/mail.c b/mail.c index 996121c..faa883f 100644 --- a/mail.c +++ b/mail.c @@ -111,9 +111,7 @@ bounce(struct qitem *it, const char *reason) goto fail; } } else { - while (!feof(it->mailf)) { - if (fgets(line, sizeof(line), it->mailf) == NULL) - break; + while (fgets(line, sizeof(line), it->mailf)) { if (line[0] == '\n') break; if (fwrite(line, strlen(line), 1, bounceq.mailf) != 1) @@ -123,8 +121,12 @@ bounce(struct qitem *it, const char *reason) if (linkspool(&bounceq) != 0) goto fail; + + /* warn on partial bounce, but still try delivery */ + if (ferror(it->mailf)) + syslog(LOG_ERR, "error I/O error while crating bounce: %m"); + /* bounce is safe */ - delqueue(it); run_queue(&bounceq);