Re: smtpd filters: banner hostname

2019-08-28 Thread gilles
28 août 2019 15:10 "Martijn van Duren"  a 
écrit:

> On 8/28/19 2:52 PM, gil...@poolp.org wrote:
> 
>> 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.
> 
> I'll commit it without the textstring.
> 

ok !


> I just don't understand what you mean by reject at banner.
> We only allow filtering on smtp-in and since we are the once that
> produce the banner, how will you filter at that point?

I still haven't fully gone through this but the bottom line is:

- today we have report link-connect and we have filter phase connect
- link-connect makes sense in getting info from the connection to build 
session, this is good
- filter phase connect is kinda of weird as it gets the same info as the 
previous report event which is not right and it answers results in the banner 
displayed, it's kind of a strange filter phase in my opinion

I think the filter phase connect should really be a filter phase "greeting" and 
this would get the session id to retrieve session state accumulated from report 
link-connect, as well as the banner as input, resulting in a proceed, rewrite, 
reject or disconnect action.

It would make more sense but i haven't gone completely through this to make 
sure its correct, I'll make a proposal soon.



Re: smtpd filters: banner hostname

2019-08-28 Thread Martijn van Duren
On 8/28/19 2:52 PM, gil...@poolp.org wrote:
> 
> 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.
> 
I'll commit it without the textstring.

I just don't understand what you mean by reject at banner.
We only allow filtering on smtp-in and since we are the once that 
produce the banner, how will you filter at that point?



Re: smtpd filters: banner hostname

2019-08-28 Thread gilles
28 août 2019 14:07 "Martijn van Duren"  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 , 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 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 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 

Re: smtpd filters: banner hostname

2019-08-28 Thread Martijn van Duren
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"  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 , 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 

Re: smtpd filters: banner hostname

2019-08-28 Thread gilles
28 août 2019 12:26 "Gilles Chehade"  a écrit:

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

By the way, as a side note, I already had a use-case which required a
banner filtering phase to allow rejecting at banner and which doesn't
need the server hostname so this tends to go in favour of what I said
above:

my filter would not register to receive the server hostname because I
don't care about that, but I'd still need to get the banner so I will
be able to take a decision to disconnect with a different message.



Re: smtpd filters: banner hostname

2019-08-28 Thread Gilles Chehade
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"  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 -  1.239
> +++ lka.c 28 Aug 2019 08:32:33 -
> @@ -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();
>  
>   lka_report_smtp_link_connect(direction, , reqid, rdns, 
> fcrdns, _src, _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 -  1.239
> +++ lka.c 28 Aug 2019 08:32:33 -
> @@ -84,6 +84,8 @@ lka_imsg(struct 

Re: smtpd filters: banner hostname

2019-08-28 Thread Martijn van Duren
On 8/28/19 9:23 AM, gil...@poolp.org wrote:
> 28 août 2019 09:04 "Martijn van Duren"  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?

martijn@

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 -  1.239
+++ lka.c   28 Aug 2019 08:32:33 -
@@ -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();
 
lka_report_smtp_link_connect(direction, , reqid, rdns, 
fcrdns, _src, _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 -  1.239
+++ lka.c   28 Aug 2019 08:32:33 -
@@ -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();
 
lka_report_smtp_link_connect(direction, , reqid, rdns, 
fcrdns, _src, _dest);
+   return;
+
+   case IMSG_REPORT_SMTP_LINK_GREETING:
+   m_msg(, imsg);
+   m_get_string(, );
+   m_get_timeval(, );
+   m_get_id(, );
+   m_get_string(, );
+   m_get_string(, );
+   m_end();
+
+   lka_report_smtp_link_greeting(direction, reqid, , 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.c18 Aug 2019 16:52:02 -  1.41
+++ lka_filter.c28 Aug 2019 08:32:33 -
@@ -35,7 +35,7 @@
 #include "smtpd.h"
 #include "log.h"
 
-#definePROTOCOL_VERSION"0.1"
+#definePROTOCOL_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.c18 Aug 2019 16:52:02 -  1.24
+++ lka_report.c28 Aug 2019 08:32:33 -
@@ -35,7 +35,7 @@
 #include "smtpd.h"
 #include "log.h"
 
-#definePROTOCOL_VERSION"0.1"
+#definePROTOCOL_VERSION"0.2"
 
 struct reporter_proc {
TAILQ_ENTRY(reporter_proc)  entries;

Re: smtpd filters: banner hostname

2019-08-28 Thread Martijn van Duren
On 8/28/19 9:23 AM, gil...@poolp.org wrote:
> 28 août 2019 09:04 "Martijn van Duren"  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 ?

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.

I can use gethostname(3), but that could contradict the banner, hence I
want to use the hostname used in the banner.
> 
> 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.

You're right. I'll cook up something else.
> 
> 
>> 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 - 1.239
>> +++ lka.c 28 Aug 2019 06:28:33 -
>> @@ -88,6 +88,7 @@ lka_imsg(struct mproc *p, struct imsg *i
>> const char *heloname;
>> const char *filter_name;
>> const char *result;
>> + const char *banner;
>> struct sockaddr_storage ss_src, ss_dest;
>> int filter_response;
>> int filter_phase;
>> @@ -388,9 +389,11 @@ lka_imsg(struct mproc *p, struct imsg *i
>> m_get_int(, );
>> m_get_sockaddr(, (struct sockaddr *)_src);
>> m_get_sockaddr(, (struct sockaddr *)_dest);
>> + m_get_string(, );
>> m_end();
>>
>> - lka_report_smtp_link_connect(direction, , reqid, rdns, fcrdns, _src, 
>> _dest);
>> + lka_report_smtp_link_connect(direction, , reqid, rdns,
>> + fcrdns, _src, _dest, banner);
>> return;
>>
>> case IMSG_REPORT_SMTP_LINK_DISCONNECT:
>> 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 - 1.24
>> +++ lka_report.c 28 Aug 2019 06:28:33 -
>> @@ -165,10 +165,10 @@ report_smtp_broadcast(uint64_t reqid, co
>> }
>>
>> void
>> -lka_report_smtp_link_connect(const char *direction, struct timeval *tv, 
>> uint64_t reqid, const char
>> *rdns,
>> - int fcrdns,
>> +lka_report_smtp_link_connect(const char *direction, struct timeval *tv,
>> + uint64_t reqid, const char *rdns, int fcrdns,
>> const struct sockaddr_storage *ss_src,
>> - const struct sockaddr_storage *ss_dest)
>> + const struct sockaddr_storage *ss_dest, const char *banner)
>> {
>> char src[NI_MAXHOST + 5];
>> char dest[NI_MAXHOST + 5];
>> @@ -207,8 +207,8 @@ lka_report_smtp_link_connect(const char 
>> }
>>
>> report_smtp_broadcast(reqid, direction, tv, "link-connect",
>> - "%016"PRIx64"|%s|%s|%s|%s\n",
>> - reqid, rdns, fcrdns_str, src, dest);
>> + "%016"PRIx64"|%s|%s|%s|%s|%s\n",
>> + reqid, rdns, fcrdns_str, src, dest, banner);
>> }
>>
>> 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 - 1.8
>> +++ report_smtp.c 28 Aug 2019 06:28:33 -
>> @@ -46,7 +46,7 @@
>> void
>> report_smtp_link_connect(const char *direction, uint64_t qid, const char 
>> *rdns, int fcrdns,
>> const struct sockaddr_storage *ss_src,
>> - const struct sockaddr_storage *ss_dest)
>> + const struct sockaddr_storage *ss_dest, const char *banner)
>> {
>> struct timeval tv;
>>
>> @@ -60,6 +60,7 @@ report_smtp_link_connect(const char *dir
>> m_add_int(p_lka, fcrdns);
>> m_add_sockaddr(p_lka, (const struct sockaddr *)ss_src);
>> m_add_sockaddr(p_lka, (const struct sockaddr *)ss_dest);
>> + m_add_string(p_lka, banner);
>> m_close(p_lka);
>> }
>>
>> Index: smtp_session.c
>> 

Re: smtpd filters: banner hostname

2019-08-28 Thread gilles
28 août 2019 09:04 "Martijn van Duren"  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.


> 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 - 1.239
> +++ lka.c 28 Aug 2019 06:28:33 -
> @@ -88,6 +88,7 @@ lka_imsg(struct mproc *p, struct imsg *i
> const char *heloname;
> const char *filter_name;
> const char *result;
> + const char *banner;
> struct sockaddr_storage ss_src, ss_dest;
> int filter_response;
> int filter_phase;
> @@ -388,9 +389,11 @@ lka_imsg(struct mproc *p, struct imsg *i
> m_get_int(, );
> m_get_sockaddr(, (struct sockaddr *)_src);
> m_get_sockaddr(, (struct sockaddr *)_dest);
> + m_get_string(, );
> m_end();
> 
> - lka_report_smtp_link_connect(direction, , reqid, rdns, fcrdns, _src, 
> _dest);
> + lka_report_smtp_link_connect(direction, , reqid, rdns,
> + fcrdns, _src, _dest, banner);
> return;
> 
> case IMSG_REPORT_SMTP_LINK_DISCONNECT:
> 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 - 1.24
> +++ lka_report.c 28 Aug 2019 06:28:33 -
> @@ -165,10 +165,10 @@ report_smtp_broadcast(uint64_t reqid, co
> }
> 
> void
> -lka_report_smtp_link_connect(const char *direction, struct timeval *tv, 
> uint64_t reqid, const char
> *rdns,
> - int fcrdns,
> +lka_report_smtp_link_connect(const char *direction, struct timeval *tv,
> + uint64_t reqid, const char *rdns, int fcrdns,
> const struct sockaddr_storage *ss_src,
> - const struct sockaddr_storage *ss_dest)
> + const struct sockaddr_storage *ss_dest, const char *banner)
> {
> char src[NI_MAXHOST + 5];
> char dest[NI_MAXHOST + 5];
> @@ -207,8 +207,8 @@ lka_report_smtp_link_connect(const char 
> }
> 
> report_smtp_broadcast(reqid, direction, tv, "link-connect",
> - "%016"PRIx64"|%s|%s|%s|%s\n",
> - reqid, rdns, fcrdns_str, src, dest);
> + "%016"PRIx64"|%s|%s|%s|%s|%s\n",
> + reqid, rdns, fcrdns_str, src, dest, banner);
> }
> 
> 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 - 1.8
> +++ report_smtp.c 28 Aug 2019 06:28:33 -
> @@ -46,7 +46,7 @@
> void
> report_smtp_link_connect(const char *direction, uint64_t qid, const char 
> *rdns, int fcrdns,
> const struct sockaddr_storage *ss_src,
> - const struct sockaddr_storage *ss_dest)
> + const struct sockaddr_storage *ss_dest, const char *banner)
> {
> struct timeval tv;
> 
> @@ -60,6 +60,7 @@ report_smtp_link_connect(const char *dir
> m_add_int(p_lka, fcrdns);
> m_add_sockaddr(p_lka, (const struct sockaddr *)ss_src);
> m_add_sockaddr(p_lka, (const struct sockaddr *)ss_dest);
> + m_add_string(p_lka, banner);
> m_close(p_lka);
> }
> 
> 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 - 1.407
> +++ smtp_session.c 28 Aug 2019 06:28:33 -
> @@ -2029,7 +2029,7 @@ smtp_connected(struct smtp_session *s)
> smtp_filter_begin(s);
> 
> report_smtp_link_connect("smtp-in", s->id, s->rdns, s->fcrdns, >ss,
> - >listener->ss);
> + >listener->ss, s->smtpname);
> 
> smtp_filter_phase(FILTER_CONNECT, s, ss_to_text(>ss));
> }
> Index: smtpd.h
> 

smtpd filters: banner hostname

2019-08-28 Thread Martijn van Duren
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?

martijn@

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 -  1.239
+++ lka.c   28 Aug 2019 06:28:33 -
@@ -88,6 +88,7 @@ lka_imsg(struct mproc *p, struct imsg *i
const char  *heloname;
const char  *filter_name;
const char  *result;
+   const char  *banner;
struct sockaddr_storage ss_src, ss_dest;
int  filter_response;
int  filter_phase;
@@ -388,9 +389,11 @@ lka_imsg(struct mproc *p, struct imsg *i
m_get_int(, );
m_get_sockaddr(, (struct sockaddr *)_src);
m_get_sockaddr(, (struct sockaddr *)_dest);
+   m_get_string(, );
m_end();
 
-   lka_report_smtp_link_connect(direction, , reqid, rdns, 
fcrdns, _src, _dest);
+   lka_report_smtp_link_connect(direction, , reqid, rdns,
+   fcrdns, _src, _dest, banner);
return;
 
case IMSG_REPORT_SMTP_LINK_DISCONNECT:
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.c18 Aug 2019 16:52:02 -  1.24
+++ lka_report.c28 Aug 2019 06:28:33 -
@@ -165,10 +165,10 @@ report_smtp_broadcast(uint64_t reqid, co
 }
 
 void
-lka_report_smtp_link_connect(const char *direction, struct timeval *tv, 
uint64_t reqid, const char *rdns,
-int fcrdns,
+lka_report_smtp_link_connect(const char *direction, struct timeval *tv,
+uint64_t reqid, const char *rdns, int fcrdns,
 const struct sockaddr_storage *ss_src,
-const struct sockaddr_storage *ss_dest)
+const struct sockaddr_storage *ss_dest, const char *banner)
 {
charsrc[NI_MAXHOST + 5];
chardest[NI_MAXHOST + 5];
@@ -207,8 +207,8 @@ lka_report_smtp_link_connect(const char 
}

report_smtp_broadcast(reqid, direction, tv, "link-connect",
-   "%016"PRIx64"|%s|%s|%s|%s\n",
-   reqid, rdns, fcrdns_str, src, dest);
+   "%016"PRIx64"|%s|%s|%s|%s|%s\n",
+   reqid, rdns, fcrdns_str, src, dest, banner);
 }
 
 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 -  1.8
+++ report_smtp.c   28 Aug 2019 06:28:33 -
@@ -46,7 +46,7 @@
 void
 report_smtp_link_connect(const char *direction, uint64_t qid, const char 
*rdns, int fcrdns,
 const struct sockaddr_storage *ss_src,
-const struct sockaddr_storage *ss_dest)
+const struct sockaddr_storage *ss_dest, const char *banner)
 {
struct timeval  tv;
 
@@ -60,6 +60,7 @@ report_smtp_link_connect(const char *dir
m_add_int(p_lka, fcrdns);
m_add_sockaddr(p_lka, (const struct sockaddr *)ss_src);
m_add_sockaddr(p_lka, (const struct sockaddr *)ss_dest);
+   m_add_string(p_lka, banner);
m_close(p_lka);
 }
 
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 -  1.407
+++ smtp_session.c  28 Aug 2019 06:28:33 -
@@ -2029,7 +2029,7 @@ smtp_connected(struct smtp_session *s)
smtp_filter_begin(s);
 
report_smtp_link_connect("smtp-in", s->id, s->rdns, s->fcrdns, >ss,
-   >listener->ss);
+   >listener->ss, s->smtpname);
 
smtp_filter_phase(FILTER_CONNECT, s, ss_to_text(>ss));
 }
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 -  1.632
+++ smtpd.h 28 Aug 2019 06:28:33 -
@@ -1330,7 +1330,7 @@ struct io *lka_proc_get_io(const char *)