On 8/28/19 12:26 PM, Gilles Chehade wrote:
> 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.

Yes, because that's the only point in the transaction where I can be
100% certain on which hostname the transaction took place. This is
because of listen on * hostnames <table>, where table can change at
runtime based on its backend.
> 
> 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.

It's not related to my use-case. I included it for completeness.
E.g. if someone wants to monitor what mailservers are being connected to
in case of when we implement the smtp-out case.
Since there's no uniform way to distil this information (it's freetext
after all) I thought I'd return the textstring as specified by RFC5321.
If you see harm in having the textstring part in the filters I don't
mind dropping them, but I expect at some point people will ask for it.
> 
> 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.

No, SPF filters at HELO/EHLO and MAIL FROM. If a host is rejected
(because of a -all match) and the spf-record contains an "exp" (explain)
entry it resolves fetches the txt record of where "exp" points to.
Said record is then being evaluated by domain-spec and allows for the
expansion of "%{r}", which evaluates to the domainname of the host
doing the spf-checking.

Say I would mail from openbsd+t...@imperialat.at to gil...@poolp.org and
changed the txt imperialat.at to "v=spf1 mx -all exp:exp.imperialat.at"
and txt exp.imperialat.at would be: "Message rejected by %{r}"
If mx2.pool.org would reject the message because the sending ip is not
listed in the spf imperialat.at, the resulting rejecting status would
be:
"550 5.7.1 Message rejected by mx2.poolp.org".

See RFC7208 section 6.2, 7.2, 7,3 and 8.4

So for the above I would need to know what the hostname of the
parsing agent is. According to RFC5321 this information is available in
the greeting directly after the status code. So if I were to register
this on report|smtp-in|link-greeting I would have the domain name and I
can cache this information on my session variable until I (potentially)
need it during filter|helo, filter|ehlo and filter|mailfrom.
> 
> 
> 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 ?

I don't want to do anything at banner, I just want to know the banner,
or more specifically, the hostname in the banner.
> 
> 
>> 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 
>> *);
>>
> 

Reply via email to