diff options
author | Shreyas Kalyan <shreyas.kalyan@10gen.com> | 2019-03-01 16:35:26 -0500 |
---|---|---|
committer | Shreyas Kalyan <shreyas.kalyan@10gen.com> | 2019-03-15 16:07:25 -0400 |
commit | 4ea844780f42c090841ceb61a0180171a7d5442e (patch) | |
tree | 296a7ad9d68f8be117a80bac1e06c5fcf26c827d | |
parent | 477ceade3dae70498c0a6b4c89007ac98934e1c6 (diff) | |
download | mongo-4ea844780f42c090841ceb61a0180171a7d5442e.tar.gz |
SERVER-39056 Further refine readWriteAnyDatabase
(cherry picked from commit f35415816415b9ff93bb963690ac4f8c9f6bc453)
-rw-r--r-- | jstests/auth/lib/commands_lib.js | 18 | ||||
-rw-r--r-- | jstests/auth/renameRestrictedCollections.js | 110 | ||||
-rw-r--r-- | jstests/auth/renameSystemCollections.js | 78 | ||||
-rw-r--r-- | src/mongo/db/auth/authorization_session.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/auth/role_graph_builtin_roles.cpp | 4 |
5 files changed, 135 insertions, 79 deletions
diff --git a/jstests/auth/lib/commands_lib.js b/jstests/auth/lib/commands_lib.js index 97be3e19890..27b63641f90 100644 --- a/jstests/auth/lib/commands_lib.js +++ b/jstests/auth/lib/commands_lib.js @@ -3150,6 +3150,24 @@ var authCommandsLib = { ] }, { + testname: "dbstats_on_local", + command: {dbStats: 1}, + skipSharded: true, + testcases: [ + { + runOnDb: "local", + roles: { + "readLocal": 1, + "readWriteLocal": 1, + "clusterAdmin": 1, + "clusterMonitor": 1, + "root": 1, + "__system": 1, + }, + }, + ] + }, + { testname: "find_config_changelog", command: {find: "changelog"}, testcases: [ diff --git a/jstests/auth/renameRestrictedCollections.js b/jstests/auth/renameRestrictedCollections.js new file mode 100644 index 00000000000..23a0ebc86e9 --- /dev/null +++ b/jstests/auth/renameRestrictedCollections.js @@ -0,0 +1,110 @@ +(function() { + 'use strict'; + + // SERVER-8623: Test that renameCollection can't be used to bypass auth checks on system + // namespaces + const conn = MongoRunner.runMongod({auth: ""}); + + const adminDB = conn.getDB("admin"); + const configDB = conn.getDB("config"); + const localDB = conn.getDB("local"); + const CodeUnauthorized = 13; + + const backdoorUserDoc = {user: 'backdoor', db: 'admin', pwd: 'hashed', roles: ['root']}; + + adminDB.createUser({user: 'userAdmin', pwd: 'password', roles: ['userAdminAnyDatabase']}); + + adminDB.auth('userAdmin', 'password'); + adminDB.createUser({user: 'readWriteAdmin', pwd: 'password', roles: ['readWriteAnyDatabase']}); + adminDB.createUser({ + user: 'readWriteAndUserAdmin', + pwd: 'password', + roles: ['readWriteAnyDatabase', 'userAdminAnyDatabase'] + }); + adminDB.createUser({user: 'root', pwd: 'password', roles: ['root']}); + adminDB.createUser({user: 'rootier', pwd: 'password', roles: ['__system']}); + adminDB.logout(); + + jsTestLog("Test that a readWrite user can't rename system.profile to something they can read"); + adminDB.auth('readWriteAdmin', 'password'); + var res = adminDB.system.profile.renameCollection("profile"); + assert.eq(0, res.ok); + assert.eq(CodeUnauthorized, res.code); + + jsTestLog("Test that a readWrite user can't rename system.users to something they can read"); + res = adminDB.system.users.renameCollection("users"); + assert.eq(0, res.ok); + assert.eq(CodeUnauthorized, res.code); + assert.eq(0, adminDB.users.count()); + + jsTestLog("Test that a readWrite user can't use renameCollection to override system.users"); + adminDB.users.insert(backdoorUserDoc); + res = adminDB.users.renameCollection("system.users", true); + assert.eq(0, res.ok); + assert.eq(CodeUnauthorized, res.code); + adminDB.users.drop(); + + jsTestLog("Test that a userAdmin can't rename system.users without readWrite"); + adminDB.logout(); + adminDB.auth('userAdmin', 'password'); + res = adminDB.system.users.renameCollection("users"); + assert.eq(0, res.ok); + assert.eq(CodeUnauthorized, res.code); + assert.eq(5, adminDB.system.users.count()); + + adminDB.auth('readWriteAndUserAdmin', 'password'); + assert.eq(0, adminDB.users.count()); + + jsTestLog("Test that even with userAdmin AND dbAdmin you CANNOT rename to/from system.users"); + res = adminDB.system.users.renameCollection("users"); + assert.eq(0, res.ok); + assert.eq(CodeUnauthorized, res.code); + assert.eq(5, adminDB.system.users.count()); + + adminDB.users.drop(); + adminDB.users.insert(backdoorUserDoc); + res = adminDB.users.renameCollection("system.users"); + assert.eq(0, res.ok); + assert.eq(CodeUnauthorized, res.code); + + assert.eq(null, adminDB.system.users.findOne({user: backdoorUserDoc.user})); + assert.neq(null, adminDB.system.users.findOne({user: 'userAdmin'})); + + adminDB.auth('rootier', 'password'); + + jsTestLog("Test that with __system you CAN rename to/from system.users"); + res = adminDB.system.users.renameCollection("users", true); + assert.eq(1, res.ok, tojson(res)); + + // Test permissions against the configDB and localDB + + // Start with test against inserting to and renaming collections in config and local + // as userAdminAnyDatabase. + assert.writeOK(configDB.test.insert({'a': 1})); + assert.commandWorked(configDB.test.renameCollection('test2')); + + assert.writeOK(localDB.test.insert({'a': 1})); + assert.commandWorked(localDB.test.renameCollection('test2')); + adminDB.createUser({user: 'readWriteAdmin', pwd: 'password', roles: ['readWriteAnyDatabase']}); + adminDB.logout(); + + // Test renaming collection in config with readWriteAnyDatabase + assert(adminDB.auth('readWriteAdmin', 'password')); + res = configDB.test2.insert({'b': 2}); + assert.writeError(res, 13, "not authorized on config to execute command"); + res = configDB.test2.renameCollection('test'); + assert.eq(0, res.ok); + assert.eq(CodeUnauthorized, res.code); + + // Test renaming collection in local with readWriteAnyDatabase + res = localDB.test2.insert({'b': 2}); + assert.writeError(res, 13, "not authorized on config to execute command"); + res = localDB.test2.renameCollection('test'); + assert.eq(0, res.ok); + assert.eq(CodeUnauthorized, res.code); + + // At this point, all the user documents are gone, so further activity may be unauthorized, + // depending on cluster configuration. So, this is the end of the test. + MongoRunner.stopMongod(conn, {user: 'userAdmin', pwd: 'password'}); + +})();
\ No newline at end of file diff --git a/jstests/auth/renameSystemCollections.js b/jstests/auth/renameSystemCollections.js deleted file mode 100644 index e97706bb6bf..00000000000 --- a/jstests/auth/renameSystemCollections.js +++ /dev/null @@ -1,78 +0,0 @@ -// SERVER-8623: Test that renameCollection can't be used to bypass auth checks on system namespaces -var conn = MongoRunner.runMongod({auth: ""}); - -var adminDB = conn.getDB("admin"); -var testDB = conn.getDB("testdb"); -var testDB2 = conn.getDB("testdb2"); - -var CodeUnauthorized = 13; - -var backdoorUserDoc = {user: 'backdoor', db: 'admin', pwd: 'hashed', roles: ['root']}; - -adminDB.createUser({user: 'userAdmin', pwd: 'password', roles: ['userAdminAnyDatabase']}); - -adminDB.auth('userAdmin', 'password'); -adminDB.createUser({user: 'readWriteAdmin', pwd: 'password', roles: ['readWriteAnyDatabase']}); -adminDB.createUser({ - user: 'readWriteAndUserAdmin', - pwd: 'password', - roles: ['readWriteAnyDatabase', 'userAdminAnyDatabase'] -}); -adminDB.createUser({user: 'root', pwd: 'password', roles: ['root']}); -adminDB.createUser({user: 'rootier', pwd: 'password', roles: ['__system']}); -adminDB.logout(); - -jsTestLog("Test that a readWrite user can't rename system.profile to something they can read"); -adminDB.auth('readWriteAdmin', 'password'); -res = adminDB.system.profile.renameCollection("profile"); -assert.eq(0, res.ok); -assert.eq(CodeUnauthorized, res.code); - -jsTestLog("Test that a readWrite user can't rename system.users to something they can read"); -var res = adminDB.system.users.renameCollection("users"); -assert.eq(0, res.ok); -assert.eq(CodeUnauthorized, res.code); -assert.eq(0, adminDB.users.count()); - -jsTestLog("Test that a readWrite user can't use renameCollection to override system.users"); -adminDB.users.insert(backdoorUserDoc); -res = adminDB.users.renameCollection("system.users", true); -assert.eq(0, res.ok); -assert.eq(CodeUnauthorized, res.code); -adminDB.users.drop(); - -jsTestLog("Test that a userAdmin can't rename system.users without readWrite"); -adminDB.logout(); -adminDB.auth('userAdmin', 'password'); -var res = adminDB.system.users.renameCollection("users"); -assert.eq(0, res.ok); -assert.eq(CodeUnauthorized, res.code); -assert.eq(5, adminDB.system.users.count()); - -adminDB.auth('readWriteAndUserAdmin', 'password'); -assert.eq(0, adminDB.users.count()); - -jsTestLog("Test that even with userAdmin AND dbAdmin you CANNOT rename to/from system.users"); -var res = adminDB.system.users.renameCollection("users"); -assert.eq(0, res.ok); -assert.eq(CodeUnauthorized, res.code); -assert.eq(5, adminDB.system.users.count()); - -adminDB.users.drop(); -adminDB.users.insert(backdoorUserDoc); -var res = adminDB.users.renameCollection("system.users"); -assert.eq(0, res.ok); -assert.eq(CodeUnauthorized, res.code); - -assert.eq(null, adminDB.system.users.findOne({user: backdoorUserDoc.user})); -assert.neq(null, adminDB.system.users.findOne({user: 'userAdmin'})); - -adminDB.auth('rootier', 'password'); - -jsTestLog("Test that with __system you CAN rename to/from system.users"); -var res = adminDB.system.users.renameCollection("users", true); -assert.eq(1, res.ok, tojson(res)); - -// At this point, all the user documents are gone, so further activity may be unauthorized, -// depending on cluster configuration. So, this is the end of the test. -MongoRunner.stopMongod(conn, {user: 'userAdmin', pwd: 'password'}); diff --git a/src/mongo/db/auth/authorization_session.cpp b/src/mongo/db/auth/authorization_session.cpp index 4b8b2aa52a0..7c9a8c73c54 100644 --- a/src/mongo/db/auth/authorization_session.cpp +++ b/src/mongo/db/auth/authorization_session.cpp @@ -779,7 +779,9 @@ static int buildResourceSearchList(const ResourcePattern& target, } resourceSearchList[size++] = ResourcePattern::forCollectionName(target.ns().coll()); } else if (target.isDatabasePattern()) { - resourceSearchList[size++] = ResourcePattern::forAnyNormalResource(); + if (target.ns().db() != "local" && target.ns().db() != "config") { + resourceSearchList[size++] = ResourcePattern::forAnyNormalResource(); + } } resourceSearchList[size++] = target; dassert(size <= resourceSearchListCapacity); diff --git a/src/mongo/db/auth/role_graph_builtin_roles.cpp b/src/mongo/db/auth/role_graph_builtin_roles.cpp index 9f572788b2a..28200596a1d 100644 --- a/src/mongo/db/auth/role_graph_builtin_roles.cpp +++ b/src/mongo/db/auth/role_graph_builtin_roles.cpp @@ -357,6 +357,10 @@ void addUserAdminAnyDbPrivileges(PrivilegeVector* privileges) { Privilege::addPrivilegeToPrivilegeVector( privileges, Privilege(ResourcePattern::forAnyNormalResource(), userAdminRoleActions)); Privilege::addPrivilegeToPrivilegeVector( + privileges, Privilege(ResourcePattern::forDatabaseName("local"), userAdminRoleActions)); + Privilege::addPrivilegeToPrivilegeVector( + privileges, Privilege(ResourcePattern::forDatabaseName("config"), userAdminRoleActions)); + Privilege::addPrivilegeToPrivilegeVector( privileges, Privilege(ResourcePattern::forClusterResource(), ActionType::listDatabases)); Privilege::addPrivilegeToPrivilegeVector( privileges, |