Merge pull request #335 from geebee/master

Use a copy of custom headers when calling UserAgentIfUnset to avoid races

psiphon/TCPConn.go

@@ -177,13 +177,13 @@ func proxiedTcpDial(
 		return tcpDial(addr, config, dialResult)
 	}
 
-	config.UpstreamProxyCustomHeaders, _ = common.UserAgentIfUnset(config.UpstreamProxyCustomHeaders)
+	dialHeaders, _ := common.UserAgentIfUnset(config.UpstreamProxyCustomHeaders)
 
 	upstreamDialer := upstreamproxy.NewProxyDialFunc(
 		&upstreamproxy.UpstreamProxyConfig{
 			ForwardDialFunc: dialer,
 			ProxyURIString:  config.UpstreamProxyUrl,
-			CustomHeaders:   config.UpstreamProxyCustomHeaders,
+			CustomHeaders:   dialHeaders,
 		})
 	netConn, err := upstreamDialer("tcp", addr)
 	if _, ok := err.(*upstreamproxy.Error); ok {

psiphon/common/userAgentPicker.go

@@ -30,7 +30,7 @@ func RegisterUserAgentPicker(generator func() string) {
 	registeredUserAgentPicker.Store(generator)
 }
 
-func PickUserAgent() string {
+func pickUserAgent() string {
 	generator := registeredUserAgentPicker.Load()
 	if generator != nil {
 		return generator.(func() string)()
@@ -38,21 +38,30 @@ func PickUserAgent() string {
 	return ""
 }
 
+// UserAgentIfUnset returns an http.Header object and a boolean
+// representing whether or not its User-Agent header was modified.
+// Any modifications are made to a copy of the original header map
 func UserAgentIfUnset(h http.Header) (http.Header, bool) {
-	selectedUserAgent := false
+	var dialHeaders http.Header
+
 	if _, ok := h["User-Agent"]; !ok {
-		if h == nil {
-			h = make(map[string][]string)
+		dialHeaders = make(map[string][]string)
+
+		if h != nil {
+			for k, v := range h {
+				dialHeaders[k] = make([]string, len(v))
+				copy(dialHeaders[k], v)
+			}
 		}
 
 		if FlipCoin() {
-			h.Set("User-Agent", PickUserAgent())
+			dialHeaders.Set("User-Agent", pickUserAgent())
 		} else {
-			h.Set("User-Agent", "")
+			dialHeaders.Set("User-Agent", "")
 		}
 
-		selectedUserAgent = true
+		return dialHeaders, true
 	}
 
-	return h, selectedUserAgent
+	return h, false
 }

psiphon/tunnel.go

@@ -586,6 +586,8 @@ func dialSsh(
 	useObfuscatedSsh := false
 	var directTCPDialAddress string
 	var meekConfig *MeekConfig
+	var dialHeaders map[string][]string
+	var selectedUserAgent bool
 	var err error
 
 	switch selectedProtocol {
@@ -598,6 +600,8 @@ func dialSsh(
 
 	default:
 		useObfuscatedSsh = true
+		dialHeaders, selectedUserAgent = common.UserAgentIfUnset(config.UpstreamProxyCustomHeaders)
+
 		meekConfig, err = initMeekConfig(config, serverEntry, selectedProtocol, sessionId)
 		if err != nil {
 			return nil, common.ContextError(err)
@@ -623,13 +627,10 @@ func dialSsh(
 		resolvedIPAddress.Store(IPAddress)
 	}
 
-	var selectedUserAgent bool
-	config.UpstreamProxyCustomHeaders, selectedUserAgent = common.UserAgentIfUnset(config.UpstreamProxyCustomHeaders)
-
 	// Create the base transport: meek or direct connection
 	dialConfig := &DialConfig{
 		UpstreamProxyUrl:              config.UpstreamProxyUrl,
-		UpstreamProxyCustomHeaders:    config.UpstreamProxyCustomHeaders,
+		UpstreamProxyCustomHeaders:    dialHeaders,
 		ConnectTimeout:                time.Duration(*config.TunnelConnectTimeoutSeconds) * time.Second,
 		PendingConns:                  pendingConns,
 		DeviceBinder:                  config.DeviceBinder,