Merge pull request #112 from rod-hynes/master

Bug fixes

psiphon/TCPConn.go

@@ -21,14 +21,14 @@ package psiphon
 
 import (
 	"errors"
-	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/upstreamproxy"
 	"net"
 	"sync"
-	"time"
+
+	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/upstreamproxy"
 )
 
 // TCPConn is a customized TCP connection that:
-// - can be interrupted while connecting;
+// - can be interrupted while dialing;
 // - implements a connect timeout;
 // - uses an upstream proxy when specified, and includes
 //   upstream proxy dialing in the connect timeout;
@@ -36,9 +36,9 @@ import (
 //   routing compatibility, for example);
 type TCPConn struct {
 	net.Conn
-	mutex         sync.Mutex
-	isClosed      bool
-	interruptible interruptibleTCPSocket
+	mutex      sync.Mutex
+	isClosed   bool
+	dialResult chan error
 }
 
 // NewTCPDialer creates a TCPDialer.
@@ -51,11 +51,9 @@ func DialTCP(addr string, config *DialConfig) (conn net.Conn, err error) {
 	return makeTCPDialer(config)("tcp", addr)
 }
 
-// makeTCPDialer creates a custom dialer which creates TCPConn. An upstream
-// proxy is used when specified.
+// makeTCPDialer creates a custom dialer which creates TCPConn.
 func makeTCPDialer(config *DialConfig) func(network, addr string) (net.Conn, error) {
-
-	dialer := func(network, addr string) (net.Conn, error) {
+	return func(network, addr string) (net.Conn, error) {
 		if network != "tcp" {
 			return nil, errors.New("unsupported network type in TCPConn dialer")
 		}
@@ -65,69 +63,111 @@ func makeTCPDialer(config *DialConfig) func(network, addr string) (net.Conn, err
 		}
 		return conn, nil
 	}
+}
+
+// interruptibleTCPDial establishes a TCP network connection. A conn is added
+// to config.PendingConns before blocking on network I/O, which enables interruption.
+// The caller is responsible for removing an established conn from PendingConns.
+// An upstream proxy is used when specified.
+//
+// Note: do not to set a UpstreamProxyUrl in the config when using
+// NewTCPDialer as a custom dialer for NewProxyAuthTransport (or http.Transport
+// with a ProxyUrl), as that would result in double proxy chaining.
+//
+// Note: interruption does not actually cancel a connection in progress; it
+// stops waiting for the goroutine blocking on connect()/Dial.
+func interruptibleTCPDial(addr string, config *DialConfig) (*TCPConn, error) {
+
+	// Buffers the first result; senders should discard results when
+	// sending would block, as that means the first result is already set.
+	conn := &TCPConn{dialResult: make(chan error, 1)}
+
+	// Enable interruption
+	if !config.PendingConns.Add(conn) {
+		return nil, ContextError(errors.New("pending connections already closed"))
+	}
+
+	// Call the blocking Connect() in a goroutine. ConnectTimeout is handled
+	// in the platform-specific tcpDial helper function.
+	// Note: since this goroutine may be left running after an interrupt, don't
+	// call Notice() or perform other actions unexpected after a Controller stops.
+	// The lifetime of the goroutine may depend on the host OS TCP connect timeout
+	// when tcpDial, amoung other things, when makes a blocking syscall.Connect()
+	// call.
+	go func() {
+		var netConn net.Conn
+		var err error
+		if config.UpstreamProxyUrl != "" {
+			netConn, err = proxiedTcpDial(addr, config, conn.dialResult)
+		} else {
+			netConn, err = tcpDial(addr, config, conn.dialResult)
+		}
+
+		// Mutex is necessary for referencing conn.isClosed and conn.Conn as
+		// TCPConn.Close may be called while this goroutine is running.
+		conn.mutex.Lock()
+
+		// If already interrupted, cleanup the net.Conn resource and discard.
+		if conn.isClosed && netConn != nil {
+			netConn.Close()
+			conn.mutex.Unlock()
+			return
+		}
 
-	if config.UpstreamProxyUrl != "" {
-
-		upstreamDialer := upstreamproxy.NewProxyDialFunc(
-			&upstreamproxy.UpstreamProxyConfig{
-				ForwardDialFunc: dialer,
-				ProxyURIString:  config.UpstreamProxyUrl,
-			})
-
-		dialer = func(network, addr string) (conn net.Conn, err error) {
-
-			// The entire upstream dial is wrapped in an explicit timeout. This
-			// may include network connection read and writes when proxy auth negotation
-			// is performed.
-
-			type upstreamDialResult struct {
-				conn net.Conn
-				err  error
-			}
-			if config.ConnectTimeout != 0 {
-				resultChannel := make(chan *upstreamDialResult, 2)
-				time.AfterFunc(config.ConnectTimeout, func() {
-					// TODO: we could "interrupt" the underlying TCPConn at this point, as
-					// it's being abandoned. But we don't have a reference to it. It's left
-					// to the outer DialConfig.PendingConns to track and clean up that TCPConn.
-					resultChannel <- &upstreamDialResult{nil, errors.New("upstreamproxy dial timeout")}
-				})
-				go func() {
-					conn, err := upstreamDialer(network, addr)
-					resultChannel <- &upstreamDialResult{conn, err}
-				}()
-				result := <-resultChannel
-
-				conn, err = result.conn, result.err
-			} else {
-				conn, err = upstreamDialer(network, addr)
-			}
-
-			if _, ok := err.(*upstreamproxy.Error); ok {
-				NoticeUpstreamProxyError(err)
-			}
-			return conn, err
+		conn.Conn = netConn
+		conn.mutex.Unlock()
+
+		select {
+		case conn.dialResult <- err:
+		default:
 		}
+	}()
+
+	// Wait until Dial completes (or times out) or until interrupt
+	err := <-conn.dialResult
+	if err != nil {
+		return nil, ContextError(err)
 	}
 
-	return dialer
+	return conn, nil
+}
+
+// proxiedTcpDial wraps a tcpDial call in an upstreamproxy dial.
+func proxiedTcpDial(
+	addr string, config *DialConfig, dialResult chan error) (net.Conn, error) {
+	dialer := func(network, addr string) (net.Conn, error) {
+		return tcpDial(addr, config, dialResult)
+	}
+	upstreamDialer := upstreamproxy.NewProxyDialFunc(
+		&upstreamproxy.UpstreamProxyConfig{
+			ForwardDialFunc: dialer,
+			ProxyURIString:  config.UpstreamProxyUrl,
+		})
+	netConn, err := upstreamDialer("tcp", addr)
+	if _, ok := err.(*upstreamproxy.Error); ok {
+		NoticeUpstreamProxyError(err)
+	}
+	return netConn, err
 }
 
-// Close terminates a connected (net.Conn) or connecting (socketFd) TCPConn.
-// A mutex is required to support net.Conn concurrency semantics.
-// Note also use of mutex around conn.interruptible and conn.Conn in
-// TCPConn_unix.go.
+// Close terminates a connected TCPConn or interrupts a dialing TCPConn.
 func (conn *TCPConn) Close() (err error) {
 	conn.mutex.Lock()
 	defer conn.mutex.Unlock()
 
-	if !conn.isClosed {
-		conn.isClosed = true
-		if conn.Conn == nil {
-			err = interruptibleTCPClose(conn.interruptible)
-		} else {
-			err = conn.Conn.Close()
-		}
+	if conn.isClosed {
+		return
+	}
+	conn.isClosed = true
+
+	if conn.Conn != nil {
+		err = conn.Conn.Close()
 	}
+
+	select {
+	case conn.dialResult <- errors.New("dial interrupted"):
+	default:
+	}
+
 	return err
 }

psiphon/TCPConn_bind.go

@@ -0,0 +1,117 @@
+// +build !windows
+
+/*
+ * Copyright (c) 2015, Psiphon Inc.
+ * All rights reserved.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package psiphon
+
+import (
+	"errors"
+	"fmt"
+	"net"
+	"os"
+	"strconv"
+	"syscall"
+	"time"
+)
+
+// tcpDial is the platform-specific part of interruptibleTCPDial
+//
+// To implement socket device binding, the lower-level syscall APIs are used.
+// The sequence of syscalls in this implementation are taken from:
+// https://code.google.com/p/go/issues/detail?id=6966
+func tcpDial(addr string, config *DialConfig, dialResult chan error) (net.Conn, error) {
+
+	// Like interruption, this timeout doesn't stop this connection goroutine,
+	// it just unblocks the calling interruptibleTCPDial.
+	if config.ConnectTimeout != 0 {
+		time.AfterFunc(config.ConnectTimeout, func() {
+			select {
+			case dialResult <- errors.New("connect timeout"):
+			default:
+			}
+		})
+	}
+
+	// Get the remote IP and port, resolving a domain name if necessary
+	host, strPort, err := net.SplitHostPort(addr)
+	if err != nil {
+		return nil, ContextError(err)
+	}
+	port, err := strconv.Atoi(strPort)
+	if err != nil {
+		return nil, ContextError(err)
+	}
+	ipAddrs, err := LookupIP(host, config)
+	if err != nil {
+		return nil, ContextError(err)
+	}
+	if len(ipAddrs) < 1 {
+		return nil, ContextError(errors.New("no IP address"))
+	}
+
+	// Select an IP at random from the list, so we're not always
+	// trying the same IP (when > 1) which may be blocked.
+	// TODO: retry all IPs until one connects? For now, this retry
+	// will happen on subsequent TCPDial calls, when a different IP
+	// is selected.
+	index, err := MakeSecureRandomInt(len(ipAddrs))
+	if err != nil {
+		return nil, ContextError(err)
+	}
+
+	// TODO: IPv6 support
+	var ip [4]byte
+	copy(ip[:], ipAddrs[index].To4())
+
+	// Create a socket and bind to device, when configured to do so
+	socketFd, err := syscall.Socket(syscall.AF_INET, syscall.SOCK_STREAM, 0)
+	if err != nil {
+		return nil, ContextError(err)
+	}
+
+	if config.DeviceBinder != nil {
+		// WARNING: this potentially violates the direction to not call into
+		// external components after the Controller may have been stopped.
+		// TODO: rework DeviceBinder as an internal 'service' which can trap
+		// external calls when they should not be made?
+		err = config.DeviceBinder.BindToDevice(socketFd)
+		if err != nil {
+			syscall.Close(socketFd)
+			return nil, ContextError(fmt.Errorf("BindToDevice failed: %s", err))
+		}
+	}
+
+	sockAddr := syscall.SockaddrInet4{Addr: ip, Port: port}
+	err = syscall.Connect(socketFd, &sockAddr)
+	if err != nil {
+		syscall.Close(socketFd)
+		return nil, ContextError(err)
+	}
+
+	// Convert the socket fd to a net.Conn
+	file := os.NewFile(uintptr(socketFd), "")
+	netConn, err := net.FileConn(file) // net.FileConn() dups socketFd
+	file.Close()                       // file.Close() closes socketFd
+	if err != nil {
+		return nil, ContextError(err)
+	}
+
+	return netConn, nil
+}

psiphon/TCPConn_nobind.go

@@ -0,0 +1,37 @@
+// +build windows
+
+/*
+ * Copyright (c) 2015, Psiphon Inc.
+ * All rights reserved.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package psiphon
+
+import (
+	"errors"
+	"net"
+)
+
+// tcpDial is the platform-specific part of interruptibleTCPDial
+func tcpDial(addr string, config *DialConfig, dialResult chan error) (net.Conn, error) {
+
+	if config.DeviceBinder != nil {
+		return nil, ContextError(errors.New("psiphon.interruptibleTCPDial with DeviceBinder not supported"))
+	}
+
+	return net.DialTimeout("tcp", addr, config.ConnectTimeout)
+}

psiphon/TCPConn_unix.go

@@ -1,166 +0,0 @@
-// +build android darwin dragonfly freebsd linux nacl netbsd openbsd solaris
-
-/*
- * Copyright (c) 2015, Psiphon Inc.
- * All rights reserved.
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- */
-
-package psiphon
-
-import (
-	"errors"
-	"fmt"
-	"net"
-	"os"
-	"strconv"
-	"syscall"
-	"time"
-)
-
-type interruptibleTCPSocket struct {
-	socketFd int
-}
-
-const _INVALID_FD = -1
-
-// interruptibleTCPDial establishes a TCP network connection. A conn is added
-// to config.PendingConns before blocking on network IO, which enables interruption.
-// The caller is responsible for removing an established conn from PendingConns.
-//
-// To implement socket device binding and interruptible connecting, the lower-level
-// syscall APIs are used. The sequence of syscalls in this implementation are
-// taken from: https://code.google.com/p/go/issues/detail?id=6966
-func interruptibleTCPDial(addr string, config *DialConfig) (*TCPConn, error) {
-
-	// Get the remote IP and port, resolving a domain name if necessary
-	// TODO: domain name resolution isn't interruptible
-	host, strPort, err := net.SplitHostPort(addr)
-	if err != nil {
-		return nil, ContextError(err)
-	}
-	port, err := strconv.Atoi(strPort)
-	if err != nil {
-		return nil, ContextError(err)
-	}
-	ipAddrs, err := LookupIP(host, config)
-	if err != nil {
-		return nil, ContextError(err)
-	}
-	if len(ipAddrs) < 1 {
-		return nil, ContextError(errors.New("no IP address"))
-	}
-
-	// Select an IP at random from the list, so we're not always
-	// trying the same IP (when > 1) which may be blocked.
-	// TODO: retry all IPs until one connects? For now, this retry
-	// will happen on subsequent TCPDial calls, when a different IP
-	// is selected.
-	index, err := MakeSecureRandomInt(len(ipAddrs))
-	if err != nil {
-		return nil, ContextError(err)
-	}
-
-	// TODO: IPv6 support
-	var ip [4]byte
-	copy(ip[:], ipAddrs[index].To4())
-
-	// Create a socket and then, before connecting, add a TCPConn with
-	// the unconnected socket to pendingConns. This allows pendingConns to
-	// interrupt/abort connections in progress.
-	socketFd, err := syscall.Socket(syscall.AF_INET, syscall.SOCK_STREAM, 0)
-	if err != nil {
-		return nil, ContextError(err)
-	}
-
-	conn := &TCPConn{interruptible: interruptibleTCPSocket{socketFd: socketFd}}
-
-	// Cleanup on error
-	defer func() {
-		// Mutex required since conn may be in pendingConns, through which
-		// conn.Close() may be called from another goroutine. There are two
-		// risks:
-		// 1. standard race conditions reading/writing conn members.
-		// 2. closing the fd more than once, with the chance that other
-		//    concurrent goroutines or threads may have already reused the fd.
-		conn.mutex.Lock()
-		if err != nil && conn.interruptible.socketFd != _INVALID_FD {
-			syscall.Close(conn.interruptible.socketFd)
-			conn.interruptible.socketFd = _INVALID_FD
-		}
-		conn.mutex.Unlock()
-	}()
-
-	// Enable interruption
-	if !config.PendingConns.Add(conn) {
-		return nil, ContextError(errors.New("pending connections already closed"))
-	}
-
-	if config.DeviceBinder != nil {
-		err = config.DeviceBinder.BindToDevice(conn.interruptible.socketFd)
-		if err != nil {
-			return nil, ContextError(fmt.Errorf("BindToDevice failed: %s", err))
-		}
-	}
-
-	// Connect the socket
-	// TODO: adjust the timeout to account for time spent resolving hostname
-	sockAddr := syscall.SockaddrInet4{Addr: ip, Port: port}
-	if config.ConnectTimeout != 0 {
-		errChannel := make(chan error, 2)
-		time.AfterFunc(config.ConnectTimeout, func() {
-			errChannel <- errors.New("connect timeout")
-		})
-		go func() {
-			errChannel <- syscall.Connect(conn.interruptible.socketFd, &sockAddr)
-		}()
-		err = <-errChannel
-	} else {
-		err = syscall.Connect(conn.interruptible.socketFd, &sockAddr)
-	}
-	if err != nil {
-		return nil, ContextError(err)
-	}
-
-	// Convert the socket fd to a net.Conn
-	// See mutex note above.
-	conn.mutex.Lock()
-
-	file := os.NewFile(uintptr(conn.interruptible.socketFd), "")
-	fileConn, err := net.FileConn(file) // net.FileConn() dups the fd
-	file.Close()                        // file.Close() closes the fd
-	conn.interruptible.socketFd = _INVALID_FD
-	if err != nil {
-		conn.mutex.Unlock()
-		return nil, ContextError(err)
-	}
-	conn.Conn = fileConn
-	conn.mutex.Unlock()
-
-	return conn, nil
-}
-
-func interruptibleTCPClose(interruptible interruptibleTCPSocket) error {
-
-	// Assumes conn.mutex is held
-
-	if interruptible.socketFd == _INVALID_FD {
-		return nil
-	}
-	err := syscall.Close(interruptible.socketFd)
-	interruptible.socketFd = _INVALID_FD
-	return err
-}

psiphon/TCPConn_windows.go

@@ -1,81 +0,0 @@
-// +build windows
-
-/*
- * Copyright (c) 2015, Psiphon Inc.
- * All rights reserved.
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- */
-
-package psiphon
-
-import (
-	"errors"
-	"net"
-)
-
-// interruptibleTCPSocket simulates interruptible semantics on Windows. A call
-// to interruptibleTCPClose doesn't actually interrupt a connect in progress,
-// but abandons a dial that's running in a goroutine.
-// Interruptible semantics are required by the controller for timely component
-// state changes.
-// TODO: implement true interruptible semantics on Windows; use syscall and
-// a HANDLE similar to how TCPConn_unix uses a file descriptor?
-type interruptibleTCPSocket struct {
-	results chan *interruptibleDialResult
-}
-
-type interruptibleDialResult struct {
-	netConn net.Conn
-	err     error
-}
-
-// interruptibleTCPDial establishes a TCP network connection. A conn is added
-// to config.PendingConns before blocking on network IO, which enables interruption.
-// The caller is responsible for removing an established conn from PendingConns.
-func interruptibleTCPDial(addr string, config *DialConfig) (conn *TCPConn, err error) {
-	if config.DeviceBinder != nil {
-		return nil, ContextError(errors.New("psiphon.interruptibleTCPDial with DeviceBinder not supported on Windows"))
-	}
-
-	// Enable interruption
-	conn = &TCPConn{
-		interruptible: interruptibleTCPSocket{results: make(chan *interruptibleDialResult, 2)}}
-
-	if !config.PendingConns.Add(conn) {
-		return nil, ContextError(errors.New("pending connections already closed"))
-	}
-
-	// Call the blocking Dial in a goroutine
-	results := conn.interruptible.results
-	go func() {
-		netConn, err := net.DialTimeout("tcp", addr, config.ConnectTimeout)
-		results <- &interruptibleDialResult{netConn, err}
-	}()
-
-	// Block until Dial completes (or times out) or until interrupt
-	result := <-conn.interruptible.results
-	if result.err != nil {
-		return nil, ContextError(result.err)
-	}
-	conn.Conn = result.netConn
-
-	return conn, nil
-}
-
-func interruptibleTCPClose(interruptible interruptibleTCPSocket) error {
-	interruptible.results <- &interruptibleDialResult{nil, errors.New("socket interrupted")}
-	return nil
-}

psiphon/config.go

@@ -244,11 +244,11 @@ type Config struct {
 	// parameter is only supported on platforms built with OpenSSL.
 	UseIndistinguishableTLS bool
 
-	// SystemCACertificateDirectory specifies a directory containing OpenSSL-format
-	// CA certificate files (OpenSSL 1.0.1+ format). When specified, this enables
-	// use of indistinguishable TLS for HTTPS requests that require typical (system
-	// CA) server authentication.
-	SystemCACertificateDirectory string
+	// TrustedCACertificatesFilename specifies a file containing trusted CA certs.
+	// The file contents should be compatible with OpenSSL's SSL_CTX_load_verify_locations.
+	// When specified, this enables use of indistinguishable TLS for HTTPS requests
+	// that require typical (system CA) server authentication.
+	TrustedCACertificatesFilename string
 }
 
 // LoadConfig parses and validates a JSON format Psiphon config JSON

psiphon/controller.go

@@ -78,12 +78,12 @@ func NewController(config *Config) (controller *Controller, err error) {
 	// used to exclude these requests and connection from VPN routing.
 	untunneledPendingConns := new(Conns)
 	untunneledDialConfig := &DialConfig{
-		UpstreamProxyUrl:             config.UpstreamProxyUrl,
-		PendingConns:                 untunneledPendingConns,
-		DeviceBinder:                 config.DeviceBinder,
-		DnsServerGetter:              config.DnsServerGetter,
-		UseIndistinguishableTLS:      config.UseIndistinguishableTLS,
-		SystemCACertificateDirectory: config.SystemCACertificateDirectory,
+		UpstreamProxyUrl:              config.UpstreamProxyUrl,
+		PendingConns:                  untunneledPendingConns,
+		DeviceBinder:                  config.DeviceBinder,
+		DnsServerGetter:               config.DnsServerGetter,
+		UseIndistinguishableTLS:       config.UseIndistinguishableTLS,
+		TrustedCACertificatesFilename: config.TrustedCACertificatesFilename,
 	}
 
 	controller = &Controller{
@@ -742,7 +742,7 @@ func (controller *Controller) stopEstablishing() {
 	}
 	NoticeInfo("stop establishing")
 	close(controller.stopEstablishingBroadcast)
-	// Note: on Windows, interruptibleTCPClose doesn't really interrupt socket connects
+	// Note: interruptibleTCPClose doesn't really interrupt socket connects
 	// and may leave goroutines running for a time after the Wait call.
 	controller.establishPendingConns.CloseAll()
 	// Note: establishCandidateGenerator closes controller.candidateServerEntries

psiphon/meekConn.go

@@ -162,13 +162,13 @@ func DialMeek(
 
 		dialer = NewCustomTLSDialer(
 			&CustomTLSConfig{
-				Dial:                         NewTCPDialer(meekConfig),
-				Timeout:                      meekConfig.ConnectTimeout,
-				FrontingAddr:                 fmt.Sprintf("%s:%d", frontingAddress, 443),
-				SendServerName:               false,
-				SkipVerify:                   true,
-				UseIndistinguishableTLS:      config.UseIndistinguishableTLS,
-				SystemCACertificateDirectory: config.SystemCACertificateDirectory,
+				Dial:                          NewTCPDialer(meekConfig),
+				Timeout:                       meekConfig.ConnectTimeout,
+				FrontingAddr:                  fmt.Sprintf("%s:%d", frontingAddress, 443),
+				SendServerName:                false,
+				SkipVerify:                    true,
+				UseIndistinguishableTLS:       config.UseIndistinguishableTLS,
+				TrustedCACertificatesFilename: config.TrustedCACertificatesFilename,
 			})
 	} else {
 		// In the unfronted case, host is both what is dialed and what ends up in the HTTP Host header

psiphon/net.go

@@ -49,10 +49,10 @@ type DialConfig struct {
 
 	ConnectTimeout time.Duration
 
-	// PendingConns is used to interrupt dials in progress.
-	// The dial may be interrupted using PendingConns.CloseAll(): on platforms
-	// that support this, the new conn is added to pendingConns before the network
-	// connect begins and removed from pendingConns once the connect succeeds or fails.
+	// PendingConns is used to track and interrupt dials in progress.
+	// Dials may be interrupted using PendingConns.CloseAll(). Once instantiated,
+	// a conn is added to pendingConns before the network connect begins and
+	// removed from pendingConns once the connect succeeds or fails.
 	PendingConns *Conns
 
 	// BindToDevice parameters are used to exclude connections and
@@ -72,11 +72,11 @@ type DialConfig struct {
 	// Only applies to TLS connections.
 	UseIndistinguishableTLS bool
 
-	// SystemCACertificateDirectory specifies a directory containing
-	// CA certs. Directory contents should be compatible with OpenSSL's
-	// SSL_CTX_load_verify_locations
+	// TrustedCACertificatesFilename specifies a file containing trusted
+	// CA certs. The file contents should be compatible with OpenSSL's
+	// SSL_CTX_load_verify_locations.
 	// Only applies to UseIndistinguishableTLS connections.
-	SystemCACertificateDirectory string
+	TrustedCACertificatesFilename string
 }
 
 // DeviceBinder defines the interface to the external BindToDevice provider

psiphon/notice.go

@@ -26,6 +26,7 @@ import (
 	"io"
 	"log"
 	"os"
+	"sort"
 	"strings"
 	"sync"
 	"time"
@@ -108,10 +109,12 @@ func NoticeCandidateServers(region, protocol string, count int) {
 // NoticeAvailableEgressRegions is what regions are available for egress from.
 // Consecutive reports of the same list of regions are suppressed.
 func NoticeAvailableEgressRegions(regions []string) {
-	repetitionMessage := strings.Join(regions, "")
+	sortedRegions := append([]string(nil), regions...)
+	sort.Strings(sortedRegions)
+	repetitionMessage := strings.Join(sortedRegions, "")
 	outputRepetitiveNotice(
 		"AvailableEgressRegions", repetitionMessage, 0,
-		"AvailableEgressRegions", false, "regions", regions)
+		"AvailableEgressRegions", false, "regions", sortedRegions)
 }
 
 // NoticeConnectingServer is details on a connection attempt

psiphon/opensslConn.go

@@ -46,10 +46,10 @@ func newOpenSSLConn(rawConn net.Conn, hostname string, config *CustomTLSConfig)
 			// TODO: verify with VerifyLegacyCertificate
 			return nil, errors.New("newOpenSSLConn does not support VerifyLegacyCertificate")
 		} else {
-			if config.SystemCACertificateDirectory == "" {
-				return nil, errors.New("newOpenSSLConn cannot verify without SystemCACertificateDirectory")
+			if config.TrustedCACertificatesFilename == "" {
+				return nil, errors.New("newOpenSSLConn cannot verify without TrustedCACertificatesFilename")
 			}
-			err = ctx.LoadVerifyLocations("", config.SystemCACertificateDirectory)
+			err = ctx.LoadVerifyLocations(config.TrustedCACertificatesFilename, "")
 			if err != nil {
 				return nil, ContextError(err)
 			}

psiphon/remoteServerList.go

@@ -21,6 +21,7 @@ package psiphon
 
 import (
 	"errors"
+	"fmt"
 	"io/ioutil"
 	"net"
 	"net/http"
@@ -53,11 +54,11 @@ func FetchRemoteServerList(config *Config, dialConfig *DialConfig) (err error) {
 	if requestUrl.Scheme == "https" {
 		dialer = NewCustomTLSDialer(
 			&CustomTLSConfig{
-				Dial:                         dialer,
-				SendServerName:               true,
-				SkipVerify:                   false,
-				UseIndistinguishableTLS:      config.UseIndistinguishableTLS,
-				SystemCACertificateDirectory: config.SystemCACertificateDirectory,
+				Dial:                          dialer,
+				SendServerName:                true,
+				SkipVerify:                    false,
+				UseIndistinguishableTLS:       config.UseIndistinguishableTLS,
+				TrustedCACertificatesFilename: config.TrustedCACertificatesFilename,
 			})
 
 		// Change the scheme to "http"; otherwise http.Transport will try to do
@@ -94,6 +95,12 @@ func FetchRemoteServerList(config *Config, dialConfig *DialConfig) (err error) {
 	}
 
 	response, err := httpClient.Do(request)
+
+	if err == nil &&
+		(response.StatusCode != http.StatusOK && response.StatusCode != http.StatusNotModified) {
+		response.Body.Close()
+		err = fmt.Errorf("unexpected response status code: %d", response.StatusCode)
+	}
 	if err != nil {
 		return ContextError(err)
 	}

psiphon/serverApi.go

@@ -270,6 +270,10 @@ func (session *Session) doHandshakeRequest() error {
 // doGetRequest makes a tunneled HTTPS request and returns the response body.
 func (session *Session) doGetRequest(requestUrl string) (responseBody []byte, err error) {
 	response, err := session.psiphonHttpsClient.Get(requestUrl)
+	if err == nil && response.StatusCode != http.StatusOK {
+		response.Body.Close()
+		err = fmt.Errorf("unexpected response status code: %d", response.StatusCode)
+	}
 	if err != nil {
 		// Trim this error since it may include long URLs
 		return nil, ContextError(TrimError(err))
@@ -288,6 +292,10 @@ func (session *Session) doGetRequest(requestUrl string) (responseBody []byte, er
 // doPostRequest makes a tunneled HTTPS POST request.
 func (session *Session) doPostRequest(requestUrl string, bodyType string, body io.Reader) (err error) {
 	response, err := session.psiphonHttpsClient.Post(requestUrl, bodyType, body)
+	if err == nil && response.StatusCode != http.StatusOK {
+		response.Body.Close()
+		err = fmt.Errorf("unexpected response status code: %d", response.StatusCode)
+	}
 	if err != nil {
 		// Trim this error since it may include long URLs
 		return ContextError(TrimError(err))

psiphon/splitTunnel.go

@@ -248,6 +248,12 @@ func (classifier *SplitTunnelClassifier) getRoutes(tunnel *Tunnel) (routesData [
 	useCachedRoutes := false
 
 	response, err := httpClient.Do(request)
+
+	if err == nil &&
+		(response.StatusCode != http.StatusOK && response.StatusCode != http.StatusNotModified) {
+		response.Body.Close()
+		err = fmt.Errorf("unexpected response status code: %d", response.StatusCode)
+	}
 	if err != nil {
 		NoticeAlert("failed to request split tunnel routes package: %s", ContextError(err))
 		useCachedRoutes = true

psiphon/tlsDialer.go

@@ -112,11 +112,11 @@ type CustomTLSConfig struct {
 	// Go's TLS has a distinct fingerprint that may be used for blocking.
 	UseIndistinguishableTLS bool
 
-	// SystemCACertificateDirectory specifies a directory containing
+	// TrustedCACertificatesFilename specifies a file containing trusted
 	// CA certs. Directory contents should be compatible with OpenSSL's
 	// SSL_CTX_load_verify_locations
 	// Only applies to UseIndistinguishableTLS connections.
-	SystemCACertificateDirectory string
+	TrustedCACertificatesFilename string
 }
 
 func NewCustomTLSDialer(config *CustomTLSConfig) Dialer {
@@ -189,7 +189,7 @@ func CustomTLSDial(network, addr string, config *CustomTLSConfig) (net.Conn, err
 	if config.UseIndistinguishableTLS &&
 		(config.SkipVerify ||
 			// TODO: config.VerifyLegacyCertificate != nil ||
-			config.SystemCACertificateDirectory != "") {
+			config.TrustedCACertificatesFilename != "") {
 
 		conn, err = newOpenSSLConn(rawConn, hostname, config)
 		if err != nil {

psiphon/tunnel.go

@@ -378,13 +378,13 @@ func dialSsh(
 
 	// Create the base transport: meek or direct connection
 	dialConfig := &DialConfig{
-		UpstreamProxyUrl:             config.UpstreamProxyUrl,
-		ConnectTimeout:               TUNNEL_CONNECT_TIMEOUT,
-		PendingConns:                 pendingConns,
-		DeviceBinder:                 config.DeviceBinder,
-		DnsServerGetter:              config.DnsServerGetter,
-		UseIndistinguishableTLS:      config.UseIndistinguishableTLS,
-		SystemCACertificateDirectory: config.SystemCACertificateDirectory,
+		UpstreamProxyUrl:              config.UpstreamProxyUrl,
+		ConnectTimeout:                TUNNEL_CONNECT_TIMEOUT,
+		PendingConns:                  pendingConns,
+		DeviceBinder:                  config.DeviceBinder,
+		DnsServerGetter:               config.DnsServerGetter,
+		UseIndistinguishableTLS:       config.UseIndistinguishableTLS,
+		TrustedCACertificatesFilename: config.TrustedCACertificatesFilename,
 	}
 	if useMeek {
 		conn, err = DialMeek(serverEntry, sessionId, frontingAddress, dialConfig)

psiphon/upgradeDownload.go

@@ -74,6 +74,16 @@ func DownloadUpgrade(config *Config, clientUpgradeVersion string, tunnel *Tunnel
 	}
 
 	response, err := httpClient.Do(request)
+
+	// The resumeable download may ask for bytes past the resource range
+	// since it doesn't store the "completed download" state. In this case,
+	// the HTTP server returns 416. Otherwise, we expect 206.
+	if err == nil &&
+		(response.StatusCode != http.StatusPartialContent &&
+			response.StatusCode != http.StatusRequestedRangeNotSatisfiable) {
+		response.Body.Close()
+		err = fmt.Errorf("unexpected response status code: %d", response.StatusCode)
+	}
 	if err != nil {
 		return ContextError(err)
 	}