Skip to content

Commit 06d308f

Browse files
panvasxa
panva
authored and
sxa
committed
tools: prevent lib code from reading KeyObject and CryptoKey accessors
Add ESLint rules that reject public KeyObject and CryptoKey accessor reads after internal brand checks. Internal code must use the private key helpers so it reads native-backed slots instead of user-replaceable properties. Add a separate rule that rejects instanceof checks against KeyObject and CryptoKey constructors, including the global CryptoKey constructor. Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #63111 Backport-PR-URL: #63563 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
1 parent 4433fca commit 06d308f

8 files changed

Lines changed: 942 additions & 0 deletions

‎lib/eslint.config_partial.mjs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ export default [
6363
'no-buffer-constructor': 'error',
6464
'no-restricted-syntax': [
6565
...noRestrictedSyntax,
66+
{
67+
selector: "VariableDeclarator[init.type='CallExpression'][init.callee.name='internalBinding'][init.arguments.0.value='crypto'] > ObjectPattern > Property[key.name='getCryptoKeySlots']",
68+
message: "Use `const { getCryptoKeySlots } = require('internal/crypto/keys');` instead of destructuring it from `internalBinding('crypto')`.",
69+
},
6670
{
6771
selector: "VariableDeclarator[init.type='CallExpression'][init.callee.name='internalBinding'][init.arguments.0.value='crypto'] > ObjectPattern > Property[key.name='getKeyObjectSlots']",
6872
message: "Use `const { getKeyObjectSlots } = require('internal/crypto/keys');` instead of destructuring it from `internalBinding('crypto')`.",
@@ -398,6 +402,9 @@ export default [
398402
'node-core/lowercase-name-for-primitive': 'error',
399403
'node-core/non-ascii-character': 'error',
400404
'node-core/no-array-destructuring': 'error',
405+
'node-core/no-cryptokey-public-accessors': 'error',
406+
'node-core/no-keyobject-cryptokey-instanceof': 'error',
407+
'node-core/no-keyobject-public-accessors': 'error',
401408
'node-core/prefer-primordials': [
402409
'error',
403410
{ name: 'AggregateError' },

‎lib/internal/crypto/keys.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ const {
1616
// eslint-disable-next-line no-restricted-syntax -- intended here
1717
getKeyObjectSlots: nativeGetKeyObjectSlots,
1818
createCryptoKeyClass,
19+
// eslint-disable-next-line no-restricted-syntax -- intended here
1920
getCryptoKeySlots: nativeGetCryptoKeySlots,
2021
kKeyTypeSecret,
2122
kKeyTypePublic,

‎test/parallel/test-eslint-no-cryptokey-public-accessors.js‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
common.skipIfEslintMissing();
5+
6+
const RuleTester = require('../../tools/eslint/node_modules/eslint').RuleTester;
7+
const rule = require('../../tools/eslint-rules/no-cryptokey-public-accessors');
8+
9+
new RuleTester().run('no-cryptokey-public-accessors', rule, {
10+
valid: [
11+
'foo.algorithm;',
12+
`
13+
const { isCryptoKey, getCryptoKeyAlgorithm } =
14+
require('internal/crypto/keys');
15+
if (isCryptoKey(key)) {
16+
getCryptoKeyAlgorithm(key);
17+
}
18+
`,
19+
`
20+
const { CryptoKey } = require('internal/crypto/keys');
21+
class Key extends CryptoKey {
22+
get type() { return 'secret'; }
23+
}
24+
`,
25+
`
26+
key = webidl.converters.KeyFormat(key);
27+
key.algorithm;
28+
`,
29+
],
30+
invalid: [
31+
{
32+
code: `
33+
const { isCryptoKey } = require('internal/crypto/keys');
34+
if (isCryptoKey(key)) {
35+
key.type;
36+
}
37+
`,
38+
errors: [{ messageId: 'noPublicAccessor' }],
39+
},
40+
{
41+
code: `
42+
const { isCryptoKey: check } = require('internal/crypto/keys');
43+
if (check(key) && key.extractable) {}
44+
`,
45+
errors: [{ messageId: 'noPublicAccessor' }],
46+
},
47+
{
48+
code: `
49+
const { isCryptoKey } = require('internal/crypto/keys');
50+
if (!isCryptoKey(key)) {
51+
throw new TypeError();
52+
}
53+
key.algorithm.name;
54+
`,
55+
errors: [{ messageId: 'noPublicAccessor' }],
56+
},
57+
{
58+
code: `
59+
const keys = require('internal/crypto/keys');
60+
if (!keys.isCryptoKey(key)) throw new TypeError();
61+
key['usages'];
62+
`,
63+
errors: [{ messageId: 'noPublicAccessor' }],
64+
},
65+
{
66+
code: `
67+
key = webidl.converters.CryptoKey(key);
68+
key.algorithm;
69+
`,
70+
errors: [{ messageId: 'noPublicAccessor' }],
71+
},
72+
{
73+
code: `
74+
const key = webidl.converters.CryptoKey(value);
75+
key.usages;
76+
`,
77+
errors: [{ messageId: 'noPublicAccessor' }],
78+
},
79+
{
80+
code: `
81+
class CryptoKey {
82+
inspect() { return this.algorithm; }
83+
}
84+
`,
85+
errors: [{ messageId: 'noPublicAccessor' }],
86+
},
87+
],
88+
});

‎test/parallel/test-eslint-no-keyobject-cryptokey-instanceof.js‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
common.skipIfEslintMissing();
5+
6+
const RuleTester = require('../../tools/eslint/node_modules/eslint').RuleTester;
7+
const rule = require('../../tools/eslint-rules/no-keyobject-cryptokey-instanceof');
8+
9+
new RuleTester().run('no-keyobject-cryptokey-instanceof', rule, {
10+
valid: [
11+
'key instanceof Buffer;',
12+
'key instanceof KeyObject;',
13+
`
14+
const { isKeyObject } = require('internal/crypto/keys');
15+
isKeyObject(key);
16+
`,
17+
`
18+
const { isCryptoKey } = require('internal/crypto/keys');
19+
isCryptoKey(key);
20+
`,
21+
],
22+
invalid: [
23+
{
24+
code: `
25+
const { KeyObject } = require('internal/crypto/keys');
26+
key instanceof KeyObject;
27+
`,
28+
errors: [{ messageId: 'noKeyObjectInstanceof' }],
29+
},
30+
{
31+
code: `
32+
const { KeyObject: KO } = require('internal/crypto/keys');
33+
key instanceof KO;
34+
`,
35+
errors: [{ messageId: 'noKeyObjectInstanceof' }],
36+
},
37+
{
38+
code: `
39+
const keys = require('internal/crypto/keys');
40+
key instanceof keys.KeyObject;
41+
`,
42+
errors: [{ messageId: 'noKeyObjectInstanceof' }],
43+
},
44+
{
45+
code: `
46+
key instanceof CryptoKey;
47+
`,
48+
errors: [{ messageId: 'noCryptoKeyInstanceof' }],
49+
},
50+
{
51+
code: `
52+
const { CryptoKey } = require('internal/crypto/keys');
53+
key instanceof CryptoKey;
54+
`,
55+
errors: [{ messageId: 'noCryptoKeyInstanceof' }],
56+
},
57+
{
58+
code: `
59+
const { CryptoKey: CK } = require('internal/crypto/webcrypto');
60+
key instanceof CK;
61+
`,
62+
errors: [{ messageId: 'noCryptoKeyInstanceof' }],
63+
},
64+
{
65+
code: `
66+
const webcrypto = require('internal/crypto/webcrypto');
67+
key instanceof webcrypto.CryptoKey;
68+
`,
69+
errors: [{ messageId: 'noCryptoKeyInstanceof' }],
70+
},
71+
{
72+
code: `
73+
key instanceof globalThis.CryptoKey;
74+
`,
75+
errors: [{ messageId: 'noCryptoKeyInstanceof' }],
76+
},
77+
],
78+
});

‎test/parallel/test-eslint-no-keyobject-public-accessors.js‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
common.skipIfEslintMissing();
5+
6+
const RuleTester = require('../../tools/eslint/node_modules/eslint').RuleTester;
7+
const rule = require('../../tools/eslint-rules/no-keyobject-public-accessors');
8+
9+
new RuleTester().run('no-keyobject-public-accessors', rule, {
10+
valid: [
11+
'foo.type;',
12+
`
13+
const { isKeyObject, getKeyObjectType } =
14+
require('internal/crypto/keys');
15+
if (isKeyObject(key)) {
16+
getKeyObjectType(key);
17+
}
18+
`,
19+
`
20+
const { isKeyObject } = require('internal/crypto/keys');
21+
if (format === 'raw-public') {
22+
key.asymmetricKeyType;
23+
}
24+
`,
25+
`
26+
const { KeyObject } = require('internal/crypto/keys');
27+
class Key extends KeyObject {
28+
get type() { return 'secret'; }
29+
}
30+
`,
31+
],
32+
invalid: [
33+
{
34+
code: `
35+
const { isKeyObject } = require('internal/crypto/keys');
36+
if (isKeyObject(key)) {
37+
key.type;
38+
}
39+
`,
40+
errors: [{ messageId: 'noPublicAccessor' }],
41+
},
42+
{
43+
code: `
44+
const { isKeyObject: check } = require('internal/crypto/keys');
45+
if (check(key) && key.symmetricKeySize === 32) {}
46+
`,
47+
errors: [{ messageId: 'noPublicAccessor' }],
48+
},
49+
{
50+
code: `
51+
const { isKeyObject } = require('internal/crypto/keys');
52+
if (!isKeyObject(otherKeyObject)) {
53+
throw new TypeError();
54+
}
55+
otherKeyObject.asymmetricKeyType;
56+
`,
57+
errors: [{ messageId: 'noPublicAccessor' }],
58+
},
59+
{
60+
code: `
61+
const keys = require('internal/crypto/keys');
62+
if (!keys.isKeyObject(otherKeyObject)) throw new TypeError();
63+
otherKeyObject.asymmetricKeyDetails;
64+
`,
65+
errors: [{ messageId: 'noPublicAccessor' }],
66+
},
67+
{
68+
code: `
69+
class SecretKeyObject extends KeyObject {
70+
export() { return this.symmetricKeySize; }
71+
}
72+
`,
73+
errors: [{ messageId: 'noPublicAccessor' }],
74+
},
75+
{
76+
code: `
77+
const { isKeyObject } = require('internal/crypto/keys');
78+
if (isKeyObject(key)) {
79+
key.equals(otherKey);
80+
}
81+
`,
82+
errors: [{ messageId: 'noPublicAccessor' }],
83+
},
84+
],
85+
});

0 commit comments

Comments
 (0)