Jared Johnson wrote: > > >> - Introduces support for URIBL services that may not have worked right, > >> at > >> least out of the box, before. Defines the subtle differences between > >> various known URIBL services in order to maximize compatibility. > > > > Is it worth pulling some of this config out of the code and putting it > > into some sort of config file? > > Probably would be worth it, and not that difficult to do. Any new config > scheme should probably be both backward compatible and non-spaghetti. The > current scheme looks like: > > multi.uribl.com 2,4 deny > > the new plugin does add one additional feature, you can now do: > > multi.uribl.com 2 deny > multi.uribl.com 4 log > > but $zones defines a bunch of other things - labels for the zone in > general and specific masks, check_ip, check_host, > host_wildcards_supported, lookup_a_record, lookup_ns_record. Ignoring the > label issue, all the other directives are related only to the zone; > perhaps we could add a new scheme on top of the mask definition scheme > (which we would continue to read in the old way), getting even further > away from the one-line-per-zone style: > > multi.uribl.com action deny > multi.uribl.com check_ip 1 > multi.uribl.com check_host 1 > multi.uribl.com lookup_a_record 0 > multi.uribl.com lookup_ns_record 0 > multi.uribl.com host_wildcards_support 0 > > If nobody hates this scheme it does seem like something I could add pretty > easily send an updated copy
I don't hate it, FWIW. :) Sticking with one line per zone is NOT a requirement. (Heck, I'd even be willing to go so far as saying we could go to a even more complicated config scheme. One of qpsmtpd's unwritten goals is to NOT pull in half of CPAN to run, but we could expand a little.) > > >> - Uses Net::DNS::Async to simplify the code, and also to ensure the > >> afore-mentioned A and NS lookups will prompt new URIBL lookups in an > >> efficient and simple manner via callbacks > > > > Does the code still work with the async qpsmtpd cores? > > Well... I didn't change the existing plugin a whole lot; I did change from > data_handler() registered via register(), to plain old hook_data_post(); > this may need changed considering that the comments above each sub (which > I also removed) suggested that hook_data_post() and register() were 'not > used' for async. These things could easily be changed if needed. However > we don't run async anywhere, not even in test environments, so I'm not > very qualified to know if they need changed, or give any guarantees. And > I'm confused by that bit. From what I can tell, the async/uribl plugin > ignores plugins/uribl entirely and uses > Qpsmtpd::Plugins::Async::DNSBLBase, which in turn uses ParaDNS. In that case, it's possible (and likely) that it may never have worked with async. When time is short, it's easy to just ask questions :) > > Now I noticed the async rhsbl plugin used Q::P::A::D and that this used > ParaDNS before and looked at it briefly to see if I could use it for my > purposes, but at the top of the docs it's mentioned that only A lookups > are supported, so I skipped it and went to Net::DNS::Async. I don't > really know the strongpoints of N::D::A vs. ParaDNS but it did note that > the latter more expressly claims to be interested in performance and > scalability. So in conclusion: > > (1) I think I'll look into using ParaDNS and maybe even > Q::P::Async::DNSBLBase if appropriate, even though my plugin is not in > async; it might still have advantages, and the interesting logic is in the > callbacks and not the module use anyway > (2) I don't know if it matters whether this particular plugin works with > async, I guess it depends on whether the old one really ever did. At > least Net::DNS::Async should not make it any more blocking than the old > plugin's collect_results(). At any rate somebody who plays in async > territory should probably comment before any action is taken on the new > plugin :) > > >> Attached also is tld_lists.pl, a companion file that needs to be dropped > >> in lib/Qpsmtpd/ which provides the list of first, second, and third > >> level > >> TLDs that we care about. It's derived from our URIBL datafeed as well > >> as > > > > Do the owners of that data care about it being used this way? You may > > be violating any agreement with them. Would they be ok if this was > > released as an independent CPAN module? Either way, can we > > structure this as an API instead of just an include file? > > (1) I suspect that anyone who runs a URIBL service would be thrilled with > this information being available for this particular use, because it > minimizes unnecessary queries to their services. Consider the old method > that sent 2 or 3 queries instead of one because it didn't know which one > was right. The fact that SURBL and URIBL both publish the two and three > level lists and encourage their use is evidence of this :) > > (2) First-level TLDs are public information, of course, and with the > two-level and three-level lists being public, the interesting business is > that the first-level list and the SURBL second and third level lists are > pruned based on URIBL's data. So it comes down to whether anyone can be > considered to have a claim on the privilege of knowing how to *exclude* > results. I guess maybe they could if it came down to it... > > (3) I'm pretty sure I could get URIBL's explicit permission, but it would > take a long time. The founder and sole owner of URIBL was also the > original author of DoubleCheck, so I feel we would have a bit of an in :) > However even so he always takes forever to get back to us in > correspondence. For instance we're still waiting around to hear from him > since I noticed, while working on the script that generated tld_lists.pl, > that our datafeed is no longer updating. Maybe he 'fixed the glitch'... > :P > > > It would be cool if it was configurable (i.e. use MIME::Parser or not.) > > That does sound handy, maybe I'll work on that soon. I think if one does > go back to the 'old' method, it would be good to skip any lines that > contain no whitespace, or some similar check that passes the > 3-million-line test, so that we no longer pointlessly scan reams of > base64-encoded data etc. for URI's Thanks Jared. This is an exciting change. -R
