John Fawcett:
> 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.

As a proof of concept this seems to work. As for redundancy, I think
it is useful to log the pipelining violation detail once per session
because it immediately answers the question 'why' Postfix did something.

However, there is a difference between proof of concept and full
implementation. For example, if RCPT TO was rejected, then one would
expect that 'errors=' always shows some error category. You can
already see where this is going.

Therefore, at this point it may be better to just log that the
client sent plaintext, once per session.

I have real work that leaves few cycles for Postfix, and a full
implementation of errors= stats may therefore have to wait until
after the stable 3.4 release.

JFTR, this is what a full implementation would look like.
A full implementation would update a new SMTP_STATE violation_mask
field for specific violation categories (syntax, pipelining,
plaintext, relay, unverified-address, unknown-user, access-deny,
dnsbl, tls-handshake, lost-connection, timeout, etc).

This has a huge code footprint, because every piece of code that
detects a violation will need to set a violation category bit. TBD
is which error categories: we don't want too few (then we could
just use the existing 'protocol', 'software', 'policy', 'resource',
etc.  break-down) or too many buckets.

For output, the implementation would use str_name_mask(), probably
called from a separate function smtpd_format_error_stats().

        Wietse

Reply via email to