> 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]


Reply via email to