QPID-4631: C++ Broker federated links are protected by ACL policy.

This issue evolved a bit between the original discussion and the final
commit. See https://reviews.apache.org/r/10658/ for the details.



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

5 files changed, 129 insertions, 23 deletions

diff --git a/cpp/src/qpid/acl/AclConnectionCounter.cpp b/cpp/src/qpid/acl/AclConnectionCounter.cpp
index 875137bf55..3d703066b4 100644
--- a/cpp/src/qpid/acl/AclConnectionCounter.cpp
+++ b/cpp/src/qpid/acl/AclConnectionCounter.cpp
@@ -288,7 +288,6 @@ std::string ConnectionCounter::getClientHost(const std::string mgmtId)
     }
 
     // no hyphen found - use whole string
-    assert(false);
     return mgmtId;
 }
 
diff --git a/cpp/src/qpid/broker/ConnectionHandler.cpp b/cpp/src/qpid/broker/ConnectionHandler.cpp
index ac05178fce..d2ab675ecd 100644
--- a/cpp/src/qpid/broker/ConnectionHandler.cpp
+++ b/cpp/src/qpid/broker/ConnectionHandler.cpp
@@ -201,12 +201,22 @@ void ConnectionHandler::Handler::startOk(const ConnectionStartOkBody& body)
     }
     if (connection.isFederationLink()) {
         AclModule* acl =  connection.getBroker().getAcl();
-        FieldTable properties;
-    	if (acl && !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;
+        if (acl) {
+            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 {
+            Broker::Options& conf = connection.getBroker().getOptions();
+            if (conf.auth) {
+                proxy.close(framing::connection::CLOSE_CODE_CONNECTION_FORCED,
+                            QPID_MSG("User " << connection.getUserId()
+                                << " federation connection denied. Systems with authentication "
+                                   "enabled must specify ACL create link rules."));
+                return;
+            }
         }
         QPID_LOG(info, "Connection is a federation link");
     }
diff --git a/cpp/src/tests/ha_test.py b/cpp/src/tests/ha_test.py
index f2ebb16ccc..4e27675438 100755
--- a/cpp/src/tests/ha_test.py
+++ b/cpp/src/tests/ha_test.py
@@ -79,6 +79,16 @@ 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
+        acl=os.path.join(os.getcwd(), "unrestricted.acl")
+        if not os.path.exists(acl):
+            aclf=file(acl,"w")
+            aclf.write("""
+acl allow all all
+ """)
+            aclf.close()
+        if not "--acl-file" in args:
+            args += [ "--acl-file", acl, "--load-module", os.getenv("ACL_LIB") ]
         Broker.__init__(self, test, args, **kwargs)
         self.qpid_ha_path=os.path.join(os.getenv("PYTHON_COMMANDS"), "qpid-ha")
         assert os.path.exists(self.qpid_ha_path)
diff --git a/cpp/src/tests/run_acl_tests b/cpp/src/tests/run_acl_tests
index d259f89255..0fff6de1b8 100755
--- a/cpp/src/tests/run_acl_tests
+++ b/cpp/src/tests/run_acl_tests
@@ -39,6 +39,33 @@ start_brokers() {
     LOCAL_PORTQ=`cat qpiddq.port`
 }
 
+start_noacl_noauth_brokers() {
+    ../qpidd --daemon --port 0 --no-module-dir --data-dir $DATA_DIR --auth no --log-to-file local.log > qpidd.port
+    LOCAL_PORT=`cat qpidd.port`
+    ../qpidd --daemon --port 0 --no-module-dir --data-dir $DATA_DIRI --auth no --log-to-file locali.log > qpiddi.port
+    LOCAL_PORTI=`cat qpiddi.port`
+    ../qpidd --daemon --port 0 --no-module-dir --data-dir $DATA_DIRU --auth no --log-to-file localu.log > qpiddu.port
+    LOCAL_PORTU=`cat qpiddu.port`
+    ../qpidd --daemon --port 0 --no-module-dir --data-dir $DATA_DIRQ --auth no --log-to-file localq.log > qpiddq.port
+    LOCAL_PORTQ=`cat qpiddq.port`
+}
+
+start_noacl_auth_brokers() {
+    sasl_config_file=$builddir/sasl_config
+    if [ ! -f $sasl_config_file ] ; then
+	echo Creating sasl database
+	. $srcdir/sasl_test_setup.sh
+    fi
+    ../qpidd --daemon --port 0 --no-module-dir --data-dir $DATA_DIR --auth yes --sasl-config=$sasl_config_file --log-to-file local.log > qpidd.port
+    LOCAL_PORT=`cat qpidd.port`
+    ../qpidd --daemon --port 0 --no-module-dir --data-dir $DATA_DIRI --auth yes --sasl-config=$sasl_config_file --log-to-file locali.log > qpiddi.port
+    LOCAL_PORTI=`cat qpiddi.port`
+    ../qpidd --daemon --port 0 --no-module-dir --data-dir $DATA_DIRU --auth yes --sasl-config=$sasl_config_file --log-to-file localu.log > qpiddu.port
+    LOCAL_PORTU=`cat qpiddu.port`
+    ../qpidd --daemon --port 0 --no-module-dir --data-dir $DATA_DIRQ --auth yes --sasl-config=$sasl_config_file --log-to-file localq.log > qpiddq.port
+    LOCAL_PORTQ=`cat qpiddq.port`
+}
+
 stop_brokers() {
         $QPIDD_EXEC --no-module-dir -q --port $LOCAL_PORT
         $QPIDD_EXEC --no-module-dir -q --port $LOCAL_PORTI
@@ -46,6 +73,34 @@ stop_brokers() {
         $QPIDD_EXEC --no-module-dir -q --port $LOCAL_PORTQ
 }
 
+delete_directories() {
+    rm -rf $DATA_DIR
+    rm -rf $DATA_DIRI
+    rm -rf $DATA_DIRU
+    rm -rf $DATA_DIRQ
+}
+
+delete_logfiles() {
+    rm -rf local.log
+    rm -rf locali.log
+    rm -rf localu.log
+    rm -rf localq.log
+}
+
+create_directories() {
+    mkdir -p $DATA_DIR
+    mkdir -p $DATA_DIRI
+    mkdir -p $DATA_DIRU
+    mkdir -p $DATA_DIRQ
+}
+
+populate_directories() {
+    cp $srcdir/policy.acl $DATA_DIR
+    cp $srcdir/policy.acl $DATA_DIRI
+    cp $srcdir/policy.acl $DATA_DIRU
+    cp $srcdir/policy.acl $DATA_DIRQ
+}
+
 test_loading_acl_from_absolute_path(){
     POLICY_FILE=$srcdir/policy.acl
     rm -f temp.log
@@ -59,28 +114,53 @@ test_loading_acl_from_absolute_path(){
    rm temp.log
 }
 
+test_noacl_deny_create_link() {
+    delete_logfiles
+    start_noacl_noauth_brokers
+    echo "Running no-acl, no-auth tests using brokers on ports $LOCAL_PORT, $LOCAL_PORTI, $LOCAL_PORTU, and $LOCAL_PORTQ"
+    $QPID_CONFIG_EXEC -a localhost:$LOCAL_PORT  add exchange topic fed.topic
+    $QPID_CONFIG_EXEC -a localhost:$LOCAL_PORTI add exchange topic fed.topic
+    $QPID_ROUTE_EXEC dynamic add localhost:$LOCAL_PORT localhost:$LOCAL_PORTI fed.topic 2>/dev/null
+    sleep 2
+    stop_brokers
+    grep -q "must specify ACL create link rules" local.log
+    if [ $? -eq 0 ]
+    then
+	echo "Test fail - Broker with auth=no should have allowed link creation";
+	return 1;
+    fi
+
+    delete_logfiles
+    start_noacl_auth_brokers
+    echo "Running no-acl, auth tests using brokers on ports $LOCAL_PORT, $LOCAL_PORTI, $LOCAL_PORTU, and $LOCAL_PORTQ"
+    $QPID_CONFIG_EXEC -a localhost:$LOCAL_PORT  add exchange topic fed.topic
+    $QPID_CONFIG_EXEC -a localhost:$LOCAL_PORTI add exchange topic fed.topic
+    $QPID_ROUTE_EXEC dynamic add localhost:$LOCAL_PORT localhost:$LOCAL_PORTI fed.topic 2>/dev/null
+    sleep 2
+    stop_brokers
+    grep -q "must specify ACL create link rules" local.log
+    if [ $? -ne 0 ]
+    then
+	echo "Test fail - Broker with no ACL and --auth=yes file did not deny link creation";
+	return 1;
+    fi
+}
+
 if test -d ${PYTHON_DIR} ;  then
-    rm -rf $DATA_DIR
-    rm -rf $DATA_DIRI
-    rm -rf $DATA_DIRU
-    rm -rf $DATA_DIRQ
-    mkdir -p $DATA_DIR
-    mkdir -p $DATA_DIRI
-    mkdir -p $DATA_DIRU
-    mkdir -p $DATA_DIRQ
-    cp $srcdir/policy.acl $DATA_DIR
-    cp $srcdir/policy.acl $DATA_DIRI
-    cp $srcdir/policy.acl $DATA_DIRU
-    cp $srcdir/policy.acl $DATA_DIRQ
+    # run acl.py test file
+    delete_directories
+    create_directories
+    populate_directories
+    delete_logfiles
     start_brokers
     echo "Running acl tests using brokers on ports $LOCAL_PORT, $LOCAL_PORTI, $LOCAL_PORTU, and $LOCAL_PORTQ"
     $QPID_PYTHON_TEST -b localhost:$LOCAL_PORT -m acl -Dport-i=$LOCAL_PORTI -Dport-u=$LOCAL_PORTU -Dport-q=$LOCAL_PORTQ || EXITCODE=1
     stop_brokers || EXITCODE=1
+    #
     test_loading_acl_from_absolute_path || EXITCODE=1
-    rm -rf $DATA_DIR
-    rm -rf $DATA_DIRI
-    rm -rf $DATA_DIRU
-    rm -rf $DATA_DIRQ
+    #
+    test_noacl_deny_create_link || EXITCODE=1
+    delete_directories
     exit $EXITCODE
 fi
 
diff --git a/cpp/src/tests/sasl_fed b/cpp/src/tests/sasl_fed
index 9dc2dd46e2..bd7b15f2d8 100755
--- a/cpp/src/tests/sasl_fed
+++ b/cpp/src/tests/sasl_fed
@@ -46,6 +46,9 @@ my_random_number=$RANDOM
 tmp_root=/tmp/sasl_fed_$my_random_number
 mkdir -p $tmp_root
 
+# create ACL file to allow links
+echo acl allow all all > $tmp_root/sasl_fed.acl
+
 
 #--------------------------------------------------
 #echo " Starting broker 1"
@@ -59,6 +62,8 @@ $QPIDD_EXEC                                  \
   --log-source yes                           \
   --log-to-file $tmp_root/qpidd_1.log        \
   --sasl-config=$sasl_config_file            \
+  --load-module acl.so                       \
+  --acl-file $tmp_root/sasl_fed.acl          \
   -d > $tmp_root/broker_1_port
 
 broker_1_port=`cat $tmp_root/broker_1_port`
@@ -76,6 +81,8 @@ $QPIDD_EXEC                                  \
   --log-source yes                           \
   --log-to-file $tmp_root/qpidd_2.log        \
   --sasl-config=$sasl_config_file            \
+  --load-module acl.so                       \
+  --acl-file $tmp_root/sasl_fed.acl          \
   -d > $tmp_root/broker_2_port
 
 broker_2_port=`cat $tmp_root/broker_2_port`