You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by "William A. Rowe, Jr." <wr...@rowe-clan.net> on 2004/05/27 05:16:34 UTC
Re: apr_password_validate on win32 silently mishandles crypted
hashes
At 09:05 PM 5/26/2004, Stas Bekman wrote:
>I've posted the following wording:
>
>/**
> * Validate hashes created by APR-supported algorithms: md5 and base64.
> * hashes created by crypt are supported only on platforms that provide
> * crypt(3), so don't rely on that function unless you know that your
> * application will be run only on platforms that support it.
> * @param passwd The password to validate
> * @param hash The password to validate against
> */
>
>Is that good enough?
_Please_ use @bug to document any platform specific, unpredictable,
or wildly incorrect behavior. The days of /* XXX: */ should be long gone
now that doxygen will tabulate all our problems for us :)
Bill
Re: apr_password_validate on win32 silently mishandles crypted
hashes
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 07:34 AM 5/27/2004, Geoffrey Young wrote:
>> I beg your pardon, gentlemen. Would you be so kind to decide first
>> between yourself whether this is a bug or not? According to Ryan it's
>> not a bug, according to your comment above, Bill, it is.
>
>while I've been up most of the night, so I might not be thinking clearly, it
>seems as though if someone were to move a unix-generated crypt .htpasswd
>file to win32,
Whoa - nobody every promised file compatibility, only functionality
on the same machine. That is not a problem. In apache 2.0 we
went to md5 hashes for *exactly* this reason, across all platforms,
by default.
Even Un*x'es don't have consistent implementations of crypt()!!!
Some are 16 and some are 32 bit seeds, etc.
The @bug should not say "Win32 yadda yadda" - but it should
spell out that different platforms will support different password
storage formats.
>mod_auth's call to apr_password_validate would end up simply
>comparing the two values for equality.
That's wrong, IIRC we test md5's/sha1 first (???). If you want to
drop support for plaintext storage in APR 1.0 - I'd strongly back
that move.
Bill
> meaning that passing the actual hash
>as cleartext would succeed. at least that's what I see when I boil down the
>logic.
>
>APU_DECLARE(apr_status_t) apr_password_validate(const char *passwd,
> const char *hash)
>{
>...
> else {
> /*
> * It's not our algorithm, so feed it to crypt() if possible.
> */
>#if defined(WIN32) || defined(BEOS) || defined(NETWARE)
> apr_cpystrn(sample, passwd, sizeof(sample) - 1);
>...
> return (strcmp(sample, hash) == 0) ? APR_SUCCESS : APR_EMISMATCH;
>}
>
>--Geoff
Re: apr_password_validate on win32 silently mishandles crypted hashes
Posted by Stas Bekman <st...@stason.org>.
rbb@rkbloom.net wrote:
[...]
>>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.
>
>
> I'm positive it isn't a copy-n-paste bug. It is me suffering from my
> standard problem of not being 100% clear. I was generalizing to Windows,
> because Windows is the first platform to have suffered from this problem,
> the rest of the platforms came later, and I don't generally think of them.
> However, the htpasswd docs do clearly state netware.
OK, but you realize that chances are very little that someone will go look for
htpasswd to figure out whey apr_password_validate does things in a certain
way. And I couldn't have guessed that while you talked about win32, it was
also relevant to other platforms.
Just trying to save you and others the wasted time in the future, since I'm
certain I'm not the last person to ask that question :)
Anyways, it's clear to me now, so I'll go and work to find some new bugs to
report. :)
--
__________________________________________________________________
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
Re: apr_password_validate on win32 silently mishandles crypted
hashes
Posted by rb...@rkbloom.net.
On Thu, 27 May 2004, Stas Bekman wrote:
> 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.
I'm positive it isn't a copy-n-paste bug. It is me suffering from my
standard problem of not being 100% clear. I was generalizing to Windows,
because Windows is the first platform to have suffered from this problem,
the rest of the platforms came later, and I don't generally think of them.
However, the htpasswd docs do clearly state netware.
Ryan
Re: apr_password_validate on win32 silently mishandles crypted hashes
Posted by Stas Bekman <st...@stason.org>.
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
Re: apr_password_validate on win32 silently mishandles crypted
hashes
Posted by rb...@rkbloom.net.
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?
Ryan
Re: apr_password_validate on win32 silently mishandles crypted hashes
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> 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
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?
anyway, I'm just saying (as someone unfamiliar with the history here :) that
I'm surprised the current behavior is considered desirable.
> , but since I wrote this code originally
> for 1.3, I'll just answer it here. Apache on Windows doesn't support
> .htpasswd files that were generated on Unix using crypt(). It never has.
> Yes, if you try to use it, and send the crypt() encoded password, it will
> succeed. But, that just doesn't happen in practice.
ok, it's unsupported behavior in httpd-land. but there is a difference
between "not supported" and "oh, by the way, we let invalid passwords
succeed when they shouldn't," especially when it seems such an easy thing to
get right.
but I've said my piece now, so I'll leave it alone. those with apr karma
are in charge.
--Geoff
Re: apr_password_validate on win32 silently mishandles crypted
hashes
Posted by rb...@rkbloom.net.
On Thu, 27 May 2004, Geoffrey Young wrote:
>
> > I beg your pardon, gentlemen. Would you be so kind to decide first
> > between yourself whether this is a bug or not? According to Ryan it's
> > not a bug, according to your comment above, Bill, it is.
>
> I think there is a bug lurking around, at least someplace.
>
> while I've been up most of the night, so I might not be thinking clearly, it
> seems as though if someone were to move a unix-generated crypt .htpasswd
> file to win32, mod_auth's call to apr_password_validate would end up simply
> comparing the two values for equality. meaning that passing the actual hash
> as cleartext would succeed. at least that's what I see when I boil down the
> logic.
>
> APU_DECLARE(apr_status_t) apr_password_validate(const char *passwd,
> const char *hash)
> {
> ...
> else {
> /*
> * It's not our algorithm, so feed it to crypt() if possible.
> */
> #if defined(WIN32) || defined(BEOS) || defined(NETWARE)
> apr_cpystrn(sample, passwd, sizeof(sample) - 1);
> ...
> return (strcmp(sample, hash) == 0) ? APR_SUCCESS : APR_EMISMATCH;
> }
This should move to the httpd list, but since I wrote this code originally
for 1.3, I'll just answer it here. Apache on Windows doesn't support
.htpasswd files that were generated on Unix using crypt(). It never has.
Yes, if you try to use it, and send the crypt() encoded password, it will
succeed. But, that just doesn't happen in practice. In practice, people
try to copy their .htpasswd file to Windows, try sending the real password
as a test to make sure things work. When it doesn't. they either ask
questions or read the htpasswd docs which are quite clear that crypt
passwords don't work on Windows. There may be other docs, but I just
looked where I knew they were.
Ryan
Re: apr_password_validate on win32 silently mishandles crypted hashes
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> I beg your pardon, gentlemen. Would you be so kind to decide first
> between yourself whether this is a bug or not? According to Ryan it's
> not a bug, according to your comment above, Bill, it is.
I think there is a bug lurking around, at least someplace.
while I've been up most of the night, so I might not be thinking clearly, it
seems as though if someone were to move a unix-generated crypt .htpasswd
file to win32, mod_auth's call to apr_password_validate would end up simply
comparing the two values for equality. meaning that passing the actual hash
as cleartext would succeed. at least that's what I see when I boil down the
logic.
APU_DECLARE(apr_status_t) apr_password_validate(const char *passwd,
const char *hash)
{
...
else {
/*
* It's not our algorithm, so feed it to crypt() if possible.
*/
#if defined(WIN32) || defined(BEOS) || defined(NETWARE)
apr_cpystrn(sample, passwd, sizeof(sample) - 1);
...
return (strcmp(sample, hash) == 0) ? APR_SUCCESS : APR_EMISMATCH;
}
--Geoff
Re: apr_password_validate on win32 silently mishandles crypted hashes
Posted by Stas Bekman <st...@stason.org>.
William A. Rowe, Jr. wrote:
> At 09:05 PM 5/26/2004, Stas Bekman wrote:
>
>
>>I've posted the following wording:
>>
>>/**
>>* Validate hashes created by APR-supported algorithms: md5 and base64.
>>* hashes created by crypt are supported only on platforms that provide
>>* crypt(3), so don't rely on that function unless you know that your
>>* application will be run only on platforms that support it.
>>* @param passwd The password to validate
>>* @param hash The password to validate against
>>*/
>>
>>Is that good enough?
>
>
> _Please_ use @bug to document any platform specific, unpredictable,
> or wildly incorrect behavior. The days of /* XXX: */ should be long gone
> now that doxygen will tabulate all our problems for us :)
I beg your pardon, gentlemen. Would you be so kind to decide first between
yourself whether this is a bug or not? According to Ryan it's not a bug,
according to your comment above, Bill, it is.
Or may be there is a need for a new tag @platform or similar which will have
platform specific nuances, which aren't planned to be fixed and therefore
don't fall into the category of a bug?
Besides, I didn't have XXX in the proposed doc section.
--
__________________________________________________________________
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
Re: apr_password_validate on win32 silently mishandles crypted hashes
Posted by Bill Stoddard <bi...@wstoddard.com>.
William A. Rowe, Jr. wrote:
> At 09:05 PM 5/26/2004, Stas Bekman wrote:
>
>
>>I've posted the following wording:
>>
>>/**
>>* Validate hashes created by APR-supported algorithms: md5 and base64.
>>* hashes created by crypt are supported only on platforms that provide
>>* crypt(3), so don't rely on that function unless you know that your
>>* application will be run only on platforms that support it.
>>* @param passwd The password to validate
>>* @param hash The password to validate against
>>*/
>>
>>Is that good enough?
>
>
> _Please_ use @bug to document any platform specific, unpredictable,
> or wildly incorrect behavior. The days of /* XXX: */ should be long gone
> now that doxygen will tabulate all our problems for us :)
Cool!
Bill
Re: apr_password_validate on win32 silently mishandles crypted
hashes
Posted by rb...@rkbloom.net.
I am generally avoiding the branching, because I haven't found a good
explanation of when we back-port and when we don't. So, if somebody else
cares, they can merge.
Ryan
On Thu, 27 May 2004, Stas Bekman wrote:
> rbb@rkbloom.net wrote:
> > This doc update has been committed.
>
> Thanks Ryan.
>
> Shouldn't it get into the APU_0_9_BRANCH branch as well?
>
> --
> __________________________________________________________________
> 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
>
Re: apr_password_validate on win32 silently mishandles crypted hashes
Posted by Stas Bekman <st...@stason.org>.
rbb@rkbloom.net wrote:
> This doc update has been committed.
Thanks Ryan.
Shouldn't it get into the APU_0_9_BRANCH branch as well?
--
__________________________________________________________________
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
Re: apr_password_validate on win32 silently mishandles crypted
hashes
Posted by rb...@rkbloom.net.
This doc update has been committed.
Ryan
On Thu, 27 May 2004, Stas Bekman wrote:
> rbb@rkbloom.net wrote:
> >
> > On Wed, 26 May 2004, William A. Rowe, Jr. wrote:
> >
> >
> >>At 09:05 PM 5/26/2004, Stas Bekman wrote:
> >>
> >>
> >>>I've posted the following wording:
> >>>
> >>>/**
> >>>* Validate hashes created by APR-supported algorithms: md5 and base64.
> >>>* hashes created by crypt are supported only on platforms that provide
> >>>* crypt(3), so don't rely on that function unless you know that your
> >>>* application will be run only on platforms that support it.
> >>>* @param passwd The password to validate
> >>>* @param hash The password to validate against
> >>>*/
> >>>
> >>>Is that good enough?
> >>
> >>_Please_ use @bug to document any platform specific, unpredictable,
> >>or wildly incorrect behavior. The days of /* XXX: */ should be long gone
> >>now that doxygen will tabulate all our problems for us :)
> >
> >
> > One more time now, this isn't a bug. Labeling platform specific behaviour
> > a bug isn't correct. Especially not in this case, where the code works,
> > it just doesn't accept all possible data on some platforms. This wording
> > looks fine to me. I'll try to apply it tonight (no promises at all,
> > between a wife who is due today and house guests, coding time is probably
> > shot to hell for a while, so somebody else should try to beat me to this).
>
> And while you are at it, Ryan, please add a note regarding that clear text
> matching, both in the docs and the code itself. So in the future you get this
> question again and again.
>
>
> --
> __________________________________________________________________
> 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
>
Re: apr_password_validate on win32 silently mishandles crypted hashes
Posted by Stas Bekman <st...@stason.org>.
rbb@rkbloom.net wrote:
>
> On Wed, 26 May 2004, William A. Rowe, Jr. wrote:
>
>
>>At 09:05 PM 5/26/2004, Stas Bekman wrote:
>>
>>
>>>I've posted the following wording:
>>>
>>>/**
>>>* Validate hashes created by APR-supported algorithms: md5 and base64.
>>>* hashes created by crypt are supported only on platforms that provide
>>>* crypt(3), so don't rely on that function unless you know that your
>>>* application will be run only on platforms that support it.
>>>* @param passwd The password to validate
>>>* @param hash The password to validate against
>>>*/
>>>
>>>Is that good enough?
>>
>>_Please_ use @bug to document any platform specific, unpredictable,
>>or wildly incorrect behavior. The days of /* XXX: */ should be long gone
>>now that doxygen will tabulate all our problems for us :)
>
>
> One more time now, this isn't a bug. Labeling platform specific behaviour
> a bug isn't correct. Especially not in this case, where the code works,
> it just doesn't accept all possible data on some platforms. This wording
> looks fine to me. I'll try to apply it tonight (no promises at all,
> between a wife who is due today and house guests, coding time is probably
> shot to hell for a while, so somebody else should try to beat me to this).
And while you are at it, Ryan, please add a note regarding that clear text
matching, both in the docs and the code itself. So in the future you get this
question again and again.
--
__________________________________________________________________
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
Re: apr_password_validate on win32 silently mishandles crypted
hashes
Posted by rb...@rkbloom.net.
On Wed, 26 May 2004, William A. Rowe, Jr. wrote:
> At 09:05 PM 5/26/2004, Stas Bekman wrote:
>
> >I've posted the following wording:
> >
> >/**
> > * Validate hashes created by APR-supported algorithms: md5 and base64.
> > * hashes created by crypt are supported only on platforms that provide
> > * crypt(3), so don't rely on that function unless you know that your
> > * application will be run only on platforms that support it.
> > * @param passwd The password to validate
> > * @param hash The password to validate against
> > */
> >
> >Is that good enough?
>
> _Please_ use @bug to document any platform specific, unpredictable,
> or wildly incorrect behavior. The days of /* XXX: */ should be long gone
> now that doxygen will tabulate all our problems for us :)
One more time now, this isn't a bug. Labeling platform specific behaviour
a bug isn't correct. Especially not in this case, where the code works,
it just doesn't accept all possible data on some platforms. This wording
looks fine to me. I'll try to apply it tonight (no promises at all,
between a wife who is due today and house guests, coding time is probably
shot to hell for a while, so somebody else should try to beat me to this).
Ryan