Merge pull request #318 from rod-hynes/master

Support multiple download URLs

psiphon/config.go

@@ -20,6 +20,7 @@
 package psiphon
 
 import (
+	"encoding/base64"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -120,8 +121,20 @@ type Config struct {
 	// be established to known servers.
 	// This value is supplied by and depends on the Psiphon Network, and is
 	// typically embedded in the client binary.
+	//
+	// Deprecated: Use RemoteServerListURLs. When RemoteServerListURLs is
+	// not nil, this parameter is ignored.
 	RemoteServerListUrl string
 
+	// RemoteServerListURLs is list of URLs which specify locations to fetch
+	// out-of-band server entries. This facility is used when a tunnel cannot
+	// be established to known servers.
+	// This value is supplied by and depends on the Psiphon Network, and is
+	// typically embedded in the client binary.
+	// All URLs must point to the same entity with the same ETag. At least
+	// one DownloadURL must have OnlyAfterAttempts = 0.
+	RemoteServerListURLs []*DownloadURL
+
 	// RemoteServerListDownloadFilename specifies a target filename for
 	// storing the remote server list download. Data is stored in co-located
 	// files (RemoteServerListDownloadFilename.part*) to allow for resumable
@@ -138,8 +151,19 @@ type Config struct {
 	// from which to fetch obfuscated server list files.
 	// This value is supplied by and depends on the Psiphon Network, and is
 	// typically embedded in the client binary.
+	//
+	// Deprecated: Use ObfuscatedServerListRootURLs. When
+	// ObfuscatedServerListRootURLs is not nil, this parameter is ignored.
 	ObfuscatedServerListRootURL string
 
+	// ObfuscatedServerListRootURLs is a list of URLs which specify root
+	// locations from which to fetch obfuscated server list files.
+	// This value is supplied by and depends on the Psiphon Network, and is
+	// typically embedded in the client binary.
+	// All URLs must point to the same entity with the same ETag. At least
+	// one DownloadURL must have OnlyAfterAttempts = 0.
+	ObfuscatedServerListRootURLs []*DownloadURL
+
 	// ObfuscatedServerListDownloadDirectory specifies a target directory for
 	// storing the obfuscated remote server list downloads. Data is stored in
 	// co-located files (<OSL filename>.part*) to allow for resumable
@@ -284,17 +308,30 @@ type Config struct {
 	// download facility which downloads this resource and emits a notice when complete.
 	// This value is supplied by and depends on the Psiphon Network, and is
 	// typically embedded in the client binary.
+	//
+	// Deprecated: Use UpgradeDownloadURLs. When UpgradeDownloadURLs
+	// is not nil, this parameter is ignored.
 	UpgradeDownloadUrl string
 
+	// UpgradeDownloadURLs is list of URLs which specify locations from which to
+	// download a host client upgrade file, when one is available. The core tunnel
+	// controller provides a resumable download facility which downloads this resource
+	// and emits a notice when complete.
+	// This value is supplied by and depends on the Psiphon Network, and is
+	// typically embedded in the client binary.
+	// All URLs must point to the same entity with the same ETag. At least
+	// one DownloadURL must have OnlyAfterAttempts = 0.
+	UpgradeDownloadURLs []*DownloadURL
+
 	// UpgradeDownloadClientVersionHeader specifies the HTTP header name for the
-	// entity at UpgradeDownloadUrl which specifies the client version (an integer
+	// entity at UpgradeDownloadURLs 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.
+	// UpgradeDownloadURLs. UpgradeDownloadClientVersionHeader is required when
+	// UpgradeDownloadURLs is specified.
 	UpgradeDownloadClientVersionHeader string
 
 	// UpgradeDownloadFilename is the local target filename for an upgrade download.
-	// This parameter is required when UpgradeDownloadUrl is specified.
+	// This parameter is required when UpgradeDownloadURLs is specified.
 	// Data is stored in co-located files (UpgradeDownloadFilename.part*) to allow
 	// for resumable downloading.
 	UpgradeDownloadFilename string
@@ -414,6 +451,26 @@ type Config struct {
 	EmitSLOKs bool
 }
 
+// DownloadURL specifies a URL for downloading resources along with parameters
+// for the download strategy.
+type DownloadURL struct {
+
+	// URL is the location of the resource. This string is slightly obfuscated
+	// with base64 encoding to mitigate trivial binary executable string scanning.
+	URL string
+
+	// SkipVerify indicates whether to verify HTTPS certificates. It some
+	// circumvention scenarios, verification is not possible. This must
+	// only be set to true when the resource its own verification mechanism.
+	SkipVerify bool
+
+	// OnlyAfterAttempts specifies how to schedule this URL when downloading
+	// the same resource (same entity, same ETag) from multiple different
+	// candidate locations. For a value of N, this URL is only a candidate
+	// after N rounds of attempting the download from other URLs.
+	OnlyAfterAttempts int
+}
+
 // LoadConfig parses and validates a JSON format Psiphon config JSON
 // string and returns a Config struct populated with config values.
 func LoadConfig(configJson []byte) (*Config, error) {
@@ -506,15 +563,37 @@ func LoadConfig(configJson []byte) (*Config, error) {
 			errors.New("invalid TargetApiProtocol"))
 	}
 
-	if config.UpgradeDownloadUrl != "" &&
-		(config.UpgradeDownloadClientVersionHeader == "" || config.UpgradeDownloadFilename == "") {
-		return nil, common.ContextError(errors.New(
-			"UpgradeDownloadUrl requires UpgradeDownloadClientVersionHeader and UpgradeDownloadFilename"))
+	if config.UpgradeDownloadUrl != "" && config.UpgradeDownloadURLs == nil {
+		config.UpgradeDownloadURLs = promoteLegacyDownloadURL(config.UpgradeDownloadUrl)
+	}
+
+	if config.UpgradeDownloadURLs != nil {
+
+		err := decodeAndValidateDownloadURLs("UpgradeDownloadURLs", config.UpgradeDownloadURLs)
+		if err != nil {
+			return nil, common.ContextError(err)
+		}
+
+		if config.UpgradeDownloadClientVersionHeader == "" {
+			return nil, common.ContextError(errors.New("missing UpgradeDownloadClientVersionHeader"))
+		}
+		if config.UpgradeDownloadFilename == "" {
+			return nil, common.ContextError(errors.New("missing UpgradeDownloadFilename"))
+		}
 	}
 
 	if !config.DisableRemoteServerListFetcher {
 
-		if config.RemoteServerListUrl != "" {
+		if config.RemoteServerListUrl != "" && config.RemoteServerListURLs == nil {
+			config.RemoteServerListURLs = promoteLegacyDownloadURL(config.RemoteServerListUrl)
+		}
+
+		if config.RemoteServerListURLs != nil {
+
+			err := decodeAndValidateDownloadURLs("RemoteServerListURLs", config.RemoteServerListURLs)
+			if err != nil {
+				return nil, common.ContextError(err)
+			}
 
 			if config.RemoteServerListSignaturePublicKey == "" {
 				return nil, common.ContextError(errors.New("missing RemoteServerListSignaturePublicKey"))
@@ -525,7 +604,16 @@ func LoadConfig(configJson []byte) (*Config, error) {
 			}
 		}
 
-		if config.ObfuscatedServerListRootURL != "" {
+		if config.ObfuscatedServerListRootURL != "" && config.ObfuscatedServerListRootURLs == nil {
+			config.ObfuscatedServerListRootURLs = promoteLegacyDownloadURL(config.ObfuscatedServerListRootURL)
+		}
+
+		if config.ObfuscatedServerListRootURLs != nil {
+
+			err := decodeAndValidateDownloadURLs("ObfuscatedServerListRootURLs", config.ObfuscatedServerListRootURLs)
+			if err != nil {
+				return nil, common.ContextError(err)
+			}
 
 			if config.RemoteServerListSignaturePublicKey == "" {
 				return nil, common.ContextError(errors.New("missing RemoteServerListSignaturePublicKey"))
@@ -594,3 +682,73 @@ func LoadConfig(configJson []byte) (*Config, error) {
 
 	return &config, nil
 }
+
+func promoteLegacyDownloadURL(URL string) []*DownloadURL {
+	downloadURLs := make([]*DownloadURL, 1)
+	downloadURLs[0] = &DownloadURL{
+		URL:               base64.StdEncoding.EncodeToString([]byte(URL)),
+		SkipVerify:        false,
+		OnlyAfterAttempts: 0,
+	}
+	return downloadURLs
+}
+
+func decodeAndValidateDownloadURLs(name string, downloadURLs []*DownloadURL) error {
+
+	hasOnlyAfterZero := false
+	for _, downloadURL := range downloadURLs {
+		if downloadURL.OnlyAfterAttempts == 0 {
+			hasOnlyAfterZero = true
+		}
+		decodedURL, err := base64.StdEncoding.DecodeString(downloadURL.URL)
+		if err != nil {
+			return fmt.Errorf("failed to decode URL in %s: %s", name, err)
+		}
+
+		downloadURL.URL = string(decodedURL)
+	}
+
+	var err error
+	if !hasOnlyAfterZero {
+		err = fmt.Errorf("must be at least one DownloadURL with OnlyAfterAttempts = 0 in %s", name)
+	}
+
+	return err
+}
+
+func selectDownloadURL(attempt int, downloadURLs []*DownloadURL) (string, string, bool) {
+
+	// The first OnlyAfterAttempts = 0 URL is the canonical URL. This
+	// is the value used as the key for SetUrlETag when multiple download
+	// URLs can be used to fetch a single entity.
+
+	canonicalURL := ""
+	for _, downloadURL := range downloadURLs {
+		if downloadURL.OnlyAfterAttempts == 0 {
+			canonicalURL = downloadURL.URL
+			break
+		}
+	}
+
+	candidates := make([]int, 0)
+	for index, URL := range downloadURLs {
+		if attempt >= URL.OnlyAfterAttempts {
+			candidates = append(candidates, index)
+		}
+	}
+
+	if len(candidates) < 1 {
+		// This case is not expected, as decodeAndValidateDownloadURLs
+		// should reject configs that would have no candidates for
+		// 0 attempts.
+		return "", "", true
+	}
+
+	selection, err := common.MakeSecureRandomInt(len(candidates))
+	if err != nil {
+		selection = 0
+	}
+	downloadURL := downloadURLs[candidates[selection]]
+
+	return downloadURL.URL, canonicalURL, downloadURL.SkipVerify
+}

psiphon/config_test.go

@@ -20,6 +20,7 @@
 package psiphon
 
 import (
+	"encoding/base64"
 	"encoding/json"
 	"io/ioutil"
 	"strings"
@@ -157,3 +158,168 @@ func (suite *ConfigTestSuite) Test_LoadConfig_GoodJson() {
 	_, err = LoadConfig(testObjJSON)
 	suite.Nil(err, "JSON with null for optional values should succeed")
 }
+
+func TestDownloadURLs(t *testing.T) {
+
+	decodedA := "a.example.com"
+	encodedA := base64.StdEncoding.EncodeToString([]byte(decodedA))
+	encodedB := base64.StdEncoding.EncodeToString([]byte("b.example.com"))
+	encodedC := base64.StdEncoding.EncodeToString([]byte("c.example.com"))
+
+	testCases := []struct {
+		description                string
+		downloadURLs               []*DownloadURL
+		attempts                   int
+		expectedValid              bool
+		expectedCanonicalURL       string
+		expectedDistinctSelections int
+	}{
+		{
+			"missing OnlyAfterAttempts = 0",
+			[]*DownloadURL{
+				&DownloadURL{
+					URL:               encodedA,
+					OnlyAfterAttempts: 1,
+				},
+			},
+			1,
+			false,
+			decodedA,
+			0,
+		},
+		{
+			"single URL, multiple attempts",
+			[]*DownloadURL{
+				&DownloadURL{
+					URL:               encodedA,
+					OnlyAfterAttempts: 0,
+				},
+			},
+			2,
+			true,
+			decodedA,
+			1,
+		},
+		{
+			"multiple URLs, single attempt",
+			[]*DownloadURL{
+				&DownloadURL{
+					URL:               encodedA,
+					OnlyAfterAttempts: 0,
+				},
+				&DownloadURL{
+					URL:               encodedB,
+					OnlyAfterAttempts: 1,
+				},
+				&DownloadURL{
+					URL:               encodedC,
+					OnlyAfterAttempts: 1,
+				},
+			},
+			1,
+			true,
+			decodedA,
+			1,
+		},
+		{
+			"multiple URLs, multiple attempts",
+			[]*DownloadURL{
+				&DownloadURL{
+					URL:               encodedA,
+					OnlyAfterAttempts: 0,
+				},
+				&DownloadURL{
+					URL:               encodedB,
+					OnlyAfterAttempts: 1,
+				},
+				&DownloadURL{
+					URL:               encodedC,
+					OnlyAfterAttempts: 1,
+				},
+			},
+			2,
+			true,
+			decodedA,
+			3,
+		},
+		{
+			"multiple URLs, multiple attempts",
+			[]*DownloadURL{
+				&DownloadURL{
+					URL:               encodedA,
+					OnlyAfterAttempts: 0,
+				},
+				&DownloadURL{
+					URL:               encodedB,
+					OnlyAfterAttempts: 1,
+				},
+				&DownloadURL{
+					URL:               encodedC,
+					OnlyAfterAttempts: 3,
+				},
+			},
+			4,
+			true,
+			decodedA,
+			3,
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.description, func(t *testing.T) {
+
+			err := decodeAndValidateDownloadURLs(
+				testCase.description,
+				testCase.downloadURLs)
+
+			if testCase.expectedValid {
+				if err != nil {
+					t.Fatalf("unexpected validation error: %s", err)
+				}
+			} else {
+				if err == nil {
+					t.Fatalf("expected validation error")
+				}
+				return
+			}
+
+			// Track distinct selections for each attempt; the
+			// expected number of distinct should be for at least
+			// one particular attempt.
+			attemptDistinctSelections := make(map[int]map[string]int)
+			for i := 0; i < testCase.attempts; i++ {
+				attemptDistinctSelections[i] = make(map[string]int)
+			}
+
+			// Perform enough runs to account for random selection.
+			runs := 1000
+
+			attempt := 0
+			for i := 0; i < runs; i++ {
+				url, canonicalURL, skipVerify := selectDownloadURL(attempt, testCase.downloadURLs)
+				if canonicalURL != testCase.expectedCanonicalURL {
+					t.Fatalf("unexpected canonical URL: %s", canonicalURL)
+				}
+				if skipVerify {
+					t.Fatalf("expected skipVerify")
+				}
+				attemptDistinctSelections[attempt][url] += 1
+				attempt = (attempt + 1) % testCase.attempts
+			}
+
+			maxDistinctSelections := 0
+			for _, m := range attemptDistinctSelections {
+				if len(m) > maxDistinctSelections {
+					maxDistinctSelections = len(m)
+				}
+			}
+
+			if maxDistinctSelections != testCase.expectedDistinctSelections {
+				t.Fatalf("got %d distinct selections, expected %d",
+					maxDistinctSelections,
+					testCase.expectedDistinctSelections)
+			}
+		})
+	}
+
+}

psiphon/controller.go

@@ -184,7 +184,7 @@ func (controller *Controller) Run(shutdownBroadcast <-chan struct{}) {
 		retryPeriod := time.Duration(
 			*controller.config.FetchRemoteServerListRetryPeriodSeconds) * time.Second
 
-		if controller.config.RemoteServerListUrl != "" {
+		if controller.config.RemoteServerListURLs != nil {
 			controller.runWaitGroup.Add(1)
 			go controller.remoteServerListFetcher(
 				"common",
@@ -194,7 +194,7 @@ func (controller *Controller) Run(shutdownBroadcast <-chan struct{}) {
 				FETCH_REMOTE_SERVER_LIST_STALE_PERIOD)
 		}
 
-		if controller.config.ObfuscatedServerListRootURL != "" {
+		if controller.config.ObfuscatedServerListRootURLs != nil {
 			controller.runWaitGroup.Add(1)
 			go controller.remoteServerListFetcher(
 				"obfuscated",
@@ -205,9 +205,7 @@ func (controller *Controller) Run(shutdownBroadcast <-chan struct{}) {
 		}
 	}
 
-	if controller.config.UpgradeDownloadUrl != "" &&
-		controller.config.UpgradeDownloadFilename != "" {
-
+	if controller.config.UpgradeDownloadURLs != nil {
 		controller.runWaitGroup.Add(1)
 		go controller.upgradeDownloader()
 	}
@@ -324,7 +322,7 @@ fetcherLoop:
 		}
 
 	retryLoop:
-		for {
+		for attempt := 0; ; attempt++ {
 			// Don't attempt to fetch while there is no network connectivity,
 			// to avoid alert notice noise.
 			if !WaitForNetworkConnectivity(
@@ -339,6 +337,7 @@ fetcherLoop:
 
 			err := fetcher(
 				controller.config,
+				attempt,
 				tunnel,
 				controller.untunneledDialConfig)
 
@@ -491,7 +490,7 @@ downloadLoop:
 		}
 
 	retryLoop:
-		for {
+		for attempt := 0; ; attempt++ {
 			// Don't attempt to download while there is no network connectivity,
 			// to avoid alert notice noise.
 			if !WaitForNetworkConnectivity(
@@ -506,6 +505,7 @@ downloadLoop:
 
 			err := DownloadUpgrade(
 				controller.config,
+				attempt,
 				handshakeVersion,
 				tunnel,
 				controller.untunneledDialConfig)
@@ -584,25 +584,15 @@ loop:
 
 			if controller.isImpairedProtocol(establishedTunnel.protocol) {
 
-				NoticeAlert("established tunnel with impaired protocol: %s", establishedTunnel.protocol)
+				// Protocol was classified as impaired while this tunnel established.
+				// This is most likely to occur with TunnelPoolSize > 0. We log the
+				// event but take no action. Discarding the tunnel would break the
+				// impaired logic unless we did that (a) only if there are other
+				// unimpaired protocols; (b) only during the first interation of the
+				// ESTABLISH_TUNNEL_WORK_TIME loop. By not discarding here, a true
+				// impaired protocol may require an extra reconnect.
 
-				// Take action only when TunnelProtocol is unset, as it limits the
-				// controller to a single protocol. For testing and diagnostics, we
-				// still allow classification when TunnelProtocol is set.
-				if controller.config.TunnelProtocol == "" {
-
-					// Protocol was classified as impaired while this tunnel
-					// established, so discard.
-					controller.discardTunnel(establishedTunnel)
-
-					// Reset establish generator to stop producing tunnels
-					// with impaired protocols.
-					if controller.isEstablishing {
-						controller.stopEstablishing()
-						controller.startEstablishing()
-					}
-					break
-				}
+				NoticeAlert("established tunnel with impaired protocol: %s", establishedTunnel.protocol)
 			}
 
 			tunnelCount, registered := controller.registerTunnel(establishedTunnel)
@@ -693,16 +683,24 @@ loop:
 //
 // Concurrency note: only the runTunnels() goroutine may call classifyImpairedProtocol
 func (controller *Controller) classifyImpairedProtocol(failedTunnel *Tunnel) {
+
 	if failedTunnel.establishedTime.Add(IMPAIRED_PROTOCOL_CLASSIFICATION_DURATION).After(monotime.Now()) {
 		controller.impairedProtocolClassification[failedTunnel.protocol] += 1
 	} else {
 		controller.impairedProtocolClassification[failedTunnel.protocol] = 0
 	}
-	if len(controller.getImpairedProtocols()) == len(protocol.SupportedTunnelProtocols) {
-		// Reset classification if all protocols are classified as impaired as
-		// the network situation (or attack) may not be protocol-specific.
-		// TODO: compare against count of distinct supported protocols for
-		// current known server entries.
+
+	// Reset classification once all known protocols are classified as impaired, as
+	// there is now no way to proceed with only unimpaired protocols. The network
+	// situation (or attack) resulting in classification may not be protocol-specific.
+	//
+	// Note: with controller.config.TunnelProtocol set, this will always reset once
+	// that protocol has reached IMPAIRED_PROTOCOL_CLASSIFICATION_THRESHOLD.
+	if CountNonImpairedProtocols(
+		controller.config.EgressRegion,
+		controller.config.TunnelProtocol,
+		controller.getImpairedProtocols()) == 0 {
+
 		controller.impairedProtocolClassification = make(map[string]int)
 	}
 }
@@ -1090,11 +1088,9 @@ loop:
 			// evade the attack; (b) there's a good chance of false
 			// positives (such as short tunnel durations due to network
 			// hopping on a mobile device).
-			// Impaired protocols logic is not applied when
-			// config.TunnelProtocol is specified.
 			// The edited serverEntry is temporary copy which is not
 			// stored or reused.
-			if i == 0 && controller.config.TunnelProtocol == "" {
+			if i == 0 {
 				serverEntry.DisableImpairedProtocols(impairedProtocols)
 				if len(serverEntry.GetSupportedProtocols()) == 0 {
 					// Skip this server entry, as it has no supported

psiphon/controller_test.go

@@ -37,10 +37,10 @@ import (
 	"time"
 
 	"github.com/Psiphon-Inc/goarista/monotime"
+	"github.com/Psiphon-Inc/goproxy"
 	socks "github.com/Psiphon-Inc/goptlib"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/protocol"
-	"github.com/elazarl/goproxy"
 )
 
 var testDataDirName string
@@ -650,8 +650,15 @@ func controllerRun(t *testing.T, runConfig *controllerRunConfig) {
 
 				count, ok := classification[serverProtocol]
 				if ok && count >= IMPAIRED_PROTOCOL_CLASSIFICATION_THRESHOLD {
-					// TODO: wrong goroutine for t.FatalNow()
-					t.Fatalf("unexpected tunnel using impaired protocol: %s, %+v",
+
+					// TODO: Fix this test case. Use of TunnelPoolSize breaks this
+					// case, as many tunnel establishments are occurring in parallel,
+					// and it can happen that a protocol is classified as impaired
+					// while a tunnel with that protocol is established and set
+					// active.
+
+					// *not* t.Fatalf
+					t.Logf("unexpected tunnel using impaired protocol: %s, %+v",
 						serverProtocol, classification)
 				}
 

psiphon/dataStore.go

@@ -365,13 +365,6 @@ func insertRankedServerEntry(tx *bolt.Tx, serverEntryId string, position int) er
 	return nil
 }
 
-func serverEntrySupportsProtocol(serverEntry *protocol.ServerEntry, protocol string) bool {
-	// Note: for meek, the capabilities are FRONTED-MEEK and UNFRONTED-MEEK
-	// and the additonal OSSH service is assumed to be available internally.
-	requiredCapability := strings.TrimSuffix(protocol, "-OSSH")
-	return common.Contains(serverEntry.Capabilities, requiredCapability)
-}
-
 // ServerEntryIterator is used to iterate over
 // stored server entries in rank order.
 type ServerEntryIterator struct {
@@ -573,7 +566,7 @@ func (iterator *ServerEntryIterator) Next() (serverEntry *protocol.ServerEntry,
 
 		// Check filter requirements
 		if (iterator.region == "" || serverEntry.Region == iterator.region) &&
-			(iterator.protocol == "" || serverEntrySupportsProtocol(serverEntry, iterator.protocol)) {
+			(iterator.protocol == "" || serverEntry.SupportsProtocol(iterator.protocol)) {
 
 			break
 		}
@@ -630,7 +623,7 @@ func CountServerEntries(region, tunnelProtocol string) int {
 	count := 0
 	err := scanServerEntries(func(serverEntry *protocol.ServerEntry) {
 		if (region == "" || serverEntry.Region == region) &&
-			(tunnelProtocol == "" || serverEntrySupportsProtocol(serverEntry, tunnelProtocol)) {
+			(tunnelProtocol == "" || serverEntry.SupportsProtocol(tunnelProtocol)) {
 			count += 1
 		}
 	})
@@ -643,6 +636,47 @@ func CountServerEntries(region, tunnelProtocol string) int {
 	return count
 }
 
+// CountNonImpairedProtocols returns the number of distinct tunnel
+// protocols supported by stored server entries, excluding the
+// specified impaired protocols.
+func CountNonImpairedProtocols(
+	region, tunnelProtocol string,
+	impairedProtocols []string) int {
+
+	checkInitDataStore()
+
+	distinctProtocols := make(map[string]bool)
+
+	err := scanServerEntries(func(serverEntry *protocol.ServerEntry) {
+		if region == "" || serverEntry.Region == region {
+			if tunnelProtocol != "" {
+				if serverEntry.SupportsProtocol(tunnelProtocol) {
+					distinctProtocols[tunnelProtocol] = true
+					// Exit early, since only one protocol is enabled
+					return
+				}
+			} else {
+				for _, protocol := range protocol.SupportedTunnelProtocols {
+					if serverEntry.SupportsProtocol(protocol) {
+						distinctProtocols[protocol] = true
+					}
+				}
+			}
+		}
+	})
+
+	for _, protocol := range impairedProtocols {
+		delete(distinctProtocols, protocol)
+	}
+
+	if err != nil {
+		NoticeAlert("CountNonImpairedProtocols failed: %s", err)
+		return 0
+	}
+
+	return len(distinctProtocols)
+}
+
 // ReportAvailableRegions prints a notice with the available egress regions.
 // Note that this report ignores config.TunnelProtocol.
 func ReportAvailableRegions() {

psiphon/feedback.go

@@ -151,7 +151,8 @@ func SendFeedback(configJson, diagnosticsJson, b64EncodedPublicKey, uploadServer
 
 // Attempt to upload feedback data to server.
 func uploadFeedback(config *DialConfig, feedbackData []byte, url string, headerPieces []string) error {
-	client, parsedUrl, err := MakeUntunneledHttpsClient(config, nil, url, time.Duration(FEEDBACK_UPLOAD_TIMEOUT_SECONDS*time.Second))
+	client, parsedUrl, err := MakeUntunneledHttpsClient(
+		config, nil, url, false, time.Duration(FEEDBACK_UPLOAD_TIMEOUT_SECONDS*time.Second))
 	if err != nil {
 		return err
 	}

psiphon/net.go

@@ -239,11 +239,16 @@ func MakeUntunneledHttpsClient(
 	dialConfig *DialConfig,
 	verifyLegacyCertificate *x509.Certificate,
 	requestUrl string,
+	skipVerify bool,
 	requestTimeout time.Duration) (*http.Client, string, error) {
 
 	// Change the scheme to "http"; otherwise http.Transport will try to do
 	// another TLS handshake inside the explicit TLS session. Also need to
 	// force an explicit port, as the default for "http", 80, won't talk TLS.
+	//
+	// TODO: set http.Transport.DialTLS instead of Dial to avoid this hack?
+	// See: https://golang.org/pkg/net/http/#Transport. DialTLS was added in
+	// Go 1.4 but this code may pre-date that.
 
 	urlComponents, err := url.Parse(requestUrl)
 	if err != nil {
@@ -272,7 +277,7 @@ func MakeUntunneledHttpsClient(
 			Dial: NewTCPDialer(dialConfig),
 			VerifyLegacyCertificate:       verifyLegacyCertificate,
 			SNIServerName:                 host,
-			SkipVerify:                    false,
+			SkipVerify:                    skipVerify,
 			UseIndistinguishableTLS:       useIndistinguishableTLS,
 			TrustedCACertificatesFilename: dialConfig.TrustedCACertificatesFilename,
 		})
@@ -297,6 +302,7 @@ func MakeUntunneledHttpsClient(
 func MakeTunneledHttpClient(
 	config *Config,
 	tunnel *Tunnel,
+	skipVerify bool,
 	requestTimeout time.Duration) (*http.Client, error) {
 
 	tunneledDialer := func(_, addr string) (conn net.Conn, err error) {
@@ -307,7 +313,12 @@ func MakeTunneledHttpClient(
 		Dial: tunneledDialer,
 	}
 
-	if config.UseTrustedCACertificatesForStockTLS {
+	if skipVerify {
+
+		transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
+
+	} else if config.UseTrustedCACertificatesForStockTLS {
+
 		if config.TrustedCACertificatesFilename == "" {
 			return nil, common.ContextError(errors.New(
 				"UseTrustedCACertificatesForStockTLS requires TrustedCACertificatesFilename"))
@@ -336,6 +347,7 @@ func MakeDownloadHttpClient(
 	tunnel *Tunnel,
 	untunneledDialConfig *DialConfig,
 	requestUrl string,
+	skipVerify bool,
 	requestTimeout time.Duration) (*http.Client, string, error) {
 
 	var httpClient *http.Client
@@ -343,7 +355,8 @@ func MakeDownloadHttpClient(
 
 	if tunnel != nil {
 		// MakeTunneledHttpClient works with both "http" and "https" schemes
-		httpClient, err = MakeTunneledHttpClient(config, tunnel, requestTimeout)
+		httpClient, err = MakeTunneledHttpClient(
+			config, tunnel, skipVerify, requestTimeout)
 		if err != nil {
 			return nil, "", common.ContextError(err)
 		}
@@ -355,7 +368,7 @@ func MakeDownloadHttpClient(
 		// MakeUntunneledHttpsClient works only with "https" schemes
 		if urlComponents.Scheme == "https" {
 			httpClient, requestUrl, err = MakeUntunneledHttpsClient(
-				untunneledDialConfig, nil, requestUrl, requestTimeout)
+				untunneledDialConfig, nil, requestUrl, skipVerify, requestTimeout)
 			if err != nil {
 				return nil, "", common.ContextError(err)
 			}
@@ -467,6 +480,10 @@ func ResumeDownload(
 	// receive 412 on ETag mismatch.
 	if err == nil &&
 		(response.StatusCode != http.StatusPartialContent &&
+
+			// Certain http servers return 200 OK where we expect 206, so accept that.
+			response.StatusCode != http.StatusOK &&
+
 			response.StatusCode != http.StatusRequestedRangeNotSatisfiable &&
 			response.StatusCode != http.StatusPreconditionFailed &&
 			response.StatusCode != http.StatusNotModified) {

psiphon/remoteServerList.go

@@ -34,7 +34,7 @@ import (
 )
 
 type RemoteServerListFetcher func(
-	config *Config, tunnel *Tunnel, untunneledDialConfig *DialConfig) error
+	config *Config, attempt int, tunnel *Tunnel, untunneledDialConfig *DialConfig) error
 
 // FetchCommonRemoteServerList downloads the common remote server list from
 // config.RemoteServerListUrl. It validates its digital signature using the
@@ -45,16 +45,21 @@ type RemoteServerListFetcher func(
 // be unique and persistent.
 func FetchCommonRemoteServerList(
 	config *Config,
+	attempt int,
 	tunnel *Tunnel,
 	untunneledDialConfig *DialConfig) error {
 
 	NoticeInfo("fetching common remote server list")
 
+	downloadURL, canonicalURL, skipVerify := selectDownloadURL(attempt, config.RemoteServerListURLs)
+
 	newETag, err := downloadRemoteServerListFile(
 		config,
 		tunnel,
 		untunneledDialConfig,
-		config.RemoteServerListUrl,
+		downloadURL,
+		canonicalURL,
+		skipVerify,
 		"",
 		config.RemoteServerListDownloadFilename)
 	if err != nil {
@@ -78,7 +83,7 @@ func FetchCommonRemoteServerList(
 
 	// Now that the server entries are successfully imported, store the response
 	// ETag so we won't re-download this same data again.
-	err = SetUrlETag(config.RemoteServerListUrl, newETag)
+	err = SetUrlETag(canonicalURL, newETag)
 	if err != nil {
 		NoticeAlert("failed to set ETag for common remote server list: %s", common.ContextError(err))
 		// This fetch is still reported as a success, even if we can't store the etag
@@ -100,13 +105,17 @@ func FetchCommonRemoteServerList(
 // must be unique and persistent.
 func FetchObfuscatedServerLists(
 	config *Config,
+	attempt int,
 	tunnel *Tunnel,
 	untunneledDialConfig *DialConfig) error {
 
 	NoticeInfo("fetching obfuscated remote server lists")
 
 	downloadFilename := osl.GetOSLRegistryFilename(config.ObfuscatedServerListDownloadDirectory)
-	downloadURL := osl.GetOSLRegistryURL(config.ObfuscatedServerListRootURL)
+
+	rootURL, canonicalRootURL, skipVerify := selectDownloadURL(attempt, config.ObfuscatedServerListRootURLs)
+	downloadURL := osl.GetOSLRegistryURL(rootURL)
+	canonicalURL := osl.GetOSLRegistryURL(canonicalRootURL)
 
 	// failed is set if any operation fails and should trigger a retry. When the OSL registry
 	// fails to download, any cached registry is used instead; when any single OSL fails
@@ -122,6 +131,8 @@ func FetchObfuscatedServerLists(
 		tunnel,
 		untunneledDialConfig,
 		downloadURL,
+		canonicalURL,
+		skipVerify,
 		"",
 		downloadFilename)
 	if err != nil {
@@ -173,7 +184,7 @@ func FetchObfuscatedServerLists(
 	// When a new registry is downloaded, validated, and parsed, store the
 	// response ETag so we won't re-download this same data again.
 	if !failed && newETag != "" {
-		err = SetUrlETag(downloadURL, newETag)
+		err = SetUrlETag(canonicalURL, newETag)
 		if err != nil {
 			NoticeAlert("failed to set ETag for obfuscated server list registry: %s", common.ContextError(err))
 			// This fetch is still reported as a success, even if we can't store the etag
@@ -199,16 +210,20 @@ func FetchObfuscatedServerLists(
 		})
 
 	for _, oslID := range oslIDs {
+
 		downloadFilename := osl.GetOSLFilename(config.ObfuscatedServerListDownloadDirectory, oslID)
-		downloadURL := osl.GetOSLFileURL(config.ObfuscatedServerListRootURL, oslID)
+
+		downloadURL := osl.GetOSLFileURL(rootURL, oslID)
+		canonicalURL := osl.GetOSLFileURL(canonicalRootURL, oslID)
+
 		hexID := hex.EncodeToString(oslID)
 
 		// Note: the MD5 checksum step assumes the remote server list host's ETag uses MD5
-		// with a hex encoding. If this is not the case, the remoteETag should be left blank.
-		remoteETag := ""
+		// with a hex encoding. If this is not the case, the sourceETag should be left blank.
+		sourceETag := ""
 		md5sum, err := oslRegistry.GetOSLMD5Sum(oslID)
 		if err == nil {
-			remoteETag = hex.EncodeToString(md5sum)
+			sourceETag = fmt.Sprintf("\"%s\"", hex.EncodeToString(md5sum))
 		}
 
 		// TODO: store ETags in OSL registry to enable skipping requests entirely
@@ -218,7 +233,9 @@ func FetchObfuscatedServerLists(
 			tunnel,
 			untunneledDialConfig,
 			downloadURL,
-			remoteETag,
+			canonicalURL,
+			skipVerify,
+			sourceETag,
 			downloadFilename)
 		if err != nil {
 			failed = true
@@ -255,7 +272,7 @@ func FetchObfuscatedServerLists(
 
 		// Now that the server entries are successfully imported, store the response
 		// ETag so we won't re-download this same data again.
-		err = SetUrlETag(downloadURL, newETag)
+		err = SetUrlETag(canonicalURL, newETag)
 		if err != nil {
 			failed = true
 			NoticeAlert("failed to set Etag for obfuscated server list file (%s): %s", hexID, common.ContextError(err))
@@ -280,14 +297,20 @@ func downloadRemoteServerListFile(
 	config *Config,
 	tunnel *Tunnel,
 	untunneledDialConfig *DialConfig,
-	sourceURL, sourceETag, destinationFilename string) (string, error) {
-
-	lastETag, err := GetUrlETag(sourceURL)
+	sourceURL string,
+	canonicalURL string,
+	skipVerify bool,
+	sourceETag string,
+	destinationFilename string) (string, error) {
+
+	// All download URLs with the same canonicalURL
+	// must have the same entity and ETag.
+	lastETag, err := GetUrlETag(canonicalURL)
 	if err != nil {
 		return "", common.ContextError(err)
 	}
 
-	// sourceETag, when specified, is prior knowlegde of the
+	// sourceETag, when specified, is prior knowledge of the
 	// remote ETag that can be used to skip the request entirely.
 	// This will be set in the case of OSL files, from the MD5Sum
 	// values stored in the registry.
@@ -304,6 +327,7 @@ func downloadRemoteServerListFile(
 		tunnel,
 		untunneledDialConfig,
 		sourceURL,
+		skipVerify,
 		time.Duration(*config.FetchRemoteServerListTimeoutSeconds)*time.Second)
 	if err != nil {
 		return "", common.ContextError(err)

psiphon/remoteServerList_test.go

@@ -22,6 +22,7 @@ package psiphon
 import (
 	"bytes"
 	"crypto/md5"
+	"encoding/base64"
 	"encoding/hex"
 	"fmt"
 	"io"
@@ -182,38 +183,55 @@ func TestObfuscatedRemoteServerLists(t *testing.T) {
 	// run mock remote server list host
 	//
 
-	remoteServerListHostAddress := net.JoinHostPort(serverIPaddress, "8081")
+	// Exercise using multiple download URLs
+	remoteServerListHostAddresses := []string{
+		net.JoinHostPort(serverIPaddress, "8081"),
+		net.JoinHostPort(serverIPaddress, "8082"),
+	}
 
 	// The common remote server list fetches will 404
-	remoteServerListURL := fmt.Sprintf("http://%s/server_list_compressed", remoteServerListHostAddress)
+	remoteServerListURL := fmt.Sprintf("http://%s/server_list_compressed", remoteServerListHostAddresses[0])
 	remoteServerListDownloadFilename := filepath.Join(testDataDirName, "server_list_compressed")
 
-	obfuscatedServerListRootURL := fmt.Sprintf("http://%s/", remoteServerListHostAddress)
-	obfuscatedServerListDownloadDirectory := testDataDirName
+	obfuscatedServerListRootURLsJSONConfig := "["
+	obfuscatedServerListRootURLs := make([]string, len(remoteServerListHostAddresses))
+	for i := 0; i < len(remoteServerListHostAddresses); i++ {
 
-	go func() {
-		startTime := time.Now()
-		serveMux := http.NewServeMux()
-		for _, paveFile := range paveFiles {
-			file := paveFile
-			serveMux.HandleFunc("/"+file.Name, func(w http.ResponseWriter, req *http.Request) {
-				md5sum := md5.Sum(file.Contents)
-				w.Header().Add("Content-Type", "application/octet-stream")
-				w.Header().Add("ETag", hex.EncodeToString(md5sum[:]))
-				http.ServeContent(w, req, file.Name, startTime, bytes.NewReader(file.Contents))
-			})
-		}
-		httpServer := &http.Server{
-			Addr:    remoteServerListHostAddress,
-			Handler: serveMux,
-		}
-		err := httpServer.ListenAndServe()
-		if err != nil {
-			// TODO: wrong goroutine for t.FatalNow()
-			t.Fatalf("error running remote server list host: %s", err)
+		obfuscatedServerListRootURLs[i] = fmt.Sprintf("http://%s/", remoteServerListHostAddresses[i])
 
+		obfuscatedServerListRootURLsJSONConfig += fmt.Sprintf(
+			"{\"URL\" : \"%s\"}", base64.StdEncoding.EncodeToString([]byte(obfuscatedServerListRootURLs[i])))
+		if i == len(remoteServerListHostAddresses)-1 {
+			obfuscatedServerListRootURLsJSONConfig += "]"
+		} else {
+			obfuscatedServerListRootURLsJSONConfig += ","
 		}
-	}()
+
+		go func(remoteServerListHostAddress string) {
+			startTime := time.Now()
+			serveMux := http.NewServeMux()
+			for _, paveFile := range paveFiles {
+				file := paveFile
+				serveMux.HandleFunc("/"+file.Name, func(w http.ResponseWriter, req *http.Request) {
+					md5sum := md5.Sum(file.Contents)
+					w.Header().Add("Content-Type", "application/octet-stream")
+					w.Header().Add("ETag", fmt.Sprintf("\"%s\"", hex.EncodeToString(md5sum[:])))
+					http.ServeContent(w, req, file.Name, startTime, bytes.NewReader(file.Contents))
+				})
+			}
+			httpServer := &http.Server{
+				Addr:    remoteServerListHostAddress,
+				Handler: serveMux,
+			}
+			err := httpServer.ListenAndServe()
+			if err != nil {
+				// TODO: wrong goroutine for t.FatalNow()
+				t.Fatalf("error running remote server list host: %s", err)
+			}
+		}(remoteServerListHostAddresses[i])
+	}
+
+	obfuscatedServerListDownloadDirectory := testDataDirName
 
 	//
 	// run Psiphon server
@@ -264,7 +282,7 @@ func TestObfuscatedRemoteServerLists(t *testing.T) {
 					defer waitGroup.Done()
 					io.Copy(remoteConn, localConn)
 				}()
-				if localConn.Req.Target == remoteServerListHostAddress {
+				if common.Contains(remoteServerListHostAddresses, localConn.Req.Target) {
 					io.CopyN(localConn, remoteConn, 500)
 				} else {
 					io.Copy(localConn, remoteConn)
@@ -295,7 +313,7 @@ func TestObfuscatedRemoteServerLists(t *testing.T) {
 		"RemoteServerListSignaturePublicKey" : "%s",
 		"RemoteServerListUrl" : "%s",
 		"RemoteServerListDownloadFilename" : "%s",
-		"ObfuscatedServerListRootURL" : "%s",
+		"ObfuscatedServerListRootURLs" : %s,
 		"ObfuscatedServerListDownloadDirectory" : "%s",
 		"UpstreamProxyUrl" : "%s"
     }`
@@ -305,7 +323,7 @@ func TestObfuscatedRemoteServerLists(t *testing.T) {
 		signingPublicKey,
 		remoteServerListURL,
 		remoteServerListDownloadFilename,
-		obfuscatedServerListRootURL,
+		obfuscatedServerListRootURLsJSONConfig,
 		obfuscatedServerListDownloadDirectory,
 		disruptorProxyURL)
 
@@ -360,11 +378,11 @@ func TestObfuscatedRemoteServerLists(t *testing.T) {
 	}
 
 	for _, paveFile := range paveFiles {
-		u, _ := url.Parse(obfuscatedServerListRootURL)
+		u, _ := url.Parse(obfuscatedServerListRootURLs[0])
 		u.Path = path.Join(u.Path, paveFile.Name)
 		etag, _ := GetUrlETag(u.String())
 		md5sum := md5.Sum(paveFile.Contents)
-		if etag != hex.EncodeToString(md5sum[:]) {
+		if etag != fmt.Sprintf("\"%s\"", hex.EncodeToString(md5sum[:])) {
 			t.Fatalf("unexpected ETag for %s", u)
 		}
 	}

psiphon/server/geoip.go

@@ -145,7 +145,7 @@ func (geoIP *GeoIPService) Lookup(ipAddress string) GeoIPData {
 	}
 
 	// Each database will populate geoIPFields with the values it contains. In the
-	// currnt MaxMind deployment, the City database populates Country and City and
+	// current MaxMind deployment, the City database populates Country and City and
 	// the separate ISP database populates ISP.
 	for _, database := range geoIP.databases {
 		database.ReloadableFile.RLock()

psiphon/serverApi.go

@@ -525,6 +525,7 @@ func (serverContext *ServerContext) doUntunneledStatusRequest(
 		dialConfig,
 		certificate,
 		url,
+		false,
 		timeout)
 	if err != nil {
 		return common.ContextError(err)

psiphon/upgradeDownload.go

@@ -55,10 +55,14 @@ import (
 // upgrade is still pending install by the outer client.
 func DownloadUpgrade(
 	config *Config,
+	attempt int,
 	handshakeVersion string,
 	tunnel *Tunnel,
 	untunneledDialConfig *DialConfig) error {
 
+	// Note: this downloader doesn't use ETags since many client binaries, with
+	// different embedded values, exist for a single version.
+
 	// Check if complete file already downloaded
 
 	if _, err := os.Stat(config.UpgradeDownloadFilename); err == nil {
@@ -68,11 +72,14 @@ func DownloadUpgrade(
 
 	// Select tunneled or untunneled configuration
 
+	downloadURL, _, skipVerify := selectDownloadURL(attempt, config.UpgradeDownloadURLs)
+
 	httpClient, requestUrl, err := MakeDownloadHttpClient(
 		config,
 		tunnel,
 		untunneledDialConfig,
-		config.UpgradeDownloadUrl,
+		downloadURL,
+		skipVerify,
 		DOWNLOAD_UPGRADE_TIMEOUT)
 
 	// If no handshake version is supplied, make an initial HEAD request