From 7e6c487fdfebe029b36dc7be0e3e06e99a1b7033 Mon Sep 17 00:00:00 2001 From: Robert Gemmell Date: Sat, 11 Jan 2014 21:22:51 +0000 Subject: QPID-5373: move retrieval of the peer Principal into the connection IO thread, retrieving from the NetworkConnection during the AMQP handshak after the SSL handshake must have already been completed. git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1557467 13f79535-47bb-0310-9956-ffa450edef68 --- .../protocol/MultiVersionProtocolEngine.java | 40 +++++++++++--------- .../server/protocol/v0_10/ProtocolEngine_0_10.java | 10 +---- .../server/protocol/v0_10/ServerConnection.java | 8 +--- .../protocol/v0_8/InternalTestProtocolSession.java | 5 --- .../transport/websocket/WebSocketProvider.java | 7 +--- .../client/transport/TestNetworkConnection.java | 5 --- .../qpid/transport/network/NetworkConnection.java | 2 - .../transport/network/io/IoNetworkConnection.java | 43 ++++++++++++++-------- .../transport/network/io/IoNetworkTransport.java | 13 ------- .../network/io/IdleTimeoutTickerTest.java | 5 --- .../MultiVersionProtocolEngineFactoryTest.java | 5 --- .../client/protocol/AMQProtocolSessionTest.java | 5 --- 12 files changed, 53 insertions(+), 95 deletions(-) diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/protocol/MultiVersionProtocolEngine.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/protocol/MultiVersionProtocolEngine.java index b2b585f692..01b220fd79 100755 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/protocol/MultiVersionProtocolEngine.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/protocol/MultiVersionProtocolEngine.java @@ -30,6 +30,8 @@ import java.util.Set; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLPeerUnverifiedException; +import javax.net.ssl.SSLSocket; + import org.apache.log4j.Logger; import org.apache.qpid.protocol.ServerProtocolEngine; import org.apache.qpid.server.logging.actors.CurrentActor; @@ -144,11 +146,6 @@ public class MultiVersionProtocolEngine implements ServerProtocolEngine private static final int MINIMUM_REQUIRED_HEADER_BYTES = 8; - public void setNetworkConnection(NetworkConnection networkConnection) - { - setNetworkConnection(networkConnection, networkConnection.getSender()); - } - public void setNetworkConnection(NetworkConnection network, Sender sender) { _network = network; @@ -477,7 +474,7 @@ public class MultiVersionProtocolEngine implements ServerProtocolEngine SSLStatus sslStatus = new SSLStatus(); _sslReceiver = new SSLReceiver(_engine,_decryptEngine,sslStatus); _sslSender = new SSLBufferingSender(_engine,_sender,sslStatus); - _decryptEngine.setNetworkConnection(new SSLNetworkConnection(_engine,_network, _sslSender)); + _decryptEngine.setNetworkConnection(new SSLNetworkConnection(_engine,_network, _sslSender), _sslSender); } @Override @@ -594,6 +591,9 @@ public class MultiVersionProtocolEngine implements ServerProtocolEngine private final NetworkConnection _network; private final SSLBufferingSender _sslSender; private final SSLEngine _engine; + private Principal _principal; + private boolean _principalChecked; + private final Object _lock = new Object(); public SSLNetworkConnection(SSLEngine engine, NetworkConnection network, SSLBufferingSender sslSender) @@ -648,22 +648,26 @@ public class MultiVersionProtocolEngine implements ServerProtocolEngine _network.setMaxReadIdle(sec); } - @Override - public void setPeerPrincipal(Principal principal) - { - _network.setPeerPrincipal(principal); - } - @Override public Principal getPeerPrincipal() { - try + synchronized (_lock) { - return _engine.getSession().getPeerPrincipal(); - } - catch (SSLPeerUnverifiedException e) - { - return null; + if(!_principalChecked) + { + try + { + _principal = _engine.getSession().getPeerPrincipal(); + } + catch (SSLPeerUnverifiedException e) + { + _principal = null; + } + + _principalChecked = true; + } + + return _principal; } } diff --git a/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ProtocolEngine_0_10.java b/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ProtocolEngine_0_10.java index 9e41a5234c..b8fcdbfe6d 100755 --- a/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ProtocolEngine_0_10.java +++ b/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ProtocolEngine_0_10.java @@ -60,15 +60,8 @@ public class ProtocolEngine_0_10 extends InputHandler implements ServerProtocol if(network != null) { - setNetworkConnection(network); + setNetworkConnection(network, network.getSender()); } - - - } - - public void setNetworkConnection(NetworkConnection network) - { - setNetworkConnection(network, network.getSender()); } public void setNetworkConnection(NetworkConnection network, Sender sender) @@ -77,7 +70,6 @@ public class ProtocolEngine_0_10 extends InputHandler implements ServerProtocol _connection.setNetworkConnection(network); _connection.setSender(new Disassembler(wrapSender(sender), MAX_FRAME_SIZE)); - _connection.setPeerPrincipal(_network.getPeerPrincipal()); // FIXME Two log messages to maintain compatibility with earlier protocol versions _connection.getLogActor().message(ConnectionMessages.OPEN(null, null, null, null, false, false, false, false)); _connection.getLogActor().message(ConnectionMessages.OPEN(null, "0-10", null, null, false, true, false, false)); diff --git a/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnection.java b/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnection.java index 72d6a0832d..b4d591a72f 100644 --- a/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnection.java +++ b/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnection.java @@ -73,7 +73,6 @@ public class ServerConnection extends Connection implements AMQConnectionModel, private Port _port; private AtomicLong _lastIoTime = new AtomicLong(); private boolean _blocking; - private Principal _peerPrincipal; private NetworkConnection _networkConnection; private Transport _transport; private volatile boolean _stopped; @@ -529,12 +528,7 @@ public class ServerConnection extends Connection implements AMQConnectionModel, public Principal getPeerPrincipal() { - return _peerPrincipal; - } - - public void setPeerPrincipal(Principal peerPrincipal) - { - _peerPrincipal = peerPrincipal; + return _networkConnection.getPeerPrincipal(); } @Override diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/InternalTestProtocolSession.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/InternalTestProtocolSession.java index 5b1e5af7bd..f1dfc52ba4 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/InternalTestProtocolSession.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/InternalTestProtocolSession.java @@ -313,11 +313,6 @@ public class InternalTestProtocolSession extends AMQProtocolEngine implements Pr { } - @Override - public void setPeerPrincipal(Principal principal) - { - } - @Override public Principal getPeerPrincipal() { diff --git a/qpid/java/broker-plugins/websocket/src/main/java/org/apache/qpid/server/transport/websocket/WebSocketProvider.java b/qpid/java/broker-plugins/websocket/src/main/java/org/apache/qpid/server/transport/websocket/WebSocketProvider.java index b44ed70040..095ac993ae 100644 --- a/qpid/java/broker-plugins/websocket/src/main/java/org/apache/qpid/server/transport/websocket/WebSocketProvider.java +++ b/qpid/java/broker-plugins/websocket/src/main/java/org/apache/qpid/server/transport/websocket/WebSocketProvider.java @@ -267,15 +267,10 @@ class WebSocketProvider implements AcceptingTransport _maxReadIdle = sec; } - @Override - public void setPeerPrincipal(final Principal principal) - { - _principal = principal; - } - @Override public Principal getPeerPrincipal() { + //TODO: how do we populate this? return _principal; } diff --git a/qpid/java/client/src/test/java/org/apache/qpid/client/transport/TestNetworkConnection.java b/qpid/java/client/src/test/java/org/apache/qpid/client/transport/TestNetworkConnection.java index 1ec217e468..c9af1de6a7 100644 --- a/qpid/java/client/src/test/java/org/apache/qpid/client/transport/TestNetworkConnection.java +++ b/qpid/java/client/src/test/java/org/apache/qpid/client/transport/TestNetworkConnection.java @@ -74,11 +74,6 @@ public class TestNetworkConnection implements NetworkConnection } - @Override - public void setPeerPrincipal(Principal principal) - { - } - @Override public Principal getPeerPrincipal() { diff --git a/qpid/java/common/src/main/java/org/apache/qpid/transport/network/NetworkConnection.java b/qpid/java/common/src/main/java/org/apache/qpid/transport/network/NetworkConnection.java index 050d194c47..1b8bbebdf5 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/transport/network/NetworkConnection.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/transport/network/NetworkConnection.java @@ -47,8 +47,6 @@ public interface NetworkConnection void setMaxReadIdle(int sec); - void setPeerPrincipal(Principal principal); - Principal getPeerPrincipal(); int getMaxReadIdle(); diff --git a/qpid/java/common/src/main/java/org/apache/qpid/transport/network/io/IoNetworkConnection.java b/qpid/java/common/src/main/java/org/apache/qpid/transport/network/io/IoNetworkConnection.java index f5c09ac2cc..4a4bd3ddc0 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/transport/network/io/IoNetworkConnection.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/transport/network/io/IoNetworkConnection.java @@ -24,11 +24,14 @@ import java.net.Socket; import java.net.SocketAddress; import java.nio.ByteBuffer; import java.security.Principal; + +import javax.net.ssl.SSLPeerUnverifiedException; +import javax.net.ssl.SSLSocket; + import org.apache.qpid.transport.Receiver; import org.apache.qpid.transport.Sender; import org.apache.qpid.transport.network.Ticker; import org.apache.qpid.transport.network.NetworkConnection; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -39,15 +42,11 @@ public class IoNetworkConnection implements NetworkConnection private final long _timeout; private final IoSender _ioSender; private final IoReceiver _ioReceiver; - private Principal _principal; private int _maxReadIdle; private int _maxWriteIdle; - - public IoNetworkConnection(Socket socket, Receiver delegate, - int sendBufferSize, int receiveBufferSize, long timeout) - { - this(socket,delegate,sendBufferSize,receiveBufferSize,timeout,null); - } + private Principal _principal; + private boolean _principalChecked; + private final Object _lock = new Object(); public IoNetworkConnection(Socket socket, Receiver delegate, int sendBufferSize, int receiveBufferSize, long timeout, Ticker ticker) @@ -107,16 +106,30 @@ public class IoNetworkConnection implements NetworkConnection _maxReadIdle = sec; } - @Override - public void setPeerPrincipal(Principal principal) - { - _principal = principal; - } - @Override public Principal getPeerPrincipal() { - return _principal; + synchronized (_lock) + { + if(!_principalChecked) + { + if(_socket instanceof SSLSocket) + { + try + { + _principal = ((SSLSocket) _socket).getSession().getPeerPrincipal(); + } + catch(SSLPeerUnverifiedException e) + { + _principal = null; + } + } + + _principalChecked = true; + } + + return _principal; + } } @Override diff --git a/qpid/java/common/src/main/java/org/apache/qpid/transport/network/io/IoNetworkTransport.java b/qpid/java/common/src/main/java/org/apache/qpid/transport/network/io/IoNetworkTransport.java index 18a8bf2779..b584769de0 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/transport/network/io/IoNetworkTransport.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/transport/network/io/IoNetworkTransport.java @@ -245,19 +245,6 @@ public class IoNetworkTransport implements OutgoingNetworkTransport, IncomingNet ticker.setConnection(connection); - if(_sslContext != null && socket instanceof SSLSocket) - { - try - { - Principal peerPrincipal = ((SSLSocket) socket).getSession().getPeerPrincipal(); - connection.setPeerPrincipal(peerPrincipal); - } - catch(SSLPeerUnverifiedException e) - { - // ignore - } - } - engine.setNetworkConnection(connection, connection.getSender()); connection.start(); diff --git a/qpid/java/common/src/test/java/org/apache/qpid/transport/network/io/IdleTimeoutTickerTest.java b/qpid/java/common/src/test/java/org/apache/qpid/transport/network/io/IdleTimeoutTickerTest.java index 5cdd7a8597..a445cff0a7 100644 --- a/qpid/java/common/src/test/java/org/apache/qpid/transport/network/io/IdleTimeoutTickerTest.java +++ b/qpid/java/common/src/test/java/org/apache/qpid/transport/network/io/IdleTimeoutTickerTest.java @@ -232,11 +232,6 @@ public class IdleTimeoutTickerTest extends TestCase implements TransportActivity _maxReadIdle = sec; } - @Override - public void setPeerPrincipal(Principal principal) - { - } - @Override public Principal getPeerPrincipal() { diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/server/protocol/MultiVersionProtocolEngineFactoryTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/server/protocol/MultiVersionProtocolEngineFactoryTest.java index c999be634b..099e011054 100644 --- a/qpid/java/systests/src/main/java/org/apache/qpid/server/protocol/MultiVersionProtocolEngineFactoryTest.java +++ b/qpid/java/systests/src/main/java/org/apache/qpid/server/protocol/MultiVersionProtocolEngineFactoryTest.java @@ -253,11 +253,6 @@ public class MultiVersionProtocolEngineFactoryTest extends QpidTestCase { } - @Override - public void setPeerPrincipal(Principal principal) - { - } - @Override public Principal getPeerPrincipal() { diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/client/protocol/AMQProtocolSessionTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/client/protocol/AMQProtocolSessionTest.java index e893a58282..3ffa73b9b7 100644 --- a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/client/protocol/AMQProtocolSessionTest.java +++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/client/protocol/AMQProtocolSessionTest.java @@ -153,11 +153,6 @@ public class AMQProtocolSessionTest extends QpidBrokerTestCase { } - @Override - public void setPeerPrincipal(Principal principal) - { - } - @Override public Principal getPeerPrincipal() { -- cgit v1.2.1