Re: [patch] remove smtpd filter code

2017-08-04 Thread Gilles Chehade
On Fri, Aug 04, 2017 at 02:56:21PM +0200, Gilles Chehade wrote:
> On Fri, Aug 04, 2017 at 01:13:06PM +0200, Eric Faurot wrote:
> > Hi,
> > 
> > Experimental support for filters has been removed some time ago from
> > the config parser.  Now we want to get rid of the remaining code.
> > It's not that trivial, so we proceed in several steps.
> > 
> > The first (and trickiest) one is to bypass the filter code for
> > incoming smtp sessions, so that filter.c can be unhooked.
> > This is what the following diff does:
> > 
> > - drop filter configuration,
> > - drop filter events,
> > - simulate a positive reply for all filter queries,
> > - write message content directly to the file.
> > 
> > There should be no functionnal change.
> > 
> 
> this should be tested by many people right away to spot subtle regressions
> 

Just a clarification because i've received the same question three times
since this mail :-)

We're not killing filters, most of the implementation is correct and the
filters support is pretty much functional at the exception of some cases
which are very unfortunately show stoppers...

We're removing the filter code from the SMTP state machine to plug it at
a different spot and fix the current limitations. Most of the code is to
be reused, this is not a rewrite but a refactor.

-- 
Gilles Chehade

https://www.poolp.org  @poolpOrg



Re: [patch] remove smtpd filter code

2017-08-04 Thread Gilles Chehade
On Fri, Aug 04, 2017 at 01:13:06PM +0200, Eric Faurot wrote:
> Hi,
> 
> Experimental support for filters has been removed some time ago from
> the config parser.  Now we want to get rid of the remaining code.
> It's not that trivial, so we proceed in several steps.
> 
> The first (and trickiest) one is to bypass the filter code for
> incoming smtp sessions, so that filter.c can be unhooked.
> This is what the following diff does:
> 
> - drop filter configuration,
> - drop filter events,
> - simulate a positive reply for all filter queries,
> - write message content directly to the file.
> 
> There should be no functionnal change.
> 

this should be tested by many people right away to spot subtle regressions

ok gilles@


> Index: pony.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/pony.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 pony.c
> --- pony.c9 Jan 2017 09:53:23 -   1.17
> +++ pony.c3 Aug 2017 09:57:22 -
> @@ -60,7 +60,7 @@ pony_imsg(struct mproc *p, struct imsg *
>   case IMSG_CONF_START:
>   return;
>   case IMSG_CONF_END:
> - filter_configure();
> + smtp_configure();
>   return;
>   case IMSG_CTL_VERBOSE:
>   m_msg(, imsg);
> @@ -148,7 +148,6 @@ pony(void)
>   mda_postfork();
>   mta_postfork();
>   smtp_postfork();
> - filter_postfork();
>  
>   /* do not purge listeners and pki, they are purged
>* in smtp_configure()
> Index: smtp_session.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
> retrieving revision 1.304
> diff -u -p -r1.304 smtp_session.c
> --- smtp_session.c19 Jun 2017 08:35:56 -  1.304
> +++ smtp_session.c4 Aug 2017 10:32:41 -
> @@ -69,9 +69,6 @@ enum session_flags {
>   SF_BOUNCE   = 0x0010,
>   SF_VERIFIED = 0x0020,
>   SF_BADINPUT = 0x0080,
> - SF_FILTERCONN   = 0x0100,
> - SF_FILTERDATA   = 0x0200,
> - SF_FILTERTX = 0x0400,
>  };
>  
>  enum message_flags {
> @@ -116,7 +113,7 @@ struct smtp_tx {
>  
>   size_t   datain;
>   size_t   odatalen;
> - struct io   *oev;
> + FILE*ofile;
>   int  hdrdone;
>   int  rcvcount;
>   int  dataeom;
> @@ -167,7 +164,6 @@ static void smtp_connected(struct smtp_s
>  static void smtp_send_banner(struct smtp_session *);
>  static void smtp_tls_verified(struct smtp_session *);
>  static void smtp_io(struct io *, int, void *);
> -static void smtp_data_io(struct io *, int, void *);
>  static void smtp_data_io_done(struct smtp_session *);
>  static void smtp_enter_state(struct smtp_session *, int);
>  static void smtp_reply(struct smtp_session *, char *, ...);
> @@ -194,11 +190,6 @@ static void smtp_queue_commit(struct smt
>  static void smtp_queue_rollback(struct smtp_session *);
>  
>  static void smtp_filter_connect(struct smtp_session *, struct sockaddr *);
> -static void smtp_filter_rset(struct smtp_session *);
> -static void smtp_filter_disconnect(struct smtp_session *);
> -static void smtp_filter_tx_begin(struct smtp_session *);
> -static void smtp_filter_tx_commit(struct smtp_session *);
> -static void smtp_filter_tx_rollback(struct smtp_session *);
>  static void smtp_filter_eom(struct smtp_session *);
>  static void smtp_filter_helo(struct smtp_session *);
>  static void smtp_filter_mail(struct smtp_session *);
> @@ -728,12 +719,10 @@ smtp_session_imsg(struct mproc *p, struc
>   break;
>  
>   case LKA_PERMFAIL:
> - smtp_filter_tx_rollback(s);
>   smtp_tx_free(s->tx);
>   smtp_reply(s, "%d %s", 530, "Sender rejected");
>   break;
>   case LKA_TEMPFAIL:
> - smtp_filter_tx_rollback(s);
>   smtp_tx_free(s->tx);
>   smtp_reply(s, "421 %s: Temporary Error",
>   esc_code(ESC_STATUS_TEMPFAIL, 
> ESC_OTHER_MAIL_SYSTEM_STATUS));
> @@ -785,7 +774,6 @@ smtp_session_imsg(struct mproc *p, struc
>   smtp_reply(s, "250 %s: Ok",
>   esc_code(ESC_STATUS_OK, ESC_OTHER_STATUS));
>   } else {
> - smtp_filter_tx_rollback(s);
>   smtp_tx_free(s->tx);
>   smtp_reply(s, "421 %s: Temporary Error",
>   esc_code(ESC_STATUS_TEMPFAIL, 
> ESC_OTHER_MAIL_SYSTEM_STATUS));
> @@ -813,7 +801,7 @@ smtp_session_imsg(struct mproc *p, struc
>   log_debug("smtp: %p: fd %d from queue", s, imsg->fd);
>  
>   tree_xset(_filter, s->id, s);
> - filter_build_fd_chain(s->id, imsg->fd);
> +