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

Reply via email to