Skip to content

Commit 4a32ed8

Browse files
panvaaduh95
panva
authored and
aduh95
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 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
1 parent fd509a7 commit 4a32ed8

8 files changed

Lines changed: 947 additions & 6 deletions

‎lib/eslint.config_partial.mjs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,9 @@ export default [
424424
'node-core/lowercase-name-for-primitive': 'error',
425425
'node-core/non-ascii-character': 'error',
426426
'node-core/no-array-destructuring': 'error',
427+
'node-core/no-cryptokey-public-accessors': 'error',
428+
'node-core/no-keyobject-cryptokey-instanceof': 'error',
429+
'node-core/no-keyobject-public-accessors': 'error',
427430
'node-core/prefer-primordials': [
428431
'error',
429432
{ name: 'AggregateError' },

‎lib/internal/crypto/diffiehellman.js‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ const {
5555
getCryptoKeyAlgorithm,
5656
getCryptoKeyHandle,
5757
getCryptoKeyType,
58+
getKeyObjectAsymmetricKeyType,
59+
getKeyObjectType,
5860
preparePrivateKey,
5961
preparePublicOrPrivateKey,
6062
} = require('internal/crypto/keys');
@@ -296,20 +298,22 @@ function diffieHellman(options, callback) {
296298
throw new ERR_INVALID_ARG_VALUE('options.publicKey', publicKey);
297299

298300
if (isKeyObject(privateKey)) {
299-
if (privateKey.type !== 'private')
300-
throw new ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE(privateKey.type, 'private');
301+
const type = getKeyObjectType(privateKey);
302+
if (type !== 'private')
303+
throw new ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE(type, 'private');
301304
}
302305

303306
if (isKeyObject(publicKey)) {
304-
if (publicKey.type !== 'public' && publicKey.type !== 'private') {
305-
throw new ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE(publicKey.type,
307+
const type = getKeyObjectType(publicKey);
308+
if (type !== 'public' && type !== 'private') {
309+
throw new ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE(type,
306310
'private or public');
307311
}
308312
}
309313

310314
if (isKeyObject(privateKey) && isKeyObject(publicKey)) {
311-
const privateType = privateKey.asymmetricKeyType;
312-
const publicType = publicKey.asymmetricKeyType;
315+
const privateType = getKeyObjectAsymmetricKeyType(privateKey);
316+
const publicType = getKeyObjectAsymmetricKeyType(publicKey);
313317
if (privateType !== publicType || !dhEnabledKeyTypes.has(privateType)) {
314318
throw new ERR_CRYPTO_INCOMPATIBLE_KEY('key types for Diffie-Hellman',
315319
`${privateType} and ${publicType}`);

‎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)