Fixes for SSH keep alive

- SendRequest err return value was masked
- added explicit timeout for each SendRequest

psiphon/config.go

@@ -40,6 +40,7 @@ const (
 	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                = 30 * time.Second
 	ESTABLISH_TUNNEL_TIMEOUT_SECONDS             = 300
 	ESTABLISH_TUNNEL_WORK_TIME_SECONDS           = 60 * time.Second
 	ESTABLISH_TUNNEL_PAUSE_PERIOD                = 5 * time.Second

psiphon/net.go

@@ -82,6 +82,13 @@ type DnsServerGetter interface {
 	GetDnsServer() string
 }
 
+// TimeoutError implements the error interface
+type TimeoutError struct{}
+
+func (TimeoutError) Error() string   { return "timed out" }
+func (TimeoutError) Timeout() bool   { return true }
+func (TimeoutError) Temporary() bool { return true }
+
 // Dialer is a custom dialer compatible with http.Transport.Dial.
 type Dialer func(string, string) (net.Conn, error)
 

psiphon/tlsDialer.go

@@ -79,12 +79,6 @@ import (
 	"time"
 )
 
-type timeoutError struct{}
-
-func (timeoutError) Error() string   { return "tls: DialWithDialer timed out" }
-func (timeoutError) Timeout() bool   { return true }
-func (timeoutError) Temporary() bool { return true }
-
 // CustomTLSConfig contains parameters to determine the behavior
 // of CustomTLSDial.
 type CustomTLSConfig struct {
@@ -139,7 +133,7 @@ func CustomTLSDial(network, addr string, config *CustomTLSConfig) (*tls.Conn, er
 	if config.Timeout != 0 {
 		errChannel = make(chan error, 2)
 		time.AfterFunc(config.Timeout, func() {
-			errChannel <- timeoutError{}
+			errChannel <- TimeoutError{}
 		})
 	}
 

psiphon/tunnel.go

@@ -565,11 +565,10 @@ func (tunnel *Tunnel) operateTunnel(config *Config, tunnelOwner TunnelOwner) {
 			statsTimer.Reset(nextStatusRequestPeriod())
 
 		case <-sshKeepAliveTimer.C:
-			// Random padding to frustrate fingerprinting
-			_, _, err := tunnel.sshClient.SendRequest(
-				"[email protected]", true,
-				MakeSecureRandomPadding(0, TUNNEL_SSH_KEEP_ALIVE_PAYLOAD_MAX_BYTES))
-			err = fmt.Errorf("ssh keep alive failed: %s", err)
+			err = sendSshKeepAlive(tunnel.sshClient)
+			if err != nil {
+				err = fmt.Errorf("ssh keep alive failed: %s", err)
+			}
 			sshKeepAliveTimer.Reset(nextSshKeepAlivePeriod())
 
 		case failures := <-tunnel.portForwardFailures:
@@ -598,6 +597,33 @@ func (tunnel *Tunnel) operateTunnel(config *Config, tunnelOwner TunnelOwner) {
 	}
 }
 
+// sendSshKeepAlive is a helper which sends a [email protected] request
+// on the specified SSH connections and returns true of the request succeeds
+// within a specified timeout.
+func sendSshKeepAlive(sshClient *ssh.Client) error {
+
+	errChannel := make(chan error, 2)
+	time.AfterFunc(TUNNEL_SSH_KEEP_ALIVE_TIMEOUT, func() {
+		errChannel <- TimeoutError{}
+	})
+
+	go func() {
+		// Random padding to frustrate fingerprinting
+
+		NoticeInfo("sending [email protected]")
+
+		_, _, err := sshClient.SendRequest(
+			"[email protected]", true,
+			MakeSecureRandomPadding(0, TUNNEL_SSH_KEEP_ALIVE_PAYLOAD_MAX_BYTES))
+
+		NoticeInfo("done [email protected]")
+
+		errChannel <- err
+	}()
+
+	return <-errChannel
+}
+
 // sendStats is a helper for sending session stats to the server.
 func sendStats(tunnel *Tunnel) {