Bug #67827 [Com]: [PATCH] broken detection of system crypt sha256/sha512 support

From: Date: Thu, 14 Aug 2014 10:10:34 +0000
Subject: Bug #67827 [Com]: [PATCH] broken detection of system crypt sha256/sha512 support
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-187120@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=67827&edit=1

 ID:                 67827
 Comment by:         ncopa at alpinelinux dot org
 Reported by:        ncopa at alpinelinux dot org
 Summary:            [PATCH] broken detection of system crypt
                     sha256/sha512 support
 Status:             Open
 Type:               Bug
 Package:            *Encryption and hash functions
 Operating System:   Alpine Linux
 PHP Version:        5.5.15
 Block user comment: N
 Private report:     N

 New Comment:

This happens because of this check found in /ext/standard/config.m4:

dnl
dnl If one of them is missing, use our own implementation, portable code is then possible
dnl
if test "$ac_cv_crypt_blowfish" = "no" || test "$ac_cv_crypt_des" =
"no" || test "$ac_cv_crypt_ext_des" = "no" || test
"x$php_crypt_r" = "x0"; then

   # enables bundled crypt_r/des/blowfish/ext_des/md5/sha256/sha512

else

   # hits the buggy codepath in question.
   # I assume the intention was to enable system provided hashing algos
   # but what actually happens is that they gets disabled.

fi


It is also interesting that the above test does not test for sha256/512 at all.

So currently what happens:

On systems that lacks one of blowfish/crypt_r/des/ext_des, php will use the bundled hashing algos
for everything. I believe this happens on most systems and I believe this is why CRYPT_512 is 1 on
centos/ubuntu/archlinux and openbsd (the systems I have checked)

On systems that supports all of blowfish/crypt_r/des/ext_des, but lacks sha256/sha512, php will not
use bundled hashing algos for anything, but instead disable sha256/sha512.

On systems that supports all of blowfish/crypt_r/des/ext_des, *and* sha256/512, php will not use
bundled hashing algos for anything, and hit the buggy codepath in question (with the
uppercase/lowercase typo that my patch fixes) and disable sha256/512. This is what happens with musl
libc and Alpine Linux.

Now the question is, why not only use bundled algos for the things system lacks and use system
crypt(3) for the alogos it supports?

If bundled algos are preferred, why not always use bundled?

Why is test for md5/sha256/sha512 excluded in the above test?

After studying the code i think it would be better to s/ac_cv_crypt_SHA/ac_cv_crypt_sha/g, eg. use
the lowecase variant instead of my proposed patch. This seems to be more consistent with the other
algos which are all in lowecase.


Previous Comments:
------------------------------------------------------------------------
[2014-08-14 06:54:07] fa@php.net

Not that I disagree, but it works for me on a 5.5.15 built on Ubuntu 12.04 on Aug 5th:

PHP Version => 5.5.15

Configure Command =>  './configure'  '--prefix=/usr/local/stow/php-5.5.15'
'--with-config-file-path=/etc/php5/5.5.15' '--enable-fpm'
'--with-fpm-user=www-data' '--with-fpm-group=www-data' '--with-gd'
'--enable-zip' '--with-mysqli=mysqlnd' '--with-pdo-mysql=mysqlnd'
'--with-mysql=mysqlnd' '--enable-mbstring' '--with-curl'
'--with-freetype-dir=/usr/include/freetype2' '--enable-gd-native-ttf'

Hashing Engines => md2 md4 md5 sha1 sha224 sha256 sha384 sha512 ripemd128 ripemd160 ripemd256
ripemd320 whirlpool tiger128,3 tiger160,3 tiger192,3 tiger128,4 tiger160,4 tiger192,4 snefru
snefru256 gost adler32 crc32 crc32b fnv132 fnv164 joaat haval128,3 haval160,3 haval192,3 haval224,3
haval256,3 haval128,4 haval160,4 haval192,4 haval224,4 haval256,4 haval128,5 haval160,5 haval192,5
haval224,5 haval256,5

$ /usr/local/stow/php-5.5.15/bin/php -r 'echo CRYPT_SHA256.PHP_EOL;'
1
$ /usr/local/stow/php-5.5.15/bin/php -r 'echo CRYPT_SHA512.PHP_EOL;'
1

------------------------------------------------------------------------
[2014-08-12 11:11:45] ncopa at alpinelinux dot org

Description:
------------
the configure script checks for sha256/sha512 support in system crypt(3) and sucessfully finds it.
But it will set PHP_SHA_CRYPT to 0 due to a bad test and the result is that CRYPT_256 and CRYPT_512
constants are wrong set to 0 in runtime.

This happens due to a uppercase vs lowercase mismatch in ext/standard/config.m4:
It start with setting ac_vc_crypt_SHA512 (note uppercase SHA512)

AC_CACHE_CHECK(for SHA512 crypt, ac_cv_crypt_SHA512,[
...
}],[
  ac_cv_crypt_SHA512=yes
],[
  ac_cv_crypt_SHA512=no
],[
  ac_cv_crypt_SHA512=no
])])


But some lines below it checks with lowercase $ac_cv_crypt_sha512:

   if test "$ac_cv_crypt_sha512" = "yes"; then
     ac_result=1
     ac_crypt_sha512=1
   else
    ac_result=0
    ac_crypt_sha512=0
  fi
  AC_DEFINE_UNQUOTED(PHP_SHA512_CRYPT, $ac_result, [Whether the system supports SHA512 salt])

And it wil end up setting PHP_SHA512_CRYPT to 0 even if we actually have sha512 support. Same thing
happens with sha256.




Test script:
---------------
echo CRYPT_SHA256."\n";
echo CRYPT_SHA512."\n";


Expected result:
----------------
1

Actual result:
--------------
0


------------------------------------------------------------------------



--
Edit this bug report at https://bugs.php.net/bug.php?id=67827&edit=1


Thread (5 messages)

« previous php.bugs (#187120) next »