Skip to content

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

Merged
merged 5 commits into from
Apr 19, 2021

Conversation

paragonie-security
Copy link
Contributor

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

Copy link
Member

@nikic nikic left a 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
Copy link
Member

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.

Copy link
Contributor Author

@paragonie-security paragonie-security Apr 15, 2021

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).

Copy link
Member

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
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);
Copy link
Member

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...

Copy link
Contributor Author

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.

@nikic nikic merged commit f7f1f7f into php:master Apr 19, 2021
@nikic
Copy link
Member

nikic commented Apr 19, 2021

Fixed a few more issues in 6adfe9d, which came up during local testing.

@paragonie-security paragonie-security deleted the sodium-stream-xchacha20 branch April 21, 2021 07:49
paragonie-security added a commit to paragonie/php-src that referenced this pull request May 7, 2021
Also includes the UPGRADING changes for php#6868
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 11, 2022
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
jrfnl added a commit to jrfnl/doc-en that referenced this pull request Mar 11, 2022
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
cmb69 pushed a commit to php/doc-en that referenced this pull request Mar 11, 2022
* 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.
tiffany-taylor pushed a commit to tiffany-taylor/doc-en that referenced this pull request Jan 16, 2023
* 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants