Permalink
Browse files

some security refactoring

localhost connections sometimes have special priviliges in particular if there are no admin.system.user documents
this now makes that check be at connection opening instead of later
that way hopefully it is easier to do the locking
this is bad in the sense that the test result then "sticks" for the connection -- but we are sticking auth
anyway so that can be revisited when fixed in total.
the old code i think was doing a query every single time on that code path?  this won't.
please review.
let me know what this breaks, switching locations so going to push now.
SERVER-4328
  • Loading branch information...
1 parent f631d11 commit 6c05a8e9158f629b612f7e3ba2372bce48c9c3e5 @dwight dwight committed Mar 26, 2012
View
@@ -168,7 +168,8 @@ namespace mongo {
public:
virtual void connected( AbstractMessagingPort* p ) {
Client& c = Client::initThread("conn", p);
- c.getAuthenticationInfo()->isLocalHost = p->remote().isLocalHost();
+ if( p->remote().isLocalHost() )
+ c.getAuthenticationInfo()->setIsALocalHostConnectionWithSpecialAuthPowers();
}
virtual void process( Message& m , AbstractMessagingPort* port , LastError * le) {
@@ -1600,7 +1600,7 @@ namespace mongo {
virtual bool run(const string& dbname, BSONObj& cmdObj, int, string& errmsg, BSONObjBuilder& result, bool) {
AuthenticationInfo *ai = cc().getAuthenticationInfo();
- if ( ! ai->isLocalHost ) {
+ if ( ! ai->isLocalHost() ) {
errmsg = "godinsert only works locally";
return false;
}
@@ -1824,7 +1824,7 @@ namespace mongo {
AuthenticationInfo *ai = client.getAuthenticationInfo();
- if( c->adminOnly() && c->localHostOnlyIfNoAuth( cmdObj ) && noauth && !ai->isLocalHost ) {
+ if( c->adminOnly() && c->localHostOnlyIfNoAuth( cmdObj ) && noauth && !ai->isLocalHost() ) {
result.append( "errmsg" ,
"unauthorized: this command must run from localhost when running db without auth" );
log() << "command denied: " << cmdObj.toString() << endl;
View
@@ -54,8 +54,13 @@ namespace mongo {
bool AuthenticationInfo::_isAuthorizedSpecialChecks( const string& dbname ) const {
if ( cc().isGod() )
return true;
+ return _isLocalHostAndLocalHostIsAuthorizedForAll;
+ }
- if ( isLocalHost ) {
+ void AuthenticationInfo::setIsALocalHostConnectionWithSpecialAuthPowers() {
+ verify(!_isLocalHost);
+ _isLocalHost = true;
+ {
Client::GodScope gs;
Client::ReadContext ctx("admin.system.users");
BSONObj result;
@@ -65,11 +70,9 @@ namespace mongo {
_warned = true;
log() << "note: no users configured in admin.system.users, allowing localhost access" << endl;
}
- return true;
+ _isLocalHostAndLocalHostIsAuthorizedForAll = true;
}
}
-
- return false;
}
bool CmdAuthenticate::getUserObj(const string& dbname, const string& user, BSONObj& userObj, string& pwd) {
View
@@ -30,11 +30,16 @@ namespace mongo {
/** An AuthenticationInfo object is present within every mongo::Client object */
class AuthenticationInfo : boost::noncopyable {
+ bool _isLocalHost;
+ bool _isLocalHostAndLocalHostIsAuthorizedForAll;
public:
- bool isLocalHost;
-
- AuthenticationInfo(){ isLocalHost = false; }
+ void setIsALocalHostConnectionWithSpecialAuthPowers(); // called, if localhost, when conneciton established.
+ AuthenticationInfo() {
+ _isLocalHost = false;
+ _isLocalHostAndLocalHostIsAuthorizedForAll = false;
+ }
~AuthenticationInfo() {}
+ bool isLocalHost() const { return _isLocalHost; } // why are you calling this? makes no sense to be externalized
// -- modifiers ----
@@ -92,6 +97,7 @@ namespace mongo {
// it too thus we need this
mutable SpinLock _lock;
+ // todo: caching should not last forever
typedef map<string,Auth> MA;
MA _dbs; // dbname -> auth
@@ -33,7 +33,9 @@
namespace mongo {
+ // this is a config setting, set at startup and not changing after initialization.
bool noauth = true;
+
AuthInfo internalSecurity;
bool setUpSecurityKey(const string& filename) {
@@ -129,12 +129,14 @@ namespace QueryTests {
class FindOneEmptyObj : public Base {
public:
void run() {
+ // todo: this is BAD.
+ cc().getAuthenticationInfo()->setIsALocalHostConnectionWithSpecialAuthPowers();
+
// We don't normally allow empty objects in the database, but test that we can find
// an empty object (one might be allowed inside a reserved namespace at some point).
Lock::GlobalWrite lk;
Client::Context ctx( "unittests.querytests" );
// Set up security so godinsert command can run.
- cc().getAuthenticationInfo()->isLocalHost = true;
DBDirectClient cl;
BSONObj info;
ASSERT( cl.runCommand( "unittests", BSON( "godinsert" << "querytests" << "obj" << BSONObj() ), info ) );
@@ -1555,7 +1555,7 @@ namespace mongo {
errmsg = "unauthorized";
anObjBuilder.append( "note" , str::stream() << "need to authorized on db: " << cl << " for command: " << e.fieldName() );
}
- else if( c->adminOnly() && c->localHostOnlyIfNoAuth( jsobj ) && noauth && !ai->isLocalHost ) {
+ else if( c->adminOnly() && c->localHostOnlyIfNoAuth( jsobj ) && noauth && !ai->isLocalHost() ) {
ok = false;
errmsg = "unauthorized: this command must run from localhost when running db without auth";
log() << "command denied: " << jsobj.toString() << endl;
View
@@ -63,8 +63,13 @@ namespace mongo {
return true;
}
+ void AuthenticationInfo::setIsALocalHostConnectionWithSpecialAuthPowers() {
+ verify(!_isLocalHost);
+ _isLocalHost = true;
+ }
+
bool AuthenticationInfo::_isAuthorizedSpecialChecks( const string& dbname ) const {
- if ( !isLocalHost ) {
+ if ( !_isLocalHost ) {
return false;
}
View
@@ -86,7 +86,8 @@ namespace mongo {
virtual void connected( AbstractMessagingPort* p ) {
ClientInfo *c = ClientInfo::get();
massert(15849, "client info not defined", c);
- c->getAuthenticationInfo()->isLocalHost = p->remote().isLocalHost();
+ if( p->remote().isLocalHost() )
+ c->getAuthenticationInfo()->setIsALocalHostConnectionWithSpecialAuthPowers();
}
virtual void process( Message& m , AbstractMessagingPort* p , LastError * le) {

0 comments on commit 6c05a8e

Please sign in to comment.