Skip to content
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

HKDF key-length inconsistency #3211

Closed
burgerdev opened this issue Nov 1, 2016 · 0 comments

Comments

Projects
None yet
2 participants
@burgerdev
Copy link

commented Nov 1, 2016

For too small key sizes, HKDF.derive() outputs an empty array instead of a small key:

Program:

#!/usr/bin/env python3.5
import cryptography
from cryptography.hazmat.primitives import hashes
from cryptography.hazmat.primitives.kdf.hkdf import HKDF
from cryptography.hazmat.backends import default_backend

print("cryptography.io:{}".format(cryptography.__version__))

hkdf = HKDF(algorithm=hashes.SHA256(), length=4, salt=b"salt",
            info=b"some-test", backend=default_backend())

key = hkdf.derive(b"my secret passphrase")
print("Derived key: {}".format(key))

Output:

cryptography.io:1.5.2
Derived key: b''

Suggested fix:

I am not quite sure why the division by 8 in the snippet below was added. The cumulative size of the output array is always self._algorithm.digest_size * len(output) and thus we can stop after self._algorithm.digest_size * len(output) >= self._length. At first I thought this might be a clever trick taken from the paper, but I didn't find it there. I guess there was a mixup between bits and bytes at some point.

# class HKDFExpand
def _expand(self, key_material):
        output = [b""]
        counter = 1

        while (self._algorithm.digest_size // 8) * len(output) < self._length:
            h = hmac.HMAC(key_material, self._algorithm, backend=self._backend)
            h.update(output[-1])
            h.update(self._info)
            h.update(six.int2byte(counter))
            output.append(h.finalize())
            counter += 1

        return b"".join(output)[:self._length]

alex added a commit that referenced this issue Nov 6, 2016

@alex alex added the security label Nov 6, 2016

@reaperhulk reaperhulk closed this in b924696 Nov 6, 2016

alex added a commit to alex/cryptography that referenced this issue Nov 6, 2016

reaperhulk added a commit that referenced this issue Nov 6, 2016

Backport hkdf short output (#3216)
* Fixes #3211 -- fixed hkdf's output with short length (#3215)

* added a changelog

EnTeQuAk added a commit to mozilla/addons-server that referenced this issue Nov 8, 2016

Update cryptography to 1.5.3
This is a security fix, see
pyca/cryptography#3211 for more details.

We don't have any code directly in olympia that uses cryptography but a
few external libraries that depend on it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.