[Discussion] HKDF

Hi all,

Comments are appreciated.
If there is no comment, I'll work on these improvements hopefully soon.

Forgot to mention one more thing.
I also would like to make HKDF a PHPAPI.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Hi all,

Hi all,

There's a pending GitHub pull request of mine to include a HKDF
implementation into ext/hash.
Mostly anybody who saw it agreed that it probably doesn't require an RFC
vote, but I hadn't originally announced it here on the list either, so
this
is what I'm doing now ...

For technical details, I'd say it is best to read IETF RFC 5869, which
defines it, but here's the TL;DR version:

• HKDF stands for "HMAC-based Key Derivation Function"
• Useful in constructing encryption schemes, most notably to derive
  separate keys for encryption, authentication using only a single input
  key.
  Unless you're doing that, you probably don't care about it.
• Unlike e.g. PBKDF2, it is supposed to be fast (as it's not a
  password-based KDF), making it great for encryption/decryption on the fly
  in web applications

There's one thing that may be contentious - whether to call it hkdf() or
hash_hkdf(); there are valid reasons for both and that's what I'd like to
discuss mostly, as everything else boils down to just a tumbs up/down for
the entire thing. But of course, I appreciate all feedback. :)

GitHub PR: https://github.com/php/php-src/pull/1105
IETF RFC: https://tools.ietf.org/html/rfc5869

P.S.: The PR was submitted a long time ago - almost 2 years - thanks to
Joe
for bumping it up.

Nice function, I like it.
Modified patch is committed to master
http://git.php.net/?p=php-src.git;a=commitdiff;h=
4bf7ef08061720586cb0a2f410720e26719d97f3

I have 2 improvement ideas.

1. Make salt parameter required.

Current hash_hkdf() has this signature.

proto string hash_hkdf(string algo, string ikm [, int length = 0, string
info = '', string salt = ''])

I posted inline comment to PR so that make hkdf stronger, but it seems it
was overlooked.

RFC 5869 Section 3.1 states

HKDF is defined to operate with and without random salt. This is
done to accommodate applications where a salt value is not available.
We stress, however, that the use of salt adds significantly to the
strength of HKDF, ensuring independence between different uses of the
hash function, supporting "source-independent" extraction, and
strengthening the analytical results that back the HKDF design.

*SNIP

It is worth noting that, while not the typical case, some
applications may even have a secret salt value available for use; in
such a case, HKDF provides an even stronger security guarantee.

RFC 5869 hkdf is stronger with known random salt, even stronger with
secret random salt. However, salt is the last optional option. This
discourages stronger hkdf usage.

Therefore, I would like to change the signature to

proto string hash_hkdf(string algo, string ikm, string salt [, int
length = 0, string info = ''])

• when salt is null, don't use salt. NOT recommended, but this
  may need for compatibility with other systems.
  Note: This is the same as undefined salt with current patch.

• when salt is ''(empty string), use default static known random salt
  value.
  Note: hkdf's salt could be known, yet provide stronger result as RFC
  states.

• when salt is set, use the value. (The same as now)

Note: ikm does not have to include random salt. Even when ikm includes
random value, secret salt improves security.

Making the salt required makes no sense to me.

HKDF has a number of different applications:
a) Derive multiple strong keys from strong keying material. Typical case
for this is deriving independent encryption and authentication keys from a
master key. This requires only specification of $length. A salt is neither
necessary nor useful in this case, because you start with strong
cryptographic keying material.
b) Generating per-session (or similar) keys from a (strong cryptographic)
master key. For this purpose you can specify the $info parameter. again, a
salt is neither necessary nor useful in this case. (You could probably also
use $salt instead of $info in this case, but the design of the function
implies that $info should be used for this purpose.)
c) Extracting strong cryptographic keying material from weak cryptographic
keying material. Standard example here is extracting strong keys from DH
g^xy values (which are non-uniform) and similar. This is the usage that
benefits from a $salt.
d) Combinations thereof.

Remember that HKDF is an extract-and-expand algorithm, and the extract step
(which uses the salt) is only necessary if the input keying material is
weak. We always include the extract step for compatibility with the overall
HKDF construction (per the RFCs recommendation), but it's essentially just
an unnecessary operation if you work on strong keying material.

The only thing that we may want to discuss is whether we should swap the
$info and the $salt parameters. This depends on which usage (b or c) we
consider more likely.

2. Add hash function whitelisting

There is hash function blacklisting which disallows insecure usage. It's
good enough for now, but whitelisting would help in 3 ways.

1. Warn obsolete hash function usage. e.g. md2()

2. When new hash function is added and blacklist update is
   forgotten, whitelist mitigate it.
   (We are better to have .phpt for it to notify problem to new hash
   function author)

3. Raise E_RECOVEREABLE _ERROR for blacklisted hashes.
   Note: Blacklisted hash usage results in returning FALSE. This may
   result in very weak encryption/etc with @ operator, for example.

The blacklist is an intermediate solution for PHP 7.1. For master the
information on whether a hash function is cryptographic will be integrated
into the hash ops and we will check against this value. The check will also
be expanded to HMAC and PBKDF2.

Note that the reason we're doing these checks is not to discourage use of
obsolete hash functions, but because the HMAC construction has certain
requirements on the hash function, which are generally not satisfied by
non-cryptographic hashes. In particular, HMAC requires a block-based hash
where the block size is greater than or equal to the digest size.

Nikita

Hi,

Nice function, I like it.
Modified patch is committed to master
http://git.php.net/?p=php-src.git;a=commitdiff;h=4bf7ef08061
720586cb0a2f410720e26719d97f3

I have 2 improvement ideas.

1. Make salt parameter required.

Current hash_hkdf() has this signature.

proto string hash_hkdf(string algo, string ikm [, int length = 0, string
info = '', string salt = ''])

I posted inline comment to PR so that make hkdf stronger, but it seems it
was overlooked.

RFC 5869 Section 3.1 states

HKDF is defined to operate with and without random salt. This is
done to accommodate applications where a salt value is not available.
We stress, however, that the use of salt adds significantly to the
strength of HKDF, ensuring independence between different uses of the
hash function, supporting "source-independent" extraction, and
strengthening the analytical results that back the HKDF design.

*SNIP

It is worth noting that, while not the typical case, some
applications may even have a secret salt value available for use; in
such a case, HKDF provides an even stronger security guarantee.

There's no comment from you on the PR, inline or not, but I can assure you
this was not overlooked.

Salt is optional because RFC 5869 allows it to be optional. There's a
reason for each of the current defaults work as they do, as well as the
parameter positions:

• Length is in no way actually described as optional, and that makes sense
  as the function's purpose is to create cryptographic keys, which by nature
  have fixed lengths. The only reason we could make Length optional is
  because hash functions' output sizes are known values, and matching the
  desired OKM length with the hash function size makes for better performance.
• Info can be empty, but the algorithm is pretty much meaningless without
  it. The purpose of HKDF is to derive 2+ outputs from a single input, with
  the Info parameter serving as the differentiating factor.
• Salt is ... while recommended, the only thing actually optional.

Thus, as you can probably see - the one parameter that you want to make
required is actually the only one that should be left optional.

That being said, if anything is to be changed, I would be in favor of
making all parameters required, but I mean that as either make them all
required or leave it untouched; no middle ground.
I just didn't originally feel that this was in PHP's "spirit" ... and again

• this is a 2 year old PR.

RFC 5869 hkdf is stronger with known random salt, even stronger with secret random salt. However, salt is the last optional option. This discourages stronger hkdf usage.

Therefore, I would like to change the signature to

proto string hash_hkdf(string algo, string ikm, string salt [, int length = 0, string info = ''])

• when salt is null, don't use salt. NOT recommended, but this
  may need for compatibility with other systems.
  Note: This is the same as undefined salt with current patch.

• when salt is ''(empty string), use default static known random salt
  value.
  Note: hkdf's salt could be known, yet provide stronger result as RFC
  states.

If it is static and known - it is not random. On the other hand, if you
wanted to generate it on the fly - it would be unusable as the output
wouldn't be reproducible
Section 3.1 of RC 5869 that you quoted above is advice for application
developers; it is NOT a description of how the function should be
implemented as a primitive.

• when salt is set, use the value. (The same as now)

Note: ikm does not have to include random salt. Even when ikm includes
random value, secret salt improves security.

2. Add hash function whitelisting

There is hash function blacklisting which disallows insecure usage. It's
good enough for now, but whitelisting would help in 3 ways.

The blacklist was added for technical reasons, more specifically because
checksum hashes simply don't work in the same way as cryptographic ones;
and there's no php_hash_ops flag to tell us if the hash function in use is
cryptographic one or not.
Yet ... I'll submit another PR to introduce that flag, and switch to a
whitelist approach based on that.

But while that does have the side-effect of enforcing better choices,
"disallowing insecure usage" in the way you meant that is not an achievable
goal.

1. Warn obsolete hash function usage. e.g. md2()

I'd be in favor of such a warning, but only if 1) we're talking about ALL
of ext/hash and not just this function; and 2) it's just an E_WARNING and
doesn't actually block the functionality, for BC reasons.

2. When new hash function is added and blacklist update is
   forgotten, whitelist mitigate it.
   (We are better to have .phpt for it to notify problem to new hash
   function author)

This is a 2-edged sword that can also restrict usage of newly introduced
but more secure hashes - I don't think it's a good idea.

3. Raise E_RECOVEREABLE _ERROR for blacklisted hashes.
   Note: Blacklisted hash usage results in returning FALSE. This may
   result in very weak encryption/etc with @ operator, for example.

Depends on what you mean ... For non-cryptographic hashes - meh, I don't
care; for those that are considered obsolete - no thanks.

Comments are appreciated.
If there is no comment, I'll work on these improvements hopefully soon.

This is not the first time you go ahead with your ideas like that ... There
have been BC breaks in ext/session because of this approach, and I remember
at least one instance of an RM having to revert your frivolous commits.
Please don't do that.

Cheers,
Andrey.