[tor-commits] [snowflake/master] Create new smux session for each SOCKS connection

[cohosh at torproject.org]

commit 114df695ceff25d0213200a3368ed7a1bb1c7668
Author: Cecylia Bocovich <cohosh at torproject.org>
Date:   Mon Nov 23 14:02:54 2020 -0500

    Create new smux session for each SOCKS connection
    
    Each SOCKS connection has its own set of snowflakes and broker poll
    loop. Since the session manager was tied to a single set of snowflakes,
    this resulted in a bug where RedialPacketConn would sometimes try to
    pull snowflakes from a previously melted pool. The fix is to maintain
    separate smux sessions for each SOCKS connection, tied to its own
    snowflake pool.
---
 client/lib/snowflake.go | 57 ++++++-------------------------------------------
 1 file changed, 6 insertions(+), 51 deletions(-)

diff --git a/client/lib/snowflake.go b/client/lib/snowflake.go
index e888160..171e173 100644
--- a/client/lib/snowflake.go
+++ b/client/lib/snowflake.go
@@ -6,7 +6,6 @@ import (
 	"io"
 	"log"
 	"net"
-	"sync"
 	"time"
 
 	"git.torproject.org/pluggable-transports/snowflake.git/common/turbotunnel"
@@ -92,54 +91,6 @@ func newSession(snowflakes SnowflakeCollector) (net.PacketConn, *smux.Session, e
 	return pconn, sess, err
 }
 
-// sessionManager_ maintains a single global smux.Session that is shared among
-// incoming SOCKS connections.
-type sessionManager_ struct {
-	mutex sync.Mutex
-	sess  *smux.Session
-}
-
-// Get creates and returns a new global smux.Session if none exists yet. If one
-// already exists, it returns the existing one. It monitors the returned session
-// and if it ever fails, sets things up so the next call to Get will create a
-// new session.
-func (manager *sessionManager_) Get(snowflakes SnowflakeCollector) (*smux.Session, error) {
-	manager.mutex.Lock()
-	defer manager.mutex.Unlock()
-
-	if manager.sess == nil {
-		log.Printf("starting a new session")
-		pconn, sess, err := newSession(snowflakes)
-		if err != nil {
-			return nil, err
-		}
-		manager.sess = sess
-		go func() {
-			// If the session dies, set it to be recreated.
-			for {
-				<-time.After(5 * time.Second)
-				if sess.IsClosed() {
-					break
-				}
-			}
-			log.Printf("discarding finished session")
-			// Close the underlying to force any ongoing WebRTC
-			// connection to close as well, and relinquish the
-			// SnowflakeCollector.
-			pconn.Close()
-			manager.mutex.Lock()
-			manager.sess = nil
-			manager.mutex.Unlock()
-		}()
-	} else {
-		log.Printf("reusing the existing session")
-	}
-
-	return manager.sess, nil
-}
-
-var sessionManager = sessionManager_{}
-
 // Given an accepted SOCKS connection, establish a WebRTC connection to the
 // remote peer and exchange traffic.
 func Handler(socks net.Conn, tongue Tongue) error {
@@ -155,8 +106,9 @@ func Handler(socks net.Conn, tongue Tongue) error {
 	log.Printf("---- Handler: begin collecting snowflakes ---")
 	go connectLoop(snowflakes)
 
-	// Return the global smux.Session.
-	sess, err := sessionManager.Get(snowflakes)
+	// Create a new smux session
+	log.Printf("---- Handler: starting a new session ---")
+	pconn, sess, err := newSession(snowflakes)
 	if err != nil {
 		return err
 	}
@@ -174,6 +126,9 @@ func Handler(socks net.Conn, tongue Tongue) error {
 	log.Printf("---- Handler: closed stream %v ---", stream.ID())
 	snowflakes.End()
 	log.Printf("---- Handler: end collecting snowflakes ---")
+	pconn.Close()
+	sess.Close()
+	log.Printf("---- Handler: discarding finished session ---")
 	return nil
 }