-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add crypto_stream_xchacha20 to ext/sodium #6868
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a reindent from spaces to tabs, but otherwise should be fine per upstream review.
#ifdef crypto_stream_xchacha20_KEYBYTES | ||
#define arginfo_sodium_crypto_stream_xchacha20_xor arginfo_sodium_crypto_stream_xor | ||
#define arginfo_sodium_crypto_stream_xchacha20 arginfo_sodium_crypto_stream | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a generated file, run build/gen_stub.php ext/sodium to regenerate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function isn't guaranteed to exist on older versions (1.0.11 and earlier) of libsodium. Does build/gen_stub.php
need to take this into consideration?
3v4l.org in particular seems to use an ancient version of the library (possibly 1.0.8).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you need to add the corresponding #ifdef
in the stub file as well. They will be transferred to the generated output.
Paragon Initiative Enterprises is aware of PHP applications that use sodium_compat's ParagonIE\Sodium\Core\XChaCha20 class directly for stream encryption. Greater performance and security assurance is offered by exposing libsodium's crypto_stream_xchacha20 API to PHP users. It's acceptable to only include this change in PHP 8.1+; the offending applications are more than welcome to either install ext/sodium from PECL or upgrade to 8.1 when it comes out later this year. Ref: jedisct1/libsodium-php#211
61ac3b6
to
86254fd
Compare
zend_argument_error(sodium_exception_ce, 3, "key must be SODIUM_CRYPTO_STREAM_XCHACHA20_KEYBYTES bytes long"); | ||
RETURN_THROWS(); | ||
} | ||
ciphertext = zend_string_checked_alloc((size_t) ciphertext_len, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, it looks like this function was added in the PECL extension in jedisct1/libsodium-php@75701f2, but is missing here...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
./configure
doesn't work locally, otherwise we would have caught these without having to depend on CI.
Fixed a few more issues in 6adfe9d, which came up during local testing. |
Also includes the UPGRADING changes for php#6868
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well... Includes unit tests. Refs: * php/php-src#6868 * php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well... Refs: * php/php-src#6868 * php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357 While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well... Refs: * php/php-src#6922 * php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1] > * Added CURLOPT_DOH_URL option > * Added certificate blob options when for libcurl >= 7.71.0: > > CURLOPT_ISSUERCERT_BLOB > CURLOPT_PROXY_ISSUERCERT > CURLOPT_PROXY_ISSUERCERT_BLOB > CURLOPT_PROXY_SSLCERT_BLOB > CURLOPT_PROXY_SSLKEY_BLOB > CURLOPT_SSLCERT_BLOB > CURLOPT_SSLKEY_BLOB Refs: * https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229 * php/php-src#6612 * php/php-src@3dad63b * php/php-src#7194 * php/php-src@b11785c * PHP 8.1 | MigrationGuide/New constants: add missing constants [2] > GD: > * Avif support is now available through the `imagecreatefromavif()` and > `imageavif()` functions, if libgd has been built with avif support. While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration... Refs: * https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247 * php/php-src#7026 * php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373 > GD: > * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as > quality. This constant is only defined, if a libgd is used which supports > lossless WebP encoding. Refs: * https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571 * php/php-src#7348 * php/php-src@eb6c9eb * PHP 8.1 | MigrationGuide/New constants: add missing constants [3] > Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD. Refs: * https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix * php/php-src#6608 * php/php-src@ebca8de * PHP 8.1 | MigrationGuide/New constants: add missing constants [4] Refs: * https://wiki.php.net/rfc/readonly_properties_v2 * php/php-src#7089 * php/php-src@6780aaa * PHP 8.1 | MigrationGuide/New constants: add missing constants [5] While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well... Refs: * php/php-src#6868 * php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357 While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well... Refs: * php/php-src#6922 * php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381 Co-authored-by: jrfnl <jrfnl@users.noreply.github.com> Closes GH-1449.
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1] > * Added CURLOPT_DOH_URL option > * Added certificate blob options when for libcurl >= 7.71.0: > > CURLOPT_ISSUERCERT_BLOB > CURLOPT_PROXY_ISSUERCERT > CURLOPT_PROXY_ISSUERCERT_BLOB > CURLOPT_PROXY_SSLCERT_BLOB > CURLOPT_PROXY_SSLKEY_BLOB > CURLOPT_SSLCERT_BLOB > CURLOPT_SSLKEY_BLOB Refs: * https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229 * php/php-src#6612 * php/php-src@3dad63b * php/php-src#7194 * php/php-src@b11785c * PHP 8.1 | MigrationGuide/New constants: add missing constants [2] > GD: > * Avif support is now available through the `imagecreatefromavif()` and > `imageavif()` functions, if libgd has been built with avif support. While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration... Refs: * https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247 * php/php-src#7026 * php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373 > GD: > * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as > quality. This constant is only defined, if a libgd is used which supports > lossless WebP encoding. Refs: * https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571 * php/php-src#7348 * php/php-src@eb6c9eb * PHP 8.1 | MigrationGuide/New constants: add missing constants [3] > Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD. Refs: * https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix * php/php-src#6608 * php/php-src@ebca8de * PHP 8.1 | MigrationGuide/New constants: add missing constants [4] Refs: * https://wiki.php.net/rfc/readonly_properties_v2 * php/php-src#7089 * php/php-src@6780aaa * PHP 8.1 | MigrationGuide/New constants: add missing constants [5] While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well... Refs: * php/php-src#6868 * php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357 While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well... Refs: * php/php-src#6922 * php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381 Co-authored-by: jrfnl <jrfnl@users.noreply.github.com> Closes phpGH-1449.
Paragon Initiative Enterprises is aware of PHP applications that use sodium_compat's ParagonIE\Sodium\Core\XChaCha20 class directly for stream encryption.
Greater performance and security assurance is offered by exposing libsodium's crypto_stream_xchacha20 API to PHP users.
It's acceptable to only include this change in PHP 8.1+; the offending applications are more than welcome to either install ext/sodium from PECL or upgrade to 8.1 when it comes out later this year.
Ref: jedisct1/libsodium-php#211
Ref: paragonie/sodium_compat#128