Qmail
[Top] [All Lists]

Re: question about old AUTH+TLS patch

To: qmail@list.cr.yp.to
Subject: Re: question about old AUTH+TLS patch
From: John Simpson <jms1@jms1.net>
Date: Sun, 24 Dec 2006 20:00:20 -0500
Delivered-to: sp-com-lists@consult.net
Delivered-to: gmail-qmail@securepoint.com
Delivered-to: sp.com.list@gmail.com
Delivered-to: mailing list qmail@list.cr.yp.to
In-reply-to: <3.0.6.32.20061213205756.01444c70@192.168.192.1>
Mailing-list: contact qmail-help@list.cr.yp.to; run by ezmlm
References: <3.0.6.32.20061213205756.01444c70@192.168.192.1>
On 2006-12-13, at 1457, Erwin Hoffmann wrote:

gosh. This bug is so old. It has already a St. Claus beard.

http://www.fehcom.de/qmail/smtpauth.html

Check out any of my Auth patches and go thru the README.

i think i found a bug in (at least) the 064 and 065 versions of your AUTH patch, at least on certain platforms.

in your base64.c, in the b64decode() function, we find these lines:

36   out->len = (n * 3) - p;
37   if (!stralloc_ready(out,out->len)) return -1;
38   s - out->s;

the gcc compiler's "-O2" option does some strange things to this code, and line 37 ends up setting out->len back to zero for some reason, which ends up causing auth_plain() to return "malformed auth input" in response to an AUTH PLAIN command. (it doesn't help that stralloc_ready() is written as a macro and is therefore impossible to debug directly.)

what i have found is that by changing the code to the following, it works correctly again, both with and without the "-O2" in the conf-cc file:

36   i = (n * 3) - p;
37   if (!stralloc_ready(out,i)) return -1;
38   out->len = i;
39   s = out->s;

i don't know if this is a bug in the stralloc_ready() function or a bug in the compiler optimizations, but for platforms where this is an issue (like my own server) this fix is necessary, and for platforms where this is not an issue i don't see that the functionality of the code is actually changing, so unless you can think of a reason to not make this change, i'm suggesting it.

i have added it to my own combined patch, as it fixes an issue that the users of my patch are complaining about right now. i am interested in hearing your thoughts on whether this is necessary (it appears to be, at least when combined with the rest of my combined patch and used on systems with gcc) and whether or not it hurts anything else (i don't think it does.)

--------------------------------------------------
| John M. Simpson - KG4ZOW - Programmer At Large |
| http://www.jms1.net/           <jms1@jms1.net> |
--------------------------------------------------
| Mac OS X proves that it's easier to make UNIX  |
| pretty than it is to make Windows secure.      |
--------------------------------------------------


Attachment: PGP.sig
Description: This is a digitally signed message part

<Prev in Thread] Current Thread [Next in Thread>