Add upgrade download when not connected

* Upgrade downloader is now triggered at the same time as
  a remote server list fetch, after a period of failing to
  connect.
* The downloader will perform either a tunneled or untunneled
  download. The untunneled download enables upgrading when
  unable to connect at all.
* The handshake-triggered download still uses the available
  upgrade client version returned by handshake. The new case
  performs a HEAD request on the upgrade entity to fetch a
  new custom HTTP header which indicates the available
  upgrade client version.

psiphon/config.go

@@ -22,7 +22,9 @@ package psiphon
 import (
 	"encoding/json"
 	"errors"
+	"fmt"
 	"os"
+	"strconv"
 	"time"
 )
 
@@ -65,7 +67,8 @@ const (
 	PSIPHON_API_TUNNEL_STATS_MAX_COUNT             = 1000
 	FETCH_ROUTES_TIMEOUT                           = 1 * time.Minute
 	DOWNLOAD_UPGRADE_TIMEOUT                       = 15 * time.Minute
-	DOWNLOAD_UPGRADE_RETRY_PAUSE_PERIOD            = 5 * time.Second
+	DOWNLOAD_UPGRADE_RETRY_PERIOD                  = 5 * time.Second
+	DOWNLOAD_UPGRADE_STALE_PERIOD                  = 6 * time.Hour
 	IMPAIRED_PROTOCOL_CLASSIFICATION_DURATION      = 2 * time.Minute
 	IMPAIRED_PROTOCOL_CLASSIFICATION_THRESHOLD     = 3
 	TOTAL_BYTES_TRANSFERRED_NOTICE_PERIOD          = 5 * time.Minute
@@ -252,6 +255,13 @@ type Config struct {
 	// typically embedded in the client binary.
 	UpgradeDownloadUrl string
 
+	// UpgradeDownloadClientVersionHeader specifies the HTTP header name for the
+	// entity at UpgradeDownloadUrl which specifies the client version (an integer
+	// value). A HEAD request may be made to check the version number available at
+	// UpgradeDownloadUrl. UpgradeDownloadClientVersionHeader is required when
+	// UpgradeDownloadUrl is specified.
+	UpgradeDownloadClientVersionHeader string
+
 	// UpgradeDownloadFilename is the local target filename for an upgrade download.
 	// This parameter is required when UpgradeDownloadUrl is specified.
 	UpgradeDownloadFilename string
@@ -331,6 +341,12 @@ func LoadConfig(configJson []byte) (*Config, error) {
 		config.ClientVersion = "0"
 	}
 
+	_, err = strconv.Atoi(config.ClientVersion)
+	if err != nil {
+		return nil, ContextError(
+			fmt.Errorf("invalid client version: %s", err))
+	}
+
 	if config.TunnelProtocol != "" {
 		if !Contains(SupportedTunnelProtocols, config.TunnelProtocol) {
 			return nil, ContextError(
@@ -367,6 +383,12 @@ func LoadConfig(configJson []byte) (*Config, error) {
 		return nil, ContextError(errors.New("HostNameTransformer interface must be set at runtime"))
 	}
 
+	if config.UpgradeDownloadUrl != "" &&
+		(config.UpgradeDownloadClientVersionHeader == "" || config.UpgradeDownloadFilename == "") {
+		return nil, ContextError(errors.New(
+			"UpgradeDownloadUrl requires UpgradeDownloadClientVersionHeader and UpgradeDownloadFilename"))
+	}
+
 	if config.EmitDiagnosticNotices {
 		setEmitDiagnosticNotices(true)
 	}

psiphon/controller.go

@@ -47,7 +47,6 @@ type Controller struct {
 	tunnels                        []*Tunnel
 	nextTunnel                     int
 	startedConnectedReporter       bool
-	startedUpgradeDownloader       bool
 	isEstablishing                 bool
 	establishWaitGroup             *sync.WaitGroup
 	stopEstablishingBroadcast      chan struct{}
@@ -57,6 +56,7 @@ type Controller struct {
 	untunneledDialConfig           *DialConfig
 	splitTunnelClassifier          *SplitTunnelClassifier
 	signalFetchRemoteServerList    chan struct{}
+	signalDownloadUpgrade          chan string
 	impairedProtocolClassification map[string]int
 	signalReportConnected          chan struct{}
 	serverAffinityDoneBroadcast    chan struct{}
@@ -115,7 +115,6 @@ func NewController(config *Config) (controller *Controller, err error) {
 		tunnels:                        make([]*Tunnel, 0),
 		establishedOnce:                false,
 		startedConnectedReporter:       false,
-		startedUpgradeDownloader:       false,
 		isEstablishing:                 false,
 		establishPendingConns:          new(Conns),
 		untunneledPendingConns:         untunneledPendingConns,
@@ -125,6 +124,7 @@ func NewController(config *Config) (controller *Controller, err error) {
 		// starting? Trade-off is potential back-to-back fetch remotes. As-is,
 		// establish will eventually signal another fetch remote.
 		signalFetchRemoteServerList: make(chan struct{}),
+		signalDownloadUpgrade:       make(chan string),
 		signalReportConnected:       make(chan struct{}),
 	}
 
@@ -173,6 +173,13 @@ func (controller *Controller) Run(shutdownBroadcast <-chan struct{}) {
 		go controller.remoteServerListFetcher()
 	}
 
+	if controller.config.UpgradeDownloadUrl != "" &&
+		controller.config.UpgradeDownloadFilename != "" {
+
+		controller.runWaitGroup.Add(1)
+		go controller.upgradeDownloader()
+	}
+
 	/// Note: the connected reporter isn't started until a tunnel is
 	// established
 
@@ -370,63 +377,76 @@ func (controller *Controller) startOrSignalConnectedReporter() {
 // download. DownloadUpgrade() is resumable, so each attempt has potential for
 // getting closer to completion, even in conditions where the download or
 // tunnel is repeatedly interrupted.
-// Once the download is complete, the downloader exits and is not run again:
+// An upgrade download is triggered by either a handshake response indicating
+// that a new version is available; or after failing to connect, in which case
+// it's useful to check, out-of-band, for an upgrade with new circumvention
+// capabilities.
+// Once the download operation completes successfully, the downloader exits
+// and is not run again: either there is not a newer version, or the upgrade
+// has been downloaded and is ready to be applied.
 // We're assuming that the upgrade will be applied and the entire system
 // restarted before another upgrade is to be downloaded.
-func (controller *Controller) upgradeDownloader(clientUpgradeVersion string) {
+//
+// TODO: refactor upgrade downloader and remote server list fetcher to use
+// common code (including the resumable download routines).
+//
+func (controller *Controller) upgradeDownloader() {
 	defer controller.runWaitGroup.Done()
 
-loop:
-	for {
-		// Pick any active tunnel and make the next download attempt. No error
-		// is logged if there's no active tunnel, as that's not an unexpected condition.
-		tunnel := controller.getNextActiveTunnel()
-		if tunnel != nil {
-			err := DownloadUpgrade(controller.config, clientUpgradeVersion, tunnel)
-			if err == nil {
-				break loop
-			}
-			NoticeAlert("upgrade download failed: %s", err)
-		}
+	var lastDownloadTime time.Time
 
-		timeout := time.After(DOWNLOAD_UPGRADE_RETRY_PAUSE_PERIOD)
+downloadLoop:
+	for {
+		// Wait for a signal before downloading
+		var handshakeVersion string
 		select {
-		case <-timeout:
-			// Make another download attempt
+		case handshakeVersion = <-controller.signalDownloadUpgrade:
 		case <-controller.shutdownBroadcast:
-			break loop
+			break downloadLoop
 		}
-	}
 
-	NoticeInfo("exiting upgrade downloader")
-}
+		// Skip download entirely when a recent download was successful
+		if time.Now().Before(lastDownloadTime.Add(DOWNLOAD_UPGRADE_STALE_PERIOD)) {
+			continue
+		}
 
-func (controller *Controller) startClientUpgradeDownloader(
-	serverContext *ServerContext) {
+	retryLoop:
+		for {
+			// Don't attempt to download while there is no network connectivity,
+			// to avoid alert notice noise.
+			if !WaitForNetworkConnectivity(
+				controller.config.NetworkConnectivityChecker,
+				controller.shutdownBroadcast) {
+				break downloadLoop
+			}
 
-	// serverContext is nil when DisableApi is set
-	if controller.config.DisableApi {
-		return
-	}
+			// Pick any active tunnel and make the next download attempt. If there's
+			// no active tunnel, the untunneledDialConfig will be used.
+			tunnel := controller.getNextActiveTunnel()
 
-	if controller.config.UpgradeDownloadUrl == "" ||
-		controller.config.UpgradeDownloadFilename == "" {
-		// No upgrade is desired
-		return
-	}
+			err := DownloadUpgrade(
+				controller.config,
+				handshakeVersion,
+				tunnel,
+				controller.untunneledDialConfig)
 
-	if serverContext.clientUpgradeVersion == "" {
-		// No upgrade is offered
-		return
-	}
+			if err == nil {
+				lastDownloadTime = time.Now()
+				break retryLoop
+			}
 
-	// Start the client upgrade downloaded after the first tunnel is established.
-	// Concurrency note: only the runTunnels goroutine may access startClientUpgradeDownloader.
-	if !controller.startedUpgradeDownloader {
-		controller.startedUpgradeDownloader = true
-		controller.runWaitGroup.Add(1)
-		go controller.upgradeDownloader(serverContext.clientUpgradeVersion)
+			NoticeAlert("failed to download upgrade: %s", err)
+
+			timeout := time.After(DOWNLOAD_UPGRADE_RETRY_PERIOD)
+			select {
+			case <-timeout:
+			case <-controller.shutdownBroadcast:
+				break downloadLoop
+			}
+		}
 	}
+
+	NoticeInfo("exiting upgrade downloader")
 }
 
 // runTunnels is the controller tunnel management main loop. It starts and stops
@@ -504,8 +524,18 @@ loop:
 					// tunnel is established.
 					controller.startOrSignalConnectedReporter()
 
-					controller.startClientUpgradeDownloader(
-						establishedTunnel.serverContext)
+					// If the handshake indicated that a new client version is available,
+					// trigger an upgrade download.
+					// Note: serverContext is nil when DisableApi is set
+					if establishedTunnel.serverContext != nil &&
+						establishedTunnel.serverContext.clientUpgradeVersion != "" {
+
+						handshakeVersion := establishedTunnel.serverContext.clientUpgradeVersion
+						select {
+						case controller.signalDownloadUpgrade <- handshakeVersion:
+						default:
+						}
+					}
 				}
 
 			} else {
@@ -948,6 +978,14 @@ loop:
 		default:
 		}
 
+		// Trigger an out-of-band upgrade availability check and download.
+		// Since we may have failed to connect, we may benefit from upgrading
+		// to a new client version with new circumvention capabilities.
+		select {
+		case controller.signalDownloadUpgrade <- "":
+		default:
+		}
+
 		// After a complete iteration of candidate servers, pause before iterating again.
 		// This helps avoid some busy wait loop conditions, and also allows some time for
 		// network conditions to change. Also allows for fetch remote to complete,

psiphon/upgradeDownload.go

@@ -26,32 +26,119 @@ import (
 	"io/ioutil"
 	"net/http"
 	"os"
+	"strconv"
 )
 
-// DownloadUpgrade performs a tunneled, resumable download of client upgrade files.
+// DownloadUpgrade performs a resumable download of client upgrade files.
+//
 // While downloading/resuming, a temporary file is used. Once the download is complete,
 // a notice is issued and the upgrade is available at the destination specified in
 // config.UpgradeDownloadFilename.
-// NOTE: this code does not check that any existing file at config.UpgradeDownloadFilename
-// is actually the version specified in clientUpgradeVersion.
-func DownloadUpgrade(config *Config, clientUpgradeVersion string, tunnel *Tunnel) error {
+//
+// The upgrade download may be either tunneled or untunneled. As the untunneled case may
+// happen with no handshake request response, the downloader cannot rely on having the
+// upgrade_client_version output from handshake and instead this logic performs a
+// comparison between the config.ClientVersion and the client version recorded in the
+// remote entity's UpgradeDownloadClientVersionHeader. A HEAD request is made to check the
+// version before proceeding with a full download.
+//
+// NOTE: This code does not check that any existing file at config.UpgradeDownloadFilename
+// is actually the version specified in handshakeVersion.
+//
+// TODO: This logic requires the outer client to *omit* config.UpgradeDownloadFilename
+// when there's already a downloaded upgrade pending. Because the outer client currently
+// handles the authenticated package phase, and because the outer client deletes the
+// intermediate files (including config.UpgradeDownloadFilename), if the outer client
+// does not omit config.UpgradeDownloadFilename then the new version will be downloaded
+// repeatedly. Implement a new scheme where tunnel core does the authenticated package phase
+// and tracks the the output by version number so that (a) tunnel core knows when it's not
+// necessary to re-download; (b) newer upgrades will be downloaded even when an older
+// upgrade is still pending install by the outer client.
+func DownloadUpgrade(
+	config *Config,
+	handshakeVersion string,
+	tunnel *Tunnel,
+	untunneledDialConfig *DialConfig) error {
 
 	// Check if complete file already downloaded
+
 	if _, err := os.Stat(config.UpgradeDownloadFilename); err == nil {
 		NoticeClientUpgradeDownloaded(config.UpgradeDownloadFilename)
 		return nil
 	}
 
-	httpClient, err := MakeTunneledHttpClient(config, tunnel, DOWNLOAD_UPGRADE_TIMEOUT)
-	if err != nil {
-		return ContextError(err)
+	requestUrl := config.UpgradeDownloadUrl
+	var httpClient *http.Client
+	var err error
+
+	// Select tunneled or untunneled configuration
+
+	if tunnel != nil {
+		httpClient, err = MakeTunneledHttpClient(config, tunnel, DOWNLOAD_UPGRADE_TIMEOUT)
+		if err != nil {
+			return ContextError(err)
+		}
+	} else {
+		httpClient, requestUrl, err = MakeUntunneledHttpsClient(
+			untunneledDialConfig, nil, requestUrl, DOWNLOAD_UPGRADE_TIMEOUT)
+		if err != nil {
+			return ContextError(err)
+		}
+	}
+
+	// If no handshake version is supplied, make an initial HEAD request
+	// to get the current version from the version header.
+
+	availableClientVersion := handshakeVersion
+	if availableClientVersion == "" {
+		request, err := http.NewRequest("HEAD", requestUrl, nil)
+		if err != nil {
+			return ContextError(err)
+		}
+		response, err := httpClient.Do(request)
+		if err == nil && response.StatusCode != http.StatusOK {
+			response.Body.Close()
+			err = fmt.Errorf("unexpected response status code: %d", response.StatusCode)
+		}
+		if err != nil {
+			return ContextError(err)
+		}
+		defer response.Body.Close()
+
+		currentClientVersion, err := strconv.Atoi(config.ClientVersion)
+		if err != nil {
+			return ContextError(err)
+		}
+
+		// Note: if the header is missing, Header.Get returns "" and then
+		// strconv.Atoi returns a parse error.
+		headerValue := response.Header.Get(config.UpgradeDownloadClientVersionHeader)
+		availableClientVersion, err := strconv.Atoi(headerValue)
+		if err != nil {
+			// If the header is missing or malformed, we can't determine the available
+			// version number. This is unexpected; but if it happens, it's likely due
+			// to a server-side configuration issue. In this one case, we don't
+			// return an error so that we don't go into a rapid retry loop making
+			// ineffective HEAD requests (the client may still signal an upgrade
+			// download later in the session).
+			NoticeAlert(
+				"failed to download upgrade: invalid %s header value %s: %s",
+				config.UpgradeDownloadClientVersionHeader, headerValue, err)
+			return nil
+		}
+
+		if currentClientVersion >= availableClientVersion {
+			NoticeInfo("skipping download of available client version %d", availableClientVersion)
+		}
 	}
 
+	// Proceed with full download
+
 	partialFilename := fmt.Sprintf(
-		"%s.%s.part", config.UpgradeDownloadFilename, clientUpgradeVersion)
+		"%s.%s.part", config.UpgradeDownloadFilename, availableClientVersion)
 
 	partialETagFilename := fmt.Sprintf(
-		"%s.%s.part.etag", config.UpgradeDownloadFilename, clientUpgradeVersion)
+		"%s.%s.part.etag", config.UpgradeDownloadFilename, availableClientVersion)
 
 	file, err := os.OpenFile(partialFilename, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600)
 	if err != nil {
@@ -85,7 +172,7 @@ func DownloadUpgrade(config *Config, clientUpgradeVersion string, tunnel *Tunnel
 
 	}
 
-	request, err := http.NewRequest("GET", config.UpgradeDownloadUrl, nil)
+	request, err := http.NewRequest("GET", requestUrl, nil)
 	if err != nil {
 		return ContextError(err)
 	}