28 août 2019 14:07 "Martijn van Duren" <[email protected]> a 
écrit:
>> 
>> 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.
> 

Ok, so far so good :-)


>> 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.
> 

There's no harm just as there's no harm adding more information to pretty
much any reporting event: "no harm" shouldn't be the argument.

The hostname makes a lot of sense because it is part of the session, this
means that currently you can't fully rebuild a session state in a filter,
we need a report event to allow filters to learn that hostname for sure.

The textstring part doesn't because:

a- it isn't a session state, the SMTP engine doesn't track it, it serves no 
purpose in a session
b- ALL report events will have the same "ESMTP OpenSMTPD" value, it carries no 
information, it is hardcoded
c- if there is a use-case that really required it, it can still be obtained 
with protocol-server events

For b-, smtp-out will not be hardcoded but c- is still valid and if it so
happens that displaying the banner part is a very common use-case, we are
still allowed to create a specific report event.

For the time being, I think your diff without textstring is ok



>> 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.
> 

Ok, so you can use actual filter hooks


> Say I would mail from [email protected] to [email protected] 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 understand and do you agree that link-greeting _without_ textstring is
not preventing you from doing that ?

Because nowhere is the textstring used in this pattern.


>> 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.
> 

Yes sorry my bad, but this still covers the textstring argument:

We need to be able to reject at banner and this is going to be implemented
as a filter phase, so basically if we expect people to need it for filters
the proper place to pass your textstring to filters is in that filter phase.



>>> 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