Date: Thu, 17 Jul 2003 17:29:18 -0400
From: John Simpson <jms1@...1.net>
To: Uwe Ohse <uwe@...e.de>
Cc: smtpauth@...t.elysium.pl, qmail@...t.cr.yp.to, bugtraq@...urityfocus.com
Subject: Re: possible open relay hole in qmail-smtpd-auth patch

short story: if you have downloaded my "auth-jms1.2.patch" file, go back to 
http://www.jms1.net/qmail/ and download "auth-jms1.3.patch" instead. there 
was a typo in the "2" version, which Uwe was kind enough to point out.

long story follows:

On Wednesday 16 July 2003 07:54, Uwe Ohse wrote:
> On Tue, Jul 15, 2003 at 12:36:05PM -0400, John Simpson wrote:
> > the current version of the SMTP-AUTH patch contains a serious bug which
> > can accidentally allow somebody who forgets one or more of the command
> > line parameters to start running an open relay by accident. it has been
> > reported in several places over the last week, including this message
> > on the qmail mailing list:
>
> A possible configuration problem is NOT a reason to disallow legitimate
> configurations

with the original patch (without my changes) there are two legitimate 
configurations:

(1) tcpserver ... qmail-smtpd

this configuration is how djb's documentation tells you to set it up. it 
does not support the AUTH command, although if you're using the patch 
without my changes the server will incorrectly advertise the AUTH 
capability in response to the client's EHLO command.

(2) tcpserver ... qmail-smtpd hostname checkpassword dummy

this is the correct way to run with this patch, with or without my changes.

as you can see, leaving out "hostname" causes "checkpassword" to be used as 
the hostname, and "dummy" (usually /bin/true) to be used as the 
checkpassword program. my changes cause the AUTH command to be totally 
ignord unless at least three items are present on the command line.

the original version of the patch didn't use argc at all- it blindly assumed 
that if anything were present, that three items were present. my changes 
cause it to not 

> or to enforce the use of useless dummy arguments

it's not "useless".

the dummy argument is required by the fact that the patch's author elected 
to go with the "checkpassword" standard for authentication handlers, 
probably because so many of them already existed.

my changes have nothing to do with the fact that his patch requires a dummy 
program to return zero.

a "checkpassword" program is designed to exec another program after 
processing a valid set of credentials. the choice was to either force 
everybody who had ever written a checkpassword implementation to patch them 
so they didn't require another program on the command line, or to supply a 
dummy program which always returns zero on the command line. i understand 
why he chose to have his users go with a dummy program, and i agree with 
his choice.

by using a dummy "return zero" program, every existing checkpassword program 
can now be used as an SMTP AUTH password validator as well.

> and unless you are going to invent some kind of artificial intelligence
> inside tools or installers using checkpassword i request that you
> stop this.

take this up with the author of the original patch, and with everybody else 
on the planet who's running his code. i'm simply trying to make his code 
more resistant to errors.

> While the usual usage of checkpassword is something like this
>   tcpserver host port prog1 [args] checkpassword prog2 [args]
> there is no reason why one has to call it that way. One may replace
> the part starting from "checkpassword" by a wrapper (this may be
> especially handy to add logging, to try more than one checkpassword
> style program or when one has to use different "tcpserver...prog1"
> combinations all sharing identical later commandline arguments -
> i've seen this being used).

correct. if you want/need to write a wrapper for your situation, feel free.

> Cluttering the commandline with "dummy-argument-for-stupid-patches"
> or whatever is not going to do any good, is not looking good, and
> does not add any security at all.

again, the patch doesn't require the dummy program. the fact that it's using 
the checkpassword interface means that the dummy program has to be there.

the "checkpassword" interface REQUIRES a program to exec after it validates 
a set of credentials.

http://cr.yp.to/checkpwd/interface.html

> In fact, someone stupid enough to patch qmail without understanding
> the patch and stupid enough to ignore vital parts of it's
> documentation (the examples, at least) is likely to be stupid
> enough to change the order of checkpassword and /bin/true, too.

in a way i agree with you, in that there are people out there who blindly 
apply patches without really understanding what they do.

however, these are usually system administrators who spend all day long 
trying to keep their systems running, with users screaming for features and 
supervisors screaming to "just make it work" so they can shut the customers 
up. many of these people don't have ten years of networking or *nix 
experience, and their employers don't want to spend the extra money to hire 
people with that level of experience.

however, i think you are wrong in calling these people stupid. there may be 
a few stupid people in situations like this, but for the most part they're 
smart guys, under-experienced, and over-worked. i know, because i've been 
there and i have many friends who are there right now.

> Are you now going to patch /bin/true to bail out in case it finds
> commandline arguments?

of course not.

> please? Are you seriously expecting that this:
> +  if(argc > 2)
> +  {
> +    hostname = argv[1];
> +    childargs = argv + 2;
> +    useauth = 1;
> +  }
> will work?
>
> Hint: when called "qmail-smtpd HOST CHECKPASSWORD" argc will be 3.
> I'd call this a beginners mistake, but even beginners are supposed
> to test their code.

i've been programming in c since 1986, but the last few years i've been 
mostly programming perl (which counts argc differently.)

the code WAS tested, but it was tested after about thirty hours of 
non-sleep, while also worrying about a client's machine which has just been 
hacked, and i guess i missed it.

i am honest enough to admit a mistake: Uwe, thank you for catching it. if 
you ever get to orlando i owe you a beer.

for those running the updated patch, change "if(argc > 2)" to "if(argc > 3)" 
at your earliest convenience.

> While i'm at it: Your qmail-1.03-jms1-antispam.patch not only violates
> the SMTP protocol (replying OK when the mail will definitively not reach
> the recipient)

the idea is this: i own the machine. if i decide that i don't want to 
receive a given message, for whatever reason, i have three choices:

(1) i can reject it, in which case the remote server will continue to 
attempt to deliver the same message over and over until it times out of, or 
is manually removed from, the remote machine's queue.

this is less-than-acceptable to me because the repeated delivery attempts 
tie up my machine's bandwidth and CPU in order to reject the same message 
over and over.

(2) accept it anyway, and use some later filtering program to delete it 
before it hits my inbox.

this is totally unacceptable to me because it allows the message to take up 
space on my disk, even if only for a short period of time.

(3) ignore it, but tell the remote server that i've accepted it. this way it 
never touches my disk, and the remote server stops trying to deliver it.

if the meaning of the 250 message is "Requested mail action completed" (see 
RFC 821 section 4.2.2) then the message is correct and the RFC is not being 
violated, because my server has indeed taken the appropriate action to 
route the message to where i (as the machine's owner) feel it belongs.

> but does that in case of almost any programming or
> configuration problem in the filter chain, causing shell/filters to
> exit with 1.

can you show an example?

this patch was written as an add-on for systems running with the QMAILQUEUE 
patch, which allows other programs to inspect a message (RBL checking, 
virus scanning, etc.) before handing it to the real qmail-queue program. 
the alternate program cannot directly send messages back to the smtp 
client, so this adds a way for such a program to tell qmail-smtpd to return 
an alternate 250 message without ever really calling qmail-queue.

it's actually written to work with one specific filtering program (which i 
wrote in perl and will soon be re-writing in c) which checks the Received: 
headers for IP addresses and looks them up on certain RBL lists, and then 
runs a virus scanner on them. it's not meant for use by every qmail server 
in the world, which is why i'm not "advertising" it on qmail.org or 
anywhere other than my own web page.

and yes, i will make sure i've had enough sleep before testing the new 
filtering program... and i will put the code out there for anybody, 
yourself included, to check and improve upon.

> Why the heck did you change exit code 1 from "temporary problem,
> try again later" to "throw away that mail"?

the meaning of exit code 1 is not "throw away that mail", it's "this message 
has been ignored".

if the qmail-smtpd has been patched with this particular patch, and the 
qmail-queue replacement programs returns "1", qmail-smtpd will send "250 
Your SPAM has been ignored" to the remote machine, which makes the remote 
machine will think it's been delivered and will stop trying over and over.

if the qmail-smtpd has not been patched, it will send a 400-series 
"temporary failure" message, which causes the remote machine to continue 
trying to deliver the message over and over until it's removed from the 
remote queue.

> Why do you actually run a smtp server if your mail is worthless?

if my mail is worthless, why are you wasting the time to argue about it?

look. you found a typo in my code. fine, thank you for letting me know. i 
truly appreciate it.

however, that shouldn't be a reason to try and attack every other piece of 
code i've ever written.

are you even running qmail with the qmail-smtpd-auth patch? did YOU figure 
out why the problem was happening? did YOU come up with your own fix for 
it? or do you just like trying to make other people look bad...

i'm trying to help people here, not attack them. i may not be perfect, but 
i'm willing to admit when i make a mistake... and i think this paragraph is 
the closest i've ever come to attacking anybody in public.

peace.

-- 
-----------------------------------------------
| John Simpson - KG4ZOW - Programmer At Large |
| http://www.jms1.net/        <jms1@...1.net> |
-----------------------------------------------

Content of type "application/pgp-signature" skipped

Powered by blists - more mailing lists