Bug fixes

Flagged by golangci-lint/ineffassign.

- Bug: client broker packet relay only relayed one packet

- Bug: packetman randomized TCP flag transform not properly applied

- Bug: ParametersAccessor.Close had no effect

- Avoid allocation in udpgwPortForward.relayDownstream buffer pool

- Add/fix error return checks

- Remove some unused code

psiphon/common/authPackage.go

@@ -408,7 +408,11 @@ func (streamer *limitedJSONStreamer) Stream() error {
 
 			case stateJSONSeekingStringValueStart:
 				if b == '"' {
-					state = stateJSONSeekingStringValueEnd
+
+					// Note: this assignment is flagged by github.com/gordonklaus/ineffassign,
+					// but is technically the correct state.
+					//
+					//state = stateJSONSeekingStringValueEnd
 
 					key := keyBuffer.String()
 

psiphon/common/inproxy/client.go

@@ -44,7 +44,6 @@ const (
 // initial dial address.
 type ClientConn struct {
 	config       *ClientConfig
-	brokerClient *BrokerClient
 	webRTCConn   *webRTCConn
 	connectionID ID
 	remoteAddr   net.Addr

psiphon/common/inproxy/session_test.go

@@ -347,7 +347,7 @@ func runTestSessions() error {
 
 	request = roundTripper.MakeRequest()
 
-	response, err = unknownInitiatorSessions.RoundTrip(
+	_, err = unknownInitiatorSessions.RoundTrip(
 		ctx,
 		roundTripper,
 		responderPublicKey,
@@ -674,7 +674,6 @@ func runTestNoise() error {
 		return errors.Trace(err)
 	}
 
-	receivedPayload = nil
 	receivedPayload, _, _, err = initiatorHandshake.ReadMessage(nil, responderMsg)
 	if err != nil {
 		return errors.Trace(err)

psiphon/common/packetman/packetman.go

@@ -249,12 +249,15 @@ func (spec *compiledSpec) apply(interceptedPacket gopacket.Packet) ([][]byte, er
 		tmpDataOffset := transformedTCP.DataOffset
 		tmpChecksum := transformedTCP.Checksum
 
-		gopacket.SerializeLayers(
+		err = gopacket.SerializeLayers(
 			buffer,
 			options,
 			serializableNetworkLayer,
 			&transformedTCP,
 			payload)
+		if err != nil {
+			return nil, errors.Trace(err)
+		}
 
 		// In the first SerializeLayers call, all IP and TCP length and checksums
 		// are recalculated and set to the correct values with transformations
@@ -268,13 +271,19 @@ func (spec *compiledSpec) apply(interceptedPacket gopacket.Packet) ([][]byte, er
 		if setCalculatedField {
 			transformedTCP.DataOffset = tmpDataOffset
 			transformedTCP.Checksum = tmpChecksum
-			buffer.Clear()
-			gopacket.SerializeLayers(
+			err = buffer.Clear()
+			if err != nil {
+				return nil, errors.Trace(err)
+			}
+			err = gopacket.SerializeLayers(
 				buffer,
 				gopacket.SerializeOptions{FixLengths: fixLengths, ComputeChecksums: computeChecksums},
 				serializableNetworkLayer,
 				&transformedTCP,
 				payload)
+			if err != nil {
+				return nil, errors.Trace(err)
+			}
 		}
 
 		packets[i] = buffer.Bytes()
@@ -388,15 +397,15 @@ func (t *transformationTCPFlags) apply(tcp *layers.TCP, _ *gopacket.Payload) {
 		flags = t.flags
 	}
 
-	tcp.FIN = strings.Index(t.flags, "F") != -1
-	tcp.SYN = strings.Index(t.flags, "S") != -1
-	tcp.RST = strings.Index(t.flags, "R") != -1
-	tcp.PSH = strings.Index(t.flags, "P") != -1
-	tcp.ACK = strings.Index(t.flags, "A") != -1
-	tcp.URG = strings.Index(t.flags, "U") != -1
-	tcp.ECE = strings.Index(t.flags, "E") != -1
-	tcp.CWR = strings.Index(t.flags, "C") != -1
-	tcp.NS = strings.Index(t.flags, "N") != -1
+	tcp.FIN = strings.Contains(flags, "F")
+	tcp.SYN = strings.Contains(flags, "S")
+	tcp.RST = strings.Contains(flags, "R")
+	tcp.PSH = strings.Contains(flags, "P")
+	tcp.ACK = strings.Contains(flags, "A")
+	tcp.URG = strings.Contains(flags, "U")
+	tcp.ECE = strings.Contains(flags, "E")
+	tcp.CWR = strings.Contains(flags, "C")
+	tcp.NS = strings.Contains(flags, "N")
 }
 
 type transformationTCPField struct {

psiphon/common/parameters/parameters.go

@@ -1687,7 +1687,10 @@ func (p ParametersAccessor) IsNil() bool {
 // where memory footprint is a concern, and where the ParametersAccessor is
 // not immediately going out of scope. After Close is called, all other
 // ParametersAccessor functions will panic if called.
-func (p ParametersAccessor) Close() {
+//
+// Limitation: since ParametersAccessor is typically passed by value, this
+// Close call only impacts the immediate copy.
+func (p *ParametersAccessor) Close() {
 	p.snapshot = nil
 }
 

psiphon/common/prng/prng.go

@@ -198,7 +198,7 @@ func (p *PRNG) Int63() int64 {
 // Uint64 is equivilent to math/rand.Uint64.
 func (p *PRNG) Uint64() uint64 {
 	var b [8]byte
-	p.Read(b[:])
+	_, _ = p.Read(b[:])
 	return binary.BigEndian.Uint64(b[:])
 }
 
@@ -300,7 +300,7 @@ func (p *PRNG) RangeUint32(min, max uint32) uint32 {
 // Bytes returns a new slice containing length random bytes.
 func (p *PRNG) Bytes(length int) []byte {
 	b := make([]byte, length)
-	p.Read(b)
+	_, _ = p.Read(b)
 	return b
 }
 

psiphon/common/quic/obfuscator.go

@@ -270,7 +270,7 @@ func (conn *ObfuscatedPacketConn) Close() error {
 	if conn.isServer {
 
 		// Interrupt any blocked writes.
-		conn.PacketConn.SetWriteDeadline(time.Now())
+		_ = conn.PacketConn.SetWriteDeadline(time.Now())
 
 		close(conn.stopBroadcast)
 		conn.runWaitGroup.Wait()
@@ -568,7 +568,6 @@ func (conn *ObfuscatedPacketConn) readPacket(
 				firstFlowPacket = true
 			} else {
 				isObfuscated = mode.isObfuscated
-				isIETF = mode.isIETF
 			}
 			mode.lastPacketTime = lastPacketTime
 
@@ -636,7 +635,7 @@ func (conn *ObfuscatedPacketConn) readPacket(
 				// There's a possible race condition between the two instances of locking
 				// peerModesMutex: the client might redial in the meantime. Check that the
 				// mode state is unchanged from when the lock was last held.
-				if !ok || mode.isObfuscated != true || mode.isIETF != false ||
+				if !ok || !mode.isObfuscated || mode.isIETF ||
 					mode.lastPacketTime != lastPacketTime {
 					conn.peerModesMutex.Unlock()
 					return n, oobn, flags, addr, true, newTemporaryNetError(
@@ -764,7 +763,7 @@ func (conn *ObfuscatedPacketConn) writePacket(
 			}
 
 			nonce := buffer[0:NONCE_SIZE]
-			conn.noncePRNG.Read(nonce)
+			_, _ = conn.noncePRNG.Read(nonce)
 
 			// This transform may reduce the entropy of the nonce, which increases
 			// the chance of nonce reuse. However, this chacha20 encryption is for
@@ -782,7 +781,7 @@ func (conn *ObfuscatedPacketConn) writePacket(
 			buffer[NONCE_SIZE] = uint8(paddingLen)
 
 			padding := buffer[(NONCE_SIZE + 1) : (NONCE_SIZE+1)+paddingLen]
-			conn.paddingPRNG.Read(padding)
+			_, _ = conn.paddingPRNG.Read(padding)
 
 			copy(buffer[(NONCE_SIZE+1)+paddingLen:], p)
 			dataLen := (NONCE_SIZE + 1) + paddingLen + n

psiphon/common/resolver/resolver.go

@@ -1523,7 +1523,10 @@ func performDNSQuery(
 	startTime := time.Now()
 
 	// Send the DNS request
-	dnsConn.WriteMsg(request)
+	err := dnsConn.WriteMsg(request)
+	if err != nil {
+		return nil, nil, -1, errors.Trace(err)
+	}
 
 	// Read and process the DNS response
 	var IPs []net.IP

psiphon/common/resolver/resolver_test.go

@@ -647,7 +647,7 @@ func runTestResolver() error {
 
 	cancelFunc()
 
-	IPs, err = resolver.ResolveIP(ctx, networkID, params, exampleDomain)
+	_, err = resolver.ResolveIP(ctx, networkID, params, exampleDomain)
 	if err == nil {
 		return errors.TraceNew("unexpected success")
 	}

psiphon/common/tactics/tactics.go

@@ -800,19 +800,6 @@ func (server *Server) GetTacticsPayload(
 	return payload, nil
 }
 
-func marshalTactics(tactics *Tactics) ([]byte, string, error) {
-	marshaledTactics, err := json.Marshal(tactics)
-	if err != nil {
-		return nil, "", errors.Trace(err)
-	}
-
-	// MD5 hash is used solely as a data checksum and not for any security purpose.
-	digest := md5.Sum(marshaledTactics)
-	tag := hex.EncodeToString(digest[:])
-
-	return marshaledTactics, tag, nil
-}
-
 // GetTacticsWithTag returns a GetTactics value along with the associated tag value.
 //
 // Callers must not mutate returned tactics data, which is cached.
@@ -1264,7 +1251,13 @@ func (server *Server) handleSpeedTestRequest(
 	}
 
 	w.WriteHeader(http.StatusOK)
-	w.Write(response)
+	_, err = w.Write(response)
+	if err != nil {
+		server.logger.WithTraceFields(
+			common.LogFields{"error": err}).Warning("failed to write response")
+		common.TerminateHTTPConnection(w, r)
+		return
+	}
 }
 
 func (server *Server) handleTacticsRequest(
@@ -1336,8 +1329,13 @@ func (server *Server) handleTacticsRequest(
 	}
 
 	w.WriteHeader(http.StatusOK)
-	w.Write(boxedResponse)
-
+	_, err = w.Write(boxedResponse)
+	if err != nil {
+		server.logger.WithTraceFields(
+			common.LogFields{"error": err}).Warning("failed to write response")
+		common.TerminateHTTPConnection(w, r)
+		return
+	}
 	// Log a metric.
 
 	logFields := server.logFieldFormatter(geoIPData, apiParams)

psiphon/common/utils.go

@@ -136,8 +136,8 @@ func TruncateTimestampToHour(timestamp string) string {
 func Compress(data []byte) []byte {
 	var compressedData bytes.Buffer
 	writer := zlib.NewWriter(&compressedData)
-	writer.Write(data)
-	writer.Close()
+	_, _ = writer.Write(data)
+	_ = writer.Close()
 	return compressedData.Bytes()
 }
 

psiphon/controller.go

@@ -80,8 +80,6 @@ type Controller struct {
 	candidateServerEntries                  chan *candidateServerEntry
 	untunneledDialConfig                    *DialConfig
 	untunneledSplitTunnelClassifications    *lrucache.Cache
-	splitTunnelClassificationTTL            time.Duration
-	splitTunnelClassificationMaxEntries     int
 	signalFetchCommonRemoteServerList       chan struct{}
 	signalFetchObfuscatedServerLists        chan struct{}
 	signalDownloadUpgrade                   chan string
@@ -1171,7 +1169,11 @@ func (controller *Controller) registerTunnel(tunnel *Tunnel) bool {
 	// Connecting to a TargetServerEntry does not change the
 	// ranking.
 	if controller.config.TargetServerEntry == "" {
-		PromoteServerEntry(controller.config, tunnel.dialParams.ServerEntry.IpAddress)
+		err := PromoteServerEntry(controller.config, tunnel.dialParams.ServerEntry.IpAddress)
+		if err != nil {
+			NoticeWarning("PromoteServerEntry failed: %v", errors.Trace(err))
+			// Proceed with using tunnel
+		}
 	}
 
 	return true
@@ -1414,7 +1416,7 @@ func (controller *Controller) Dial(
 		// The server has indicated that the client should make a direct,
 		// untunneled dial. Cache the classification to avoid this round trip in
 		// the immediate future.
-		untunneledCache.Add(remoteAddr, true, lrucache.DefaultExpiration)
+		untunneledCache.Set(remoteAddr, true, lrucache.DefaultExpiration)
 	}
 
 	NoticeUntunneled(remoteAddr)
@@ -2255,7 +2257,7 @@ loop:
 			if err != nil {
 				NoticeError("failed to get next candidate: %v", errors.Trace(err))
 				controller.SignalComponentFailure()
-				break loop
+				return
 			}
 			if serverEntry == nil {
 				// Completed this iteration
@@ -2414,7 +2416,12 @@ loop:
 		}
 		timer.Stop()
 
-		iterator.Reset()
+		err := iterator.Reset()
+		if err != nil {
+			NoticeError("failed to reset iterator: %v", errors.Trace(err))
+			controller.SignalComponentFailure()
+			return
+		}
 	}
 }
 
@@ -2749,6 +2756,9 @@ loop:
 
 			// Clear the reference to this discarded tunnel and immediately run
 			// a garbage collection to reclaim its memory.
+			//
+			// Note: this assignment is flagged by github.com/gordonklaus/ineffassign,
+			// but should still have some effect on garbage collection?
 			tunnel = nil
 			DoGarbageCollection()
 		}
@@ -2829,6 +2839,7 @@ func (controller *Controller) runInproxyProxy() {
 
 	p := controller.config.GetParameters().Get()
 	allowProxy := p.Bool(parameters.InproxyAllowProxy)
+	activityNoticePeriod := p.Duration(parameters.InproxyProxyTotalActivityNoticePeriod)
 	p.Close()
 
 	// Running an upstream proxy is also an incompatible case.
@@ -2867,7 +2878,6 @@ func (controller *Controller) runInproxyProxy() {
 	// and formatting when debug logging is off.
 	debugLogging := controller.config.InproxyEnableWebRTCDebugLogging
 
-	activityNoticePeriod := p.Duration(parameters.InproxyProxyTotalActivityNoticePeriod)
 	var lastActivityNotice time.Time
 	var lastActivityConnectingClients, lastActivityConnectedClients int32
 	var lastActivityConnectingClientsTotal, lastActivityConnectedClientsTotal int32

psiphon/dataStore.go

@@ -975,7 +975,10 @@ func (iterator *ServerEntryIterator) Next() (*protocol.ServerEntry, error) {
 		}
 
 		if doDeleteServerEntry {
-			deleteServerEntry(iterator.config, serverEntryID)
+			err := deleteServerEntry(iterator.config, serverEntryID)
+			NoticeWarning(
+				"ServerEntryIterator.Next: deleteServerEntry failed: %s",
+				errors.Trace(err))
 			continue
 		}
 
@@ -1039,12 +1042,12 @@ func (iterator *ServerEntryIterator) Next() (*protocol.ServerEntry, error) {
 					return errors.Trace(err)
 				}
 
-				serverEntries.put(serverEntryID, jsonServerEntryFields)
+				err = serverEntries.put(serverEntryID, jsonServerEntryFields)
 				if err != nil {
 					return errors.Trace(err)
 				}
 
-				serverEntryTags.put([]byte(serverEntryTag), serverEntryID)
+				err = serverEntryTags.put([]byte(serverEntryTag), serverEntryID)
 				if err != nil {
 					return errors.Trace(err)
 				}
@@ -1140,7 +1143,7 @@ func pruneServerEntry(config *Config, serverEntryTag string) error {
 		var serverEntry *protocol.ServerEntry
 		err := json.Unmarshal(serverEntryJson, &serverEntry)
 		if err != nil {
-			errors.Trace(err)
+			return errors.Trace(err)
 		}
 
 		// Only prune sufficiently old server entries. This mitigates the case where
@@ -1148,7 +1151,7 @@ func pruneServerEntry(config *Config, serverEntryTag string) error {
 		// being invalid/deleted.
 		serverEntryLocalTimestamp, err := time.Parse(time.RFC3339, serverEntry.LocalTimestamp)
 		if err != nil {
-			errors.Trace(err)
+			return errors.Trace(err)
 		}
 		if serverEntryLocalTimestamp.Add(minimumAgeForPruning).After(time.Now()) {
 			return nil
@@ -1162,7 +1165,7 @@ func pruneServerEntry(config *Config, serverEntryTag string) error {
 
 		err = serverEntryTags.delete(serverEntryTagBytes)
 		if err != nil {
-			errors.Trace(err)
+			return errors.Trace(err)
 		}
 
 		if doDeleteServerEntry {
@@ -1174,7 +1177,7 @@ func pruneServerEntry(config *Config, serverEntryTag string) error {
 				keyValues,
 				dialParameters)
 			if err != nil {
-				errors.Trace(err)
+				return errors.Trace(err)
 			}
 		}
 
@@ -1231,7 +1234,7 @@ func deleteServerEntry(config *Config, serverEntryID []byte) error {
 			keyValues,
 			dialParameters)
 		if err != nil {
-			errors.Trace(err)
+			return errors.Trace(err)
 		}
 
 		// Remove any tags pointing to the deleted server entry.
@@ -1259,7 +1262,7 @@ func deleteServerEntryHelper(
 
 	err := serverEntries.delete(serverEntryID)
 	if err != nil {
-		errors.Trace(err)
+		return errors.Trace(err)
 	}
 
 	affinityServerEntryID := keyValues.get(datastoreAffinityServerEntryIDKey)
@@ -1608,14 +1611,14 @@ func TakeOutUnreportedPersistentStats(config *Config) (map[string][][]byte, erro
 			for key, value := cursor.first(); key != nil; key, value = cursor.next() {
 
 				// Perform a test JSON unmarshaling. In case of data corruption or a bug,
-				// delete and skip the record.
+				// attempt to delete and skip the record.
 				var jsonData interface{}
 				err := json.Unmarshal(key, &jsonData)
 				if err != nil {
 					NoticeWarning(
 						"Invalid key in TakeOutUnreportedPersistentStats: %s: %s",
 						string(key), err)
-					bucket.delete(key)
+					_ = bucket.delete(key)
 					continue
 				}
 

psiphon/inproxy.go

@@ -1074,11 +1074,9 @@ func MakeInproxyBrokerDialParameters(
 
 	currentTimestamp := time.Now()
 
-	var brokerDialParams *InproxyBrokerDialParameters
-
 	// Select new broker dial parameters
 
-	brokerDialParams = &InproxyBrokerDialParameters{
+	brokerDialParams := &InproxyBrokerDialParameters{
 		brokerSpec:             brokerSpec,
 		LastUsedTimestamp:      currentTimestamp,
 		LastUsedBrokerSpecHash: hashBrokerSpec(brokerSpec),
@@ -2317,13 +2315,6 @@ func newInproxyUDPConn(ctx context.Context, config *Config) (net.PacketConn, err
 	return conn, nil
 }
 
-func inproxyUDPAddrFromAddrPort(addrPort netip.AddrPort) *net.UDPAddr {
-	return &net.UDPAddr{
-		IP:   addrPort.Addr().AsSlice(),
-		Port: int(addrPort.Port()),
-	}
-}
-
 func (conn *inproxyUDPConn) ReadFrom(p []byte) (int, net.Addr, error) {
 
 	// net.UDPConn.ReadFrom currently allocates a &UDPAddr{} per call, and so

psiphon/remoteServerList.go

@@ -167,7 +167,11 @@ func FetchObfuscatedServerLists(
 	// the registry, so clear the ETag to ensure that always happens.
 	_, err := os.Stat(cachedFilename)
 	if os.IsNotExist(err) {
-		SetUrlETag(canonicalURL, "")
+		err := SetUrlETag(canonicalURL, "")
+		if err != nil {
+			NoticeWarning("SetUrlETag failed: %v", errors.Trace(err))
+			// Continue
+		}
 	}
 
 	// failed is set if any operation fails and should trigger a retry. When the OSL registry

psiphon/server/api.go

@@ -1788,7 +1788,7 @@ func isGeoHashString(_ *Config, value string) bool {
 		return false
 	}
 	for _, c := range value {
-		if strings.Index(geohashAlphabet, string(c)) == -1 {
+		if !strings.Contains(geohashAlphabet, string(c)) {
 			return false
 		}
 	}

psiphon/server/meek.go

@@ -1838,7 +1838,10 @@ func (server *MeekServer) inproxyReloadTactics() error {
 		return errors.Trace(err)
 	}
 
-	server.inproxyBroker.SetCommonCompartmentIDs(commonCompartmentIDs)
+	err = server.inproxyBroker.SetCommonCompartmentIDs(commonCompartmentIDs)
+	if err != nil {
+		return errors.Trace(err)
+	}
 
 	server.inproxyBroker.SetTimeouts(
 		p.Duration(parameters.InproxyBrokerProxyAnnounceTimeout),

psiphon/server/replay.go

@@ -229,7 +229,7 @@ func (r *ReplayCache) SetReplayParameters(
 	r.cacheMutex.Lock()
 	defer r.cacheMutex.Unlock()
 
-	r.cache.Add(key, value, TTL)
+	r.cache.Set(key, value, TTL)
 
 	// go-cache-lru is typically safe for concurrent access but explicit
 	// synchronization is required when accessing Items. Items may include

psiphon/server/server_test.go

@@ -1965,7 +1965,7 @@ func runServer(t *testing.T, runConfig *runServerConfig) {
 
 		// Access the unexported controller.steeringIPCache
 		controllerStruct := reflect.ValueOf(controller).Elem()
-		steeringIPCacheField := controllerStruct.Field(40)
+		steeringIPCacheField := controllerStruct.FieldByName("steeringIPCache")
 		steeringIPCacheField = reflect.NewAt(
 			steeringIPCacheField.Type(), unsafe.Pointer(steeringIPCacheField.UnsafeAddr())).Elem()
 		steeringIPCache := steeringIPCacheField.Interface().(*lrucache.Cache)

psiphon/server/tunnelServer.go

@@ -1455,8 +1455,10 @@ func (sshServer *sshServer) reloadTactics() error {
 			// for broker public keys no longer in the known/expected list;
 			// but will retain any existing sessions for broker public keys
 			// that remain in the list.
-			sshServer.inproxyBrokerSessions.SetKnownBrokerPublicKeys(brokerPublicKeys)
-
+			err = sshServer.inproxyBrokerSessions.SetKnownBrokerPublicKeys(brokerPublicKeys)
+			if err != nil {
+				return errors.Trace(err)
+			}
 		}
 	}
 
@@ -1553,7 +1555,7 @@ func (sshServer *sshServer) handleClient(
 					conn.Close()
 				})
 			}
-			io.Copy(ioutil.Discard, conn)
+			_, _ = io.Copy(ioutil.Discard, conn)
 			conn.Close()
 			afterFunc.Stop()
 
@@ -2316,7 +2318,8 @@ func (sshClient *sshClient) run(
 			// It is recommended to set ServerOSSHPrefixSpecs, etc., in default
 			// tactics.
 
-			p, err := sshClient.sshServer.support.ServerTacticsParametersCache.Get(sshClient.peerGeoIPData)
+			var p parameters.ParametersAccessor
+			p, err = sshClient.sshServer.support.ServerTacticsParametersCache.Get(sshClient.peerGeoIPData)
 
 			// Log error, but continue. A default prefix spec will be used by the server.
 			if err != nil {
@@ -2655,8 +2658,8 @@ func (sshClient *sshClient) authLogCallback(conn ssh.ConnMetadata, method string
 // I/O, as newly connecting clients need to await stop completion of any
 // existing connection that shares the same session ID.
 func (sshClient *sshClient) stop() {
-	sshClient.sshConn.Close()
-	sshClient.sshConn.Wait()
+	_ = sshClient.sshConn.Close()
+	_ = sshClient.sshConn.Wait()
 }
 
 // awaitStopped will block until sshClient.run has exited, at which point all
@@ -3677,7 +3680,7 @@ func (sshClient *sshClient) rejectNewChannel(newChannel ssh.NewChannel, logMessa
 	}
 
 	// Note: logMessage is internal, for logging only; just the reject reason is sent to the client.
-	newChannel.Reject(reason, reason.String())
+	_ = newChannel.Reject(reason, reason.String())
 }
 
 // setHandshakeState sets the handshake state -- that it completed and
@@ -4783,7 +4786,7 @@ func (sshClient *sshClient) handleTCPChannel(
 				return
 			}
 
-			newChannel.Reject(protocol.CHANNEL_REJECT_REASON_SPLIT_TUNNEL, "")
+			_ = newChannel.Reject(protocol.CHANNEL_REJECT_REASON_SPLIT_TUNNEL, "")
 			return
 		}
 	}

psiphon/server/udp.go

@@ -367,7 +367,8 @@ type udpgwPortForward struct {
 
 var udpgwBufferPool = &sync.Pool{
 	New: func() any {
-		return make([]byte, udpgwProtocolMaxMessageSize)
+		b := make([]byte, udpgwProtocolMaxMessageSize)
+		return &b
 	},
 }
 
@@ -386,10 +387,17 @@ func (portForward *udpgwPortForward) relayDownstream() {
 	// TODO: is the buffer size larger than necessary?
 
 	// Use a buffer pool to minimize GC churn resulting from frequent,
-	// short-lived UDP flows, including DNS requests.
-	buffer := udpgwBufferPool.Get().([]byte)
+	// short-lived UDP flows, including DNS requests. A pointer to a slice is
+	// used with sync.Pool to avoid an allocation on Put, as would happen if
+	// passing in a slice instead of a pointer; see
+	// https://github.com/dominikh/go-tools/issues/1042#issuecomment-869064445
+	// and
+	// https://github.com/dominikh/go-tools/issues/1336#issuecomment-1331206290
+	// (which should not apply here).
+	b := udpgwBufferPool.Get().(*[]byte)
+	buffer := *b
 	clear(buffer)
-	defer udpgwBufferPool.Put(buffer)
+	defer udpgwBufferPool.Put(b)
 
 	packetBuffer := buffer[portForward.preambleSize:udpgwProtocolMaxMessageSize]
 	for {

psiphon/serverApi.go

@@ -1459,7 +1459,7 @@ func HandleOSLRequest(
 
 	defer func() {
 		if retErr != nil {
-			request.Reply(false, nil)
+			_ = request.Reply(false, nil)
 		}
 	}()
 
@@ -1470,7 +1470,11 @@ func HandleOSLRequest(
 	}
 
 	if oslRequest.ClearLocalSLOKs {
-		DeleteSLOKs()
+		err := DeleteSLOKs()
+		if err != nil {
+			NoticeWarning("DeleteSLOKs failed: %v", errors.Trace(err))
+			// Continue
+		}
 	}
 
 	seededNewSLOK := false
@@ -1479,7 +1483,7 @@ func HandleOSLRequest(
 		duplicate, err := SetSLOK(slok.ID, slok.Key)
 		if err != nil {
 			// TODO: return error to trigger retry?
-			NoticeWarning("SetSLOK failed: %s", errors.Trace(err))
+			NoticeWarning("SetSLOK failed: %v", errors.Trace(err))
 		} else if !duplicate {
 			seededNewSLOK = true
 		}
@@ -1493,7 +1497,10 @@ func HandleOSLRequest(
 		tunnelOwner.SignalSeededNewSLOK()
 	}
 
-	request.Reply(true, nil)
+	err = request.Reply(true, nil)
+	if err != nil {
+		return errors.Trace(err)
+	}
 
 	return nil
 }
@@ -1503,7 +1510,7 @@ func HandleAlertRequest(
 
 	defer func() {
 		if retErr != nil {
-			request.Reply(false, nil)
+			_ = request.Reply(false, nil)
 		}
 	}()
 
@@ -1517,7 +1524,10 @@ func HandleAlertRequest(
 		NoticeServerAlert(alertRequest)
 	}
 
-	request.Reply(true, nil)
+	err = request.Reply(true, nil)
+	if err != nil {
+		return errors.Trace(err)
+	}
 
 	return nil
 }

psiphon/tactics.go

@@ -79,7 +79,7 @@ func GetTactics(ctx context.Context, config *Config, useStoredTactics bool) (fet
 			GetTacticsStorer(config),
 			config.GetNetworkID())
 		if err != nil {
-			NoticeWarning("get stored tactics failed: %s", err)
+			NoticeWarning("get stored tactics failed: %s", errors.Trace(err))
 
 			// The error will be due to a local datastore problem.
 			// While we could proceed with the tactics request, this
@@ -97,7 +97,7 @@ func GetTactics(ctx context.Context, config *Config, useStoredTactics bool) (fet
 
 		iterator, err := NewTacticsServerEntryIterator(config)
 		if err != nil {
-			NoticeWarning("tactics iterator failed: %s", err)
+			NoticeWarning("tactics iterator failed: %s", errors.Trace(err))
 			return
 		}
 		defer iterator.Close()
@@ -113,7 +113,7 @@ func GetTactics(ctx context.Context, config *Config, useStoredTactics bool) (fet
 
 			serverEntry, err := iterator.Next()
 			if err != nil {
-				NoticeWarning("tactics iterator failed: %s", err)
+				NoticeWarning("tactics iterator failed: %s", errors.Trace(err))
 				return
 			}
 
@@ -126,7 +126,11 @@ func GetTactics(ctx context.Context, config *Config, useStoredTactics bool) (fet
 					return
 				}
 
-				iterator.Reset()
+				err := iterator.Reset()
+				if err != nil {
+					NoticeWarning("tactics iterator failed: %s", errors.Trace(err))
+					return
+				}
 				continue
 			}
 
@@ -159,7 +163,7 @@ func GetTactics(ctx context.Context, config *Config, useStoredTactics bool) (fet
 				}
 			}
 
-			NoticeWarning("tactics request failed: %s", err)
+			NoticeWarning("tactics request failed: %s", errors.Trace(err))
 
 			// On error, proceed with a retry, as the error is likely
 			// due to a network failure.
@@ -190,7 +194,7 @@ func GetTactics(ctx context.Context, config *Config, useStoredTactics bool) (fet
 		err := config.SetParameters(
 			tacticsRecord.Tag, true, tacticsRecord.Tactics.Parameters)
 		if err != nil {
-			NoticeWarning("apply tactics failed: %s", err)
+			NoticeWarning("apply tactics failed: %s", errors.Trace(err))
 
 			// The error will be due to invalid tactics values from
 			// the server. When SetParameters fails, all
@@ -258,7 +262,7 @@ func fetchTactics(
 		return nil, errors.Tracef(
 			"failed to make dial parameters for %s: %v",
 			serverEntry.GetDiagnosticID(),
-			err)
+			errors.Trace(err))
 	}
 
 	NoticeRequestingTactics(dialParams)

psiphon/tlsDialer.go

@@ -485,29 +485,32 @@ func CustomTLSDial(
 			return nil, errors.Trace(err)
 		}
 
-		ss := utls.MakeClientSessionState(
+		sessionState := utls.MakeClientSessionState(
 			obfuscatedSessionState.SessionTicket,
 			obfuscatedSessionState.Vers,
 			obfuscatedSessionState.CipherSuite,
 			obfuscatedSessionState.MasterSecret,
 			nil, nil)
-		ss.SetCreatedAt(obfuscatedSessionState.CreatedAt)
-		ss.SetEMS(obfuscatedSessionState.ExtMasterSecret)
+		sessionState.SetCreatedAt(obfuscatedSessionState.CreatedAt)
+		sessionState.SetEMS(obfuscatedSessionState.ExtMasterSecret)
 		// TLS 1.3-only fields
-		ss.SetAgeAdd(obfuscatedSessionState.AgeAdd)
-		ss.SetUseBy(obfuscatedSessionState.UseBy)
+		sessionState.SetAgeAdd(obfuscatedSessionState.AgeAdd)
+		sessionState.SetUseBy(obfuscatedSessionState.UseBy)
 
 		if isTLS13 {
 			// Sets OOB PSK if required.
 			if containsPSKExt(utlsClientHelloID, utlsClientHelloSpec) {
 				if wrappedCache, ok := clientSessionCache.(*common.UtlsClientSessionCacheWrapper); ok {
-					wrappedCache.Put("", ss)
+					wrappedCache.Put("", sessionState)
 				} else {
 					return nil, errors.TraceNew("unexpected clientSessionCache type")
 				}
 			}
 		} else {
-			conn.SetSessionState(ss)
+			err := conn.SetSessionState(sessionState)
+			if err != nil {
+				return nil, errors.Trace(err)
+			}
 		}
 
 		// Apply changes to utls

psiphon/tunnel.go

@@ -258,30 +258,38 @@ func (tunnel *Tunnel) Activate(
 			go func() {
 				defer wg.Done()
 				notice := true
-				select {
-				case serverRequest := <-tunnel.sshServerRequests:
-					if serverRequest != nil {
-						if serverRequest.Type == protocol.PSIPHON_API_INPROXY_RELAY_REQUEST_NAME {
-
-							if notice {
-								NoticeInfo(
-									"relaying inproxy broker packets for %s",
-									tunnel.dialParams.ServerEntry.GetDiagnosticID())
-								notice = false
+				for {
+					select {
+					case serverRequest := <-tunnel.sshServerRequests:
+						if serverRequest != nil {
+							if serverRequest.Type == protocol.PSIPHON_API_INPROXY_RELAY_REQUEST_NAME {
+
+								if notice {
+									NoticeInfo(
+										"relaying inproxy broker packets for %s",
+										tunnel.dialParams.ServerEntry.GetDiagnosticID())
+									notice = false
+								}
+								err := tunnel.relayInproxyPacketRoundTrip(handshakeCtx, serverRequest)
+								if err != nil {
+									NoticeWarning(
+										"relay inproxy broker packets failed: %v",
+										errors.Trace(err))
+									// Continue
+								}
+
+							} else {
+
+								// There's a potential race condition in which
+								// post-handshake SSH requests, such as OSL or
+								// alert requests, arrive to this handler instead
+								// of operateTunnel, so invoke HandleServerRequest here.
+								HandleServerRequest(tunnelOwner, tunnel, serverRequest)
 							}
-							tunnel.relayInproxyPacketRoundTrip(handshakeCtx, serverRequest)
-
-						} else {
-
-							// There's a potential race condition in which
-							// post-handshake SSH requests, such as OSL or
-							// alert requests, arrive to this handler instead
-							// of operateTunnel, so invoke HandleServerRequest here.
-							HandleServerRequest(tunnelOwner, tunnel, serverRequest)
 						}
+					case <-handshakeCtx.Done():
+						return
 					}
-				case <-handshakeCtx.Done():
-					return
 				}
 			}()
 		}
@@ -363,7 +371,7 @@ func (tunnel *Tunnel) relayInproxyPacketRoundTrip(
 
 	defer func() {
 		if retErr != nil {
-			request.Reply(false, nil)
+			_ = request.Reply(false, nil)
 		}
 	}()
 
@@ -373,6 +381,9 @@ func (tunnel *Tunnel) relayInproxyPacketRoundTrip(
 
 	var relayRequest protocol.InproxyRelayRequest
 	err := cbor.Unmarshal(request.Payload, &relayRequest)
+	if err != nil {
+		return errors.Trace(err)
+	}
 
 	inproxyConn := tunnel.dialParams.inproxyConn.Load().(*inproxy.ClientConn)
 	if inproxyConn == nil {