Changes to notice functionality

* Don't emit diagnotic-type notices unless configured to do so
* Emit build info as soon as possible, so it will appear before
  initialization errors

AndroidLibrary/psi/psi.go

@@ -68,6 +68,8 @@ func Start(
 			provider.Notice(string(notice))
 		}))
 
+	psiphon.EmitNoticeBuildInfo()
+
 	// TODO: should following errors be Notices?
 
 	err = psiphon.InitDataStore(config)

ConsoleClient/psiphonClient.go

@@ -61,6 +61,8 @@ func main() {
 	}
 	psiphon.SetNoticeOutput(noticeWriter)
 
+	psiphon.EmitNoticeBuildInfo()
+
 	// Handle required config file parameter
 
 	if configFilename == "" {

psiphon/buildinfo.go

@@ -44,22 +44,11 @@ var goVersion string
 // -X github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon.gomobileVersion=`gomobile version | perl -ne '/gomobile version (.*?) / && print $1'`
 var gomobileVersion string
 
-func NoticeBuildInfo() {
-
-	if len(strings.TrimSpace(gomobileVersion)) > 0 {
-		NoticeInfo(
-			"Built: %#v from %#v at rev %#v using go: %#v and gomobile: %#v",
-			strings.TrimSpace(buildDate),
-			strings.TrimSpace(buildRepo),
-			strings.TrimSpace(buildRev),
-			strings.TrimSpace(goVersion),
-			strings.TrimSpace(gomobileVersion))
-	} else {
-		NoticeInfo(
-			"Built: %#v from %#v at rev %#v using go: %#v",
-			strings.TrimSpace(buildDate),
-			strings.TrimSpace(buildRepo),
-			strings.TrimSpace(buildRev),
-			strings.TrimSpace(goVersion))
-	}
+func EmitNoticeBuildInfo() {
+	NoticeBuildInfo(
+		strings.TrimSpace(buildDate),
+		strings.TrimSpace(buildRepo),
+		strings.TrimSpace(buildRev),
+		strings.TrimSpace(goVersion),
+		strings.TrimSpace(gomobileVersion))
 }

psiphon/config.go

@@ -289,6 +289,12 @@ type Config struct {
 	// When provided, this value may be used, pre-connection, to select performance
 	// or circumvention optimization strategies for the given region.
 	DeviceRegion string
+
+	// EmitDiagnosticNotices indicates whether to output notices containing detailed
+	// information about the Psiphon session. As these notices may contain sensitive
+	// network information, they should not be insecurely distributed or displayed
+	// to users.
+	EmitDiagnosticNotices bool
 }
 
 // LoadConfig parses and validates a JSON format Psiphon config JSON
@@ -353,5 +359,9 @@ func LoadConfig(configJson []byte) (*Config, error) {
 		return nil, ContextError(errors.New("DnsServerGetter interface must be set at runtime"))
 	}
 
+	if config.EmitDiagnosticNotices {
+		setEmitDiagnosticNotices(true)
+	}
+
 	return &config, nil
 }

psiphon/controller.go

@@ -138,7 +138,6 @@ func NewController(config *Config) (controller *Controller, err error) {
 // - a local SOCKS proxy that port forwards through the pool of tunnels
 // - a local HTTP proxy that port forwards through the pool of tunnels
 func (controller *Controller) Run(shutdownBroadcast <-chan struct{}) {
-	NoticeBuildInfo()
 	ReportAvailableRegions()
 
 	// Start components

psiphon/notice.go

@@ -29,11 +29,25 @@ import (
 	"sort"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"time"
 )
 
 var noticeLoggerMutex sync.Mutex
 var noticeLogger = log.New(os.Stderr, "", 0)
+var noticeLogDiagnostics = int32(0)
+
+func setEmitDiagnosticNotices(enable bool) {
+	if enable {
+		atomic.StoreInt32(&noticeLogDiagnostics, 1)
+	} else {
+		atomic.StoreInt32(&noticeLogDiagnostics, 0)
+	}
+}
+
+func getEmitDiagnoticNotices() bool {
+	return atomic.LoadInt32(&noticeLogDiagnostics) == 1
+}
 
 // SetNoticeOutput sets a target writer to receive notices. By default,
 // notices are written to stderr.
@@ -60,7 +74,12 @@ func SetNoticeOutput(output io.Writer) {
 }
 
 // outputNotice encodes a notice in JSON and writes it to the output writer.
-func outputNotice(noticeType string, showUser bool, args ...interface{}) {
+func outputNotice(noticeType string, isDiagnostic, showUser bool, args ...interface{}) {
+
+	if isDiagnostic && !getEmitDiagnoticNotices() {
+		return
+	}
+
 	obj := make(map[string]interface{})
 	noticeData := make(map[string]interface{})
 	obj["noticeType"] = noticeType
@@ -88,22 +107,22 @@ func outputNotice(noticeType string, showUser bool, args ...interface{}) {
 
 // NoticeInfo is an informational message
 func NoticeInfo(format string, args ...interface{}) {
-	outputNotice("Info", false, "message", fmt.Sprintf(format, args...))
+	outputNotice("Info", true, false, "message", fmt.Sprintf(format, args...))
 }
 
 // NoticeAlert is an alert message; typically a recoverable error condition
 func NoticeAlert(format string, args ...interface{}) {
-	outputNotice("Alert", false, "message", fmt.Sprintf(format, args...))
+	outputNotice("Alert", true, false, "message", fmt.Sprintf(format, args...))
 }
 
 // NoticeError is an error message; typically an unrecoverable error condition
 func NoticeError(format string, args ...interface{}) {
-	outputNotice("Error", true, "message", fmt.Sprintf(format, args...))
+	outputNotice("Error", true, false, "message", fmt.Sprintf(format, args...))
 }
 
 // NoticeCandidateServers is how many possible servers are available for the selected region and protocol
 func NoticeCandidateServers(region, protocol string, count int) {
-	outputNotice("CandidateServers", false, "region", region, "protocol", protocol, "count", count)
+	outputNotice("CandidateServers", false, false, "region", region, "protocol", protocol, "count", count)
 }
 
 // NoticeAvailableEgressRegions is what regions are available for egress from.
@@ -114,63 +133,63 @@ func NoticeAvailableEgressRegions(regions []string) {
 	repetitionMessage := strings.Join(sortedRegions, "")
 	outputRepetitiveNotice(
 		"AvailableEgressRegions", repetitionMessage, 0,
-		"AvailableEgressRegions", false, "regions", sortedRegions)
+		"AvailableEgressRegions", false, false, "regions", sortedRegions)
 }
 
 // NoticeConnectingServer is details on a connection attempt
 func NoticeConnectingServer(ipAddress, region, protocol, frontingAddress string) {
-	outputNotice("ConnectingServer", false, "ipAddress", ipAddress, "region",
+	outputNotice("ConnectingServer", true, false, "ipAddress", ipAddress, "region",
 		region, "protocol", protocol, "frontingAddress", frontingAddress)
 }
 
 // NoticeActiveTunnel is a successful connection that is used as an active tunnel for port forwarding
 func NoticeActiveTunnel(ipAddress, protocol string) {
-	outputNotice("ActiveTunnel", false, "ipAddress", ipAddress, "protocol", protocol)
+	outputNotice("ActiveTunnel", true, false, "ipAddress", ipAddress, "protocol", protocol)
 }
 
 // NoticeSocksProxyPortInUse is a failure to use the configured LocalSocksProxyPort
 func NoticeSocksProxyPortInUse(port int) {
-	outputNotice("SocksProxyPortInUse", true, "port", port)
+	outputNotice("SocksProxyPortInUse", false, true, "port", port)
 }
 
 // NoticeListeningSocksProxyPort is the selected port for the listening local SOCKS proxy
 func NoticeListeningSocksProxyPort(port int) {
-	outputNotice("ListeningSocksProxyPort", false, "port", port)
+	outputNotice("ListeningSocksProxyPort", false, false, "port", port)
 }
 
 // NoticeSocksProxyPortInUse is a failure to use the configured LocalHttpProxyPort
 func NoticeHttpProxyPortInUse(port int) {
-	outputNotice("HttpProxyPortInUse", true, "port", port)
+	outputNotice("HttpProxyPortInUse", false, true, "port", port)
 }
 
 // NoticeListeningSocksProxyPort is the selected port for the listening local HTTP proxy
 func NoticeListeningHttpProxyPort(port int) {
-	outputNotice("ListeningHttpProxyPort", false, "port", port)
+	outputNotice("ListeningHttpProxyPort", false, false, "port", port)
 }
 
 // NoticeClientUpgradeAvailable is an available client upgrade, as per the handshake. The
 // client should download and install an upgrade.
 func NoticeClientUpgradeAvailable(version string) {
-	outputNotice("ClientUpgradeAvailable", false, "version", version)
+	outputNotice("ClientUpgradeAvailable", false, false, "version", version)
 }
 
 // NoticeClientUpgradeAvailable is a sponsor homepage, as per the handshake. The client
 // should display the sponsor's homepage.
 func NoticeHomepage(url string) {
-	outputNotice("Homepage", false, "url", url)
+	outputNotice("Homepage", false, false, "url", url)
 }
 
 // NoticeClientRegion is the client's region, as determined by the server and
 // reported to the client in the handshake.
 func NoticeClientRegion(region string) {
-	outputNotice("ClientRegion", false, "region", region)
+	outputNotice("ClientRegion", true, false, "region", region)
 }
 
 // NoticeTunnels is how many active tunnels are available. The client should use this to
 // determine connecting/unexpected disconnect state transitions. When count is 0, the core is
 // disconnected; when count > 1, the core is connected.
 func NoticeTunnels(count int) {
-	outputNotice("Tunnels", false, "count", count)
+	outputNotice("Tunnels", false, false, "count", count)
 }
 
 // NoticeUntunneled indicates than an address has been classified as untunneled and is being
@@ -180,48 +199,48 @@ func NoticeTunnels(count int) {
 // users, not for diagnostics logs.
 //
 func NoticeUntunneled(address string) {
-	outputNotice("Untunneled", true, "address", address)
+	outputNotice("Untunneled", false, true, "address", address)
 }
 
 // NoticeSplitTunnelRegion reports that split tunnel is on for the given region.
 func NoticeSplitTunnelRegion(region string) {
-	outputNotice("SplitTunnelRegion", true, "region", region)
+	outputNotice("SplitTunnelRegion", false, true, "region", region)
 }
 
 // NoticeUpstreamProxyError reports an error when connecting to an upstream proxy. The
 // user may have input, for example, an incorrect address or incorrect credentials.
 func NoticeUpstreamProxyError(err error) {
-	outputNotice("UpstreamProxyError", true, "message", err.Error())
+	outputNotice("UpstreamProxyError", false, true, "message", err.Error())
 }
 
 // NoticeClientUpgradeDownloaded indicates that a client upgrade download
 // is complete and available at the destination specified.
 func NoticeClientUpgradeDownloaded(filename string) {
-	outputNotice("ClientUpgradeDownloaded", false, "filename", filename)
+	outputNotice("ClientUpgradeDownloaded", false, false, "filename", filename)
 }
 
 // NoticeBytesTransferred reports how many tunneled bytes have been
 // transferred since the last NoticeBytesTransferred, for the tunnel
 // to the server at ipAddress.
 func NoticeBytesTransferred(ipAddress string, sent, received int64) {
-	outputNotice("BytesTransferred", false, "ipAddress", ipAddress, "sent", sent, "received", received)
+	if getEmitDiagnoticNotices() {
+		outputNotice("BytesTransferred", true, false, "ipAddress", ipAddress, "sent", sent, "received", received)
+	} else {
+		// This case keeps the EmitBytesTransferred and EmitDiagnosticNotices config options independent
+		outputNotice("BytesTransferred", false, false, "sent", sent, "received", received)
+	}
 }
 
 // NoticeTotalBytesTransferred reports how many tunneled bytes have been
 // transferred in total up to this point, for the tunnel to the server
 // at ipAddress.
 func NoticeTotalBytesTransferred(ipAddress string, sent, received int64) {
-	outputNotice("TotalBytesTransferred", false, "ipAddress", ipAddress, "sent", sent, "received", received)
-}
-
-// NoticeFrontedMeekStats reports extra network details for a
-// FRONTED-MEEK-OSSH tunnel connection.
-func NoticeFrontedMeekStats(ipAddress string, frontedMeekStats *FrontedMeekStats) {
-	outputNotice("NoticeFrontedMeekStats", false, "ipAddress", ipAddress,
-		"frontingAddress", frontedMeekStats.frontingAddress,
-		"resolvedIPAddress", frontedMeekStats.resolvedIPAddress,
-		"enabledSNI", frontedMeekStats.enabledSNI,
-		"frontingHost", frontedMeekStats.frontingHost)
+	if getEmitDiagnoticNotices() {
+		outputNotice("TotalBytesTransferred", true, false, "ipAddress", ipAddress, "sent", sent, "received", received)
+	} else {
+		// This case keeps the EmitBytesTransferred and EmitDiagnosticNotices config options independent
+		outputNotice("TotalBytesTransferred", false, false, "sent", sent, "received", received)
+	}
 }
 
 // NoticeLocalProxyError reports a local proxy error message. Repetitive
@@ -241,7 +260,27 @@ func NoticeLocalProxyError(proxyType string, err error) {
 
 	outputRepetitiveNotice(
 		"LocalProxyError"+proxyType, repetitionMessage, 1,
-		"LocalProxyError", false, "message", err.Error())
+		"LocalProxyError", true, false, "message", err.Error())
+}
+
+// NoticeFrontedMeekStats reports extra network details for a
+// FRONTED-MEEK-OSSH or FRONTED-MEEK-HTTP-OSSH tunnel connection.
+func NoticeFrontedMeekStats(ipAddress string, frontedMeekStats *FrontedMeekStats) {
+	outputNotice("NoticeFrontedMeekStats", true, false, "ipAddress", ipAddress,
+		"frontingAddress", frontedMeekStats.frontingAddress,
+		"resolvedIPAddress", frontedMeekStats.resolvedIPAddress,
+		"enabledSNI", frontedMeekStats.enabledSNI,
+		"frontingHost", frontedMeekStats.frontingHost)
+}
+
+// NoticeBuildInfo reports build version info.
+func NoticeBuildInfo(buildDate, buildRepo, buildRev, goVersion, gomobileVersion string) {
+	outputNotice("NoticeBuildInfo", false, false,
+		"buildDate", buildDate,
+		"buildRepo", buildRepo,
+		"buildRev", buildRev,
+		"goVersion", goVersion,
+		"gomobileVersion", gomobileVersion)
 }
 
 type repetitiveNoticeState struct {
@@ -258,7 +297,7 @@ var repetitiveNoticeStates = make(map[string]*repetitiveNoticeState)
 // until the repetitionMessage differs.
 func outputRepetitiveNotice(
 	repetitionKey, repetitionMessage string, repeatLimit int,
-	noticeType string, showUser bool, args ...interface{}) {
+	noticeType string, isDiagnostic, showUser bool, args ...interface{}) {
 
 	repetitiveNoticeMutex.Lock()
 	defer repetitiveNoticeMutex.Unlock()
@@ -284,7 +323,7 @@ func outputRepetitiveNotice(
 		if state.repeats > 0 {
 			args = append(args, "repeats", state.repeats)
 		}
-		outputNotice(noticeType, showUser, args...)
+		outputNotice(noticeType, isDiagnostic, showUser, args...)
 	}
 }