Securely generate rcon password

Replace Math.random() based password generation for the rcon interface
with a more secure method based crypto.randomBytes.

CHANGELOG.md

@@ -9,6 +9,7 @@ Version 2.0.0
 - Added error handling during master startup.
 - Factorio game and rcon port now defaults to a random port above 49151.
 - Removed unimplemented mods update command.
+- Fixed rcon password being generated with Math.random().
 
 ### Breaking Changes
 

client.js

@@ -13,6 +13,8 @@ const getMac = require('getmac').getMac;
 const rmdirSync = require('rmdir-sync');
 const ioClient = require("socket.io-client");
 const asTable = require("as-table").configure({delimiter: ' | '});
+const util = require("util");
+const crypto = require("crypto");
 
 // internal libraries
 const objectOps = require("lib/objectOps.js");
@@ -105,6 +107,42 @@ function randomDynamicPort() {
 	return Math.floor(Math.random() * (end - start) + start)
 }
 
+/**
+ * Generate a secure random password of the given length
+ *
+ * Uses crypto.randomBytes to generate a secure alphanumeric password of
+ * the given length.
+ *
+ * @param {number} length - the length of the password to generate.
+ * @return {string} password of the given length
+ */
+async function generatePassword(length) {
+	function validChar(byte) {
+		const ranges = ['az', 'AZ', '09'];
+		return ranges.some(range =>
+			range.codePointAt(0) <= byte && byte <= range.codePointAt(1)
+		);
+	}
+	let randomBytesAsync = util.promisify(crypto.randomBytes);
+
+	let password = "";
+	while (true) {
+		let bytes = await randomBytesAsync((length - password.length) * 3);
+		for (let byte of bytes) {
+
+			// Crop to ASCII values only
+			byte = byte & 0x7f;
+
+			if (validChar(byte)) {
+				password += String.fromCharCode(byte);
+				if (password.length == length) {
+					return password;
+				}
+			}
+		}
+	}
+}
+
 function printUsage() {
 	console.error("Usage: ");
 	console.error("node client.js start [instance name]");
@@ -280,7 +318,7 @@ write-data=${ path.resolve(config.instanceDirectory, instance) }\r\n
 		"factorioPort": args.port || process.env.FACTORIOPORT || randomDynamicPort(),
 		"clientPort": args["rcon-port"] || process.env.RCONPORT || randomDynamicPort(),
 		"__comment_clientPassword": "This is the rcon password. Its also used for making an instanceID. Make sure its unique and not blank.",
-		"clientPassword": args["rcon-password"] || Math.random().toString(36).replace(/[^a-z]+/g, '').substr(0, 8),
+		"clientPassword": args["rcon-password"] || await generatePassword(10),
 		"info": {}
 	}
 	console.log("Clusterio | Created instance with settings:")
@@ -816,9 +854,9 @@ function hashMods(instanceName, callback) {
 }
 
 module.exports = {
-
 	// For testing only
 	_randomDynamicPort: randomDynamicPort,
+	_generatePassword: generatePassword,
 };
 
 if (module === require.main) {

client.spec.js

@@ -20,4 +20,21 @@ describe("Client testing", function() {
 			}
 		});
 	});
+
+	describe("generatePassword()", function() {
+		it("should return a string", async function() {
+			let password = await client._generatePassword(1);
+			assert.equal(typeof password, "string");
+		});
+
+		it("should return a string of the given length", async function() {
+			let password = await client._generatePassword(10);
+			assert.equal(password.length, 10);
+		});
+
+		it("should contain only a-z, A-Z, 0-9", async function() {
+			let password = await client._generatePassword(10);
+			assert(/^[a-zA-Z0-9]+$/.test(password), `${password} failed test`);
+		});
+	});
 });