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 function requires a salt/nonce #176

Open
rohintoncollins opened this issue Feb 29, 2024 · 2 comments
Open

Hkdf function requires a salt/nonce #176

rohintoncollins opened this issue Feb 29, 2024 · 2 comments

Comments

@rohintoncollins
Copy link

rohintoncollins commented Feb 29, 2024

According to the HMAC-based Extract-and-Expand Key Derivation Function (HKDF) standard RFC 5869 (https://www.rfc-editor.org/rfc/rfc5869)

salt     optional salt value (a non-secret random value);
         if not provided, it is set to a string of HashLen zeros.

With cryptography version 2.7.0, the salt or nonce is required, even though the parameter is defined as optional with a default value of an empty list.

This code:

    final hkdf = Hkdf(hmac: Hmac.sha256(), outputLength: 32);
    final keyData = await hkdf.deriveKey(secretKey: secretKey);

will emit the following error:

Invalid argument (secretKey): Secret key must be non-empty: Instance of 'SecretKeyData'

This is because on line 44 of HKDF.dart, the nonce is used as the secretKey in a call to the Mac.calculateMac function:

   final nonceAsSecretKey = SecretKey(nonce);
   final prkMac = await Mac.calculateMac(
      secretKeyBytes,
      secretKey: nonceAsSecretKey,
      nonce: nonce
   );

This eventually causes an exception due to the following code in Mac.dart, line 123:

   if (secretKey.bytes.empty) {
      throw ArgumentError.noValue(
         secretKey,
         'secretKey',
         'secretKey must be non-empty',
      )
   }

A workaround is to manually do what the spec suggests: "if not provided, it is set to a string of HashLen zeros".

nonce: List<int>.filled(32, 0)

My testing indicates the latter successfully cross-tests with Botan and CryptoKit.

@rohintoncollins
Copy link
Author

rohintoncollins commented Mar 1, 2024

@terrier989 Please add me as a contributor and I'll push a PR to fix this issue.

Tested fix which does not require workaround in client code would be, in hkdf.dart:

 @override
  Future<SecretKeyData> deriveKey({
    required SecretKey secretKey,
    List<int> nonce = const <int>[],
    List<int> info = const <int>[],
  }) async {
    /// https://github.com/dint-dev/cryptography/issues/176
    if (nonce.length == 0) {
      nonce = List<int>.filled(hmac.hashAlgorithm.hashLengthInBytes, 0);
    }
...

Although you may want to advise whether this fix would be required in other implementations of Hkdf.

@elliotwutingfeng
Copy link

@rohintoncollins Unless this repository has some unusual permissions settings, you do not need to be a collaborator or member to open a pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants