Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:100401
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain ohgaki.net designates 180.42.98.130 as permitted sender)
MIME-Version: 1.0
Date: Wed, 6 Sep 2017 10:15:56 +0900
Message-ID: <CAGa2bXbDNe-ruMrkv=gTcq8x_jfuQCjOhzzTRsKy4VdVkOyaWQ@mail.gmail.com>
To: "internals@lists.php.net" <internals@lists.php.net>
Cc: Nikita Popov <nikita.ppv@gmail.com>
Content-Type: multipart/alternative; boundary="94eb2c07aab46a5e2d05587b18c0"
Subject: hash_hkdf() signature and return value
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

--94eb2c07aab46a5e2d05587b18c0
Content-Type: text/plain; charset="UTF-8"

Hi all,

This is the last recommendation for hash_hkdf[1]. In fact,
this would be the last chance to fix because we'll have 7.2 soon.
The issue is secure usage and API consistency.

Currently hash_hkdf() has following signature:

<binary string> hash_hkdf(string $algo , string $ikm [, int $length = 0 [,
string $info = '' [, string $salt = '' ]]] )

These are rationals behind recommendation. There are many, but
please read on.

=== Parameter Order ===

HKDF[2] algorithm is:
 1. General purpose key derivation function as per RFC 5869
 2. "$salt" parameter is a "pre-shared _KEY_" in many cases as mentioned
RFC 5869
 3. "$salt" (or preshared key) is very strongly recommended for security
season as per RFC 5869
 4. Supplying salt that the same length of input key does not affect
performance as per RFC 5969
 5. "$info" is what makes HKDF useful, but it's less important as described
in RFC 5869
 6. "$length" is truly an optional parameter for very specific encryption
algorithm or usage.

Rationale for change:
 1. Key derivations without secondary key ($salt) does not make sense when
     secondary key could be available. HKDF is designed for best possible
key
     security with the key. Not using secondary key ($salt) simply
downgrades
     key security without no reason. i.e. HKDF performance is the same
     when $salt has the same as hash is set.
 2. HKDF is based on HMAC. When $info has no use, HMAC would be the best
     choice for it. i.e. $newkey = hash_hmac($ikm, $key);
 3. It does not make sense violating RFC recommendations for a RFC
implementation.

From these facts and reasons, $salt, $info and $length parameter order and
requirement should be changed from

string $algo , string $ikm [, int $length = 0 [, string $info = '' [,
string $salt = '' ]]]

to

string $algo , string $ikm , string $salt, string $info = '' [, int $length
= 0 ]
Note: Users can set empty string if they really don't need $salt and/or
$info.

Conclusion:
This way, users would have better chances to use hash_hkdf() more securely
and
properly.

[1] http://php.net/hash_hkdf
[2] http://www.faqs.org/rfcs/rfc5869.html

=== Return Value and Output Option ===

The most common HKDF usage with PHP would be:
 1. CSRF token generation that is specific to a request with expiration
time.
     (HEX return value would be appropriate, not BINARY)
 2. API access token generation that does not transmit "The API Key", but
     derived key by HKDF. It also should have expiration time.
     (HEX return value would be appropriate, not BINARY)

Consistency with other hash_*() functions:
 1. All of other hash_*()  returns HEX string hash value.
 2. hash_hkdf() is the only one returns BINARY hash value.

Conclusion:
hash_hkdf() should return HEX by default, not BINARY.
Optional [, bool $raw_output = false ] should be added just like other
hash_*().


=== Compatibility ===

IMHO, current hash_hkdf() should not be added by PHP 7.1.2, but 7.2.0
in the first place. The mess could be resolved by 7.2.

Anyway, hash_hkdf() is added 7.1.2. Hopefully not many users are
using it yet. If we change API with 7.2 release, there would be least
possible confusions. (We may remove it from 7.1 to avoid confusions, too)

Our choices:
 - Keep the current insecure/inconsistent API forever.
 - Change the API to have secure/consistent API forever.

Conclusion:
No conclusion for this. There would be conflicting options.


I strongly think it is not worth to keep this insecure/inconsistent API
forever.
I prefer to change the API to what it should be.

What should we do for this?
Comments?



P.S.

Nikita, you've said following during HKDF discussion:
 - HKDF for CSRF tokens/etc does not make sense at all.
 - Current parameter order and return value makes perfect sense
   and has valid/common usages.
 - Everyone one this list, shouldn't listen to me because I'm insane and
   totally misunderstood what the HKDF is.

Phrases are not the exact, but it should be correct enough. Some part
is my fault, w/o reading mail and/or poor English. I blindly assumed
you've understand RFC 5869 and possible common usages with PHP. I
apologized for the confusion and tried to explain why w/o success.

If you still believe what you've said is correct, I don't request you to
take these back, show us at least one common/reasonable hash_hkdf()
usage example for current API, and point out what's wrong my recommendations
and rationales above.

If not, I request you to take it back. I respect your contributions much,
but
the last one you've said is out of tolerance.

--
Yasuo Ohgaki
yohgaki@ohgaki.net

--94eb2c07aab46a5e2d05587b18c0--