Re: Rewritten URIBL plugin
- 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 - 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. 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
Re: Rewritten URIBL plugin
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? Forgot to mention, yes, I'd be happy to add the few lines it would take to make this an API, even if it's just in lib/Qpsmtpd. It might be nice for us to get datafeeds with other services (we're looking to at least get one from DBL, possibly SURBL and imvURI) before publishing to CPAN, mainly so that we know for sure that we have not over-pruned the first level or SURBL second third level lists. As they are though, I consider the lists good enough to warrant production use of this method over the current method in the existing QP uribl plugin. After all any TLD that manages to make it onto other services but has garnered *zero* hits on URIBL, probably has very very few entries on those other services. Anyway, if it's a bit of a licensing/copyright problem to use the data from URIBL, it'll be more of one to use the data from other services as well, but we can probably work that question in from the get-go -Jared
Re: Rewritten URIBL plugin
On Sun, Jul 25, 2010 at 04:43:36AM -0500, Jared Johnson wrote: The plugin has the following advantages over the original: Based on a single read-through of the code, I like most of it. Some assorted observations, though: - Parsing out the MIME sections is worthwhile, though it's really something that ought to be done in the core libs when plugins indicate they need it, or in a utility plugin if that's practical. This was something I meant to bring up a while back -- we have quite a few different content-scanning plugins, and if they're all obliged to invoke MIME::Parser repeatedly it'll be even more costly than that module already is. Not that I have time to do it myself, though. :P - For messages below a few hundred kb or so, consider MIME::Parser-output_to_core(), so we can avoid the fs churn. Annoyingly, MIME::Parser doesn't provide a mechanism to only consider particular mimetypes, or we could skip decoding non-text bodies. - It looks like you're scanning inside all parts of multipart MIME, where doing so only on text parts might be preferable. - The config really needs to live in a config file -- changing or expanding the format is fine if needed, but config that lives in code can't really be edited. I'm not bothered by having per-list defaults in the plugin, but one should be able to fully add a new list or alter an existing one without changing Perl code. - In the unwinding of HTML entity escaping -- I suggest doing amp; last. amp;gt; is gt;, not . the SURBL two-level and three-level lists -- but the SURBL lists are pruned to exclude stuff that we're pretty sure is never going to actually get listed -- when was the last time you saw a spammer use .mil? uribl.com never has, according to our datafeed :) We'll probably see compromised .gov and .mil sites hosting malware, much as we see in other domains. They're not that useful for classical spammer purposes since they can't be registered in the usual fashion. Devin -- Devin \ aqua(at)devin.com, IRC:Requiem; http://www.devin.com Carraway \ 1024D/E9ABFCD2: 13E7 199E DD1E 65F0 8905 2E43 5395 CA0D E9AB FCD2
Re: tls plugin and SSL version
On Sun, 25 Jul 2010, Matt Simerson wrote: Here's a chunk of code from one of my projects: A config file setting allows an admin to choose from; all, high, medium, or pci. my $s = $ciphers eq 'all'? 'ALL' : $ciphers eq 'high' ? 'HIGH:!SSLv2' : $ciphers eq 'medium' ? 'HIGH:MEDIUM:!SSLv2' : $ciphers eq 'pci'? 'DEFAULT:!ADH:!LOW:!EXP:!SSLv2:+HIGH:+MEDIUM' :'DEFAULT'; Then you set SSL_cipher_list in the call to IO::Socket::SSL::SSL_Context-new. Please note that SSL_version is distinct from cipher list, and is not settable in the current code. Matt PS: IIRC, I pulled the high, medium, low settings out of the openssl docs. On Jul 22, 2010, at 7:29 PM, Charlie Brady wrote: I've seen some reports that qpsmtp fails some PCI compliance testing because it can be accessed via SSLv2. http://en.wikipedia.org/wiki/Payment_Card_Industry_Data_Security_Standard http://bugs.contribs.org/show_bug.cgi?id=6141 Here's a simple, and untested, patch - someone might care to do something more elaborate to allow choice of TLSv1 or SSLv3 (unfortunately IO::Socket::SSL doesn't seem to allow disable of just SSLv2). --- qpsmtpd-0.83/plugins/tls.orig 2010-07-22 22:04:00.0 -0400 +++ qpsmtpd-0.83/plugins/tls2010-07-22 22:09:35.0 -0400 @@ -80,6 +80,7 @@ local $^W; # this bit is very noisy... my $ssl_ctx = IO::Socket::SSL::SSL_Context-new( SSL_use_cert = 1, +SSL_version = 'TLSv1', SSL_cert_file = $self-tls_cert, SSL_key_file = $self-tls_key, SSL_ca_file = $self-tls_ca, @@ -176,6 +177,7 @@ my $tlssocket = IO::Socket::SSL-new_from_fd( fileno(STDIN), '+', SSL_use_cert = 1, +SSL_version = 'TLSv1', SSL_cert_file = $self-tls_cert, SSL_key_file = $self-tls_key, SSL_ca_file = $self-tls_ca,
Re: [BUG] Default search path used in require_resolvable_fromhost
On Sun, 25 Jul 2010, Robert Spier wrote: I've committed this as ab7c2601f0740fac1c3c117e7e5c0a5690348194. I'm not 100% sure it's a good idea, but I think it's mostly a good thing. What are your reservations? I don't think it would ever be acceptable for the fromhost to be resolvable only when the server's default domain is appended as suffix. And as reported, the current code is exploitable, and Jesper claimed to see it being exploited (but I am skeptical - is a spambot really injecting mail to u...@localhost.localdomain direct to his server?). Charlie - It would be great if you could send patches instead of suggestions. It wasn't my suggestion - I was just relaying it. But point taken. -R Charlie Brady wrote: http://bugs.contribs.org/show_bug.cgi?id=5808 Jesper Knudsen 2010-03-01 01:29:10 MST When using the require_resolvable_fromhost plugin for qpsmtpd I noticed that mails from u...@localhost.localdomain was actually getting through this filter. I finally found out that the plugin has a bug that causes it to insert default search path if it cannot find the domain. This means in my case that localhost.localdomain was then tried resolved as localhost.localdomain.swerts-knudsen.dk and since I have a wilcard CNAME was resolved as my public IP. Since this plugin is only enabled for public interface the fix is to set the dnsrch flag when creating the Net::DNS object. In require_resolvable_fromhost: my $res = Net::DNS::Resolver-new ( dnsrch = 0 );
Re: Rewritten URIBL plugin
On Sun, Jul 25, 2010 at 04:43:36AM -0500, Jared Johnson wrote:
The plugin has the following advantages over the original:
Based on a single read-through of the code, I like most of it. Some
assorted observations, though:
- Parsing out the MIME sections is worthwhile, though it's really
something
that ought to be done in the core libs when plugins indicate they need
it,
or in a utility plugin if that's practical. This was something I meant
to bring up a while back -- we have quite a few different
content-scanning
plugins, and if they're all obliged to invoke MIME::Parser repeatedly
it'll
be even more costly than that module already is. Not that I have time
to
do it myself, though. :P
Our internal plugin actually just uses $txn-notes('mime_body') instead of
creating a new $msg, because we do indeed have such a utility plugin. A
bit short of being transparently handled by internals, but I could at
least include code in the plugin to use $txn-notes('mime_body') instead
of parsing when that exists. This would also allow our internal plugin to
be slightly less forked from what winds up in upstream :) Your idea of
doing this internally sounds cool to me, but we're unlikely to get around
to implementing it ourselves any time soon given the system that already
exists is working OK. And unfortunately our utility plugin is really just
one aspect of a bigger plugin that wouldn't really be useful to QP, and we
have no method of 'asking' for MIME parsing from said plugin since for our
purposes it's *always* necessary to parse mime.
- For messages below a few hundred kb or so, consider
MIME::Parser-output_to_core(), so we can avoid the fs churn.
Annoyingly,
MIME::Parser doesn't provide a mechanism to only consider particular
mimetypes, or we could skip decoding non-text bodies.
The MIME::Parser docs are pretty insistent that using output_to_core is
detrimental to performance in general, and should only be bothered with
when there are actual disk constraints. I've never tested this theory for
really small messages, but it's kept me away from trying it :)
If it's not very difficult, perhaps it could be optional via a plugin
argument, and later via the handy dandy general
mime-parsing-needed-feature if it's ever written
- It looks like you're scanning inside all parts of multipart MIME, where
doing so only on text parts might be preferable.
No, in scan_body() we do:
return unless $part-effective_type ~~ [qw( text/plain text/html )];
I think that's what we want anyway...
- The config really needs to live in a config file -- changing or
expanding
the format is fine if needed, but config that lives in code can't really
be edited. I'm not bothered by having per-list defaults in the plugin,
but one should be able to fully add a new list or alter an existing one
without changing Perl code.
I agree. If you don't object to the spec I proposed in my reply to
Robert, I can get that changed soon.
- In the unwinding of HTML entity escaping -- I suggest doing amp; last.
amp;gt; is gt;, not .
Good idea, thanks :)
the SURBL two-level and three-level lists -- but the SURBL lists are
pruned to exclude stuff that we're pretty sure is never going to
actually
get listed -- when was the last time you saw a spammer use .mil?
uribl.com never has, according to our datafeed :)
We'll probably see compromised .gov and .mil sites hosting malware, much
as we
see in other domains. They're not that useful for classical spammer
purposes
since they can't be registered in the usual fashion.
Ideally, we should only bother querying services for URIs that have some
chance of being listed on those services, regardless of their chance of
actually being used in spam. Thus, if the datafeeds don't have any hosts
with .mil, there's no point checking them. However, (1) we're only
deriving our list from URIBL right now, and even as we expand we can never
say that we derived our list from every service that someone might use;
(2) even if I ran my tld_list generating script every day against every
service and sent in an updated list as soon as something got listed on
.mil, there would still be the lag time to get a release out with the
updated list and then have people upgrade, during which time there would
be some misses.
I think that using 'pruned' lists is eventually going to cause some misses
somewhere. For our own use, since we have control of update timelines,
it's worth the risk, and we also don't have to worry about legal issues
with using the data. If either of these is a big enough problem for QP in
general, though, you could work around it by using an un-pruned
tld_lists.pl. This would give %firstlevel_tlds 263 entries instead of
128; %surbl_secondlevel_tlds would have 3,801 instead of 2,426; and
%surbl_thirdlevel_tlds would have 155 instead of 135. If you're worried
about the legal issue but *not* the timeliness issue, you could still
exclude a few
Re: [PATCH 2/2] increased default TLS security setting
On Sun, 25 Jul 2010, Robert Spier wrote: Applied: 3a7f46aa3e75988686ef9fcae5158fc29f6a86f6 This doesn't seem to be in either of these repos: http://git.develooper.com/qpsmtpd.git http://github.com/abh/qpsmtpd/ Where should I be looking? Thanks. Matt Simerson wrote: switched default TLS security in config/tls_ciphers from HIGH to HIGH:!SSLv2. Added note for how to set the minimum level of security necessary for PCI compliance. --- config.sample/tls_ciphers |8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/config.sample/tls_ciphers b/config.sample/tls_ciphers index e889731..7bb0204 100644 --- a/config.sample/tls_ciphers +++ b/config.sample/tls_ciphers @@ -1,4 +1,10 @@ # Override default security using suitable string from available ciphers at # Lhttp://www.openssl.org/docs/apps/ciphers.html#CIPHER_STRINGS # See plugins/tls for details. -HIGH +# +# HIGH is a reasonable default that should satisfy most installations +HIGH:!SSLv2 +# +# if you have legacy clients that require less secure connections, +# consider using this less secure, but PCI compliant setting: +#DEFAULT:!ADH:!LOW:!EXP:!SSLv2:+HIGH:+MEDIUM -- 1.7.1.1
Re: tls plugin and SSL version
On Mon, 26 Jul 2010, Matt Simerson wrote: Please note that SSL_version is distinct from cipher list, and is not settable in the current code. Sure, but what types of SSL connections are allowed when we exclude the insecure SSLv2 ciphers? Good point. Mine is also a simple untested patch, but I'm reasonably confident it will do exactly what is desired. By changing a config file rather than code, my solution is less likely to cause future problems. OK, but you are proposing adding code aren't you, to add the all/high/medium/pci aliases? Your solution will require further code changes when TLSv1 is insecure and TLSv2 is desirable instead. OK. If you still wish to set SSL_version explicitly, make it a config file option and set the default value to 'SSLv3' instead, which includes TLSv1. That effectively does what you said cannot be done (disable only SSLv2). I didn't realise the SSLv3 includes TLSv1. Did I miss that in the pod? Or is it just implicit in that TLSv1 is SSLv3.1, and SSLv3 means SSLv3.x? I'm confident such a patch would be welcomed and committed. I think you've convinced me that it is not required. Do you think it would be useful? Matt On Jul 22, 2010, at 7:29 PM, Charlie Brady wrote: Here's a simple, and untested, patch - someone might care to do something more elaborate to allow choice of TLSv1 or SSLv3 (unfortunately IO::Socket::SSL doesn't seem to allow disable of just SSLv2). --- qpsmtpd-0.83/plugins/tls.orig2010-07-22 22:04:00.0 -0400 +++ qpsmtpd-0.83/plugins/tls2010-07-22 22:09:35.0 -0400 @@ -80,6 +80,7 @@ local $^W; # this bit is very noisy... my $ssl_ctx = IO::Socket::SSL::SSL_Context-new( SSL_use_cert = 1, +SSL_version = 'TLSv1', SSL_cert_file = $self-tls_cert, SSL_key_file = $self-tls_key, SSL_ca_file = $self-tls_ca, @@ -176,6 +177,7 @@ my $tlssocket = IO::Socket::SSL-new_from_fd( fileno(STDIN), '+', SSL_use_cert = 1, +SSL_version = 'TLSv1', SSL_cert_file = $self-tls_cert, SSL_key_file = $self-tls_key, SSL_ca_file = $self-tls_ca,
New git master repository
Hi everyone, Charlie asked a question that reminded me that I don't think I mentioned this: The canonical git repository is now http://github.com/smtpd/qpsmtpd I'll get the github people to update things so all the references to abh/qpsmtpd (forks etc) will hopefully be automatically updated to point to smtpd/qpsmtpd. - ask
Re: [BUG] Default search path used in require_resolvable_fromhost
On Mon, 26 Jul 2010, Charlie Brady wrote:
On Sun, 25 Jul 2010, Robert Spier wrote:
I've committed this as ab7c2601f0740fac1c3c117e7e5c0a5690348194.
I'm not 100% sure it's a good idea, but I think it's mostly a good
thing.
What are your reservations?
I don't think it would ever be acceptable for the fromhost to be
resolvable only when the server's default domain is appended as suffix.
And as reported, the current code is exploitable, and Jesper claimed to
see it being exploited (but I am skeptical - is a spambot really injecting
mail to u...@localhost.localdomain direct to his server?).
There are multiple other uses of Net::DNS::Resolver, and some thought
should be given to where some or all should also use dnsrch = 0 in the
constructor.
[charl...@localhost qpsmtpd-0.83]$ grep -r Net::DNS::Resolver .
./qpsmtpd-async:$Net::DNS::Resolver::global{id} = 1;
./qpsmtpd-async:$Net::DNS::Resolver::global{id} =
int(rand(Net::DNS::Resolver::MAX_ID()));
./qpsmtpd-async:# print Next DNS ID:
$Net::DNS::Resolver::global{id}\n;
./lib/Qpsmtpd/TcpServer.pm: my $res = new Net::DNS::Resolver;
./plugins/require_resolvable_fromhost: my $res = new Net::DNS::Resolver;
./plugins/require_resolvable_fromhost: my $res = new Net::DNS::Resolver;
./plugins/uribl:use Net::DNS::Resolver;
./plugins/uribl:$self-{resolver} = new Net::DNS::Resolver or return undef;
./plugins/dns_whitelist_soft: my $res = new Net::DNS::Resolver;
./plugins/dns_whitelist_soft: my $res = new Net::DNS::Resolver;
./plugins/dnsbl: my $res = new Net::DNS::Resolver;
./plugins/dnsbl: my $res = new Net::DNS::Resolver;
./plugins/rhsbl: my $res = new Net::DNS::Resolver;
./plugins/rhsbl: my $res = new Net::DNS::Resolver;
[charl...@localhost qpsmtpd-0.83]$
Charlie - It would be great if you could send patches instead of
suggestions.
It wasn't my suggestion - I was just relaying it. But point taken.
-R
Charlie Brady wrote:
http://bugs.contribs.org/show_bug.cgi?id=5808
Jesper Knudsen 2010-03-01 01:29:10 MST
When using the require_resolvable_fromhost plugin for qpsmtpd I noticed
that mails from u...@localhost.localdomain was actually getting through
this filter. I finally found out that the plugin has a bug that causes it
to insert default search path if it cannot find the domain. This means in
my case that localhost.localdomain was then tried resolved as
localhost.localdomain.swerts-knudsen.dk and since I have a wilcard CNAME
was resolved as my public IP.
Since this plugin is only enabled for public interface the fix is to set
the dnsrch flag when creating the Net::DNS object.
In require_resolvable_fromhost:
my $res = Net::DNS::Resolver-new (
dnsrch = 0
);
Re: [BUG] Default search path used in require_resolvable_fromhost
On Mon, 26 Jul 2010, Robert Spier wrote: On Sun, 25 Jul 2010, Robert Spier wrote: I've committed this as ab7c2601f0740fac1c3c117e7e5c0a5690348194. I'm not 100% sure it's a good idea, but I think it's mostly a good thing. What are your reservations? I don't think it would ever be acceptable for the fromhost to be resolvable only when the server's default domain is appended as suffix. And as reported, the current code is exploitable, and Jesper claimed to see it being exploited (but I am skeptical - is a spambot really injecting mail to u...@localhost.localdomain direct to his server?). Internal systems to companies might not use fully qualified names when exchanging mail. I suspect that's not the common use case for qpsmtpd, or for mailservers, so shouldn't be a big deal. Would those also be using require_resolvable_fromhost plugin for internal network SMTP mail? Personally I think anyway using: From: per...@workstationx should have very low expectation of reliable mail transport. Charlie - It would be great if you could send patches instead of suggestions. It wasn't my suggestion - I was just relaying it. But point taken. -R Charlie Brady wrote: http://bugs.contribs.org/show_bug.cgi?id=5808 Jesper Knudsen 2010-03-01 01:29:10 MST When using the require_resolvable_fromhost plugin for qpsmtpd I noticed that mails from u...@localhost.localdomain was actually getting through this filter. I finally found out that the plugin has a bug that causes it to insert default search path if it cannot find the domain. This means in my case that localhost.localdomain was then tried resolved as localhost.localdomain.swerts-knudsen.dk and since I have a wilcard CNAME was resolved as my public IP. Since this plugin is only enabled for public interface the fix is to set the dnsrch flag when creating the Net::DNS object. In require_resolvable_fromhost: my $res = Net::DNS::Resolver-new ( dnsrch = 0 );
Re: Rewritten URIBL plugin
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
Re: Rewritten URIBL plugin
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 :) OK, I looked closer and I guess the async plugin actually 'inherits' from the non-async one, so it uses its methods. That's a new topic for me but I think I can come up with the necessary changes to the non-async plugin and maybe a patch to the async one to use it correctly. But I won't be able to test the theory :) -Jared
per-recipient configuration
On Jul 25, 2010, at 2:43, Jared Johnson wrote: it will soon be hopelessly forked from normal QP on account of per-recipient config directives, etc. Having a common API for per-recipient things have long been on the todo list. We've talked about it a couple times before; but the requirements/needs/wishes never sunk in deep enough in my head to do anything about it. So: How do you use this? My thoughts all along have been to just have some way to make a find user data plugin that'll fetch a per-recipient blob of information that other plugins can, uh, plug into. The big stumbling block is what the common infrastructure for dealing with diverting needs of the recipients should be. What would we need in terms of per-recipient header rewriting, post-DATA processing etc? - ask -- http://develooper.com/ - http://askask.com/
Re: per-recipient configuration
Having a common API for per-recipient things have long been on the todo
list.
We've talked about it a couple times before; but the
requirements/needs/wishes never sunk in deep enough in my head to do
anything about it. So: How do you use this?
My thoughts all along have been to just have some way to make a find user
data plugin that'll fetch a per-recipient blob of information that other
plugins can, uh, plug into.
The big stumbling block is what the common infrastructure for dealing with
diverting needs of the recipients should be. What would we need in terms
of per-recipient header rewriting, post-DATA processing etc?
It is kid of a big scary can of worms. In large part because AFAICT, a
number of shops including my own have grown their own solutions, often
leaving things just as forked as we are. It seems that everybody's doing
it in different, incompatible ways which each shop is fully pleased with
and would find it difficult to move back way from, and really each method
seems valid enough. It may be workable to try to support multiple
different treatments, or it may be necessary to choose one and go with it.
The latter would mean that some existing users might just remain forked,
but if you made the right choices then probably new users would never
bother to move away. There was a time when this was more at the forefront
of my mind and I was interested enough to 'put together a proposal', I
sent some ideas to the list at the time but now I'm not so sure we even
follow those ideas in our own fork; we've moved beyond, and yet what we
have now is still a work in progress. So to 'do it right the first time'
some new thinking would have to take place. But FWIW, here's some about
what we do now:
- Qpsmtpd::Address has a config() method which passes through to plugins
that call 'hook_user_config'. Note that I used to call this
'hook_rcpt_config' but really it's quite valid to call
$txn-sender-config(...) when you've determined that this is an outbound
message and you want to know something about the sender's preferences
instead. The way I implemented this is a bit of a mess. At any rate I
think I submitted a patch to do this sort of thing, but I might not have
managed to stick with the review process long enough to answer everyone's
concerns/questions. Or maybe it was accepted! I can't really remember ;)
- Qpsmtpd::DSN is heavily modified so that it can be used to store
information in Qpsmtpd::Address (which itself has methods to manipulate
such things) about what we have decided we want to do with each recipient
and why we have chosen to do it; e.g. ( Rejected = 'Spam' ).
- Qpsmtpd::Transaction has been enhanced so that recipients() returns only
the recipients that we have (thus far) decided we're going to be
delivering to, and all_recipients() and rejected_recipients() have been
added to give information about recipients that we have decided not to
accept.
Keeping these lists around is useful for logging but may also be necessary
depending on how one decides to deal with the issue of we've reached the
end of DATA and want to reject this recipient and accept this one based on
their different preferences, but we can only give one response.
Obviously we respond with '250' in this case; as for the 'rejected'
recipients, some choose to drop them silently; some have a 'quarantine'
method in place and choose to quarantine for these recipients; some choose
to bounce (this last method is the most reviled, but what can I say? I
don't want to do it, but my lead developer does. And the situation is
pretty rare anyhow).
The content of these methods in our forked code might shed a little light
on how they work with the DSN data stored in each recipient:
sub recipients {
my $self = shift;
@_ and $self-{_recipients} = [...@_];
return () unless $self-{_recipients};
return grep { ! $_-dsn or $_-dsn-action ~~ [qw( Accepted Delivered
Queued Quarantined )] }
@{$self-{_recipients}}
}
sub rejected_recipients {
my $self = shift;
@_ and $self-{_rejected_recipients} = [...@_];
return () unless $self-{_recipients};
return grep { $_-dsn and ! $_-dsn-action ~~ [qw(Delivered Queued
Quarantined)] }
@{$self-{_recipients}}
}
sub all_recipients {
my $self = shift;
return () unless $self-{_recipients};
return @{$self-{_recipients}};
}
- All of our per-recip-pref-aware post-data scanning plugins loop through
each to-be-accepted recipient and determine what we want to do for each
recipient. Then a single plugin afterward handles the results and
responding to the client. So basically, an empty $txn-recipients()
becomes a short-circuit for whatever post-data plugins are left in the
mix. We actually don't do this with the DSN objects; we use a separate
'class' note to denote that something i 'spam', 'clean', 'whitelisted',
etc. Then the last plugin sets the DSNs for each recipient. Maybe we
should have figured a way to do that, idunno.
- Each recipient object can
Re: per-recipient configuration
sub rejected_recipients {
my $self = shift;
@_ and $self-{_rejected_recipients} = [...@_];
I noticed after posting this that the line above is a no-op. Ha :)
return () unless $self-{_recipients};
return grep { $_-dsn and ! $_-dsn-action ~~ [qw(Delivered Queued
Quarantined)] }
@{$self-{_recipients}}
}
-Jared
Re: Rewritten URIBL 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 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? I've already pretty much done the work to access tld_lists as an API, but I'm now doubting the use unless it is released to CPAN: it really makes the most sense to have Qpsmtpd::TLDLists.pm itself still pull in the contents of tld_lists.pl as I posted them, in a 'require', because those contents are created by an automated script. It seems silly to require the modification of said automated script if we want it to generate different logic. But when Qpsmtpd::TLDLists require's tld_lists.pl, then comes the question 'if Qpsmtpd::TLDLIsts is the only thing that accesses this data, and plugins/uribl is the only thing that uses Qpsmtpd::TLDLists, then why doesn't plugins/uribl just read the data itself? You guys can have it whichever way you want it but I say, don't be afraid to treat a .pl file as if it were basically a static, shipped config file in a format easily read by perl plugins :) -Jared
Re: tls plugin and SSL version
On Jul 26, 2010, at 3:32 PM, Charlie Brady wrote: On Mon, 26 Jul 2010, Matt Simerson wrote: Please note that SSL_version is distinct from cipher list, and is not settable in the current code. Sure, but what types of SSL connections are allowed when we exclude the insecure SSLv2 ciphers? Good point. Mine is also a simple untested patch, but I'm reasonably confident it will do exactly what is desired. By changing a config file rather than code, my solution is less likely to cause future problems. OK, but you are proposing adding code aren't you, to add the all/high/medium/pci aliases? I did, initially. But once I peeked under the hood, I noticed that the plugin already has the openssl cipher strings stored in a config file. Rather than add the aliases, I just submitted a patch that altered the config file. If you still wish to set SSL_version explicitly, make it a config file option and set the default value to 'SSLv3' instead, which includes TLSv1. That effectively does what you said cannot be done (disable only SSLv2). I didn't realise the SSLv3 includes TLSv1. Did I miss that in the pod? Or is it just implicit in that TLSv1 is SSLv3.1, and SSLv3 means SSLv3.x? I didn't bother much with the POD, because I'm familiar enough with openssl and what it provides. The TLS being included in SSLv3 is noted on this page: http://www.openssl.org/docs/apps/ciphers.html I'm confident such a patch would be welcomed and committed. I think you've convinced me that it is not required. Do you think it would be useful? Perhaps, but I can't imagine why. Matt Matt
[smtpd/qpsmtpd] d11b87: give badrcptto a reasonable name
Branch: refs/heads/master Home: http://github.com/smtpd/qpsmtpd Commit: d11b87e0509e1482e6c76f203d0d9cacd581db5e http://github.com/smtpd/qpsmtpd/commit/d11b87e0509e1482e6c76f203d0d9cacd581db5e Author: Matt Simerson m...@tnpi.net Date: 2010-07-25 (Sun, 25 Jul 2010) Changed paths: M t/plugin_tests/check_badrcptto Log Message: --- give badrcptto a reasonable name renamed check_badrcptto test from foo to test_check_badrcptto_ok Commit: 0c4a76ffe75190a82010dca5dd7e2bd4bdbe14cb http://github.com/smtpd/qpsmtpd/commit/0c4a76ffe75190a82010dca5dd7e2bd4bdbe14cb Author: Matt Simerson m...@tnpi.net Date: 2010-07-25 (Sun, 25 Jul 2010) Changed paths: M t/plugin_tests/check_badrcptto Log Message: --- add test name to test output
[smtpd/qpsmtpd] fa9176: renamed test from foo to rcpt_ok
Branch: refs/heads/master Home: http://github.com/smtpd/qpsmtpd Commit: fa91764f88a72bd0853f7af9d17ef7f8e0649621 http://github.com/smtpd/qpsmtpd/commit/fa91764f88a72bd0853f7af9d17ef7f8e0649621 Author: Matt Simerson m...@tnpi.net Date: 2010-07-25 (Sun, 25 Jul 2010) Changed paths: M t/plugin_tests/rcpt_ok Log Message: --- renamed test from foo to rcpt_ok
[abh/qpsmtpd] 8b892c: fix copy/paste error in auth_flat_file
Branch: refs/heads/master Home: http://github.com/abh/qpsmtpd Commit: 8b892c33ad456bf8f422b77292d0e288e5994643 http://github.com/abh/qpsmtpd/commit/8b892c33ad456bf8f422b77292d0e288e5994643 Author: Matt Simerson m...@tnpi.net Date: 2010-05-11 (Tue, 11 May 2010) Changed paths: M plugins/auth/auth_flat_file M t/plugin_tests/auth/auth_flat_file Log Message: --- fix copy/paste error in auth_flat_file correct copy/paste error, where auth_flat_file methods were named authsql in auth_flat plugin Signed-off-by: Robert rsp...@pobox.com Commit: b1c3d2f333c807fb40b7a8e5d71086b54f69e562 http://github.com/abh/qpsmtpd/commit/b1c3d2f333c807fb40b7a8e5d71086b54f69e562 Author: Matt Simerson m...@tnpi.net Date: 2010-05-11 (Tue, 11 May 2010) Changed paths: M docs/authentication.pod M plugins/auth/auth_checkpassword A plugins/auth/auth_vpopmail M plugins/auth/auth_vpopmail_sql M plugins/auth/auth_vpopmaild Log Message: --- added auth_vpopmail plugin added auth_vpopmail plugin, using the perl-vpopmail module added VPOPMAIL auth methods description to docs/authentication added SEE ALSO section to each module, noting the VPOPMAIL description Signed-off-by: Robert rsp...@pobox.com Commit: 02912602842a5b2251b1455cf7206cfee3d18553 http://github.com/abh/qpsmtpd/commit/02912602842a5b2251b1455cf7206cfee3d18553 Author: Matt Simerson m...@tnpi.net Date: 2010-05-11 (Tue, 11 May 2010) Changed paths: M plugins/sender_permitted_from Log Message: --- rewrote sender_permitted_from rewrote the plugin using Mail::SPF, which is the replacement for Mail::SPF::Query (by the same author). The two plugins are mutually exclusive and SpamAssassin expects to have Mail::SPF available. Signed-off-by: Robert rsp...@pobox.com Commit: 671a6953b0c9503717bda10dd07f434cbd302c9c http://github.com/abh/qpsmtpd/commit/671a6953b0c9503717bda10dd07f434cbd302c9c Author: Matt Simerson m...@tnpi.net Date: 2010-05-11 (Tue, 11 May 2010) Changed paths: M lib/Qpsmtpd/TcpServer.pm M plugins/greylisting M plugins/ident/p0f Log Message: --- add TCPLOCAL* variables to $qp-connection (patch remade against latest rspier/qpsmtpd) added remote_port, local_ip, local_port, and local_host to $qp-connection, as the p0f plugin relies on it. added notes to TcpServer.pm and the p0f plugin noting the dependence, and the lack of support for models other than tcpserver. Signed-off-by: Robert rsp...@pobox.com Commit: cc2d8ccca6a7fafe2c08b7d180e81aeae8eb1b35 http://github.com/abh/qpsmtpd/commit/cc2d8ccca6a7fafe2c08b7d180e81aeae8eb1b35 Author: Matt Simerson m...@tnpi.net Date: 2010-05-11 (Tue, 11 May 2010) Changed paths: M plugins/ident/p0f Log Message: --- added local_ip option to p0f plugin (updated patch against rspier/qpsmtpd) The p0f plugin defaulted to binding to TCPLOCALIP, which doesn't work when the mail server is running behind a firewall with a private IP. If the local_ip option is set in the config file, it overrides TCPLOCALIP. Added POD documentation for local_ip option and p0f general usage Signed-off-by: Robert rsp...@pobox.com Commit: b81d464c872867f8df65847f522db6a0df4a96bf http://github.com/abh/qpsmtpd/commit/b81d464c872867f8df65847f522db6a0df4a96bf Author: Matt Simerson m...@tnpi.net Date: 2010-05-11 (Tue, 11 May 2010) Changed paths: M config.sample/plugins A t/plugin_tests/greylisting Log Message: --- added p0f support to greylist plugin - these changes are in the previous TCPLOCAL patch. Documented here. added p0f config option added POD docs to explain usage modified $dbdir selection logic. The previous logic failed when QPHOME was not selected (as is the case when tests are being run). Added '.' as the dir of last resort for $dbdir selection (others $EMPTY/dir dumped greylisting database in / ) - These changes are included in this patch - Added t/plugin_tests/greylisting, with greylist logic testing (tests are disabled by default, as greylisting is disabled in config.sample/plugins) Added example entry in config.sample/plugins Signed-off-by: Robert rsp...@pobox.com Commit: e13952164df61ac289f9f124a7a8bc63d290d4bc http://github.com/abh/qpsmtpd/commit/e13952164df61ac289f9f124a7a8bc63d290d4bc Author: Matt Simerson m...@tnpi.net Date: 2010-05-11 (Tue, 11 May 2010) Changed paths: M MANIFEST M MANIFEST.SKIP Log Message: --- packaging updates added to MANIFEST plugins/check_bogus_bounce plugins/auth/auth_vpopmaild t/plugin_tests/greylisting added packaging to MANIFEST.SKIP Signed-off-by: Robert rsp...@pobox.com Commit: 0d2b724b9317bbfddf402279ceff2f523814b8ac http://github.com/abh/qpsmtpd/commit/0d2b724b9317bbfddf402279ceff2f523814b8ac Author: Robin Bowes ro...@hero.robinbowes.com Date: 2010-05-12 (Wed, 12 May 2010) Changed paths: M plugins/auth/auth_vpopmaild Log Message: --- Check for the exact string resonses from vpopmaild rather than using regexes Commit:
