* Stop creating a circuit if the client fails authentication (i.e. the region server wasn't told that it was coming)

* This moves authentication from the client thread (where failure was difficult to detect) to the particular thread handling that packet
* I've kept the authentication outside of the crucial clientCircuits lock (though any delay here is probably swamped by the other delays associated with login)
* Also added more to the unit test to ensure this doesn't regress

4 files changed, 109 insertions, 69 deletions

diff --git a/OpenSim/Region/ClientStack/LindenUDP/LLClientView.cs b/OpenSim/Region/ClientStack/LindenUDP/LLClientView.cs
index fc76086..b517c13 100644
--- a/OpenSim/Region/ClientStack/LindenUDP/LLClientView.cs
+++ b/OpenSim/Region/ClientStack/LindenUDP/LLClientView.cs
@@ -105,7 +105,6 @@ namespace OpenSim.Region.ClientStack.LindenUDP
         protected Dictionary<PacketType, PacketMethod> m_packetHandlers = new Dictionary<PacketType, PacketMethod>();
 
         protected IScene m_scene;
-        protected AgentCircuitManager m_authenticateSessionsHandler;
 
         protected LLPacketServer m_networkServer;
 
@@ -409,7 +408,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP
         /// <param name="proxyEP"></param>
         public LLClientView(
             EndPoint remoteEP, IScene scene, AssetCache assetCache, LLPacketServer packServer,
-            AgentCircuitManager authenSessions, UUID agentId, UUID sessionId, uint circuitCode, EndPoint proxyEP,
+            AuthenticateResponse sessionInfo, UUID agentId, UUID sessionId, uint circuitCode, EndPoint proxyEP,
             ClientStackUserSettings userSettings)
         {
             m_moneyBalance = 1000;
@@ -422,17 +421,22 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             m_assetCache = assetCache;
 
             m_networkServer = packServer;
-            // m_inventoryCache = inventoryCache;
-            m_authenticateSessionsHandler = authenSessions;
 
             m_agentId = agentId;
             m_sessionId = sessionId;
             m_circuitCode = circuitCode;
 
             m_userEndPoint = remoteEP;
-            m_proxyEndPoint = proxyEP;
+            m_proxyEndPoint = proxyEP;           
+            
+            m_firstName = sessionInfo.LoginInfo.First;
+            m_lastName = sessionInfo.LoginInfo.Last;
+            m_startpos = sessionInfo.LoginInfo.StartPos;
 
-            m_startpos = m_authenticateSessionsHandler.GetPosition(circuitCode);
+            if (sessionInfo.LoginInfo.SecureSession != UUID.Zero)
+            {
+                m_secureSessionId = sessionInfo.LoginInfo.SecureSession;
+            }
 
             // While working on this, the BlockingQueue had me fooled for a bit.
             // The Blocking queue causes the thread to stop until there's something
@@ -444,7 +448,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP
 
             RegisterLocalPacketHandlers();
 
-            m_clientThread = new Thread(new ThreadStart(AuthUser));
+            m_clientThread = new Thread(new ThreadStart(Start));
             m_clientThread.Name = "ClientThread";
             m_clientThread.IsBackground = true;
             m_clientThread.Start();
@@ -759,9 +763,9 @@ namespace OpenSim.Region.ClientStack.LindenUDP
         }
 
         /// <summary>
-        /// Authorize an incoming user session.  This method lies at the base of the entire client thread.
+        /// Start a user session.  This method lies at the base of the entire client thread.
         /// </summary>
-        protected virtual void AuthUser()
+        protected virtual void Start()
         {
             //tell this thread we are using the culture set up for the sim (currently hardcoded to en_US)
             //otherwise it will override this and use the system default
@@ -769,37 +773,9 @@ namespace OpenSim.Region.ClientStack.LindenUDP
 
             try
             {
-                // AuthenticateResponse sessionInfo = m_gridServer.AuthenticateSession(m_cirpack.m_circuitCode.m_sessionId, m_cirpack.m_circuitCode.ID, m_cirpack.m_circuitCode.Code);
-                AuthenticateResponse sessionInfo =
-                    m_authenticateSessionsHandler.AuthenticateSession(m_sessionId, m_agentId, m_circuitCode);
-
-                if (!sessionInfo.Authorised)
-                {
-                    //session/circuit not authorised
-                    m_log.WarnFormat(
-                        "[CLIENT]: New user request denied to avatar {0} connecting with circuit code {1} from {2}",
-                        m_agentId, m_circuitCode, m_userEndPoint);
-
-                    m_PacketHandler.Stop();
-                    m_clientThread.Abort();
-                }
-                else
-                {
-                    m_log.Info("[CLIENT]: Got authenticated connection from " + m_userEndPoint.ToString());
-                    //session is authorised
-                    m_firstName = sessionInfo.LoginInfo.First;
-                    m_lastName = sessionInfo.LoginInfo.Last;
-
-                    if (sessionInfo.LoginInfo.SecureSession != UUID.Zero)
-                    {
-                        m_secureSessionId = sessionInfo.LoginInfo.SecureSession;
-                    }
-
-                    // This sets up all the timers
-                    InitNewClient();
-
-                    ClientLoop();
-                }
+                // This sets up all the timers
+                InitNewClient();
+                ClientLoop();
             }                
             catch (System.Exception e)
             {
diff --git a/OpenSim/Region/ClientStack/LindenUDP/LLPacketServer.cs b/OpenSim/Region/ClientStack/LindenUDP/LLPacketServer.cs
index 5d3dba0..e3a02bc 100644
--- a/OpenSim/Region/ClientStack/LindenUDP/LLPacketServer.cs
+++ b/OpenSim/Region/ClientStack/LindenUDP/LLPacketServer.cs
@@ -27,6 +27,8 @@
 
 using System.Net;
 using System.Net.Sockets;
+using System.Reflection;
+using log4net;
 using OpenMetaverse;
 using OpenMetaverse.Packets;
 using OpenSim.Framework;
@@ -36,8 +38,8 @@ namespace OpenSim.Region.ClientStack.LindenUDP
 {
     public class LLPacketServer
     {
-        //private static readonly log4net.ILog m_log
-        //    = log4net.LogManager.GetLogger(System.Reflection.MethodBase.GetCurrentMethod().DeclaringType);
+        private static readonly log4net.ILog m_log
+            = log4net.LogManager.GetLogger(System.Reflection.MethodBase.GetCurrentMethod().DeclaringType);
 
         protected readonly ILLClientStackNetworkHandler m_networkHandler;
         protected IScene m_scene;
@@ -87,49 +89,77 @@ namespace OpenSim.Region.ClientStack.LindenUDP
         /// <returns></returns>
         protected virtual IClientAPI CreateNewCircuit(EndPoint remoteEP, UseCircuitCodePacket initialcirpack,
                                                       ClientManager clientManager, IScene scene, AssetCache assetCache,
-                                                      LLPacketServer packServer, AgentCircuitManager authenSessions,
+                                                      LLPacketServer packServer, AuthenticateResponse sessionInfo,
                                                       UUID agentId, UUID sessionId, uint circuitCode, EndPoint proxyEP)
         {
             return
                 new LLClientView(
-                     remoteEP, scene, assetCache, packServer, authenSessions, agentId, sessionId, circuitCode, proxyEP,
+                     remoteEP, scene, assetCache, packServer, sessionInfo, agentId, sessionId, circuitCode, proxyEP,
                      m_userSettings);
         }
 
         /// <summary>
+        /// Check whether a given client is authorized to connect
+        /// </summary>
+        /// <param name="useCircuit"></param>
+        /// <param name="circuitManager"></param>
+        /// <returns></returns>
+        public virtual bool IsClientAuthorized(
+            UseCircuitCodePacket useCircuit, AgentCircuitManager circuitManager, out AuthenticateResponse sessionInfo)
+        {
+            UUID agentId = useCircuit.CircuitCode.ID;
+            UUID sessionId = useCircuit.CircuitCode.SessionID;
+            uint circuitCode = useCircuit.CircuitCode.Code;
+
+            sessionInfo = circuitManager.AuthenticateSession(sessionId, agentId, circuitCode); 
+
+            if (!sessionInfo.Authorised)
+                return false;  
+            
+            return true;
+        }
+
+        /// <summary>
         /// Add a new client circuit
         /// </summary>
         /// <param name="epSender"></param>
         /// <param name="useCircuit"></param>
         /// <param name="assetCache"></param>
-        /// <param name="circuitManager"></param>
+        /// <param name="sessionInfo"></param>
         /// <param name="proxyEP"></param>
         /// <returns>
         /// true if a new circuit was created, false if a circuit with the given circuit code already existed
-        /// </returns>
-        public virtual bool AddNewClient(EndPoint epSender, UseCircuitCodePacket useCircuit, AssetCache assetCache,
-                                         AgentCircuitManager circuitManager, EndPoint proxyEP)
+        /// </returns>        
+        public virtual bool AddNewClient(
+            EndPoint epSender, UseCircuitCodePacket useCircuit, AssetCache assetCache,
+            AuthenticateResponse sessionInfo, EndPoint proxyEP)
         {
             IClientAPI newuser;
-
-            if (m_scene.ClientManager.TryGetClient(useCircuit.CircuitCode.Code, out newuser))
+            uint circuitCode = useCircuit.CircuitCode.Code;
+            
+            if (m_scene.ClientManager.TryGetClient(circuitCode, out newuser))
             {
+                // The circuit is already known to the scene.  This not actually a problem since this will currently
+                // occur if a client is crossing borders (hence upgrading its circuit).  However, we shouldn't 
+                // really by trying to add a new client if this is the case.
                 return false;
             }
-            else
-            {
-                newuser = CreateNewCircuit(epSender, useCircuit, m_scene.ClientManager, m_scene, assetCache, this,
-                                           circuitManager, useCircuit.CircuitCode.ID,
-                                           useCircuit.CircuitCode.SessionID, useCircuit.CircuitCode.Code, proxyEP);
+            
+            UUID agentId = useCircuit.CircuitCode.ID;
+            UUID sessionId = useCircuit.CircuitCode.SessionID;
+            
+            newuser 
+                = CreateNewCircuit(
+                    epSender, useCircuit, m_scene.ClientManager, m_scene, assetCache, this, sessionInfo, 
+                    agentId, sessionId, circuitCode, proxyEP);
 
-                m_scene.ClientManager.Add(useCircuit.CircuitCode.Code, newuser);
+            m_scene.ClientManager.Add(circuitCode, newuser);
 
-                newuser.OnViewerEffect += m_scene.ClientManager.ViewerEffectHandler;
-                newuser.OnLogout += LogoutHandler;
-                newuser.OnConnectionClosed += CloseClient;
+            newuser.OnViewerEffect += m_scene.ClientManager.ViewerEffectHandler;
+            newuser.OnLogout += LogoutHandler;
+            newuser.OnConnectionClosed += CloseClient;
 
-                return true;
-            }
+            return true;
         }
 
         public void LogoutHandler(IClientAPI client)
diff --git a/OpenSim/Region/ClientStack/LindenUDP/LLUDPServer.cs b/OpenSim/Region/ClientStack/LindenUDP/LLUDPServer.cs
index 2e9af74..40b4a42 100644
--- a/OpenSim/Region/ClientStack/LindenUDP/LLUDPServer.cs
+++ b/OpenSim/Region/ClientStack/LindenUDP/LLUDPServer.cs
@@ -385,11 +385,25 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             //Slave regions don't accept new clients
             if (m_localScene.Region_Status != RegionStatus.SlaveScene)
             {                
+                AuthenticateResponse sessionInfo;
                 bool isNewCircuit = false;
                 
+                if (!m_packetServer.IsClientAuthorized(useCircuit, m_circuitManager, out sessionInfo))
+                {
+                    m_log.WarnFormat(
+                        "[CLIENT]: New user request denied to avatar {0} connecting with unauthorized circuit code {1} from {2}",
+                        useCircuit.CircuitCode.ID, useCircuit.CircuitCode.Code, epSender);
+                    
+                    return;
+                }
+                else
+                {
+                    m_log.Info("[CLIENT]: Got authenticated connection from " + epSender);
+                }
+                
                 lock (clientCircuits)
                 {
-                    if (!clientCircuits.ContainsKey(epSender))  
+                    if (!clientCircuits.ContainsKey(epSender))
                     {
                         m_log.DebugFormat(
                             "[CLIENT]: Adding new circuit for agent {0}, circuit code {1}", 
@@ -409,8 +423,8 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                     {
                         proxyCircuits[useCircuit.CircuitCode.Code] = epProxy;
                     }
-
-                    m_packetServer.AddNewClient(epSender, useCircuit, m_assetCache, m_circuitManager, epProxy);
+                    
+                    m_packetServer.AddNewClient(epSender, useCircuit, m_assetCache, sessionInfo, epProxy);
                 }
             }            
             
@@ -533,6 +547,17 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             useCircuit.CircuitCode.Code = circuit.circuitcode;
             useCircuit.CircuitCode.ID = circuit.AgentID;
             useCircuit.CircuitCode.SessionID = circuit.SessionID;
+            
+            AuthenticateResponse sessionInfo;
+            
+            if (!m_packetServer.IsClientAuthorized(useCircuit, m_circuitManager, out sessionInfo))
+            {
+                m_log.WarnFormat(
+                    "[CLIENT]: Restore request denied to avatar {0} connecting with unauthorized circuit code {1}",
+                    useCircuit.CircuitCode.ID, useCircuit.CircuitCode.Code);
+                
+                return;
+            }               
 
             lock (clientCircuits)
             {
@@ -562,7 +587,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                 }
             }
 
-            m_packetServer.AddNewClient(userEP, useCircuit, m_assetCache, m_circuitManager, proxyEP);
+            m_packetServer.AddNewClient(userEP, useCircuit, m_assetCache, sessionInfo, proxyEP);
         }
     }
 }
diff --git a/OpenSim/Region/ClientStack/LindenUDP/Tests/BasicCircuitTests.cs b/OpenSim/Region/ClientStack/LindenUDP/Tests/BasicCircuitTests.cs
index c90aecf..819e4e8 100644
--- a/OpenSim/Region/ClientStack/LindenUDP/Tests/BasicCircuitTests.cs
+++ b/OpenSim/Region/ClientStack/LindenUDP/Tests/BasicCircuitTests.cs
@@ -26,6 +26,7 @@
  */
 
 using System.Net;
+//using System.Threading;
 using log4net;
 using NUnit.Framework;
 using OpenMetaverse;
@@ -69,8 +70,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP.Tests
             AgentCircuitManager acm = new AgentCircuitManager();
             AgentCircuitData acd = new AgentCircuitData();
             acd.AgentID = myAgentUuid;
-            acd.SessionID = mySessionUuid;            
-            acm.AddNewCircuit(myCircuitCode, acd);
+            acd.SessionID = mySessionUuid;                        
             
             uint port = 666;            
             testLLUDPServer.Initialise(null, ref port, 0, false, userSettings, null, acm);
@@ -89,10 +89,19 @@ namespace OpenSim.Region.ClientStack.LindenUDP.Tests
             EndPoint testEp = new IPEndPoint(IPAddress.Loopback, 999);
             
             testLLUDPServer.LoadReceive(uccp, testEp);            
-            testLLUDPServer.ReceiveData(null);        
+            testLLUDPServer.ReceiveData(null);
             
-            Assert.IsFalse(testLLUDPServer.HasCircuit(101));
+            // Circuit shouildn't exist since the circuit manager doesn't know about this circuit for authentication yet
+            Assert.IsFalse(testLLUDPServer.HasCircuit(myCircuitCode));
+                        
+            acm.AddNewCircuit(myCircuitCode, acd);
+            
+            testLLUDPServer.LoadReceive(uccp, testEp);            
+            testLLUDPServer.ReceiveData(null);            
+            
+            // Should succeed now
             Assert.IsTrue(testLLUDPServer.HasCircuit(myCircuitCode));
+            Assert.IsFalse(testLLUDPServer.HasCircuit(101));
         }
     }
 }