QPID-4631: Lock down link creation using ACLqpid-4631

This commit makes link creation contingent on having an ACL file and then having an ACL rule approve the request. There is no longer a requirement for an explicit CREATE LINK rule; either 'allow all all' or 'deny all all' is sufficient.



git-svn-id: https://svn.apache.org/repos/asf/qpid/branches/qpid-4631@1469525 13f79535-47bb-0310-9956-ffa450edef68

12 files changed, 9 insertions, 44 deletions

diff --git a/qpid/cpp/src/qpid/acl/Acl.cpp b/qpid/cpp/src/qpid/acl/Acl.cpp
index bdec6791b8..8fba746982 100644
--- a/qpid/cpp/src/qpid/acl/Acl.cpp
+++ b/qpid/cpp/src/qpid/acl/Acl.cpp
@@ -53,7 +53,7 @@ using qpid::management::Manageable;
 using qpid::management::Args;
 namespace _qmf = qmf::org::apache::qpid::acl;
 
-Acl::Acl (AclValues& av, Broker& b): aclValues(av), broker(&b), transferAcl(false), createlinkAcl(false),
+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)){
 
@@ -255,16 +255,11 @@ bool Acl::readAclFile(std::string& aclFile, std::string& errorText) {
         data = d;
     }
     transferAcl = data->transferAcl; // any PUBLISH EXCHANGE transfer ACL
-    createlinkAcl = data->createlinkAcl; // any CREATE LINK ACL
 
     if (data->transferAcl){
         QPID_LOG(debug,"ACL: Transfer ACL is Enabled!");
     }
 
-    if (data->createlinkAcl){
-        QPID_LOG(debug,"ACL: CREATE LINK ACL is Enabled!");
-    }
-
     if (data->enforcingConnectionQuotas()){
         QPID_LOG(debug, "ACL: Connection quotas are Enabled.");
     }
@@ -276,7 +271,6 @@ bool Acl::readAclFile(std::string& aclFile, std::string& errorText) {
     data->aclSource = aclFile;
     if (mgmtObject!=0){
         mgmtObject->set_transferAcl(transferAcl?1:0);
-        mgmtObject->set_createLinkAcl(createlinkAcl?1:0);
         mgmtObject->set_policyFile(aclFile);
         sys::AbsTime now = sys::AbsTime::now();
         int64_t ns = sys::Duration(sys::EPOCH, now);
diff --git a/qpid/cpp/src/qpid/acl/Acl.h b/qpid/cpp/src/qpid/acl/Acl.h
index 9721a60fba..ea3c6586a3 100644
--- a/qpid/cpp/src/qpid/acl/Acl.h
+++ b/qpid/cpp/src/qpid/acl/Acl.h
@@ -61,7 +61,6 @@ private:
     acl::AclValues                       aclValues;
     broker::Broker*                      broker;
     bool                                 transferAcl;
-    bool                                 createlinkAcl;
     boost::shared_ptr<AclData>           data;
     qmf::org::apache::qpid::acl::Acl::shared_ptr mgmtObject;
     qpid::management::ManagementAgent*   agent;
@@ -82,10 +81,6 @@ public:
         return transferAcl;
     };
 
-    inline virtual bool isCreatelinkAcl() {
-        return createlinkAcl;
-    };
-
     inline virtual uint16_t getMaxConnectTotal() {
         return aclValues.aclMaxConnectTotal;
     };
diff --git a/qpid/cpp/src/qpid/acl/AclData.cpp b/qpid/cpp/src/qpid/acl/AclData.cpp
index 5ffd0462dc..922f65ba69 100644
--- a/qpid/cpp/src/qpid/acl/AclData.cpp
+++ b/qpid/cpp/src/qpid/acl/AclData.cpp
@@ -46,7 +46,6 @@ namespace acl {
     AclData::AclData():
         decisionMode(qpid::acl::DENY),
         transferAcl(false),
-        createlinkAcl(false),
         aclSource("UNKNOWN"),
         connQuotaRulesExist(false),
         connQuotaRuleSettings(new quotaRuleSet),
@@ -75,7 +74,6 @@ namespace acl {
             delete[] actionList[cnt];
         }
         transferAcl = false;
-        createlinkAcl = false;
         connQuotaRulesExist = false;
         connQuotaRuleSettings->clear();
         queueQuotaRulesExist = false;
diff --git a/qpid/cpp/src/qpid/acl/AclData.h b/qpid/cpp/src/qpid/acl/AclData.h
index ad8b741be5..c561b95e09 100644
--- a/qpid/cpp/src/qpid/acl/AclData.h
+++ b/qpid/cpp/src/qpid/acl/AclData.h
@@ -118,7 +118,6 @@ public:
     aclAction*           actionList[qpid::acl::ACTIONSIZE];
     qpid::acl::AclResult decisionMode;  // allow/deny[-log] if no matching rule found
     bool                 transferAcl;
-    bool                 createlinkAcl;
     std::string          aclSource;
 
     AclResult lookup(
diff --git a/qpid/cpp/src/qpid/acl/AclReader.cpp b/qpid/cpp/src/qpid/acl/AclReader.cpp
index f0974b8585..1fd5445b52 100644
--- a/qpid/cpp/src/qpid/acl/AclReader.cpp
+++ b/qpid/cpp/src/qpid/acl/AclReader.cpp
@@ -160,10 +160,6 @@ namespace acl {
                         ocnt < acl::OBJECTSIZE;
                     (*i)->objStatus != aclRule::VALUE ? ocnt++ : ocnt = acl::OBJECTSIZE) {
 
-                        // Observe existance of CREATE LINK rules
-                        if (acnt == acl::ACT_CREATE && ocnt == acl::OBJ_LINK)
-                            d->createlinkAcl = true;
-
                         //find the Object, create if not exist
                         if (d->actionList[acnt][ocnt] == NULL)
                             d->actionList[acnt][ocnt] =
diff --git a/qpid/cpp/src/qpid/acl/management-schema.xml b/qpid/cpp/src/qpid/acl/management-schema.xml
index e0368db96b..2ac20bb324 100644
--- a/qpid/cpp/src/qpid/acl/management-schema.xml
+++ b/qpid/cpp/src/qpid/acl/management-schema.xml
@@ -26,7 +26,6 @@
     <property name="maxConnectionsPerIp"   type="uint16"  access="RO"       desc="Maximum allowed connections"/>
     <property name="maxConnectionsPerUser" type="uint16"  access="RO"       desc="Maximum allowed connections"/>
     <property name="maxQueuesPerUser"      type="uint16"  access="RO"       desc="Maximum allowed queues"/>
-    <property name="createLinkAcl"         type="bool"    access="RO"       desc="Create Link ACL rules may allow link creation"/>
     <statistic name="aclDenyCount"         type="count64" unit="request"    desc="Number of ACL requests denied"/>
     <statistic name="connectionDenyCount"  type="count64" unit="connection" desc="Number of connections denied"/>
     <statistic name="queueQuotaDenyCount"  type="count64" unit="queue"      desc="Number of queue creations denied"/>
diff --git a/qpid/cpp/src/qpid/broker/AclModule.h b/qpid/cpp/src/qpid/broker/AclModule.h
index 1c138c62a8..c01697ace9 100644
--- a/qpid/cpp/src/qpid/broker/AclModule.h
+++ b/qpid/cpp/src/qpid/broker/AclModule.h
@@ -132,11 +132,6 @@ namespace broker {
         // doTransferAcl pervents time consuming ACL calls on a per-message basis.
         virtual bool doTransferAcl()=0;
 
-        // Federation link creation is denied unless ACL module is loaded and
-        // at least one CREATE LINK rule is specified.
-        // This flag indicates that a CREATE LINK rule was processed.
-        virtual bool isCreatelinkAcl()=0;
-
         virtual uint16_t getMaxConnectTotal()=0;
 
         virtual bool authorise(
diff --git a/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp b/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp
index 39a8664aab..13ff4cc15f 100644
--- a/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp
+++ b/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp
@@ -202,24 +202,17 @@ void ConnectionHandler::Handler::startOk(const ConnectionStartOkBody& body)
         AclModule* acl =  connection.getBroker().getAcl();
         FieldTable properties;
         if (acl) {
-            if (acl->isCreatelinkAcl()) {
-                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()
-                                         << " creating a federation link"));
-                    return;
-                }
-            } else {
+            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()
-                                << ". Federation links require explicit CREATE LINK ACL rules"));
+                                        << " creating a federation link"));
                 return;
             }
         } else {
             proxy.close(framing::connection::CLOSE_CODE_CONNECTION_FORCED,
                         QPID_MSG("ACL denied " << connection.getUserId()
-                            << ". Federation links require ACL module and explicit CREATE LINK ACL rules"));
-                        return;
+                            << ". Federation links require ACL module and explicit authorization"));
+            return;
         }
         QPID_LOG(info, "Connection is a federation link");
     }
diff --git a/qpid/cpp/src/tests/ha_test.py b/qpid/cpp/src/tests/ha_test.py
index 5c42780f50..4e27675438 100755
--- a/qpid/cpp/src/tests/ha_test.py
+++ b/qpid/cpp/src/tests/ha_test.py
@@ -79,12 +79,11 @@ class HaBroker(Broker):
         if ha_replicate is not None:
             args += [ "--ha-replicate=%s"%ha_replicate ]
         if brokers_url: args += [ "--ha-brokers-url", brokers_url ]
-        # Set up default ACL file to allow all create link
+        # Set up default ACL
         acl=os.path.join(os.getcwd(), "unrestricted.acl")
         if not os.path.exists(acl):
             aclf=file(acl,"w")
             aclf.write("""
-acl allow all create link
 acl allow all all
  """)
             aclf.close()
diff --git a/qpid/cpp/src/tests/run_federation_sys_tests b/qpid/cpp/src/tests/run_federation_sys_tests
index f88d313b05..a55ab3f21f 100755
--- a/qpid/cpp/src/tests/run_federation_sys_tests
+++ b/qpid/cpp/src/tests/run_federation_sys_tests
@@ -42,8 +42,7 @@ SKIPTESTS="${SKIPTESTS} -i federation_sys.C_* -i federation_sys.D_*"
 
 start_brokers() {
     start_broker() {
-	echo acl allow all create link  > $moduledir/federation-sys-tests.acl
-	echo acl allow all all         >> $moduledir/federation-sys-tests.acl
+	echo acl allow all all > $moduledir/federation-sys-tests.acl
         ${QPIDD_EXEC} --daemon --port 0 --auth no --no-data-dir --load-module acl.so --acl-file $moduledir/federation-sys-tests.acl $1 > qpidd.port
         PORT=`cat qpidd.port`
         eval "$2=${PORT}"
diff --git a/qpid/cpp/src/tests/run_federation_tests b/qpid/cpp/src/tests/run_federation_tests
index 1e7fdcb047..1405454535 100755
--- a/qpid/cpp/src/tests/run_federation_tests
+++ b/qpid/cpp/src/tests/run_federation_tests
@@ -36,8 +36,7 @@ fi
 QPIDD_CMD="../qpidd --daemon --port 0 --no-data-dir $MODULES --auth no --log-enable=info+ --log-enable=debug+:Bridge --load-module acl.so --acl-file $moduledir/federation-tests.acl --log-to-file"
 start_brokers() {
     rm -f fed_local.log fed_remote.log fed_b1.log fed_b2.log $moduledir/federation-tests.acl
-    echo acl allow all create link  > $moduledir/federation-tests.acl
-    echo acl allow all all         >> $moduledir/federation-tests.acl
+    echo acl allow all all > $moduledir/federation-tests.acl
     LOCAL_PORT=$($QPIDD_CMD fed_local.log --federation-tag LOCAL)
     REMOTE_PORT=$($QPIDD_CMD fed_remote.log --federation-tag REMOTE)
     REMOTE_B1=$($QPIDD_CMD fed_b1.log --federation-tag B1)
diff --git a/qpid/cpp/src/tests/sasl_fed b/qpid/cpp/src/tests/sasl_fed
index 6a6e4ec161..bd7b15f2d8 100755
--- a/qpid/cpp/src/tests/sasl_fed
+++ b/qpid/cpp/src/tests/sasl_fed
@@ -47,8 +47,7 @@ tmp_root=/tmp/sasl_fed_$my_random_number
 mkdir -p $tmp_root
 
 # create ACL file to allow links
-echo acl allow all create link  > $tmp_root/sasl_fed.acl
-echo acl allow all all         >> $tmp_root/sasl_fed.acl
+echo acl allow all all > $tmp_root/sasl_fed.acl
 
 
 #--------------------------------------------------