On 01/01/2019 17:56, Wietse Venema wrote: > John Fawcett: >>>> 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 >>> Thanks for the feedback. >>> >>> I can take a look at it, but aside from time, I agree that any invasive >>> modifications would be better done in a new experimental release. I also >>> would not put the a temporary modification in 3.4 since people may >>> become accustomed to it, when it is likely to change again. >>> >>> John >>> >> To take this one step further, I have documented the current behaviour >> for errors during the smtp chat, indicating whether they are chatted to >> the client, logged to maillog and whether they set existing error flags. >> I did that so far for smtpd.c, smtpd_sasl_glue.c and smtpd_sasl_proto.c >> from postfix-3.4-20181229-nonprod. I did not include normal smtp chats, >> verbose logging or logging done outside an smtp session or done during >> initializations. >> >> I suppose that the next step would be to evalute which of these errors >> are worthy of being reported in the disconnect line, with a particular >> eye to errors that are currently not logged in the maillog and that >> would not be available if postmaster notifications are disabled (more >> likely for PROTOCOL and POLICY errors). >> >> Any thoughts about this approach? PS the attachment is a LibreOffice >> spreadsheet. Let's see if it gets through. > Basically everything that detects a violation sets state->error_mask, > and there would need to be another line of code that sets a violation > type bit. That's about 124 places. I don't see that smtpd_chat.c > is in a violation detection code path; it reports violations that > are detected elsewhere. > > Wietse
smtp_chat.c wasn't one of the files I looked at but I agree it is not relevant. For clarity: the "smtp chat" column in the spreadhset is to contextualize what errors are being sent back to the client. Grepping on MAIL_ERROR_* errors for smtpd directory, I get MAIL_ERROR_POLICY 32 MAIL_ERROR_PROTOCOL 74 (1 is in excluded code) MAIL_ERROR_RESOURCE 3 MAIL_ERROR_SOFTWARE 8 MAIL_ERROR_BOUNCE 2 MAIL_ERROR_DATA 1 and I've tracked all of them in the revised spreadsheet as well as correcting a copy and paste error in the last few lines of the previous version. In column H I have added the proposal for the new violation_mask bucket. I have only done that for errors that are currently flagged as violations in error_mask, plus the following additional cases: * in smtpd_proto(), when smtpd_client_connection_rate_limit exceeded * in smtpd_proto() for improper pipelining Maybe I put too many buckets, but they can be condensed. From the example buckets you gave, I didn't find places where the following were flagged as violations: relay, unverified-address, unknown-user, dnsbl, tls-handshake, lost connection and timeout. That needs more work. There was also one that I could not classify (see ???? in column H). John
postfix smtpd errors.ods
Description: application/vnd.oasis.opendocument.spreadsheet
