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.

Sign up for GitHub

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

feat(ext/crypto): import JWK key for HMAC #11716

Merged
merged 22 commits into from
Aug 27, 2021
Merged

feat(ext/crypto): import JWK key for HMAC #11716

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
c5b75eb
importKey JWK HMAC
littledivy Aug 15, 2021
1a66fcf
update expectations
littledivy Aug 15, 2021
bccacf3
update expectations
littledivy Aug 15, 2021
2aff32e
decodeSymmetricKey
littledivy Aug 15, 2021
ad4fc0a
Merge branch 'main' into importKey_hmac_jwk
littledivy Aug 17, 2021
991c2d0
update expectations
littledivy Aug 17, 2021
cf66ab5
typings and test
littledivy Aug 17, 2021
24a17c8
fix lint
littledivy Aug 17, 2021
da6cb31
fix types
littledivy Aug 17, 2021
00d622a
fix test
littledivy Aug 17, 2021
1cafcf3
Merge branch 'main' into importKey_hmac_jwk
littledivy Aug 25, 2021
44674c8
fixes
littledivy Aug 25, 2021
7dc2eb5
prefer-primordials lint rule is awesome
littledivy Aug 25, 2021
2139dcc
fix
littledivy Aug 25, 2021
4a40ae9
fixy
littledivy Aug 25, 2021
2349caa
fix
lucacasonato Aug 26, 2021
2cf3313
Merge remote-tracking branch 'origin/main' into importKey_hmac_jwk
lucacasonato Aug 26, 2021
0290447
fmt
lucacasonato Aug 26, 2021
ca6dd48
Merge branch 'main' into importKey_hmac_jwk
littledivy Aug 27, 2021
8b9ca27
remove (
littledivy Aug 27, 2021
5bb2c79
make decodeSymmetricKey more readable
littledivy Aug 27, 2021
045d751
fix lint
littledivy Aug 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 36 additions & 11 deletions cli/tests/unit/webcrypto_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,22 +243,43 @@ unitTest(async function testSignRSASSAKey() {
assert(signature);
});

// deno-fmt-ignore
const rawKey = new Uint8Array([
1, 2, 3, 4, 5, 6, 7, 8,
9, 10, 11, 12, 13, 14, 15, 16
]);

const jwk: JsonWebKey = {
kty: "oct",
// unpadded base64 for rawKey.
k: "AQIDBAUGBwgJCgsMDQ4PEA",
alg: "HS256",
};

unitTest(async function subtleCryptoHmacImportExport() {
// deno-fmt-ignore
const rawKey = new Uint8Array([
1, 2, 3, 4, 5, 6, 7, 8,
9, 10, 11, 12, 13, 14, 15, 16
]);
const key = await crypto.subtle.importKey(
const key1 = await crypto.subtle.importKey(
"raw",
rawKey,
{ name: "HMAC", hash: "SHA-256" },
true,
["sign"],
);
const actual = await crypto.subtle.sign(
const key2 = await crypto.subtle.importKey(
"jwk",
jwk,
{ name: "HMAC", hash: "SHA-256" },
true,
["sign"],
);
const actual1 = await crypto.subtle.sign(
{ name: "HMAC" },
key,
key1,
new Uint8Array([1, 2, 3, 4]),
);

const actual2 = await crypto.subtle.sign(
{ name: "HMAC" },
key2,
new Uint8Array([1, 2, 3, 4]),
);
// deno-fmt-ignore
Expand All @@ -269,10 +290,14 @@ unitTest(async function subtleCryptoHmacImportExport() {
23, 122, 222, 1, 146, 46, 182, 87,
]);
assertEquals(
new Uint8Array(actual),
new Uint8Array(actual1),
expected,
);

const exportedKey = await crypto.subtle.exportKey("raw", key);
assertEquals(
new Uint8Array(actual2),
expected,
);
// TODO(@littledivy): Add a test for exporting JWK key when supported.
const exportedKey = await crypto.subtle.exportKey("raw", key1);
assertEquals(new Uint8Array(exportedKey), rawKey);
});
259 changes: 205 additions & 54 deletions ext/crypto/00_crypto.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,15 @@

const {
ArrayPrototypeFind,
ArrayBufferIsView,
ArrayPrototypeEvery,
ArrayPrototypeIncludes,
ArrayPrototypeMap,
ArrayBufferIsView,
BigInt64Array,
StringPrototypeToUpperCase,
StringPrototypeReplace,
StringPrototypeSplit,
StringPrototypeCharCodeAt,
Symbol,
SymbolFor,
SymbolToStringTag,
Expand Down Expand Up @@ -95,6 +100,25 @@
},
};

// Decodes the unpadded base64 to the octet sequence containing key value `k` defined in RFC7518 Section 6.4
function decodeSymmetricKey(key) {
return new Uint8Array(
ArrayPrototypeMap(
StringPrototypeSplit(
atob(
littledivy marked this conversation as resolved.
Show resolved Hide resolved
StringPrototypeReplace(
StringPrototypeReplace(key, /\-/g, "+"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using split() and replace() has the (global, user visible) side effect of clobbering RegExp.$_, etc.

(Aside: I also feel this code is really hard to read.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@littledivy Can you split this across multiple lines rather than nesting these calls?

Copy link
Member Author

@littledivy littledivy Aug 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done (also made it a bit more readable)

/\_/g,
"/",
),
),
"",
),
(c) => StringPrototypeCharCodeAt(c, 0),
),
);
}

// See https://www.w3.org/TR/WebCryptoAPI/#dfn-normalize-an-algorithm
// 18.4.4
function normalizeAlgorithm(algorithm, op) {
Expand Down Expand Up @@ -626,7 +650,7 @@
prefix,
context: "Argument 1",
});
keyData = webidl.converters.BufferSource(keyData, {
keyData = webidl.converters["BufferSource or JsonWebKey"](keyData, {
prefix,
context: "Argument 2",
});
Expand All @@ -644,84 +668,211 @@
});

// 2.
if (ArrayBufferIsView(keyData)) {
keyData = new Uint8Array(
keyData.buffer,
keyData.byteOffset,
keyData.byteLength,
);
if (format !== "jwk") {
if (ArrayBufferIsView(keyData) || (keyData instanceof ArrayBuffer)) {
littledivy marked this conversation as resolved.
Show resolved Hide resolved
if (ArrayBufferIsView(keyData)) {
keyData = new Uint8Array(
keyData.buffer,
keyData.byteOffset,
keyData.byteLength,
);
} else {
keyData = new Uint8Array(keyData);
}
keyData = TypedArrayPrototypeSlice(keyData);
} else {
throw new TypeError("keyData is a JsonWebKey");
}
} else {
keyData = new Uint8Array(keyData);
if (ArrayBufferIsView(keyData) || (keyData instanceof ArrayBuffer)) {
throw new TypeError("keyData is not a JsonWebKey");
}
}
keyData = TypedArrayPrototypeSlice(keyData);

const normalizedAlgorithm = normalizeAlgorithm(algorithm, "importKey");

if (
ArrayPrototypeFind(
keyUsages,
(u) => !ArrayPrototypeIncludes(["sign", "verify"], u),
) !== undefined
) {
throw new DOMException("Invalid key usages", "SyntaxError");
}

switch (normalizedAlgorithm.name) {
// https://w3c.github.io/webcrypto/#hmac-operations
case "HMAC": {
// 2.
if (
ArrayPrototypeFind(
keyUsages,
(u) => !ArrayPrototypeIncludes(["sign", "verify"], u),
) !== undefined
) {
throw new DOMException("Invalid key usages", "SyntaxError");
}

// 3.
let hash;
let data;
// 4. https://w3c.github.io/webcrypto/#hmac-operations
switch (format) {
case "raw": {
const hash = normalizedAlgorithm.hash;
data = keyData;
hash = normalizedAlgorithm.hash;
break;
}
case "jwk": {
// TODO(@littledivy): Why does the spec validate JWK twice?
const jwk = keyData;
// 2.
if (jwk.kty !== "oct") {
throw new DOMException(
"`kty` member of JsonWebKey must be `oct`",
"DataError",
);
}

// Section 6.4.1 of RFC7518
if (!jwk.k) {
throw new DOMException(
"`k` member of JsonWebKey must be present",
"DataError",
);
}

// 4.
data = decodeSymmetricKey(jwk.k);
// 5.
let length = keyData.byteLength * 8;
hash = normalizedAlgorithm.hash;
// 6.
if (length === 0) {
throw new DOMException("Key length is zero", "DataError");
switch (hash.name) {
case "SHA-1": {
if (jwk.alg !== undefined && jwk.alg !== "HS1") {
throw new DOMException(
"`alg` member of JsonWebKey must be `HS1`",
"DataError",
);
}
break;
}
case "SHA-256": {
if (jwk.alg !== undefined && jwk.alg !== "HS256") {
throw new DOMException(
"`alg` member of JsonWebKey must be `HS256`",
"DataError",
);
}
break;
}
case "SHA-384": {
if (jwk.alg !== undefined && jwk.alg !== "HS384") {
throw new DOMException(
"`alg` member of JsonWebKey must be `HS384`",
"DataError",
);
}
break;
}
case "SHA-512": {
if (jwk.alg !== undefined && jwk.alg !== "HS512") {
throw new DOMException(
"`alg` member of JsonWebKey must be `HS512`",
"DataError",
);
}
break;
}
default:
throw new TypeError("unreachable");
}
if (normalizeAlgorithm.length) {
// 7.

// 7.
if (keyUsages.length > 0 && jwk.use && jwk.use !== "sign") {
throw new DOMException(
"`use` member of JsonWebKey must be `sign`",
"DataError",
);
}

// 8.
// Section 4.3 of RFC7517
if (jwk.key_ops) {
if (
normalizedAlgorithm.length > length ||
normalizedAlgorithm.length <= (length - 8)
ArrayPrototypeFind(
jwk.key_ops,
(u) => !ArrayPrototypeIncludes(recognisedUsages, u),
) !== undefined
) {
throw new DOMException(
"Key length is invalid",
"`key_ops` member of JsonWebKey is invalid",
"DataError",
);
}
length = normalizeAlgorithm.length;
}

if (keyUsages.length == 0) {
throw new DOMException("Key usage is empty", "SyntaxError");
if (
!ArrayPrototypeEvery(
jwk.key_ops,
(u) => ArrayPrototypeIncludes(keyUsages, u),
)
) {
throw new DOMException(
"`key_ops` member of JsonWebKey is invalid",
"DataError",
);
}
}

const handle = {};
WeakMapPrototypeSet(KEY_STORE, handle, {
type: "raw",
data: keyData,
});

const algorithm = {
name: "HMAC",
length,
hash,
};

const key = constructKey(
"secret",
extractable,
usageIntersection(keyUsages, recognisedUsages),
algorithm,
handle,
);
// 9.
if (jwk.ext == false && extractable == true) {
littledivy marked this conversation as resolved.
Show resolved Hide resolved
throw new DOMException(
"`ext` member of JsonWebKey is invalid",
"DataError",
);
}

return key;
break;
}
// TODO(@littledivy): jwk
default:
throw new DOMException("Not implemented", "NotSupportedError");
}

// 5.
let length = data.byteLength * 8;
// 6.
if (length === 0) {
throw new DOMException("Key length is zero", "DataError");
}
if (normalizeAlgorithm.length !== undefined) {
lucacasonato marked this conversation as resolved.
Show resolved Hide resolved
// 7.
if (
normalizedAlgorithm.length > length ||
normalizedAlgorithm.length <= (length - 8)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks kind of weird to me. length === data.byteLength * 8 so is this trying to say if (length !== normalizedAlgoritm.length) in a roundabout way?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It expresses the following:
image

) {
throw new DOMException(
"Key length is invalid",
"DataError",
);
}
length = normalizeAlgorithm.length;
}

if (keyUsages.length == 0) {
throw new DOMException("Key usage is empty", "SyntaxError");
}
Comment on lines +855 to +857
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does the spec say to do this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://w3c.github.io/webcrypto/#SubtleCrypto-method-importKey (importKey Step 9)


const handle = {};
WeakMapPrototypeSet(KEY_STORE, handle, {
type: "raw",
data,
});

const algorithm = {
name: "HMAC",
length,
hash,
};

const key = constructKey(
"secret",
extractable,
usageIntersection(keyUsages, recognisedUsages),
algorithm,
handle,
);

return key;
}
// TODO(@littledivy): RSASSA-PKCS1-v1_5
// TODO(@littledivy): RSA-PSS
Expand Down
Loading