Victor Duchovni: > On Wed, Nov 17, 2010 at 02:37:27PM -0500, Wietse Venema wrote: > > > > The problem here is that smtpd_access_denied is misused from its original > > > intent of reporting 503 after a client fails to heed a 554 banner. Perhaps > > > the intended "421" disconnect on the next command should use a different > > > mechanism, or alternatively the code above could peek at the 2nd and 3rd > > > characters of the buffer and disconnect if these match "21" (a big ugly, > > > but avoids new complexity in the internal state). > > > > I forced the 503 reply code for RFC 2821 compliance in Postfix > > 1.1.0, and I concluded independently that we can make an exception > > for [45]21 here, since those replies are final. > > Something along the lines of: > > Index: src/smtpd/smtpd.c > --- src/smtpd/smtpd.c 7 Oct 2010 20:02:54 -0000 1.1.1.6 > +++ src/smtpd/smtpd.c 17 Nov 2010 19:52:05 -0000 > @@ -4521,8 +4521,11 @@ > } > /* XXX We use the real client for connect access control. */ > if (state->access_denied && cmdp->action != quit_cmd) { > - smtpd_chat_reply(state, "503 5.7.0 Error: access denied for %s", > - state->namaddr); /* RFC 2821 Sec 3.1 */ > + if (strncmp(state->acccess_denied+1, "21", 2) == 0) > + smtpd_chat_reply(state, "%s", state->access_denied); > + else /* RFC 2821 Sec 3.1 */ > + smtpd_chat_reply(state, "503 5.7.0 Error: access denied" > + " for %s", state->namaddr); > state->error_count++; > continue; > } > > or did I misunderstand?
I had a similar patch that I was going to post 30 mins ago when someone walked into my room: *** ./smtpd.c- Thu Sep 30 14:18:18 2010 --- ./smtpd.c Wed Nov 17 14:56:13 2010 *************** *** 4521,4526 **** --- 4521,4531 ---- } /* XXX We use the real client for connect access control. */ if (state->access_denied && cmdp->action != quit_cmd) { + /* Make exception for final response. */ + if (strcmp(state->access_denied + 1, "21", 2) == 0) { + smtpd_chat_reply(state, "%s", state->access_denied); + continue; + } smtpd_chat_reply(state, "503 5.7.0 Error: access denied for %s", state->namaddr); /* RFC 2821 Sec 3.1 */ state->error_count++; No point updating the error counter, in case it triggers its own magical behavior. > On a somewhat related note, should the documentation explicitly warn that > with smtpd_delay_reject=no, clients can keep going even when rejected by > helo restrictions, if "smtpd_helo_required = no"? Of course the client > could just not send "helo/ehlo", and avoid the helo restrictions that way. > This may not be clear to those tempted to put substantive checks in > the HELO branch, without enforcing the use of "HELO". Um, people who put restrictions on HELO commands need smtpd_helo_required=yes, regardless of smtpd_delay_reject settings. Wietse