Milter-greylist

My MTA (Sun/Oracle CommSuite Messaging Server) does not have an
{if_addr} macro to designate which IP address of the MTA received
the SMTP connection. This value, if available, is used in two
places in the code so far:

1) To build the X-Greylist string part "(my_hostname [my_ip])"
Failure to fetch just fills in the IPv4 or IPv6 localhost.

2) To match destinations for P0F fingerprinting
Failure to fetch the macro essentially disables p0f

In an earlier episode I had a go at replacing this macro with
another, but that was a rather foolish attempt :)

Now I've added a toggle "nolog_missing_if_addr" that can be set
in the config file to simply disable the logging of the string
"smfi_getsymval failed for {if_addr}" if debug logging is on.
In my case, this line carries no informative meaning and just
pollutes the logs.

Now, a separate question is if we can (or should) try to detect
a "reasonable" value ourselves, in the milter, in case the macro
value is not provided - i.e. try the only non-loopback IP address
if there is only one, or review the system's SMTP connections
like "netstat -an | grep ESTABLISHED" to find one(s) with that
remote host and see which local IP address is used, if any?..
As a possible aid for the latter technique, is there any macro that
would pass us the remote relay's port number - just like we can have
its host name and/or IP address?

Thanks,
//Jim Klimov

Jim Klimov <jimklimov@...> wrote:

> Now I've added a toggle "nolog_missing_if_addr" that can be set
> in the config file to simply disable the logging of the string
> "smfi_getsymval failed for {if_addr}" if debug logging is on.
> In my case, this line carries no informative meaning and just
> pollutes the logs.

Your config option is ugly! :-)

We could just report the error once, and remain silent for later
messages. After all the thing is configured,(or not configured) in
sendmail, and will produce the same result for all messages. Having more
than one alert is probably not useful.
-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...

On 2013-08-11 06:32, manu@... wrote:
> Jim Klimov <jimklimov@... <mailto:jimklimov%40cos.ru>> wrote:
>
>  > Now I've added a toggle "nolog_missing_if_addr" that can be set
>  > in the config file to simply disable the logging of the string
>  > "smfi_getsymval failed for {if_addr}" if debug logging is on.
>  > In my case, this line carries no informative meaning and just
>  > pollutes the logs.
>
> Your config option is ugly! :-)

The name? Probably so, but it won the role in a casting competition ;)
First names were even uglier and less to the point, for example,
"nomacro_if_addr" - does it handle not-logging or not-handling at all?
And it's not like we're gonna see it every day on each installation...


> We could just report the error once, and remain silent for later
> messages. After all the thing is configured,(or not configured) in
> sendmail, and will produce the same result for all messages. Having more
> than one alert is probably not useful.

That was another option I considered (first, comparing the efforts),
but I was not sure if absence or emptiness of {if_addr} could be a
transient error - i.e. that something could be passed with some
milter API streams, and not passed with others... So I decided to
make a config toggle, to be certain :)

//Jim

Jim Klimov <jimklimov@...> wrote:

> That was another option I considered (first, comparing the efforts),
> but I was not sure if absence or emptiness of {if_addr} could be a
> transient error - i.e. that something could be passed with some
> milter API streams, and not passed with others... So I decided to
> make a config toggle, to be certain :)

In fact I understand we get it each time a mail is received through a
Unix socket. I think we should  completely get rid of the message, as it
fail to distinguish a config error from normal situation.

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...

On 2013-08-11 04:12, Jim Klimov wrote:
> Now, a separate question is if we can (or should) try to detect
> a "reasonable" value ourselves, in the milter, in case the macro
> value is not provided - i.e. try the only non-loopback IP address
> if there is only one, or review the system's SMTP connections
> like "netstat -an | grep ESTABLISHED" to find one(s) with that
> remote host and see which local IP address is used, if any?..
> As a possible aid for the latter technique, is there any macro that
> would pass us the remote relay's port number - just like we can have
> its host name and/or IP address?


I see an as-of-yet undocumented config keyword "localaddr" which
can be defined, apparently, for Postfix users to use "spf self".
I wonder if it is for any reason NOT proper to use this value (if
defined) in place of missing {if_addr} values in the two other
places?

Also, would the netstat-like idea outlined above be feasible for
setting the value automatically (per-connection)?

//Jim

On 2013-08-11 11:58, manu@... wrote:
> Jim Klimov <jimklimov@... <mailto:jimklimov%40cos.ru>> wrote:
>
>  > That was another option I considered (first, comparing the efforts),
>  > but I was not sure if absence or emptiness of {if_addr} could be a
>  > transient error - i.e. that something could be passed with some
>  > milter API streams, and not passed with others... So I decided to
>  > make a config toggle, to be certain :)
>
> In fact I understand we get it each time a mail is received through a
> Unix socket. I think we should completely get rid of the message, as it
> fail to distinguish a config error from normal situation.

This I cannot vouch for or against... It would seem logical for the API
client capable of setting the macro (Sendmail) to do so regardless of
the socket type used to interact with the milter?

Or did I not understand your point?

/Jim

Jim Klimov <jimklimov@...> wrote:

> I see an as-of-yet undocumented config keyword "localaddr" which
> can be defined, apparently, for Postfix users to use "spf self".
> I wonder if it is for any reason NOT proper to use this value (if
> defined) in place of missing {if_addr} values in the two other
> places?

No, it seems fine to do so
 
> Also, would the netstat-like idea outlined above be feasible for
> setting the value automatically (per-connection)?

You want to run netstat in popen()? Or use the system non standard API
to retreive the information?

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...

On 2013-08-11 15:43, manu@... wrote:
> Jim Klimov <jimklimov@... <mailto:jimklimov%40cos.ru>> wrote:
>
>  > I see an as-of-yet undocumented config keyword "localaddr" which
>  > can be defined, apparently, for Postfix users to use "spf self".
>  > I wonder if it is for any reason NOT proper to use this value (if
>  > defined) in place of missing {if_addr} values in the two other
>  > places?
>
> No, it seems fine to do so
>
>  > Also, would the netstat-like idea outlined above be feasible for
>  > setting the value automatically (per-connection)?
>
> You want to run netstat in popen()? Or use the system non standard API
> to retreive the information?


I thought that maybe "netstat -an" relies on some standard API
to list the kernel's established TCP sessions on various OSes.
Forking an executable for each connection would of course be an
overkill. At most - make a netstat daemon (or thread) and query
it with a pipe ;)


Given your reply above, I think for many intents and purposes
the single value defined by admin in a config file would do.
It just has to become not only defined for POSIX, and used in
place of missing {if_addr} in those two spots. And also it
should become documented, at all and for such use in particular :)

I cannot promise I'd look into this anytime soon, there are some
quests at $work to do with higher priority... Then again, I thought
so about several of the features I've committed this weekend ;)

//Jim

Jim Klimov <jimklimov@...> wrote:

> I thought that maybe "netstat -an" relies on some standard API
> to list the kernel's established TCP sessions on various OSes.

There is no such standard API, unfortunately. Older system peek at
kernel internals through /dev/kmem. Modern systems use sysctl, but the
data format is not standardized.

> Forking an executable for each connection would of course be an
> overkill. At most - make a netstat daemon (or thread) and query
> it with a pipe ;)

Your netstat is able to loop reporting information? Mine does not. 

> Given your reply above, I think for many intents and purposes
> the single value defined by admin in a config file would do.

Yes, localaddr is the least resistance path to solving the problem, IMO.

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...

On 2013-08-11 17:42, Jim Klimov wrote:
> Given your reply above, I think for many intents and purposes
> the single value defined by admin in a config file would do.
> It just has to become not only defined for POSIX, and used in

-POSIX
+Postfix
:)

> place of missing {if_addr} in those two spots. And also it
> should become documented, at all and for such use in particular :)
>
> I cannot promise I'd look into this anytime soon, there are some
> quests at $work to do with higher priority... Then again, I thought
> so about several of the features I've committed this weekend ;)

So here is a couple of patches: one allows use of "localaddr" by
local_ipstr() for '%V' format string parsing, I believe; its result
can be seen in the X-Greylist headers; and also for p0f which I did
not yet test.

Given that our mail relay is on a "virtual" IP address behind NAT,
it might be reasonable to actually use two separate variables here
(IP on this host's NIC for p0f, and public IP on NAT for "spf self")...
but so far so much.

This change also removes the ifdef's which limited the logic for
Postfix at compile-time, and now similar decisions are made run-time
based on availability of either if_addr or localaddr.

Also introduced is a conf.c_localaddr_string character array to only
generate the string once (via conf_lex).

A follow-up idea might be to add format strings for local host's
address and name...



Another patch deals with some more verbosity in SPF lookup progress
in the debug logs (which result are we testing for, and did we hit
it, in human-readable terms - taking result={0,1} as readable).

BTW, I believe that these SPF lookups are needlessly done for each
invokation of the keyword? While they may be cached by DNS, there is
still an overhead of processing. Can't (or don't already?) we cache
the first SPF test's result in this SMTP dialog and later on just
match it to the currently tested expected result (pass, fail, etc.)?



HTH,
//Jim Klimov

Jim Klimov <jimklimov@...> wrote:

> So here is a couple of patches: one allows use of "localaddr" by
> local_ipstr() for '%V' format string parsing, I believe; its result
> can be seen in the X-Greylist headers; and also for p0f which I did
> not yet test.

You forgot the bits from conf.c and conf.h. I added them and refactored
the patch a bit. For instance, local_ipstr() never returns NULL,
therefore there is no need to check it. 

I also modified %V so that [0.0.0.0] does appear if things are
misconfigured. That will help admins fixing it.

Here is my version, please tell me if that is fine for you:
http://ftp.espci.fr/shadow/manu/localaddr.patch

I checked in your other patch on SPF verbosity.

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...

On 2013-08-12 06:21, manu@... wrote:
> Jim Klimov <jimklimov@... <mailto:jimklimov%40cos.ru>> wrote:
>
>  > So here is a couple of patches: one allows use of "localaddr" by
>  > local_ipstr() for '%V' format string parsing, I believe; its result
>  > can be seen in the X-Greylist headers; and also for p0f which I did
>  > not yet test.
>
> You forgot the bits from conf.c and conf.h. I added them

Huh? I think they are in your patch as well as in mine...

 > and refactored
> the patch a bit. For instance, local_ipstr() never returns NULL,
> therefore there is no need to check it.

So far it doesn't... I did consider it at one point as a cheaper
check than strncmp(), however.

> I also modified %V so that [0.0.0.0] does appear if things are
> misconfigured. That will help admins fixing it.

In this case, the "char* ipstr" is superfluous, and you can keep
the old logic which invoked local_ipstr() right from snprintf().

>
> Here is my version, please tell me if that is fine for you:
> http://ftp.espci.fr/shadow/manu/localaddr.patch

Ok, makes sense - less ambiguities and code-duplication such as in
p0f.c, and (for other readers) this also obsoletes my other recent
"toggle"  milter-greylist-4.4.3-nolog_missing_if_addr.patch

> I checked in your other patch on SPF verbosity.

Thanks :)

//Jim Klimov

Jim Klimov <jimklimov@...> wrote:

> > I also modified %V so that [0.0.0.0] does appear if things are
> > misconfigured. That will help admins fixing it.
> 
> In this case, the "char* ipstr" is superfluous, and you can keep
> the old logic which invoked local_ipstr() right from snprintf().

Right, I fixed it and checked the patch in.

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...