Merge pull request #116 from rod-hynes/master

Timeout tuning after tesing with degraded network

psiphon/config.go

@@ -32,20 +32,21 @@ const (
 	DATA_STORE_FILENAME                          = "psiphon.db"
 	CONNECTION_WORKER_POOL_SIZE                  = 10
 	TUNNEL_POOL_SIZE                             = 1
-	TUNNEL_CONNECT_TIMEOUT                       = 15 * time.Second
+	TUNNEL_CONNECT_TIMEOUT                       = 20 * time.Second
 	TUNNEL_OPERATE_SHUTDOWN_TIMEOUT              = 500 * time.Millisecond
 	TUNNEL_PORT_FORWARD_DIAL_TIMEOUT             = 10 * time.Second
 	TUNNEL_SSH_KEEP_ALIVE_PAYLOAD_MAX_BYTES      = 256
 	TUNNEL_SSH_KEEP_ALIVE_PERIOD_MIN             = 60 * time.Second
 	TUNNEL_SSH_KEEP_ALIVE_PERIOD_MAX             = 120 * time.Second
-	TUNNEL_SSH_KEEP_ALIVE_TIMEOUT                = 10 * time.Second
+	TUNNEL_SSH_KEEP_ALIVE_PERIODIC_TIMEOUT       = 30 * time.Second
+	TUNNEL_SSH_KEEP_ALIVE_PROBE_TIMEOUT          = 5 * time.Second
 	ESTABLISH_TUNNEL_TIMEOUT_SECONDS             = 300
-	ESTABLISH_TUNNEL_WORK_TIME_SECONDS           = 60 * time.Second
+	ESTABLISH_TUNNEL_WORK_TIME                   = 60 * time.Second
 	ESTABLISH_TUNNEL_PAUSE_PERIOD                = 5 * time.Second
 	PORT_FORWARD_FAILURE_THRESHOLD               = 0
 	HTTP_PROXY_ORIGIN_SERVER_TIMEOUT             = 15 * time.Second
 	HTTP_PROXY_MAX_IDLE_CONNECTIONS_PER_HOST     = 50
-	FETCH_REMOTE_SERVER_LIST_TIMEOUT             = 10 * time.Second
+	FETCH_REMOTE_SERVER_LIST_TIMEOUT             = 30 * time.Second
 	FETCH_REMOTE_SERVER_LIST_RETRY_PERIOD        = 5 * time.Second
 	FETCH_REMOTE_SERVER_LIST_STALE_PERIOD        = 6 * time.Hour
 	PSIPHON_API_CLIENT_SESSION_ID_LENGTH         = 16
@@ -249,6 +250,11 @@ type Config struct {
 	// When specified, this enables use of indistinguishable TLS for HTTPS requests
 	// that require typical (system CA) server authentication.
 	TrustedCACertificatesFilename string
+
+	// EnablePeriodicSshKeepAlive indicates whether to send an SSH keepalive every
+	// 1-2 minutes. If the SSH keepalive timesout, the tunnel is considered to have
+	// failed.
+	EnablePeriodicSshKeepAlive bool
 }
 
 // LoadConfig parses and validates a JSON format Psiphon config JSON

psiphon/controller.go

@@ -798,7 +798,7 @@ loop:
 			}
 
 			// Disable impaired protocols. This is only done for the
-			// first iteration of the ESTABLISH_TUNNEL_WORK_TIME_SECONDS
+			// first iteration of the ESTABLISH_TUNNEL_WORK_TIME
 			// loop since (a) one iteration should be sufficient to
 			// evade the attack; (b) there's a good chance of false
 			// positives (such as short session durations due to network
@@ -828,7 +828,7 @@ loop:
 				break loop
 			}
 
-			if time.Now().After(startTime.Add(ESTABLISH_TUNNEL_WORK_TIME_SECONDS)) {
+			if time.Now().After(startTime.Add(ESTABLISH_TUNNEL_WORK_TIME)) {
 				// Start over, after a brief pause, with a new shuffle of the server
 				// entries, and potentially some newly fetched server entries.
 				break

psiphon/tunnel.go

@@ -539,7 +539,7 @@ func (tunnel *Tunnel) operateTunnel(config *Config, tunnelOwner TunnelOwner) {
 			TUNNEL_SSH_KEEP_ALIVE_PERIOD_MAX)
 	}
 
-	// TODO: don't initialize if !config.EmitBytesTransferred
+	// TODO: don't initialize timer if !config.EmitBytesTransferred
 	noticeBytesTransferredTicker := time.NewTicker(1 * time.Second)
 	if !config.EmitBytesTransferred {
 		noticeBytesTransferredTicker.Stop()
@@ -550,8 +550,13 @@ func (tunnel *Tunnel) operateTunnel(config *Config, tunnelOwner TunnelOwner) {
 	statsTimer := time.NewTimer(nextStatusRequestPeriod())
 	defer statsTimer.Stop()
 
+	// TODO: don't initialize timer if !config.EnablePeriodicSshKeepAlive
 	sshKeepAliveTimer := time.NewTimer(nextSshKeepAlivePeriod())
-	defer sshKeepAliveTimer.Stop()
+	if !config.EnablePeriodicSshKeepAlive {
+		sshKeepAliveTimer.Stop()
+	} else {
+		defer sshKeepAliveTimer.Stop()
+	}
 
 	var err error
 	for err == nil {
@@ -569,7 +574,8 @@ func (tunnel *Tunnel) operateTunnel(config *Config, tunnelOwner TunnelOwner) {
 			statsTimer.Reset(nextStatusRequestPeriod())
 
 		case <-sshKeepAliveTimer.C:
-			err = sendSshKeepAlive(tunnel.sshClient, tunnel.conn)
+			err = sendSshKeepAlive(
+				tunnel.sshClient, tunnel.conn, TUNNEL_SSH_KEEP_ALIVE_PERIODIC_TIMEOUT)
 			sshKeepAliveTimer.Reset(nextSshKeepAlivePeriod())
 
 		case failures := <-tunnel.portForwardFailures:
@@ -586,8 +592,11 @@ func (tunnel *Tunnel) operateTunnel(config *Config, tunnelOwner TunnelOwner) {
 				// on the server, so we don't abort the connection until the threshold
 				// is hit. But if we can't make a simple round trip request to the
 				// server, we'll immediately abort.
-				err = sendSshKeepAlive(tunnel.sshClient, tunnel.conn)
-				sshKeepAliveTimer.Reset(nextSshKeepAlivePeriod())
+				err = sendSshKeepAlive(
+					tunnel.sshClient, tunnel.conn, TUNNEL_SSH_KEEP_ALIVE_PROBE_TIMEOUT)
+				if config.EnablePeriodicSshKeepAlive {
+					sshKeepAliveTimer.Reset(nextSshKeepAlivePeriod())
+				}
 			}
 
 		case <-tunnel.shutdownOperateBroadcast:
@@ -607,10 +616,11 @@ func (tunnel *Tunnel) operateTunnel(config *Config, tunnelOwner TunnelOwner) {
 // sendSshKeepAlive is a helper which sends a keepalive@openssh.com request
 // on the specified SSH connections and returns true of the request succeeds
 // within a specified timeout.
-func sendSshKeepAlive(sshClient *ssh.Client, conn net.Conn) error {
+func sendSshKeepAlive(
+	sshClient *ssh.Client, conn net.Conn, timeout time.Duration) error {
 
 	errChannel := make(chan error, 2)
-	time.AfterFunc(TUNNEL_SSH_KEEP_ALIVE_TIMEOUT, func() {
+	time.AfterFunc(timeout, func() {
 		errChannel <- TimeoutError{}
 	})