Author: chug Date: Wed Mar 5 02:33:46 2014 New Revision: 1574291 URL: http://svn.apache.org/r1574291 Log: QPID-5599: C++ Broker silently ignores --max-connections option when no ACL file is loaded
Simply installing a null and permissive rule file trips up the 'create link' security check. The security check from https://issues.apache.org/jira/browse/QPID-4631 reasons that if authentication is enabled and no ACL rule file is specified then interbroker links are denied. The check for 'ACL rule file is loaded' is simply the existence of the ACL object. That check is voided by always having an ACL object regardless of whether the ACL rule file was specified or not. One fix considered was adding an ACL rule "acl deny-log all create link" to the formerly null rule set when no ACL file is specified. This solution has too much complexity in several places and is too hard. The fix implemented here is a boolean flag indicating if the ACL rule set in force is specified by the user or not. Then the security check tests that the acl exists (always true) and that the rule set is specified by the user. Modified: qpid/trunk/qpid/cpp/src/qpid/acl/Acl.cpp qpid/trunk/qpid/cpp/src/qpid/acl/Acl.h qpid/trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp qpid/trunk/qpid/cpp/src/qpid/broker/AclModule.h qpid/trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp qpid/trunk/qpid/cpp/src/qpid/broker/amqp/Authorise.cpp Modified: qpid/trunk/qpid/cpp/src/qpid/acl/Acl.cpp URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/acl/Acl.cpp?rev=1574291&r1=1574290&r2=1574291&view=diff ============================================================================== --- qpid/trunk/qpid/cpp/src/qpid/acl/Acl.cpp (original) +++ qpid/trunk/qpid/cpp/src/qpid/acl/Acl.cpp Wed Mar 5 02:33:46 2014 @@ -55,7 +55,8 @@ namespace _qmf = qmf::org::apache::qpid: Acl::Acl (AclValues& av, Broker& b): aclValues(av), broker(&b), transferAcl(false), connectionCounter(new ConnectionCounter(*this, aclValues.aclMaxConnectPerUser, aclValues.aclMaxConnectPerIp, aclValues.aclMaxConnectTotal)), - resourceCounter(new ResourceCounter(*this, aclValues.aclMaxQueuesPerUser)){ + resourceCounter(new ResourceCounter(*this, aclValues.aclMaxQueuesPerUser)),userRules(true) +{ if (aclValues.aclMaxConnectPerUser > AclData::getConnectMaxSpec()) throw Exception("--connection-limit-per-user switch cannot be larger than " + AclData::getMaxConnectSpecStr()); @@ -86,6 +87,7 @@ Acl::Acl (AclValues& av, Broker& b): acl } } else { loadEmptyAclRuleset(); + userRules = false; QPID_LOG(debug, "ACL loaded empty rule set"); } broker->getConnectionObservers().add(connectionCounter); Modified: qpid/trunk/qpid/cpp/src/qpid/acl/Acl.h URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/acl/Acl.h?rev=1574291&r1=1574290&r2=1574291&view=diff ============================================================================== --- qpid/trunk/qpid/cpp/src/qpid/acl/Acl.h (original) +++ qpid/trunk/qpid/cpp/src/qpid/acl/Acl.h Wed Mar 5 02:33:46 2014 @@ -67,6 +67,7 @@ private: mutable qpid::sys::Mutex dataLock; boost::shared_ptr<ConnectionCounter> connectionCounter; boost::shared_ptr<ResourceCounter> resourceCounter; + bool userRules; public: Acl (AclValues& av, broker::Broker& b); @@ -85,6 +86,10 @@ public: return aclValues.aclMaxConnectTotal; }; + inline virtual bool userAclRules() { + return userRules; + }; + // create specilied authorise methods for cases that need faster matching as needed. virtual bool authorise( const std::string& id, Modified: qpid/trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp?rev=1574291&r1=1574290&r2=1574291&view=diff ============================================================================== --- qpid/trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp (original) +++ qpid/trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp Wed Mar 5 02:33:46 2014 @@ -62,20 +62,17 @@ struct AclPlugin : public Plugin { Options* getOptions() { return &options; } void init(broker::Broker& b) { - if (acl) throw Exception("ACL plugin cannot be initialized twice in one process."); + if (acl) throw Exception("ACL plugin cannot be initialized twice in one process."); - if (values.aclFile.empty()){ - QPID_LOG(info, "ACL Policy file not specified."); - } else { - sys::Path aclFile(values.aclFile); - sys::Path dataDir(b.getDataDir().getPath()); - if (!aclFile.isAbsolute() && !dataDir.empty()) - values.aclFile = (dataDir + aclFile).str(); - - acl = new Acl(values, b); - b.setAcl(acl.get()); - b.addFinalizer(boost::bind(&AclPlugin::shutdown, this)); - } + if (!values.aclFile.empty()){ + sys::Path aclFile(values.aclFile); + sys::Path dataDir(b.getDataDir().getPath()); + if (!aclFile.isAbsolute() && !dataDir.empty()) + values.aclFile = (dataDir + aclFile).str(); + } + acl = new Acl(values, b); + b.setAcl(acl.get()); + b.addFinalizer(boost::bind(&AclPlugin::shutdown, this)); } template <class T> bool init(Plugin::Target& target) { Modified: qpid/trunk/qpid/cpp/src/qpid/broker/AclModule.h URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/broker/AclModule.h?rev=1574291&r1=1574290&r2=1574291&view=diff ============================================================================== --- qpid/trunk/qpid/cpp/src/qpid/broker/AclModule.h (original) +++ qpid/trunk/qpid/cpp/src/qpid/broker/AclModule.h Wed Mar 5 02:33:46 2014 @@ -142,6 +142,8 @@ namespace broker { virtual uint16_t getMaxConnectTotal()=0; + virtual bool userAclRules()=0; + virtual bool authorise( const std::string& id, const acl::Action& action, Modified: qpid/trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp?rev=1574291&r1=1574290&r2=1574291&view=diff ============================================================================== --- qpid/trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp (original) +++ qpid/trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp Wed Mar 5 02:33:46 2014 @@ -198,7 +198,7 @@ void ConnectionHandler::Handler::startOk } if (connection.isFederationLink()) { AclModule* acl = connection.getBroker().getAcl(); - if (acl) { + if (acl && acl->userAclRules()) { if (!acl->authorise(connection.getUserId(),acl::ACT_CREATE,acl::OBJ_LINK,"")){ proxy.close(framing::connection::CLOSE_CODE_CONNECTION_FORCED, QPID_MSG("ACL denied " << connection.getUserId() Modified: qpid/trunk/qpid/cpp/src/qpid/broker/amqp/Authorise.cpp URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/broker/amqp/Authorise.cpp?rev=1574291&r1=1574290&r2=1574291&view=diff ============================================================================== --- qpid/trunk/qpid/cpp/src/qpid/broker/amqp/Authorise.cpp (original) +++ qpid/trunk/qpid/cpp/src/qpid/broker/amqp/Authorise.cpp Wed Mar 5 02:33:46 2014 @@ -121,7 +121,7 @@ void Authorise::route(boost::shared_ptr< void Authorise::interlink() { - if (acl) { + if (acl && acl->userAclRules()) { if (!acl->authorise(user, acl::ACT_CREATE, acl::OBJ_LINK, "")){ throw Exception(qpid::amqp::error_conditions::UNAUTHORIZED_ACCESS, QPID_MSG("ACL denied " << user << " a AMQP 1.0 link")); } --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org For additional commands, e-mail: commits-h...@qpid.apache.org
