From daea065f249648a005a9caaa548a76db42eacbb1 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 7 Jul 2024 02:56:04 +0200 Subject: [PATCH] tls: remove prototype primordials Co-authored-by: Yagiz Nizipli PR-URL: https://github.com/nodejs/node/pull/53699 Reviewed-By: Moshe Atlow Reviewed-By: Yagiz Nizipli Reviewed-By: Marco Ippolito Reviewed-By: Luigi Pinca Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina Reviewed-By: Rafael Gonzaga Reviewed-By: Chemi Atlow Reviewed-By: James M Snell --- doc/contributing/primordials.md | 1 + lib/_tls_common.js | 32 ++++++------- lib/_tls_wrap.js | 46 ++++++++----------- lib/eslint.config_partial.mjs | 4 +- lib/tls.js | 81 ++++++++++++--------------------- 5 files changed, 68 insertions(+), 96 deletions(-) diff --git a/doc/contributing/primordials.md b/doc/contributing/primordials.md index a3362c71def..e7d94147a87 100644 --- a/doc/contributing/primordials.md +++ b/doc/contributing/primordials.md @@ -9,6 +9,7 @@ important than reliability against prototype pollution: * `node:http` * `node:http2` +* `node:tls` Usage of primordials should be preferred for new code in other areas, but replacing current code with primordials should be diff --git a/lib/_tls_common.js b/lib/_tls_common.js index b828785decc..362b8e41893 100644 --- a/lib/_tls_common.js +++ b/lib/_tls_common.js @@ -22,9 +22,7 @@ 'use strict'; const { - ArrayPrototypePush, JSONParse, - RegExpPrototypeSymbolReplace, } = primordials; const tls = require('tls'); @@ -133,21 +131,21 @@ function translatePeerCertificate(c) { c.infoAccess = { __proto__: null }; // XXX: More key validation? - RegExpPrototypeSymbolReplace(/([^\n:]*):([^\n]*)(?:\n|$)/g, info, - (all, key, val) => { - if (val.charCodeAt(0) === 0x22) { - // The translatePeerCertificate function is only - // used on internally created legacy certificate - // objects, and any value that contains a quote - // will always be a valid JSON string literal, - // so this should never throw. - val = JSONParse(val); - } - if (key in c.infoAccess) - ArrayPrototypePush(c.infoAccess[key], val); - else - c.infoAccess[key] = [val]; - }); + info.replace(/([^\n:]*):([^\n]*)(?:\n|$)/g, + (all, key, val) => { + if (val.charCodeAt(0) === 0x22) { + // The translatePeerCertificate function is only + // used on internally created legacy certificate + // objects, and any value that contains a quote + // will always be a valid JSON string literal, + // so this should never throw. + val = JSONParse(val); + } + if (key in c.infoAccess) + c.infoAccess[key].push(val); + else + c.infoAccess[key] = [val]; + }); } return c; } diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index cc79b30aaca..4a065ecf26e 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -22,19 +22,11 @@ 'use strict'; const { - ArrayPrototypeForEach, - ArrayPrototypeJoin, - ArrayPrototypePush, - FunctionPrototype, ObjectAssign, ObjectDefineProperty, ObjectSetPrototypeOf, ReflectApply, RegExp, - RegExpPrototypeExec, - RegExpPrototypeSymbolReplace, - StringPrototypeReplaceAll, - StringPrototypeSlice, Symbol, SymbolFor, } = primordials; @@ -119,7 +111,7 @@ const kPskIdentityHint = Symbol('pskidentityhint'); const kPendingSession = Symbol('pendingSession'); const kIsVerified = Symbol('verified'); -const noop = FunctionPrototype; +const noop = () => {}; let ipServernameWarned = false; let tlsTracingWarned = false; @@ -475,8 +467,7 @@ function onerror(err) { owner.destroy(err); } else if (owner._tlsOptions?.isServer && owner._rejectUnauthorized && - RegExpPrototypeExec(/peer did not return a certificate/, - err.message) !== null) { + /peer did not return a certificate/.test(err.message)) { // Ignore server's authorization errors owner.destroy(); } else { @@ -1162,7 +1153,7 @@ function makeSocketMethodProxy(name) { }; } -ArrayPrototypeForEach([ +[ 'getCipher', 'getSharedSigalgs', 'getEphemeralKeyInfo', @@ -1173,7 +1164,7 @@ ArrayPrototypeForEach([ 'getTLSTicket', 'isSessionReused', 'enableTrace', -], (method) => { +].forEach((method) => { TLSSocket.prototype[method] = makeSocketMethodProxy(method); }); @@ -1470,10 +1461,10 @@ Server.prototype.setSecureContext = function(options) { if (options.sessionIdContext) { this.sessionIdContext = options.sessionIdContext; } else { - this.sessionIdContext = StringPrototypeSlice( - crypto.createHash('sha1') - .update(ArrayPrototypeJoin(process.argv, ' ')) - .digest('hex'), 0, 32); + this.sessionIdContext = crypto.createHash('sha1') + .update(process.argv.join(' ')) + .digest('hex') + .slice(0, 32); } if (options.sessionTimeout) @@ -1568,10 +1559,10 @@ Server.prototype.setOptions = deprecate(function(options) { if (options.sessionIdContext) { this.sessionIdContext = options.sessionIdContext; } else { - this.sessionIdContext = StringPrototypeSlice( - crypto.createHash('sha1') - .update(ArrayPrototypeJoin(process.argv, ' ')) - .digest('hex'), 0, 32); + this.sessionIdContext = crypto.createHash('sha1') + .update(process.argv.join(' ')) + .digest('hex') + .slice(0, 32); } if (options.pskCallback) this[kPskCallback] = options.pskCallback; if (options.pskIdentityHint) this[kPskIdentityHint] = options.pskIdentityHint; @@ -1588,14 +1579,15 @@ Server.prototype.addContext = function(servername, context) { throw new ERR_TLS_REQUIRED_SERVER_NAME(); } - const re = new RegExp('^' + StringPrototypeReplaceAll( - RegExpPrototypeSymbolReplace(/([.^$+?\-\\[\]{}])/g, servername, '\\$1'), - '*', '[^.]*', - ) + '$'); + const re = new RegExp(`^${ + servername + .replace(/([.^$+?\-\\[\]{}])/g, '\\$1') + .replaceAll('*', '[^.]*') + }$`); const secureContext = context instanceof common.SecureContext ? context : tls.createSecureContext(context); - ArrayPrototypePush(this._contexts, [re, secureContext.context]); + this._contexts.push([re, secureContext.context]); }; Server.prototype[EE.captureRejectionSymbol] = function( @@ -1616,7 +1608,7 @@ function SNICallback(servername, callback) { for (let i = contexts.length - 1; i >= 0; --i) { const elem = contexts[i]; - if (RegExpPrototypeExec(elem[0], servername) !== null) { + if (elem[0].test(servername)) { callback(null, elem[1]); return; } diff --git a/lib/eslint.config_partial.mjs b/lib/eslint.config_partial.mjs index 26ee6f5e1b8..a51fafafaf1 100644 --- a/lib/eslint.config_partial.mjs +++ b/lib/eslint.config_partial.mjs @@ -492,16 +492,18 @@ export default [ { files: [ 'lib/_http_*.js', + 'lib/_tls_*.js', 'lib/http.js', 'lib/http2.js', 'lib/internal/http.js', 'lib/internal/http2/*.js', + 'lib/tls.js', ], rules: { 'no-restricted-syntax': [ ...noRestrictedSyntax, { - selector: 'VariableDeclarator:has(.init[name="primordials"]) Identifier[name=/Prototype/]:not([name=/^(Object|Reflect)(Get|Set)PrototypeOf$/])', + selector: 'VariableDeclarator:has(.init[name="primordials"]) Identifier[name=/Prototype[A-Z]/]:not([name=/^(Object|Reflect)(Get|Set)PrototypeOf$/])', message: 'We do not use prototype primordials in this file', }, ], diff --git a/lib/tls.js b/lib/tls.js index b78f56d49a1..0c1b3350618 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -24,26 +24,10 @@ const { Array, ArrayIsArray, - ArrayPrototypeForEach, - ArrayPrototypeIncludes, - ArrayPrototypeJoin, - ArrayPrototypePush, - ArrayPrototypeReduce, - ArrayPrototypeSome, JSONParse, ObjectDefineProperty, ObjectFreeze, - RegExpPrototypeExec, - RegExpPrototypeSymbolReplace, StringFromCharCode, - StringPrototypeCharCodeAt, - StringPrototypeEndsWith, - StringPrototypeIncludes, - StringPrototypeIndexOf, - StringPrototypeSlice, - StringPrototypeSplit, - StringPrototypeStartsWith, - StringPrototypeSubstring, } = primordials; const { @@ -122,7 +106,7 @@ ObjectDefineProperty(exports, 'rootCertificates', { // ("\x06spdy/2\x08http/1.1\x08http/1.0") function convertProtocols(protocols) { const lens = new Array(protocols.length); - const buff = Buffer.allocUnsafe(ArrayPrototypeReduce(protocols, (p, c, i) => { + const buff = Buffer.allocUnsafe(protocols.reduce((p, c, i) => { const len = Buffer.byteLength(c); if (len > 255) { throw new ERR_OUT_OF_RANGE('The byte length of the protocol at index ' + @@ -158,20 +142,17 @@ exports.convertALPNProtocols = function convertALPNProtocols(protocols, out) { }; function unfqdn(host) { - return RegExpPrototypeSymbolReplace(/[.]$/, host, ''); + return host.replace(/[.]$/, ''); } // String#toLowerCase() is locale-sensitive so we use // a conservative version that only lowercases A-Z. function toLowerCase(c) { - return StringFromCharCode(32 + StringPrototypeCharCodeAt(c, 0)); + return StringFromCharCode(32 + c.charCodeAt(0)); } function splitHost(host) { - return StringPrototypeSplit( - RegExpPrototypeSymbolReplace(/[A-Z]/g, unfqdn(host), toLowerCase), - '.', - ); + return unfqdn(host).replace(/[A-Z]/g, toLowerCase).split('.'); } function check(hostParts, pattern, wildcards) { @@ -185,15 +166,15 @@ function check(hostParts, pattern, wildcards) { return false; // Pattern has empty components, e.g. "bad..example.com". - if (ArrayPrototypeIncludes(patternParts, '')) + if (patternParts.includes('')) return false; // RFC 6125 allows IDNA U-labels (Unicode) in names but we have no // good way to detect their encoding or normalize them so we simply // reject them. Control characters and blanks are rejected as well // because nothing good can come from accepting them. - const isBad = (s) => RegExpPrototypeExec(/[^\u0021-\u007F]/u, s) !== null; - if (ArrayPrototypeSome(patternParts, isBad)) + const isBad = (s) => /[^\u0021-\u007F]/u.test(s); + if (patternParts.some(isBad)) return false; // Check host parts from right to left first. @@ -204,13 +185,13 @@ function check(hostParts, pattern, wildcards) { const hostSubdomain = hostParts[0]; const patternSubdomain = patternParts[0]; - const patternSubdomainParts = StringPrototypeSplit(patternSubdomain, '*'); + const patternSubdomainParts = patternSubdomain.split('*'); // Short-circuit when the subdomain does not contain a wildcard. // RFC 6125 does not allow wildcard substitution for components // containing IDNA A-labels (Punycode) so match those verbatim. if (patternSubdomainParts.length === 1 || - StringPrototypeIncludes(patternSubdomain, 'xn--')) + patternSubdomain.includes('xn--')) return hostSubdomain === patternSubdomain; if (!wildcards) @@ -229,10 +210,10 @@ function check(hostParts, pattern, wildcards) { if (prefix.length + suffix.length > hostSubdomain.length) return false; - if (!StringPrototypeStartsWith(hostSubdomain, prefix)) + if (!hostSubdomain.startsWith(prefix)) return false; - if (!StringPrototypeEndsWith(hostSubdomain, suffix)) + if (!hostSubdomain.endsWith(suffix)) return false; return true; @@ -250,13 +231,12 @@ function splitEscapedAltNames(altNames) { let currentToken = ''; let offset = 0; while (offset !== altNames.length) { - const nextSep = StringPrototypeIndexOf(altNames, ', ', offset); - const nextQuote = StringPrototypeIndexOf(altNames, '"', offset); + const nextSep = altNames.indexOf(',', offset); + const nextQuote = altNames.indexOf('"', offset); if (nextQuote !== -1 && (nextSep === -1 || nextQuote < nextSep)) { // There is a quote character and there is no separator before the quote. - currentToken += StringPrototypeSubstring(altNames, offset, nextQuote); - const match = RegExpPrototypeExec( - jsonStringPattern, StringPrototypeSubstring(altNames, nextQuote)); + currentToken += altNames.substring(offset, nextQuote); + const match = jsonStringPattern.exec(altNames.substring(nextQuote)); if (!match) { throw new ERR_TLS_CERT_ALTNAME_FORMAT(); } @@ -264,16 +244,16 @@ function splitEscapedAltNames(altNames) { offset = nextQuote + match[0].length; } else if (nextSep !== -1) { // There is a separator and no quote before it. - currentToken += StringPrototypeSubstring(altNames, offset, nextSep); - ArrayPrototypePush(result, currentToken); + currentToken += altNames.substring(offset, nextSep); + result.push(currentToken); currentToken = ''; offset = nextSep + 2; } else { - currentToken += StringPrototypeSubstring(altNames, offset); + currentToken += altNames.substring(offset); offset = altNames.length; } } - ArrayPrototypePush(result, currentToken); + result.push(currentToken); return result; } @@ -286,14 +266,14 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) { hostname = '' + hostname; if (altNames) { - const splitAltNames = StringPrototypeIncludes(altNames, '"') ? + const splitAltNames = altNames.includes('"') ? splitEscapedAltNames(altNames) : - StringPrototypeSplit(altNames, ', '); - ArrayPrototypeForEach(splitAltNames, (name) => { - if (StringPrototypeStartsWith(name, 'DNS:')) { - ArrayPrototypePush(dnsNames, StringPrototypeSlice(name, 4)); - } else if (StringPrototypeStartsWith(name, 'IP Address:')) { - ArrayPrototypePush(ips, canonicalizeIP(StringPrototypeSlice(name, 11))); + altNames.split(', '); + splitAltNames.forEach((name) => { + if (name.startsWith('DNS:')) { + dnsNames.push(name.slice(4)); + } else if (name.startsWith('IP Address:')) { + ips.push(canonicalizeIP(name.slice(11))); } }); } @@ -304,16 +284,15 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) { hostname = unfqdn(hostname); // Remove trailing dot for error messages. if (net.isIP(hostname)) { - valid = ArrayPrototypeIncludes(ips, canonicalizeIP(hostname)); + valid = ips.includes(canonicalizeIP(hostname)); if (!valid) - reason = `IP: ${hostname} is not in the cert's list: ` + - ArrayPrototypeJoin(ips, ', '); + reason = `IP: ${hostname} is not in the cert's list: ` + ips.join(', '); } else if (dnsNames.length > 0 || subject?.CN) { const hostParts = splitHost(hostname); const wildcard = (pattern) => check(hostParts, pattern, true); if (dnsNames.length > 0) { - valid = ArrayPrototypeSome(dnsNames, wildcard); + valid = dnsNames.some(wildcard); if (!valid) reason = `Host: ${hostname}. is not in the cert's altnames: ${altNames}`; @@ -322,7 +301,7 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) { const cn = subject.CN; if (ArrayIsArray(cn)) - valid = ArrayPrototypeSome(cn, wildcard); + valid = cn.some(wildcard); else if (cn) valid = wildcard(cn);