Sorry, I did not recognize the diff because all whitespace was using
UTF8 code points, and I read mail with a text editor that is optimized
for programing, not for text processing.
After fixing the whitespace:
> --- smtpd/smtpd.c.orig 2018-12-28 13:18:55.427162049 +0100
> +++ smtpd/smtpd.c 2018-12-29 11:25:49.294927986 +0100
> @@ -5677,6 +5677,8 @@
> if (var_smtpd_enforce_tls &&
> !state->tls_context &&
> (cmdp->flags & SMTPD_CMD_FLAG_PRE_TLS) == 0) {
> + msg_warn("Missing required STARTTLS command from %s",
> + state->namaddr);
I would not log this for EVERY command. Especially because the
logged text size by far exceeds the command size (each logfile
record takes ~100 bytes, while the client needs to send only four
characters plus CRLF.
For example, Postfix logs pipelining errors including evidence, but
it logs this at most once per session, to avoid flooding the log
with garbage.
> - if (cmdp->action(state, argc, argv) != 0)
> + if (cmdp->action(state, argc, argv) != 0) {
> state->error_count++;
> + if (state->error_mask & MAIL_ERROR_PROTOCOL)
> + msg_info("smtp protocol error in %s from %s",
> + cmdp->name, state->namaddr);
> + }
Unfortunately, once the protocol error bit is set, this will flag
all subsequent errors as protocol errors, writing O(100) byte records
and introducing the same logfile flooding opportunity as mentioned
above.
Even if you log this only once per session, it would blame the wrong
command (cmdp->name), because the protocol error bit may have been
set when the client sent an unknown command, and that code path never
reaches the code fragment shown above.
Wietse
