On Wed, Aug 28, 2019 at 10:33:29AM +0200, Martijn van Duren wrote:
> On 8/28/19 9:23 AM, gil...@poolp.org wrote:
> > 28 ao??t 2019 09:04 "Martijn van Duren" <openbsd+t...@list.imperialat.at> a 
> > ??crit:
> > 
> >> Currently looking into writing an spf filter based on libopensmtpd.
> >> While working through the spec I found in RFC7208 section 7.3 that:
> >> The "r" macro expands to the name of the receiving MTA.
> >> In other words the hostname presented in the banner. Unfortunately we
> >> also support the hostnames directive, which supports ip-hostname
> >> mappings via dynamic tables, which makes it impossible to transfer via
> >> "config|".
> >>
> >> This is a major change that can break (and in the case of libopensmtpd
> >> will break) parsers. We're currently at 0.1, so I don't know if we want
> >> push it to 1 just yet, or if we want to call 1 release-stable and just
> >> bump it to 0.2 for now since we don't have a release yet with filters.
> >>
> >> thoughts?
> >>
> > 
> > I'm sorry but I'm unsure I understand what you're trying to do with the 
> > banner,
> > can you explain ?
> > 
> > If there's need for the hostname presented in the banner to be passed to 
> > filters,
> > which makes sense, it needs its own reporting event which is basically the 
> > server
> > side of the link-identify event.
> > 
> > One thing for sure, you can't put that info in the link-connect event 
> > because the
> > banner is displayed _after_ link-connect and while in smtp-in we already 
> > know the
> > hostname we'll use in the banner, this isn't the case for smtp-out which 
> > will not
> > be able to infer that information before actually seeing the banner.
> > 
> So the diff below implements report|link-greeting
> 
> I haven't implemented the smtp-out case, since none of the smtp-out
> cases appear to be currently implemented.
> 
> Does this read better?
> 

No sorry, I'm very confused :-)

Let me explain my understanding just to make sure we're talking about
the same thing.

You said:

> spf has an "exp" modifier, which allows the reject message to be
> specified by the spf administrator. To do so it can utilize several
> macros. One option being "%{r}", which expands to:
> domain name of host performing the check.


So my understanding is that you'd like a filter to be able to use the
result from an SPF lookup to reject a message in your filter and have
the hostname that was used in the banner part of the message.

If that's so, I think the diff below is both not enough and a bit too
specific, preventing filters with different use-cases than yours from
using the information as easily:

1- first, i think your report event is too specific, keep in mind the
   report events are used to build the internal state of the session,
   so that you can basically duplicate struct smtp_session from smtpd
   in a filter. the textstring part here is not really related to the
   state of the session more than it is related to your use-case.

2- then, your use case is to allow rejecting with a custom message so
   this can't be done with report events, which are informative, this
   requires a filter event which allow taking decisions (like reject,
   in this case). So I think that what's missing here is a phase that
   allows filters to take a decision before printing the banner and I
   think THIS is where your textstring goes, the filter receives that
   report event with the hostname, it receives the filter request for
   the banner and it can decide to write a custom banner.


I may want to write filters that keep track of the hostname for which
the MX is operating and that do not necessarily do anything at banner
so to me tying banner stuff outside of a banner phase is not correct.

Does it make sense ?


> Index: lka.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/lka.c,v
> retrieving revision 1.239
> diff -u -p -r1.239 lka.c
> --- lka.c     26 Jul 2019 06:30:13 -0000      1.239
> +++ lka.c     28 Aug 2019 08:32:33 -0000
> @@ -84,6 +84,8 @@ lka_imsg(struct mproc *p, struct imsg *i
>       const char              *response;
>       const char              *ciphers;
>       const char              *address;
> +     const char              *domain;
> +     const char              *textstring;
>       const char              *helomethod;
>       const char              *heloname;
>       const char              *filter_name;
> @@ -391,6 +393,19 @@ lka_imsg(struct mproc *p, struct imsg *i
>               m_end(&m);
>  
>               lka_report_smtp_link_connect(direction, &tv, reqid, rdns, 
> fcrdns, &ss_src, &ss_dest);
> +             return;
> +
> +     case IMSG_REPORT_SMTP_LINK_GREETING:Index: lka.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/lka.c,v
> retrieving revision 1.239
> diff -u -p -r1.239 lka.c
> --- lka.c     26 Jul 2019 06:30:13 -0000      1.239
> +++ lka.c     28 Aug 2019 08:32:33 -0000
> @@ -84,6 +84,8 @@ lka_imsg(struct mproc *p, struct imsg *i
>       const char              *response;
>       const char              *ciphers;
>       const char              *address;
> +     const char              *domain;
> +     const char              *textstring;
>       const char              *helomethod;
>       const char              *heloname;
>       const char              *filter_name;
> @@ -391,6 +393,19 @@ lka_imsg(struct mproc *p, struct imsg *i
>               m_end(&m);
>  
>               lka_report_smtp_link_connect(direction, &tv, reqid, rdns, 
> fcrdns, &ss_src, &ss_dest);
> +             return;
> +
> +     case IMSG_REPORT_SMTP_LINK_GREETING:
> +             m_msg(&m, imsg);
> +             m_get_string(&m, &direction);
> +             m_get_timeval(&m, &tv);
> +             m_get_id(&m, &reqid);
> +             m_get_string(&m, &domain);
> +             m_get_string(&m, &textstring);
> +             m_end(&m);
> +
> +             lka_report_smtp_link_greeting(direction, reqid, &tv, domain,
> +                 textstring);
>               return;
>  
>       case IMSG_REPORT_SMTP_LINK_DISCONNECT:
> Index: lka_filter.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 lka_filter.c
> --- lka_filter.c      18 Aug 2019 16:52:02 -0000      1.41
> +++ lka_filter.c      28 Aug 2019 08:32:33 -0000
> @@ -35,7 +35,7 @@
>  #include "smtpd.h"
>  #include "log.h"
>  
> -#define      PROTOCOL_VERSION        "0.1"
> +#define      PROTOCOL_VERSION        "0.2"
>  
>  struct filter;
>  struct filter_session;
> Index: lka_report.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/lka_report.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 lka_report.c
> --- lka_report.c      18 Aug 2019 16:52:02 -0000      1.24
> +++ lka_report.c      28 Aug 2019 08:32:33 -0000
> @@ -35,7 +35,7 @@
>  #include "smtpd.h"
>  #include "log.h"
>  
> -#define      PROTOCOL_VERSION        "0.1"
> +#define      PROTOCOL_VERSION        "0.2"
>  
>  struct reporter_proc {
>       TAILQ_ENTRY(reporter_proc)      entries;
> @@ -51,6 +51,7 @@ static struct smtp_events {
>  } smtp_events[] = {
>       { "link-connect" },
>       { "link-disconnect" },
> +     { "link-greeting" },
>       { "link-identify" },
>       { "link-tls" },
>       { "link-auth" },
> @@ -216,6 +217,14 @@ lka_report_smtp_link_disconnect(const ch
>  {
>       report_smtp_broadcast(reqid, direction, tv, "link-disconnect",
>           "%016"PRIx64"\n", reqid);
> +}
> +
> +void
> +lka_report_smtp_link_greeting(const char *direction, uint64_t reqid,
> +    struct timeval *tv, const char *domain, const char *textstring)
> +{
> +     report_smtp_broadcast(reqid, direction, tv, "link-greeting", "%s|%s\n",
> +         domain, textstring);
>  }
>  
>  void
> Index: report_smtp.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/report_smtp.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 report_smtp.c
> --- report_smtp.c     26 Jul 2019 06:30:13 -0000      1.8
> +++ report_smtp.c     28 Aug 2019 08:32:33 -0000
> @@ -64,6 +64,23 @@ report_smtp_link_connect(const char *dir
>  }
>  
>  void
> +report_smtp_link_greeting(const char *direction, uint64_t qid, const char 
> *domain,
> +    const char *textstring)
> +{
> +     struct timeval  tv;
> +
> +     gettimeofday(&tv, NULL);
> +
> +     m_create(p_lka, IMSG_REPORT_SMTP_LINK_GREETING, 0, 0, -1);
> +     m_add_string(p_lka, direction);
> +     m_add_timeval(p_lka, &tv);
> +     m_add_id(p_lka, qid);
> +     m_add_string(p_lka, domain);
> +     m_add_string(p_lka, textstring);
> +     m_close(p_lka);
> +}
> +
> +void
>  report_smtp_link_identify(const char *direction, uint64_t qid, const char 
> *method, const char *identity)
>  {
>       struct timeval  tv;
> Index: smtp_session.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
> retrieving revision 1.407
> diff -u -p -r1.407 smtp_session.c
> --- smtp_session.c    14 Aug 2019 21:11:25 -0000      1.407
> +++ smtp_session.c    28 Aug 2019 08:32:33 -0000
> @@ -2046,8 +2046,11 @@ smtp_proceed_connected(struct smtp_sessi
>  static void
>  smtp_send_banner(struct smtp_session *s)
>  {
> +     char textstring[256];
>       smtp_reply(s, "220 %s ESMTP %s", s->smtpname, SMTPD_NAME);
>       s->banner_sent = 1;
> +     snprintf(textstring, sizeof(textstring), "ESMTP %s", SMTPD_NAME);
> +     report_smtp_link_greeting("smtp-in", s->id, s->smtpname, textstring);
>  }
>  
>  void
> Index: smtpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
> retrieving revision 1.632
> diff -u -p -r1.632 smtpd.h
> --- smtpd.h   23 Aug 2019 07:09:52 -0000      1.632
> +++ smtpd.h   28 Aug 2019 08:32:33 -0000
> @@ -310,6 +310,7 @@ enum imsg_type {
>  
>       IMSG_REPORT_SMTP_LINK_CONNECT,
>       IMSG_REPORT_SMTP_LINK_DISCONNECT,
> +     IMSG_REPORT_SMTP_LINK_GREETING,
>       IMSG_REPORT_SMTP_LINK_IDENTIFY,
>       IMSG_REPORT_SMTP_LINK_TLS,
>       IMSG_REPORT_SMTP_LINK_AUTH,
> @@ -1332,6 +1333,8 @@ void lka_report_register_hook(const char
>  void lka_report_smtp_link_connect(const char *, struct timeval *, uint64_t, 
> const char *, int,
>      const struct sockaddr_storage *, const struct sockaddr_storage *);
>  void lka_report_smtp_link_disconnect(const char *, struct timeval *, 
> uint64_t);
> +void lka_report_smtp_link_greeting(const char *, uint64_t, struct timeval *, 
> const char *,
> +    const char *);
>  void lka_report_smtp_link_identify(const char *, struct timeval *, uint64_t, 
> const char *, const char *);
>  void lka_report_smtp_link_tls(const char *, struct timeval *, uint64_t, 
> const char *);
>  void lka_report_smtp_link_auth(const char *, struct timeval *, uint64_t, 
> const char *, const char *);
> @@ -1501,6 +1504,8 @@ int queue_message_walk(struct envelope *
>  void report_smtp_link_connect(const char *, uint64_t, const char *, int,
>      const struct sockaddr_storage *, const struct sockaddr_storage *);
>  void report_smtp_link_disconnect(const char *, uint64_t);
> +void report_smtp_link_greeting(const char *, uint64_t, const char *,
> +    const char *);
>  void report_smtp_link_identify(const char *, uint64_t, const char *, const 
> char *);
>  void report_smtp_link_tls(const char *, uint64_t, const char *);
>  void report_smtp_link_auth(const char *, uint64_t, const char *, const char 
> *);
> 
> +             m_msg(&m, imsg);
> +             m_get_string(&m, &direction);
> +             m_get_timeval(&m, &tv);
> +             m_get_id(&m, &reqid);
> +             m_get_string(&m, &domain);
> +             m_get_string(&m, &textstring);
> +             m_end(&m);
> +
> +             lka_report_smtp_link_greeting(direction, reqid, &tv, domain,
> +                 textstring);
>               return;
>  
>       case IMSG_REPORT_SMTP_LINK_DISCONNECT:
> Index: lka_filter.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 lka_filter.c
> --- lka_filter.c      18 Aug 2019 16:52:02 -0000      1.41
> +++ lka_filter.c      28 Aug 2019 08:32:33 -0000
> @@ -35,7 +35,7 @@
>  #include "smtpd.h"
>  #include "log.h"
>  
> -#define      PROTOCOL_VERSION        "0.1"
> +#define      PROTOCOL_VERSION        "0.2"
>  
>  struct filter;
>  struct filter_session;
> Index: lka_report.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/lka_report.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 lka_report.c
> --- lka_report.c      18 Aug 2019 16:52:02 -0000      1.24
> +++ lka_report.c      28 Aug 2019 08:32:33 -0000
> @@ -35,7 +35,7 @@
>  #include "smtpd.h"
>  #include "log.h"
>  
> -#define      PROTOCOL_VERSION        "0.1"
> +#define      PROTOCOL_VERSION        "0.2"
>  
>  struct reporter_proc {
>       TAILQ_ENTRY(reporter_proc)      entries;
> @@ -51,6 +51,7 @@ static struct smtp_events {
>  } smtp_events[] = {
>       { "link-connect" },
>       { "link-disconnect" },
> +     { "link-greeting" },
>       { "link-identify" },
>       { "link-tls" },
>       { "link-auth" },
> @@ -216,6 +217,14 @@ lka_report_smtp_link_disconnect(const ch
>  {
>       report_smtp_broadcast(reqid, direction, tv, "link-disconnect",
>           "%016"PRIx64"\n", reqid);
> +}
> +
> +void
> +lka_report_smtp_link_greeting(const char *direction, uint64_t reqid,
> +    struct timeval *tv, const char *domain, const char *textstring)
> +{
> +     report_smtp_broadcast(reqid, direction, tv, "link-greeting", "%s|%s\n",
> +         domain, textstring);
>  }
>  
>  void
> Index: report_smtp.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/report_smtp.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 report_smtp.c
> --- report_smtp.c     26 Jul 2019 06:30:13 -0000      1.8
> +++ report_smtp.c     28 Aug 2019 08:32:33 -0000
> @@ -64,6 +64,23 @@ report_smtp_link_connect(const char *dir
>  }
>  
>  void
> +report_smtp_link_greeting(const char *direction, uint64_t qid, const char 
> *domain,
> +    const char *textstring)
> +{
> +     struct timeval  tv;
> +
> +     gettimeofday(&tv, NULL);
> +
> +     m_create(p_lka, IMSG_REPORT_SMTP_LINK_GREETING, 0, 0, -1);
> +     m_add_string(p_lka, direction);
> +     m_add_timeval(p_lka, &tv);
> +     m_add_id(p_lka, qid);
> +     m_add_string(p_lka, domain);
> +     m_add_string(p_lka, textstring);
> +     m_close(p_lka);
> +}
> +
> +void
>  report_smtp_link_identify(const char *direction, uint64_t qid, const char 
> *method, const char *identity)
>  {
>       struct timeval  tv;
> Index: smtp_session.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
> retrieving revision 1.407
> diff -u -p -r1.407 smtp_session.c
> --- smtp_session.c    14 Aug 2019 21:11:25 -0000      1.407
> +++ smtp_session.c    28 Aug 2019 08:32:33 -0000
> @@ -2046,8 +2046,11 @@ smtp_proceed_connected(struct smtp_sessi
>  static void
>  smtp_send_banner(struct smtp_session *s)
>  {
> +     char textstring[256];
>       smtp_reply(s, "220 %s ESMTP %s", s->smtpname, SMTPD_NAME);
>       s->banner_sent = 1;
> +     snprintf(textstring, sizeof(textstring), "ESMTP %s", SMTPD_NAME);
> +     report_smtp_link_greeting("smtp-in", s->id, s->smtpname, textstring);
>  }
>  
>  void
> Index: smtpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
> retrieving revision 1.632
> diff -u -p -r1.632 smtpd.h
> --- smtpd.h   23 Aug 2019 07:09:52 -0000      1.632
> +++ smtpd.h   28 Aug 2019 08:32:33 -0000
> @@ -310,6 +310,7 @@ enum imsg_type {
>  
>       IMSG_REPORT_SMTP_LINK_CONNECT,
>       IMSG_REPORT_SMTP_LINK_DISCONNECT,
> +     IMSG_REPORT_SMTP_LINK_GREETING,
>       IMSG_REPORT_SMTP_LINK_IDENTIFY,
>       IMSG_REPORT_SMTP_LINK_TLS,
>       IMSG_REPORT_SMTP_LINK_AUTH,
> @@ -1332,6 +1333,8 @@ void lka_report_register_hook(const char
>  void lka_report_smtp_link_connect(const char *, struct timeval *, uint64_t, 
> const char *, int,
>      const struct sockaddr_storage *, const struct sockaddr_storage *);
>  void lka_report_smtp_link_disconnect(const char *, struct timeval *, 
> uint64_t);
> +void lka_report_smtp_link_greeting(const char *, uint64_t, struct timeval *, 
> const char *,
> +    const char *);
>  void lka_report_smtp_link_identify(const char *, struct timeval *, uint64_t, 
> const char *, const char *);
>  void lka_report_smtp_link_tls(const char *, struct timeval *, uint64_t, 
> const char *);
>  void lka_report_smtp_link_auth(const char *, struct timeval *, uint64_t, 
> const char *, const char *);
> @@ -1501,6 +1504,8 @@ int queue_message_walk(struct envelope *
>  void report_smtp_link_connect(const char *, uint64_t, const char *, int,
>      const struct sockaddr_storage *, const struct sockaddr_storage *);
>  void report_smtp_link_disconnect(const char *, uint64_t);
> +void report_smtp_link_greeting(const char *, uint64_t, const char *,
> +    const char *);
>  void report_smtp_link_identify(const char *, uint64_t, const char *, const 
> char *);
>  void report_smtp_link_tls(const char *, uint64_t, const char *);
>  void report_smtp_link_auth(const char *, uint64_t, const char *, const char 
> *);
> 

-- 
Gilles Chehade                                                 @poolpOrg

https://www.poolp.org            patreon: https://www.patreon.com/gilles

Reply via email to