ext/sodium: revert addition of extendable output functions

[DanielEScherzer]

The extendable output functions treated the state as a string, rather than an opaque object, and only validated the lengths of the strings before converting them to the underlying libsodium objects. Thus, incorrect state values could be used to trigger out of bounds reads or updates.

The following constants are removed:

• SODIUM_CRYPTO_XOF_SHAKE128_BLOCKBYTES
• SODIUM_CRYPTO_XOF_SHAKE128_STATEBYTES
• SODIUM_CRYPTO_XOF_SHAKE256_BLOCKBYTES
• SODIUM_CRYPTO_XOF_SHAKE256_STATEBYTES
• SODIUM_CRYPTO_XOF_TURBOSHAKE128_BLOCKBYTES
• SODIUM_CRYPTO_XOF_TURBOSHAKE128_STATEBYTES
• SODIUM_CRYPTO_XOF_TURBOSHAKE256_BLOCKBYTES
• SODIUM_CRYPTO_XOF_TURBOSHAKE256_STATEBYTES

The following functions are removed:

• sodium_crypto_xof_shake128()
• sodium_crypto_xof_shake128_init()
• sodium_crypto_xof_shake128_update()
• sodium_crypto_xof_shake128_squeeze()
• sodium_crypto_xof_shake256()
• sodium_crypto_xof_shake256_init()
• sodium_crypto_xof_shake256_update()
• sodium_crypto_xof_shake256_squeeze()
• sodium_crypto_xof_turboshake128()
• sodium_crypto_xof_turboshake128_init()
• sodium_crypto_xof_turboshake128_update()
• sodium_crypto_xof_turboshake128_squeeze()
• sodium_crypto_xof_turboshake256()
• sodium_crypto_xof_turboshake256_init()
• sodium_crypto_xof_turboshake256_update()
• sodium_crypto_xof_turboshake256_squeeze()

[DanielEScherzer]

Partial revert of #20960, CC @jedisct1

Example problematic code:

$state = str_repeat("\0", SODIUM_CRYPTO_XOF_SHAKE128_STATEBYTES);
$state = substr_replace($state, pack('P', 256), 224, 8);
$state[232] = "\1"; // Skip finalization and preserve the forged offset.

$output = sodium_crypto_xof_shake128_squeeze($state, 32);
printf("leaked=%s\n", bin2hex($output));

or

$state = str_repeat("\0", SODIUM_CRYPTO_XOF_SHAKE128_STATEBYTES);
$state = substr_replace($state, pack('P', 256), 224, 8);
$state[232] = "\0"; // Absorbing phase keeps the attacker-supplied offset.

sodium_crypto_xof_shake128_update($state, str_repeat('A', 16));
echo "update returned\n";

The alternative to a revert is to switch to opaque objects that cannot be forged by userland code, rather than just a string. The question is: is it too late in the release process for 8.6 to add such opaque objects (i.e. add new classes)? Lets ask the 8.6 RMs

[svpernova09]

We've already tagged RC3. Would we need to tag a new RC4 after this is merged?

[TimWolla]

As discussed in DMs.

[jedisct1]

Thanks for catching this!

Yes, no problem, these functions can wait for the next release.

[TimWolla]

We've already tagged RC3. Would we need to tag a new RC4 after this is merged?

@svpernova09 Not necessarily, but getting it in earlier would be preferable of course. Decision is up to you.

[svpernova09]

We've already tagged RC3. Would we need to tag a new RC4 after this is merged?

@svpernova09 Not necessarily, but getting it in earlier would be preferable of course. Decision is up to you.

Given the timing, and assuming builds have all been made for RC3, and it is still in the RC phase where no one should be running it in production, I think it's safe to let RC3 go as planned, and this will land in 2 weeks in RC4. Happy to do something else if there's a precedent.

[TimWolla]

I think it's safe to let RC3 go as planned, and this will land in 2 weeks in RC4.

Okay, in that case, I suggest to wait with the merge until next week: I planned to prepare a PR to change to opaque objects. When we have both, we then can make a decision which one should be included.