Fetch remote server list improvements

* Resumable download, which helps remote server list
eventually download even when download is interrupted
or timesout due to slow network conditions.
* Support new compressed remote server list format.
* Increase retry delay from 5 to 30 seconds to reduce
excess load on remote server list hosts.

psiphon/config.go

@@ -52,7 +52,7 @@ const (
 	HTTP_PROXY_ORIGIN_SERVER_TIMEOUT_SECONDS       = 15
 	HTTP_PROXY_MAX_IDLE_CONNECTIONS_PER_HOST       = 50
 	FETCH_REMOTE_SERVER_LIST_TIMEOUT_SECONDS       = 30
-	FETCH_REMOTE_SERVER_LIST_RETRY_PERIOD          = 5 * time.Second
+	FETCH_REMOTE_SERVER_LIST_RETRY_PERIOD          = 30 * time.Second
 	FETCH_REMOTE_SERVER_LIST_STALE_PERIOD          = 6 * time.Hour
 	PSIPHON_API_CLIENT_SESSION_ID_LENGTH           = 16
 	PSIPHON_API_SERVER_TIMEOUT_SECONDS             = 20

psiphon/controller.go

@@ -275,8 +275,14 @@ fetcherLoop:
 				break fetcherLoop
 			}
 
+			// Pick any active tunnel and make the next fetch attempt. If there's
+			// no active tunnel, the untunneledDialConfig will be used.
+			tunnel := controller.getNextActiveTunnel()
+
 			err := FetchRemoteServerList(
-				controller.config, controller.untunneledDialConfig)
+				controller.config,
+				tunnel,
+				controller.untunneledDialConfig)
 
 			if err == nil {
 				lastFetchTime = time.Now()

psiphon/notice.go

@@ -323,7 +323,18 @@ func NoticeBuildInfo(buildDate, buildRepo, buildRev, goVersion, gomobileVersion
 
 // NoticeExiting indicates that tunnel-core is exiting imminently.
 func NoticeExiting() {
-	outputNotice("Exiting", false, true)
+	outputNotice("Exiting", false, false)
+}
+
+// NoticeRemoteServerListDownloadedBytes reports remote server list download progress.
+func NoticeRemoteServerListDownloadedBytes(bytes int64) {
+	outputNotice("NoticeRemoteServerListDownloadedBytes", true, false, "bytes", bytes)
+}
+
+// NoticeRemoteServerListDownloaded indicates that a remote server list download
+// completed successfully.
+func NoticeRemoteServerListDownloaded() {
+	outputNotice("NoticeRemoteServerListDownloaded", false, false)
 }
 
 type repetitiveNoticeState struct {

psiphon/remoteServerList.go

@@ -20,10 +20,11 @@
 package psiphon
 
 import (
+	"compress/zlib"
 	"errors"
-	"fmt"
 	"io/ioutil"
-	"net/http"
+	"os"
+	"strings"
 	"time"
 )
 
@@ -31,7 +32,11 @@ import (
 // config.RemoteServerListUrl; validates its digital signature using the
 // public key config.RemoteServerListSignaturePublicKey; and parses the
 // data field into ServerEntry records.
-func FetchRemoteServerList(config *Config, dialConfig *DialConfig) (err error) {
+func FetchRemoteServerList(
+	config *Config,
+	tunnel *Tunnel,
+	untunneledDialConfig *DialConfig) (err error) {
+
 	NoticeInfo("fetching remote server list")
 
 	if config.RemoteServerListUrl == "" {
@@ -41,48 +46,66 @@ func FetchRemoteServerList(config *Config, dialConfig *DialConfig) (err error) {
 		return ContextError(errors.New("remote server list signature public key blank"))
 	}
 
-	httpClient, requestUrl, err := MakeUntunneledHttpsClient(
-		dialConfig, nil, config.RemoteServerListUrl, time.Duration(*config.FetchRemoteServerListTimeoutSeconds)*time.Second)
+	// Select tunneled or untunneled configuration
+
+	httpClient, requestUrl, err := makeDownloadHttpClient(
+		config,
+		tunnel,
+		untunneledDialConfig,
+		config.RemoteServerListUrl,
+		time.Duration(*config.FetchRemoteServerListTimeoutSeconds)*time.Second)
 	if err != nil {
 		return ContextError(err)
 	}
 
-	request, err := http.NewRequest("GET", requestUrl, nil)
+	// Proceed with download
+
+	splitPath := strings.Split(config.RemoteServerListUrl, "/")
+	downloadFilename := splitPath[len(splitPath)-1]
+
+	lastETag, err := GetUrlETag(config.RemoteServerListUrl)
 	if err != nil {
 		return ContextError(err)
 	}
 
-	etag, err := GetUrlETag(config.RemoteServerListUrl)
+	n, responseETag, err := resumeDownload(
+		httpClient, requestUrl, downloadFilename, lastETag)
+
+	if responseETag == lastETag {
+		// The remote server list is unchanged and no data was downloaded
+		return nil
+	}
+
+	NoticeRemoteServerListDownloadedBytes(n)
+
 	if err != nil {
 		return ContextError(err)
 	}
-	if etag != "" {
-		request.Header.Add("If-None-Match", etag)
-	}
 
-	response, err := httpClient.Do(request)
+	NoticeRemoteServerListDownloaded()
 
-	if err == nil &&
-		(response.StatusCode != http.StatusOK && response.StatusCode != http.StatusNotModified) {
-		response.Body.Close()
-		err = fmt.Errorf("unexpected response status code: %d", response.StatusCode)
-	}
+	// The downloaded content is a zlib compressed authenticated
+	// data package containing a list of encoded server entries.
+
+	downloadContent, err := os.Open(downloadFilename)
 	if err != nil {
 		return ContextError(err)
 	}
-	defer response.Body.Close()
+	defer downloadContent.Close()
 
-	if response.StatusCode == http.StatusNotModified {
-		return nil
+	zlibReader, err := zlib.NewReader(downloadContent)
+	if err != nil {
+		return ContextError(err)
 	}
 
-	body, err := ioutil.ReadAll(response.Body)
+	dataPackage, err := ioutil.ReadAll(zlibReader)
+	zlibReader.Close()
 	if err != nil {
 		return ContextError(err)
 	}
 
 	remoteServerList, err := ReadAuthenticatedDataPackage(
-		body, config.RemoteServerListSignaturePublicKey)
+		dataPackage, config.RemoteServerListSignaturePublicKey)
 	if err != nil {
 		return ContextError(err)
 	}
@@ -100,11 +123,13 @@ func FetchRemoteServerList(config *Config, dialConfig *DialConfig) (err error) {
 		return ContextError(err)
 	}
 
-	etag = response.Header.Get("ETag")
-	if etag != "" {
-		err := SetUrlETag(config.RemoteServerListUrl, etag)
+	// Now that the server entries are successfully imported, store the response
+	// ETag so we won't re-download this same data again.
+
+	if responseETag != "" {
+		err := SetUrlETag(config.RemoteServerListUrl, responseETag)
 		if err != nil {
-			NoticeAlert("failed to set remote server list etag: %s", ContextError(err))
+			NoticeAlert("failed to set remote server list ETag: %s", ContextError(err))
 			// This fetch is still reported as a success, even if we can't store the etag
 		}
 	}

psiphon/splitTunnel.go

@@ -295,8 +295,7 @@ func (classifier *SplitTunnelClassifier) getRoutes(tunnel *Tunnel) (routesData [
 	}
 
 	if !useCachedRoutes {
-		bytesReader := bytes.NewReader(compressedRoutesData)
-		zlibReader, err := zlib.NewReader(bytesReader)
+		zlibReader, err := zlib.NewReader(bytes.NewReader(compressedRoutesData))
 		if err == nil {
 			routesData, err = ioutil.ReadAll(zlibReader)
 			zlibReader.Close()

psiphon/upgradeDownload.go

@@ -27,6 +27,7 @@ import (
 	"net/http"
 	"os"
 	"strconv"
+	"time"
 )
 
 // DownloadUpgrade performs a resumable download of client upgrade files.
@@ -67,24 +68,14 @@ func DownloadUpgrade(
 		return nil
 	}
 
-	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)
-		}
-	}
+	httpClient, requestUrl, err := makeDownloadHttpClient(
+		config,
+		tunnel,
+		untunneledDialConfig,
+		config.UpgradeDownloadUrl,
+		DOWNLOAD_UPGRADE_TIMEOUT)
 
 	// If no handshake version is supplied, make an initial HEAD request
 	// to get the current version from the version header.
@@ -133,23 +124,95 @@ func DownloadUpgrade(
 		}
 	}
 
-	// Proceed with full download
+	// Proceed with download
 
-	partialFilename := fmt.Sprintf(
-		"%s.%s.part", config.UpgradeDownloadFilename, availableClientVersion)
+	// An intermediate filename is used since the presence of
+	// config.UpgradeDownloadFilename indicates a completed download.
 
-	partialETagFilename := fmt.Sprintf(
-		"%s.%s.part.etag", config.UpgradeDownloadFilename, availableClientVersion)
+	downloadFilename := fmt.Sprintf(
+		"%s.%s", config.UpgradeDownloadFilename, availableClientVersion)
 
-	file, err := os.OpenFile(partialFilename, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600)
+	n, _, err := resumeDownload(
+		httpClient, requestUrl, downloadFilename, "")
+
+	NoticeClientUpgradeDownloadedBytes(n)
+
+	if err != nil {
+		return ContextError(err)
+	}
+
+	err = os.Rename(downloadFilename, config.UpgradeDownloadFilename)
 	if err != nil {
 		return ContextError(err)
 	}
+
+	NoticeClientUpgradeDownloaded(config.UpgradeDownloadFilename)
+
+	return nil
+}
+
+// makeDownloadHttpClient is a resusable helper that sets up a
+// http.Client for use either untunneled or through a tunnel.
+// See MakeUntunneledHttpsClient for a note about request URL
+// rewritting.
+func makeDownloadHttpClient(
+	config *Config,
+	tunnel *Tunnel,
+	untunneledDialConfig *DialConfig,
+	requestUrl string,
+	requestTimeout time.Duration) (*http.Client, string, error) {
+
+	var httpClient *http.Client
+	var err error
+
+	if tunnel != nil {
+		httpClient, err = MakeTunneledHttpClient(config, tunnel, requestTimeout)
+		if err != nil {
+			return nil, "", ContextError(err)
+		}
+	} else {
+		httpClient, requestUrl, err = MakeUntunneledHttpsClient(
+			untunneledDialConfig, nil, requestUrl, requestTimeout)
+		if err != nil {
+			return nil, "", ContextError(err)
+		}
+	}
+
+	return httpClient, requestUrl, nil
+}
+
+// resumeDownload is a resuable helper that downloads requestUrl via the
+// httpClient, storing the result in downloadFilename when the download is
+// complete. Intermediate, partial downloads state is stored in
+// downloadFilename.part and downloadFilename.part.etag.
+//
+// In the case where the remote object has change while a partial download
+// is to be resumed, the partial state is reset and resumeDownload fails.
+// The caller must restart the download.
+//
+// When ifNoneMatchETag is specified, no download is made if the remote
+// object has the same ETag. ifNoneMatchETag has an effect only when no
+// partial download is in progress.
+//
+func resumeDownload(
+	httpClient *http.Client,
+	requestUrl string,
+	downloadFilename string,
+	ifNoneMatchETag string) (int64, string, error) {
+
+	partialFilename := fmt.Sprintf("%s.part", downloadFilename)
+
+	partialETagFilename := fmt.Sprintf("%s.part.etag", downloadFilename)
+
+	file, err := os.OpenFile(partialFilename, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600)
+	if err != nil {
+		return 0, "", ContextError(err)
+	}
 	defer file.Close()
 
 	fileInfo, err := file.Stat()
 	if err != nil {
-		return ContextError(err)
+		return 0, "", ContextError(err)
 	}
 
 	// A partial download should have an ETag which is to be sent with the
@@ -167,23 +230,41 @@ func DownloadUpgrade(
 		if err != nil {
 			os.Remove(partialFilename)
 			os.Remove(partialETagFilename)
-			return ContextError(
+			return 0, "", ContextError(
 				fmt.Errorf("failed to load partial download ETag: %s", err))
 		}
-
 	}
 
 	request, err := http.NewRequest("GET", requestUrl, nil)
 	if err != nil {
-		return ContextError(err)
+		return 0, "", ContextError(err)
 	}
+
 	request.Header.Add("Range", fmt.Sprintf("bytes=%d-", fileInfo.Size()))
 
-	// Note: not using If-Range, since not all remote server list host servers
-	// support it. Using If-Match means we need to check for status code 412
-	// and reset when the ETag has changed since the last partial download.
 	if partialETag != nil {
+
+		// Note: not using If-Range, since not all host servers support it.
+		// Using If-Match means we need to check for status code 412 and reset
+		// when the ETag has changed since the last partial download.
 		request.Header.Add("If-Match", string(partialETag))
+
+	} else if ifNoneMatchETag != "" {
+
+		// Can't specify both If-Match and If-None-Match. Behavior is undefined.
+		// https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.26
+		// So for downloaders that store an ETag and wish to use that to prevent
+		// redundant downloads, that ETag is sent as If-None-Match in the case
+		// where a partial download is not in progress. When a partial download
+		// is in progress, the partial ETag is sent as If-Match: either that's
+		// a version that was never fully received, or it's no longer current in
+		// which case the response will be StatusPreconditionFailed, the partial
+		// download will be discarded, and then the next retry will use
+		// If-None-Match.
+
+		// Note: in this case, fileInfo.Size() == 0
+
+		request.Header.Add("If-None-Match", ifNoneMatchETag)
 	}
 
 	response, err := httpClient.Do(request)
@@ -195,52 +276,59 @@ func DownloadUpgrade(
 	if err == nil &&
 		(response.StatusCode != http.StatusPartialContent &&
 			response.StatusCode != http.StatusRequestedRangeNotSatisfiable &&
-			response.StatusCode != http.StatusPreconditionFailed) {
+			response.StatusCode != http.StatusPreconditionFailed &&
+			response.StatusCode != http.StatusNotModified) {
 		response.Body.Close()
 		err = fmt.Errorf("unexpected response status code: %d", response.StatusCode)
 	}
 	if err != nil {
-		return ContextError(err)
+		return 0, "", ContextError(err)
 	}
 	defer response.Body.Close()
 
+	responseETag := response.Header.Get("ETag")
+
 	if response.StatusCode == http.StatusPreconditionFailed {
 		// When the ETag no longer matches, delete the partial download. As above,
-		// simply failing and relying on the controller's upgradeDownloader retry.
+		// simply failing and relying on the caller's retry schedule.
+		os.Remove(partialFilename)
+		os.Remove(partialETagFilename)
+		return 0, "", ContextError(errors.New("partial download ETag mismatch"))
+
+	} else if response.StatusCode == http.StatusNotModified {
+		// This status code is possible in the "If-None-Match" case. Don't leave
+		// any partial download in progress. Caller should check that responseETag
+		// matches ifNoneMatchETag.
 		os.Remove(partialFilename)
 		os.Remove(partialETagFilename)
-		return ContextError(errors.New("partial download ETag mismatch"))
+		return 0, responseETag, nil
 	}
 
 	// Not making failure to write ETag file fatal, in case the entire download
 	// succeeds in this one request.
-	ioutil.WriteFile(partialETagFilename, []byte(response.Header.Get("ETag")), 0600)
+	ioutil.WriteFile(partialETagFilename, []byte(responseETag), 0600)
 
 	// A partial download occurs when this copy is interrupted. The io.Copy
 	// will fail, leaving a partial download in place (.part and .part.etag).
 	n, err := io.Copy(NewSyncFileWriter(file), response.Body)
 
-	NoticeClientUpgradeDownloadedBytes(n)
-
 	if err != nil {
-		return ContextError(err)
+		return 0, "", ContextError(err)
 	}
 
 	// Ensure the file is flushed to disk. The deferred close
 	// will be a noop when this succeeds.
 	err = file.Close()
 	if err != nil {
-		return ContextError(err)
+		return 0, "", ContextError(err)
 	}
 
-	err = os.Rename(partialFilename, config.UpgradeDownloadFilename)
+	err = os.Rename(partialFilename, downloadFilename)
 	if err != nil {
-		return ContextError(err)
+		return 0, "", ContextError(err)
 	}
 
 	os.Remove(partialETagFilename)
 
-	NoticeClientUpgradeDownloaded(config.UpgradeDownloadFilename)
-
-	return nil
+	return n, responseETag, nil
 }