php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Doc Bug #76794 Incorrect use of "$keypair" in LibSodium signing functions
Submitted: 2018-08-25 19:20 UTC Modified: 2018-08-27 14:24 UTC
From: craig at craigfrancis dot co dot uk Assigned:
Status: Open Package: *Encryption and hash functions
PHP Version: Irrelevant OS:
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: craig at craigfrancis dot co dot uk
New email:
PHP Version: OS:

 

 [2018-08-25 19:20 UTC] craig at craigfrancis dot co dot uk
Description:
------------
http://www.php.net/function.sodium-crypto-sign-detached.php

Needs to be changed from "$keypair" to "$secret_key".

http://www.php.net/function.sodium-crypto-sign-open
http://www.php.net/function.sodium-crypto-sign-verify-detached

Both should use "$public_key" - not "$keypair" or "$key".

This would be consistent with the use of "$secret_key" in the signing function:

http://www.php.net/function.sodium-crypto-sign.php


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2018-08-27 12:52 UTC] cmb@php.net
-Package: Documentation problem +Package: *Encryption and hash functions
 [2018-08-27 12:52 UTC] cmb@php.net
In my opinion, it is most sensible to document the parameter names
as they are given by reflection.  E.g.:

# php --rf sodium_crypto_sign_detached
Function [ <internal:sodium> function sodium_crypto_sign_detached ] {

  - Parameters [2] {
    Parameter #0 [ <required> $string ]
    Parameter #1 [ <required> $keypair ]
  }
}

# php --rf sodium_crypto_sign
Function [ <internal:sodium> function sodium_crypto_sign ] {

  - Parameters [2] {
    Parameter #0 [ <required> $string ]
    Parameter #1 [ <required> $keypair ]
  }
}
 [2018-08-27 14:07 UTC] craig at craigfrancis dot co dot uk
Fair point... in which case the underlying code needs to be fixed.

Taking `sodium_crypto_sign_detached` as an example:

<?php

$key_pair = sodium_crypto_sign_keypair();
$secret_key = sodium_crypto_sign_secretkey($key_pair);
$public_key = sodium_crypto_sign_publickey($key_pair);

$signature = sodium_crypto_sign_detached('Hello', $secret_key);
$signature = sodium_crypto_sign_detached('Hello', $key_pair);

?>

The second line, which is provided a keypair (as per the documentation), returns the following error:

  SodiumException: secret key size should be SODIUM_CRYPTO_SIGN_SECRETKEYBYTES bytes

And if you go to the LibSodium documentation, it references "sk" (aka the secret key):

"The crypto_sign_detached() function signs the message m whose length is mlen bytes, using the secret key sk..."

https://download.libsodium.org/doc/public-key_cryptography/public-key_signatures.html#detached-mode

Do I assume that all the LibSodium functions will need to be checked/updated?
 [2018-08-27 14:24 UTC] craig at craigfrancis dot co dot uk
I'm assuming:

https://github.com/php/php-src/blob/49a4e695845bf55e059e7f88e54b1111fe284223/ext/sodium/libsodium.c#L321

Needs to be changed from:

    PHP_FE(sodium_crypto_sign_detached, AI_StringAndKeyPair)

To either use AI_StringAndKey (leaving it ambiguous as to the key type)

    PHP_FE(sodium_crypto_sign_detached, AI_StringAndKey)

Or to introduce some new ARG_INFO's:

    ZEND_BEGIN_ARG_INFO_EX(AI_StringAndSecretKey, 0, 0, 2)
        ZEND_ARG_INFO(0, string)
        ZEND_ARG_INFO(0, secret_key)
    ZEND_END_ARG_INFO()

    ZEND_BEGIN_ARG_INFO_EX(AI_StringAndPublicKey, 0, 0, 2)
        ZEND_ARG_INFO(0, string)
        ZEND_ARG_INFO(0, public_key)
    ZEND_END_ARG_INFO()

    ...

    PHP_FE(sodium_crypto_sign_detached, AI_StringAndSecretKey)
    PHP_FE(sodium_crypto_sign_open, AI_StringAndPublicKey)
 
PHP Copyright © 2001-2018 The PHP Group
All rights reserved.
Last updated: Wed Dec 19 02:01:25 2018 UTC