perf(AUTO-1063): skip parsing hostnames and cache verdicts in isAddressRestricted

lib/requester/core.js

@@ -35,6 +35,27 @@ var dns = require('dns'),
 
     ERROR_ADDRESS_RESOLVE = 'NETERR: getaddrinfo ENOTFOUND ',
 
+    /**
+     * Upper bound on remembered isAddressRestricted verdicts per networkOptions object, so a
+     * run that fans out to many hosts (e.g. via redirects) cannot grow the cache without limit.
+     *
+     * @private
+     * @type {Number}
+     */
+    MAX_CACHED_ADDRESS_VERDICTS = 1000,
+
+    /**
+     * Matches any character that cannot appear in an IPv4 literal accepted by ipaddr.js
+     * (decimal, octal or 0x-hex parts joined by dots). A host with no ':' that matches it is
+     * not an IP address: ipaddr.js's IPv6 regex does not itself need a ':' ('deadbeef',
+     * '1%eth0' match it), but its part-count check rejects every colon-free IPv6 form. Unit
+     * tests assert this against ipaddr.js, so an upgrade that widens either set fails them.
+     *
+     * @private
+     * @type {RegExp}
+     */
+    NON_IPV4_LITERAL_CHAR = /[^0-9a-fx.]/,
+
     /**
      * List of request methods without body.
      *
@@ -209,6 +230,24 @@ var dns = require('dns'),
         return hostname.substring(hostname.lastIndexOf('.') + 1);
     },
 
+    /**
+     * Remembers an isAddressRestricted verdict for a host string, evicting the least recently used
+     * entry (the first key in Map insertion order) when the cache is at MAX_CACHED_ADDRESS_VERDICTS.
+     *
+     * @private
+     *
+     * @param {Map} verdicts
+     * @param {String} host - the lower-cased host string the verdict was computed for
+     * @param {Boolean} verdict
+     * @returns {Boolean} the verdict, so callers can return it directly
+     */
+    rememberAddressVerdict = function (verdicts, host, verdict) {
+        verdicts.size >= MAX_CACHED_ADDRESS_VERDICTS && verdicts.delete(verdicts.keys().next().value);
+        verdicts.set(host, verdict);
+
+        return verdict;
+    },
+
     /**
      * Abstracts out the logic for domain resolution
      *
@@ -845,6 +884,13 @@ module.exports = {
      * on the networkOptions object; restrictedAddresses is treated as immutable
      * after the first call — later mutations to CIDR entries are not picked up.
      *
+     * Verdicts are remembered per host string on networkOptions.restrictedVerdicts (bounded by
+     * MAX_CACHED_ADDRESS_VERDICTS). This needs a stronger contract than restrictedCidrs: a
+     * cache hit also skips the exact-match check, so restrictedAddresses must not change at
+     * all after the first call, exact-match entries included. The cache is keyed
+     * by the exact string checked, never by where a name resolves to, so an address a name
+     * later resolves to is still checked on its own.
+     *
      * @param {String} host
      * @param {Object} networkOptions
      * @param {Object} networkOptions.restrictedAddresses
@@ -853,11 +899,24 @@ module.exports = {
      */
     isAddressRestricted (host, networkOptions) {
         var restrictedAddresses = networkOptions.restrictedAddresses,
-            lowerHost, hasBrackets, strippedHost, compatEmbedded, addr, range;
+            lowerHost, hasBrackets, strippedHost, compatEmbedded, addr, range, verdicts, verdict;
 
         if (!(host && restrictedAddresses)) { return false; }
 
         lowerHost = host.toLowerCase();
+
+        verdicts = networkOptions.restrictedVerdicts || (networkOptions.restrictedVerdicts = new Map());
+
+        if (verdicts.has(lowerHost)) {
+            verdict = verdicts.get(lowerHost);
+
+            // re-insert so the entry becomes the most recently used
+            verdicts.delete(lowerHost);
+            verdicts.set(lowerHost, verdict);
+
+            return verdict;
+        }
+
         // strip [ ] so IPv6 literals from URI hostnames are parseable by ipaddr.js
         hasBrackets = lowerHost[0] === '[' && lowerHost[lowerHost.length - 1] === ']';
         strippedHost = hasBrackets ? lowerHost.slice(1, -1) : lowerHost;
@@ -867,7 +926,14 @@ module.exports = {
         if (restrictedAddresses[lowerHost] || restrictedAddresses[strippedHost] ||
             (!hasBrackets && _.includes(lowerHost, COLON) &&
                 restrictedAddresses['[' + lowerHost + ']'])) {
-            return true;
+            return rememberAddressVerdict(verdicts, lowerHost, true);
+        }
+
+        // a host with no ':' and a character outside [0-9a-fx.] cannot be an IPv6 literal or any
+        // IPv4 form ipaddr.js accepts, so it can never match a CIDR entry; skip the parse, which
+        // would only throw
+        if (!_.includes(strippedHost, COLON) && NON_IPV4_LITERAL_CHAR.test(strippedHost)) {
+            return rememberAddressVerdict(verdicts, lowerHost, false);
         }
 
         // lazy-init the parsed CIDR list on first call; cached for subsequent calls
@@ -886,7 +952,7 @@ module.exports = {
                 }, []);
         }
 
-        if (!networkOptions.restrictedCidrs.length) { return false; }
+        if (!networkOptions.restrictedCidrs.length) { return rememberAddressVerdict(verdicts, lowerHost, false); }
 
         // IPv4-compatible IPv6 (::a.b.c.d) is deprecated and ipaddr.js refuses to parse
         // it, so detect the shape and match on the embedded IPv4 directly. Colon-only
@@ -923,13 +989,13 @@ module.exports = {
                 }
             }
 
-            return networkOptions.restrictedCidrs.some(function (cidr) {
+            return rememberAddressVerdict(verdicts, lowerHost, networkOptions.restrictedCidrs.some(function (cidr) {
                 try { return addr.match(cidr); }
                 catch (e) { return false; } // family mismatch — skip to next CIDR
-            });
+            }));
         }
         catch (e) { /* host is not a parseable IP address */ }
 
-        return false;
+        return rememberAddressVerdict(verdicts, lowerHost, false);
     }
 };

test/unit/requester-core.test.js

@@ -1444,6 +1444,217 @@ describe('requester util', function () {
                 expect(opts.restrictedCidrs).to.be.an('array').with.lengthOf(0);
             });
         });
+
+        describe('hostname pre-check and verdict cache', function () {
+            var ipaddr = require('ipaddr.js'),
+                sinon = require('sinon'),
+                POLICY = {
+                    localhost: true,
+                    127.1: true,
+                    '::1': true,
+                    '[::1]': true,
+                    '127.0.0.0/8': true,
+                    '0.0.0.0/8': true,
+                    '::1/128': true,
+                    'fe80::/10': true,
+                    '169.254.0.0/16': true,
+                    '10.0.0.0/8': true,
+                    '172.16.0.0/12': true,
+                    '192.168.0.0/16': true,
+                    'fc00::/7': true
+                },
+                // verdicts recorded from the implementation before the pre-check and cache were added
+                BLOCKED = [
+                    'localhost', 'LOCALHOST', '127.0.0.1', '127.1', '0x7f.0.0.1', '0177.0.0.1', '2130706433',
+                    '0x7f000001', '10.1.2.3', '192.168.1.5', '169.254.169.254', '::1', '[::1]',
+                    '::ffff:127.0.0.1', '[::ffff:10.0.0.1]', '::10.0.0.1', '64:ff9b::a9fe:a9fe',
+                    '2002:a9fe:a9fe::', 'fe80::1', 'fe80::1%eth0', 'fc00::1'
+                ],
+                ALLOWED = [
+                    'example.com', 'EXAMPLE.COM', 'example.com.', 'localhost.example.com', 'postman-echo.com',
+                    'k8s-gotunabl-4726ea8024.elb.us-east-1.amazonaws.com', '93.184.216.34', '8.8.8.8',
+                    '2001:4860:4860::8888', 'cafe.babe', 'deadbeef', 'abc', 'a.b.c.d', '127.0.0.1.', 'x'
+                ],
+                // hostnames that contain characters no IP literal can have ('x', 'cafe.babe' and
+                // 'deadbeef' only use IP-literal characters, so they stay on the full path)
+                NAMES = ['example.com', 'EXAMPLE.COM', 'example.com.', 'localhost.example.com', 'postman-echo.com',
+                    'k8s-gotunabl-4726ea8024.elb.us-east-1.amazonaws.com'],
+                processSpy;
+
+            function freshOpts () {
+                return { restrictedAddresses: { ...POLICY } };
+            }
+
+            beforeEach(function () {
+                processSpy = sinon.spy(ipaddr, 'process');
+            });
+
+            afterEach(function () {
+                processSpy.restore();
+            });
+
+            it('should keep every blocked verdict unchanged', function () {
+                BLOCKED.forEach(function (host) {
+                    expect(requesterCore.isAddressRestricted(host, freshOpts()), host).to.be.true;
+                });
+            });
+
+            it('should keep every allowed verdict unchanged', function () {
+                ALLOWED.forEach(function (host) {
+                    expect(requesterCore.isAddressRestricted(host, freshOpts()), host).to.be.false;
+                });
+            });
+
+            it('should give the same verdicts on a repeat call with the same networkOptions', function () {
+                var opts = freshOpts();
+
+                BLOCKED.concat(ALLOWED).forEach(function (host) {
+                    var first = requesterCore.isAddressRestricted(host, opts);
+
+                    expect(requesterCore.isAddressRestricted(host, opts), host).to.equal(first);
+                });
+            });
+
+            it('should not try to parse a hostname as an IP address', function () {
+                NAMES.forEach(function (host) {
+                    expect(requesterCore.isAddressRestricted(host, freshOpts()), host).to.be.false;
+                });
+                expect(processSpy.callCount).to.equal(0);
+            });
+
+            it('should still check a listed name before the pre-check', function () {
+                expect(requesterCore.isAddressRestricted('localhost', freshOpts())).to.be.true;
+                expect(requesterCore.isAddressRestricted('LocalHost', freshOpts())).to.be.true;
+            });
+
+            it('should still parse anything shaped like an IP address', function () {
+                // none of these are exact entries in POLICY, so each must reach the parser
+                ['127.0.0.1', '0x7f.0.0.1', '0177.0.0.1', '2130706433', '10.1', '::2', 'fe80::1%eth0',
+                    '93.184.216.34', 'cafe.babe', 'deadbeef', '127.0.0.1.'].forEach(function (host) {
+                    processSpy.resetHistory();
+                    requesterCore.isAddressRestricted(host, freshOpts());
+                    expect(processSpy.callCount, host).to.be.at.least(1);
+                });
+            });
+
+            // the pre-check is only safe while every colon-free string ipaddr.js accepts stays inside
+            // [0-9a-fx.]; this fails if an ipaddr.js upgrade widens that set (e.g. '0o' octal parts)
+            it('should send every colon-free string ipaddr.js accepts to the parser', function () {
+                var seed = 1571,
+                    PART_PREFIXES = ['', '', '0', '0x', '0o', '0b', '0d', '+', '-', '00x', '0x0'],
+                    PART_CHARS = '0123456789abcdefxobdghnz_%-+ ',
+                    JUNK = ['', '', '', '.', '%eth0', '%0', '/8', ' ', '..', 'e1', 'n'],
+                    checked = 0,
+                    host,
+                    i;
+
+                // seeded so a failure reproduces
+                function rand (n) {
+                    seed = (Math.imul(seed, 1103515245) + 12345) >>> 0;
+
+                    return (seed >>> 16) % n;
+                }
+
+                function randomHost () {
+                    var parts = [],
+                        count = 1 + rand(4),
+                        part,
+                        j,
+                        k;
+
+                    for (j = 0; j < count; j++) {
+                        part = PART_PREFIXES[rand(PART_PREFIXES.length)];
+                        for (k = 1 + rand(4); k > 0; k--) {
+                            part += rand(3) ? String(rand(10)) : PART_CHARS[rand(PART_CHARS.length)];
+                        }
+                        parts.push(part);
+                    }
+
+                    return parts.join('.') + JUNK[rand(JUNK.length)];
+                }
+
+                for (i = 0; i < 50000; i++) {
+                    host = randomHost().toLowerCase();
+
+                    if (ipaddr.isValid(host) && !host.includes(':')) {
+                        checked++;
+                        processSpy.resetHistory();
+                        requesterCore.isAddressRestricted(host, { restrictedAddresses: { '169.254.0.0/16': true } });
+                        expect(processSpy.callCount, host).to.be.at.least(1);
+                    }
+                }
+
+                // guard against a generator that stops producing valid addresses
+                expect(checked).to.be.above(1000);
+            });
+
+            it('should not parse the same address twice for the same networkOptions', function () {
+                var opts = freshOpts();
+
+                expect(requesterCore.isAddressRestricted('93.184.216.34', opts)).to.be.false;
+                expect(requesterCore.isAddressRestricted('169.254.169.254', opts)).to.be.true;
+                processSpy.resetHistory();
+
+                expect(requesterCore.isAddressRestricted('93.184.216.34', opts)).to.be.false;
+                expect(requesterCore.isAddressRestricted('169.254.169.254', opts)).to.be.true;
+                expect(processSpy.callCount).to.equal(0);
+            });
+
+            it('should treat different spellings as separate entries', function () {
+                var opts = freshOpts();
+
+                expect(requesterCore.isAddressRestricted('example.com', opts)).to.be.false;
+                expect(requesterCore.isAddressRestricted('[::1]', opts)).to.be.true;
+                expect(requesterCore.isAddressRestricted('::1', opts)).to.be.true;
+                expect(requesterCore.isAddressRestricted('0x7f.0.0.1', opts)).to.be.true;
+            });
+
+            it('should still block an address a name later resolves to (DNS rebinding)', function () {
+                var opts = freshOpts();
+
+                // Check A on the name, Check B on the first resolved address
+                expect(requesterCore.isAddressRestricted('example.com', opts)).to.be.false;
+                expect(requesterCore.isAddressRestricted('93.184.216.34', opts)).to.be.false;
+
+                // DNS now answers with the metadata address: Check A on the name, Check B on the new address
+                expect(requesterCore.isAddressRestricted('example.com', opts)).to.be.false;
+                expect(requesterCore.isAddressRestricted('169.254.169.254', opts)).to.be.true;
+            });
+
+            it('should keep the cache bounded and correct after many distinct addresses', function () {
+                var opts = freshOpts(),
+                    i;
+
+                for (i = 0; i < 1500; i++) {
+                    expect(requesterCore.isAddressRestricted('8.8.' + Math.floor(i / 256) + '.' + (i % 256), opts))
+                        .to.be.false;
+                }
+
+                expect(opts.restrictedVerdicts.size).to.be.at.most(1000);
+                expect(requesterCore.isAddressRestricted('10.1.2.3', opts)).to.be.true;
+                expect(requesterCore.isAddressRestricted('93.184.216.34', opts)).to.be.false;
+            });
+
+            it('should evict the least recently used entry, keeping recently used ones', function () {
+                var opts = freshOpts(),
+                    i;
+
+                requesterCore.isAddressRestricted('93.184.216.34', opts);
+                requesterCore.isAddressRestricted('10.1.2.3', opts);
+
+                for (i = 0; i < 998; i++) {
+                    requesterCore.isAddressRestricted('8.8.' + Math.floor(i / 256) + '.' + (i % 256), opts);
+                }
+
+                // cache is full; touch the oldest entry, then overflow by one
+                expect(opts.restrictedVerdicts.size).to.equal(1000);
+                requesterCore.isAddressRestricted('93.184.216.34', opts);
+                requesterCore.isAddressRestricted('9.9.9.9', opts);
+
+                expect(opts.restrictedVerdicts.size).to.equal(1000);
+                expect(opts.restrictedVerdicts.has('93.184.216.34')).to.be.true;
+                expect(opts.restrictedVerdicts.has('10.1.2.3')).to.be.false;
+            });
+        });
     });
 });
-