Bug fixes

- Add includeServerSideOnly to tactics cache key scope.
- Immediately log listener errors.

psiphon/common/tactics/tactics.go

@@ -1000,7 +1000,7 @@ func (server *Server) getTactics(
 	// TODO: log cache metrics; similar to what is done in
 	// psiphon/server.ServerTacticsParametersCache.GetMetrics.
 
-	cacheKey := getCacheKey(filterMatchCount > 0, filterMatches)
+	cacheKey := getCacheKey(includeServerSideOnly, filterMatchCount > 0, filterMatches)
 
 	cacheValue, ok := server.cachedTacticsData.Get(cacheKey)
 	if ok {
@@ -1029,18 +1029,28 @@ func (server *Server) getTactics(
 	return tacticsData, nil
 }
 
-func getCacheKey(hasFilterMatches bool, filterMatches []bool) string {
-	// When no filters match, the key is "". The input hasFilterMatches allows
-	// for skipping the strings.Builder setup and loop entirely.
+func getCacheKey(
+	includeServerSideOnly bool, hasFilterMatches bool, filterMatches []bool) string {
+
+	prefix := "0-"
+	if includeServerSideOnly {
+		prefix = "1-"
+	}
+
+	// hasFilterMatches allows for skipping the strings.Builder setup and loop
+	// entirely.
 	if !hasFilterMatches {
-		return ""
+		return prefix
 	}
+
 	var b strings.Builder
+	_, _ = b.WriteString(prefix)
 	for filterIndex, match := range filterMatches {
 		if match {
 			fmt.Fprintf(&b, "%x-", filterIndex)
 		}
 	}
+
 	return b.String()
 }
 

psiphon/common/tactics/tactics_test.go

@@ -343,7 +343,9 @@ func TestTactics(t *testing.T) {
 		}
 
 		for _, filterMatches := range cacheEntryFilterMatches {
-			cacheKey := getCacheKey(true, filterMatches)
+			includeServerSizeOnly := false
+			hasFilterMatches := true
+			cacheKey := getCacheKey(includeServerSizeOnly, hasFilterMatches, filterMatches)
 			_, ok := server.cachedTacticsData.Get(cacheKey)
 			if !ok {
 				t.Fatalf("Unexpected missing cachedTacticsData entry: %v", filterMatches)

psiphon/server/demux.go

@@ -68,7 +68,11 @@ type protocolClassifier struct {
 // Limitation: the conn is also closed after reading maxBytesToMatch and
 // failing to find a match, which can be a fingerprint for a raw conn with no
 // preceding anti-probing measure, such as TLS passthrough.
-func newProtocolDemux(ctx context.Context, listener net.Listener, classifiers []protocolClassifier, connClassificationTimeout time.Duration) (*protocolDemux, []protoListener) {
+func newProtocolDemux(
+	ctx context.Context,
+	listener net.Listener,
+	classifiers []protocolClassifier,
+	connClassificationTimeout time.Duration) (*protocolDemux, []protoListener) {
 
 	ctx, cancelFunc := context.WithCancel(ctx)
 

psiphon/server/listener.go

@@ -125,7 +125,9 @@ func (listener *TacticsListener) accept() (net.Conn, error) {
 	// See the comment in server.LoadConfig regarding provider ID limitations.
 	if protocol.TunnelProtocolIsDirect(listener.tunnelProtocol) &&
 		common.ContainsAny(
-			p.KeyStrings(parameters.RestrictDirectProviderRegions, listener.support.Config.GetProviderID()), []string{"", listener.support.Config.GetRegion()}) {
+			p.KeyStrings(parameters.RestrictDirectProviderRegions,
+				listener.support.Config.GetProviderID()),
+			[]string{"", listener.support.Config.GetRegion()}) {
 
 		if p.WeightedCoinFlip(
 			parameters.RestrictDirectProviderIDsServerProbability) {

psiphon/server/tunnelServer.go

@@ -51,6 +51,7 @@ import (
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/protocol"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/quic"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/refraction"
+	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/stacktrace"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/tactics"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/transforms"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/tun"
@@ -553,6 +554,23 @@ type additionalTransportData struct {
 	steeringIP             string
 }
 
+// reportListenerError logs a listener error and sends it the
+// TunnelServer.Run. Callers should wrap the input err in an immediate
+// errors.Trace.
+func reportListenerError(listenerError chan<- error, err error) {
+
+	// Record "caller" just in case the caller fails to wrap err in an
+	// errors.Trace.
+	log.WithTraceFields(
+		LogFields{
+			"error":  err,
+			"caller": stacktrace.GetParentFunctionName()}).Error("listener error")
+	select {
+	case listenerError <- err:
+	default:
+	}
+}
+
 // runListener is intended to run an a goroutine; it blocks
 // running a particular listener. If an unrecoverable error
 // occurs, it will send the error to the listenerError channel.
@@ -624,10 +642,7 @@ func (sshServer *sshServer) runListener(sshListener *sshListener, listenerError
 			}
 
 			if err != nil {
-				select {
-				case listenerError <- errors.Trace(err):
-				default:
-				}
+				reportListenerError(listenerError, errors.Trace(err))
 				return
 			}
 		}
@@ -673,10 +688,7 @@ func (sshServer *sshServer) runMeekTLSOSSHDemuxListener(
 		sshListener.tunnelProtocol,
 		sshListener.port)
 	if err != nil {
-		select {
-		case listenerError <- errors.Trace(err):
-		default:
-		}
+		reportListenerError(listenerError, errors.Trace(err))
 		return
 	}
 
@@ -713,10 +725,7 @@ func (sshServer *sshServer) runMeekTLSOSSHDemuxListener(
 
 		err := mux.run()
 		if err != nil {
-			select {
-			case listenerError <- errors.Trace(err):
-			default:
-			}
+			reportListenerError(listenerError, errors.Trace(err))
 			return
 		}
 	}()
@@ -764,10 +773,7 @@ func (sshServer *sshServer) runMeekTLSOSSHDemuxListener(
 		}
 
 		if err != nil {
-			select {
-			case listenerError <- errors.Trace(err):
-			default:
-			}
+			reportListenerError(listenerError, errors.Trace(err))
 			return
 		}
 	}()
@@ -805,10 +811,7 @@ func runListener(
 				continue
 			}
 
-			select {
-			case listenerError <- errors.Trace(err):
-			default:
-			}
+			reportListenerError(listenerError, errors.Trace(err))
 			return
 		}