diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2c723f8e6b..c523fa0cf4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -110,6 +110,26 @@ jobs: labels: ${{ steps.meta.outputs.labels }} + test_isolation: + name: "Test: tenant isolation" + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Log in to GitHub Container Registry + uses: docker/login-action@v3 + with: + registry: ${{ env.REGISTRY }} + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + # The image is only a runtime carrier here: it supplies node_modules (mongodb is not a + # package.json dependency), while the code under test is this checkout, mounted at /src. + - name: Run tenant isolation tests + run: test/run-tests.sh ${{ env.REGISTRY }}/${{ env.ORGANISATION }}/${{ env.REPOSITORY }}/meshcentral:latest + + test_helm: name: "Test Helm Chart" needs: [changes] @@ -135,7 +155,7 @@ jobs: all-checks: name: "All Checks" - needs: [changes, test, test_helm] + needs: [changes, test, test_isolation, test_helm] runs-on: ubuntu-latest if: always() steps: diff --git a/db.js b/db.js index 9bb61727ce..28331be29f 100644 --- a/db.js +++ b/db.js @@ -38,8 +38,23 @@ function deriveTenantDomain(domains) { } module.exports.deriveTenantDomain = deriveTenantDomain; +// This pod's tenant, or null when not running in OpenFrame mode (single-tenant / OSS installs, +// where none of the scoping below applies and behaviour must stay exactly as upstream). +// +// Resolved ONCE per database instance, not per call: the tenant a server belongs to cannot change +// while it runs, and reading process.env on every query made the scoping depend on when the query +// happened to run relative to the environment. Every caller below must see the same answer as the +// write guard, which binds itself at setup. +function makeOpenframeDomain(parent) { + const resolved = (process.env.OPENFRAME_MODE === 'true') + ? (deriveTenantDomain(parent.config.domains) || null) + : null; + return function () { return resolved; }; +} + module.exports.CreateDB = function (parent, func) { var obj = {}; + const openframeDomain = makeOpenframeDomain(parent); var Datastore = null; var expireEventsSeconds = (60 * 60 * 24 * 20); // By default, expire events after 20 days (1728000). (Seconds * Minutes * Hours * Days) var expirePowerEventsSeconds = (60 * 60 * 24 * 10); // By default, expire power events after 10 days (864000). (Seconds * Minutes * Hours * Days) @@ -208,8 +223,16 @@ module.exports.CreateDB = function (parent, func) { } } - // Check if any device groups have a inactive device removal setting + // Check if any device groups have a inactive device removal setting. + // In OpenFrame mode, only this pod's own domain: parent.webserver.meshes is loaded + // unscoped from the shared database, so without this filter one tenant enabling + // expireDevs makes every other tenant's server delete that tenant's devices. Worse, + // the "is it still connected" test below (GetConnectivityState) reads this pod's + // memory, which never holds another tenant's agents — so a foreign pod would delete + // devices that are online on their own. + const removeInactiveOwnDomain = openframeDomain(); for (var i in parent.webserver.meshes) { + if ((removeInactiveOwnDomain != null) && (parent.webserver.meshes[i].domain !== removeInactiveOwnDomain)) continue; if (typeof parent.webserver.meshes[i].expireDevs == 'number') { var v = parent.webserver.meshes[i].expireDevs; if ((v >= 1) && (v <= 2000)) { @@ -229,6 +252,9 @@ module.exports.CreateDB = function (parent, func) { // For each domain with a inactive device removal setting, get a list of last device connections for (var domainid in minRemoveInactiveDevicesPerDomain) { + // Second guard on the same invariant as the mesh loop above: nothing in this + // function may read or delete outside this pod's own domain. + if ((removeInactiveOwnDomain != null) && (domainid !== removeInactiveOwnDomain)) continue; obj.GetAllTypeNoTypeField('lastconnect', domainid, function (err, docs) { if ((err != null) || (docs == null)) return; for (var j in docs) { @@ -478,8 +504,24 @@ module.exports.CreateDB = function (parent, func) { // MariaDB sqlDbQuery('DELETE FROM Main WHERE (extra LIKE ("mesh/%") AND (extra NOT IN ?)', [meshlist], function (err, response) { }); } else if (obj.databaseType == DB_MONGODB) { - // MongoDB - obj.file.deleteMany({ meshid: { $exists: true, $nin: meshlist } }, { multi: true }); + // MongoDB: deliberately does NOT prune orphans. + // + // The upstream statement here was: + // obj.file.deleteMany({ meshid: { $exists: true, $nin: meshlist } }) + // i.e. "delete every document whose meshid is not in the list we just + // read". Correct when one server owns the database; unsafe here, where + // every tenant server shares one. Two ways it destroys data: + // 1. meshlist is built inside `if ((err == null) && (docs.length > 0))` + // above, but the delete ran outside it. A transient read error left + // meshlist empty, and `$nin: []` matches everything — one boot would + // wipe every tenant's nodes, interfaces and notes. + // 2. A mesh created by another tenant between the read and the delete + // is absent from meshlist, so its nodes are deleted. + // Note this also means the read above must stay unscoped for the other + // branches: scoping it to one domain while the delete stays fleet-wide + // would make case 1 the normal path, not the failure path. + // Orphan accounting is done out of band by an audit query scoped with + // an explicit ^mesh// prefix. } else { // NeDB or MongoJS obj.file.remove({ meshid: { $exists: true, $nin: meshlist } }, { multi: true }); @@ -1118,8 +1160,13 @@ module.exports.CreateDB = function (parent, func) { console.log('WARNING: watch() is not a function, MongoDB ChangeStream not supported.'); } else { const tenantDomain = deriveTenantDomain(parent.config.domains); + // Own domain only. The empty domain used to be included here, but it is one + // shared space in a shared database: anything landing in it is visible to every + // tenant server, so treating it as ours contradicts the boot cache, which now + // loads this domain alone. Keeping both in sync matters — a document the cache + // does not hold must not arrive as a change event for an object we never loaded. const changeStreamServerDomains = (process.env.OPENFRAME_MODE === 'true' && tenantDomain) - ? [tenantDomain, ''] + ? [tenantDomain] : Object.keys(parent.config.domains); obj.fileChangeStream = obj.file.watch([{ $match: { $or: [{ 'fullDocument.type': { $in: ['node', 'mesh', 'user', 'ugrp'] } }, { 'operationType': 'delete' }] } }], { fullDocument: 'updateLookup' }); obj.fileChangeStream.on('change', function (change) { @@ -2871,6 +2918,13 @@ module.exports.CreateDB = function (parent, func) { obj.file.find(x, { type: 0 }).toArray(function (err, docs) { func(err, performTypedRecordDecrypt(docs)); }); }; obj.GetAllType = function (type, func) { obj.file.find({ type: type }).toArray(function (err, docs) { func(err, performTypedRecordDecrypt(docs)); }); }; + // Domain-scoped GetAllType. Same query shape as GetAllTypeNoTypeField above, but it + // does NOT project the type field away: callers of this one keep the documents (the + // boot caches, which later save them back), and a document saved without its `type` + // stops being returned by every type query — the user disappears, the mesh drops out + // of any mesh listing. Callers that legitimately want the whole database (the CLI + // branches, export) keep using GetAllType. + obj.GetAllTypeForDomain = function (type, domain, func) { obj.file.find({ type: type, domain: domain }).toArray(function (err, docs) { func(err, performTypedRecordDecrypt(docs)); }); }; obj.GetAllIdsOfType = function (ids, domain, type, func) { obj.file.find({ type: type, domain: domain, _id: { $in: ids } }).toArray(function (err, docs) { func(err, performTypedRecordDecrypt(docs)); }); }; obj.GetUserWithEmail = function (domain, email, func) { obj.file.find({ type: 'user', domain: domain, email: email }).toArray(function (err, docs) { func(err, performTypedRecordDecrypt(docs)); }); }; obj.GetUserWithVerifiedEmail = function (domain, email, func) { obj.file.find({ type: 'user', domain: domain, email: email, emailVerified: true }).toArray(function (err, docs) { func(err, performTypedRecordDecrypt(docs)); }); }; @@ -3021,16 +3075,52 @@ module.exports.CreateDB = function (parent, func) { // Database actions on the Server Stats collection obj.SetServerStats = function (data, func) { obj.serverstatsfile.insertOne(data, func); }; - obj.GetServerStats = function (hours, func) { var t = new Date(); t.setTime(t.getTime() - (60 * 60 * 1000 * hours)); obj.serverstatsfile.find({ time: { $gt: t } }, { _id: 0, cpu: 0 }).toArray(func); }; + // Server stats carry no tenant marker upstream — one server, one timeline. Here every + // tenant server writes into the same collection, so an unfiltered read shows each + // tenant the summed load of the whole fleet. The write side now stamps `domain`; + // rows written before that have none and are still shown during the transition so the + // timeline does not go blank. They expire on their own (expireServerStatsSeconds). + obj.GetServerStats = function (hours, func) { + var t = new Date(); t.setTime(t.getTime() - (60 * 60 * 1000 * hours)); + var q = { time: { $gt: t } }; + var d = openframeDomain(); + if (d != null) { q.$or = [{ domain: d }, { domain: { $exists: false } }]; } + obj.serverstatsfile.find(q, { _id: 0, cpu: 0 }).toArray(func); + }; - // Read a configuration file from the database - obj.getConfigFile = function (path, func) { obj.Get('cfile/' + path, func); } + // Read a configuration file from the database. + // Config files (certificates, terms, images) are keyed by bare filename upstream, so on + // a shared database every tenant server reads and writes the SAME row: whichever pod + // boots last wins, and all tenants end up with identical server certificates — hence an + // identical ServerID, which is exactly what agents pin. + // + // The key is scoped per tenant now. Reads fall back to the legacy shared row, and that + // fallback is what keeps certificates byte-identical through the migration: a pod pulls + // the shared certificate and pushes it back under its own key. Regenerating instead + // would change the ServerID and every installed agent would stop trusting the server. + obj.getConfigFile = function (path, func) { + var d = openframeDomain(); + if (d == null) { obj.Get('cfile/' + path, func); return; } + obj.Get('cfile/' + d + '/' + path, function (err, docs) { + if ((err == null) && (docs != null) && (docs.length > 0)) { func(err, docs); return; } + obj.Get('cfile/' + path, func); // legacy shared row + }); + } // Write a configuration file to the database - obj.setConfigFile = function (path, data, func) { obj.Set({ _id: 'cfile/' + path, type: 'cfile', data: data.toString('base64') }, func); } + obj.setConfigFile = function (path, data, func) { + var d = openframeDomain(); + var doc = { _id: (d == null) ? ('cfile/' + path) : ('cfile/' + d + '/' + path), type: 'cfile', data: data.toString('base64') }; + if (d != null) { doc.domain = d; } + obj.Set(doc, func); + } // List all configuration files - obj.listConfigFiles = function (func) { obj.file.find({ type: 'cfile' }).sort({ _id: 1 }).toArray(func); } + obj.listConfigFiles = function (func) { + var d = openframeDomain(); + var q = (d == null) ? { type: 'cfile' } : { type: 'cfile', _id: { $regex: '^cfile/' + d + '/' } }; + obj.file.find(q).sort({ _id: 1 }).toArray(func); + } // Get database information obj.getDbStats = function (func) { @@ -3287,11 +3377,17 @@ module.exports.CreateDB = function (parent, func) { // Get all configuration files obj.getAllConfigFiles = function (password, func) { + const cfileDomain = openframeDomain(); obj.GetAllType('cfile', function (err, docs) { if (err != null) { func(null); return; } var r = null; for (var i = 0; i < docs.length; i++) { - var name = docs[i]._id.split('/')[1]; + // Keys are 'cfile/' (legacy, shared) or 'cfile//' (scoped), + // so the file name is the last segment, not the second one. Skip other tenants' + // rows: on a shared database GetAllType('cfile') returns the whole fleet's. + var idParts = docs[i]._id.split('/'); + if ((cfileDomain != null) && (idParts.length > 2) && (idParts[1] !== cfileDomain)) continue; + var name = idParts[idParts.length - 1]; var data = obj.decryptData(password, docs[i].data); if (data != null) { if (r == null) { r = {}; } r[name] = data; } } @@ -3299,6 +3395,54 @@ module.exports.CreateDB = function (parent, func) { }); } + // Cross-tenant write guard. Every tenant server shares one database and separation is + // by the `domain` field alone, so a missing domain check anywhere upstream lands here + // as a write into another tenant's document. Wrapping at this single point covers all + // backends: Set/SetUser/Remove are already bound to the concrete one by now. + // + // Only enforced when a domain can actually be determined — from the document, or from + // the id, whose shape is `[prefix]//` (e.g. 'lc' + node//...). + // Documents that carry no tenant at all (cfile/*, serverstats, power, DatabaseIdentifier) + // are out of scope here and are handled by giving them a domain of their own. + // + // Set OPENFRAME_CROSS_TENANT_WRITES=allow to log without blocking (rollback switch). + { + const guardDomain = openframeDomain(); + const guardEnforce = (process.env.OPENFRAME_CROSS_TENANT_WRITES !== 'allow'); + const guardIdDomain = /(?:^|[a-z]{2})(?:user|node|mesh|ugrp)\/([^/]*)\//; + const guardDomainOf = function (doc, id) { + if ((doc != null) && (typeof doc.domain === 'string')) { return doc.domain; } + if (typeof id !== 'string') { return null; } + const m = guardIdDomain.exec(id); + return (m != null) ? m[1] : null; + }; + if (guardDomain) { + ['Set', 'SetUser', 'Remove'].forEach(function (guardName) { + const guardOrig = obj[guardName]; + if (typeof guardOrig != 'function') return; + obj[guardName] = function (guardArg) { + const d = (guardName === 'Remove') + ? guardDomainOf(null, guardArg) + : guardDomainOf(guardArg, (guardArg != null) ? guardArg._id : null); + if ((d != null) && (d !== guardDomain)) { + const guardId = ((guardArg != null) && (guardArg._id != null)) ? guardArg._id : guardArg; + console.log(new Date().toISOString() + ' CROSS-TENANT ' + guardName + + (guardEnforce ? ' BLOCKED' : ' ALLOWED') + + ' domain=' + d + ' mine=' + guardDomain + ' id=' + guardId); + if (guardEnforce) { + // Blocking must not hang a caller that is waiting on a callback + // (Set(data, func) / Remove(id, func)), so fail it explicitly. + const guardCb = arguments[arguments.length - 1]; + if (typeof guardCb == 'function') { setImmediate(function () { guardCb(new Error('cross-tenant write blocked')); }); } + return; + } + } + return guardOrig.apply(obj, arguments); + }; + }); + } + } + func(obj); // Completed function setup } diff --git a/meshcentral.js b/meshcentral.js index ff759f9ee6..2a4e7c0103 100644 --- a/meshcentral.js +++ b/meshcentral.js @@ -31,6 +31,24 @@ function CreateMeshCentralServer(config, args) { obj.msgserver = null; // Messaging server, used to sent used messages obj.amtEventHandler = null; obj.pluginHandler = null; + // This pod's tenant, or null outside OpenFrame mode. Used where a record or a database key + // would otherwise be shared by every tenant server writing into the one database. + // Resolved on first use and cached: a server's tenant cannot change while it runs, and every + // caller must get the same answer regardless of when it asks (db.js does the same). + var openframeDomainCache; + obj.openframeDomain = function () { + if (openframeDomainCache === undefined) { + openframeDomainCache = (process.env.OPENFRAME_MODE === 'true') + ? (require('./db.js').deriveTenantDomain(obj.config.domains) || null) + : null; + } + return openframeDomainCache; + }; + // Database keys that must not be one shared row across the fleet. + obj.tenantDbKey = function (baseId) { + const d = obj.openframeDomain(); + return (d == null) ? baseId : (baseId + '_' + d); + }; obj.amtScanner = null; obj.amtManager = null; // Intel AMT manager, used to oversee all Intel AMT devices, activate them and sync policies obj.meshScanner = null; @@ -1747,11 +1765,25 @@ function CreateMeshCentralServer(config, args) { obj.fs.open(obj.path.join(obj.datapath, 'agenterrorlogs.txt'), 'a', function (err, fd) { obj.agentErrorLog = fd; }) } - // Perform other database cleanup - obj.db.cleanup(); - - // Set all nodes to power state of unknown (0) - obj.db.storePowerEvent({ time: new Date(), nodeid: '*', power: 0, s: 1 }, obj.multiServer); // s:1 indicates that the server is starting up. + // Perform other database cleanup. + // Skipped in OpenFrame mode: every tenant server shares one database, and cleanup() + // is written for "one server owns the whole database". It rewrites every tenant's + // user and mesh documents (db.js, obj.Set inside the GetAllType('user'/'mesh') + // callbacks) and deletes fleet-wide. What it repairs — pre-1.0 field formats and + // legacy type:'event'/'power'/'smbios' rows in the main collection — never existed + // in this database: those types are written to their own collections here. + // Routine housekeeping is covered elsewhere: RemoveMeshDocuments() on device group + // deletion, the explicit Remove() calls on device deletion, and the TTL indexes. + if (process.env.OPENFRAME_MODE !== 'true') { obj.db.cleanup(); } + + // Set all nodes to power state of unknown (0). + // Skipped in OpenFrame mode: this marker is stored against the wildcard node id '*' and the + // power collection has no domain field, while getPowerTimeline() selects {nodeid: {$in: + // ['*', nodeid]}} — so on a shared database every pod's boot shows up in every tenant's + // device power timeline. + if (obj.openframeDomain() == null) { + obj.db.storePowerEvent({ time: new Date(), nodeid: '*', power: 0, s: 1 }, obj.multiServer); // s:1 indicates that the server is starting up. + } // Read or setup database configuration values obj.db.Get('dbconfig', function (err, dbconfig) { @@ -2174,23 +2206,28 @@ function CreateMeshCentralServer(config, args) { if ((obj.loginCookieEncryptionKey == null) || (obj.loginCookieEncryptionKey.length != 80)) { addServerWarning("Invalid \"LoginCookieEncryptionKey\" in config.json.", 20); obj.loginCookieEncryptionKey = null; } } - // Login cookie encryption key not set, use one from the database + // Login cookie encryption key not set, use one from the database. + // Per tenant (tenantDbKey) rather than one row for the whole database: this key + // signs login tokens, relay cookies and auth cookies, so a single shared row + // means a cookie minted by one tenant's server verifies on every other one. + // Rotating it invalidates existing sessions for that tenant — expected, and the + // reason this needs a maintenance window. if (obj.loginCookieEncryptionKey == null) { - obj.db.Get('LoginCookieEncryptionKey', function (err, docs) { + obj.db.Get(obj.tenantDbKey('LoginCookieEncryptionKey'), function (err, docs) { if ((docs != null) && (docs.length > 0) && (docs[0].key != null) && (obj.args.logintokengen == null) && (docs[0].key.length >= 160)) { obj.loginCookieEncryptionKey = Buffer.from(docs[0].key, 'hex'); } else { - obj.loginCookieEncryptionKey = obj.generateCookieKey(); obj.db.Set({ _id: 'LoginCookieEncryptionKey', key: obj.loginCookieEncryptionKey.toString('hex'), time: Date.now() }); + obj.loginCookieEncryptionKey = obj.generateCookieKey(); obj.db.Set({ _id: obj.tenantDbKey('LoginCookieEncryptionKey'), key: obj.loginCookieEncryptionKey.toString('hex'), time: Date.now() }); } }); } // Load the invitation link encryption key from the database - obj.db.Get('InvitationLinkEncryptionKey', function (err, docs) { + obj.db.Get(obj.tenantDbKey('InvitationLinkEncryptionKey'), function (err, docs) { if ((docs != null) && (docs.length > 0) && (docs[0].key != null) && (docs[0].key.length >= 160)) { obj.invitationLinkEncryptionKey = Buffer.from(docs[0].key, 'hex'); } else { - obj.invitationLinkEncryptionKey = obj.generateCookieKey(); obj.db.Set({ _id: 'InvitationLinkEncryptionKey', key: obj.invitationLinkEncryptionKey.toString('hex'), time: Date.now() }); + obj.invitationLinkEncryptionKey = obj.generateCookieKey(); obj.db.Set({ _id: obj.tenantDbKey('InvitationLinkEncryptionKey'), key: obj.invitationLinkEncryptionKey.toString('hex'), time: Date.now() }); } }); @@ -2238,6 +2275,10 @@ function CreateMeshCentralServer(config, args) { const node = obj.connectivityByNode[i]; if (node && typeof node.connectivity !== 'undefined' && node.connectivity === 4) { data.conn.am++; } } + // Stamp the tenant: the collection is shared and upstream writes no domain, + // so without this every tenant's "My Server" timeline is the fleet's sum. + const statsDomain = obj.openframeDomain(); + if (statsDomain != null) { data.domain = statsDomain; } if (obj.firstStats === true) { delete obj.firstStats; data.first = true; } if (obj.multiServer != null) { data.s = obj.multiServer.serverid; } obj.db.SetServerStats(data); // Save the stats to the database @@ -3856,7 +3897,7 @@ function CreateMeshCentralServer(config, args) { func('User ' + userid + ' not found.'); return; } else { // Load the login cookie encryption key from the database - obj.db.Get('LoginCookieEncryptionKey', function (err, docs) { + obj.db.Get(obj.tenantDbKey('LoginCookieEncryptionKey'), function (err, docs) { if ((docs.length > 0) && (docs[0].key != null) && (obj.args.logintokengen == null) && (docs[0].key.length >= 160)) { // Key is present, use it. obj.loginCookieEncryptionKey = Buffer.from(docs[0].key, 'hex'); @@ -3864,7 +3905,7 @@ function CreateMeshCentralServer(config, args) { } else { // Key is not present, generate one. obj.loginCookieEncryptionKey = obj.generateCookieKey(); - obj.db.Set({ _id: 'LoginCookieEncryptionKey', key: obj.loginCookieEncryptionKey.toString('hex'), time: Date.now() }, function () { func(obj.encodeCookie({ u: userid, a: 3 }, obj.loginCookieEncryptionKey)); }); + obj.db.Set({ _id: obj.tenantDbKey('LoginCookieEncryptionKey'), key: obj.loginCookieEncryptionKey.toString('hex'), time: Date.now() }, function () { func(obj.encodeCookie({ u: userid, a: 3 }, obj.loginCookieEncryptionKey)); }); } }); } @@ -3874,14 +3915,14 @@ function CreateMeshCentralServer(config, args) { // Show the user login token generation key obj.showLoginTokenKey = function (func) { // Load the login cookie encryption key from the database - obj.db.Get('LoginCookieEncryptionKey', function (err, docs) { + obj.db.Get(obj.tenantDbKey('LoginCookieEncryptionKey'), function (err, docs) { if ((docs.length > 0) && (docs[0].key != null) && (obj.args.logintokengen == null) && (docs[0].key.length >= 160)) { // Key is present, use it. func(docs[0].key); } else { // Key is not present, generate one. obj.loginCookieEncryptionKey = obj.generateCookieKey(); - obj.db.Set({ _id: 'LoginCookieEncryptionKey', key: obj.loginCookieEncryptionKey.toString('hex'), time: Date.now() }, function () { func(obj.loginCookieEncryptionKey.toString('hex')); }); + obj.db.Set({ _id: obj.tenantDbKey('LoginCookieEncryptionKey'), key: obj.loginCookieEncryptionKey.toString('hex'), time: Date.now() }, function () { func(obj.loginCookieEncryptionKey.toString('hex')); }); } }); }; diff --git a/meshuser.js b/meshuser.js index 34013e1a8b..b37d5f2d23 100644 --- a/meshuser.js +++ b/meshuser.js @@ -1967,6 +1967,9 @@ module.exports.CreateMeshUser = function (parent, db, ws, req, args, domain, use if (typeof command.removeMultiFactor != 'boolean') break; if ((command.pass != '') && (common.checkPasswordRequirements(command.pass, domain.passwordrequirements) == false)) break; // Password does not meet requirements + // Every tenant shares one database and parent.users holds all of them, so a + // bare id lookup resolves another tenant's user. Same check as 'deleteuser'. + if ((command.userid.split('/').length != 3) || (command.userid.split('/')[1] != domain.id)) break; var chguser = parent.users[command.userid]; if (chguser) { // If we are not full administrator, we can't change anything on a different full administrator @@ -2020,6 +2023,9 @@ module.exports.CreateMeshUser = function (parent, db, ws, req, args, domain, use if (common.validateString(command.userid, 1, 2048) == false) break; if (common.validateString(command.msg, 1, 4096) == false) break; + // Every tenant shares one database and parent.users holds all of them, so a + // bare id lookup resolves another tenant's user. Same check as 'deleteuser'. + if ((command.userid.split('/').length != 3) || (command.userid.split('/')[1] != domain.id)) break; // Can only perform this operation on other users of our group. var chguser = parent.users[command.userid]; if (chguser == null) break; // This user does not exists @@ -2058,6 +2064,9 @@ module.exports.CreateMeshUser = function (parent, db, ws, req, args, domain, use // Send a notification message to a user if ((user.siteadmin & 2) == 0) break; + // Every tenant shares one database and parent.users holds all of them, so a + // bare id lookup resolves another tenant's user. Same check as 'deleteuser'. + if ((command.userid.split('/').length != 3) || (command.userid.split('/')[1] != domain.id)) break; // Can only perform this operation on other users of our group. var chguser = parent.users[command.userid]; if (chguser == null) break; // This user does not exists @@ -4779,8 +4788,10 @@ module.exports.CreateMeshUser = function (parent, db, ws, req, args, domain, use var result = { users: [], userGroups: [], meshes: [], nodes: [] }; - // Get all users - parent.db.GetAllType('user', function(err, docs) { + // Get all users in this domain. Domain-scoped because every tenant shares one + // database: an unscoped read here returns every tenant's users, and their ids + // (user//) disclose the other tenants' domain keys and admin names. + parent.db.GetAllTypeForDomain('user', domain.id, function(err, docs) { if (docs) { docs.forEach(function(u) { if (u.name && u._id) { @@ -4789,8 +4800,8 @@ module.exports.CreateMeshUser = function (parent, db, ws, req, args, domain, use }); } - // Get all user groups - parent.db.GetAllType('ugrp', function(err, ugrps) { + // Get all user groups in this domain + parent.db.GetAllTypeForDomain('ugrp', domain.id, function(err, ugrps) { if (ugrps) { ugrps.forEach(function(ug) { if (ug.name && ug._id) { @@ -4799,8 +4810,8 @@ module.exports.CreateMeshUser = function (parent, db, ws, req, args, domain, use }); } - // Get all meshes (device groups) - parent.db.GetAllType('mesh', function(err, meshes) { + // Get all meshes (device groups) in this domain + parent.db.GetAllTypeForDomain('mesh', domain.id, function(err, meshes) { if (meshes) { meshes.forEach(function(m) { if (m.name && m._id && !m.deleted) { @@ -4809,8 +4820,8 @@ module.exports.CreateMeshUser = function (parent, db, ws, req, args, domain, use }); } - // Get all nodes (devices) - parent.db.GetAllType('node', function(err, nodes) { + // Get all nodes (devices) in this domain + parent.db.GetAllTypeForDomain('node', domain.id, function(err, nodes) { if (nodes) { // Create a map of meshid to meshname for grouping var meshMap = {}; @@ -6531,7 +6542,7 @@ module.exports.CreateMeshUser = function (parent, db, ws, req, args, domain, use else if (common.validateString(command.subject, 1, 1000) == false) { errMsg = 'Invalid subject message'; } else if (common.validateString(command.msg, 1, 10000) == false) { errMsg = 'Invalid message'; } else { - emailuser = parent.users[command.userid]; + emailuser = ((command.userid.split('/').length == 3) && (command.userid.split('/')[1] == domain.id)) ? parent.users[command.userid] : null; // Domain-scoped: parent.users holds every tenant's users if (emailuser == null) { errMsg = 'Invalid userid'; } else if (emailuser.email == null) { errMsg = 'No validated email address for this user'; } else if (emailuser.emailVerified !== true) { errMsg = 'No validated email address for this user'; } @@ -7032,7 +7043,7 @@ module.exports.CreateMeshUser = function (parent, db, ws, req, args, domain, use else if (common.validateString(command.userid, 1, 2048) == false) { errMsg = "Invalid username"; } else if (common.validateString(command.msg, 1, 160) == false) { errMsg = "Invalid SMS message"; } else { - smsuser = parent.users[command.userid]; + smsuser = ((command.userid.split('/').length == 3) && (command.userid.split('/')[1] == domain.id)) ? parent.users[command.userid] : null; // Domain-scoped: parent.users holds every tenant's users if (smsuser == null) { errMsg = "Invalid username"; } else if (smsuser.phone == null) { errMsg = "No phone number for this user"; } } @@ -7055,7 +7066,7 @@ module.exports.CreateMeshUser = function (parent, db, ws, req, args, domain, use else if (common.validateString(command.userid, 1, 2048) == false) { errMsg = "Invalid username"; } else if (common.validateString(command.msg, 1, 160) == false) { errMsg = "Invalid message"; } else { - msguser = parent.users[command.userid]; + msguser = ((command.userid.split('/').length == 3) && (command.userid.split('/')[1] == domain.id)) ? parent.users[command.userid] : null; // Domain-scoped: parent.users holds every tenant's users if (msguser == null) { errMsg = "Invalid username"; } else if (msguser.msghandle == null) { errMsg = "No messaging service configured for this user"; } } diff --git a/test/README.md b/test/README.md new file mode 100644 index 0000000000..7961cc68be --- /dev/null +++ b/test/README.md @@ -0,0 +1,49 @@ +# Tenant isolation tests + +In OpenFrame mode every tenant runs its own MeshCentral server, but all of them share **one MongoDB +database** with shared collections, and tenants are separated only by the `domain` field on each +document (which is also embedded in the `_id`). Upstream MeshCentral is written for "one server owns +the whole database", so several of its queries and maintenance routines legitimately span everything +they can see — which here means every tenant. + +These tests pin the places that were changed for that. Each one seeds two tenant domains into a +single database, drives `db.js` as tenant A, and asserts that tenant B is left alone. + +## Running + +```bash +test/run-tests.sh # against :latest +test/run-tests.sh ghcr.io/flamingo-stack/meshcentral/meshcentral:0.0.28 +``` + +The script starts a throwaway MongoDB replica set and runs `node --test` inside the MeshCentral +image with this checkout mounted read-only at `/src`. Two reasons it is not a plain `npm test`: + +- `mongodb` is not a `package.json` dependency of this repository — the driver lives in the image. + `NODE_PATH` points module resolution there while the code under test comes from `/src`. +- The tests use a real replica set rather than a mock. What is being tested is the *shape of the + queries* (does this find/delete carry a domain?), and a mock would only assert that the code calls + the mock. + +CI runs the same script (`test_isolation` job in `.github/workflows/test.yml`). + +## What is covered + +| Test | Guards against | +|---|---| +| `GetAllTypeForDomain` returns one tenant and keeps `type` | the boot cache loading the whole fleet; and the projection trap — `GetAllTypeNoTypeField` strips `type`, and a document saved back without it disappears from every type query | +| `cleanup()` deletes nothing | upstream's `deleteMany({meshid: {$nin: meshlist}})`, which deleted other tenants' devices and, with an empty `meshlist` after a read error, everything | +| `removeInactiveDevices` stays in its domain | the hourly maintenance pass reading device groups of other tenants out of the shared boot cache and deleting their devices | +| cross-tenant writes are blocked, own-domain writes pass | any missing domain check upstream of `Set`/`SetUser`/`Remove` reaching another tenant's documents | +| the write guard can be disabled | that `OPENFRAME_CROSS_TENANT_WRITES=allow` still works as a rollback switch | +| server stats are read per tenant | the "My Server" timeline showing the summed load of the whole fleet | +| config files are per tenant, with a legacy fallback | every pod overwriting the same `cfile/` row, which gave all tenants identical server certificates — and therefore the identical ServerID agents pin | +| `deriveTenantDomain` picks the tenant | the domain resolution the rest depends on, including that a legacy single-domain install stays unscoped | + +## What is not covered here + +- Anything above the database layer: `getpluginpermissionlist` and `changeuserpass` are WebSocket + commands and need a running server plus an authenticated session. They were verified manually + against a two-domain database; a WS-level suite would be the next step. +- The gateway command allowlist, which lives in `openframe-oss-lib` and has its own unit tests. +- Agent connectivity, and the certificate sync (`autoSyncConfigFiles`) against real certificates. diff --git a/test/run-tests.sh b/test/run-tests.sh new file mode 100755 index 0000000000..802f071b1d --- /dev/null +++ b/test/run-tests.sh @@ -0,0 +1,49 @@ +#!/usr/bin/env bash +# Runs the tenant isolation tests. +# +# They need two things this repository does not carry: a MongoDB replica set (changeStream setup in +# db.js needs one, and it is what production runs), and the node_modules that live in the built +# image rather than in the repo — `mongodb` is not a package.json dependency here. +# +# So: start a throwaway Mongo, then run `node --test` inside the meshcentral image with this +# checkout mounted read-only at /src. NODE_PATH points module resolution at the image's +# node_modules, so db.js can require('mongodb') while its own code comes from /src. +# +# Usage: test/run-tests.sh [image] +set -euo pipefail + +IMAGE="${1:-ghcr.io/flamingo-stack/meshcentral/meshcentral:latest}" +SRC="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +NET=mc-test-net +MONGO=mc-test-mongo +MONGO_IMAGE="${MONGO_IMAGE:-ghcr.io/flamingo-stack/registry/mongo:7.0.40}" + +cleanup() { + docker rm -f "$MONGO" >/dev/null 2>&1 || true + docker network rm "$NET" >/dev/null 2>&1 || true +} +trap cleanup EXIT + +cleanup +docker network create "$NET" >/dev/null +docker run -d --name "$MONGO" --network "$NET" "$MONGO_IMAGE" --replSet rs0 --bind_ip_all >/dev/null + +echo "Waiting for MongoDB..." +for _ in $(seq 1 30); do + if docker exec "$MONGO" mongosh --quiet --eval 'db.adminCommand("ping").ok' >/dev/null 2>&1; then break; fi + sleep 2 +done +docker exec "$MONGO" mongosh --quiet --eval "rs.initiate({_id:'rs0',members:[{_id:0,host:'$MONGO:27017'}]})" >/dev/null +for _ in $(seq 1 30); do + if [ "$(docker exec "$MONGO" mongosh --quiet --eval 'db.adminCommand("hello").isWritablePrimary')" = "true" ]; then break; fi + sleep 2 +done + +echo "Running tests..." +docker run --rm --network "$NET" \ + -v "$SRC:/src:ro" \ + -e NODE_PATH=/opt/meshcentral/meshcentral/node_modules \ + -e MC_SRC=/src \ + -e MC_TEST_MONGO_URL="mongodb://$MONGO:27017/meshcentral_test?replicaSet=rs0" \ + "$IMAGE" \ + node --test --test-force-exit --test-timeout=60000 /src/test/*.test.js diff --git a/test/tenant-isolation.test.js b/test/tenant-isolation.test.js new file mode 100644 index 0000000000..ee5f05345c --- /dev/null +++ b/test/tenant-isolation.test.js @@ -0,0 +1,233 @@ +/** + * Tenant isolation tests for the OpenFrame fork. + * + * Every tenant server shares ONE MongoDB database and is separated only by the `domain` field, so + * upstream's "one server owns the whole database" queries reach across tenants here. These tests + * pin the behaviours that were changed for that: each one seeds two tenant domains into a single + * database, drives db.js as tenant A, and asserts that tenant B is untouched. + * + * They run against a real MongoDB replica set rather than a mock: what is being tested is the shape + * of the queries themselves, so a mock would only assert that the code calls the mock. + * + * Run with test/run-tests.sh (starts the datastore and executes this inside the meshcentral image). + */ + +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert'); +const path = require('path'); + +const MESH_DIR = process.env.MC_SRC || path.join(__dirname, '..'); +const MONGO_URL = process.env.MC_TEST_MONGO_URL || 'mongodb://localhost:27017/meshcentral_test?replicaSet=rs0'; + +const A = 'aaaa1111-1111-1111-1111-111111111111'; +const B = 'bbbb2222-2222-2222-2222-222222222222'; + +// --- Helpers --------------------------------------------------------------- + +function config(domain) { + // Mirrors the deployed shape: the default domain, the tenant's own, and the static share domain + // that deriveTenantDomain() must skip. + const domains = { '': { title: 'MeshCentral' }, openframe_public: { share: '/opt/mesh/public' } }; + domains[domain] = { title: domain }; + return { settings: { mongodb: MONGO_URL, mongodbname: dbName(), mongodbcol: 'meshcentral' }, domains: domains }; +} + +function dbName() { + const m = /\/([^/?]+)(\?|$)/.exec(MONGO_URL); + return m ? m[1] : 'meshcentral_test'; +} + +/** A minimal MeshCentral "parent", the same shim plugins/migrate.js uses to drive db.js standalone. */ +function parentShim(domain) { + const cfg = config(domain); + return { + datapath: '/tmp/meshcentral-test-data', + args: { mongodb: cfg.settings.mongodb, mongodbname: cfg.settings.mongodbname, mongodbcol: cfg.settings.mongodbcol }, + config: cfg, + crypto: require('crypto'), fs: require('fs'), path: require('path'), + common: require(path.join(MESH_DIR, 'common.js')), + debug: function () { }, DispatchEvent: function () { }, + GetConnectivityState: function () { return null; }, + webserver: { meshes: {}, users: {}, CreateNodeDispatchTargets: function () { return []; } }, + userGroups: {} + }; +} + +/** Open db.js as a tenant server for `domain`. `env` is applied before setup, since the write guard + * binds itself at setup time. */ +function openDb(domain, env) { + const previous = {}; + const applied = Object.assign({ OPENFRAME_MODE: 'true' }, env || {}); + for (const k in applied) { previous[k] = process.env[k]; process.env[k] = applied[k]; } + + const parent = parentShim(domain); + return new Promise((resolve) => { + const db = require(path.join(MESH_DIR, 'db.js')).CreateDB(parent, function () { + db.SetupDatabase(function () { + for (const k in previous) { if (previous[k] === undefined) { delete process.env[k]; } else { process.env[k] = previous[k]; } } + resolve({ db, parent }); + }); + }); + }); +} + +function seedDocs() { + const old = Date.now() - (400 * 86400000); // far past any expireDevs setting + const perDomain = (d) => ([ + { _id: 'user/' + d + '/admin', type: 'user', name: 'admin', domain: d, siteadmin: 0xFFFFFFFF, links: {} }, + { _id: 'mesh/' + d + '/GRP', type: 'mesh', name: 'OpenFrame', domain: d, mtype: 2, expireDevs: 30, links: {} }, + { _id: 'node/' + d + '/DEV', type: 'node', name: 'device', domain: d, meshid: 'mesh/' + d + '/GRP' }, + { _id: 'ifnode/' + d + '/DEV', domain: d, netif: {} }, + { _id: 'lcnode/' + d + '/DEV', type: 'lastconnect', domain: d, meshid: 'mesh/' + d + '/GRP', time: old }, + { _id: 'ugrp/' + d + '/G1', type: 'ugrp', name: 'group', domain: d, links: {} }, + // An orphan: its device group does not exist. Upstream cleanup() deleted these DB-wide. + { _id: 'node/' + d + '/ORPHAN', type: 'node', name: 'orphan', domain: d, meshid: 'mesh/' + d + '/GONE' } + ]); + return perDomain(A).concat(perDomain(B)); +} + +async function freshDatabase() { + const { MongoClient } = require('mongodb'); + const client = await MongoClient.connect(MONGO_URL); + const db = client.db(dbName()); + for (const c of ['meshcentral', 'serverstats', 'events', 'power', 'smbios']) { + await db.collection(c).deleteMany({}); + } + await db.collection('meshcentral').insertMany(seedDocs()); + return { client, db }; +} + +function ids(docs) { return docs.map((d) => d._id).sort(); } + +// --- Tests ----------------------------------------------------------------- + +test('GetAllTypeForDomain returns only this tenant and keeps the type field', async () => { + const { client, db: raw } = await freshDatabase(); + const { db } = await openDb(A); + try { + const users = await new Promise((res) => db.GetAllTypeForDomain('user', A, (err, docs) => res(docs))); + assert.deepStrictEqual(ids(users), ['user/' + A + '/admin']); + + // The type field must survive: these documents are held in the boot cache and written back + // later, and a document saved without `type` stops being returned by every type query. + assert.strictEqual(users[0].type, 'user'); + + // Guard against the projection trap by round-tripping the object the way the server does. + // (SetUser takes no callback in MeshCentral, so the write is awaited through Set.) + await new Promise((res) => db.Set(users[0], res)); + const after = await raw.collection('meshcentral').findOne({ _id: 'user/' + A + '/admin' }); + assert.strictEqual(after.type, 'user'); + } finally { await client.close(); } +}); + +test('cleanup() does not delete documents whose device group is missing', async () => { + // Upstream ended cleanup() with deleteMany({meshid: {$nin: meshlist}}) over the whole database. + // On a shared database that deletes other tenants' devices, and an empty meshlist (a transient + // read error) matches everything. The statement is gone; nothing may be deleted here. + const { client, db: raw } = await freshDatabase(); + const { db } = await openDb(A); + try { + // Counted per domain rather than globally: SetupDatabase fires off its own + // DatabaseIdentifier / SchemaVersion writes without waiting, which would race a total. + const countA = () => raw.collection('meshcentral').countDocuments({ domain: A }); + const countB = () => raw.collection('meshcentral').countDocuments({ domain: B }); + const beforeA = await countA(), beforeB = await countB(); + await new Promise((res) => db.cleanup(res)); + assert.strictEqual(await countA(), beforeA); + assert.strictEqual(await countB(), beforeB); + assert.strictEqual(await raw.collection('meshcentral').countDocuments({ _id: 'node/' + B + '/ORPHAN' }), 1); + assert.strictEqual(await raw.collection('meshcentral').countDocuments({ _id: 'node/' + A + '/ORPHAN' }), 1); + } finally { await client.close(); } +}); + +test('removeInactiveDevices only removes devices of this tenant', async () => { + const { client, db: raw } = await freshDatabase(); + const { db, parent } = await openDb(A); + try { + // The boot cache still holds both tenants here on purpose: that is the state the function + // used to read foreign domains from. + const meshes = await new Promise((res) => db.GetAllType('mesh', (err, docs) => res(docs))); + for (const m of meshes) { parent.webserver.meshes[m._id] = m; } + + await new Promise((res) => { db.removeInactiveDevices(false, function () { }); setTimeout(res, 2000); }); + + assert.strictEqual(await raw.collection('meshcentral').countDocuments({ _id: 'node/' + A + '/DEV' }), 0, "own tenant's inactive device should be removed"); + assert.strictEqual(await raw.collection('meshcentral').countDocuments({ _id: 'node/' + B + '/DEV' }), 1, "other tenant's device must be untouched"); + assert.strictEqual(await raw.collection('meshcentral').countDocuments({ _id: 'lcnode/' + B + '/DEV' }), 1); + } finally { await client.close(); } +}); + +test('writes into another tenant are blocked, own-domain writes pass', async () => { + const { client, db: raw } = await freshDatabase(); + const { db } = await openDb(A); + try { + const foreign = await new Promise((res) => db.Set({ _id: 'user/' + B + '/admin', type: 'user', domain: B, name: 'overwritten' }, (err) => res(err))); + assert.ok(foreign instanceof Error, 'a cross-tenant Set must fail its callback rather than hang'); + assert.strictEqual((await raw.collection('meshcentral').findOne({ _id: 'user/' + B + '/admin' })).name, 'admin'); + + const own = await new Promise((res) => db.Set({ _id: 'user/' + A + '/second', type: 'user', domain: A, name: 'second' }, (err) => res(err))); + assert.ok(!own, 'own-domain writes must not be affected'); + assert.strictEqual(await raw.collection('meshcentral').countDocuments({ _id: 'user/' + A + '/second' }), 1); + + // Remove resolves the domain from the id, which carries it even for prefixed keys. + await new Promise((res) => db.Remove('lcnode/' + B + '/DEV', res)); + assert.strictEqual(await raw.collection('meshcentral').countDocuments({ _id: 'lcnode/' + B + '/DEV' }), 1); + } finally { await client.close(); } +}); + +test('the write guard can be turned off for rollback', async () => { + const { client, db: raw } = await freshDatabase(); + const { db } = await openDb(A, { OPENFRAME_CROSS_TENANT_WRITES: 'allow' }); + try { + await new Promise((res) => db.Set({ _id: 'user/' + B + '/admin', type: 'user', domain: B, name: 'overwritten' }, res)); + assert.strictEqual((await raw.collection('meshcentral').findOne({ _id: 'user/' + B + '/admin' })).name, 'overwritten'); + } finally { await client.close(); } +}); + +test('server stats are read per tenant, with legacy rows still visible', async () => { + const { client, db: raw } = await freshDatabase(); + const { db } = await openDb(A); + try { + const now = new Date(); + await raw.collection('serverstats').insertMany([ + { time: now, domain: A, conn: { ca: 1 } }, + { time: now, domain: B, conn: { ca: 2 } }, + { time: now, conn: { ca: 3 } } // written before the domain field existed + ]); + const rows = await new Promise((res) => db.GetServerStats(24, (err, docs) => res(docs))); + const seen = rows.map((r) => r.conn.ca).sort(); + assert.deepStrictEqual(seen, [1, 3], 'own rows plus legacy rows, never another tenant'); + } finally { await client.close(); } +}); + +test('config files are stored per tenant and fall back to the shared legacy key', async () => { + const { client, db: raw } = await freshDatabase(); + const { db } = await openDb(A); + try { + await new Promise((res) => db.setConfigFile('agentserver-cert-public.crt', Buffer.from('CERT'), res)); + assert.strictEqual(await raw.collection('meshcentral').countDocuments({ _id: 'cfile/' + A + '/agentserver-cert-public.crt' }), 1); + + // A file that only exists under the legacy shared key must still resolve: that fallback is + // what keeps certificates (and therefore the ServerID agents pin) identical through the + // migration, instead of every pod minting a new one. + await raw.collection('meshcentral').insertOne({ _id: 'cfile/legacy.crt', type: 'cfile', data: Buffer.from('LEGACY').toString('base64') }); + const legacy = await new Promise((res) => db.getConfigFile('legacy.crt', (err, docs) => res(docs))); + assert.strictEqual(legacy.length, 1); + assert.strictEqual(legacy[0]._id, 'cfile/legacy.crt'); + + // Another tenant's file must not be listed. + await raw.collection('meshcentral').insertOne({ _id: 'cfile/' + B + '/secret.crt', type: 'cfile', data: 'x' }); + const listed = await new Promise((res) => db.listConfigFiles((err, docs) => res(docs))); + assert.deepStrictEqual(ids(listed), ['cfile/' + A + '/agentserver-cert-public.crt']); + } finally { await client.close(); } +}); + +test('deriveTenantDomain picks the tenant, ignoring the default and share domains', () => { + const { deriveTenantDomain } = require(path.join(MESH_DIR, 'db.js')); + assert.strictEqual(deriveTenantDomain(config(A).domains), A); + // A legacy single-tenant install has only the default domain and must stay unscoped. + assert.strictEqual(deriveTenantDomain({ '': { title: 'MeshCentral' } }), ''); + assert.strictEqual(deriveTenantDomain(null), ''); +}); diff --git a/webserver.js b/webserver.js index 26af9d2d73..3108604155 100644 --- a/webserver.js +++ b/webserver.js @@ -287,8 +287,22 @@ module.exports.CreateWebServer = function (parent, db, args, certificates, doneF function EscapeHtml(x) { if (typeof x == 'string') return x.replace(/&/g, '&').replace(/>/g, '>').replace(//g, '>').replace(/').replace(/\n/g, '').replace(/\t/g, '  '); if (typeof x == "boolean") return x; if (typeof x == "number") return x; } + // Boot cache scope. All tenant servers share one database, and these three loads are what + // put every tenant's users, device groups and user groups into this pod's memory: the maps + // are keyed by full _id, so any lookup that does not separately re-check .domain resolves a + // foreign object (that is how 'changeuserpass' reached another tenant's admin, and how + // removeInactiveDevices found another tenant's device groups). Loading only our own domain + // removes the class rather than the individual call sites, and makes memory O(tenant) + // instead of O(fleet). Null outside OpenFrame mode, where the load stays unscoped. + const bootCacheDomain = (process.env.OPENFRAME_MODE === 'true') + ? require('./db.js').deriveTenantDomain(parent.config.domains) + : null; + const loadAllOfType = function (type, func) { + if (bootCacheDomain) { obj.db.GetAllTypeForDomain(type, bootCacheDomain, func); } else { obj.db.GetAllType(type, func); } + }; + // Fetch all users from the database, keep this in memory - obj.db.GetAllType('user', function (err, docs) { + loadAllOfType('user', function (err, docs) { if (err != null) { parent.diagLog('ERROR', new Date().toISOString() + ' ERROR: failed to load users from database at startup: ' + err); } obj.common.unEscapeAllLinksFieldName(docs); var domainUserCount = {}, i = 0; @@ -305,7 +319,7 @@ module.exports.CreateWebServer = function (parent, db, args, certificates, doneF // Fetch all device groups (meshes) from the database, keep this in memory // As we load things in memory, we will also be doing some cleaning up. // We will not save any clean up in the database right now, instead it will be saved next time there is a change. - obj.db.GetAllType('mesh', function (err, docs) { + loadAllOfType('mesh', function (err, docs) { // A failed device-group load (e.g. a transient Mongo timeout) silently left // obj.meshes empty, which makes the server orphan EVERY agent ("invalid // domain/mesh, holding connection") until the next restart. Surface it. @@ -342,7 +356,7 @@ module.exports.CreateWebServer = function (parent, db, args, certificates, doneF } catch (censusEx) { parent.diagLog('DEBUG', 'DB census error: ' + censusEx); } // Fetch all user groups from the database, keep this in memory - obj.db.GetAllType('ugrp', function (err, docs) { + loadAllOfType('ugrp', function (err, docs) { obj.common.unEscapeAllLinksFieldName(docs); // Perform user group link cleanup @@ -2240,7 +2254,13 @@ module.exports.CreateWebServer = function (parent, db, args, certificates, doneF // If there is a login token, use that if (req.query.login != null) { var ucookie = parent.decodeCookie(req.query.login, parent.loginCookieEncryptionKey, 60); // Cookie with 1 hour timeout - if ((ucookie != null) && (ucookie.a === 3) && (typeof ucookie.u == 'string')) { user = obj.users[ucookie.u]; } + // The userid inside the cookie must belong to the domain this request was served on. + // Every other cookie acceptance point checks this (see the control websocket handler and + // the relay cookie above); this one did not, and obj.users is keyed by full id, so a + // token naming another domain's user resolved to that user. Sibling to the per-tenant + // cookie key: the key stops a foreign cookie from validating at all, this stops a + // validly-signed one from being used outside its domain. + if ((ucookie != null) && (ucookie.a === 3) && (typeof ucookie.u == 'string') && (ucookie.u.split('/')[1] == domain.id)) { user = obj.users[ucookie.u]; } } // If no token, see if we have an active session