From 85daf6df1932f73ded45184b5967682c2926c111 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Sat, 14 Sep 2019 00:53:50 +0200 Subject: [PATCH 1/6] Rename listPlugins to listInstances Rename incorrectly named function listPlugins to listInstances. --- client.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client.js b/client.js index cf62972d..9bcf5143 100644 --- a/client.js +++ b/client.js @@ -102,7 +102,7 @@ function printUsage() { console.error("node client.js manage"); } -async function listPlugins() { +async function listInstances() { let instanceNames = fileOps.getDirectoriesSync(config.instanceDirectory); instanceNames.unshift("Name:"); let longestInstanceName = 0; @@ -571,7 +571,7 @@ async function startClient() { printUsage(); process.exit(1); } else if (command == "list") { - await listPlugins(); + await listInstances(); process.exit(0); } else if (command == "manage"){ await manage(); From 18a904d9de77dd7accd6b93ea92ccb5750beb8e0 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Sat, 14 Sep 2019 00:54:32 +0200 Subject: [PATCH 2/6] Use as-table for listInstances Rewrite listInstances to use as-table instead of manually coded table generation. --- client.js | 51 +++++++++++---------------------------------------- 1 file changed, 11 insertions(+), 40 deletions(-) diff --git a/client.js b/client.js index 9bcf5143..53d22417 100644 --- a/client.js +++ b/client.js @@ -12,6 +12,7 @@ const deepmerge = require("deepmerge"); const getMac = require('getmac').getMac; const rmdirSync = require('rmdir-sync'); const ioClient = require("socket.io-client"); +const asTable = require("as-table").configure({delimiter: ' | '}); // internal libraries const objectOps = require("lib/objectOps.js"); @@ -104,47 +105,17 @@ function printUsage() { async function listInstances() { let instanceNames = fileOps.getDirectoriesSync(config.instanceDirectory); - instanceNames.unshift("Name:"); - let longestInstanceName = 0; - // determine longest instance name - instanceNames.forEach(function(instance){ - if(instance.length > longestInstanceName) longestInstanceName = instance.length; - }); - let displayLines = []; - // push name coloumn to array - instanceNames.forEach(function(instance){ - while(instance.length < longestInstanceName+1){ - instance += " "; - } - displayLines.push("| "+ instance + "|"); - }); - // create port colloumn - let factorioPorts = []; - instanceNames.forEach(function(instance){ - let factorioPort; - - if(instance.includes("Name:")){ - factorioPort = "Port:" - } else { - factorioPort = require(path.resolve(config.instanceDirectory, instance, 'config')).factorioPort; - } - factorioPorts.push(factorioPort); - }); - factorioPorts.forEach((port, index) => { - let longestPort = 0; - factorioPorts.forEach((port, index) => { - if(port.toString().length > longestPort) longestPort = port.toString().length; + let instances = []; + for (instance of instanceNames) { + let cfg = path.resolve(config.instanceDirectory, instance, 'config'); + let port = require(cfg).factorioPort; + instances.push({ + "Name": instance, + "Port": port.toString(), }); - while(port.toString().length < longestPort){ - port += " "; - } - factorioPorts[index] = port; - }); - instanceNames.forEach(function(instance, index){ - displayLines[index] += " " + factorioPorts[index] + " |"; - }); - - displayLines.forEach(line => console.log(line)); + } + + console.log(asTable(instances)); } async function manage() { From 500c9584eb9dc12c7ee85393f2d26b5bde7534ff Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Sun, 1 Sep 2019 16:06:43 +0200 Subject: [PATCH 3/6] Remove commented out itemStats code Remove old commented out code dealing with item statistics that is no longer relevant. --- master.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/master.js b/master.js index 8fab7728..17842674 100644 --- a/master.js +++ b/master.js @@ -411,11 +411,7 @@ app.post("/api/rconPasswords", authenticate.middleware, function(req, res) { } res.send(slaveCache.cache); }); -/* -var recievedItemStatistics = new averagedTimeSeries({ - maxEntries: config.itemStats.maxEntries, - entriesPerSecond: config.itemStats.entriesPerSecond, -}, console.log);*/ + var recievedItemStatisticsBySlaveID = {}; var sentItemStatisticsBySlaveID = {}; // From 2428cba2bdcdc4684f6424e39dd065f8eefde47f Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Wed, 11 Sep 2019 20:17:58 +0200 Subject: [PATCH 4/6] Default to ports in the dynamic range Use random ports in the dynamic range as define by RFC 6335 for instaces. --- .travis.yml | 2 ++ CHANGELOG.md | 1 + client.js | 24 ++++++++++++++++++++++-- client.spec.js | 23 +++++++++++++++++++++++ 4 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 client.spec.js diff --git a/.travis.yml b/.travis.yml index 31d07777..f7059a13 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,6 +5,8 @@ before_install: - curl -o factorio.tar.gz -L https://www.factorio.com/get-download/latest/headless/linux64 - tar -xf factorio.tar.gz - npm install -g codecov + # Needed for lib/manager/configManager + - echo {} > config.json script: - npm run-script ci-cover - codecov -f coverage/lcov.info diff --git a/CHANGELOG.md b/CHANGELOG.md index a0dfdd58..64085b0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Version 2.0.0 - Fixed sslCert and sslPrivKey entries being ignored in the config. - Changed ssl key creation to be done at startup instead of on npm install. - Added error handling during master startup. +- Factorio game and rcon port now defaults to a random port above 49151. ### Breaking Changes diff --git a/client.js b/client.js index 53d22417..d237a28c 100644 --- a/client.js +++ b/client.js @@ -90,6 +90,21 @@ function messageInterface(command, callback) { }); } +/** + * Give a random dynamic port + * + * Returns a random port number in the Dynamic Ports range as defined by + * RFC 6335. + * + * @return {number} a number in the range 49152 to 65535. + */ +function randomDynamicPort() { + const start = 49152; + const end = 65535 + 1; + + return Math.floor(Math.random() * (end - start) + start) +} + function printUsage() { console.error("Usage: "); console.error("node client.js start [instance name]"); @@ -264,8 +279,8 @@ write-data=${ path.resolve(config.instanceDirectory, instance) }\r\n // this line is probably not needed anymore but Im not gonna remove it fs.copySync('sharedMods', path.join(instancedirectory, "mods")); let instconf = { - "factorioPort": args.port || process.env.FACTORIOPORT || Math.floor(Math.random() * 65535), - "clientPort": args["rcon-port"] || process.env.RCONPORT || Math.floor(Math.random() * 65535), + "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), "info": {} @@ -802,6 +817,11 @@ function hashMods(instanceName, callback) { }); } +module.exports = { + + // For testing only + _randomDynamicPort: randomDynamicPort, +}; if (module === require.main) { console.warn(` diff --git a/client.spec.js b/client.spec.js new file mode 100644 index 00000000..5fd9e9ff --- /dev/null +++ b/client.spec.js @@ -0,0 +1,23 @@ +const assert = require('assert').strict; + +const client = require("./client"); + +describe("Client testing", function() { + describe("randomDynamicPort()", function() { + it("should return a port number", function() { + let port = client._randomDynamicPort() + assert.equal(typeof port, "number"); + assert(Number.isInteger(port)); + assert(0 <= port && port < 2**16); + }); + + it("should return a port number in the dynamic range", function() { + function validate(port) { + return 49152 <= port && port <= 65535; + } + for (let i=0; i < 20; i++) { + assert(validate(client._randomDynamicPort())); + } + }); + }); +}); From eb1b863cec4f90d1769b17610c7312fd52b63dd0 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Thu, 12 Sep 2019 16:17:56 +0200 Subject: [PATCH 5/6] Remove unimplemented mods update command Remove the mods update command from the client command line interface as this calls the non-existant modManager.updateAllMods function. --- CHANGELOG.md | 1 + client.js | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64085b0a..1eff9067 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Version 2.0.0 - Changed ssl key creation to be done at startup instead of on npm install. - Added error handling during master startup. - Factorio game and rcon port now defaults to a random port above 49151. +- Removed unimplemented mods update command. ### Breaking Changes diff --git a/client.js b/client.js index d237a28c..c16b3b9b 100644 --- a/client.js +++ b/client.js @@ -138,7 +138,7 @@ async function manage() { //const fullUsage = 'node client.js manage [instance, "shared"] ["mods", "config"] ...'; function usage(instance, tool, action){ if(tool && tool == "mods"){ - console.log('node client.js manage '+instance+' '+tool+' ["list", "search", "add", "remove", "update"]'); + console.log('node client.js manage '+instance+' '+tool+' ["list", "search", "add", "remove"]'); } else if(tool && tool == "config") { console.log('node client.js manage '+instance+' '+tool+' ["list", "edit"]'); } else if(tool && tool == "plugins") { @@ -164,8 +164,6 @@ async function manage() { await modManager.addMod(process.argv[6], instance); } else if(action == "remove" || action == "rm" || action == "delete"){ await modManager.removeMod(process.argv[6], instance); - } else if(action == "update"){ - await modManager.updateAllMods(); } else { usage(instance, tool); } From bcb22d2af740c4975b18a49a17683718abfd2329 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Wed, 11 Sep 2019 20:31:22 +0200 Subject: [PATCH 6/6] Securely generate rcon password Replace Math.random() based password generation for the rcon interface with a more secure method based crypto.randomBytes. --- CHANGELOG.md | 1 + client.js | 42 ++++++++++++++++++++++++++++++++++++++++-- client.spec.js | 17 +++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1eff9067..bc90041f 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/client.js b/client.js index c16b3b9b..5a9b5799 100644 --- a/client.js +++ b/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) { diff --git a/client.spec.js b/client.spec.js index 5fd9e9ff..f02d8a03 100644 --- a/client.spec.js +++ b/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`); + }); + }); });