php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #74896 sodium's .h defines some functions without .c implementation
Submitted: 2017-07-11 07:05 UTC Modified: 2017-07-23 11:34 UTC
From: requinix@php.net Assigned: jedisct1 (profile)
Status: Not a bug Package: Unknown/Other Function
PHP Version: master-Git-2017-07-22 (Git) OS:
Private report: No CVE-ID: None
 [2017-07-11 07:05 UTC] requinix@php.net
Description:
------------
With the libsodium PR merged I did a quick test for functions. php_libsodium.h lists a number of sodium_* functions however some don't have implementations in libsodium.c.

(I checked all 61 functions and found only the 8 listed below.)

My guess is they should be removed
- encrypt/decrypt look like duplicates of the non-'x'ed versions
- pwhash gets bundled into password_hash/verify()
- randombytes gets bundled into random_bytes()

...but I bet this will be a non-issue when the code gets refactored and cleaned up later anyways.

Test script:
---------------
Given a file "sodium" containing:

sodium_crypto_aead_xchacha20poly1305_ietf_decrypt
sodium_crypto_aead_xchacha20poly1305_ietf_encrypt
sodium_crypto_pwhash
sodium_crypto_pwhash_str
sodium_crypto_pwhash_str_verify
sodium_randombytes_buf
sodium_randombytes_random16
sodium_randombytes_uniform

$ xargs -a sodium -l php --rf | grep Exception

Expected result:
----------------
No output (all functions found)

Actual result:
--------------
Exception: Function sodium_crypto_aead_xchacha20poly1305_ietf_decrypt() does not exist
Exception: Function sodium_crypto_aead_xchacha20poly1305_ietf_encrypt() does not exist
Exception: Function sodium_crypto_pwhash() does not exist
Exception: Function sodium_crypto_pwhash_str() does not exist
Exception: Function sodium_crypto_pwhash_str_verify() does not exist
Exception: Function sodium_randombytes_buf() does not exist
Exception: Function sodium_randombytes_random16() does not exist
Exception: Function sodium_randombytes_uniform() does not exist

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2017-07-22 12:03 UTC] ab@php.net
-Assigned To: +Assigned To: jedisct1
 [2017-07-22 16:04 UTC] jedisct1@php.net
-Status: Assigned +Status: Feedback
 [2017-07-22 16:04 UTC] jedisct1@php.net
Please try using this snapshot:

  http://snaps.php.net/php-trunk-latest.tar.gz
 
For Windows:

  http://windows.php.net/snapshots/

Hi,

The sodium_randombytes_* symbols have been removed a while back, as PHP now provide similar functions without this extension (I don't know if the implementation can be defined like in libsodium, though).

sodium_crypto_aead_xchacha20poly1305_ietf_(en|de)crypt are not duplicates!

These are required for XChaCha20-Poly1305 AEAD implementation, which is the recommended construction for most applications.

The variant without the "x" should not be used unless compatibility with other libraries not implementing xchacha20 is required.

This construction was introduced in libsodium 1.0.12, but with some effort we can make it work with libsodium 1.0.9 as well.
 [2017-07-23 07:03 UTC] requinix@php.net
-Status: Feedback +Status: Open -PHP Version: master-Git-2017-07-11 (Git) +PHP Version: master-Git-2017-07-22 (Git)
 [2017-07-23 07:03 UTC] requinix@php.net
By the way I have libsodium 1.0.8 (Ubuntu 16.04). Probably helps to know that, huh?

Building from commit ad698ad earlier today,

Exception: Function sodium_crypto_aead_aes256gcm_decrypt() does not exist
Exception: Function sodium_crypto_aead_aes256gcm_encrypt() does not exist
Exception: Function sodium_crypto_aead_aes256gcm_keygen() does not exist
Exception: Function sodium_crypto_aead_xchacha20poly1305_ietf_decrypt() does not exist
Exception: Function sodium_crypto_aead_xchacha20poly1305_ietf_encrypt() does not exist
Exception: Function sodium_crypto_aead_xchacha20poly1305_ietf_keygen() does not exist
Exception: Function sodium_crypto_pwhash() does not exist
Exception: Function sodium_crypto_pwhash_str() does not exist
Exception: Function sodium_crypto_pwhash_str_verify() does not exist

randombytes was removed, but pwhash and xchacha20poly1305 are still missing, and now three aes256gcm functions are missing too.

I was lazy before and didn't check the .c. The implementations are there, of course, but I see they use #ifdefs. I think the .h should have them too?
 [2017-07-23 08:52 UTC] jedisct1@php.net
How do you get these errors messages to print? Do I need to install PHP and enable this extension?

"make test" doesn't seem to say anything about sodium.
 [2017-07-23 10:34 UTC] requinix@php.net
Yes, what I'm doing does need PHP built --with-sodium, and I do it after the make.

It's not as fancy as make test. I took all the functions from php_libsodium.h (just the function names), put them into a file, then ran "php --rf <function>" for each one. --rf will dump a function definition or error if the function is undefined.

Linux:
/path/to/php-src$ xargs -a list_of_functions.txt -l sapi/cli/php --rf | grep Exception

Windows:
C:\path\to\php-src> for /f %i in (list_of_functions.txt) do @sapi/cli/php --rf %i | findstr "Exception"


But you can ignore it all and just check the code: if the PHP_FEs and PHP_FUNCTIONs in libsodium.c are in #ifdefs then the same PHP_FUNCTIONs in php_libsodium.h should have them too.
That does mean moving some #defines out of the .c and into the .h.

Compare with what ext/curl does. ext/curl/interface.c has

const zend_function_entry curl_functions[] = {
...
#if LIBCURL_VERSION_NUM >= 0x070c00 /* 7.12.0 */
	PHP_FE(curl_strerror,            arginfo_curl_strerror)
	PHP_FE(curl_multi_strerror,      arginfo_curl_multi_strerror)
	PHP_FE(curl_share_strerror,      arginfo_curl_share_strerror)
#endif

and

#if LIBCURL_VERSION_NUM >= 0x070c00 /* Available since 7.12.0 */
/* {{{ proto bool curl_strerror(int code)
      return string describing error code */
PHP_FUNCTION(curl_strerror)
{
...
}
/* }}} */
#endif

To match that, php_curl.h has

#if LIBCURL_VERSION_NUM >= 0x070c00 /* 7.12.0 */
PHP_FUNCTION(curl_strerror);
PHP_FUNCTION(curl_multi_strerror);
PHP_FUNCTION(curl_share_strerror);
#endif
 [2017-07-23 10:40 UTC] jedisct1@php.net
But why does it matter to have an internal prototype defined is the php_*.h file and no actual function definition?
 [2017-07-23 11:34 UTC] requinix@php.net
-Status: Assigned +Status: Not a bug
 [2017-07-23 11:34 UTC] requinix@php.net
It doesn't matter, as far as I know. With randombytes already removed, pwhash where it needs to be, and xchacha20poly1305 not a duplicate, there's nothing really to do here.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Apr 18 04:01:27 2024 UTC