Yahoo Groups archive

Milter-greylist

Index last updated: 2026-04-28 23:32 UTC

Message

Re: [milter-greylist] Submitter DNS name resolution and forgery detection

2013-08-11 by Jim Klimov

Okay, so here goes the third version of the un-bracketer :)

There was quite a bit of indentation, so formatting may still be a bit
ugly, especially around the debug-printers; but at least it should all
fit into 80 chars now, sometimes just barely missing the mark ;)

I've also moved the whole routine into a separate function as proposed
earlier, so now we have a generic DNS PTR resolver that can be fed both
bracketed and "clear" dot-quad IP numbers (resolution of "clear" IPs
was still not tested though; resolution of bracketed strings works as
it did before the code-move). If the resolution fails at any step, the
original hostname (IP number, bracketed or clear) is copied into the
target storage string.

I am not sure how much this routine would be usable in the forgery
tests I had in mind (compare the HELO string, reverse name for the IP
address and the forward IPs for these names, and "pass" the test if
these match "strictly" or at least are in the same domain "loosely").
The problem is that while preparing this code, I've read somewhere
that in general, several PTR entries for one IP number are allowed
(even though few programs implement and handle this). For the forgery
test to be correct, this should iterate for all PTR entries and their
forward A/CNAME->A resolutions. The function I made so far only returns
the last resolved PTR entry (though it should debug-log them all if
it ever encounters several replies to one IP.in-addr.arpa. request).


On 2013-08-11 02:06, Emmanuel Dreyfus wrote:
> Jim Klimov <jimklimov@...> wrote:
>
>> I tried to adhere to this myself, but I guess a few lines slipped
>> past, mostly due to long strings or tabbing... as an important
>> factor: what indentation to the unpublished guidelines prefer?
>> 4-space tabs? 8? Another? ;)
>
> The Unpublished Rules says that tabs are used as 8 spaces :-)
>
>> Would you require the style and/or structure changes like this
>> before accepting the patch?
>
> Well, since I will do it if you do not, I of course prefer if you do.

Done ... hopefully ... ummm... acceptably now :)
As I said, some indentations remained "compressed" into 4 spaces,
but mostly 8-long tabs are used.

> It would also be nice to move the thing that happen after if (
> priv->priv_hostname[0] == '[' ) to a separate function, for the sake of
> readability.

Done, there's even more sake's involved :)

 >  > Why ADDRLEN+1 and not the NS_MAXDNAME+1 ?
 >
 > Because it was introduced at a time milter-greylist did not do any DNS
 > lookup on its own, and therefore it did not include the header that
 > would have given it NS_MAXDNAME. I guess NS_MAXDNAME makes more sense
 > now we have it at hand.

To remain compatible with current code, left ADDRLEN+1 in my additions,
and commented on that discrepancy. Changes to the more reasonable value
should be done carefully by inspecting all locations, and not just some
point-and-should that I did initially. Not that we're likely to overflow
either one with real names - but some spammer-squatters might like to
crash SMTP filters by forging max-sized names and hunting for such
one-off discrepancies, I suppose...

HTH,
//Jim Klimov

Attachments

Move to quarantaine

This moves the raw source file on disk only. The archive index is not changed automatically, so you still need to run a manual refresh afterward.