Everything

php

[RFC] [VOTE] Improve hash_hkdf() parameter

php.internals

Yasuo Ohgaki

9 years ago

Hi all,

Since hash_hkdf() is in PHP 7.1.2, I start vote from today.

Current hash_hkdf() function signature does not make sense.

- hash_hkdf() is simple hash_hmac() extension, yet it has totally
different signature.
- Return value is binary unlike other hash functions.
- The signature is insecure.

https://wiki.php.net/rfc/improve_hash_hkdf_parameter

Current signature is overly optimized very limited crypto operation
and cannot be optimal by above reasons.

Fortunately, almost all users are not using current hash_hkdf().
It's only from 7.1.2 to 7.1.4 now. We should avoid yet another
new inconsistent and insecure function. It would be better to be
fixed ASAP, IMHO.

Vote start: 2017-03-25
Vote end: 2017-04-06 UTC 23:59:59

Thank you for voting.
<https://wiki.php.net/rfc/improve_hash_hkdf_parameter>

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Niklas Keller

9 years ago

>
> Hi all,
>
> Since hash_hkdf() is in PHP 7.1.2, I start vote from today.
>
> Current hash_hkdf() function signature does not make sense.
>
>  - hash_hkdf() is simple hash_hmac() extension, yet it has totally
>    different signature.
>  - Return value is binary unlike other hash functions.
>  - The signature is insecure.
>
> https://wiki.php.net/rfc/improve_hash_hkdf_parameter
>
> Current signature is overly optimized very limited crypto operation
> and cannot be optimal by above reasons.
>
> Fortunately, almost all users are not using current hash_hkdf().
> It's only from 7.1.2 to 7.1.4 now. We should avoid yet another
> new inconsistent and insecure function. It would be better to be
> fixed ASAP, IMHO.
>
> Vote start: 2017-03-25
> Vote end: 2017-04-06 UTC 23:59:59
>
> Thank you for voting.
> <https://wiki.php.net/rfc/improve_hash_hkdf_parameter>
> --
> Yasuo Ohgaki
> yohgaki@ohgaki.net
>

Please don't forget to update https://wiki.php.net/rfc next time. Fixed
that for now.

Regards, Niklas

Nikita Popov

9 years ago

On Sat, Mar 25, 2017 at 3:25 AM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:

> Hi all,
>
> Since hash_hkdf() is in PHP 7.1.2, I start vote from today.
>
> Current hash_hkdf() function signature does not make sense.
>
>  - hash_hkdf() is simple hash_hmac() extension, yet it has totally
>    different signature.
>  - Return value is binary unlike other hash functions.
>  - The signature is insecure.
>
> https://wiki.php.net/rfc/improve_hash_hkdf_parameter
>
> Current signature is overly optimized very limited crypto operation
> and cannot be optimal by above reasons.
>
> Fortunately, almost all users are not using current hash_hkdf().
> It's only from 7.1.2 to 7.1.4 now. We should avoid yet another
> new inconsistent and insecure function. It would be better to be
> fixed ASAP, IMHO.
>
> Vote start: 2017-03-25
> Vote end: 2017-04-06 UTC 23:59:59
>

Voting against this because it introduces a BC break on a stable branch in
a point release. Of course I also disagree with the proposed change itself,
but this has already been extensively discussed in previous threads, and I
believe the BC break is sufficient grounds for rejection on its own.

I cannot, however, entirely refrain from pointing out the irony of making
all parameters but $length required, while $length is actually the one
parameter that any reasonable use of this function must specify: otherwise
you would depend on the digest size of the hash function magically
coinciding with the key length of your cipher (for example).

Nikita

Yasuo Ohgaki

9 years ago

Hi Nikita,

On Sat, Mar 25, 2017 at 8:17 PM, Nikita Popov <nikita.ppv@gmail.com> wrote:

> I cannot, however, entirely refrain from pointing out the irony of making
> all parameters but $length required, while $length is actually the one
> parameter that any reasonable use of this function must specify: otherwise
> you would depend on the digest size of the hash function magically
> coinciding with the key length of your cipher (for example).

"info" is the what HKDF makes most important because HMAC does not separate
'secret' (derivation KEY or salt) and 'info' (non secret context). I fail
to see the reason why "derivation KEY" being least important for generic
KEY derivation function.

I totally agree that 'modified length' is mandatory for 'specific crypto',
but they are very limited. "length" cannot be most used option with almost
all PHP applications. See list of possible PHP HKDF applications in the
RFC. I cannot agree opinion that 'length' is the most important HKDF
option. I could be wrong. Could you list applications that requires
modified hash length that could make it most important for PHP apps? I
didn't see any practical examples in discussion so far.

Since hash_hkdf() only exists in PHP 7.1.2/7.1.3, if we are going to fix
"insecure" and "inconsistent" signature, now is the only chance.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Niklas Keller

9 years ago

https://wiki.php.net/rfc/improve_hash_hkdf_parameter#backward_incompatible_changes
says "It is merged into PHP 7.1.2.", but doesn't talk about what it's
supposed to say: It breaks BC with the already released implementation.

https://wiki.php.net/rfc/improve_hash_hkdf_parameter#rfc_impact says
"None.", while it's clearly a BC break.

https://wiki.php.net/rfc/improve_hash_hkdf_parameter#unaffected_php_functionality
says "Nothing is affected. hash_hkdf() is new function does not affect
any.", but hash_hkdf has been released with PHP 7.1.2 and therefore is no
longer a new (unreleased) function.

The BC break and those misleading / wrong paragraphs are enough to vote
against.

Regards, Niklas

2017-03-25 3:25 GMT+01:00 Yasuo Ohgaki <yohgaki@ohgaki.net>:

Yasuo Ohgaki

9 years ago

On Sat, Mar 25, 2017 at 10:16 PM, Niklas Keller <me@kelunik.com> wrote:

> https://wiki.php.net/rfc/improve_hash_hkdf_parameter#
> backward_incompatible_changes says "It is merged into PHP 7.1.2.", but
> doesn't talk about what it's supposed to say: It breaks BC with the already
> released implementation.
>
> https://wiki.php.net/rfc/improve_hash_hkdf_parameter#rfc_impact says
> "None.", while it's clearly a BC break.
>
> https://wiki.php.net/rfc/improve_hash_hkdf_parameter#
> unaffected_php_functionality says "Nothing is affected. hash_hkdf() is
> new function does not affect any.", but hash_hkdf has been released with
> PHP 7.1.2 and therefore is no longer a new (unreleased) function.
>
> The BC break and those misleading / wrong paragraphs are enough to vote
> against.
>

Thank you.
I missed to update RFC and fixed them now.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net