Re: [vchkpw] 5.4.18 release candidate
here's a "slightly better" idea: have the code read a text file from ~vpopmail/etc/, or even better from an environment variable, which contains the string you wish to use for each dynamically added IP address... and if that file or variable doesn't exist, the entire "write out the IP address and rebuild the cdb file" process would be skipped. Hi John! This a good idea! but here's a "much better" idea: tell your users to use AUTH. Yes! But check my "patch"... - :allow,RELAYCLIENT="",RBLSMTPD="" + :allow,QMAILQUEUE="/var/qmail/bin/simscan",RBLSMTPD="" I have removed RELAYCLIENT and add QMAILQUEUE. In this case my user are enabled to connect to SMTP port afer POP, jumping RBL check, but they will be obligates to use AUTH for send email Thanks for reply
Re: [vchkpw] 5.4.18 release candidate
On 2006-12-29, at 1418, Sim wrote: I need this change for disable RBL check after Pop (using pop-before-smtp function): /* append the current ip address to the tmp file * using the format x.x.x.x:ALLOW,RELAYCLIENT="",RBLSMTPD=""timestamp */ - fprintf( fs_tmp_file, "%s:allow,RELAYCLIENT=\"\",RBLSMTPD=\"\"\t% d\n", + fprintf( fs_tmp_file, "%s:allow,QMAILQUEUE=\"/var/qmail/bin/simscan\",RBLSMTPD=\"\"\t%d\n", ipaddr, (int)mytime); fclose(fs_cur_file); fclose(fs_tmp_file); My users can now connect to smtp port and send email with AUTH. Can you create a ./configure options for this ":allow" values? here's a "slightly better" idea: have the code read a text file from ~vpopmail/etc/, or even better from an environment variable, which contains the string you wish to use for each dynamically added IP address... and if that file or variable doesn't exist, the entire "write out the IP address and rebuild the cdb file" process would be skipped. this way the whole "add POP3 client IPs to the smtpd access control list" process becomes something which can be configured at run time, rather than having to be explicitly configured into or out of the code. the contents of the file would look like this: :allow,RELAYCLIENT="",RBLSMTPD"" or if you use an environment variable, you would add these two lines to the "run" script for your POP3 and/or IMAP services (assuming you use "DYNAMIC_SMTPD_ACL" as the variable name)... DYNAMIC_SMTPD_ACL=":allow,RELAYCLIENT=\"\",RBLSMTPD=\"\"" export DYNAMIC_SMTPD_ACL the code would just write out the IP address, the string from this file/variable, a TAB, a timestamp, and a newline. === but here's a "much better" idea: tell your users to use AUTH. | John M. Simpson--- KG4ZOW ---Programmer At Large | | http://www.jms1.net/ <[EMAIL PROTECTED]> | | http://video.google.com/videoplay?docid=-4312730277175242198 | PGP.sig Description: This is a digitally signed message part
Re: [vchkpw] 5.4.18 release candidate
Hi! I need this change for disable RBL check after Pop (using pop-before-smtp function): /* append the current ip address to the tmp file * using the format x.x.x.x:ALLOW,RELAYCLIENT="",RBLSMTPD=""timestamp */ - fprintf( fs_tmp_file, "%s:allow,RELAYCLIENT=\"\",RBLSMTPD=\"\"\t%d\n", + fprintf( fs_tmp_file, "%s:allow,QMAILQUEUE=\"/var/qmail/bin/simscan\",RBLSMTPD=\"\"\t%d\n", ipaddr, (int)mytime); fclose(fs_cur_file); fclose(fs_tmp_file); My users can now connect to smtp port and send email with AUTH. Can you create a ./configure options for this ":allow" values? Thanks!
Re: [vchkpw] 5.4.18 release candidate
On 2006-12-28, at 0936, Joshua Megerman wrote: OK, I did some more testing with a test cdb and tcprulescheck, and got some interesting results: I thought that the daemontools documentation stated that it takes the first match it finds period, but I misunderstood it slightly. It states that it looks for a match of the exact IP first, and then shorter and shorter prefixes, taking the first match. This is close to the exhibited behavior, but not quite. A rule that has all 4 octets listed, either a single IP (A.B.C.d) or an IP range within a single /24 block (A.B.C.D-E), will override any rule that is less specific (as documented). Therefore, even if a particular subnet is excluded (e.g., 127.0.0:deny), a single IP address updated by vpopmail (e.g., 127.0.0.1:allow,[...]) would be allowed to connect (at least until the cdb was scrubbed). However, if you have multiple rules with all 4 octets, it does take the first rule it finds. So the rule (e.g.) 127.0.0.0-7:deny would override the rule (e.g.) 127.0.0.2:allow, assuming that the deny rule appeared first. The same pattern holds true for shorter rules, where there are multiple rules that could match with the same number of octets listed. Thus, it may be possible that there are people denying entire netblocks and then using the pop-before-smtp (or maybe even imap-before-smtp, though I know there are problems with at least one major IMAP server out there) to "poke holes" in their tcpserver cdb and allow connections from otherwise denied addresses, and that one case would break with this patch. However, I have a possible idea for that - see below. this is because a rule like this: 192.168.43.32-39:allow actually goes into the cdb file as eight rules, like so: 192.168.43.32 -> allow 192.168.43.33 -> allow 192.168.43.34 -> allow 192.168.43.35 -> allow 192.168.43.36 -> allow 192.168.43.37 -> allow 192.168.43.38 -> allow 192.168.43.39 -> allow tcpserver actually checks four entries in the cdb file... for the IP address "a.b.c.d", it searches for the following keys, in this order: a.b.c.d a.b.c a.b a it would be nice if the file format included the subnet mask as part of the key, it would allow for more granular searching, like so: a.b.c.d/32 a.b.c.?/31 a.b.c.?/30 a.b.c.?/29 a.b.c.?/28 a.b.c.?/27 a.b.c.?/26 a.b.c.?/25 a.b.c/24 a.b.?/23 ... a/8 of course "?" represents the original octet, ANDed with the appropriate value to make it match the base address in a network (i.e. for a "/28" entry, the last octet would be one of 0, 16, 32, 48, 64, etc.) I just came up with an option 4, which may well resolve the issue: 4) modify the patch so that it looks for a specific comment line ("#UPDATESTATIC" perhaps - I'm open to suggestions) at the beginning of the static cdb file. Lines starting with '#' characters are ignored by tcprules, but I can easily modify my patch to check for them, especially on the first line, and short-circuit the search. Then all that is needed is to document the effects of the patch, and state that in order to continue updating the CDB file with addresses that have static matches you have to insert that comment on the first line of the static CDB. This changes it from a compile-time configureation variable to a runtime one, and has the benefit of not requiring additional configuration files. Thoughts? (I'll go ahead and make a new patch anyway, just because it's simple and I like the idea :)) i don't know enough about how the patch works to comment directly on this... from rick's one-line description at the beginning of the thread i gather the patch keeps statically-entered /etc/tcp/smtp entries from having dynamic lines created for them when they authenticate via POP3 or IMAP. if this is not the problem you're trying to solve, then ignore the rest of this message. but here's another idea: - when a user authenticates via POP3/IMAP, the code "touches" a file in a specific directory, whose name is the client's IP address. the directory would be set via a ./configure option, and if the option is not there, the code would be disabled entirely. - vpopmail's "make" procedure would also build a program called "check-relay", which can be called from the "tcpserver ... qmail- smtpd" command line. it would work something like this (quick and dirty pseudo-C)... int main(int argc, char **argv, char **envp) { #ifdef RELAY_DIR char *filename; char *ip ; ip = env_get("TCPREMOTEIP") ; if ( ip ) { filename = malloc ( sizeof(RELAY_DIR) + 17 ) ; sprintf ( filename, "%s/%s", RELAY_DIR, ip ) ;
Re: [vchkpw] 5.4.18 release candidate
On 2006-12-26, at , Joshua Megerman wrote: That is correct - this patch actually helps eliminate race conditions by reducing the frequency of CDB update. Because an address that is covered by a static rule will never be overridden by a dynamic one (since tcpserver is supposed to use the first rule it finds), actually, tcpserver uses the MOST SPECIFIC rule it finds. for example, if there are three rules matching a given IP (a /28 rule, a / 24 rule, and a specific /32 rule) then it will go with the specific / 32 rule. if there are multiple identical keys in the cdb file, the order in which they are retrieved is undefined (per djb's original spec.) and even the example above is not fully accurate... because of the fact that tcprules builds the cdb file with keys whose values are based on octet boundaries, a /28 rule in the text file actually appears as 16 /32 rules in the cdb file... so in my example above, there's actually a 50/50 chance whether the /32 rule or the /28 rule would end up being followed. so whatever process is building the cdb file, it should probably take steps to ensure that any given key is not added more than once- otherwise there is no guarantee which rule will actually be used.) i wonder how hard it would be to rewrite tcprules and tcpserver to implement real CIDR-based rules in the cdb file, instead of breaking each entry which is not a perfect /8, /16, or /24 into multiple cdb keys which are? and unfortunately, because the order is undefined, there is a chance that a dynamic rule could "override" a static rule, if they both have identical keys in the cdb file. However, IMHO SMTP-AUTH should be used instead as it's both more reliable and more secure, i couldn't agree more. I'm not sure how to best address it, but I see 3 choices: 1) exclude the patch from the main tree but publish it as an add-on (not great); 2) include the patch and document the changes in how the CDB is built and works (better, but may cause breakage for some people); or 3) put the code inside an #ifdef and make it a configure option (I'd enable it by default, but it could go either way). I know nothing about configure, so I'm not sure how to do it, but I'd guess it's pretty simple to change... i would agree with either #1 or #3- if it's going to be added to the main-stream code, it should be something which can be electively enabled and disabled on the ./configure command line. it doesn't affect me directly either way (i've been using AUTH for years) but i'm not real crazy about the idea of forcibly changing the behaviour of existing programs without a good reason- and the fact that it CAN be made an optional thing means that this isn't a good enough reason to force the change on all of the people who may be using this feature now and would be affected by the change when/if they upgrade. | John M. Simpson--- KG4ZOW ---Programmer At Large | | http://www.jms1.net/ <[EMAIL PROTECTED]> | | http://video.google.com/videoplay?docid=-4312730277175242198 | PGP.sig Description: This is a digitally signed message part
Re: [vchkpw] 5.4.18 release candidate
> Joshua Megerman wrote: >> I just tested and that's not what actually happens - it takes the best >> match. So there is one potential problem with this (though I consider >> it >> minimal) - if you have a rule that doesn't include the 'RELAYCLINET=""' >> and/or 'RBLSMTPD=""', you may end up getting denied if you're depending >> on >> pop-before-smtp. However, IMHO SMTP-AUTH should be used instead as it's >> both more reliable and more secure, but not everyone pushes that. So >> there may be unintended consequences with this patch... > > I think the consequences are - If you explicitly deny access, or > relaying to an address or address range, a pop connection will no longer > allow the connection to relay. It is probably very rare, and might > even be considered a good thing. > OK, I did some more testing with a test cdb and tcprulescheck, and got some interesting results: I thought that the daemontools documentation stated that it takes the first match it finds period, but I misunderstood it slightly. It states that it looks for a match of the exact IP first, and then shorter and shorter prefixes, taking the first match. This is close to the exhibited behavior, but not quite. A rule that has all 4 octets listed, either a single IP (A.B.C.d) or an IP range within a single /24 block (A.B.C.D-E), will override any rule that is less specific (as documented). Therefore, even if a particular subnet is excluded (e.g., 127.0.0:deny), a single IP address updated by vpopmail (e.g., 127.0.0.1:allow,[...]) would be allowed to connect (at least until the cdb was scrubbed). However, if you have multiple rules with all 4 octets, it does take the first rule it finds. So the rule (e.g.) 127.0.0.0-7:deny would override the rule (e.g.) 127.0.0.2:allow, assuming that the deny rule appeared first. The same pattern holds true for shorter rules, where there are multiple rules that could match with the same number of octets listed. Thus, it may be possible that there are people denying entire netblocks and then using the pop-before-smtp (or maybe even imap-before-smtp, though I know there are problems with at least one major IMAP server out there) to "poke holes" in their tcpserver cdb and allow connections from otherwise denied addresses, and that one case would break with this patch. However, I have a possible idea for that - see below. >> I'm not sure how to best address it, but I see 3 choices: 1) exclude the >> patch from the main tree but publish it as an add-on (not great); 2) >> include the patch and document the changes in how the CDB is built and >> works (better, but may cause breakage for some people); or 3) put the >> code >> inside an #ifdef and make it a configure option (I'd enable it by >> default, >> but it could go either way). > > I'm partial to 2, but hope to hear what others think. I'd rather not > add any more configure options, [1] so if it is considered too dangerous > to have enabled full time I'd rather leave it as a patch, and maybe add > it to the contrib directory. On the other hand, how many people are > depending on pop before smtp from otherwise blacklisted addresses, that > couldn't switch to smtp auth? How many are still using pop before smtp > at all? I haven't supported it for a couple of years. > > Do you want to write the README text for this patch? > I just came up with an option 4, which may well resolve the issue: 4) modify the patch so that it looks for a specific comment line ("#UPDATESTATIC" perhaps - I'm open to suggestions) at the beginning of the static cdb file. Lines starting with '#' characters are ignored by tcprules, but I can easily modify my patch to check for them, especially on the first line, and short-circuit the search. Then all that is needed is to document the effects of the patch, and state that in order to continue updating the CDB file with addresses that have static matches you have to insert that comment on the first line of the static CDB. This changes it from a compile-time configureation variable to a runtime one, and has the benefit of not requiring additional configuration files. Thoughts? (I'll go ahead and make a new patch anyway, just because it's simple and I like the idea :)) Josh -- Joshua Megerman SJGames MIB #5273 - OGRE AI Testing Division You can't win; You can't break even; You can't even quit the game. - Layman's translation of the Laws of Thermodynamics [EMAIL PROTECTED]
Re: [vchkpw] 5.4.18 release candidate
Joshua Megerman wrote: I just tested and that's not what actually happens - it takes the best match. So there is one potential problem with this (though I consider it minimal) - if you have a rule that doesn't include the 'RELAYCLINET=""' and/or 'RBLSMTPD=""', you may end up getting denied if you're depending on pop-before-smtp. However, IMHO SMTP-AUTH should be used instead as it's both more reliable and more secure, but not everyone pushes that. So there may be unintended consequences with this patch... I think the consequences are - If you explicitly deny access, or relaying to an address or address range, a pop connection will no longer allow the connection to relay. It is probably very rare, and might even be considered a good thing. I'm not sure how to best address it, but I see 3 choices: 1) exclude the patch from the main tree but publish it as an add-on (not great); 2) include the patch and document the changes in how the CDB is built and works (better, but may cause breakage for some people); or 3) put the code inside an #ifdef and make it a configure option (I'd enable it by default, but it could go either way). I'm partial to 2, but hope to hear what others think. I'd rather not add any more configure options, [1] so if it is considered too dangerous to have enabled full time I'd rather leave it as a patch, and maybe add it to the contrib directory. On the other hand, how many people are depending on pop before smtp from otherwise blacklisted addresses, that couldn't switch to smtp auth? How many are still using pop before smtp at all? I haven't supported it for a couple of years. Do you want to write the README text for this patch? Rick [1] I hope no one is under the delusion that all permutations of the existing ./configure switches are tested before a new release!
Re: [vchkpw] 5.4.18 release candidate
> > > Tom Collins wrote: > >> On Dec 23, 2006, at 6:41 PM, Rick Widmer wrote: >> >>> - SQL backend fixes - Extend length of domain from 64 to 96 in all >>> database backends. Add delete_spamassassin and delete_spam to the >>> limits table. REQUIRES DATABASE STRUCTURE CHANGES! >> >> Since MySQL is the most common SQL backend, we should include the SQL >> statements (ALTER TABLE) someone would have to use to make the >> necessary changes. > > I have a description of the changes in UPGRADE and have asked Joshua if > he has the SQL statements available. If not I'll figure them out and > update UPGRADE. > The following assumes that '--enable-many-domains' is used. If you use '--disable-many-domains', then you need to replace the ALTER command for the 'vpopmail' table with one for each domain table. Also, the 'ip_alias_map', 'vlog' and 'limits' tables only exist if the various configuration options are enabled, so they may or may not be relevant (I only use the latter 2, but I got the ip_alias_map tablename from looking at the code). ALTER TABLE `dir_control` CHANGE `domain` `domain` CHAR( 96 ) NOT NULL; ALTER TABLE `ip_alias_map` CHANGE `domain` `domain` CHAR( 96 ) NOT NULL; ALTER TABLE `lastauth` CHANGE `domain` `domain` CHAR( 96 ) NOT NULL; ALTER TABLE `limits` CHANGE `domain` `domain` CHAR( 96 ) NOT NULL , ADD `disable_spamassassin` TINYINT( 1 ) DEFAULT '0' NOT NULL AFTER `disable_smtp` , ADD `delete_spam` TINYINT( 1 ) DEFAULT '0' NOT NULL AFTER `disable_spamassassin` ; ALTER TABLE `valias` CHANGE `domain` `domain` CHAR( 96 ) NOT NULL; ALTER TABLE `vlog` CHANGE `domain` `domain` CHAR( 96 ) NOT NULL; ALTER TABLE `vpopmail` CHANGE `domain` `domain` CHAR( 96 ) NOT NULL; I'm sure the optional statements can probably be encased inside some sort of conditional, but my SQL knowledge is rather limited... Josh -- Joshua Megerman SJGames MIB #5273 - OGRE AI Testing Division You can't win; You can't break even; You can't even quit the game. - Layman's translation of the Laws of Thermodynamics [EMAIL PROTECTED]
Re: [vchkpw] 5.4.18 release candidate
> Tom Collins wrote: > >> On Dec 23, 2006, at 6:41 PM, Rick Widmer wrote: >> >>> - Don't update the relay CDB for statically covered addresses. >>> (extended version.) >> >> >> Do we have to worry about a race condition where I do my POP pickup and >> the relay CDB isn't updated with the new timestamp (since I'm in there >> from an earlier pickup) but the entry expires before my SMTP starts? I >> don't think I've ever used the pop-before-smtp feature, so I'm not sure >> on the details of how it works... > > No. Update_rules builds the final cdb file by concatenating the static > addresses in the file specified by TCP_FILE and the dynamic addresses > either from a database or file. This patch only makes excludes the > static addresses. If you are already allowed in by a static entry, > don't bother to manage your dynamic entry... > That is correct - this patch actually helps eliminate race conditions by reducing the frequency of CDB update. Because an address that is covered by a static rule will never be overridden by a dynamic one (since tcpserver is supposed to use the first rule it finds), this won't impact the security of the CDB. If it's denied, you'd never connect in the first place, and if it's allowed it's already allowed and the dynamic rule should never get matched. The reason I say "should" instead of "will" is that that's what the documentation states. I just tested and that's not what actually happens - it takes the best match. So there is one potential problem with this (though I consider it minimal) - if you have a rule that doesn't include the 'RELAYCLINET=""' and/or 'RBLSMTPD=""', you may end up getting denied if you're depending on pop-before-smtp. However, IMHO SMTP-AUTH should be used instead as it's both more reliable and more secure, but not everyone pushes that. So there may be unintended consequences with this patch... I'm not sure how to best address it, but I see 3 choices: 1) exclude the patch from the main tree but publish it as an add-on (not great); 2) include the patch and document the changes in how the CDB is built and works (better, but may cause breakage for some people); or 3) put the code inside an #ifdef and make it a configure option (I'd enable it by default, but it could go either way). I know nothing about configure, so I'm not sure how to do it, but I'd guess it's pretty simple to change... Anyway, those are my thoughts on how it works and the potential consequences of the patch... Josh -- Joshua Megerman SJGames MIB #5273 - OGRE AI Testing Division You can't win; You can't break even; You can't even quit the game. - Layman's translation of the Laws of Thermodynamics [EMAIL PROTECTED]
Re: [vchkpw] 5.4.18 release candidate
Tom Collins wrote: On Dec 23, 2006, at 6:41 PM, Rick Widmer wrote: - SQL backend fixes - Extend length of domain from 64 to 96 in all database backends. Add delete_spamassassin and delete_spam to the limits table. REQUIRES DATABASE STRUCTURE CHANGES! Since MySQL is the most common SQL backend, we should include the SQL statements (ALTER TABLE) someone would have to use to make the necessary changes. I have a description of the changes in UPGRADE and have asked Joshua if he has the SQL statements available. If not I'll figure them out and update UPGRADE. - Don't update the relay CDB for statically covered addresses. (extended version.) Do we have to worry about a race condition where I do my POP pickup and the relay CDB isn't updated with the new timestamp (since I'm in there from an earlier pickup) but the entry expires before my SMTP starts? I don't think I've ever used the pop-before-smtp feature, so I'm not sure on the details of how it works... No. Update_rules builds the final cdb file by concatenating the static addresses in the file specified by TCP_FILE and the dynamic addresses either from a database or file. This patch only makes excludes the static addresses. If you are already allowed in by a static entry, don't bother to manage your dynamic entry...
Re: [vchkpw] 5.4.18 release candidate
On Dec 23, 2006, at 6:41 PM, Rick Widmer wrote: - SQL backend fixes - Extend length of domain from 64 to 96 in all database backends. Add delete_spamassassin and delete_spam to the limits table. REQUIRES DATABASE STRUCTURE CHANGES! Since MySQL is the most common SQL backend, we should include the SQL statements (ALTER TABLE) someone would have to use to make the necessary changes. - MySQL backquotes around table names so you can use a digit as the first character. - Don't update the relay CDB for statically covered addresses. (extended version.) Do we have to worry about a race condition where I do my POP pickup and the relay CDB isn't updated with the new timestamp (since I'm in there from an earlier pickup) but the entry expires before my SMTP starts? I don't think I've ever used the pop-before-smtp feature, so I'm not sure on the details of how it works... -- Tom Collins - [EMAIL PROTECTED] Vpopmail - virtual domains for qmail: http://vpopmail.sf.net/ QmailAdmin - web interface for Vpopmail: http://qmailadmin.sf.net/