Join blacklist to prevent unintended hosts from joining cluster

config.js

@@ -19,14 +19,16 @@
 // THE SOFTWARE.
 'use strict';
 
+var _ = require('underscore');
 var EventEmitter = require('events').EventEmitter;
 var util = require('util');
 
 // This Config class is meant to be a central store
 // for configurable parameters in Ringpop. Parameters
 // are meant to be initialized in the constructor.
-function Config(seedConfig) {
+function Config(ringpop, seedConfig) {
     seedConfig = seedConfig || {};
+    this.ringpop = ringpop;
     this.store = {};
     this._seed(seedConfig);
 }
@@ -38,6 +40,7 @@ Config.prototype.get = function get(key) {
 };
 
 Config.prototype.set = function set(key, value) {
+    // TODO Use same validation in _seed() here.
     var oldValue = this.store[key];
     this.store[key] = value;
     this.emit('set', key, value, oldValue);
@@ -61,11 +64,27 @@ Config.prototype._seed = function _seed(seed) {
     seedOrDefault('dampScoringReuseLimit', 2500);
     seedOrDefault('dampScoringSuppressDuration', 60 * 60 * 1000); // 1 hr in ms
     seedOrDefault('dampScoringSuppressLimit', 5000);
+    seedOrDefault('joinBlacklist', [], function validator(vals) {
+        return _.all(vals, function all(val) {
+            return val instanceof RegExp;
+        });
+    }, 'expected to be array of RegExp objects');
 
-    function seedOrDefault(name, defaultVal) {
+    function seedOrDefault(name, defaultVal, validator, reason) {
         var seedVal = seed[name];
         if (typeof seedVal === 'undefined') {
             self.set(name, defaultVal);
+        } else if (typeof validator === 'function' && !validator(seedVal)) {
+            if (self.ringpop) {
+                self.ringpop.logger.warn('ringpop using default value for config after' +
+                        ' being passed invalid seed value', {
+                    config: name,
+                    seedVal: seedVal,
+                    defaultVal: defaultVal,
+                    reason: reason
+                });
+            }
+            self.set(name, defaultVal);
         } else {
             self.set(name, seedVal);
         }

index.js

@@ -121,7 +121,7 @@ function RingPop(options) {
 
     // Initialize Config before all other gossip, membership, forwarding,
     // and hash ring dependencies.
-    this.config = new Config(options);
+    this.config = new Config(this, options);
 
     this.requestProxy = new RequestProxy({
         ringpop: this,
@@ -249,8 +249,8 @@ RingPop.prototype.bootstrap = function bootstrap(opts, callback) {
         return;
     }
 
-    this.checkForMissingBootstrapHost();
-    this.checkForHostnameIpMismatch();
+    checkForMissingBootstrapHost();
+    checkForHostnameIpMismatch();
 
     // Add local member to membership.
     this.membership.makeAlive(this.whoami(), Date.now());
@@ -310,60 +310,58 @@ RingPop.prototype.bootstrap = function bootstrap(opts, callback) {
 
         if (callback) callback(null, nodesJoined);
     });
-};
 
-RingPop.prototype.checkForMissingBootstrapHost = function checkForMissingBootstrapHost() {
-    if (this.bootstrapHosts.indexOf(this.hostPort) === -1) {
-        this.logger.warn('bootstrap hosts does not include the host/port of' +
-            ' the local node. this may be fine because your hosts file may' +
-            ' just be slightly out of date, but it may also be an indication' +
-            ' that your node is identifying itself incorrectly.', {
-            address: this.hostPort
-        });
+    function checkForMissingBootstrapHost() {
+        if (self.bootstrapHosts.indexOf(self.hostPort) === -1) {
+            self.logger.warn('bootstrap hosts does not include the host/port of' +
+                ' the local node. this may be fine because your hosts file may' +
+                ' just be slightly out of date, but it may also be an indication' +
+                ' that your node is identifying itself incorrectly.', {
+                address: self.hostPort
+            });
 
-        return false;
-    }
+            return false;
+        }
 
-    return true;
-};
+        return true;
+    }
 
-RingPop.prototype.checkForHostnameIpMismatch = function checkForHostnameIpMismatch() {
-    var self = this;
+    function checkForHostnameIpMismatch() {
+        function testMismatch(msg, filter) {
+            var filteredHosts = self.bootstrapHosts.filter(filter);
 
-    function testMismatch(msg, filter) {
-        var filteredHosts = self.bootstrapHosts.filter(filter);
+            if (filteredHosts.length > 0) {
+                self.logger.warn(msg, {
+                    address: self.hostPort,
+                    mismatchedBootstrapHosts: filteredHosts
+                });
 
-        if (filteredHosts.length > 0) {
-            self.logger.warn(msg, {
-                address: self.hostPort,
-                mismatchedBootstrapHosts: filteredHosts
-            });
+                return false;
+            }
 
-            return false;
+            return true;
         }
 
-        return true;
-    }
+        if (HOST_PORT_PATTERN.test(self.hostPort)) {
+            var ipMsg = 'your ringpop host identifier looks like an IP address and there are' +
+                ' bootstrap hosts that appear to be specified with hostnames. these inconsistencies' +
+                ' may lead to subtle node communication issues';
 
-    if (HOST_PORT_PATTERN.test(this.hostPort)) {
-        var ipMsg = 'your ringpop host identifier looks like an IP address and there are' +
-            ' bootstrap hosts that appear to be specified with hostnames. these inconsistencies' +
-            ' may lead to subtle node communication issues';
+            return testMismatch(ipMsg, function(host) {
+                return !HOST_PORT_PATTERN.test(host);
+            });
+        } else {
+            var hostMsg = 'your ringpop host identifier looks like a hostname and there are' +
+                ' bootstrap hosts that appear to be specified with IP addresses. these inconsistencies' +
+                ' may lead to subtle node communication issues';
 
-        return testMismatch(ipMsg, function(host) {
-            return !HOST_PORT_PATTERN.test(host);
-        });
-    } else {
-        var hostMsg = 'your ringpop host identifier looks like a hostname and there are' +
-            ' bootstrap hosts that appear to be specified with IP addresses. these inconsistencies' +
-            ' may lead to subtle node communication issues';
+            return testMismatch(hostMsg, function(host) {
+                return HOST_PORT_PATTERN.test(host);
+            });
+        }
 
-        return testMismatch(hostMsg, function(host) {
-            return HOST_PORT_PATTERN.test(host);
-        });
+        return true;
     }
-
-    return true;
 };
 
 RingPop.prototype.clearDebugFlags = function clearDebugFlags() {

server/protocol/join.js

@@ -19,6 +19,7 @@
 // THE SOFTWARE.
 'use strict';
 
+var _ = require('underscore');
 var safeParse = require('../../lib/util').safeParse;
 var TypedError = require('error/typed');
 
@@ -35,6 +36,14 @@ var InvalidJoinAppError = TypedError({
     actual: null
 });
 
+var InvalidJoinBlacklistError = TypedError({
+    type: 'ringpop.invalid-join.blacklist',
+    message: '{joiner} tried joining a cluster, but its host is part of the' +
+        ' blacklist: {blacklist}',
+    blacklist: null,
+    joiner: null
+});
+
 var InvalidJoinSourceError = TypedError({
     type: 'ringpop.invalid-join.source',
     message:  'A node tried joining a cluster by attempting to join itself.' +
@@ -59,7 +68,22 @@ function validateJoinerAddress(ringpop, joiner, callback) {
         return false;
     }
 
+    var blacklist = ringpop.config.get('joinBlacklist');
+    if (Array.isArray(blacklist) && anyBlacklisted()) {
+        callback(InvalidJoinBlacklistError({
+            joiner: joiner,
+            blacklist: blacklist
+        }));
+        return false;
+    }
+
     return true;
+
+    function anyBlacklisted() {
+        return _.any(blacklist, function any(pattern) {
+            return pattern.test(joiner);
+        });
+    }
 }
 
 function validateJoinerApp(ringpop, app, callback) {

test/unit/config_test.js

@@ -59,7 +59,7 @@ test('seeds known config', function t(assert) {
     seed[knownKey] = knownVal;
     seed[unknownKey] = unknownVal;
 
-    var config = new Config(seed);
+    var config = new Config(null, seed);
 
     assert.equals(knownVal, config.get(knownKey), 'known config is seeded');
     assert.equals(undefined, config.get(unknownKey),
@@ -74,3 +74,19 @@ test('no seed is OK', function t(assert) {
     });
     assert.end();
 });
+
+test('validates joinBlacklist seed', function t(assert) {
+    var config = new Config(null, {
+        'joinBlacklist': ['127.0.0.1:3000']
+    });
+    assert.deepEquals([], config.get('joinBlacklist'),
+        'uses default blacklist');
+
+    var regexList = [/127.0.0.1:*/];
+    config = new Config(null, {
+        'joinBlacklist': regexList
+    });
+    assert.deepEquals(regexList, config.get('joinBlacklist'),
+        'uses seed blacklist');
+    assert.end();
+});

test/unit/index.js

@@ -27,3 +27,4 @@
 require('./config_test.js');
 require('./member_test.js');
 require('./membership_test.js');
+require('./server/protocol/join_test.js');

test/unit/server/protocol/join_test.js

@@ -0,0 +1,44 @@
+// Copyright (c) 2015 Uber Technologies, Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+'use strict';
+
+var createProtocolJoinHandler = require('../../../../server/protocol/join.js');
+var Ringpop = require('../../../../index.js');
+var test = require('tape');
+
+test('join fails with blacklist error', function t(assert) {
+    var ringpop = new Ringpop({
+        app: 'ringpop',
+        hostPort: '127.0.0.1:3000',
+        joinBlacklist: [/127.0.0.1:*/]
+    });
+    var handleProtocolJoin = createProtocolJoinHandler(ringpop);
+    handleProtocolJoin(null, JSON.stringify({
+        app: 'ringpop',
+        source: '127.0.0.1:3001',
+        incarnationNumber: 1
+    }), null, function onHandled(err) {
+        assert.ok(err, 'an error occurred');
+        assert.equals(err.type, 'ringpop.invalid-join.blacklist',
+            'blacklist error occurred');
+        assert.end();
+        ringpop.destroy();
+    });
+});