LayoutTests/crypto/subtle/hkdf-deriveKey.html - Issue 822083003: Webcrypto: add layout tests for HKDF algorithm

Unified Diff: LayoutTests/crypto/subtle/hkdf-deriveKey.html

Issue 822083003: Webcrypto: add layout tests for HKDF algorithm (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Patch Set: Created 6 years ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« LayoutTests/crypto/subtle/hkdf-deriveBits-failures.html ('K') | « LayoutTests/crypto/subtle/hkdf-deriveBits-failures-expected.txt ('k') | LayoutTests/crypto/subtle/hkdf-deriveKey-expected.txt » ('j') | LayoutTests/crypto/subtle/hkdf-importKey.html » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: LayoutTests/crypto/subtle/hkdf-deriveKey.html

diff --git a/LayoutTests/crypto/subtle/hkdf-deriveKey.html b/LayoutTests/crypto/subtle/hkdf-deriveKey.html

new file mode 100644

index 0000000000000000000000000000000000000000..4bedf218773cc0b554ec9bd54ef3ebb7fe4f7157

--- /dev/null

+++ b/LayoutTests/crypto/subtle/hkdf-deriveKey.html

@@ -0,0 +1,38 @@

+<!DOCTYPE html>

+<html>

+<head>

+<script src="../../resources/js-test.js"></script>

+<script src="resources/common.js"></script>

+</head>

+<body>

+<p id="description"></p>

+<div id="console"></div>

+<script>

+description("Test deriveKey() for HKDF");

+jsTestIsAsync = true;

+kHkdfKey = "0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b";

+var extractable = true;

+Promise.resolve(null).then(function(result) {

+ // Create a key with only deriveKey usages.

eroman 2014/12/23 23:29:39 nit: This comment seems obvious, suggest removing

nharper 2015/01/06 23:51:59 Done.

+ return crypto.subtle.importKey("raw", hexStringToUint8Array(kHkdfKey), {name: "HKDF"}, extractable, ['deriveKey']);

+}).then(function(result) {

+ baseKey = result;

+ return crypto.subtle.deriveKey({name: "HKDF", hash: "SHA-256", salt: new Uint8Array(), info: new Uint8Array()}, baseKey, {name: "AES-GCM", length: 256}, extractable, ['encrypt']);

eroman 2014/12/23 23:29:39 Another test I would like to see is chaining ECDH

nharper 2015/01/06 23:51:59 ECDH currently doesn't support the get key length

eroman 2015/01/07 01:18:42 Other way around. ECDH derives a key for HKDF, and

nharper 2015/01/09 17:54:34 That makes more sense (both the order of chaining

That makes more sense (both the order of chaining ECDH->HKDF and that an asymmetric algorithm like ECDH doesn't have a get key length operation). I tried writing the test to derive an HKDF key from an ECDH key, but it fails because HKDF doesn't have a get key length operation. (It makes sense that it doesn't have a get key length operation, as you can use any length for your IKM.) This means that if one wants to derive an HKDF key ECDH keys, one has to call deriveBits with the ECDH keys and use the shared secret generated by deriveBits as the input to HKDF's importKey. If we wanted to support the use case of using deriveKey with ECDH and HKDF, we'd need to add the get key length operation to HKDF, which would mean that we'd need a subclass of KeyAlgorithm for HKDF that had a length parameter. I would like to say that we should keep the spec the way it is (i.e. to make an HKDF key from ECDH, one has to use deriveBits and importKey), but the more I think about it, the less I think this is good for the webcrypto API. If ECDH in webcrypto is used for static keys, the current API design will encourage people to derive keys (like AES keys) directly from the shared secret of ECDH key agreement, because one can call deriveKey with an ECDH keypair to generate an AES key (for example), which is a Bad Idea(tm). Doing the correct thing of using the shared secret from ECDH as input to a KDF and deriving their key through the KDF is more work for the API user, because they must call deriveBits with the ECDH keys, know what the length is of the secret generated from the curve they're using, and then import the derivedBits into an HKDF key. (If the ECDH keys in question are ephemeral and not static, then it might be acceptable to use the secret straight from ECDH key agreement, but I don't know the crypto well enough.) If the spec were changed to support deriving an HKDF key from ECDH keys, I would suggest that the spec also be changed so that deriveKey for ECDH keys can only be used to derive KDF keys. (If someone wanted the current behavior of being able to generate an AES key directly from an ECDH secret, that would still be possible, but it would require them using deriveBits and importKey instead.) Or, the API can stay as it is - after all, it is understood that it is subtle (hence the name crypto.subtle) and easy to shoot one's self in the foot with.

eroman 2015/01/09 19:27:36 Not sure I understand your claim (1) HKDF _does_

On 2015/01/09 17:54:34, nharper wrote: > On 2015/01/07 01:18:42, eroman wrote: > > On 2015/01/06 23:51:59, nharper wrote: > > > On 2014/12/23 23:29:39, eroman wrote: > > > > Another test I would like to see is chaining ECDH and HKDF, since I think > > that > > > > will be a real use-case. > > > > > > ECDH currently doesn't support the get key length operation, so I can't > derive > > > an ECDH key. Should it support this operation? (If so, that's s spec bug.) > > > > Other way around. ECDH derives a key for HKDF, and then you call deriveBits() > / > > deriveKey() on HKDF for the final key. > > > > The output of ECDH is not suited to used directly as key material, so common > > usage should feed it through a KDF first. > > > > As far as ECDH having a get key length operation, I don't think that would > make > > sense for a asymmetric key algorithm. ECDH does support the "raw" format so > you > > could imagine it being used with deriveKey(), however the expectation for raw > > import is a public key. > > That makes more sense (both the order of chaining ECDH->HKDF and that an > asymmetric algorithm like ECDH doesn't have a get key length operation). > > I tried writing the test to derive an HKDF key from an ECDH key, but it fails > because HKDF doesn't have a get key length operation. (It makes sense that it > doesn't have a get key length operation, as you can use any length for your > IKM.) This means that if one wants to derive an HKDF key ECDH keys, one has to > call deriveBits with the ECDH keys and use the shared secret generated by > deriveBits as the input to HKDF's importKey. If we wanted to support the use > case of using deriveKey with ECDH and HKDF, we'd need to add the get key length > operation to HKDF, which would mean that we'd need a subclass of KeyAlgorithm > for HKDF that had a length parameter. > > I would like to say that we should keep the spec the way it is (i.e. to make an > HKDF key from ECDH, one has to use deriveBits and importKey), but the more I > think about it, the less I think this is good for the webcrypto API. If ECDH in > webcrypto is used for static keys, the current API design will encourage people > to derive keys (like AES keys) directly from the shared secret of ECDH key > agreement, because one can call deriveKey with an ECDH keypair to generate an > AES key (for example), which is a Bad Idea(tm). Doing the correct thing of using > the shared secret from ECDH as input to a KDF and deriving their key through the > KDF is more work for the API user, because they must call deriveBits with the > ECDH keys, know what the length is of the secret generated from the curve > they're using, and then import the derivedBits into an HKDF key. (If the ECDH > keys in question are ephemeral and not static, then it might be acceptable to > use the secret straight from ECDH key agreement, but I don't know the crypto > well enough.) > > If the spec were changed to support deriving an HKDF key from ECDH keys, I would > suggest that the spec also be changed so that deriveKey for ECDH keys can only > be used to derive KDF keys. (If someone wanted the current behavior of being > able to generate an AES key directly from an ECDH secret, that would still be > possible, but it would require them using deriveBits and importKey instead.) Or, > the API can stay as it is - after all, it is understood that it is subtle (hence > the name crypto.subtle) and easy to shoot one's self in the foot with.

Not sure I understand your claim (1) HKDF _does_ support get key length, and it is an omission of your current CL that it is not implemented. (2) Deriving KDF keys from ECDH is pretty much the whole point of the deriveKey() operation (3) Agreed that deriving an AES key from ECDH would be silly. However it is called crypto.subtle.* so the burden is on the caller to do something that isn't dumb.

nharper 2015/01/09 22:41:46 My claim was based on a) not realizing that HKDF d

On 2015/01/09 19:27:36, eroman wrote: > On 2015/01/09 17:54:34, nharper wrote: > > On 2015/01/07 01:18:42, eroman wrote: > > > On 2015/01/06 23:51:59, nharper wrote: > > > > On 2014/12/23 23:29:39, eroman wrote: > > > > > Another test I would like to see is chaining ECDH and HKDF, since I > think > > > that > > > > > will be a real use-case. > > > > > > > > ECDH currently doesn't support the get key length operation, so I can't > > derive > > > > an ECDH key. Should it support this operation? (If so, that's s spec bug.) > > > > > > Other way around. ECDH derives a key for HKDF, and then you call > deriveBits() > > / > > > deriveKey() on HKDF for the final key. > > > > > > The output of ECDH is not suited to used directly as key material, so common > > > usage should feed it through a KDF first. > > > > > > As far as ECDH having a get key length operation, I don't think that would > > make > > > sense for a asymmetric key algorithm. ECDH does support the "raw" format so > > you > > > could imagine it being used with deriveKey(), however the expectation for > raw > > > import is a public key. > > > > That makes more sense (both the order of chaining ECDH->HKDF and that an > > asymmetric algorithm like ECDH doesn't have a get key length operation). > > > > I tried writing the test to derive an HKDF key from an ECDH key, but it fails > > because HKDF doesn't have a get key length operation. (It makes sense that it > > doesn't have a get key length operation, as you can use any length for your > > IKM.) This means that if one wants to derive an HKDF key ECDH keys, one has to > > call deriveBits with the ECDH keys and use the shared secret generated by > > deriveBits as the input to HKDF's importKey. If we wanted to support the use > > case of using deriveKey with ECDH and HKDF, we'd need to add the get key > length > > operation to HKDF, which would mean that we'd need a subclass of KeyAlgorithm > > for HKDF that had a length parameter. > > > > I would like to say that we should keep the spec the way it is (i.e. to make > an > > HKDF key from ECDH, one has to use deriveBits and importKey), but the more I > > think about it, the less I think this is good for the webcrypto API. If ECDH > in > > webcrypto is used for static keys, the current API design will encourage > people > > to derive keys (like AES keys) directly from the shared secret of ECDH key > > agreement, because one can call deriveKey with an ECDH keypair to generate an > > AES key (for example), which is a Bad Idea(tm). Doing the correct thing of > using > > the shared secret from ECDH as input to a KDF and deriving their key through > the > > KDF is more work for the API user, because they must call deriveBits with the > > ECDH keys, know what the length is of the secret generated from the curve > > they're using, and then import the derivedBits into an HKDF key. (If the ECDH > > keys in question are ephemeral and not static, then it might be acceptable to > > use the secret straight from ECDH key agreement, but I don't know the crypto > > well enough.) > > > > If the spec were changed to support deriving an HKDF key from ECDH keys, I > would > > suggest that the spec also be changed so that deriveKey for ECDH keys can only > > be used to derive KDF keys. (If someone wanted the current behavior of being > > able to generate an AES key directly from an ECDH secret, that would still be > > possible, but it would require them using deriveBits and importKey instead.) > Or, > > the API can stay as it is - after all, it is understood that it is subtle > (hence > > the name crypto.subtle) and easy to shoot one's self in the foot with. > > Not sure I understand your claim > > (1) HKDF _does_ support get key length, and it is an omission of your current CL > that it is not implemented. > (2) Deriving KDF keys from ECDH is pretty much the whole point of the > deriveKey() operation > (3) Agreed that deriving an AES key from ECDH would be silly. However it is > called crypto.subtle.* so the burden is on the caller to do something that isn't > dumb.

My claim was based on a) not realizing that HKDF does support get key length (and it returns null), and b) not realizing that ECDH is clever and interprets a null length in derive bits to mean return all bits of the secret. Now that I'm aware of that, my point is moot.

+}).then(function(result) {

+ derivedKey = result;

+ shouldEvaluateAs("derivedKey.type", "secret");

+ shouldEvaluateAs("derivedKey.extractable", true);

+ shouldEvaluateAs("derivedKey.algorithm.name", "AES-GCM");

+ shouldEvaluateAs("derivedKey.usages.join(',')", "encrypt");

+}).then(finishJSTest, failAndFinishJSTest);

+</script>

+</body>

+</html>