Author: aconway
Date: Fri Jun 22 18:39:56 2012
New Revision: 1352992
URL: http://svn.apache.org/viewvc?rev=1352992&view=rev
Log:
QPID-3849: Client connection breaks broker-to-broker cluster SASL
authentication
Catch-up shadow connections were not being authenticated which caused two
problems:
- new brokers failed to join the cluster if there was an authenticated session.
- possible security loophole that would allow an intruder to gain access to a
catch-up broker.
All external connections are now fully authenticated, which solves both
problems.
Modified:
qpid/trunk/qpid/cpp/src/qpid/broker/Connection.cpp
qpid/trunk/qpid/cpp/src/qpid/broker/Connection.h
qpid/trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp
qpid/trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.h
qpid/trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp
qpid/trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.h
qpid/trunk/qpid/cpp/src/qpid/cluster/Connection.cpp
qpid/trunk/qpid/cpp/src/qpid/cluster/Connection.h
qpid/trunk/qpid/cpp/src/tests/cluster_tests.py
qpid/trunk/qpid/cpp/src/tests/sasl_test_setup.sh
Modified: qpid/trunk/qpid/cpp/src/qpid/broker/Connection.cpp
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/broker/Connection.cpp?rev=1352992&r1=1352991&r2=1352992&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/broker/Connection.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/broker/Connection.cpp Fri Jun 22 18:39:56 2012
@@ -87,10 +87,14 @@ Connection::Connection(ConnectionOutputH
bool link_,
uint64_t objectId_,
bool shadow_,
- bool delayManagement) :
+ bool delayManagement,
+ bool authenticated_
+) :
ConnectionState(out_, broker_),
securitySettings(external),
- adapter(*this, link_, shadow_),
+ shadow(shadow_),
+ authenticated(authenticated_),
+ adapter(*this, link_),
link(link_),
mgmtClosing(false),
mgmtId(mgmtId_),
@@ -100,7 +104,6 @@ Connection::Connection(ConnectionOutputH
timer(broker_.getTimer()),
errorListener(0),
objectId(objectId_),
- shadow(shadow_),
outboundTracker(*this)
{
outboundTracker.wrap(out);
Modified: qpid/trunk/qpid/cpp/src/qpid/broker/Connection.h
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/broker/Connection.h?rev=1352992&r1=1352991&r2=1352992&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/broker/Connection.h (original)
+++ qpid/trunk/qpid/cpp/src/qpid/broker/Connection.h Fri Jun 22 18:39:56 2012
@@ -86,7 +86,8 @@ class Connection : public sys::Connectio
bool isLink = false,
uint64_t objectId = 0,
bool shadow=false,
- bool delayManagement = false);
+ bool delayManagement = false,
+ bool authenticated=true);
~Connection ();
@@ -151,6 +152,9 @@ class Connection : public sys::Connectio
/** True if this is a shadow connection in a cluster. */
bool isShadow() const { return shadow; }
+ /** True if this connection is authenticated */
+ bool isAuthenticated() const { return authenticated; }
+
// Used by cluster to update connection status
sys::AggregateOutput& getOutputTasks() { return outputTasks; }
@@ -180,6 +184,8 @@ class Connection : public sys::Connectio
ChannelMap channels;
qpid::sys::SecuritySettings securitySettings;
+ bool shadow;
+ bool authenticated;
ConnectionHandler adapter;
const bool link;
bool mgmtClosing;
@@ -194,7 +200,6 @@ class Connection : public sys::Connectio
boost::intrusive_ptr<ConnectionTimeoutTask> timeoutTimer;
ErrorListener* errorListener;
uint64_t objectId;
- bool shadow;
framing::FieldTable clientProperties;
/**
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=1352992&r1=1352991&r2=1352992&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp Fri Jun 22
18:39:56 2012
@@ -106,9 +106,10 @@ void ConnectionHandler::setSecureConnect
handler->secured = secured;
}
-ConnectionHandler::ConnectionHandler(Connection& connection, bool isClient,
bool isShadow) : handler(new Handler(connection, isClient, isShadow)) {}
+ConnectionHandler::ConnectionHandler(Connection& connection, bool isClient) :
+ handler(new Handler(connection, isClient)) {}
-ConnectionHandler::Handler::Handler(Connection& c, bool isClient, bool
isShadow) :
+ConnectionHandler::Handler::Handler(Connection& c, bool isClient) :
proxy(c.getOutput()),
connection(c), serverMode(!isClient), secured(0),
isOpen(false)
@@ -119,14 +120,13 @@ ConnectionHandler::Handler::Handler(Conn
properties.setString(QPID_FED_TAG,
connection.getBroker().getFederationTag());
- authenticator = SaslAuthenticator::createAuthenticator(c, isShadow);
+ authenticator = SaslAuthenticator::createAuthenticator(c);
authenticator->getMechanisms(mechanisms);
Array locales(0x95);
boost::shared_ptr<FieldValue> l(new Str16Value(en_US));
locales.add(l);
proxy.start(properties, mechanisms, locales);
-
}
maxFrameSize = (64 * 1024) - 1;
Modified: qpid/trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.h
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.h?rev=1352992&r1=1352991&r2=1352992&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.h (original)
+++ qpid/trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.h Fri Jun 22 18:39:56
2012
@@ -61,7 +61,7 @@ class ConnectionHandler : public framing
SecureConnection* secured;
bool isOpen;
- Handler(Connection& connection, bool isClient, bool isShadow=false);
+ Handler(Connection& connection, bool isClient);
~Handler();
void startOk(const qpid::framing::ConnectionStartOkBody& body);
void startOk(const qpid::framing::FieldTable& clientProperties,
@@ -99,7 +99,7 @@ class ConnectionHandler : public framing
bool handle(const qpid::framing::AMQMethodBody& method);
public:
- ConnectionHandler(Connection& connection, bool isClient, bool
isShadow=false );
+ ConnectionHandler(Connection& connection, bool isClient );
void close(framing::connection::CloseCode code, const std::string& text);
void heartbeat();
void handle(framing::AMQFrame& frame);
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=1352992&r1=1352991&r2=1352992&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp Fri Jun 22
18:39:56 2012
@@ -166,13 +166,17 @@ void SaslAuthenticator::fini(void)
#endif
-std::auto_ptr<SaslAuthenticator>
SaslAuthenticator::createAuthenticator(Connection& c, bool isShadow )
+std::auto_ptr<SaslAuthenticator>
SaslAuthenticator::createAuthenticator(Connection& c )
{
if (c.getBroker().getOptions().auth) {
- if ( isShadow )
- return std::auto_ptr<SaslAuthenticator>(new NullAuthenticator(c,
c.getBroker().getOptions().requireEncrypted));
+ // The cluster creates non-authenticated connections for internal
shadow connections
+ // that are never connected to an external client.
+ if ( !c.isAuthenticated() )
+ return std::auto_ptr<SaslAuthenticator>(
+ new NullAuthenticator(c,
c.getBroker().getOptions().requireEncrypted));
else
- return std::auto_ptr<SaslAuthenticator>(new CyrusAuthenticator(c,
c.getBroker().getOptions().requireEncrypted));
+ return std::auto_ptr<SaslAuthenticator>(
+ new CyrusAuthenticator(c,
c.getBroker().getOptions().requireEncrypted));
} else {
QPID_LOG(debug, "SASL: No Authentication Performed");
return std::auto_ptr<SaslAuthenticator>(new NullAuthenticator(c,
c.getBroker().getOptions().requireEncrypted));
Modified: qpid/trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.h
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.h?rev=1352992&r1=1352991&r2=1352992&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.h (original)
+++ qpid/trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.h Fri Jun 22 18:39:56
2012
@@ -54,7 +54,7 @@ public:
static void init(const std::string& saslName, std::string const &
saslConfigPath );
static void fini(void);
- static std::auto_ptr<SaslAuthenticator> createAuthenticator(Connection&
connection, bool isShadow);
+ static std::auto_ptr<SaslAuthenticator> createAuthenticator(Connection&
connection);
virtual void callUserIdCallbacks() { }
};
Modified: qpid/trunk/qpid/cpp/src/qpid/cluster/Connection.cpp
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/cluster/Connection.cpp?rev=1352992&r1=1352991&r2=1352992&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/cluster/Connection.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/cluster/Connection.cpp Fri Jun 22 18:39:56 2012
@@ -85,7 +85,9 @@ Connection::Connection(Cluster& c, sys::
const std::string& mgmtId,
const ConnectionId& id, const
qpid::sys::SecuritySettings& external)
: cluster(c), self(id), catchUp(false), announced(false), output(*this,
out),
- connectionCtor(&output, cluster.getBroker(), mgmtId, external, false, 0,
true),
+ connectionCtor(&output, cluster.getBroker(), mgmtId, external,
+ false/*isLink*/, 0/*objectId*/, true/*shadow*/,
false/*delayManagement*/,
+ false/*authenticated*/),
expectProtocolHeader(false),
mcastFrameHandler(cluster.getMulticast(), self),
updateIn(c.getUpdateReceiver()),
@@ -102,9 +104,10 @@ Connection::Connection(Cluster& c, sys::
external,
isLink,
isCatchUp ? ++catchUpId : 0,
- // The first catch-up connection is not considered a shadow
- // as it needs to be authenticated.
- isCatchUp && self.second > 1),
+ // The first catch-up connection is not a shadow
+ isCatchUp && self.second > 1,
+ false, // delayManagement
+ true), // catch up connecytions are authenticated
expectProtocolHeader(isLink),
mcastFrameHandler(cluster.getMulticast(), self),
updateIn(c.getUpdateReceiver()),
Modified: qpid/trunk/qpid/cpp/src/qpid/cluster/Connection.h
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/cluster/Connection.h?rev=1352992&r1=1352991&r2=1352992&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/cluster/Connection.h (original)
+++ qpid/trunk/qpid/cpp/src/qpid/cluster/Connection.h Fri Jun 22 18:39:56 2012
@@ -228,6 +228,7 @@ class Connection :
uint64_t objectId;
bool shadow;
bool delayManagement;
+ bool authenticated;
ConnectionCtor(
sys::ConnectionOutputHandler* out_,
@@ -237,17 +238,18 @@ class Connection :
bool isLink_=false,
uint64_t objectId_=0,
bool shadow_=false,
- bool delayManagement_=false
+ bool delayManagement_=false,
+ bool authenticated_=true
) : out(out_), broker(broker_), mgmtId(mgmtId_), external(external_),
isLink(isLink_), objectId(objectId_), shadow(shadow_),
- delayManagement(delayManagement_)
+ delayManagement(delayManagement_), authenticated(authenticated_)
{}
std::auto_ptr<broker::Connection> construct() {
return std::auto_ptr<broker::Connection>(
new broker::Connection(
out, broker, mgmtId, external, isLink, objectId,
- shadow, delayManagement)
+ shadow, delayManagement, authenticated)
);
}
};
Modified: qpid/trunk/qpid/cpp/src/tests/cluster_tests.py
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/tests/cluster_tests.py?rev=1352992&r1=1352991&r2=1352992&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/tests/cluster_tests.py (original)
+++ qpid/trunk/qpid/cpp/src/tests/cluster_tests.py Fri Jun 22 18:39:56 2012
@@ -227,6 +227,18 @@ acl deny all all
self.assertEqual("x", cluster[0].get_message("q").content)
self.assertEqual("y", cluster[1].get_message("q").content)
+ def test_other_mech(self):
+ """Test using a mechanism other than PLAIN/ANONYMOUS for cluster
update authentication.
+ Regression test for https://issues.apache.org/jira/browse/QPID-3849""";
+ sasl_config=os.path.join(self.rootdir, "sasl_config")
+ cluster = self.cluster(2, args=["--auth", "yes", "--sasl-config",
sasl_config,
+ "--cluster-username=zig",
+ "--cluster-password=zig",
+ "--cluster-mechanism=DIGEST-MD5"])
+ cluster[0].connect()
+ cluster.start() # Before the fix this broker falied to join
the cluster.
+ cluster[2].connect()
+
def test_link_events(self):
"""Regression test for
https://bugzilla.redhat.com/show_bug.cgi?id=611543""";
args = ["--mgmt-pub-interval", 1] # Publish management information
every second.
Modified: qpid/trunk/qpid/cpp/src/tests/sasl_test_setup.sh
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/tests/sasl_test_setup.sh?rev=1352992&r1=1352991&r2=1352992&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/tests/sasl_test_setup.sh (original)
+++ qpid/trunk/qpid/cpp/src/tests/sasl_test_setup.sh Fri Jun 22 18:39:56 2012
@@ -30,7 +30,7 @@ pwcheck_method: auxprop
auxprop_plugin: sasldb
sasldb_path: $PWD/sasl_config/qpidd.sasldb
sql_select: dummy select
-mech_list: ANONYMOUS PLAIN DIGEST-MD5 EXTERNAL
+mech_list: ANONYMOUS PLAIN DIGEST-MD5 EXTERNAL CRAM-MD5
EOF
# Populate temporary sasl db.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
