On 30/12/2018 01:19, Wietse Venema wrote:
> John Fawcett:
>>> 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.
>> I have added a flag similar to the pipelining one, so that the warning
>> is output once only per session no matter how many commands are issued
>> prior to running STARTTLS.
> Looks better, but I wonder if this could be done in a similar way
> as, or combined with, the disconnect statistics. Strawman:
>
> disconnect from host[addr] ehlo=1 mail=0/1 quit=1 commands=3
> errors=pipelining,plaintext
>
> It puts all the stats in one place, and also minimizes the amount
> of additional logfile space.
>
Here's a revised patch implementing the above logging.
I did not take out the existing pipelining logging since it provides
additional info over the new error summary in the disconnect line,
though if it's no longer judged useful (postscreen can already handle
unauthorized pipelining) it could be removed.
John
--- src/smtpd/smtpd.h.orig 2018-12-29 23:49:10.721258930 +0100
+++ src/smtpd/smtpd.h 2018-12-30 00:26:33.924684042 +0100
@@ -206,6 +206,7 @@
#define SMTPD_FLAG_ILL_PIPELINING (1<<1) /* inappropriate pipelining */
#define SMTPD_FLAG_AUTH_USED (1<<2) /* don't reuse SASL state */
#define SMTPD_FLAG_SMTPUTF8 (1<<3) /* RFC 6531/2 transaction */
+#define SMTPD_FLAG_CMD_BEFORE_STARTTLS (1<<4) /* inappropriate command prior
to STARTTLS */
/* Security: don't reset SMTPD_FLAG_AUTH_USED. */
#define SMTPD_MASK_MAIL_KEEP \
--- src/smtpd/smtpd.c.orig 2018-12-28 13:18:55.427162049 +0100
+++ src/smtpd/smtpd.c 2018-12-30 15:07:01.799363191 +0100
@@ -5677,6 +5677,7 @@
if (var_smtpd_enforce_tls &&
!state->tls_context &&
(cmdp->flags & SMTPD_CMD_FLAG_PRE_TLS) == 0) {
+ state->flags |= SMTPD_FLAG_CMD_BEFORE_STARTTLS;
smtpd_chat_reply(state,
"530 5.7.0 Must issue a STARTTLS command first");
state->error_count++;
@@ -5948,8 +5949,20 @@
* After the client has gone away, clean up whatever we have set up at
* connection time.
*/
- msg_info("disconnect from %s%s", state.namaddr,
- smtpd_format_cmd_stats(state.buffer));
+ VSTRING * error_summary;
+ error_summary = vstring_alloc(100);
+ if (state.flags & SMTPD_FLAG_CMD_BEFORE_STARTTLS) {
+ vstring_sprintf_append(error_summary," errors=plaintext");
+ }
+ if (state.flags & SMTPD_FLAG_ILL_PIPELINING) {
+ if (LEN(error_summary)==0)
+ vstring_sprintf_append(error_summary," errors=pipelining");
+ else
+ vstring_sprintf_append(error_summary,",pipelining");
+ }
+
+ msg_info("disconnect from %s%s%s", state.namaddr,
+ smtpd_format_cmd_stats(state.buffer),STR(error_summary));
teardown_milters(&state); /* duplicates xclient_cmd */
smtpd_state_reset(&state);
debug_peer_restore();
