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

Side by Side 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 unified diff | 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')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <html>

	3 <head>

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

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

	6 </head>

	7 <body>

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

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

	10

	11 <script>

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

	13

	14 jsTestIsAsync = true;

	15

	16 kHkdfKey = "0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b";

	17

	18 var extractable = true;

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

	20 // Create a key with only deriveKey usages.
	eroman 2014/12/23 23:29:39 nit: This comment seems obvious, suggest removing nit: This comment seems obvious, suggest removing it. nharper 2015/01/06 23:51:59 Done. Show quoted text On 2014/12/23 23:29:39, eroman wrote: > nit: This comment seems obvious, suggest removing it. Done.
	21 return crypto.subtle.importKey("raw", hexStringToUint8Array(kHkdfKey), {name: "HKDF"}, extractable, ['deriveKey']);

	22 }).then(function(result) {

	23 baseKey = result;

	24

	25 return crypto.subtle.deriveKey({name: "HKDF", hash: "SHA-256", salt: new Uint8 Array(), info: new Uint8Array()}, baseKey, {name: "AES-GCM", length: 256}, extra ctable, ['encrypt']);
	eroman 2014/12/23 23:29:39 Another test I would like to see is chaining ECDH Another test I would like to see is chaining ECDH and HKDF, since I think that will be a real use-case. nharper 2015/01/06 23:51:59 ECDH currently doesn't support the get key length Show quoted text 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.) eroman 2015/01/07 01:18:42 Other way around. ECDH derives a key for HKDF, and Show quoted text 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. nharper 2015/01/09 17:54:34 That makes more sense (both the order of chaining Show quoted text 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. eroman 2015/01/09 19:27:36 Not sure I understand your claim (1) HKDF _does_ Show quoted text 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 Show quoted text 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.
	26 }).then(function(result) {

	27 derivedKey = result;

	28

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

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

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

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

	33 }).then(finishJSTest, failAndFinishJSTest);

	34

	35 </script>

	36

	37 </body>

	38 </html>

OLD	NEW