Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Precedence: bulk
Message-ID: <40B622BA.8010404@stason.org>
Date: Thu, 27 May 2004 10:17:46 -0700
From: Stas Bekman <stas@stason.org>
Organization: Hope, Humanized
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040115
MIME-Version: 1.0
To: rbb@rkbloom.net
Cc: Geoffrey Young <geoff@modperlcookbook.org>,
	"William A. Rowe, Jr." <wrowe@rowe-clan.net>,
	APR Development <dev@apr.apache.org>
Subject: Re: apr_password_validate on win32 silently mishandles crypted
  hashes
References: <Pine.LNX.4.33L2.0405270926300.14581-100000@vangogh.rkbloom.net>
In-Reply-To: <Pine.LNX.4.33L2.0405270926300.14581-100000@vangogh.rkbloom.net>
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

rbb@rkbloom.net wrote:
> On Thu, 27 May 2004, Geoffrey Young wrote:
> 
> 
>>>This should move to the httpd list
>>
>>um, ok, but it's not necessarily a .htpasswd specific issue.  anyone trying
>>to use apr_password_validate on win32 could potentially run into this.
>>
>>the snag, as I see it, is that the fallback position on systems with crypt
>>is crypt, while the fallback for systems that don't understand crypt is a
>>simple string comparison.  I think that is incredibly misleading for users
>>of those latter platforms - it goes beyond the simple platform nuances we
>>all accept and into "oh, no!  that's not what I wanted!"
>>
>>since the comment for the function is currently
>>
>>  * Validate any password encypted with any algorithm that APR understands
> 
> 
> Right, which Stas has already posted a patch to fix.
> 
> 
>>and APR currently doesn't understand crypt for win32, then I would suggest
>>that it is better to return APR_EMISMATCH outright.  if people wanted a
>>simple string match they could do it themselves, right?
> 
> 
> It would be better in a perfect world.  However, we don't live in a
> perfect world.  We live in a world where we need to support legacy apps,
> and in this case, we need to support legacy Windows .htpasswd files that
> used plain text.  Could we tell Apache to do that?  Sure, but this
> function should do the work, and it was moved into APR, so we are stuck.
> If you really want to fix this, remove this method from APR all together.
> Provide a series of functions to md5 passwords, sha1 passwords, crypt
> passwords.  Then, Apache can re-implement this quickly and easily.  The
> crypt check can return APR_ENOTIMPL on Windows, and everything becomes
> happy happy.
> 
> I never liked having this stuff in APR, but it is here now, so we either
> live with supporting legacy httpd stuff, or we remove the function all
> together.  But, this function's real goal is MD5 and SHA1 password
> verification.  Anything else is just a bonus for Apache legacy support.
> That is documented with Stas' patch, so we can drop this now, right?

I think there is still a remaining problem. You said that this clear-text 
matching is special to windows, but it's not true, as the code goes:

#if defined(WIN32) || defined(BEOS) || defined(NETWARE)
         apr_cpystrn(sample, passwd, sizeof(sample) - 1);

Are you sure, this is not copy-n-paste bug? An inline comment would have made 
it clear.


-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com