Lots of tweaks to move the device ID to settings along with some lint…

…er cleanups closes #4319

src/github.com/getlantern/flashlight/app/app.go

@@ -21,7 +21,8 @@ import (
 )
 
 var (
-	log = golog.LoggerFor("flashlight")
+	log      = golog.LoggerFor("flashlight")
+	settings *Settings
 )
 
 func init() {
@@ -52,20 +53,11 @@ func (app *App) Init() {
 
 // LogPanicAndExit logs a panic and then exits the application.
 func (app *App) LogPanicAndExit(msg string) {
-	cfg, err := config.Init(settings,
-		flashlight.PackageVersion,
-		app.Flags["configdir"].(string),
-		app.Flags["stickyconfig"].(bool),
-		app.Flags,
-	)
-	if err != nil {
-		panic("Error initializing config")
-	}
 	if err := logging.EnableFileLogging(); err != nil {
 		panic("Error initializing logging")
 	}
 
-	<-logging.Configure("", cfg.Client.DeviceID, flashlight.Version, flashlight.RevisionDate)
+	<-logging.Configure("", "dummy-device-id-for-panic", flashlight.Version, flashlight.RevisionDate)
 
 	log.Error(msg)
 
@@ -102,7 +94,8 @@ func (app *App) Run() error {
 			app.afterStart,
 			app.onConfigUpdate,
 			settings,
-			app.Exit)
+			app.Exit,
+			settings.GetDeviceID())
 		if err != nil {
 			app.Exit(err)
 			return
@@ -164,7 +157,7 @@ func (app *App) beforeStart(cfg *config.Config) bool {
 
 	// Only run analytics once on startup.
 	if settings.IsAutoReport() {
-		stopAnalytics := analytics.Start(cfg.Client.DeviceID, flashlight.Version)
+		stopAnalytics := analytics.Start(settings.GetDeviceID(), flashlight.Version)
 		app.AddExitFunc(stopAnalytics)
 	}
 	watchDirectAddrs()

src/github.com/getlantern/flashlight/app/settings.go

@@ -1,11 +1,14 @@
 package app
 
 import (
+	"encoding/base64"
 	"io/ioutil"
 	"net/http"
 	"path/filepath"
 	"sync"
 
+	"code.google.com/p/go-uuid/uuid"
+
 	"github.com/getlantern/appdir"
 	"github.com/getlantern/launcher"
 	"github.com/getlantern/yaml"
@@ -19,7 +22,6 @@ const (
 
 var (
 	service    *ui.Service
-	settings   *Settings
 	httpClient *http.Client
 	path       = filepath.Join(appdir.General("Lantern"), "settings.yaml")
 	once       = &sync.Once{}
@@ -49,11 +51,10 @@ func loadSettings(version, revisionDate, buildDate string) *Settings {
 
 // loadSettings loads the initial settings at startup, either from disk or using defaults.
 func loadSettingsFrom(version, revisionDate, buildDate, path string) *Settings {
-
 	log.Debug("Loading settings")
 	// Create default settings that may or may not be overridden from an existing file
 	// on disk.
-	settings = &Settings{
+	set := &Settings{
 		AutoReport:  true,
 		AutoLaunch:  true,
 		ProxyAll:    false,
@@ -63,33 +64,37 @@ func loadSettingsFrom(version, revisionDate, buildDate, path string) *Settings {
 	// Use settings from disk if they're available.
 	if bytes, err := ioutil.ReadFile(path); err != nil {
 		log.Debugf("Could not read file %v", err)
-	} else if err := yaml.Unmarshal(bytes, settings); err != nil {
+	} else if err := yaml.Unmarshal(bytes, set); err != nil {
 		log.Errorf("Could not load yaml %v", err)
 		// Just keep going with the original settings not from disk.
 	} else {
 		log.Debugf("Loaded settings from %v", path)
 	}
 
-	if settings.AutoLaunch {
-		launcher.CreateLaunchFile(settings.AutoLaunch)
+	// We always just set the device ID to the MAC address on the system. Note
+	// this ignores what's on disk, if anything.
+	set.DeviceID = base64.StdEncoding.EncodeToString(uuid.NodeID())
+
+	if set.AutoLaunch {
+		launcher.CreateLaunchFile(set.AutoLaunch)
 	}
 	// always override below 3 attributes as they are not meant to be persisted across versions
-	settings.Version = version
-	settings.BuildDate = buildDate
-	settings.RevisionDate = revisionDate
+	set.Version = version
+	set.BuildDate = buildDate
+	set.RevisionDate = revisionDate
 
 	// Only configure the UI once. This will typically be the case in the normal
 	// application flow, but tests might call Load twice, for example, which we
 	// want to allow.
 	once.Do(func() {
-		err := settings.start()
+		err := set.start()
 		if err != nil {
 			log.Errorf("Unable to register settings service: %q", err)
 			return
 		}
-		go settings.read(service.In, service.Out)
+		go set.read(service.In, service.Out)
 	})
-	return settings
+	return set
 }
 
 // start the settings service that synchronizes Lantern's configuration with every UI client
@@ -214,6 +219,13 @@ func (s *Settings) SetDeviceID(deviceID string) {
 	s.DeviceID = deviceID
 }
 
+// GetDeviceID returns the unique ID of this device.
+func (s *Settings) GetDeviceID() string {
+	s.RLock()
+	defer s.RUnlock()
+	return s.DeviceID
+}
+
 // SetToken sets the user token
 func (s *Settings) SetToken(token string) {
 	s.Lock()

src/github.com/getlantern/flashlight/client/balancer.go

@@ -21,7 +21,7 @@ func (client *Client) getBalancer() *balancer.Balancer {
 
 // initBalancer takes hosts from cfg.ChainedServers and it uses them to create a
 // balancer.
-func (client *Client) initBalancer(cfg *ClientConfig) (*balancer.Balancer, error) {
+func (client *Client) initBalancer(cfg *ClientConfig, deviceID string) (*balancer.Balancer, error) {
 	if len(cfg.ChainedServers) == 0 {
 		return nil, fmt.Errorf("No chained servers configured, not initializing balancer")
 	}
@@ -32,7 +32,7 @@ func (client *Client) initBalancer(cfg *ClientConfig) (*balancer.Balancer, error
 	// Add chained (CONNECT proxy) servers.
 	log.Debugf("Adding %d chained servers", len(cfg.ChainedServers))
 	for _, s := range cfg.ChainedServers {
-		dialer, err := s.Dialer(cfg.DeviceID)
+		dialer, err := s.Dialer(deviceID)
 		if err == nil {
 			log.Debugf("Adding chained server: %v", s.Addr)
 			dialers = append(dialers, dialer)

src/github.com/getlantern/flashlight/client/chained.go

@@ -22,6 +22,8 @@ var ForceChainedProxyAddr string
 // ForceAuthToken - If specified, auth token will be forced to this
 var ForceAuthToken string
 
+// ChainedServerInfo contains all the data for connecting to a given chained
+// server.
 type ChainedServerInfo struct {
 	// Addr: the host:port of the upstream proxy server
 	Addr string

src/github.com/getlantern/flashlight/client/client.go

@@ -21,7 +21,7 @@ const (
 	// LanternSpecialDomain is a special domain for use by lantern that gets
 	// resolved to localhost by the proxy
 	LanternSpecialDomain          = "ui.lantern.io"
-	LanternSpecialDomainWithColon = "ui.lantern.io:"
+	lanternSpecialDomainWithColon = "ui.lantern.io:"
 )
 
 var (
@@ -30,7 +30,8 @@ var (
 	// UIAddr is the address at which UI is to be found
 	UIAddr string
 
-	addr      = eventual.NewValue()
+	addr = eventual.NewValue()
+
 	socksAddr = eventual.NewValue()
 )
 
@@ -43,7 +44,6 @@ type Client struct {
 	// WriteTimeout: (optional) timeout for write ops
 	WriteTimeout time.Duration
 
-	proxyAll  atomic.Value
 	cfgHolder atomic.Value
 	priorCfg  *ClientConfig
 	cfgMutex  sync.RWMutex
@@ -55,12 +55,18 @@ type Client struct {
 	rp eventual.Value
 
 	l net.Listener
+
+	proxyAll func() bool
 }
 
-func NewClient() *Client {
+// NewClient creates a new client that does things like starts the HTTP and
+// SOCKS proxies. It take a function for determing whether or not to proxy
+// all traffic.
+func NewClient(proxyAll func() bool) *Client {
 	return &Client{
-		bal: eventual.NewValue(),
-		rp:  eventual.NewValue(),
+		bal:      eventual.NewValue(),
+		rp:       eventual.NewValue(),
+		proxyAll: proxyAll,
 	}
 }
 
@@ -70,6 +76,8 @@ func Addr(timeout time.Duration) (interface{}, bool) {
 	return addr.Get(timeout)
 }
 
+// Addr returns the address at which the client is listening with HTTP, blocking
+// until the given timeout for an address to become available.
 func (client *Client) Addr(timeout time.Duration) (interface{}, bool) {
 	return Addr(timeout)
 }
@@ -80,6 +88,8 @@ func Socks5Addr(timeout time.Duration) (interface{}, bool) {
 	return socksAddr.Get(timeout)
 }
 
+// Socks5Addr returns the address at which the client is listening with SOCKS5,
+// blocking until the given timeout for an address to become available.
 func (client *Client) Socks5Addr(timeout time.Duration) (interface{}, bool) {
 	return Socks5Addr(timeout)
 }
@@ -116,6 +126,8 @@ func (client *Client) ListenAndServeHTTP(requestedAddr string, onListeningFn fun
 	return httpServer.Serve(l)
 }
 
+// ListenAndServeSOCKS5 starts the SOCKS server listening at the specified
+// address.
 func (client *Client) ListenAndServeSOCKS5(requestedAddr string) error {
 	var err error
 	var l net.Listener
@@ -148,7 +160,7 @@ func (client *Client) ListenAndServeSOCKS5(requestedAddr string) error {
 
 // Configure updates the client's configuration. Configure can be called
 // before or after ListenAndServe, and can be called multiple times.
-func (client *Client) Configure(cfg *ClientConfig, proxyAll func() bool) {
+func (client *Client) Configure(cfg *ClientConfig, deviceID string) {
 	client.cfgMutex.Lock()
 	defer client.cfgMutex.Unlock()
 
@@ -166,10 +178,8 @@ func (client *Client) Configure(cfg *ClientConfig, proxyAll func() bool) {
 
 	log.Debugf("Requiring minimum QOS of %d", cfg.MinQOS)
 	client.cfgHolder.Store(cfg)
-	log.Debugf("Proxy all traffic or not: %v", proxyAll())
-	client.proxyAll.Store(proxyAll)
 
-	bal, err := client.initBalancer(cfg)
+	bal, err := client.initBalancer(cfg, deviceID)
 	if err != nil {
 		log.Error(err)
 	} else if bal != nil {
@@ -185,10 +195,6 @@ func (client *Client) Stop() error {
 	return client.l.Close()
 }
 
-func (client *Client) ProxyAll() bool {
-	return client.proxyAll.Load().(func() bool)()
-}
-
 func (client *Client) cfg() *ClientConfig {
 	return client.cfgHolder.Load().(*ClientConfig)
 }
@@ -198,7 +204,7 @@ func (client *Client) proxiedDialer(orig func(network, addr string) (net.Conn, e
 
 	return func(network, addr string) (net.Conn, error) {
 		var proxied func(network, addr string) (net.Conn, error)
-		if client.ProxyAll() {
+		if client.proxyAll() {
 			proxied = orig
 		} else {
 			proxied = detourDialer
@@ -214,7 +220,7 @@ func (client *Client) proxiedDialer(orig func(network, addr string) (net.Conn, e
 }
 
 func isLanternSpecialDomain(addr string) bool {
-	return strings.Index(addr, LanternSpecialDomainWithColon) == 0
+	return strings.Index(addr, lanternSpecialDomainWithColon) == 0
 }
 
 func rewriteLanternSpecialDomain(addr string) string {

src/github.com/getlantern/flashlight/client/config.go

@@ -15,9 +15,6 @@ type ClientConfig struct {
 	// MinQOS: (optional) the minimum QOS to require from proxies.
 	MinQOS int
 
-	// Unique identifier for this device
-	DeviceID string
-
 	// List of CONNECT ports that are proxied via the remote proxy. Other ports
 	// will be handled with direct connections.
 	ProxiedCONNECTPorts []int

src/github.com/getlantern/flashlight/client/handler.go

@@ -70,13 +70,13 @@ func (client *Client) intercept(resp http.ResponseWriter, req *http.Request) {
 	// after completed send / receive so that won't cause problem.
 	closeConns := func() {
 		if clientConn != nil {
-			if err := clientConn.Close(); err != nil {
-				log.Debugf("Error closing the out connection: %s", err)
+			if closeErr := clientConn.Close(); closeErr != nil {
+				log.Debugf("Error closing the out connection: %s", closeErr)
 			}
 		}
 		if connOut != nil {
-			if err := connOut.Close(); err != nil {
-				log.Debugf("Error closing the client connection: %s", err)
+			if closeErr := connOut.Close(); closeErr != nil {
+				log.Debugf("Error closing the client connection: %s", closeErr)
 			}
 		}
 	}

src/github.com/getlantern/flashlight/client/pluggable_transports.go

@@ -53,8 +53,8 @@ func defaultDialFactory(s *ChainedServerInfo, deviceID string) (dialFN, error) {
 				return nil, err
 			}
 			if !forceProxy && !conn.ConnectionState().PeerCertificates[0].Equal(x509cert) {
-				if err := conn.Close(); err != nil {
-					log.Debugf("Error closing chained server connection: %s", err)
+				if closeErr := conn.Close(); closeErr != nil {
+					log.Debugf("Error closing chained server connection: %s", closeErr)
 				}
 				return nil, fmt.Errorf("Server's certificate didn't match expected!")
 			}

src/github.com/getlantern/flashlight/config/config.go

@@ -2,7 +2,6 @@ package config
 
 import (
 	"crypto/x509"
-	"encoding/base64"
 	"fmt"
 	"net/url"
 	"os"
@@ -11,8 +10,6 @@ import (
 	"sort"
 	"time"
 
-	"code.google.com/p/go-uuid/uuid"
-
 	"github.com/getlantern/appdir"
 	"github.com/getlantern/detour"
 	"github.com/getlantern/fronted"
@@ -227,8 +224,6 @@ func (updated *Config) applyFlags(flags map[string]interface{}) error {
 			updated.CloudConfigCA = value.(string)
 		case "frontedconfig":
 			updated.FrontedCloudConfig = value.(string)
-		case "instanceid":
-			updated.Client.DeviceID = value.(string)
 		case "cpuprofile":
 			updated.CPUProfile = value.(string)
 		case "memprofile":
@@ -326,22 +321,13 @@ func (cfg *Config) applyClientDefaults() {
 			19305, 19306, 19307, 19308, 19309,
 		}
 	}
-
-	if cfg.Client.DeviceID == "" {
-		// There is no true privacy or security in instance ID.  For that, we rely on
-		// transport security.  Hashing MAC would buy us nothing, since the space of
-		// MACs is trivially mapped, especially since the salt would be known
-		cfg.Client.DeviceID = base64.StdEncoding.EncodeToString(uuid.NodeID())
-	}
-
 }
 
 // updateFrom creates a new Config by 'merging' the given yaml into this Config.
 // The masquerade sets, the collections of servers, and the trusted CAs in the
 // update yaml  completely replace the ones in the original Config.
 func (updated *Config) updateFrom(updateBytes []byte) error {
 	// XXX: does this need a mutex, along with everyone that uses the config?
-	oldDeviceID := updated.Client.DeviceID
 	oldChainedServers := updated.Client.ChainedServers
 	oldMasqueradeSets := updated.Client.MasqueradeSets
 	oldTrustedCAs := updated.TrustedCAs
@@ -367,8 +353,5 @@ func (updated *Config) updateFrom(updateBytes []byte) error {
 		}
 		sort.Strings(updated.ProxiedSites.Cloud)
 	}
-
-	// Ignore DeviceID from yaml
-	updated.Client.DeviceID = oldDeviceID
 	return nil
 }