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
Message
Re: [milter-greylist] Submitter DNS name resolution and forgery detection
2013-08-11 by Jim Klimov
Attachments
- No local attachments were found for this message.