Merge pull request #136 from rod-hynes/master

Bug fixes; UNFRONTED-MEEK-HTTPS; secondary DNS server

AndroidLibrary/psi/psi.go

@@ -35,7 +35,8 @@ type PsiphonProvider interface {
 	Notice(noticeJSON string)
 	HasNetworkConnectivity() int
 	BindToDevice(fileDescriptor int) error
-	GetDnsServer() string
+	GetPrimaryDnsServer() string
+	GetSecondaryDnsServer() string
 }
 
 var controller *psiphon.Controller

psiphon/LookupIP.go

@@ -37,7 +37,15 @@ import (
 // to the specified DNS resolver.
 func LookupIP(host string, config *DialConfig) (addrs []net.IP, err error) {
 	if config.DeviceBinder != nil {
-		return bindLookupIP(host, config)
+		addrs, err = bindLookupIP(host, config.DnsServerGetter.GetPrimaryDnsServer(), config)
+		if err == nil {
+			return addrs, err
+		}
+		dnsServer := config.DnsServerGetter.GetSecondaryDnsServer()
+		if dnsServer == "" {
+			return addrs, err
+		}
+		return bindLookupIP(host, dnsServer, config)
 	}
 	return net.LookupIP(host)
 }
@@ -46,7 +54,7 @@ func LookupIP(host string, config *DialConfig) (addrs []net.IP, err error) {
 // To implement socket device binding, the lower-level syscall APIs are used.
 // The sequence of syscalls in this implementation are taken from:
 // https://code.google.com/p/go/issues/detail?id=6966
-func bindLookupIP(host string, config *DialConfig) (addrs []net.IP, err error) {
+func bindLookupIP(host, dnsServer string, config *DialConfig) (addrs []net.IP, err error) {
 
 	// When the input host is an IP address, echo it back
 	ipAddr := net.ParseIP(host)
@@ -65,8 +73,8 @@ func bindLookupIP(host string, config *DialConfig) (addrs []net.IP, err error) {
 		return nil, ContextError(fmt.Errorf("BindToDevice failed: %s", err))
 	}
 
-	// config.DnsServerGetter.GetDnsServer must return an IP address
-	ipAddr = net.ParseIP(config.DnsServerGetter.GetDnsServer())
+	// config.DnsServerGetter.GetDnsServers() must return IP addresses
+	ipAddr = net.ParseIP(dnsServer)
 	if ipAddr == nil {
 		return nil, ContextError(errors.New("invalid IP address"))
 	}
@@ -95,8 +103,6 @@ func bindLookupIP(host string, config *DialConfig) (addrs []net.IP, err error) {
 		conn.SetWriteDeadline(time.Now().Add(config.ConnectTimeout))
 	}
 
-	// TODO: make conn interruptible?
-
 	addrs, _, err = ResolveIP(host, conn)
 	return
 }

psiphon/config.go

@@ -85,6 +85,11 @@ type Config struct {
 	// DataStoreDirectory is the directory in which to store the persistent
 	// database, which contains information such as server entries.
 	// By default, current working directory.
+	//
+	// Warning: If the datastore file, DataStoreDirectory/DATA_STORE_FILENAME,
+	// exists but fails to open for any reason (checksum error, unexpected file
+	// format, etc.) it will be deleted in order to pave a new datastore and
+	// continue running.
 	DataStoreDirectory string
 
 	// DataStoreTempDirectory is the directory in which to store temporary

psiphon/controller.go

@@ -381,7 +381,7 @@ loop:
 			if err == nil {
 				break loop
 			}
-			NoticeAlert("upgrade download failed: ", err)
+			NoticeAlert("upgrade download failed: %s", err)
 		}
 
 		timeout := time.After(DOWNLOAD_UPGRADE_RETRY_PAUSE_PERIOD)

psiphon/dataStore.go

@@ -174,7 +174,6 @@ func StoreServerEntry(serverEntry *ServerEntry, replaceIfExists bool) error {
 	// values (e.g., many servers support all protocols), performance
 	// is expected to be acceptable.
 
-	serverEntryExists := false
 	err = singleton.db.Update(func(tx *bolt.Tx) error {
 
 		serverEntries := tx.Bucket([]byte(serverEntriesBucket))
@@ -212,15 +211,14 @@ func StoreServerEntry(serverEntry *ServerEntry, replaceIfExists bool) error {
 			return ContextError(err)
 		}
 
+		NoticeInfo("updated server %s", serverEntry.IpAddress)
+
 		return nil
 	})
 	if err != nil {
 		return ContextError(err)
 	}
 
-	if !serverEntryExists {
-		NoticeInfo("updated server %s", serverEntry.IpAddress)
-	}
 	return nil
 }
 

psiphon/meekConn.go

@@ -103,9 +103,11 @@ type transporter interface {
 // is spawned which will eventually start HTTP polling.
 // When frontingAddress is not "", fronting is used. This option assumes caller has
 // already checked server entry capabilities.
+// Fronting always uses HTTPS. Otherwise, HTTPS is optional.
 func DialMeek(
 	serverEntry *ServerEntry, sessionId string,
-	frontingAddress string, config *DialConfig) (meek *MeekConn, err error) {
+	useHttps bool, frontingAddress string,
+	config *DialConfig) (meek *MeekConn, err error) {
 
 	// Configure transport
 	// Note: MeekConn has its own PendingConns to manage the underlying HTTP transport connections,
@@ -119,14 +121,12 @@ func DialMeek(
 	*meekConfig = *config
 	meekConfig.PendingConns = pendingConns
 
-	var host string
+	// host is both what is dialed and what ends up in the HTTP Host header
+	host := fmt.Sprintf("%s:%d", serverEntry.IpAddress, serverEntry.MeekServerPort)
 	var dialer Dialer
 	var proxyUrl func(*http.Request) (*url.URL, error)
 
-	if frontingAddress != "" {
-		// In this case, host is not what is dialed but is what ends up in the HTTP Host header
-		host = serverEntry.MeekFrontingHost
-
+	if useHttps || frontingAddress != "" {
 		// Custom TLS dialer:
 		//
 		//  1. ignores the HTTP request address and uses the fronting domain
@@ -160,19 +160,24 @@ func DialMeek(
 		// exclusively connect to non-MiM CDNs); then the adversary kills the underlying TCP connection after
 		// some short period. This is mitigated by the "impaired" protocol classification mechanism.
 
-		dialer = NewCustomTLSDialer(
-			&CustomTLSConfig{
-				Dial:                          NewTCPDialer(meekConfig),
-				Timeout:                       meekConfig.ConnectTimeout,
-				FrontingAddr:                  fmt.Sprintf("%s:%d", frontingAddress, 443),
-				SendServerName:                false,
-				SkipVerify:                    true,
-				UseIndistinguishableTLS:       config.UseIndistinguishableTLS,
-				TrustedCACertificatesFilename: config.TrustedCACertificatesFilename,
-			})
+		customTLSConfig := &CustomTLSConfig{
+			Dial:                          NewTCPDialer(meekConfig),
+			Timeout:                       meekConfig.ConnectTimeout,
+			SendServerName:                false,
+			SkipVerify:                    true,
+			UseIndistinguishableTLS:       config.UseIndistinguishableTLS,
+			TrustedCACertificatesFilename: config.TrustedCACertificatesFilename,
+		}
+
+		if frontingAddress != "" {
+			// In this case, host is not what is dialed but is what ends up in the HTTP Host header
+			host = serverEntry.MeekFrontingHost
+			customTLSConfig.FrontingAddr = fmt.Sprintf("%s:%d", frontingAddress, 443)
+		}
+
+		dialer = NewCustomTLSDialer(customTLSConfig)
+
 	} else {
-		// In the unfronted case, host is both what is dialed and what ends up in the HTTP Host header
-		host = fmt.Sprintf("%s:%d", serverEntry.IpAddress, serverEntry.MeekServerPort)
 
 		if strings.HasPrefix(meekConfig.UpstreamProxyUrl, "http://") {
 			// For unfronted meek, we let the http.Transport handle proxying, as the

psiphon/net.go

@@ -99,7 +99,8 @@ type NetworkConnectivityChecker interface {
 
 // DnsServerGetter defines the interface to the external GetDnsServer provider
 type DnsServerGetter interface {
-	GetDnsServer() string
+	GetPrimaryDnsServer() string
+	GetSecondaryDnsServer() string
 }
 
 // TimeoutError implements the error interface

psiphon/serverApi.go

@@ -630,6 +630,7 @@ func makePsiphonHttpsClient(tunnel *Tunnel) (httpsClient *http.Client, err error
 		return nil, ContextError(err)
 	}
 	tunneledDialer := func(_, addr string) (conn net.Conn, err error) {
+		// TODO: check tunnel.isClosed, and apply TUNNEL_PORT_FORWARD_DIAL_TIMEOUT as in Tunnel.Dial?
 		return tunnel.sshClient.Dial("tcp", addr)
 	}
 	dialer := NewCustomTLSDialer(

psiphon/serverEntry.go

@@ -30,15 +30,17 @@ import (
 )
 
 const (
-	TUNNEL_PROTOCOL_SSH            = "SSH"
-	TUNNEL_PROTOCOL_OBFUSCATED_SSH = "OSSH"
-	TUNNEL_PROTOCOL_UNFRONTED_MEEK = "UNFRONTED-MEEK-OSSH"
-	TUNNEL_PROTOCOL_FRONTED_MEEK   = "FRONTED-MEEK-OSSH"
+	TUNNEL_PROTOCOL_SSH                  = "SSH"
+	TUNNEL_PROTOCOL_OBFUSCATED_SSH       = "OSSH"
+	TUNNEL_PROTOCOL_UNFRONTED_MEEK       = "UNFRONTED-MEEK-OSSH"
+	TUNNEL_PROTOCOL_UNFRONTED_MEEK_HTTPS = "UNFRONTED-MEEK-HTTPS-OSSH"
+	TUNNEL_PROTOCOL_FRONTED_MEEK         = "FRONTED-MEEK-OSSH"
 )
 
 var SupportedTunnelProtocols = []string{
 	TUNNEL_PROTOCOL_FRONTED_MEEK,
 	TUNNEL_PROTOCOL_UNFRONTED_MEEK,
+	TUNNEL_PROTOCOL_UNFRONTED_MEEK_HTTPS,
 	TUNNEL_PROTOCOL_OBFUSCATED_SSH,
 	TUNNEL_PROTOCOL_SSH,
 }

psiphon/tunnel.go

@@ -343,17 +343,24 @@ func dialSsh(
 	// So depending on which protocol is used, multiple layers are initialized.
 	port := 0
 	useMeek := false
+	useMeekHttps := false
 	useFronting := false
 	useObfuscatedSsh := false
 	switch selectedProtocol {
 	case TUNNEL_PROTOCOL_FRONTED_MEEK:
 		useMeek = true
+		useMeekHttps = true
 		useFronting = true
 		useObfuscatedSsh = true
 	case TUNNEL_PROTOCOL_UNFRONTED_MEEK:
 		useMeek = true
 		useObfuscatedSsh = true
 		port = serverEntry.SshObfuscatedPort
+	case TUNNEL_PROTOCOL_UNFRONTED_MEEK_HTTPS:
+		useMeek = true
+		useMeekHttps = true
+		useObfuscatedSsh = true
+		port = serverEntry.SshObfuscatedPort
 	case TUNNEL_PROTOCOL_OBFUSCATED_SSH:
 		useObfuscatedSsh = true
 		port = serverEntry.SshObfuscatedPort
@@ -403,7 +410,7 @@ func dialSsh(
 		TrustedCACertificatesFilename: config.TrustedCACertificatesFilename,
 	}
 	if useMeek {
-		conn, err = DialMeek(serverEntry, sessionId, frontingAddress, dialConfig)
+		conn, err = DialMeek(serverEntry, sessionId, useMeekHttps, frontingAddress, dialConfig)
 		if err != nil {
 			return nil, nil, ContextError(err)
 		}
@@ -556,11 +563,6 @@ func (tunnel *Tunnel) operateTunnel(tunnelOwner TunnelOwner) {
 	totalSent := int64(0)
 	totalReceived := int64(0)
 
-	// Always emit a final NoticeTotalBytesTransferred
-	defer func() {
-		NoticeTotalBytesTransferred(tunnel.serverEntry.IpAddress, totalSent, totalReceived)
-	}()
-
 	noticeBytesTransferredTicker := time.NewTicker(1 * time.Second)
 	defer noticeBytesTransferredTicker.Stop()
 
@@ -703,6 +705,14 @@ func (tunnel *Tunnel) operateTunnel(tunnelOwner TunnelOwner) {
 	close(signalStatusRequest)
 	requestsWaitGroup.Wait()
 
+	// Capture bytes transferred since the last noticeBytesTransferredTicker tick
+	sent, received := transferstats.ReportRecentBytesTransferredForServer(tunnel.serverEntry.IpAddress)
+	totalSent += sent
+	totalReceived += received
+
+	// Always emit a final NoticeTotalBytesTransferred
+	NoticeTotalBytesTransferred(tunnel.serverEntry.IpAddress, totalSent, totalReceived)
+
 	// The stats for this tunnel will be reported via the next successful
 	// status request.
 	// Note: Since client clocks are unreliable, we use the server's reported

psiphon/upgradeDownload.go

@@ -20,8 +20,10 @@
 package psiphon
 
 import (
+	"errors"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"net/http"
 	"os"
 )
@@ -32,16 +34,6 @@ import (
 // config.UpgradeDownloadFilename.
 // NOTE: this code does not check that any existing file at config.UpgradeDownloadFilename
 // is actually the version specified in clientUpgradeVersion.
-//
-// BUG: a download that resumes after automation replaces the server-side upgrade entity
-// will end up with corrupt data (some part of the older entity, followed by part of
-// the newer entity). This is not fatal since authentication of the upgrade package will
-// will detect this and the upgrade will be re-downloaded in its entirety. A fix would
-// involve storing the entity ETag with the partial download and using If-Range
-// (http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.27), or, since S3 doesn't
-// list the If-Range header as supported
-// (http://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectGET.html), If-Match followed
-// be a re-request on failure.
 func DownloadUpgrade(config *Config, clientUpgradeVersion string, tunnel *Tunnel) error {
 
 	// Check if complete file already downloaded
@@ -58,6 +50,9 @@ func DownloadUpgrade(config *Config, clientUpgradeVersion string, tunnel *Tunnel
 	partialFilename := fmt.Sprintf(
 		"%s.%s.part", config.UpgradeDownloadFilename, clientUpgradeVersion)
 
+	partialETagFilename := fmt.Sprintf(
+		"%s.%s.part.etag", config.UpgradeDownloadFilename, clientUpgradeVersion)
+
 	file, err := os.OpenFile(partialFilename, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600)
 	if err != nil {
 		return ContextError(err)
@@ -69,20 +64,50 @@ func DownloadUpgrade(config *Config, clientUpgradeVersion string, tunnel *Tunnel
 		return ContextError(err)
 	}
 
+	// A partial download should have an ETag which is to be sent with the
+	// Range request to ensure that the source object is the same as the
+	// one that is partially downloaded.
+	var partialETag []byte
+	if fileInfo.Size() > 0 {
+
+		partialETag, err = ioutil.ReadFile(partialETagFilename)
+
+		// When the ETag can't be loaded, delete the partial download. To keep the
+		// code simple, there is no immediate, inline retry here, on the assumption
+		// that the controller's upgradeDownloader will shortly call DownloadUpgrade
+		// again.
+		if err != nil {
+			os.Remove(partialFilename)
+			os.Remove(partialETagFilename)
+			return ContextError(
+				fmt.Errorf("failed to load partial download ETag: %s", err))
+		}
+
+	}
+
 	request, err := http.NewRequest("GET", config.UpgradeDownloadUrl, nil)
 	if err != nil {
 		return 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 {
+		request.Header.Add("If-Match", string(partialETag))
+	}
+
 	response, err := httpClient.Do(request)
 
 	// The resumeable download may ask for bytes past the resource range
 	// since it doesn't store the "completed download" state. In this case,
-	// the HTTP server returns 416. Otherwise, we expect 206.
+	// the HTTP server returns 416. Otherwise, we expect 206. We may also
+	// receive 412 on ETag mismatch.
 	if err == nil &&
 		(response.StatusCode != http.StatusPartialContent &&
-			response.StatusCode != http.StatusRequestedRangeNotSatisfiable) {
+			response.StatusCode != http.StatusRequestedRangeNotSatisfiable &&
+			response.StatusCode != http.StatusPreconditionFailed) {
 		response.Body.Close()
 		err = fmt.Errorf("unexpected response status code: %d", response.StatusCode)
 	}
@@ -91,6 +116,18 @@ func DownloadUpgrade(config *Config, clientUpgradeVersion string, tunnel *Tunnel
 	}
 	defer response.Body.Close()
 
+	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.
+		os.Remove(partialFilename)
+		os.Remove(partialETagFilename)
+		return ContextError(errors.New("partial download ETag mismatch"))
+	}
+
+	// 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)
+
 	n, err := io.Copy(NewSyncFileWriter(file), response.Body)
 	if err != nil {
 		return ContextError(err)
@@ -110,6 +147,8 @@ func DownloadUpgrade(config *Config, clientUpgradeVersion string, tunnel *Tunnel
 		return ContextError(err)
 	}
 
+	os.Remove(partialETagFilename)
+
 	NoticeClientUpgradeDownloaded(config.UpgradeDownloadFilename)
 
 	return nil