Merge pull request #570 from rod-hynes/master

Packet manipulation fixes

.travis.yml

@@ -2,7 +2,7 @@ dist: trusty
 language: go
 sudo: required
 go:
-- 1.14.6
+- 1.14.9
 addons:
   apt_packages:
     - libx11-dev
@@ -15,7 +15,7 @@ script:
 - go test -race -v ./common/fragmentor
 - go test -race -v ./common/obfuscator
 - go test -race -v ./common/osl
-# TODO: enable all packetman tests with tag PACKET_MANIPULATOR_TEST; requires CAP_NETADMIN and CAP_RAW
+# TODO: enable all packetman tests with tag PSIPHON_RUN_PACKET_MANIPULATOR_TEST; requires CAP_NETADMIN and CAP_RAW
 - go test -race -v ./common/packetman
 - go test -race -v ./common/parameters
 - go test -race -v ./common/protocol

ClientLibrary/Dockerfile

@@ -21,7 +21,7 @@ RUN apt-get update -y && apt-get install -y --no-install-recommends \
 
 # Install Go.
 # NOTE: Go 1.10+ is required to build c-shared for windows (https://github.com/golang/go/commit/bb0bfd002ada7e3eb9198d4287b32c2fed6e8da6)
-ENV GOVERSION=go1.14.6 GOROOT=/usr/local/go GOPATH=/go PATH=$PATH:/usr/local/go/bin:/go/bin CGO_ENABLED=1
+ENV GOVERSION=go1.14.9 GOROOT=/usr/local/go GOPATH=/go PATH=$PATH:/usr/local/go/bin:/go/bin CGO_ENABLED=1
 
 RUN curl -L https://storage.googleapis.com/golang/$GOVERSION.linux-amd64.tar.gz -o /tmp/go.tar.gz \
    && tar -C /usr/local -xzf /tmp/go.tar.gz \

ClientLibrary/build-darwin.sh

@@ -9,9 +9,9 @@ if [ -z ${2+x} ]; then BUILD_TAGS=""; else BUILD_TAGS="$2"; fi
 # Note:
 #   clangwrap.sh needs to be updated when the Go version changes.
 #   The last version was:
-#   https://github.com/golang/go/blob/go1.14.6/misc/ios/clangwrap.sh
+#   https://github.com/golang/go/blob/go1.14.9/misc/ios/clangwrap.sh
 #     - with a patch to lower -mios-version-min to 7.0
-GO_VERSION_REQUIRED="1.14.6"
+GO_VERSION_REQUIRED="1.14.9"
 
 BASE_DIR=$(cd "$(dirname "$0")" ; pwd -P)
 cd ${BASE_DIR}

ConsoleClient/Dockerfile

@@ -22,7 +22,7 @@ RUN apt-get update -y && apt-get install -y --no-install-recommends \
   && rm -rf /var/lib/apt/lists/*
 
 # Install Go.
-ENV GOVERSION=go1.14.6 GOROOT=/usr/local/go GOPATH=/go PATH=$PATH:/usr/local/go/bin:/go/bin CGO_ENABLED=1
+ENV GOVERSION=go1.14.9 GOROOT=/usr/local/go GOPATH=/go PATH=$PATH:/usr/local/go/bin:/go/bin CGO_ENABLED=1
 
 RUN curl -L https://storage.googleapis.com/golang/$GOVERSION.linux-amd64.tar.gz -o /tmp/go.tar.gz \
    && tar -C /usr/local -xzf /tmp/go.tar.gz \

MobileLibrary/Android/Dockerfile

@@ -19,7 +19,7 @@ RUN apt-get update -y && apt-get install -y --no-install-recommends \
   && rm -rf /var/lib/apt/lists/*
 
 # Install Go.
-ENV GOVERSION=go1.14.6 GOROOT=/usr/local/go GOPATH=/go PATH=$PATH:/usr/local/go/bin:/go/bin CGO_ENABLED=1
+ENV GOVERSION=go1.14.9 GOROOT=/usr/local/go GOPATH=/go PATH=$PATH:/usr/local/go/bin:/go/bin CGO_ENABLED=1
 
 RUN curl -L https://storage.googleapis.com/golang/$GOVERSION.linux-amd64.tar.gz -o /tmp/go.tar.gz \
   && tar -C /usr/local -xzf /tmp/go.tar.gz \

MobileLibrary/iOS/build-psiphon-framework.sh

@@ -5,7 +5,7 @@ set -e -u -x
 if [ -z ${1+x} ]; then BUILD_TAGS=""; else BUILD_TAGS="$1"; fi
 
 # Modify this value as we use newer Go versions.
-GO_VERSION_REQUIRED="1.14.6"
+GO_VERSION_REQUIRED="1.14.9"
 
 # At this time, gomobile doesn't support modules
 export GO111MODULE=off

Server/Dockerfile-binary-builder

@@ -1,6 +1,6 @@
 FROM alpine:3.10.2
 
-ENV GOLANG_VERSION 1.14.6
+ENV GOLANG_VERSION 1.14.9
 ENV GOLANG_SRC_URL https://golang.org/dl/go$GOLANG_VERSION.src.tar.gz
 
 RUN set -ex \

psiphon/common/marionette/marionette.go

@@ -1,4 +1,4 @@
-// +build MARIONETTE
+// +build PSIPHON_ENABLE_MARIONETTE
 
 /*
  * Copyright (c) 2018, Psiphon Inc.

psiphon/common/marionette/marionette_disabled.go

@@ -1,4 +1,4 @@
-// +build !MARIONETTE
+// +build !PSIPHON_ENABLE_MARIONETTE
 
 /*
  * Copyright (c) 2018, Psiphon Inc.

psiphon/common/packetman/packetman_linux.go

@@ -43,9 +43,8 @@ func IsSupported() bool {
 }
 
 const (
-	netlinkSocketIOTimeout = 10 * time.Millisecond
-	defaultSocketMark      = 0x70736970 // "PSIP"
-	appliedSpecCacheTTL    = 1 * time.Minute
+	defaultSocketMark   = 0x70736970 // "PSIP"
+	appliedSpecCacheTTL = 1 * time.Minute
 )
 
 // Manipulator is a SYN-ACK packet manipulator.
@@ -79,6 +78,9 @@ const (
 //
 // NFQUEUE with queue-bypass requires Linux kernel 2.6.39; 3.16 or later is
 // validated and recommended.
+//
+// Due to use of NFQUEUE, larger than max socket buffer sizes, and raw
+// sockets, Manipulator requires CAP_NET_ADMIN and CAP_NET_RAW.
 type Manipulator struct {
 	config             *Config
 	mutex              sync.Mutex
@@ -195,14 +197,19 @@ func (m *Manipulator) Start() (retErr error) {
 	// payload, this size should be more than sufficient.
 	maxPacketLen := uint32(1500)
 
-	// Use the kernel default of 1024:
+	// The kernel default is 1024:
 	// https://github.com/torvalds/linux/blob/cd8dead0c39457e58ec1d36db93aedca811d48f1/net/netfilter/nfnetlink_queue.c#L51,
 	// via https://github.com/florianl/go-nfqueue/issues/3.
-	maxQueueLen := uint32(1024)
+	// We use a larger queue size to accomodate more concurrent SYN-ACK packets.
+	maxQueueLen := uint32(2048)
 
-	// Note: runContext alone is not sufficient to interrupt the
-	// nfqueue.socketCallback goroutine spawned by nfqueue.Register; timeouts
-	// must be set. See comment in Manipulator.Stop.
+	// Timeout note: on a small subset of production servers, we have found that
+	// setting a non-zero read timeout results in occasional "orphaned" packets
+	// which remain in the queue but are not delivered to handleInterceptedPacket
+	// for a verdict. This phenomenon leads to a stall in nfqueue processing once
+	// the queue fills up with packers apparently awaiting a verdict. The shorter
+	// the timeout, the faster that orphaned packets accumulate. With no timeout,
+	// and reads in blocking mode, this phenomenon does not occur.
 
 	m.nfqueue, err = nfqueue.Open(
 		&nfqueue.Config{
@@ -211,8 +218,8 @@ func (m *Manipulator) Start() (retErr error) {
 			MaxQueueLen:  maxQueueLen,
 			Copymode:     nfqueue.NfQnlCopyPacket,
 			Logger:       newNfqueueLogger(m.config.Logger),
-			ReadTimeout:  netlinkSocketIOTimeout,
-			WriteTimeout: netlinkSocketIOTimeout,
+			ReadTimeout:  0,
+			WriteTimeout: 0,
 		})
 	if err != nil {
 		return errors.Trace(err)
@@ -223,6 +230,16 @@ func (m *Manipulator) Start() (retErr error) {
 		}
 	}()
 
+	// Set a netlink socket receive buffer size that is significantly larger than
+	// the typical default of 212992. This avoids ENOBUFS in the case of many
+	// netlink messages from the kernel (capped by the max queue size). Note that
+	// the CAP_NET_ADMIN may be required when this exceeds the configured max
+	// buffer size.
+	err = m.nfqueue.Con.SetReadBuffer(1703936)
+	if err != nil {
+		return errors.Trace(err)
+	}
+
 	runContext, stopRunning := context.WithCancel(context.Background())
 	defer func() {
 		if retErr != nil {
@@ -252,23 +269,13 @@ func (m *Manipulator) Stop() {
 		return
 	}
 
-	m.stopRunning()
+	// Call stopRunning before interrupting the blocked read; this ensures that
+	// the nfqueue socketCallback loop will exit after the read is interrupted.
 
-	// stopRunning will cancel the context passed into nfqueue.Register. The
-	// goroutine spawned by Register, nfqueue.socketCallback, polls the context
-	// after a read timeout:
-	// https://github.com/florianl/go-nfqueue/blob/1e38df738c06deffbac08da8fec4b7c28a69b918/nfqueue_gteq_1.12.go#L138-L146
-	//
-	// There's no stop synchronization exposed by nfqueue. Calling nfqueue.Close
-	// while socketCallback is still running can result in errors such as
-	// "nfqueuenfqueue_gteq_1.12.go:134: Could not unbind from queue: netlink
-	// send: sendmsg: bad file descriptor".
-	//
-	// To avoid invalid file descriptor operations and spurious error messages,
-	// sleep for two polling periods, which should be sufficient, in most cases,
-	// for socketCallback to poll the context and exit.
+	m.stopRunning()
 
-	time.Sleep(2 * netlinkSocketIOTimeout)
+	// Interrupt a blocked read.
+	m.nfqueue.Con.SetDeadline(time.Unix(0, 1))
 
 	m.nfqueue.Close()
 

psiphon/common/packetman/packetman_linux_test.go

@@ -1,4 +1,4 @@
-// +build PACKET_MANIPULATOR_TEST
+// +build PSIPHON_RUN_PACKET_MANIPULATOR_TEST
 
 /*
  * Copyright (c) 2020, Psiphon Inc.

psiphon/common/protocol/protocol.go

@@ -41,7 +41,7 @@ const (
 	TUNNEL_PROTOCOL_FRONTED_MEEK_QUIC_OBFUSCATED_SSH = "FRONTED-MEEK-QUIC-OSSH"
 	TUNNEL_PROTOCOL_MARIONETTE_OBFUSCATED_SSH        = "MARIONETTE-OSSH"
 	TUNNEL_PROTOCOL_TAPDANCE_OBFUSCATED_SSH          = "TAPDANCE-OSSH"
-	TUNNEL_PROTOCOL_CONJOUR_OBFUSCATED_SSH           = "CONJOUR-OSSH"
+	TUNNEL_PROTOCOL_CONJURE_OBFUSCATED_SSH           = "CONJURE-OSSH"
 
 	TUNNEL_PROTOCOLS_ALL = "All"
 
@@ -113,14 +113,14 @@ var SupportedTunnelProtocols = TunnelProtocols{
 	TUNNEL_PROTOCOL_FRONTED_MEEK_QUIC_OBFUSCATED_SSH,
 	TUNNEL_PROTOCOL_MARIONETTE_OBFUSCATED_SSH,
 	TUNNEL_PROTOCOL_TAPDANCE_OBFUSCATED_SSH,
-	TUNNEL_PROTOCOL_CONJOUR_OBFUSCATED_SSH,
+	TUNNEL_PROTOCOL_CONJURE_OBFUSCATED_SSH,
 }
 
 var DefaultDisabledTunnelProtocols = TunnelProtocols{
 	TUNNEL_PROTOCOL_FRONTED_MEEK_QUIC_OBFUSCATED_SSH,
 	TUNNEL_PROTOCOL_MARIONETTE_OBFUSCATED_SSH,
 	TUNNEL_PROTOCOL_TAPDANCE_OBFUSCATED_SSH,
-	TUNNEL_PROTOCOL_CONJOUR_OBFUSCATED_SSH,
+	TUNNEL_PROTOCOL_CONJURE_OBFUSCATED_SSH,
 }
 
 var SupportedServerEntrySources = TunnelProtocols{
@@ -189,11 +189,11 @@ func TunnelProtocolUsesMarionette(protocol string) bool {
 
 func TunnelProtocolUsesTapdance(protocol string) bool {
 	return protocol == TUNNEL_PROTOCOL_TAPDANCE_OBFUSCATED_SSH ||
-		protocol == TUNNEL_PROTOCOL_CONJOUR_OBFUSCATED_SSH
+		protocol == TUNNEL_PROTOCOL_CONJURE_OBFUSCATED_SSH
 }
 
 func TunnelProtocolUsesDarkDecoys(protocol string) bool {
-	return protocol == TUNNEL_PROTOCOL_CONJOUR_OBFUSCATED_SSH
+	return protocol == TUNNEL_PROTOCOL_CONJURE_OBFUSCATED_SSH
 }
 
 func TunnelProtocolIsResourceIntensive(protocol string) bool {

psiphon/common/quic/obfuscator.go

@@ -1,4 +1,4 @@
-// +build !DISABLE_QUIC
+// +build !PSIPHON_DISABLE_QUIC
 
 /*
  * Copyright (c) 2018, Psiphon Inc.

psiphon/common/quic/quic.go

@@ -1,4 +1,4 @@
-// +build !DISABLE_QUIC
+// +build !PSIPHON_DISABLE_QUIC
 
 /*
  * Copyright (c) 2018, Psiphon Inc.

psiphon/common/quic/quic_disabled.go

@@ -1,4 +1,4 @@
-// +build DISABLE_QUIC
+// +build PSIPHON_DISABLE_QUIC
 
 /*
  * Copyright (c) 2020, Psiphon Inc.

psiphon/common/quic/quic_test.go

@@ -1,4 +1,4 @@
-// +build !DISABLE_QUIC
+// +build !PSIPHON_DISABLE_QUIC
 
 /*
  * Copyright (c) 2018, Psiphon Inc.

psiphon/common/tapdance/embedded_config.go

@@ -1,4 +1,4 @@
-// +build TAPDANCE
+// +build PSIPHON_ENABLE_TAPDANCE
 
 /*
  * Copyright (c) 2018, Psiphon Inc.

psiphon/common/tapdance/tapdance.go

@@ -1,4 +1,4 @@
-// +build TAPDANCE
+// +build PSIPHON_ENABLE_TAPDANCE
 
 /*
  * Copyright (c) 2018, Psiphon Inc.

psiphon/common/tapdance/tapdance_disabled.go

@@ -1,4 +1,4 @@
-// +build !TAPDANCE
+// +build !PSIPHON_ENABLE_TAPDANCE
 
 /*
  * Copyright (c) 2018, Psiphon Inc.

psiphon/common/values/init.go

@@ -1,4 +1,4 @@
-// +build INIT_PSIPHON_COMMON_VALUES
+// +build PSIPHON_INIT_COMMON_VALUES
 
 /*
  * Copyright (c) 2019, Psiphon Inc.
@@ -21,7 +21,7 @@
 
 package values
 
-// This file is a guard against building with INIT_PSIPHON_COMMON_VALUES
+// This file is a guard against building with PSIPHON_INIT_COMMON_VALUES
 // without replacing init.go.
 
 var buildGuard = intentionally_undefined

psiphon/dataStoreRecovery_test.go

@@ -1,4 +1,4 @@
-// +build !BADGER_DB,!FILES_DB
+// +build !PSIPHON_USE_BADGER_DB,!PSIPHON_USE_FILES_DB
 
 /*
  * Copyright (c) 2019, Psiphon Inc.

psiphon/dataStore_badger.go

@@ -1,4 +1,4 @@
-// +build BADGER_DB
+// +build PSIPHON_USE_BADGER_DB
 
 /*
  * Copyright (c) 2018, Psiphon Inc.

psiphon/dataStore_bolt.go

@@ -1,4 +1,4 @@
-// +build !BADGER_DB,!FILES_DB
+// +build !PSIPHON_USE_BADGER_DB,!PSIPHON_USE_FILES_DB
 
 /*
  * Copyright (c) 2018, Psiphon Inc.

psiphon/dataStore_files.go

@@ -1,4 +1,4 @@
-// +build FILES_DB
+// +build PSIPHON_USE_FILES_DB
 
 /*
  * Copyright (c) 2018, Psiphon Inc.

psiphon/pprof.go

@@ -1,4 +1,4 @@
-// +build RUN_PPROF
+// +build PSIPHON_RUN_PPROF
 
 /*
  * Copyright (c) 2018, Psiphon Inc.

psiphon/pprof_disabled.go

@@ -1,4 +1,4 @@
-// +build !RUN_PPROF
+// +build !PSIPHON_RUN_PPROF
 
 /*
  * Copyright (c) 2018, Psiphon Inc.

psiphon/server/server_packetman_test.go

@@ -1,4 +1,4 @@
-// +build PACKET_MANIPULATOR_TEST
+// +build PSIPHON_RUN_PACKET_MANIPULATOR_TEST
 
 /*
  * Copyright (c) 2020, Psiphon Inc.