Fix potential panic during InproxySTUNDialParameters replay

- Also, stats fixes:
  - Log session_id with broker events
  - Correct validation for inproxy_broker_upstream_bytes_fragmented
  - Rename ClientRelayedPacket serverID event field

psiphon/common/inproxy/broker.go

@@ -1032,7 +1032,7 @@ func (b *Broker) handleClientRelayedPacket(
 			logFields["connection_id"] = relayedPacketRequest.ConnectionID
 		}
 		if serverID != "" {
-			logFields["server_id"] = serverID
+			logFields["destination_server_id"] = serverID
 		}
 		if retErr != nil {
 			logFields["error"] = retErr.Error()

psiphon/controller.go

@@ -2522,7 +2522,7 @@ func (controller *Controller) runInproxyProxy() {
 		} else {
 
 			// InproxySkipAwaitFullyConnected is a special case to support
-			// server/server_test, where a client mus be its own proxy; in
+			// server/server_test, where a client must be its own proxy; in
 			// this case, awaitFullyEstablished will block forever.
 			// inproxyAwaitBrokerSpecs simply waits until any broker specs
 			// become available, which is sufficient for the test but is not

psiphon/dialParameters.go

@@ -1143,6 +1143,8 @@ func MakeDialParameters(
 			if err != nil {
 				return nil, errors.Trace(err)
 			}
+		} else if dialParams.InproxySTUNDialParameters != nil {
+			dialParams.InproxySTUNDialParameters.Prepare()
 		}
 
 		if !isReplay || !replayInproxyWebRTC {

psiphon/inproxy.go

@@ -1625,6 +1625,9 @@ func (w *InproxyWebRTCDialInstance) ProxyDestinationDialTimeout() time.Duration
 // InproxySTUNDialParameters is compatible with DialParameters JSON
 // marshaling. For client in-proxy tunnel dials, DialParameters will manage
 // STUN dial parameter selection and replay.
+//
+// When an instance of InproxySTUNDialParameters is unmarshaled from JSON,
+// Prepare must be called to initialize the instance for use.
 type InproxySTUNDialParameters struct {
 	ResolveParameters        *resolver.ResolveParameters
 	STUNServerAddress        string
@@ -1716,12 +1719,18 @@ func MakeInproxySTUNDialParameters(
 		STUNServerAddressRFC5780: stunServerAddressRFC5780,
 	}
 
-	dialParams.STUNServerResolvedIPAddress.Store("")
-	dialParams.STUNServerRFC5780ResolvedIPAddress.Store("")
+	dialParams.Prepare()
 
 	return dialParams, nil
 }
 
+// Prepare initializes an InproxySTUNDialParameters for use. Prepare should be
+// called for any InproxySTUNDialParameters instance unmarshaled from JSON.
+func (dialParams *InproxySTUNDialParameters) Prepare() {
+	dialParams.STUNServerResolvedIPAddress.Store("")
+	dialParams.STUNServerRFC5780ResolvedIPAddress.Store("")
+}
+
 // IsValidClientReplay checks that the selected STUN servers remain configured
 // STUN server candidates for in-proxy clients.
 func (dialParams *InproxySTUNDialParameters) IsValidClientReplay(

psiphon/inproxy_test.go

@@ -20,6 +20,7 @@
 package psiphon
 
 import (
+    "encoding/json"
     "io/ioutil"
     "os"
     "regexp"
@@ -38,7 +39,12 @@ func TestInproxyComponents(t *testing.T) {
     // replay; actual in-proxy broker round trips are exercised in the
     // psiphon/server end-to-end tests.
 
-    err := runInproxyBrokerTest()
+    err := runInproxyBrokerDialParametersTest()
+    if err != nil {
+        t.Fatalf(errors.Trace(err).Error())
+    }
+
+    err = runInproxySTUNDialParametersTest()
     if err != nil {
         t.Fatalf(errors.Trace(err).Error())
     }
@@ -51,7 +57,7 @@ func TestInproxyComponents(t *testing.T) {
     // TODO: test inproxyUDPConn multiplexed IPv6Synthesizer
 }
 
-func runInproxyBrokerTest() error {
+func runInproxyBrokerDialParametersTest() error {
 
     testDataDirName, err := ioutil.TempDir("", "psiphon-inproxy-broker-test")
     if err != nil {
@@ -148,6 +154,8 @@ func runInproxyBrokerTest() error {
         return errors.TraceNew("unexpected compartment IDs")
     }
 
+    _ = brokerDialParams.GetMetrics()
+
     // Test: replay on success
 
     previousFrontingDialAddress := brokerDialParams.FrontingDialAddress
@@ -179,6 +187,8 @@ func runInproxyBrokerTest() error {
         return errors.TraceNew("unexpected replayed TLSProfile")
     }
 
+    _ = brokerDialParams.GetMetrics()
+
     // Test: manager's broker client and dial parameters reinitialized after
     // network ID change
 
@@ -205,6 +215,8 @@ func runInproxyBrokerTest() error {
         return errors.TraceNew("unexpected non-replayed FrontingDialAddress")
     }
 
+    _ = brokerDialParams.GetMetrics()
+
     // Test: another replay after switch back to previous network ID
 
     networkID = previousNetworkID
@@ -227,6 +239,8 @@ func runInproxyBrokerTest() error {
         return errors.TraceNew("unexpected replayed TLSProfile")
     }
 
+    _ = brokerDialParams.GetMetrics()
+
     // Test: clear replay
 
     roundTripper, err = brokerClient.GetBrokerDialCoordinator().BrokerClientRoundTripper()
@@ -251,6 +265,8 @@ func runInproxyBrokerTest() error {
         return errors.TraceNew("unexpected non-replayed FrontingDialAddress")
     }
 
+    _ = brokerDialParams.GetMetrics()
+
     // Test: no common compartment IDs sent when personal ID is set
 
     config.InproxyPersonalCompartmentIDs = personalCompartmentIDs
@@ -297,6 +313,63 @@ func runInproxyBrokerTest() error {
         return errors.TraceNew("unexpected compartment IDs")
     }
 
+    _ = brokerDialParams.GetMetrics()
+
+    return nil
+}
+
+func runInproxySTUNDialParametersTest() error {
+
+    testDataDirName, err := ioutil.TempDir("", "psiphon-inproxy-stun-test")
+    if err != nil {
+        return errors.Trace(err)
+    }
+    defer os.RemoveAll(testDataDirName)
+
+    propagationChannelID := prng.HexString(8)
+    sponsorID := prng.HexString(8)
+    networkID := "NETWORK1"
+    stunServerAddresses := []string{"example.org"}
+
+    config := &Config{
+        DataRootDirectory:                 testDataDirName,
+        PropagationChannelId:              propagationChannelID,
+        SponsorId:                         sponsorID,
+        NetworkID:                         networkID,
+        InproxySTUNServerAddresses:        stunServerAddresses,
+        InproxySTUNServerAddressesRFC5780: stunServerAddresses,
+    }
+    err = config.Commit(false)
+    if err != nil {
+        return errors.Trace(err)
+    }
+    config.SetResolver(resolver.NewResolver(&resolver.NetworkConfig{}, networkID))
+
+    p := config.GetParameters().Get()
+    defer p.Close()
+
+    dialParams, err := MakeInproxySTUNDialParameters(config, p, false)
+    if err != nil {
+        return errors.Trace(err)
+    }
+
+    _ = dialParams.GetMetrics()
+
+    dialParamsJSON, err := json.Marshal(dialParams)
+    if err != nil {
+        return errors.Trace(err)
+    }
+
+    var replayDialParams *InproxySTUNDialParameters
+    err = json.Unmarshal(dialParamsJSON, &replayDialParams)
+    if err != nil {
+        return errors.Trace(err)
+    }
+
+    replayDialParams.Prepare()
+
+    _ = replayDialParams.GetMetrics()
+
     return nil
 }
 

psiphon/server/api.go

@@ -995,7 +995,7 @@ func getTacticsAPIParameterLogFieldFormatter() common.APIParameterLogFieldFormat
 	}
 }
 
-var inproxyBrokerRequestParams = append([]requestParamSpec(nil), baseParams...)
+var inproxyBrokerRequestParams = append([]requestParamSpec(nil), baseSessionParams...)
 
 func getInproxyBrokerAPIParameterValidator(config *Config) common.APIParameterValidator {
 	return func(params common.APIParameters) error {
@@ -1151,7 +1151,7 @@ var inproxyDialParams = []requestParamSpec{
 	{"inproxy_broker_tls_version", isAnyString, requestParamOptional},
 	{"inproxy_broker_tls_fragmented", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
 	{"inproxy_broker_client_bpf", isAnyString, requestParamOptional},
-	{"inproxy_broker_upstream_bytes_fragmented", isAnyString, requestParamOptional},
+	{"inproxy_broker_upstream_bytes_fragmented", isIntString, requestParamOptional | requestParamLogStringAsInt},
 	{"inproxy_broker_http_transform", isAnyString, requestParamOptional},
 	{"inproxy_broker_dns_preresolved", isAnyString, requestParamOptional},
 	{"inproxy_broker_dns_preferred", isAnyString, requestParamOptional},