From 28c49b3882ccca72971270fcebc438c593aa6ede Mon Sep 17 00:00:00 2001 From: Spencer T Brody Date: Fri, 1 Mar 2013 14:01:34 -0500 Subject: SERVER-8802 SERVER-8814 Don't build indexes from inserts into collection names ending in ".system.indexes" --- jstests/auth/indexSystemUsers.js | 36 ++++++++++++++++++++++++++++++ jstests/indexOtherNamespace.js | 20 +++++++++++++++++ src/mongo/client/syncclusterconnection.cpp | 6 +++-- src/mongo/db/cloner.cpp | 3 ++- src/mongo/db/oplog.cpp | 4 ++-- src/mongo/db/pdfile.cpp | 9 +++++--- src/mongo/db/repl/rs_sync.cpp | 3 ++- src/mongo/s/strategy_shard.cpp | 2 +- src/mongo/tools/dump.cpp | 3 ++- src/mongo/tools/restore.cpp | 2 +- 10 files changed, 76 insertions(+), 12 deletions(-) create mode 100644 jstests/auth/indexSystemUsers.js create mode 100644 jstests/indexOtherNamespace.js diff --git a/jstests/auth/indexSystemUsers.js b/jstests/auth/indexSystemUsers.js new file mode 100644 index 00000000000..0e26cc6ff76 --- /dev/null +++ b/jstests/auth/indexSystemUsers.js @@ -0,0 +1,36 @@ +// SERVER-8802: Test that you can't build indexes on system.users and use that to drop users with +// dropDups. +var conn = MongoRunner.runMongod({auth : ""}); + +function assertGLENotOK(status) { + assert(status.ok && status.err !== null, + "Expected not-OK status object; found " + tojson(status)); +} + +var adminDB = conn.getDB("admin"); +var testDB = conn.getDB("test"); +adminDB.addUser({user:'admin', pwd:'x', roles:['userAdminAnyDatabase']}); +adminDB.auth('admin','x'); +adminDB.addUser({user:'mallory', pwd:'x', roles:[], otherDBRoles:{test:['readWrite']}}); +testDB.addUser({user:'user1', pwd:'x', roles:['read']}); +testDB.addUser({user:'user2', pwd:'x', roles:['read']}); +assert.eq(2, testDB.system.users.count()); +adminDB.logout(); + +adminDB.auth('mallory', 'x'); +testDB.system.users.createIndex({haxx:1}, {unique:true, dropDups:true}); +assertGLENotOK(testDB.getLastErrorObj()); +testDB.exploit.system.indexes.insert({ns: "test.system.users", key: { haxx: 1.0 }, name: "haxx_1", + unique: true, dropDups: true}); +assertGLENotOK(testDB.getLastErrorObj()); +// Make sure that no indexes were built. +assert.eq(null, + testDB.system.namespaces.findOne( + {$and : [{name : /^test\.system\.users\.\$/}, + {name : {$ne : "test.system.users.$_id_"}}, + {name : {$ne : "test.system.users.$user_1_userSource_1"}} ]})); +adminDB.logout(); + +adminDB.auth('admin','x'); +// Make sure that no users were actually dropped +assert.eq(2, testDB.system.users.count()); \ No newline at end of file diff --git a/jstests/indexOtherNamespace.js b/jstests/indexOtherNamespace.js new file mode 100644 index 00000000000..9410e281347 --- /dev/null +++ b/jstests/indexOtherNamespace.js @@ -0,0 +1,20 @@ +// SERVER-8814: Test that only the system.indexes namespace can be used to build indexes. + +function assertGLENotOK(status) { + assert(status.ok && status.err !== null, + "Expected not-OK status object; found " + tojson(status)); +} + +db = db.getSiblingDB("indexOtherNS"); +db.dropDatabase(); + +db.foo.insert({a:1}) +assert.eq(1, db.system.indexes.count()); +assert.eq("BasicCursor", db.foo.find({a:1}).explain().cursor); + +db.randomNS.system.indexes.insert({ns:"indexOtherNS.foo", key:{a:1}, name:"a_1"}); +assertGLENotOK(db.getLastErrorObj()); +// Assert that index didn't actually get built +assert.eq(1, db.system.indexes.count()); +assert.eq(null, db.system.namespaces.findOne({name : "indexOtherNS.foo.$a_1"})); +assert.eq("BasicCursor", db.foo.find({a:1}).explain().cursor); diff --git a/src/mongo/client/syncclusterconnection.cpp b/src/mongo/client/syncclusterconnection.cpp index 833ac5a0469..3183f337203 100644 --- a/src/mongo/client/syncclusterconnection.cpp +++ b/src/mongo/client/syncclusterconnection.cpp @@ -23,6 +23,7 @@ #include "mongo/client/dbclientcursor.h" #include "mongo/client/dbclientinterface.h" #include "mongo/db/dbmessage.h" +#include "mongo/db/namespacestring.h" // error codes 8000-8009 @@ -314,8 +315,9 @@ namespace mongo { void SyncClusterConnection::insert( const string &ns, BSONObj obj , int flags) { - uassert( 13119 , (string)"SyncClusterConnection::insert obj has to have an _id: " + obj.jsonString() , - ns.find( ".system.indexes" ) != string::npos || obj["_id"].type() ); + uassert(13119, + (string)"SyncClusterConnection::insert obj has to have an _id: " + obj.jsonString(), + NamespaceString(ns).coll == "system.indexes" || obj["_id"].type()); string errmsg; if ( ! prepare( errmsg ) ) diff --git a/src/mongo/db/cloner.cpp b/src/mongo/db/cloner.cpp index 4ea52d02a39..26eb5baa639 100644 --- a/src/mongo/db/cloner.cpp +++ b/src/mongo/db/cloner.cpp @@ -28,6 +28,7 @@ #include "mongo/db/instance.h" #include "mongo/db/jsobj.h" #include "mongo/db/kill_current_op.h" +#include "mongo/db/namespacestring.h" #include "mongo/db/pdfile.h" #include "mongo/db/repl.h" #include "mongo/db/sort_phase_one.h" @@ -138,7 +139,7 @@ namespace mongo { BSONObj js = tmp; if ( isindex ) { - verify( strstr(from_collection, "system.indexes") ); + verify(NamespaceString(from_collection).coll == "system.indexes"); js = fixindex(tmp); storedForLater->push_back( js.getOwned() ); continue; diff --git a/src/mongo/db/oplog.cpp b/src/mongo/db/oplog.cpp index c16d0e9f6d1..e1e6e370d4f 100644 --- a/src/mongo/db/oplog.cpp +++ b/src/mongo/db/oplog.cpp @@ -28,6 +28,7 @@ #include "mongo/db/commands.h" #include "mongo/db/index_update.h" #include "mongo/db/instance.h" +#include "mongo/db/namespacestring.h" #include "mongo/db/ops/update.h" #include "mongo/db/ops/delete.h" #include "mongo/db/queryoptimizer.h" @@ -764,8 +765,7 @@ namespace mongo { if ( *opType == 'i' ) { opCounters->gotInsert(); - const char *p = strchr(ns, '.'); - if ( p && strcmp(p, ".system.indexes") == 0 ) { + if (NamespaceString(ns).coll == "system.indexes") { // updates aren't allowed for indexes -- so we will do a regular insert. if index already // exists, that is ok. theDataFileMgr.insert(ns, (void*) o.objdata(), o.objsize()); diff --git a/src/mongo/db/pdfile.cpp b/src/mongo/db/pdfile.cpp index 16cd029d5bc..928b8560ced 100644 --- a/src/mongo/db/pdfile.cpp +++ b/src/mongo/db/pdfile.cpp @@ -49,6 +49,7 @@ _ disallow system* manipulations from the database. #include "mongo/db/lasterror.h" #include "mongo/db/memconcept.h" #include "mongo/db/namespace-inl.h" +#include "mongo/db/namespacestring.h" #include "mongo/db/ops/delete.h" #include "mongo/db/repl.h" #include "mongo/db/replutil.h" @@ -1076,7 +1077,7 @@ namespace mongo { s->nrecords--; } - if ( strstr(ns, ".system.indexes") ) { + if (NamespaceString(ns).coll == "system.indexes") { /* temp: if in system.indexes, don't reuse, and zero out: we want to be careful until validated more, as IndexDetails has pointers to this disk location. so an incorrectly done remove would cause @@ -1408,7 +1409,7 @@ namespace mongo { uassert( 10095 , "attempt to insert in reserved database name 'system'", sys != ns); if ( strstr(ns, ".system.") ) { // later:check for dba-type permissions here if have that at some point separate - if ( strstr(ns, ".system.indexes" ) ) + if (NamespaceString(ns).coll == "system.indexes") wouldAddIndex = true; else if ( legalClientSystemNS( ns , true ) ) { if ( obuf && strstr( ns , ".system.users" ) ) { @@ -1447,7 +1448,9 @@ namespace mongo { const string& tabletoidxns, const DiskLoc& loc, bool mayInterrupt) { - uassert( 13143 , "can't create index on system.indexes" , tabletoidxns.find( ".system.indexes" ) == string::npos ); + uassert(13143, + "can't create index on system.indexes", + NamespaceString(tabletoidxns).coll != "system.indexes"); BSONObj info = loc.obj(); bool background = info["background"].trueValue(); diff --git a/src/mongo/db/repl/rs_sync.cpp b/src/mongo/db/repl/rs_sync.cpp index 3a0f9ee9852..07d2636803d 100644 --- a/src/mongo/db/repl/rs_sync.cpp +++ b/src/mongo/db/repl/rs_sync.cpp @@ -25,6 +25,7 @@ #include "mongo/db/client.h" #include "mongo/db/commands/fsync.h" #include "mongo/db/d_concurrency.h" +#include "mongo/db/namespacestring.h" #include "mongo/db/prefetch.h" #include "mongo/db/repl.h" #include "mongo/db/repl/bgsync.h" @@ -480,7 +481,7 @@ namespace replset { if ((op["op"].valuestrsafe()[0] == 'c') || // Index builds are acheived through the use of an insert op, not a command op. // The following line is the same as what the insert code uses to detect an index build. - (strstr(op["ns"].valuestrsafe(), ".system.indexes"))) { + (NamespaceString(op["ns"].valuestrsafe()).coll == "system.indexes")) { if (ops->empty()) { // apply commands one-at-a-time ops->push_back(op); diff --git a/src/mongo/s/strategy_shard.cpp b/src/mongo/s/strategy_shard.cpp index 25b2bab21f7..2f8667de6d8 100644 --- a/src/mongo/s/strategy_shard.cpp +++ b/src/mongo/s/strategy_shard.cpp @@ -44,7 +44,7 @@ namespace mongo { class ShardStrategy : public Strategy { bool _isSystemIndexes( const char* ns ) { - return strstr( ns , ".system.indexes" ) == strchr( ns , '.' ) && strchr( ns , '.' ); + return NamespaceString(ns).coll == "system.indexes"; } virtual void queryOp( Request& r ) { diff --git a/src/mongo/tools/dump.cpp b/src/mongo/tools/dump.cpp index 3ee4dcfcf55..93d0446c3cd 100644 --- a/src/mongo/tools/dump.cpp +++ b/src/mongo/tools/dump.cpp @@ -28,6 +28,7 @@ #include "mongo/base/initializer.h" #include "mongo/client/dbclientcursor.h" #include "mongo/db/db.h" +#include "mongo/db/namespacestring.h" #include "mongo/tools/tool.h" using namespace mongo; @@ -219,7 +220,7 @@ public: continue; } - if ( endsWith(name.c_str(), ".system.indexes") ) { + if (NamespaceString(name).coll == "system.indexes") { // Create system.indexes.bson for compatibility with pre 2.2 mongorestore const string filename = name.substr( db.size() + 1 ); writeCollectionFile( name.c_str() , outdir / ( filename + ".bson" ) ); diff --git a/src/mongo/tools/restore.cpp b/src/mongo/tools/restore.cpp index e1f7d29c3f3..35daa4c6f4d 100644 --- a/src/mongo/tools/restore.cpp +++ b/src/mongo/tools/restore.cpp @@ -426,7 +426,7 @@ public: } } } - else if ( endsWith( _curns.c_str() , ".system.indexes" )) { + else if (NamespaceString(_curns).coll == "system.indexes") { createIndex(obj, true); } else if (_drop && endsWith(_curns.c_str(), ".system.users") && _users.count(obj["user"].String())) { -- cgit v1.2.1