Merge pull request #93 from rod-hynes/master

Avoid emitting user input urls in Notices/Changes to embedded server list import

ConsoleClient/psiphonClient.go

@@ -113,27 +113,43 @@ func main() {
 	}
 
 	// Handle optional embedded server list file parameter
-	// If specified, the embedded server list is loaded and stored before
-	// running Psiphon.
-
+	// If specified, the embedded server list is loaded and stored. When there
+	// are no server candidates at all, we wait for this import to complete
+	// before starting the Psiphon controller. Otherwise, we import while
+	// concurrently starting the controller to minimize delay before attempting
+	// to connect to existing candidate servers.
+	// If the import fails, an error notice is emitted, but the controller is
+	// still started: either existing candidate servers may suffice, or the
+	// remote server list fetch may obtain candidate servers.
 	if embeddedServerEntryListFilename != "" {
-		serverEntryList, err := ioutil.ReadFile(embeddedServerEntryListFilename)
-		if err != nil {
-			psiphon.NoticeError("error loading embedded server entry list file: %s", err)
-			os.Exit(1)
-		}
-		// TODO: stream embedded server list data? also, the cast makaes an unnecessary copy of a large buffer?
-		serverEntries, err := psiphon.DecodeAndValidateServerEntryList(string(serverEntryList))
-		if err != nil {
-			psiphon.NoticeError("error decoding embedded server entry list file: %s", err)
-			os.Exit(1)
-		}
-		// Since embedded server list entries may become stale, they will not
-		// overwrite existing stored entries for the same server.
-		err = psiphon.StoreServerEntries(serverEntries, false)
-		if err != nil {
-			psiphon.NoticeError("error storing embedded server entry list data: %s", err)
-			os.Exit(1)
+		embeddedServerListWaitGroup := new(sync.WaitGroup)
+		embeddedServerListWaitGroup.Add(1)
+		go func() {
+			defer embeddedServerListWaitGroup.Done()
+			serverEntryList, err := ioutil.ReadFile(embeddedServerEntryListFilename)
+			if err != nil {
+				psiphon.NoticeError("error loading embedded server entry list file: %s", err)
+				return
+			}
+			// TODO: stream embedded server list data? also, the cast makes an unnecessary copy of a large buffer?
+			serverEntries, err := psiphon.DecodeAndValidateServerEntryList(string(serverEntryList))
+			if err != nil {
+				psiphon.NoticeError("error decoding embedded server entry list file: %s", err)
+				return
+			}
+			// Since embedded server list entries may become stale, they will not
+			// overwrite existing stored entries for the same server.
+			err = psiphon.StoreServerEntries(serverEntries, false)
+			if err != nil {
+				psiphon.NoticeError("error storing embedded server entry list data: %s", err)
+				return
+			}
+		}()
+
+		if psiphon.CountServerEntries(config.EgressRegion, config.TunnelProtocol) == 0 {
+			embeddedServerListWaitGroup.Wait()
+		} else {
+			defer embeddedServerListWaitGroup.Wait()
 		}
 	}
 

psiphon/httpProxy.go

@@ -95,7 +95,7 @@ func NewHttpProxy(
 		Transport: httpTunneledRelay,
 		Jar:       nil, // TODO: cookie support for URL proxy?
 
-		// This timeout cuts downloads of large response bodies
+		// Note: don't use this timeout -- it interrupts downloads of large response bodies
 		//Timeout:   HTTP_PROXY_ORIGIN_SERVER_TIMEOUT,
 	}
 
@@ -110,9 +110,6 @@ func NewHttpProxy(
 	httpDirectClient := &http.Client{
 		Transport: httpDirectRelay,
 		Jar:       nil,
-
-		// This timeout cuts downloads of large response bodies
-		//Timeout:   HTTP_PROXY_ORIGIN_SERVER_TIMEOUT,
 	}
 
 	proxy = &HttpProxy{
@@ -231,7 +228,7 @@ func (proxy *HttpProxy) urlProxyHandler(responseWriter http.ResponseWriter, requ
 		err = errors.New("missing origin URL")
 	}
 	if err != nil {
-		NoticeAlert("%s", ContextError(err))
+		NoticeAlert("%s", ContextError(FilterUrlError(err)))
 		forceClose(responseWriter)
 		return
 	}
@@ -239,7 +236,7 @@ func (proxy *HttpProxy) urlProxyHandler(responseWriter http.ResponseWriter, requ
 	// Origin URL must be well-formed, absolute, and have a scheme of  "http" or "https"
 	url, err := url.ParseRequestURI(originUrl)
 	if err != nil {
-		NoticeAlert("%s", ContextError(err))
+		NoticeAlert("%s", ContextError(FilterUrlError(err)))
 		forceClose(responseWriter)
 		return
 	}
@@ -266,10 +263,9 @@ func relayHttpRequest(client *http.Client, request *http.Request, responseWriter
 	}
 
 	// Relay the HTTP request and get the response
-	//response, err := relay.RoundTrip(request)
 	response, err := client.Do(request)
 	if err != nil {
-		NoticeAlert("%s", ContextError(err))
+		NoticeAlert("%s", ContextError(FilterUrlError(err)))
 		forceClose(responseWriter)
 		return
 	}

psiphon/utils.go

@@ -27,6 +27,7 @@ import (
 	"fmt"
 	"math/big"
 	"net"
+	"net/url"
 	"os"
 	"runtime"
 	"strings"
@@ -120,6 +121,26 @@ func DecodeCertificate(encodedCertificate string) (certificate *x509.Certificate
 	return certificate, nil
 }
 
+// FilterUrlError transforms an error, when it is a url.Error, removing
+// the URL value. This is to avoid logging private user data in cases
+// where the URL may be a user input value.
+// This function is used with errors returned by net/http and net/url,
+// which are (currently) of type url.Error. In particular, the round trip
+// function used by our HttpProxy, http.Client.Do, returns errors of type
+// url.Error, with the URL being the url sent from the user's tunneled
+// applications:
+// https://github.com/golang/go/blob/release-branch.go1.4/src/net/http/client.go#L394
+func FilterUrlError(err error) error {
+	if urlErr, ok := err.(*url.Error); ok {
+		err = &url.Error{
+			Op:  urlErr.Op,
+			URL: "",
+			Err: urlErr.Err,
+		}
+	}
+	return err
+}
+
 // TrimError removes the middle of over-long error message strings
 func TrimError(err error) error {
 	const MAX_LEN = 100