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

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<ByteBuffer> 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)
@@ -649,21 +649,25 @@ public class MultiVersionProtocolEngine implements ServerProtocolEngine
         }
 
         @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<ByteBuffer> 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
@@ -314,11 +314,6 @@ public class InternalTestProtocolSession extends AMQProtocolEngine implements Pr
         }
 
         @Override
-        public void setPeerPrincipal(Principal principal)
-        {
-        }
-
-        @Override
         public Principal getPeerPrincipal()
         {
             return null;
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
@@ -268,14 +268,9 @@ class WebSocketProvider implements AcceptingTransport
         }
 
         @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
@@ -75,11 +75,6 @@ public class TestNetworkConnection implements NetworkConnection
     }
 
     @Override
-    public void setPeerPrincipal(Principal principal)
-    {
-    }
-
-    @Override
     public Principal getPeerPrincipal()
     {
         return null;
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<ByteBuffer> 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<ByteBuffer> delegate,
             int sendBufferSize, int receiveBufferSize, long timeout, Ticker ticker)
@@ -108,15 +107,29 @@ public class IoNetworkConnection implements NetworkConnection
     }
 
     @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
@@ -233,11 +233,6 @@ public class IdleTimeoutTickerTest extends TestCase implements TransportActivity
     }
 
     @Override
-    public void setPeerPrincipal(Principal principal)
-    {
-    }
-
-    @Override
     public Principal getPeerPrincipal()
     {
         return null;
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
@@ -254,11 +254,6 @@ public class MultiVersionProtocolEngineFactoryTest extends QpidTestCase
         }
 
         @Override
-        public void setPeerPrincipal(Principal principal)
-        {
-        }
-
-        @Override
         public Principal getPeerPrincipal()
         {
             return null;
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
@@ -154,11 +154,6 @@ public class AMQProtocolSessionTest extends QpidBrokerTestCase
         }
 
         @Override
-        public void setPeerPrincipal(Principal principal)
-        {
-        }
-
-        @Override
         public Principal getPeerPrincipal()
         {
             return null;