Merge pull request #503 from rod-hynes/master

New tunnel metrics

.travis.yml

@@ -1,7 +1,7 @@
 language: go
 sudo: required
 go:
-- 1.11.1
+- 1.11.4
 addons:
   apt_packages:
     - libx11-dev

ClientLibrary/Dockerfile

@@ -21,7 +21,7 @@ RUN apt-get update -y && apt-get install -y --no-install-recommends \
 
 # Install Go.
 # NOTE: Go 1.10+ is required to build c-shared for windows (https://github.com/golang/go/commit/bb0bfd002ada7e3eb9198d4287b32c2fed6e8da6)
-ENV GOVERSION=go1.11.1 GOROOT=/usr/local/go GOPATH=/go PATH=$PATH:/usr/local/go/bin:/go/bin CGO_ENABLED=1
+ENV GOVERSION=go1.11.4 GOROOT=/usr/local/go GOPATH=/go PATH=$PATH:/usr/local/go/bin:/go/bin CGO_ENABLED=1
 
 RUN curl -L https://storage.googleapis.com/golang/$GOVERSION.linux-amd64.tar.gz -o /tmp/go.tar.gz \
    && tar -C /usr/local -xzf /tmp/go.tar.gz \

ClientLibrary/build-darwin.sh

@@ -6,7 +6,7 @@ set -e -u -x
 if [ -z ${2+x} ]; then BUILD_TAGS=""; else BUILD_TAGS="$2"; fi
 
 # Modify this value as we use newer Go versions.
-GO_VERSION_REQUIRED="1.11.1"
+GO_VERSION_REQUIRED="1.11.4"
 
 BASE_DIR=$(cd "$(dirname "$0")" ; pwd -P)
 cd ${BASE_DIR}

ClientLibrary/clientlib/clientlib.go

@@ -122,19 +122,12 @@ func StartTunnel(ctx context.Context,
 		config.DataStoreDirectory = *params.DataRootDirectory
 		config.ObfuscatedServerListDownloadDirectory = *params.DataRootDirectory
 
-		if *params.DataRootDirectory == "" {
-			config.RemoteServerListDownloadFilename = ""
-		} else {
-			config.RemoteServerListDownloadFilename = filepath.Join(*params.DataRootDirectory, "server_list_compressed")
-		}
+		config.RemoteServerListDownloadFilename = filepath.Join(*params.DataRootDirectory, "server_list_compressed")
 	}
 
 	if params.NetworkID != nil {
 		config.NetworkID = *params.NetworkID
 	}
-	if config.NetworkID == "" {
-		return nil, common.ContextError(fmt.Errorf("networkID must be non-empty"))
-	}
 
 	if params.ClientPlatform != nil {
 		config.ClientPlatform = *params.ClientPlatform

ConsoleClient/Dockerfile

@@ -22,7 +22,7 @@ RUN apt-get update -y && apt-get install -y --no-install-recommends \
   && rm -rf /var/lib/apt/lists/*
 
 # Install Go.
-ENV GOVERSION=go1.11.1 GOROOT=/usr/local/go GOPATH=/go PATH=$PATH:/usr/local/go/bin:/go/bin CGO_ENABLED=1
+ENV GOVERSION=go1.11.4 GOROOT=/usr/local/go GOPATH=/go PATH=$PATH:/usr/local/go/bin:/go/bin CGO_ENABLED=1
 
 RUN curl -L https://storage.googleapis.com/golang/$GOVERSION.linux-amd64.tar.gz -o /tmp/go.tar.gz \
    && tar -C /usr/local -xzf /tmp/go.tar.gz \

MobileLibrary/Android/Dockerfile

@@ -19,7 +19,7 @@ RUN apt-get update -y && apt-get install -y --no-install-recommends \
   && rm -rf /var/lib/apt/lists/*
 
 # Install Go.
-ENV GOVERSION=go1.9.6 GOROOT=/usr/local/go GOPATH=/go PATH=$PATH:/usr/local/go/bin:/go/bin CGO_ENABLED=1
+ENV GOVERSION=go1.11.4 GOROOT=/usr/local/go GOPATH=/go PATH=$PATH:/usr/local/go/bin:/go/bin CGO_ENABLED=1
 
 RUN curl -L https://storage.googleapis.com/golang/$GOVERSION.linux-amd64.tar.gz -o /tmp/go.tar.gz \
   && tar -C /usr/local -xzf /tmp/go.tar.gz \

MobileLibrary/iOS/build-psiphon-framework.sh

@@ -5,7 +5,7 @@ set -e -u -x
 if [ -z ${1+x} ]; then BUILD_TAGS=""; else BUILD_TAGS="$1"; fi
 
 # Modify this value as we use newer Go versions.
-GO_VERSION_REQUIRED="1.11.1"
+GO_VERSION_REQUIRED="1.11.4"
 
 # Reset the PATH to macOS default. This is mainly so we don't execute the wrong
 # gomobile executable.

Server/Dockerfile-binary-builder

@@ -1,6 +1,6 @@
 FROM alpine:3.4
 
-ENV GOLANG_VERSION 1.11.1
+ENV GOLANG_VERSION 1.11.4
 ENV GOLANG_SRC_URL https://golang.org/dl/go$GOLANG_VERSION.src.tar.gz
 
 RUN set -ex \

psiphon/common/parameters/clientParameters.go

@@ -202,6 +202,10 @@ const (
 	APIRequestUpstreamPaddingMaxBytes          = "APIRequestUpstreamPaddingMaxBytes"
 	APIRequestDownstreamPaddingMinBytes        = "APIRequestDownstreamPaddingMinBytes"
 	APIRequestDownstreamPaddingMaxBytes        = "APIRequestDownstreamPaddingMaxBytes"
+	PersistentStatsMaxStoreRecords             = "PersistentStatsMaxStoreRecords"
+	PersistentStatsMaxSendBytes                = "PersistentStatsMaxSendBytes"
+	RecordRemoteServerListPersistentStats      = "RecordRemoteServerListPersistentStats"
+	RecordFailedTunnelPersistentStats          = "RecordFailedTunnelPersistentStats"
 )
 
 const (
@@ -334,7 +338,9 @@ var defaultClientParameters = map[string]struct {
 	PsiphonAPIStatusRequestPeriodMax:      {value: 10 * time.Minute, minimum: 1 * time.Second},
 	PsiphonAPIStatusRequestShortPeriodMin: {value: 5 * time.Second, minimum: 1 * time.Second},
 	PsiphonAPIStatusRequestShortPeriodMax: {value: 10 * time.Second, minimum: 1 * time.Second},
-	PsiphonAPIPersistentStatsMaxCount:     {value: 100, minimum: 1},
+	// PsiphonAPIPersistentStatsMaxCount parameter is obsoleted by PersistentStatsMaxSendBytes.
+	// TODO: remove once no longer required for older clients.
+	PsiphonAPIPersistentStatsMaxCount: {value: 100, minimum: 1},
 	// PsiphonAPIStatusRequestPadding parameters are obsoleted by APIRequestUp/DownstreamPadding.
 	// TODO: remove once no longer required for older clients.
 	PsiphonAPIStatusRequestPaddingMinBytes: {value: 0, minimum: 0},
@@ -409,6 +415,11 @@ var defaultClientParameters = map[string]struct {
 	APIRequestUpstreamPaddingMaxBytes:   {value: 1024, minimum: 0},
 	APIRequestDownstreamPaddingMinBytes: {value: 0, minimum: 0},
 	APIRequestDownstreamPaddingMaxBytes: {value: 1024, minimum: 0},
+
+	PersistentStatsMaxStoreRecords:        {value: 200, minimum: 1},
+	PersistentStatsMaxSendBytes:           {value: 65536, minimum: 1},
+	RecordRemoteServerListPersistentStats: {value: true},
+	RecordFailedTunnelPersistentStats:     {value: true},
 }
 
 // IsServerSideOnly indicates if the parameter specified by name is used

psiphon/common/protocol/protocol.go

@@ -215,6 +215,10 @@ func TLSProfileIsRandomized(tlsProfile string) bool {
 		tlsProfile == TLS_PROFILE_TLS13_RANDOMIZED
 }
 
+func TLSProfileIsTLS13(tlsProfile string) bool {
+	return tlsProfile == TLS_PROFILE_TLS13_RANDOMIZED
+}
+
 type TLSProfiles []string
 
 func (profiles TLSProfiles) Validate() error {

psiphon/controller.go

@@ -46,7 +46,6 @@ import (
 // route traffic through the tunnels.
 type Controller struct {
 	config                                  *Config
-	sessionId                               string
 	runCtx                                  context.Context
 	stopRunning                             context.CancelFunc
 	runWaitGroup                            *sync.WaitGroup
@@ -78,6 +77,7 @@ type Controller struct {
 	serverAffinityDoneBroadcast             chan struct{}
 	packetTunnelClient                      *tun.Client
 	packetTunnelTransport                   *PacketTunnelTransport
+	staggerMutex                            sync.Mutex
 }
 
 // NewController initializes a new controller.
@@ -105,7 +105,6 @@ func NewController(config *Config) (controller *Controller, err error) {
 
 	controller = &Controller{
 		config:       config,
-		sessionId:    config.SessionID,
 		runWaitGroup: new(sync.WaitGroup),
 		// connectedTunnels and failedTunnels buffer sizes are large enough to
 		// receive full pools of tunnels without blocking. Senders should not block.
@@ -1469,7 +1468,8 @@ func (controller *Controller) doFetchTactics(
 		canReplay,
 		selectProtocol,
 		serverEntry,
-		true)
+		true,
+		0)
 	if dialParams == nil {
 		// MakeDialParameters may return nil, nil when the server entry can't
 		// satisfy protocol selection criteria. This case in not expected
@@ -1518,10 +1518,7 @@ func (controller *Controller) doFetchTactics(
 	}
 	defer meekConn.Close()
 
-	apiParams := getBaseAPIParameters(
-		controller.config,
-		controller.sessionId,
-		dialParams)
+	apiParams := getBaseAPIParameters(controller.config, dialParams)
 
 	tacticsRecord, err := tactics.FetchTactics(
 		ctx,
@@ -1694,32 +1691,6 @@ loop:
 					}
 					timer.Stop()
 				}
-
-			} else {
-
-				// TODO: if candidates are to be skipped, this should be done
-				// _before_ the stagger.
-
-				p := controller.config.clientParameters.Get()
-				staggerPeriod := p.Duration(parameters.StaggerConnectionWorkersPeriod)
-				staggerJitter := p.Float(parameters.StaggerConnectionWorkersJitter)
-				p = nil
-
-				if staggerPeriod != 0 {
-
-					// Stagger concurrent connection workers.
-
-					timeout := prng.JitterDuration(staggerPeriod, staggerJitter)
-
-					timer := time.NewTimer(timeout)
-					select {
-					case <-timer.C:
-					case <-controller.establishCtx.Done():
-						timer.Stop()
-						break loop
-					}
-					timer.Stop()
-				}
 			}
 		}
 
@@ -1838,7 +1809,8 @@ loop:
 			canReplay,
 			selectProtocol,
 			candidateServerEntry.serverEntry,
-			false)
+			false,
+			controller.establishConnectTunnelCount)
 		if dialParams == nil || err != nil {
 
 			controller.concurrentEstablishTunnelsMutex.Unlock()
@@ -1864,6 +1836,7 @@ loop:
 		// Increment establishConnectTunnelCount only after selectProtocol has
 		// succeeded to ensure InitialLimitTunnelProtocolsCandidateCount
 		// candidates use InitialLimitTunnelProtocols.
+		establishConnectTunnelCount := controller.establishConnectTunnelCount
 		controller.establishConnectTunnelCount += 1
 
 		isIntensive := protocol.TunnelProtocolIsResourceIntensive(dialParams.TunnelProtocol)
@@ -1881,6 +1854,33 @@ loop:
 
 		controller.concurrentEstablishTunnelsMutex.Unlock()
 
+		// Apply stagger only now that we're past MakeDialParameters and
+		// protocol selection logic which may have caused the candidate to be
+		// skipped. The stagger logic delays dialing, and we don't want to
+		// incur that delay that when skipping.
+		//
+		// Locking staggerMutex serializes staggers, so that multiple workers
+		// don't simply sleep in parallel.
+		//
+		// The stagger is applied when establishConnectTunnelCount > 0 -- that
+		// is, for all but the first dial.
+
+		p := controller.config.clientParameters.Get()
+		staggerPeriod := p.Duration(parameters.StaggerConnectionWorkersPeriod)
+		staggerJitter := p.Float(parameters.StaggerConnectionWorkersJitter)
+		p = nil
+
+		if establishConnectTunnelCount > 0 && staggerPeriod != 0 {
+			controller.staggerMutex.Lock()
+			timer := time.NewTimer(prng.JitterDuration(staggerPeriod, staggerJitter))
+			select {
+			case <-timer.C:
+			case <-controller.establishCtx.Done():
+			}
+			timer.Stop()
+			controller.staggerMutex.Unlock()
+		}
+
 		// ConnectTunnel will allocate significant memory, so first attempt to
 		// reclaim as much as possible.
 		DoGarbageCollection()
@@ -1888,7 +1888,6 @@ loop:
 		tunnel, err := ConnectTunnel(
 			controller.establishCtx,
 			controller.config,
-			controller.sessionId,
 			candidateServerEntry.adjustedEstablishStartTime,
 			dialParams)
 

psiphon/dataStore.go

@@ -39,6 +39,7 @@ var (
 	datastoreUrlETagsBucket                     = []byte("urlETags")
 	datastoreKeyValueBucket                     = []byte("keyValues")
 	datastoreRemoteServerListStatsBucket        = []byte("remoteServerListStats")
+	datastoreFailedTunnelStatsBucket            = []byte("failedTunnelStats")
 	datastoreSLOKsBucket                        = []byte("SLOKs")
 	datastoreTacticsBucket                      = []byte("tactics")
 	datastoreSpeedTestSamplesBucket             = []byte("speedTestSamples")
@@ -47,6 +48,7 @@ var (
 	datastoreLastServerEntryFilterKey           = []byte("lastServerEntryFilter")
 	datastoreAffinityServerEntryIDKey           = []byte("affinityServerEntryID")
 	datastorePersistentStatTypeRemoteServerList = string(datastoreRemoteServerListStatsBucket)
+	datastorePersistentStatTypeFailedTunnel     = string(datastoreFailedTunnelStatsBucket)
 	datastoreServerEntryFetchGCThreshold        = 20
 
 	datastoreMutex    sync.RWMutex
@@ -961,6 +963,7 @@ var persistentStatStateReporting = []byte("1")
 
 var persistentStatTypes = []string{
 	datastorePersistentStatTypeRemoteServerList,
+	datastorePersistentStatTypeFailedTunnel,
 }
 
 // StorePersistentStat adds a new persistent stat record, which
@@ -970,17 +973,36 @@ var persistentStatTypes = []string{
 // The stat is a JSON byte array containing fields as
 // required by the Psiphon server API. It's assumed that the
 // JSON value contains enough unique information for the value to
-// function as a key in the key/value datastore. This assumption
-// is currently satisfied by the fields sessionId + tunnelNumber
-// for tunnel stats, and URL + ETag for remote server list stats.
-func StorePersistentStat(statType string, stat []byte) error {
+// function as a key in the key/value datastore.
+//
+// Only up to PersistentStatsMaxStoreRecords are stored. Once this
+// limit is reached, new records are discarded.
+func StorePersistentStat(config *Config, statType string, stat []byte) error {
 
 	if !common.Contains(persistentStatTypes, statType) {
 		return common.ContextError(fmt.Errorf("invalid persistent stat type: %s", statType))
 	}
 
+	maxStoreRecords := config.GetClientParameters().Int(parameters.PersistentStatsMaxStoreRecords)
+
 	err := datastoreUpdate(func(tx *datastoreTx) error {
 		bucket := tx.bucket([]byte(statType))
+
+		count := 0
+		cursor := bucket.cursor()
+		for key, _ := cursor.first(); key != nil; key, _ = cursor.next() {
+			count++
+		}
+		cursor.close()
+
+		// TODO: assuming newer metrics are more useful, replace oldest record
+		// instead of discarding?
+
+		if count >= maxStoreRecords {
+			// Silently discard.
+			return nil
+		}
+
 		err := bucket.put(stat, persistentStatStateUnreported)
 		return err
 	})
@@ -1007,7 +1029,6 @@ func CountUnreportedPersistentStats() int {
 			for key, value := cursor.first(); key != nil; key, value = cursor.next() {
 				if 0 == bytes.Compare(value, persistentStatStateUnreported) {
 					unreported++
-					break
 				}
 			}
 			cursor.close()
@@ -1023,18 +1044,21 @@ func CountUnreportedPersistentStats() int {
 	return unreported
 }
 
-// TakeOutUnreportedPersistentStats returns up to maxCount persistent
-// stats records that are in StateUnreported. The records are set to
-// StateReporting. If the records are successfully reported, clear them
+// TakeOutUnreportedPersistentStats returns persistent stats records that are
+// in StateUnreported. At least one record, if present, will be returned and
+// then additional records up to PersistentStatsMaxSendBytes. The records are
+// set to StateReporting. If the records are successfully reported, clear them
 // with ClearReportedPersistentStats. If the records are not successfully
 // reported, restore them with PutBackUnreportedPersistentStats.
-func TakeOutUnreportedPersistentStats(maxCount int) (map[string][][]byte, error) {
+func TakeOutUnreportedPersistentStats(config *Config) (map[string][][]byte, error) {
 
 	stats := make(map[string][][]byte)
 
+	maxSendBytes := config.GetClientParameters().Int(parameters.PersistentStatsMaxSendBytes)
+
 	err := datastoreUpdate(func(tx *datastoreTx) error {
 
-		count := 0
+		sendBytes := 0
 
 		for _, statType := range persistentStatTypes {
 
@@ -1042,18 +1066,15 @@ func TakeOutUnreportedPersistentStats(maxCount int) (map[string][][]byte, error)
 			cursor := bucket.cursor()
 			for key, value := cursor.first(); key != nil; key, value = cursor.next() {
 
-				if count >= maxCount {
-					break
-				}
-
 				// Perform a test JSON unmarshaling. In case of data corruption or a bug,
-				// skip the record.
+				// delete and skip the record.
 				var jsonData interface{}
 				err := json.Unmarshal(key, &jsonData)
 				if err != nil {
 					NoticeAlert(
 						"Invalid key in TakeOutUnreportedPersistentStats: %s: %s",
 						string(key), err)
+					bucket.delete(key)
 					continue
 				}
 
@@ -1067,8 +1088,13 @@ func TakeOutUnreportedPersistentStats(maxCount int) (map[string][][]byte, error)
 					}
 
 					stats[statType] = append(stats[statType], data)
-					count += 1
+
+					sendBytes += len(data)
+					if sendBytes >= maxSendBytes {
+						break
+					}
 				}
+
 			}
 			cursor.close()
 

psiphon/dataStore_bolt.go

@@ -97,6 +97,7 @@ func datastoreOpenDB(rootDataDirectory string) (*datastoreDB, error) {
 			datastoreUrlETagsBucket,
 			datastoreKeyValueBucket,
 			datastoreRemoteServerListStatsBucket,
+			datastoreFailedTunnelStatsBucket,
 			datastoreSLOKsBucket,
 			datastoreTacticsBucket,
 			datastoreSpeedTestSamplesBucket,

psiphon/dialParameters.go

@@ -54,9 +54,10 @@ import (
 //
 // DialParameters is not safe for concurrent use.
 type DialParameters struct {
-	ServerEntry *protocol.ServerEntry `json:"-"`
-	NetworkID   string                `json:"-"`
-	IsReplay    bool                  `json:"-"`
+	ServerEntry     *protocol.ServerEntry `json:"-"`
+	NetworkID       string                `json:"-"`
+	IsReplay        bool                  `json:"-"`
+	CandidateNumber int                   `json:"-"`
 
 	LastUsedTimestamp       time.Time
 	LastUsedConfigStateHash []byte
@@ -103,6 +104,8 @@ type DialParameters struct {
 	DialConnMetrics          common.MetricsSource `json:"-"`
 	ObfuscatedSSHConnMetrics common.MetricsSource `json:"-"`
 
+	DialDuration time.Duration `json:"-"`
+
 	dialConfig *DialConfig `json:"-"`
 	meekConfig *MeekConfig `json:"-"`
 }
@@ -129,7 +132,8 @@ func MakeDialParameters(
 	canReplay func(serverEntry *protocol.ServerEntry, replayProtocol string) bool,
 	selectProtocol func(serverEntry *protocol.ServerEntry) (string, bool),
 	serverEntry *protocol.ServerEntry,
-	isTactics bool) (*DialParameters, error) {
+	isTactics bool,
+	candidateNumber int) (*DialParameters, error) {
 
 	networkID := config.GetNetworkID()
 
@@ -215,6 +219,7 @@ func MakeDialParameters(
 	dialParams.ServerEntry = serverEntry
 	dialParams.NetworkID = networkID
 	dialParams.IsReplay = isReplay
+	dialParams.CandidateNumber = candidateNumber
 
 	// Even when replaying, LastUsedTimestamp is updated to extend the TTL of
 	// replayed dial parameters which will be updated in the datastore upon

psiphon/dialParameters_test.go

@@ -111,7 +111,7 @@ func runDialParametersAndReplay(t *testing.T, tunnelProtocol string) {
 
 	// Test: expected dial parameter fields set
 
-	dialParams, err := MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntries[0], false)
+	dialParams, err := MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntries[0], false, 0)
 	if err != nil {
 		t.Fatalf("MakeDialParameters failed: %s", err)
 	}
@@ -196,7 +196,7 @@ func runDialParametersAndReplay(t *testing.T, tunnelProtocol string) {
 
 	dialParams.Failed()
 
-	dialParams, err = MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntries[0], false)
+	dialParams, err = MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntries[0], false, 0)
 	if err != nil {
 		t.Fatalf("MakeDialParameters failed: %s", err)
 	}
@@ -211,7 +211,7 @@ func runDialParametersAndReplay(t *testing.T, tunnelProtocol string) {
 
 	testNetworkID = prng.HexString(8)
 
-	dialParams, err = MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntries[0], false)
+	dialParams, err = MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntries[0], false, 0)
 	if err != nil {
 		t.Fatalf("MakeDialParameters failed: %s", err)
 	}
@@ -228,7 +228,7 @@ func runDialParametersAndReplay(t *testing.T, tunnelProtocol string) {
 
 	dialParams.Succeeded()
 
-	replayDialParams, err := MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntries[0], false)
+	replayDialParams, err := MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntries[0], false, 0)
 	if err != nil {
 		t.Fatalf("MakeDialParameters failed: %s", err)
 	}
@@ -314,7 +314,7 @@ func runDialParametersAndReplay(t *testing.T, tunnelProtocol string) {
 		t.Fatalf("SetClientParameters failed: %s", err)
 	}
 
-	dialParams, err = MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntries[0], false)
+	dialParams, err = MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntries[0], false, 0)
 	if err != nil {
 		t.Fatalf("MakeDialParameters failed: %s", err)
 	}
@@ -329,7 +329,7 @@ func runDialParametersAndReplay(t *testing.T, tunnelProtocol string) {
 
 	time.Sleep(1 * time.Second)
 
-	dialParams, err = MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntries[0], false)
+	dialParams, err = MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntries[0], false, 0)
 	if err != nil {
 		t.Fatalf("MakeDialParameters failed: %s", err)
 	}
@@ -344,7 +344,7 @@ func runDialParametersAndReplay(t *testing.T, tunnelProtocol string) {
 
 	serverEntries[0].ConfigurationVersion += 1
 
-	dialParams, err = MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntries[0], false)
+	dialParams, err = MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntries[0], false, 0)
 	if err != nil {
 		t.Fatalf("MakeDialParameters failed: %s", err)
 	}
@@ -368,14 +368,14 @@ func runDialParametersAndReplay(t *testing.T, tunnelProtocol string) {
 		t.Fatalf("SetClientParameters failed: %s", err)
 	}
 
-	dialParams, err = MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntries[0], false)
+	dialParams, err = MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntries[0], false, 0)
 	if err != nil {
 		t.Fatalf("MakeDialParameters failed: %s", err)
 	}
 
 	dialParams.Succeeded()
 
-	replayDialParams, err = MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntries[0], false)
+	replayDialParams, err = MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntries[0], false, 0)
 	if err != nil {
 		t.Fatalf("MakeDialParameters failed: %s", err)
 	}
@@ -423,7 +423,7 @@ func runDialParametersAndReplay(t *testing.T, tunnelProtocol string) {
 
 		if i%10 == 0 {
 
-			dialParams, err := MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntry, false)
+			dialParams, err := MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntry, false, 0)
 			if err != nil {
 				t.Fatalf("MakeDialParameters failed: %s", err)
 			}
@@ -452,7 +452,7 @@ func runDialParametersAndReplay(t *testing.T, tunnelProtocol string) {
 				t.Fatalf("ServerEntryIterator.Next failed: %s", err)
 			}
 
-			dialParams, err := MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntry, false)
+			dialParams, err := MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntry, false, 0)
 			if err != nil {
 				t.Fatalf("MakeDialParameters failed: %s", err)
 			}
@@ -474,7 +474,7 @@ func runDialParametersAndReplay(t *testing.T, tunnelProtocol string) {
 				t.Fatalf("ServerEntryIterator.Next failed: %s", err)
 			}
 
-			dialParams, err := MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntry, false)
+			dialParams, err := MakeDialParameters(clientConfig, canReplay, selectProtocol, serverEntry, false, 0)
 			if err != nil {
 				t.Fatalf("MakeDialParameters failed: %s", err)
 			}

psiphon/remoteServerList.go

@@ -418,7 +418,7 @@ func downloadRemoteServerListFile(
 
 	NoticeRemoteServerListResourceDownloaded(sourceURL)
 
-	RecordRemoteServerListStat(sourceURL, responseETag)
+	_ = RecordRemoteServerListStat(config, sourceURL, responseETag)
 
 	return responseETag, nil
 }

psiphon/server/api.go

@@ -251,7 +251,7 @@ func handshakeAPIRequestHandler(
 		}
 
 		// Log a metric when new tactics are issued. Logging here indicates that
-		// the handshake tactics mechansim is active; but logging for every
+		// the handshake tactics mechanism is active; but logging for every
 		// handshake creates unneccesary log data.
 
 		if len(tacticsPayload.Tactics) > 0 {
@@ -317,6 +317,19 @@ var connectedRequestParams = append(
 		{"establishment_duration", isIntString, requestParamOptional | requestParamLogStringAsInt}},
 	baseRequestParams...)
 
+// updateOnConnectedParamNames are connected request parameters which are
+// copied to update data logged with server_tunnel: these fields either only
+// ship with or ship newer data with connected requests.
+var updateOnConnectedParamNames = []string{
+	"last_connected",
+	"establishment_duration",
+	"upstream_bytes_fragmented",
+	"upstream_min_bytes_written",
+	"upstream_max_bytes_written",
+	"upstream_min_delayed",
+	"upstream_max_delayed",
+}
+
 // connectedAPIRequestHandler implements the "connected" API request.
 // Clients make the connected request once a tunnel connection has been
 // established and at least once per day. The last_connected input value,
@@ -334,15 +347,17 @@ func connectedAPIRequestHandler(
 		return nil, common.ContextError(err)
 	}
 
-	// Update upstream fragmentor metrics, as the client may have performed
-	// more upstream fragmentation since the previous metrics reported by the
-	// handshake request.
+	// Update, for server_tunnel logging, upstream fragmentor metrics, as the
+	// client may have performed more upstream fragmentation since the
+	// previous metrics reported by the handshake request. Also, additional
+	// fields reported only in the connected request, are added to
+	// server_tunnel here.
 
 	// TODO: same session-ID-lookup TODO in handshakeAPIRequestHandler
 	// applies here.
 	sessionID, _ := getStringRequestParam(params, "client_session_id")
 	err = support.TunnelServer.UpdateClientAPIParameters(
-		sessionID, copyUpstreamFragmentorParams(params))
+		sessionID, copyUpdateOnConnectedParams(params))
 	if err != nil {
 		return nil, common.ContextError(err)
 	}
@@ -376,6 +391,27 @@ var statusRequestParams = append(
 		{"connected", isBooleanFlag, requestParamLogFlagAsBool}},
 	baseRequestParams...)
 
+var remoteServerListStatParams = []requestParamSpec{
+	{"session_id", isHexDigits, requestParamOptional},
+	{"propagation_channel_id", isHexDigits, requestParamOptional},
+	{"sponsor_id", isHexDigits, requestParamOptional},
+	{"client_version", isAnyString, requestParamOptional},
+	{"client_platform", isAnyString, requestParamOptional},
+	{"client_build_rev", isAnyString, requestParamOptional},
+	{"client_download_timestamp", isISO8601Date, 0},
+	{"url", isAnyString, 0},
+	{"etag", isAnyString, 0},
+}
+
+var failedTunnelStatParams = append(
+	[]requestParamSpec{
+		{"server_entry_ip_address", isIPAddress, requestParamNotLogged},
+		{"session_id", isHexDigits, 0},
+		{"last_connected", isLastConnected, 0},
+		{"client_failed_timestamp", isISO8601Date, 0},
+		{"tunnel_error", isAnyString, 0}},
+	baseRequestParams...)
+
 // statusAPIRequestHandler implements the "status" API request.
 // Clients make periodic status requests which deliver client-side
 // recorded data transfer and tunnel duration stats.
@@ -440,8 +476,11 @@ func statusAPIRequestHandler(
 		}
 	}
 
-	// Remote server list download stats
-	// Older clients may not submit this data
+	// Limitation: for "persistent" stats, host_id and geolocation is time-of-sending
+	// not time-of-recording.
+
+	// Remote server list download persistent stats.
+	// Older clients may not submit this data.
 
 	if statusData["remote_server_list_stats"] != nil {
 
@@ -451,6 +490,13 @@ func statusAPIRequestHandler(
 		}
 		for _, remoteServerListStat := range remoteServerListStats {
 
+			err := validateRequestParams(support.Config, remoteServerListStat, remoteServerListStatParams)
+			if err != nil {
+				return nil, common.ContextError(err)
+			}
+
+			// remote_server_list defaults to using the common params from the
+			// outer statusRequestParams
 			remoteServerListFields := getRequestLogFields(
 				"remote_server_list",
 				geoIPData,
@@ -458,25 +504,43 @@ func statusAPIRequestHandler(
 				params,
 				statusRequestParams)
 
-			clientDownloadTimestamp, err := getStringRequestParam(remoteServerListStat, "client_download_timestamp")
-			if err != nil {
-				return nil, common.ContextError(err)
+			for name, value := range remoteServerListStat {
+				remoteServerListFields[name] = value
 			}
-			remoteServerListFields["client_download_timestamp"] = clientDownloadTimestamp
 
-			url, err := getStringRequestParam(remoteServerListStat, "url")
-			if err != nil {
-				return nil, common.ContextError(err)
-			}
-			remoteServerListFields["url"] = url
+			logQueue = append(logQueue, remoteServerListFields)
+		}
+	}
+
+	// Failed tunnel persistent stats.
+	// Older clients may not submit this data.
 
-			etag, err := getStringRequestParam(remoteServerListStat, "etag")
+	if statusData["failed_tunnel_stats"] != nil {
+
+		failedTunnelStats, err := getJSONObjectArrayRequestParam(statusData, "failed_tunnel_stats")
+		if err != nil {
+			return nil, common.ContextError(err)
+		}
+		for _, failedTunnelStat := range failedTunnelStats {
+
+			// failed_tunnel supplies a full set of common params, but the
+			// server secret must use the corect value from the outer
+			// statusRequestParams
+			failedTunnelStat["server_secret"] = params["server_secret"]
+
+			err := validateRequestParams(support.Config, failedTunnelStat, failedTunnelStatParams)
 			if err != nil {
 				return nil, common.ContextError(err)
 			}
-			remoteServerListFields["etag"] = etag
 
-			logQueue = append(logQueue, remoteServerListFields)
+			failedTunnelFields := getRequestLogFields(
+				"failed_tunnel",
+				geoIPData,
+				authorizedAccessTypes,
+				failedTunnelStat,
+				failedTunnelStatParams)
+
+			logQueue = append(logQueue, failedTunnelFields)
 		}
 	}
 
@@ -538,63 +602,63 @@ type requestParamSpec struct {
 }
 
 const (
-	requestParamOptional             = 1
-	requestParamNotLogged            = 2
-	requestParamArray                = 4
-	requestParamJSON                 = 8
-	requestParamLogStringAsInt       = 16
-	requestParamLogStringLengthAsInt = 32
-	requestParamLogFlagAsBool        = 64
+	requestParamOptional                                      = 1
+	requestParamNotLogged                                     = 1 << 1
+	requestParamArray                                         = 1 << 2
+	requestParamJSON                                          = 1 << 3
+	requestParamLogStringAsInt                                = 1 << 4
+	requestParamLogStringLengthAsInt                          = 1 << 5
+	requestParamLogFlagAsBool                                 = 1 << 6
+	requestParamLogOnlyForFrontedMeek                         = 1 << 7
+	requestParamNotLoggedForUnfrontedMeekNonTransformedHeader = 1 << 8
 )
 
-var upstreamFragmentorParams = []requestParamSpec{
+// baseRequestParams is the list of required and optional
+// request parameters; derived from COMMON_INPUTS and
+// OPTIONAL_COMMON_INPUTS in psi_web.
+// Each param is expected to be a string, unless requestParamArray
+// is specified, in which case an array of string is expected.
+var baseRequestParams = []requestParamSpec{
+	{"server_secret", isServerSecret, requestParamNotLogged},
+	{"client_session_id", isHexDigits, requestParamNotLogged},
+	{"propagation_channel_id", isHexDigits, 0},
+	{"sponsor_id", isHexDigits, 0},
+	{"client_version", isIntString, requestParamLogStringAsInt},
+	{"client_platform", isClientPlatform, 0},
+	{"client_build_rev", isHexDigits, requestParamOptional},
+	{"relay_protocol", isRelayProtocol, 0},
+	{"tunnel_whole_device", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
+	{"device_region", isAnyString, requestParamOptional},
+	{"ssh_client_version", isAnyString, requestParamOptional},
+	{"upstream_proxy_type", isUpstreamProxyType, requestParamOptional},
+	{"upstream_proxy_custom_header_names", isAnyString, requestParamOptional | requestParamArray},
+	{"meek_dial_address", isDialAddress, requestParamOptional | requestParamLogOnlyForFrontedMeek},
+	{"meek_resolved_ip_address", isIPAddress, requestParamOptional | requestParamLogOnlyForFrontedMeek},
+	{"meek_sni_server_name", isDomain, requestParamOptional},
+	{"meek_host_header", isHostHeader, requestParamOptional | requestParamNotLoggedForUnfrontedMeekNonTransformedHeader},
+	{"meek_transformed_host_name", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
+	{"user_agent", isAnyString, requestParamOptional},
+	{"tls_profile", isAnyString, requestParamOptional},
+	{"server_entry_region", isRegionCode, requestParamOptional},
+	{"server_entry_source", isServerEntrySource, requestParamOptional},
+	{"server_entry_timestamp", isISO8601Date, requestParamOptional},
+	{tactics.APPLIED_TACTICS_TAG_PARAMETER_NAME, isAnyString, requestParamOptional},
+	{"dial_port_number", isIntString, requestParamOptional | requestParamLogStringAsInt},
+	{"quic_version", isAnyString, requestParamOptional},
+	{"quic_dial_sni_address", isAnyString, requestParamOptional},
 	{"upstream_bytes_fragmented", isIntString, requestParamOptional | requestParamLogStringAsInt},
 	{"upstream_min_bytes_written", isIntString, requestParamOptional | requestParamLogStringAsInt},
 	{"upstream_max_bytes_written", isIntString, requestParamOptional | requestParamLogStringAsInt},
 	{"upstream_min_delayed", isIntString, requestParamOptional | requestParamLogStringAsInt},
 	{"upstream_max_delayed", isIntString, requestParamOptional | requestParamLogStringAsInt},
+	{"padding", isAnyString, requestParamOptional | requestParamLogStringLengthAsInt},
+	{"pad_response", isIntString, requestParamOptional | requestParamLogStringAsInt},
+	{"is_replay", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
+	{"egress_region", isRegionCode, requestParamOptional},
+	{"dial_duration", isIntString, requestParamOptional | requestParamLogStringAsInt},
+	{"candidate_number", isIntString, requestParamOptional | requestParamLogStringAsInt},
 }
 
-// baseRequestParams is the list of required and optional
-// request parameters; derived from COMMON_INPUTS and
-// OPTIONAL_COMMON_INPUTS in psi_web.
-// Each param is expected to be a string, unless requestParamArray
-// is specified, in which case an array of string is expected.
-var baseRequestParams = append(
-	[]requestParamSpec{
-		{"server_secret", isServerSecret, requestParamNotLogged},
-		{"client_session_id", isHexDigits, requestParamNotLogged},
-		{"propagation_channel_id", isHexDigits, 0},
-		{"sponsor_id", isHexDigits, 0},
-		{"client_version", isIntString, requestParamLogStringAsInt},
-		{"client_platform", isClientPlatform, 0},
-		{"client_build_rev", isHexDigits, requestParamOptional},
-		{"relay_protocol", isRelayProtocol, 0},
-		{"tunnel_whole_device", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
-		{"device_region", isAnyString, requestParamOptional},
-		{"ssh_client_version", isAnyString, requestParamOptional},
-		{"upstream_proxy_type", isUpstreamProxyType, requestParamOptional},
-		{"upstream_proxy_custom_header_names", isAnyString, requestParamOptional | requestParamArray},
-		{"meek_dial_address", isDialAddress, requestParamOptional},
-		{"meek_resolved_ip_address", isIPAddress, requestParamOptional},
-		{"meek_sni_server_name", isDomain, requestParamOptional},
-		{"meek_host_header", isHostHeader, requestParamOptional},
-		{"meek_transformed_host_name", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
-		{"user_agent", isAnyString, requestParamOptional},
-		{"tls_profile", isAnyString, requestParamOptional},
-		{"server_entry_region", isRegionCode, requestParamOptional},
-		{"server_entry_source", isServerEntrySource, requestParamOptional},
-		{"server_entry_timestamp", isISO8601Date, requestParamOptional},
-		{tactics.APPLIED_TACTICS_TAG_PARAMETER_NAME, isAnyString, requestParamOptional},
-		{"dial_port_number", isIntString, requestParamOptional | requestParamLogStringAsInt},
-		{"quic_version", isAnyString, requestParamOptional},
-		{"quic_dial_sni_address", isAnyString, requestParamOptional},
-		{"padding", isAnyString, requestParamOptional | requestParamLogStringLengthAsInt},
-		{"pad_response", isIntString, requestParamOptional | requestParamLogStringAsInt},
-		{"is_replay", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
-	},
-	upstreamFragmentorParams...)
-
 func validateRequestParams(
 	config *Config,
 	params common.APIParameters,
@@ -650,16 +714,16 @@ func copyBaseRequestParams(params common.APIParameters) common.APIParameters {
 	return paramsCopy
 }
 
-func copyUpstreamFragmentorParams(params common.APIParameters) common.APIParameters {
+func copyUpdateOnConnectedParams(params common.APIParameters) common.APIParameters {
 
 	// Note: not a deep copy
 	paramsCopy := make(common.APIParameters)
-	for _, baseParam := range upstreamFragmentorParams {
-		value := params[baseParam.name]
+	for _, name := range updateOnConnectedParamNames {
+		value := params[name]
 		if value == nil {
 			continue
 		}
-		paramsCopy[baseParam.name] = value
+		paramsCopy[name] = value
 	}
 	return paramsCopy
 }
@@ -736,6 +800,34 @@ func getRequestLogFields(
 			continue
 		}
 
+		var tunnelProtocol string
+		if value, ok := params["relay_protocol"]; ok {
+			tunnelProtocol, _ = value.(string)
+		}
+
+		if expectedParam.flags&requestParamLogOnlyForFrontedMeek != 0 &&
+			!protocol.TunnelProtocolUsesFrontedMeek(tunnelProtocol) {
+			continue
+		}
+
+		if expectedParam.flags&requestParamNotLoggedForUnfrontedMeekNonTransformedHeader != 0 &&
+			protocol.TunnelProtocolUsesMeek(tunnelProtocol) &&
+			!protocol.TunnelProtocolUsesFrontedMeek(tunnelProtocol) {
+
+			// Non-HTTP unfronted meek protocols never tranform the host header.
+			if !protocol.TunnelProtocolUsesMeekHTTPS(tunnelProtocol) {
+				continue
+			}
+
+			var transformedHostName string
+			if value, ok := params["meek_transformed_host_name"]; ok {
+				transformedHostName, _ = value.(string)
+			}
+			if transformedHostName != "1" {
+				continue
+			}
+		}
+
 		value := params[expectedParam.name]
 		if value == nil {
 

psiphon/serverApi.go

@@ -34,7 +34,6 @@ import (
 	"net/url"
 	"strconv"
 	"strings"
-	"sync/atomic"
 
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/parameters"
@@ -50,11 +49,6 @@ import (
 // offer the Psiphon API through a web service; newer servers offer the Psiphon
 // API through SSH requests made directly through the tunnel's SSH client.
 type ServerContext struct {
-	// Note: 64-bit ints used with atomic operations are placed
-	// at the start of struct to ensure 64-bit alignment.
-	// (https://golang.org/pkg/sync/atomic/#pkg-note-BUG)
-	tunnelNumber             int64
-	sessionId                string
 	tunnel                   *Tunnel
 	psiphonHttpsClient       *http.Client
 	statsRegexps             *transferstats.Regexps
@@ -64,13 +58,6 @@ type ServerContext struct {
 	paddingPRNG              *prng.PRNG
 }
 
-// nextTunnelNumber is a monotonically increasing number assigned to each
-// successive tunnel connection. The sessionId and tunnelNumber together
-// form a globally unique identifier for tunnels, which is used for
-// stats. Note that the number is increasing but not necessarily
-// consecutive for each active tunnel in session.
-var nextTunnelNumber int64
-
 // MakeSessionId creates a new session ID. The same session ID is used across
 // multi-tunnel controller runs, where each tunnel has its own ServerContext
 // instance.
@@ -104,8 +91,6 @@ func NewServerContext(tunnel *Tunnel) (*ServerContext, error) {
 	}
 
 	serverContext := &ServerContext{
-		sessionId:          tunnel.sessionId,
-		tunnelNumber:       atomic.AddInt64(&nextTunnelNumber, 1),
 		tunnel:             tunnel,
 		psiphonHttpsClient: psiphonHttpsClient,
 		paddingPRNG:        prng.NewPRNGWithSeed(tunnel.dialParams.APIRequestPaddingSeed),
@@ -318,13 +303,10 @@ func (serverContext *ServerContext) DoConnectedRequest() error {
 
 	params := serverContext.getBaseAPIParameters()
 
-	lastConnected, err := GetKeyValue(datastoreLastConnectedKey)
+	lastConnected, err := getLastConnected()
 	if err != nil {
 		return common.ContextError(err)
 	}
-	if lastConnected == "" {
-		lastConnected = "None"
-	}
 
 	params["last_connected"] = lastConnected
 
@@ -372,6 +354,17 @@ func (serverContext *ServerContext) DoConnectedRequest() error {
 	return nil
 }
 
+func getLastConnected() (string, error) {
+	lastConnected, err := GetKeyValue(datastoreLastConnectedKey)
+	if err != nil {
+		return "", common.ContextError(err)
+	}
+	if lastConnected == "" {
+		lastConnected = "None"
+	}
+	return lastConnected, nil
+}
+
 // StatsRegexps gets the Regexps used for the statistics for this tunnel.
 func (serverContext *ServerContext) StatsRegexps() *transferstats.Regexps {
 	return serverContext.statsRegexps
@@ -386,7 +379,7 @@ func (serverContext *ServerContext) DoStatusRequest(tunnel *Tunnel) error {
 	// payload for future attempt, in all failure cases.
 
 	statusPayload, statusPayloadInfo, err := makeStatusRequestPayload(
-		serverContext.tunnel.config.clientParameters,
+		serverContext.tunnel.config,
 		tunnel.dialParams.ServerEntry.IpAddress)
 	if err != nil {
 		return common.ContextError(err)
@@ -467,15 +460,13 @@ type statusRequestPayloadInfo struct {
 }
 
 func makeStatusRequestPayload(
-	clientParameters *parameters.ClientParameters,
+	config *Config,
 	serverId string) ([]byte, *statusRequestPayloadInfo, error) {
 
 	transferStats := transferstats.TakeOutStatsForServer(serverId)
 	hostBytes := transferStats.GetStatsForStatusRequest()
 
-	maxCount := clientParameters.Get().Int(parameters.PsiphonAPIPersistentStatsMaxCount)
-
-	persistentStats, err := TakeOutUnreportedPersistentStats(maxCount)
+	persistentStats, err := TakeOutUnreportedPersistentStats(config)
 	if err != nil {
 		NoticeAlert(
 			"TakeOutUnreportedPersistentStats failed: %s", common.ContextError(err))
@@ -502,6 +493,7 @@ func makeStatusRequestPayload(
 
 	persistentStatPayloadNames := make(map[string]string)
 	persistentStatPayloadNames[datastorePersistentStatTypeRemoteServerList] = "remote_server_list_stats"
+	persistentStatPayloadNames[datastorePersistentStatTypeFailedTunnel] = "failed_tunnel_stats"
 
 	for statType, stats := range persistentStats {
 
@@ -554,15 +546,9 @@ func confirmStatusRequestPayload(payloadInfo *statusRequestPayloadInfo) {
 // records are stored in the persistent datastore and reported
 // via subsequent status requests sent to any Psiphon server.
 //
-// Note that common event field values may change between the
-// stat recording and reporting include client geo data,
-// propagation channel, sponsor ID, client version. These are not
-// stored in the datastore (client region, in particular, since
-// that would create an on-disk record of user location).
-// TODO: the server could encrypt, with a nonce and key unknown to
-// the client, a blob containing this data; return it in the
-// handshake response; and the client could store and later report
-// this blob with its tunnel stats records.
+// Note that some common event field values may change between the
+// stat recording and reporting, including client geolocation and
+// host_id.
 //
 // Multiple "status" requests may be in flight at once (due
 // to multi-tunnel, asynchronous final status retry, and
@@ -575,25 +561,66 @@ func confirmStatusRequestPayload(payloadInfo *statusRequestPayloadInfo) {
 // processes a status request but the client fails to receive
 // the response.
 func RecordRemoteServerListStat(
-	url, etag string) error {
+	config *Config, url, etag string) error {
 
-	remoteServerListStat := struct {
-		ClientDownloadTimestamp string `json:"client_download_timestamp"`
-		URL                     string `json:"url"`
-		ETag                    string `json:"etag"`
-	}{
-		common.TruncateTimestampToHour(common.GetCurrentTimestamp()),
-		url,
-		etag,
+	if !config.GetClientParameters().Bool(
+		parameters.RecordRemoteServerListPersistentStats) {
+		return nil
 	}
 
-	remoteServerListStatJson, err := json.Marshal(remoteServerListStat)
+	params := make(common.APIParameters)
+
+	params["session_id"] = config.SessionID
+	params["propagation_channel_id"] = config.PropagationChannelId
+	params["sponsor_id"] = config.GetSponsorID()
+	params["client_version"] = config.ClientVersion
+	params["client_platform"] = config.ClientPlatform
+	params["client_build_rev"] = common.GetBuildInfo().BuildRev
+
+	params["client_download_timestamp"] = common.TruncateTimestampToHour(common.GetCurrentTimestamp())
+	params["url"] = url
+	params["etag"] = etag
+
+	remoteServerListStatJson, err := json.Marshal(params)
+	if err != nil {
+		return common.ContextError(err)
+	}
+
+	return StorePersistentStat(
+		config, datastorePersistentStatTypeRemoteServerList, remoteServerListStatJson)
+}
+
+// RecordFailedTunnelStat records metrics for a failed tunnel dial, including
+// dial parameters and error condition (tunnelErr).
+//
+// This uses the same reporting facility, with the same caveats, as
+// RecordRemoteServerListStat.
+func RecordFailedTunnelStat(
+	config *Config, dialParams *DialParameters, tunnelErr error) error {
+
+	if !config.GetClientParameters().Bool(
+		parameters.RecordFailedTunnelPersistentStats) {
+		return nil
+	}
+
+	lastConnected, err := getLastConnected()
+	if err != nil {
+		return common.ContextError(err)
+	}
+
+	params := getBaseAPIParameters(config, dialParams)
+	params["server_entry_ip_address"] = dialParams.ServerEntry.IpAddress
+	params["last_connected"] = lastConnected
+	params["client_failed_timestamp"] = common.TruncateTimestampToHour(common.GetCurrentTimestamp())
+	params["tunnel_error"] = tunnelErr.Error()
+
+	failedTunnelStatJson, err := json.Marshal(params)
 	if err != nil {
 		return common.ContextError(err)
 	}
 
 	return StorePersistentStat(
-		datastorePersistentStatTypeRemoteServerList, remoteServerListStatJson)
+		config, datastorePersistentStatTypeFailedTunnel, failedTunnelStatJson)
 }
 
 // doGetRequest makes a tunneled HTTPS request and returns the response body.
@@ -667,7 +694,6 @@ func (serverContext *ServerContext) getBaseAPIParameters() common.APIParameters
 
 	params := getBaseAPIParameters(
 		serverContext.tunnel.config,
-		serverContext.sessionId,
 		serverContext.tunnel.dialParams)
 
 	// Add a random amount of padding to defend against API call traffic size
@@ -698,13 +724,12 @@ func (serverContext *ServerContext) getBaseAPIParameters() common.APIParameters
 // for metrics.
 func getBaseAPIParameters(
 	config *Config,
-	sessionID string,
 	dialParams *DialParameters) common.APIParameters {
 
 	params := make(common.APIParameters)
 
-	params["session_id"] = sessionID
-	params["client_session_id"] = sessionID
+	params["session_id"] = config.SessionID
+	params["client_session_id"] = config.SessionID
 	params["server_secret"] = dialParams.ServerEntry.WebServerSecret
 	params["propagation_channel_id"] = config.PropagationChannelId
 	params["sponsor_id"] = config.GetSponsorID()
@@ -805,6 +830,15 @@ func getBaseAPIParameters(
 	}
 	params["is_replay"] = isReplay
 
+	if config.EgressRegion != "" {
+		params["egress_region"] = config.EgressRegion
+	}
+
+	// dialParams.DialDuration is nanoseconds; divide to get to milliseconds
+	params["dial_duration"] = fmt.Sprintf("%d", dialParams.DialDuration/1000000)
+
+	params["candidate_number"] = strconv.Itoa(dialParams.CandidateNumber)
+
 	if dialParams.DialConnMetrics != nil {
 		metrics := dialParams.DialConnMetrics.GetMetrics()
 		for name, value := range metrics {

psiphon/tlsDialer.go

@@ -396,15 +396,22 @@ func CustomTLSDial(
 
 	} else {
 
-		var clientSessionCache tris.ClientSessionCache
-		if config.ObfuscatedSessionTicketKey != "" {
-			clientSessionCache = tris.NewObfuscatedClientSessionCache(
-				obfuscatedSessionTicketKey)
-		} else {
-			clientSessionCache = config.trisClientSessionCache
-			if clientSessionCache == nil {
-				clientSessionCache = tris.NewLRUClientSessionCache(0)
-			}
+		clientSessionCache := config.trisClientSessionCache
+		if clientSessionCache == nil {
+			clientSessionCache = tris.NewLRUClientSessionCache(0)
+		}
+
+		// The tris TLS provider should be used only for TLS 1.3.
+		//
+		// Obfuscated session tickets are not currently supported in TLS 1.3,
+		// but we allow UNFRONTED-MEEK-SESSION-TICKET-OSSH to use TLS 1.3
+		// profiles for additional diversity/capacity; TLS 1.3 encrypts the
+		// server certificate, so the desired obfuscated session tickets
+		// property of obfuscating server certificates is satisfied.
+		//
+		// An additional sanity check:
+		if !protocol.TLSProfileIsTLS13(selectedTLSProfile) {
+			return nil, common.ContextError(errors.New("TLS profile is not TLS 1.3"))
 		}
 
 		tlsConfig := &tris.Config{

psiphon/tunnel.go

@@ -87,7 +87,6 @@ type Tunnel struct {
 	isActivated                bool
 	isDiscarded                bool
 	isClosed                   bool
-	sessionId                  string
 	dialParams                 *DialParameters
 	serverContext              *ServerContext
 	conn                       *common.ActivityMonitoredConn
@@ -126,14 +125,12 @@ type Tunnel struct {
 func ConnectTunnel(
 	ctx context.Context,
 	config *Config,
-	sessionId string,
 	adjustedEstablishStartTime monotime.Time,
 	dialParams *DialParameters) (*Tunnel, error) {
 
 	// Build transport layers and establish SSH connection. Note that
 	// dialConn and monitoredConn are the same network connection.
-	dialResult, err := dialTunnel(
-		ctx, config, sessionId, dialParams)
+	dialResult, err := dialTunnel(ctx, config, dialParams)
 	if err != nil {
 		return nil, common.ContextError(err)
 	}
@@ -142,7 +139,6 @@ func ConnectTunnel(
 	return &Tunnel{
 		mutex:             new(sync.Mutex),
 		config:            config,
-		sessionId:         sessionId,
 		dialParams:        dialParams,
 		conn:              dialResult.monitoredConn,
 		sshClient:         dialResult.sshClient,
@@ -158,8 +154,7 @@ func ConnectTunnel(
 // request and starting operateTunnel, the worker that monitors the tunnel
 // and handles periodic management.
 func (tunnel *Tunnel) Activate(
-	ctx context.Context,
-	tunnelOwner TunnelOwner) error {
+	ctx context.Context, tunnelOwner TunnelOwner) (retErr error) {
 
 	// Ensure that, unless the context is cancelled, any replayed dial
 	// parameters are cleared, no longer to be retried, if the tunnel fails to
@@ -168,6 +163,7 @@ func (tunnel *Tunnel) Activate(
 	defer func() {
 		if !activationSucceeded && ctx.Err() == nil {
 			tunnel.dialParams.Failed()
+			_ = RecordFailedTunnelStat(tunnel.config, tunnel.dialParams, retErr)
 		}
 	}()
 
@@ -525,8 +521,14 @@ type dialResult struct {
 func dialTunnel(
 	ctx context.Context,
 	config *Config,
-	sessionId string,
-	dialParams *DialParameters) (*dialResult, error) {
+	dialParams *DialParameters) (_ *dialResult, retErr error) {
+
+	// Return immediately when overall context is canceled or timed-out. This
+	// avoids notice noise.
+	err := ctx.Err()
+	if err != nil {
+		return nil, common.ContextError(err)
+	}
 
 	p := config.clientParameters.Get()
 	timeout := p.Duration(parameters.TunnelConnectTimeout)
@@ -551,6 +553,7 @@ func dialTunnel(
 	defer func() {
 		if !dialSucceeded && baseCtx.Err() == nil {
 			dialParams.Failed()
+			_ = RecordFailedTunnelStat(config, dialParams, retErr)
 		}
 	}()
 
@@ -558,6 +561,19 @@ func dialTunnel(
 	ctx, cancelFunc = context.WithTimeout(ctx, timeout)
 	defer cancelFunc()
 
+	// DialDuration is the elapsed time for both successful and failed tunnel
+	// dials. For successful tunnels, it includes any the network protocol
+	// handshake(s), obfuscation protocol handshake(s), SSH handshake, and
+	// liveness test, when performed.
+	//
+	// Note: ensure DialDuration is set before calling any function which logs
+	// dial_duration.
+
+	startDialTime := monotime.Now()
+	defer func() {
+		dialParams.DialDuration = monotime.Since(startDialTime)
+	}()
+
 	// Note: dialParams.MeekResolvedIPAddress isn't set until the dial begins,
 	// so it will always be blank in NoticeConnectingServer.
 
@@ -566,7 +582,6 @@ func dialTunnel(
 	// Create the base transport: meek or direct connection
 
 	var dialConn net.Conn
-	var err error
 
 	if protocol.TunnelProtocolUsesMeek(dialParams.TunnelProtocol) {
 
@@ -699,7 +714,7 @@ func dialTunnel(
 	}
 
 	sshPasswordPayload := &protocol.SSHPasswordPayload{
-		SessionId:          sessionId,
+		SessionId:          config.SessionID,
 		SshPassword:        dialParams.ServerEntry.SshPassword,
 		ClientCapabilities: []string{protocol.CLIENT_CAPABILITY_SERVER_REQUESTS},
 	}
@@ -801,8 +816,11 @@ func dialTunnel(
 					livenessTestMinDownstreamBytes, livenessTestMaxDownstreamBytes,
 					dialParams.LivenessTestSeed)
 
-				NoticeLivenessTest(
-					dialParams.ServerEntry.IpAddress, metrics, err == nil)
+				// Skip notice when cancelling.
+				if baseCtx.Err() == nil {
+					NoticeLivenessTest(
+						dialParams.ServerEntry.IpAddress, metrics, err == nil)
+				}
 			}
 		}
 

vendor/github.com/Psiphon-Labs/tls-tris/obfuscated.go

@@ -62,6 +62,11 @@ import (
 //     negotiated with the server, such as the cipher suite. It's implicitly assumed that
 //     the server can support the selected parameters.
 //
+// tls-tris notes:
+//   - Obfuscated session tickets are not supported for TLS 1.3 _clients_, which use a
+//     distinct session ticket format. Obfuscated session ticket support in this package
+//     is intended to support TLS 1.2 clients.
+//
 func NewObfuscatedClientSessionCache(sharedSecret [32]byte) ClientSessionCache {
 	return &obfuscatedClientSessionCache{
 		sharedSecret: sharedSecret,

vendor/vendor.json

@@ -63,10 +63,10 @@
 			"revisionTime": "2018-09-12T16:47:43Z"
 		},
 		{
-			"checksumSHA1": "jfV4PEzL1mcC3i/V903S4gVDYFU=",
+			"checksumSHA1": "jgnMy8LzzP2+YM0UxU/Bz0Gq1Xc=",
 			"path": "github.com/Psiphon-Labs/tls-tris",
-			"revision": "a28158310549bd66234df1b4010a6565c33173cb",
-			"revisionTime": "2018-12-19T02:25:17Z"
+			"revision": "b1bee89a578d564109bdfa48f9ba1b9434d776f9",
+			"revisionTime": "2019-01-08T19:41:13Z"
 		},
 		{
 			"checksumSHA1": "pXgGBe9O8n4KGzAlakVVVwwJqxo=",