diff options
author | Moustafa Maher <m.maher@10gen.com> | 2021-03-03 00:08:53 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2021-08-04 23:26:46 +0000 |
commit | 865eccaf35aca29d1b71764d50227cdf853752d0 (patch) | |
tree | c3947ff401fc19d372908a59957e7d8b2c14831c | |
parent | 31194b8dd00e0862d2b0ebc5d6502360724e7297 (diff) | |
download | mongo-865eccaf35aca29d1b71764d50227cdf853752d0.tar.gz |
SERVER-36263 Bypassing operation validation in applyOps should require special privilege
-rw-r--r-- | jstests/auth/applyOps_privilege.js | 92 | ||||
-rw-r--r-- | jstests/auth/lib/commands_lib.js | 67 | ||||
-rw-r--r-- | src/mongo/db/auth/action_types.txt | 1 | ||||
-rw-r--r-- | src/mongo/db/auth/role_graph_builtin_roles.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/commands/oplog_application_checks.cpp | 5 |
5 files changed, 134 insertions, 33 deletions
diff --git a/jstests/auth/applyOps_privilege.js b/jstests/auth/applyOps_privilege.js new file mode 100644 index 00000000000..26423c52b24 --- /dev/null +++ b/jstests/auth/applyOps_privilege.js @@ -0,0 +1,92 @@ +// Tests that a user can only run a applyops while having applyOps privilege. +(function() { +"use strict"; + +// Special privilege required to run applyOps command. +// Role dbAdminAnyDatabase has this privilege. +const applyOps_priv = { + resource: {cluster: true}, + actions: ["applyOps"] +}; + +const testUser = "testUser"; +const testUserWithDbAdminAnyDatabaseRole = "testUserWithDbAdminAnyDatabaseRole"; +const testRole = "testRole"; +const testDBName = "test_applyOps_auth"; +const adminDbName = "admin"; +const authErrCode = 13; + +const command = { + applyOps: [{ + "op": "c", + "ns": testDBName + ".$cmd", + "o": { + "create": "x", + } + }] +}; + +function createUsers(conn) { + let adminDb = conn.getDB(adminDbName); + // Create the admin user. + assert.commandWorked( + adminDb.runCommand({createUser: "admin", pwd: "password", roles: ["__system"]})); + + assert(adminDb.auth("admin", "password")); + assert.commandWorked(adminDb.runCommand({createRole: testRole, privileges: [], roles: []})); + + let testDb = adminDb.getSiblingDB(testDBName); + assert.commandWorked(testDb.runCommand( + {createUser: testUser, pwd: "password", roles: [{role: testRole, db: adminDbName}]})); + + assert.commandWorked(testDb.runCommand({ + createUser: testUserWithDbAdminAnyDatabaseRole, + pwd: "password", + roles: [{role: "dbAdminAnyDatabase", db: adminDbName}, {role: testRole, db: adminDbName}] + })); + + adminDb.logout(); +} + +function testAuthorization(conn, privileges, user, shouldSucceed) { + let testDb = conn.getDB(testDBName); + let adminDb = conn.getDB(adminDbName); + + assert(adminDb.auth("admin", "password")); + assert.commandWorked(adminDb.runCommand({updateRole: testRole, privileges: privileges})); + adminDb.logout(); + + assert(testDb.auth(user, "password")); + if (shouldSucceed) { + assert.commandWorked(testDb.runCommand(command)); + } else { + var res = testDb.runCommand(command); + if (res.ok == 1 || res.code != authErrCode) { + let msg = "expected authorization failure " + + " but received " + tojson(res) + " with privileges " + tojson(privileges); + assert(false, msg); + } + } + + testDb.logout(); +} + +function runTest(conn) { + createUsers(conn); + let privileges = [{resource: {db: testDBName, collection: "x"}, actions: ["createCollection"]}]; + + // Test applyOps failed without applyOps privilege or dbAdminAnyDatabase role. + testAuthorization(conn, privileges, testUser, false); + + // Test applyOps succeed with applyOps privilege. + testAuthorization(conn, privileges.concat(applyOps_priv), testUser, true); + + // Test applyOps succeed with dbAdminAnyDatabase role. + testAuthorization(conn, privileges, testUserWithDbAdminAnyDatabaseRole, true); +} + +// Run the test on a standalone. +let conn = MongoRunner.runMongod({auth: ""}); +runTest(conn); +MongoRunner.stopMongod(conn); +}()); diff --git a/jstests/auth/lib/commands_lib.js b/jstests/auth/lib/commands_lib.js index eb091728118..8f872c547aa 100644 --- a/jstests/auth/lib/commands_lib.js +++ b/jstests/auth/lib/commands_lib.js @@ -297,6 +297,7 @@ var authCommandsLib = { actions: ["appendOplogNote"], removeWhenTestingAuthzFailure: false }, + {resource: {cluster: true}, actions: ["applyOps"]}, ], }, ] @@ -321,17 +322,13 @@ var authCommandsLib = { { runOnDb: adminDbName, roles: { - readWrite: 1, - dbAdmin: 1, - dbOwner: 1, - readWriteAnyDatabase: 1, dbAdminAnyDatabase: 1, root: 1, - restore: 1, __system: 1 }, privileges: [ {resource: {db: firstDbName, collection: "x"}, actions: ["createCollection"]}, + {resource: {cluster: true}, actions: ["applyOps"]}, ] }, ] @@ -357,10 +354,10 @@ var authCommandsLib = { testcases: [ { runOnDb: adminDbName, - roles: {root: 1, restore: 1, __system: 1}, + roles: {__system: 1, root: 1}, privileges: [ {resource: {db: firstDbName, collection: "x"}, actions: ["createCollection"]}, - {resource: {cluster: true}, actions: ["useUUID", "forceUUID"]} + {resource: {cluster: true}, actions: ["useUUID", "forceUUID", "applyOps"]}, ] }, ] @@ -388,7 +385,7 @@ var authCommandsLib = { privileges: [ {resource: {db: firstDbName, collection: "x"}, actions: ["createCollection"]}, // Do not have forceUUID. - {resource: {cluster: true}, actions: ["useUUID"]} + {resource: {cluster: true}, actions: ["useUUID", "applyOps"]}, ] }] }, @@ -414,17 +411,13 @@ var authCommandsLib = { { runOnDb: adminDbName, roles: { - readWrite: 1, - dbAdmin: 1, - dbOwner: 1, - readWriteAnyDatabase: 1, dbAdminAnyDatabase: 1, root: 1, - restore: 1, __system: 1 }, privileges: [ {resource: {db: firstDbName, collection: "x"}, actions: ["dropCollection"]}, + {resource: {cluster: true}, actions: ["applyOps"]}, ] }, ] @@ -459,10 +452,10 @@ var authCommandsLib = { testcases: [ { runOnDb: adminDbName, - roles: {root: 1, restore: 1, __system: 1}, + roles: {__system: 1, root: 1}, privileges: [ {resource: {db: firstDbName, collection: "x"}, actions: ["dropCollection"]}, - {resource: {cluster: true}, actions: ["useUUID"]} + {resource: {cluster: true}, actions: ["useUUID", "applyOps"]}, ] }, ] @@ -499,7 +492,8 @@ var authCommandsLib = { expectAuthzFailure: true, runOnDb: adminDbName, privileges: [ - {resource: {db: firstDbName, collection: "x"}, actions: ["dropCollection"]} + {resource: {db: firstDbName, collection: "x"}, actions: ["dropCollection"]}, + {resource: {cluster: true}, actions: ["applyOps"]}, // don't have useUUID privilege. ] }, @@ -513,13 +507,13 @@ var authCommandsLib = { { runOnDb: adminDbName, privileges: [ - {resource: {cluster: true}, actions: ["appendOplogNote"]}, + {resource: {cluster: true}, actions: ["appendOplogNote", "applyOps"]}, ], }, { runOnDb: firstDbName, privileges: [ - {resource: {cluster: true}, actions: ["appendOplogNote"]}, + {resource: {cluster: true}, actions: ["appendOplogNote", "applyOps"]}, ], expectFailure: true } @@ -552,7 +546,7 @@ var authCommandsLib = { testcases: [ { runOnDb: adminDbName, - roles: {readWriteAnyDatabase: 1, root: 1, __system: 1}, + roles: {__system: 1, root: 1}, privileges: [ { resource: {db: firstDbName, collection: "x"}, @@ -561,7 +555,8 @@ var authCommandsLib = { { resource: {db: secondDbName, collection: "y"}, actions: ["insert", "createIndex"] - } + }, + {resource: {cluster: true}, actions: ["applyOps"]}, ] }, ] @@ -585,9 +580,10 @@ var authCommandsLib = { testcases: [ { runOnDb: adminDbName, - roles: roles_write, + roles: {__system: 1, root: 1}, privileges: [ {resource: {db: firstDbName, collection: "x"}, actions: ["insert"]}, + {resource: {cluster: true}, actions: ["applyOps"]}, ], }, ] @@ -620,10 +616,10 @@ var authCommandsLib = { testcases: [ { runOnDb: adminDbName, - roles: {root: 1, restore: 1, __system: 1}, + roles: {__system: 1, root: 1}, privileges: [ {resource: {db: firstDbName, collection: "x"}, actions: ["insert"]}, - {resource: {cluster: true}, actions: ["useUUID"]} + {resource: {cluster: true}, actions: ["useUUID", "applyOps"]}, ], }, ] @@ -660,10 +656,10 @@ var authCommandsLib = { // failure. expectFail: true, runOnDb: adminDbName, - roles: {root: 1, restore: 1, __system: 1}, + roles: {__system: 1, root: 1}, privileges: [ {resource: {db: firstDbName, collection: "x"}, actions: ["insert"]}, - {resource: {cluster: true}, actions: ["useUUID"]} + {resource: {cluster: true}, actions: ["useUUID", "applyOps"]}, ], }, ] @@ -699,6 +695,7 @@ var authCommandsLib = { runOnDb: adminDbName, privileges: [ {resource: {db: firstDbName, collection: "x"}, actions: ["insert"]}, + {resource: {cluster: true}, actions: ["applyOps"]}, // Don't have useUUID privilege. ], }, @@ -740,7 +737,7 @@ var authCommandsLib = { runOnDb: adminDbName, privileges: [ {resource: {db: firstDbName, collection: "x"}, actions: ["insert"]}, - {resource: {cluster: true}, actions: ["useUUID", "forceUUID"]} + {resource: {cluster: true}, actions: ["useUUID", "forceUUID", "applyOps"]}, // Require universal privilege set. ], }, @@ -780,7 +777,7 @@ var authCommandsLib = { actions: ["createCollection", "insert"] }, {resource: {db: firstDbName, collection: "y"}, actions: ["createCollection"]}, - {resource: {cluster: true}, actions: ["useUUID", "forceUUID"]} + {resource: {cluster: true}, actions: ["useUUID", "forceUUID", "applyOps"]}, ], }, ] @@ -820,7 +817,7 @@ var authCommandsLib = { resource: {db: firstDbName, collection: "y"}, actions: ["createCollection", "insert"] }, - {resource: {cluster: true}, actions: ["useUUID", "forceUUID"]} + {resource: {cluster: true}, actions: ["useUUID", "forceUUID", "applyOps"]}, ], }, ] @@ -845,9 +842,10 @@ var authCommandsLib = { testcases: [ { runOnDb: adminDbName, - roles: Object.merge(roles_write, {restore: 0}, true), + roles: {__system: 1, root: 1}, privileges: [ {resource: {db: firstDbName, collection: "x"}, actions: ["update", "insert"]}, + {resource: {cluster: true}, actions: ["applyOps"]}, ], }, ] @@ -873,9 +871,10 @@ var authCommandsLib = { testcases: [ { runOnDb: adminDbName, - roles: Object.merge(roles_write, {restore: 0}, true), + roles: {__system: 1, root: 1}, privileges: [ {resource: {db: firstDbName, collection: "x"}, actions: ["update"]}, + {resource: {cluster: true}, actions: ["applyOps"]}, ], }, ] @@ -910,10 +909,10 @@ var authCommandsLib = { testcases: [ { runOnDb: adminDbName, - roles: {root: 1, __system: 1}, + roles: {__system: 1, root: 1}, privileges: [ {resource: {db: firstDbName, collection: "x"}, actions: ["update"]}, - {resource: {cluster: true}, actions: ["useUUID"]} + {resource: {cluster: true}, actions: ["useUUID", "applyOps"]}, ], }, ] @@ -951,6 +950,7 @@ var authCommandsLib = { runOnDb: adminDbName, privileges: [ {resource: {db: firstDbName, collection: "x"}, actions: ["update"]}, + {resource: {cluster: true}, actions: ["applyOps"]}, ], }, ] @@ -968,9 +968,10 @@ var authCommandsLib = { testcases: [ { runOnDb: adminDbName, - roles: Object.merge(roles_write, {restore: 0}, true), + roles: {__system: 1, root: 1}, privileges: [ {resource: {db: firstDbName, collection: "x"}, actions: ["remove"]}, + {resource: {cluster: true}, actions: ["applyOps"]}, ], }, ] diff --git a/src/mongo/db/auth/action_types.txt b/src/mongo/db/auth/action_types.txt index 71582f7f85b..4ecac2e6dbe 100644 --- a/src/mongo/db/auth/action_types.txt +++ b/src/mongo/db/auth/action_types.txt @@ -9,6 +9,7 @@ "anyAction", # Special ActionType that represents *all* actions "appendOplogNote", "applicationMessage", +"applyOps", "auditLogRotate", # Not used for permissions checks, but to id the event in logs. "authCheck", # Not used for permissions checks, but to id the authorization-checking event in logs. "authenticate", # Not used for permission checks, but to id authentication events in logs. diff --git a/src/mongo/db/auth/role_graph_builtin_roles.cpp b/src/mongo/db/auth/role_graph_builtin_roles.cpp index 783be516d4c..1469c0eb162 100644 --- a/src/mongo/db/auth/role_graph_builtin_roles.cpp +++ b/src/mongo/db/auth/role_graph_builtin_roles.cpp @@ -393,6 +393,8 @@ void addDbAdminAnyDbPrivileges(PrivilegeVector* privileges) { Privilege::addPrivilegeToPrivilegeVector( privileges, Privilege(ResourcePattern::forCollectionName("system.profile"), profileActions)); + Privilege::addPrivilegeToPrivilegeVector( + privileges, Privilege(ResourcePattern::forClusterResource(), ActionType::applyOps)); } void addClusterMonitorPrivileges(PrivilegeVector* privileges) { diff --git a/src/mongo/db/commands/oplog_application_checks.cpp b/src/mongo/db/commands/oplog_application_checks.cpp index 783d1002b35..1980f710f6f 100644 --- a/src/mongo/db/commands/oplog_application_checks.cpp +++ b/src/mongo/db/commands/oplog_application_checks.cpp @@ -204,6 +204,11 @@ Status OplogApplicationChecks::checkAuthForCommand(OperationContext* opCtx, const BSONObj& cmdObj, OplogApplicationValidity validity) { AuthorizationSession* authSession = AuthorizationSession::get(opCtx->getClient()); + if (!authSession->isAuthorizedForActionsOnResource(ResourcePattern::forClusterResource(), + ActionType::applyOps)) { + return Status(ErrorCodes::Unauthorized, "Unauthorized"); + } + if (validity == OplogApplicationValidity::kNeedsSuperuser) { std::vector<Privilege> universalPrivileges; RoleGraph::generateUniversalPrivileges(&universalPrivileges); |