Re: Debian/Ubuntu php_crypt_revamped.patch

[Kurt Seifried <kseifried () redhat com>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

So I'm guessing this needs a CVE #?

On 05/04/2012 09:31 AM, Solar Designer wrote:
> Hi,
>
> Boaz Rymland reported to me what he thought was a phpass bug, where
> any password would be valid when authenticated against a NULL or
> empty password hash.  (Indeed, the password hash shouldn't normally
> be NULL or empty, but it is better for authentication code to be
> fail-close rather than fail-open.)  At first, I was not able to
> reproduce the problem, but after exchanging a few e-mails we were
> able to narrow it down to PHP crypt() call returning an empty
> string when called with NULL or an empty string for the salt
> argument on Boaz' Ubuntu 11.04 system with php5 5.3.5-1ubuntu7.7.
> I was still not able to reproduce that on other systems (with other
> versions of PHP).
>
> Today, I downloaded and built clean PHP 5.3.5 on an Owl system. I
> still could not trigger the problem.  Then I applied 
> debian/patches/php_crypt_revamped.patch from Debian's 
> php5_5.3.5-1.diff.gz - and the problem finally appeared.
>
> Original:
>
> php@owl:~ $ ~/php-5.3.5/bin/php -r 'echo crypt("pass", null),
> "\n";' $1$l5Nwx5hu$NhostJ7i8jP1B.4C4zaiM78.
>
> With Debian patch:
>
> php@owl:~ $ ~/php-5.3.5-debian/bin/php -r 'echo crypt("pass",
> null), "\n";'
>
> php@owl:~ $
>
> (empty string was printed).
>
> It turns out that the patch first appeared in Debian's 5.3.2-1 in 
> response to almost a non-issue (different behavior across PHP
> versions for an invalid salt string) and general feeling that PHP
> should be using system-provided crypto instead of its bundled code
> when possible (questionable to me: each approach has its pros and
> cons):
>
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=572601
>
> The handling of NULL/empty salt strings was corrected in 5.3.6-1,
> as well as in 5.3.3-7+squeeze4 (stable-security):
>
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=581170
>
> Apparently, that fix never made it into Ubuntu 11.04 updates - so
> I guess this should happen now.
>
> Overall, the patch looks problematic to me.  Here's another problem
> with it, still reproducible on 5.3.10-1ubuntu3 (Ubuntu 12.04):
>
> user@ubuntu:~$ php -r 'echo crypt("pass", "_J9..Salt"), "\n";' 
> _J9..Saltr2Hq6I3ZH0s user@ubuntu:~$ php -r 'echo crypt("pass",
> "_J9..Saltr2Hq6I3ZH0s"), "\n";' _J0LlWX63dRZg
>
> That non-security bug is with the "salt_len == 9" check added with
> the patch.  So phpass' authentication against CRYPT_EXT_DES hashes,
> which it tries to support, would be failing on Debian/Ubuntu
> systems.  I guess I need to introduce a workaround for it now,
> complicating the code. :-(
>
> I think it may be best to drop this patch from further versions of 
> Debian/Ubuntu - and not reintroduce it even in response to the
> likely "bug" reports from Debian users complaining about the
> behavior change from previous versions of Debian.
>
> I agree that the code in upstream PHP may need improvement, but
> that patch does not improve it, and the deviation from upstream is
> bad. Altering the behavior of PHP on specific distros beyond what
> may normally happen due to PHP's ./configure is undesirable and
> should only be done for very good reasons.
>
> Sorry for the rant.
>
> Alexander


- -- 
Kurt Seifried Red Hat Security Response Team (SRT)
PGP: 0x5E267993 A90B F995 7350 148F 66BF 7554 160D 4553 5E26 7993

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJPo/8VAAoJEBYNRVNeJnmTQyoP/jQqdJ6VeZe5cCcjElPQt5AN
37zVkxkY0x5TZLhrrlWmuTGfeYKe9JitWlntQ8Ju9gaTcX6/pzRqRD7pmYkRyV2Q
3hYXd+wMgirNxA+AjgePa+MqQBStpcg4mBeRcUx6fUjVzSLSYMlPgUKABXoEuDFM
1kpYdz+1DTQlmSaV+SCQ11KUfIisNVJl1DzJrlneu27JrKbjf8TDLoxi4yuJyXNL
ZEG+aUQtpo9H+oAP8UnAP2iUlvAKpAqdf44vyBRLHMs0DW8zTCYhINg6TNZXokne
KzJ58tmGcW39nCC4BgYTK0C28ZsI7i1XGk+3ABuVMngFaDz8wXBRfx0Ht++TKED3
zMkzeKeyKYfU8XSO6xt6aOUbXW2wLu5JAevdO/Bhe0aTvZJtHiFL2ocI8tyJN8Av
9Ru66u8vD3hDslkn676MvZTIlrlR/gtBTOFHnjl233zaMERakH2ktxWgobr9qU2i
9yN9ZYEUXVZv5wlJEK48c0pd337/gcDdcbGF4bFQ2AqfBt6RX+LKdgpxKH/9DztU
YWA8sKu9yQ0UJ+PWLroDcoo92Q1XxmNN9LM5+4O+QgXnsNgwXo9BdTopfrcuuJr8
OCkxcDt4nayz2aT1VGoid2hY7vQohaS24Yf3NILhx+cUiZDlPM24xpGN1FukWvlq
RjYAtiCy3B7zyNEZBzxl
=XSLr
-----END PGP SIGNATURE-----