Explorar o código

Fix duplicate client sessionID handling

- registerEstablishedClient closes any existing
  client when a new client connects with the
  same sessionID; but the old client still calls
  unregisterEstablishedClient as it exits, and
  that removed the _new_ client from the client
  list and closed it too.
- Add log indicating when duplicate client sessionID
  case occurs.
Rod Hynes %!s(int64=9) %!d(string=hai) anos
pai
achega
af052b2c62
Modificáronse 1 ficheiros con 17 adicións e 7 borrados
  1. 17 7
      psiphon/server/tunnelServer.go

+ 17 - 7
psiphon/server/tunnelServer.go

@@ -437,27 +437,37 @@ func (sshServer *sshServer) registerEstablishedClient(client *sshClient) bool {
 	existingClient := sshServer.clients[client.sessionID]
 
 	sshServer.clients[client.sessionID] = client
+
 	sshServer.clientsMutex.Unlock()
 
 	// Call stop() outside the mutex to avoid deadlock.
 	if existingClient != nil {
 		existingClient.stop()
+		log.WithContext().Info(
+			"stopped existing client with duplicate session ID")
 	}
 
 	return true
 }
 
-func (sshServer *sshServer) unregisterEstablishedClient(sessionID string) {
+func (sshServer *sshServer) unregisterEstablishedClient(client *sshClient) {
 
 	sshServer.clientsMutex.Lock()
-	client := sshServer.clients[sessionID]
-	delete(sshServer.clients, sessionID)
+
+	registeredClient := sshServer.clients[client.sessionID]
+
+	// registeredClient will differ from client when client
+	// is the existingClient terminated in registerEstablishedClient.
+	// In that case, registeredClient remains connected, and
+	// the sshServer.clients entry should be retained.
+	if registeredClient == client {
+		delete(sshServer.clients, client.sessionID)
+	}
+
 	sshServer.clientsMutex.Unlock()
 
 	// Call stop() outside the mutex to avoid deadlock.
-	if client != nil {
-		client.stop()
-	}
+	client.stop()
 }
 
 type ProtocolStats map[string]map[string]int64
@@ -868,7 +878,7 @@ func (sshClient *sshClient) run(clientConn net.Conn) {
 	// Note: sshServer.unregisterEstablishedClient calls sshClient.stop(),
 	// which also closes underlying transport Conn.
 
-	sshClient.sshServer.unregisterEstablishedClient(sshClient.sessionID)
+	sshClient.sshServer.unregisterEstablishedClient(sshClient)
 
 	sshClient.logTunnel()