First experimental implementation of ESMTP, HELO fallback and various improvements #11

micia · 2012-10-03T20:18:09Z

These commits implement basic ESMTP parsing and support, as well as implementing a basic HELO fallback in case everything else fails. The main changes are:

ESMTP aware connections to remote hosts and separation between config structure and connection (no more ssl in config).
HELO fallback if possible and if user allows it in the configuration files.
I/O error checking when reading mails (by testing ferror()).
Better config file parsing, by ignoring trailing and ending spaces in config files (chomp() function).
Reworked read_remote() function to be more efficient with less cryptic if guards, while allowing for more flexible external buffer semantics.
Avoid infinite loops until connection timeout in read_remote(), if the remote host closes the connection on dma (read would return 0, thus pos would be equal to len, causing infinite loops).
read_remote() now returns full status number rather than status / 100 to allow for more flexible behaviour in mail delivery logic.
Support for PLAIN authentication.
Rather than assuming the remote host not to close the connection if dma requires an unsupported feature, now dma parses ESMTP and never sends requests for non advertised features (most SMTP servers will close the connection on an unsupported feature), for example it won't use CRAM MD5 authentication if EHLO response doesn't list it.
New VERBOSE configuration option to debug network communication between remote host and dma (disabled by default and dangerous for privacy).
New NOHELO configuration option that disables HELO fallback.
Minimal configuration file checking (but still very incomplete).
Other minor fixes, mainly due to code changes, also config files and man page were updated.

These features are not fully tested yet, but using dma on a daily basis I don't see any issue, but a code review and a more in depth test would be necessary.

…til crypto.c enables it

…en, improve external buffer logic to report actual length to the caller, make it return the actual status code rather than status code / 100

… ESMTP aware functions, rework authentication mechanisms accordingly

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.

USESSL implies SECURETRANS so the condition is redundant.

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.

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)

If server advertises the login possibility and the user provides us with authentication credential, dma should use them or fail.

…NS is redundant)

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.

errno could be zero leading to a confusing message like: SMTP login failed: success

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.

…able

Code and both config file and man pages refer that option as SECURETRANS, not SECURETRANSFER.

This reverts commit 1fcb993.

SECURETRANS option is actually SECURETRANSFER

Be less redundant with hostnames and addresses, the first one should be enough.

Actually send AUTH PLAIN and encode credentials properly.

Fix a typo in man page, remove unused macro constant, make ESMTP parsing more strict

bzero() and bcopy() are deprecated and marked as legacy by POSIX, bcopy() is not even specified anymore by POSIX.

Clear FILE stream error flag on retry to allow delivery logic to retry properly on I/O error failure.

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.

corecode · 2012-10-16T17:21:55Z

Thanks, still trying to find time to review the patches. Please be patient.

corecode · 2012-10-17T10:34:35Z

I suspect it might be better to create a formal parser with yacc/lex. What do you think? We can also chat more quickly on IRC ##dma@freenode

micia added 30 commits October 2, 2012 16:23

simplify SSL config flag semantics, default is having SSL disabled un…

34dbda4

…til crypto.c enables it

simplify read_remote logic and prevent infinite loops due to pos == l…

6574304

…en, improve external buffer logic to report actual length to the caller, make it return the actual status code rather than status code / 100

rework yet again read_remote to be more secure, add HELO fallback and…

0ead6c3

… ESMTP aware functions, rework authentication mechanisms accordingly

add VERBOSE feature to debug network communication

f22e7c5

various bugfixes to VERBOSE logging and ESMTP parsing

aab5fbc

fix buggy boolean guard

d508bdb

fix ESMTP parsing

31fb2f4

deliver_host simplify and make mail read loop more reliable

e5e6bc9

add VERBOSE option warning to config file

f5cc3fa

Improve configuration file routines and port parsing

e8c66f2

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.

Fix some issues with format and simplify a condition

2fca723

USESSL implies SECURETRANS so the condition is redundant.

Improve HELO fallback and ESMTP capabilities refresh

76e0821

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)

Drop entirely the no login fallback

c917cec

If server advertises the login possibility and the user provides us with authentication credential, dma should use them or fail.

Improve message on I/O error

bf5941f

Replace SECURETRANS with the more specific USESSL (probably SECURETRA…

005a007

…NS is redundant)

First implementation of the PLAIN authentication method

1ae3e6b

More read_remote improvements, to error handling and parsing

460cc2d

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.

Minor style fix

1148a3d

Update man page, minor style fix in config file.

06621be

Fix typo in man page.

c7e813d

Fix error message

b7977a8

errno could be zero leading to a confusing message like: SMTP login failed: success

Avoid mixed declaration and code.

c899ca3

Use memset() to 0 rather than the deprecated bzero()

afe9ead

Try a better guess in the unlikely event that _SC_OPEN_MAX is unavail…

138ea03

…able

Make local mail delivery function check for read errors.

d380e95

Fix config file entry parsing

1fcb993

Code and both config file and man pages refer that option as SECURETRANS, not SECURETRANSFER.

Revert "Fix config file entry parsing"

5591c0f

This reverts commit 1fcb993.

Fix man page entries.

dc4f1a7

SECURETRANS option is actually SECURETRANSFER

micia added 6 commits October 3, 2012 20:55

Simplify logging messages.

87405fe

Be less redundant with hostnames and addresses, the first one should be enough.

Make PLAIN authentication work

58109b0

Actually send AUTH PLAIN and encode credentials properly.

Some minor fixes

ef4681b

Fix a typo in man page, remove unused macro constant, make ESMTP parsing more strict

Consistently replace bcopy() and bzero() with memcpy() and memset()

8709011

bzero() and bcopy() are deprecated and marked as legacy by POSIX, bcopy() is not even specified anymore by POSIX.

Clear file error flag

8761545

Clear FILE stream error flag on retry to allow delivery logic to retry properly on I/O error failure.

Check for I/O errors even on mail bounces

4752047

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.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First experimental implementation of ESMTP, HELO fallback and various improvements #11

First experimental implementation of ESMTP, HELO fallback and various improvements #11

micia commented Oct 3, 2012

corecode commented Oct 16, 2012

corecode commented Oct 17, 2012

First experimental implementation of ESMTP, HELO fallback and various improvements #11

Are you sure you want to change the base?

First experimental implementation of ESMTP, HELO fallback and various improvements #11

Conversation

micia commented Oct 3, 2012

corecode commented Oct 16, 2012

corecode commented Oct 17, 2012