Author: mgoulish
Date: Thu Oct 13 21:50:56 2011
New Revision: 1183121
URL: http://svn.apache.org/viewvc?rev=1183121&view=rev
Log:
QPID-3528
sasl_set_path() does no check on the given directory, so when you get bad
behavior
later it can be hard to track down. Especially bad is its policy of defaulting
to
an alternate standard location if yours fails. That's a potential security bug.
So this patch checks that your dir exists, and is readable, before calling
sasl_set_path(). Either you get the sasl config dir you were expecting,
Added:
qpid/trunk/qpid/cpp/src/tests/sasl_no_dir (with props)
Modified:
qpid/trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp
qpid/trunk/qpid/cpp/src/tests/sasl.mk
Modified: qpid/trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp?rev=1183121&r1=1183120&r2=1183121&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp Thu Oct 13
21:50:56 2011
@@ -30,6 +30,7 @@
#include <boost/format.hpp>
#if HAVE_SASL
+#include <sys/stat.h>
#include <sasl/sasl.h>
#include "qpid/sys/cyrus/CyrusSecurityLayer.h"
using qpid::sys::cyrus::CyrusSecurityLayer;
@@ -98,11 +99,33 @@ void SaslAuthenticator::init(const std::
// Check if we have a version of SASL that supports sasl_set_path()
#if (SASL_VERSION_FULL >= ((2<<16)|(1<<8)|22))
// If we are not given a sasl path, do nothing and allow the default to
be used.
- if ( ! saslConfigPath.empty() ) {
- int code = sasl_set_path(SASL_PATH_TYPE_CONFIG,
- const_cast<char *>(saslConfigPath.c_str()));
+ if ( saslConfigPath.empty() ) {
+ QPID_LOG ( info, "SASL: no config path set - using default." );
+ }
+ else {
+ struct stat st;
+
+ // Make sure the directory exists and we can read up to it.
+ if ( ::stat ( saslConfigPath.c_str(), & st) ) {
+ // Note: not using strerror() here because I think its messages are
a little too hazy.
+ if ( errno == ENOENT )
+ throw Exception ( QPID_MSG ( "SASL: sasl_set_path failed: no
such directory: " << saslConfigPath ) );
+ if ( errno == EACCES )
+ throw Exception ( QPID_MSG ( "SASL: sasl_set_path failed: cannot
read parent of: " << saslConfigPath ) );
+ // catch-all stat failure
+ throw Exception ( QPID_MSG ( "SASL: sasl_set_path failed: cannot
stat: " << saslConfigPath ) );
+ }
+
+ // Make sure the directory is readable.
+ if ( ::access ( saslConfigPath.c_str(), R_OK ) ) {
+ throw Exception ( QPID_MSG ( "SASL: sasl_set_path failed:
directory not readable:" << saslConfigPath ) );
+ }
+
+ // This shouldn't fail now, but check anyway.
+ int code = sasl_set_path(SASL_PATH_TYPE_CONFIG, const_cast<char
*>(saslConfigPath.c_str()));
if(SASL_OK != code)
throw Exception(QPID_MSG("SASL: sasl_set_path failed [" << code <<
"] " ));
+
QPID_LOG(info, "SASL: config path set to " << saslConfigPath );
}
#endif
Modified: qpid/trunk/qpid/cpp/src/tests/sasl.mk
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/tests/sasl.mk?rev=1183121&r1=1183120&r2=1183121&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/tests/sasl.mk (original)
+++ qpid/trunk/qpid/cpp/src/tests/sasl.mk Thu Oct 13 21:50:56 2011
@@ -30,7 +30,7 @@ check_PROGRAMS+=sasl_version
sasl_version_SOURCES=sasl_version.cpp
sasl_version_LDADD=$(lib_client)
-TESTS += run_cluster_authentication_test sasl_fed sasl_fed_ex_dynamic
sasl_fed_ex_link sasl_fed_ex_queue sasl_fed_ex_route sasl_fed_ex_route_cluster
sasl_fed_ex_link_cluster sasl_fed_ex_queue_cluster sasl_fed_ex_dynamic_cluster
+TESTS += run_cluster_authentication_test sasl_fed sasl_fed_ex_dynamic
sasl_fed_ex_link sasl_fed_ex_queue sasl_fed_ex_route sasl_fed_ex_route_cluster
sasl_fed_ex_link_cluster sasl_fed_ex_queue_cluster sasl_fed_ex_dynamic_cluster
sasl_no_dir
LONG_TESTS += run_cluster_authentication_soak
EXTRA_DIST += run_cluster_authentication_test \
sasl_fed \
@@ -43,7 +43,8 @@ EXTRA_DIST += run_cluster_authentication
sasl_fed_ex_dynamic_cluster \
sasl_fed_ex_link_cluster \
sasl_fed_ex_queue_cluster \
- sasl_fed_ex_route_cluster
+ sasl_fed_ex_route_cluster \
+ sasl_no_dir
endif # HAVE_SASL
Added: qpid/trunk/qpid/cpp/src/tests/sasl_no_dir
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/tests/sasl_no_dir?rev=1183121&view=auto
==============================================================================
--- qpid/trunk/qpid/cpp/src/tests/sasl_no_dir (added)
+++ qpid/trunk/qpid/cpp/src/tests/sasl_no_dir Thu Oct 13 21:50:56 2011
@@ -0,0 +1,218 @@
+#! /bin/bash
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+source ./test_env.sh
+
+script_name=`basename $0`
+
+# This minimum value corresponds to sasl version 2.1.22
+minimum_sasl_version=131350
+
+sasl_version=`$QPID_TEST_EXEC_DIR/sasl_version`
+
+# This test is necessary because this sasl version is the first one that
permits
+# redirection of the sasl config file path.
+if [ "$sasl_version" -lt "$minimum_sasl_version" ]; then
+ echo "sasl_fed: must have sasl version 2.1.22 or greater. ( Integer value:
$minimum_sasl_version ) Version is: $sasl_version"
+ exit 0
+fi
+
+
+sasl_config_dir=$builddir/sasl_config
+
+
+# Debugging print. --------------------------
+debug=
+function print {
+ if [ "$debug" ]; then
+ echo "${script_name}: $1"
+ fi
+}
+
+
+my_random_number=$RANDOM
+tmp_root=/tmp/sasl_fed_$my_random_number
+mkdir -p $tmp_root
+
+
+LOG_FILE=$tmp_root/qpidd.log
+
+# If you want to see this test fail, just comment out this 'mv' command.
+print "Moving sasl configuration dir."
+mv ${sasl_config_dir} ${sasl_config_dir}-
+
+
+#--------------------------------------------------
+print " Starting broker"
+#--------------------------------------------------
+$QPIDD_EXEC \
+ -p 0 \
+ --no-data-dir \
+ --auth=yes \
+ --mgmt-enable=yes \
+ --log-enable info+ \
+ --log-source yes \
+ --log-to-file ${LOG_FILE} \
+ --sasl-config=$sasl_config_dir \
+ -d 2> /dev/null 1> $tmp_root/broker_port
+
+
+
+# If it works right, the output will look something like this: ( two lines
long )
+# Daemon startup failed: SASL: sasl_set_path failed: no such directory:
/home/mick/trunk/qpid/cpp/src/tests/sasl_config
(qpid/broker/SaslAuthenticator.cpp:112)
+# 2011-10-13 14:07:00 critical qpidd.cpp:83: Unexpected error: Daemon startup
failed: SASL: sasl_set_path failed: no such directory:
/home/mick/trunk/qpid/cpp/src/tests/sasl_config
(qpid/broker/SaslAuthenticator.cpp:112)
+
+result=`cat ${LOG_FILE} | grep "sasl_set_path failed: no such directory" | wc
-l `
+
+#--------------------------------------------------
+print "Restore the Sasl config dir to its original place."
+#--------------------------------------------------
+mv ${sasl_config_dir}- ${sasl_config_dir}
+
+if [ "2" -eq ${result} ]; then
+ print "result: success"
+ rm -rf $tmp_root
+ exit 0
+fi
+
+
+# If this test fails, the broker is still alive.
+# Kill it.
+broker_port=`cat $tmp_root/broker_port`
+#--------------------------------------------------
+print "Asking broker to quit."
+#--------------------------------------------------
+$QPIDD_EXEC --port $broker_port --quit
+
+rm -rf $tmp_root
+
+print "result: fail"
+exit 1
+
+#! /bin/bash
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+source ./test_env.sh
+
+script_name=`basename $0`
+
+# This minimum value corresponds to sasl version 2.1.22
+minimum_sasl_version=131350
+
+sasl_version=`$QPID_TEST_EXEC_DIR/sasl_version`
+
+# This test is necessary because this sasl version is the first one that
permits
+# redirection of the sasl config file path.
+if [ "$sasl_version" -lt "$minimum_sasl_version" ]; then
+ echo "sasl_fed: must have sasl version 2.1.22 or greater. ( Integer value:
$minimum_sasl_version ) Version is: $sasl_version"
+ exit 0
+fi
+
+
+sasl_config_dir=$builddir/sasl_config
+
+
+# Debugging print. --------------------------
+debug=
+function print {
+ if [ "$debug" ]; then
+ echo "${script_name}: $1"
+ fi
+}
+
+
+my_random_number=$RANDOM
+tmp_root=/tmp/sasl_fed_$my_random_number
+mkdir -p $tmp_root
+
+
+LOG_FILE=$tmp_root/qpidd.log
+
+# If you want to see this test fail, just comment out this 'mv' command.
+print "Moving sasl configuration dir."
+mv ${sasl_config_dir} ${sasl_config_dir}-
+
+
+#--------------------------------------------------
+print " Starting broker"
+#--------------------------------------------------
+$QPIDD_EXEC \
+ -p 0 \
+ --no-data-dir \
+ --auth=yes \
+ --mgmt-enable=yes \
+ --log-enable info+ \
+ --log-source yes \
+ --log-to-file ${LOG_FILE} \
+ --sasl-config=$sasl_config_dir \
+ -d 2> /dev/null 1> $tmp_root/broker_port
+
+
+
+# If it works right, the output will look something like this: ( two lines
long )
+# Daemon startup failed: SASL: sasl_set_path failed: no such directory:
/home/mick/trunk/qpid/cpp/src/tests/sasl_config
(qpid/broker/SaslAuthenticator.cpp:112)
+# 2011-10-13 14:07:00 critical qpidd.cpp:83: Unexpected error: Daemon startup
failed: SASL: sasl_set_path failed: no such directory:
/home/mick/trunk/qpid/cpp/src/tests/sasl_config
(qpid/broker/SaslAuthenticator.cpp:112)
+
+result=`cat ${LOG_FILE} | grep "sasl_set_path failed: no such directory" | wc
-l `
+
+#--------------------------------------------------
+print "Restore the Sasl config dir to its original place."
+#--------------------------------------------------
+mv ${sasl_config_dir}- ${sasl_config_dir}
+
+if [ "2" -eq ${result} ]; then
+ print "result: success"
+ rm -rf $tmp_root
+ exit 0
+fi
+
+
+# If this test fails, the broker is still alive.
+# Kill it.
+broker_port=`cat $tmp_root/broker_port`
+#--------------------------------------------------
+print "Asking broker to quit."
+#--------------------------------------------------
+$QPIDD_EXEC --port $broker_port --quit
+
+rm -rf $tmp_root
+
+print "result: fail"
+exit 1
+
Propchange: qpid/trunk/qpid/cpp/src/tests/sasl_no_dir
------------------------------------------------------------------------------
svn:executable = *
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:commits-subscr...@qpid.apache.org
