Browse Source

Merge pull request #593 from rod-hynes/master

Logging updates
Rod Hynes 5 years ago
parent
commit
4240519300

+ 0 - 5
MobileLibrary/Android/PsiphonTunnel/PsiphonTunnel.java

@@ -850,11 +850,6 @@ public class PsiphonTunnel {
             json.put("EstablishTunnelTimeoutSeconds", 0);
         }
 
-        // This parameter is for stats reporting
-        if (!json.has("TunnelWholeDevice")) {
-            json.put("TunnelWholeDevice", isVpnMode ? 1 : 0);
-        }
-
         json.put("EmitBytesTransferred", true);
 
         if (localSocksProxyPort != 0 && (!json.has("LocalSocksProxyPort") || json.getInt("LocalSocksProxyPort") == 0)) {

+ 0 - 1
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.h

@@ -107,7 +107,6 @@ typedef NS_ENUM(NSInteger, PsiphonConnectionState)
    - `FetchRoutesTimeoutSeconds`
    - `HttpProxyOriginServerTimeoutSeconds`
  - Fields which should only be set by Psiphon proper:
-   - `TunnelWholeDevice`
    - `LocalHttpProxyPort`
    - `LocalSocksProxyPort`
  @endcode

+ 1 - 2
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.m

@@ -779,10 +779,9 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
     //
 
     // We'll record our state about what mode we're in.
-    *tunnelWholeDevice = ([config[@"TunnelWholeDevice"] integerValue] == 1);
+    *tunnelWholeDevice = (config[@"PacketTunnelTunFileDescriptor"] != nil);
 
     // Other optional fields not being altered. If not set, their defaults will be used:
-    // * TunnelWholeDevice
     // * LocalSocksProxyPort
     // * LocalHttpProxyPort
     // * UpstreamProxyUrl

+ 18 - 13
psiphon/config.go

@@ -119,10 +119,10 @@ type Config struct {
 	// the client reports to the server.
 	ClientPlatform string
 
-	// TunnelWholeDevice is a flag that is passed through to the handshake
-	// request for stats purposes. Set to 1 when the host application is
-	// tunneling the whole device, 0 otherwise.
-	TunnelWholeDevice int
+	// ClientFeatures is a list of feature names denoting enabled application
+	// features. Clients report enabled features to the server for stats
+	// purposes.
+	ClientFeatures []string
 
 	// EgressRegion is a ISO 3166-1 alpha-2 country code which indicates which
 	// country to egress from. For the default, "", the best performing server
@@ -257,36 +257,29 @@ type Config struct {
 	// NetworkConnectivityChecker is an interface that enables tunnel-core to
 	// call into the host application to check for network connectivity. See:
 	// NetworkConnectivityChecker doc.
-	//
-	// This parameter is only applicable to library deployments.
 	NetworkConnectivityChecker NetworkConnectivityChecker
 
 	// DeviceBinder is an interface that enables tunnel-core to call into the
 	// host application to bind sockets to specific devices. See: DeviceBinder
 	// doc.
 	//
-	// This parameter is only applicable to library deployments.
+	// When DeviceBinder is set, the "VPN" feature name is automatically added
+	// when reporting ClientFeatures.
 	DeviceBinder DeviceBinder
 
 	// IPv6Synthesizer is an interface that allows tunnel-core to call into
 	// the host application to synthesize IPv6 addresses. See: IPv6Synthesizer
 	// doc.
-	//
-	// This parameter is only applicable to library deployments.
 	IPv6Synthesizer IPv6Synthesizer
 
 	// DnsServerGetter is an interface that enables tunnel-core to call into
 	// the host application to discover the native network DNS server
 	// settings. See: DnsServerGetter doc.
-	//
-	// This parameter is only applicable to library deployments.
 	DnsServerGetter DnsServerGetter
 
 	// NetworkIDGetter in an interface that enables tunnel-core to call into
 	// the host application to get an identifier for the host's current active
 	// network. See: NetworkIDGetter doc.
-	//
-	// This parameter is only applicable to library deployments.
 	NetworkIDGetter NetworkIDGetter
 
 	// NetworkID, when not blank, is used as the identifier for the host's
@@ -741,6 +734,8 @@ type Config struct {
 	deviceBinder    DeviceBinder
 	networkIDGetter NetworkIDGetter
 
+	clientFeatures []string
+
 	committed bool
 
 	loadTimestamp string
@@ -1113,6 +1108,16 @@ func (config *Config) Commit(migrateFromLegacyFields bool) error {
 
 	config.networkIDGetter = newLoggingNetworkIDGetter(networkIDGetter)
 
+	// Initialize config.clientFeatures, which adds feature names on top of
+	// those specified by the host application in config.ClientFeatures.
+
+	config.clientFeatures = config.ClientFeatures
+
+	feature := "VPN"
+	if config.DeviceBinder != nil && !common.Contains(config.clientFeatures, feature) {
+		config.clientFeatures = append(config.clientFeatures, feature)
+	}
+
 	// Migrate from old config fields. This results in files being moved under
 	// a config specified data root directory.
 	if migrateFromLegacyFields && needMigration {

+ 1 - 1
psiphon/controller_test.go

@@ -1029,7 +1029,7 @@ func fetchAndVerifyWebsite(t *testing.T, httpProxyPort int) error {
 
 const disruptorProxyAddress = "127.0.0.1:2160"
 const disruptorProxyURL = "socks4a://" + disruptorProxyAddress
-const disruptorMaxConnectionBytes = 250000
+const disruptorMaxConnectionBytes = 150000
 const disruptorMaxConnectionTime = 10 * time.Second
 
 func initDisruptor() {

+ 2 - 11
psiphon/server/api.go

@@ -446,15 +446,6 @@ func connectedAPIRequestHandler(
 				uniqueUserParams))
 	}
 
-	// TODO: retire the legacy "connected" log event
-	log.LogRawFieldsWithTimestamp(
-		getRequestLogFields(
-			"connected",
-			geoIPData,
-			authorizedAccessTypes,
-			params,
-			connectedRequestParams))
-
 	pad_response, _ := getPaddingSizeRequestParam(params, "pad_response")
 
 	connectedResponse := protocol.ConnectedResponse{
@@ -812,10 +803,9 @@ var baseParams = []requestParamSpec{
 	{"sponsor_id", isHexDigits, 0},
 	{"client_version", isIntString, requestParamLogStringAsInt},
 	{"client_platform", isClientPlatform, 0},
+	{"client_features", isAnyString, requestParamOptional | requestParamArray},
 	{"client_build_rev", isHexDigits, requestParamOptional},
-	{"tunnel_whole_device", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
 	{"device_region", isAnyString, requestParamOptional},
-	{"split_tunnel", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
 }
 
 // baseSessionParams adds to baseParams the required session_id parameter. For
@@ -870,6 +860,7 @@ var baseDialParams = []requestParamSpec{
 	{"client_bpf", isAnyString, requestParamOptional},
 	{"network_type", isAnyString, requestParamOptional},
 	{"conjure_transport", isAnyString, requestParamOptional},
+	{"split_tunnel", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
 }
 
 // baseSessionAndDialParams adds baseDialParams to baseSessionParams.

+ 30 - 4
psiphon/server/server_test.go

@@ -657,6 +657,7 @@ var (
 	testUserAgents          = []string{"ua1", "ua2", "ua3"}
 	testNetworkType         = "WIFI"
 	testCustomHostNameRegex = `[a-z0-9]{5,10}\.example\.org`
+	testClientFeatures      = []string{"feature 1", "feature 2"}
 )
 
 var serverRuns = 0
@@ -964,13 +965,15 @@ func runServer(t *testing.T, runConfig *runServerConfig) {
 		jsonLimitTLSProfiles = fmt.Sprintf(`,"LimitTLSProfiles" : ["%s"]`, runConfig.tlsProfile)
 	}
 
+	testClientFeaturesJSON, _ := json.Marshal(testClientFeatures)
+
 	clientConfigJSON := fmt.Sprintf(`
     {
         "ClientPlatform" : "Android_10_com.test.app",
         "ClientVersion" : "0",
+        "ClientFeatures" : %s,
         "SponsorId" : "0",
         "PropagationChannelId" : "0",
-        "TunnelWholeDevice" : 1,
         "DeviceRegion" : "US",
         "DisableRemoteServerListFetcher" : true,
         "EstablishTunnelPausePeriodSeconds" : 1,
@@ -978,7 +981,12 @@ func runServer(t *testing.T, runConfig *runServerConfig) {
         "LimitTunnelProtocols" : ["%s"]
         %s
         %s
-    }`, numTunnels, runConfig.tunnelProtocol, jsonLimitTLSProfiles, jsonNetworkID)
+    }`,
+		string(testClientFeaturesJSON),
+		numTunnels,
+		runConfig.tunnelProtocol,
+		jsonLimitTLSProfiles,
+		jsonNetworkID)
 
 	clientConfig, err := psiphon.LoadConfig([]byte(clientConfigJSON))
 	if err != nil {
@@ -1405,8 +1413,8 @@ func checkExpectedServerTunnelLogFields(
 		"propagation_channel_id",
 		"sponsor_id",
 		"client_platform",
+		"client_features",
 		"relay_protocol",
-		"tunnel_whole_device",
 		"device_region",
 		"ssh_client_version",
 		"server_entry_region",
@@ -1433,6 +1441,25 @@ func checkExpectedServerTunnelLogFields(
 		return fmt.Errorf("unexpected relay_protocol '%s'", fields["ssh_client_version"])
 	}
 
+	clientFeatures := fields["client_features"].([]interface{})
+	if len(clientFeatures) != len(testClientFeatures) {
+		return fmt.Errorf("unexpected client_features '%s'", fields["client_features"])
+	}
+	for i, feature := range testClientFeatures {
+		if clientFeatures[i].(string) != feature {
+			return fmt.Errorf("unexpected client_features '%s'", fields["client_features"])
+		}
+	}
+
+	if runConfig.doSplitTunnel {
+		if fields["split_tunnel"] == nil {
+			return fmt.Errorf("missing expected field 'split_tunnel'")
+		}
+		if fields["split_tunnel"].(bool) != true {
+			return fmt.Errorf("missing split_tunnel value")
+		}
+	}
+
 	if protocol.TunnelProtocolUsesObfuscatedSSH(runConfig.tunnelProtocol) {
 
 		for _, name := range []string{
@@ -1625,7 +1652,6 @@ func checkExpectedUniqueUserLogFields(
 		"propagation_channel_id",
 		"sponsor_id",
 		"client_platform",
-		"tunnel_whole_device",
 		"device_region",
 	} {
 		if fields[name] == nil || fmt.Sprintf("%s", fields[name]) == "" {

+ 1 - 1
psiphon/server/tunnelServer.go

@@ -2533,8 +2533,8 @@ var blocklistHitsStatParams = []requestParamSpec{
 	{"sponsor_id", isHexDigits, 0},
 	{"client_version", isIntString, requestParamLogStringAsInt},
 	{"client_platform", isClientPlatform, 0},
+	{"client_features", isAnyString, requestParamOptional | requestParamArray},
 	{"client_build_rev", isHexDigits, requestParamOptional},
-	{"tunnel_whole_device", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
 	{"device_region", isAnyString, requestParamOptional},
 	{"egress_region", isRegionCode, requestParamOptional},
 	{"session_id", isHexDigits, 0},

+ 1 - 1
psiphon/serverApi.go

@@ -879,8 +879,8 @@ func getBaseAPIParameters(
 	params["sponsor_id"] = config.GetSponsorID()
 	params["client_version"] = config.ClientVersion
 	params["client_platform"] = config.ClientPlatform
+	params["client_features"] = config.clientFeatures
 	params["client_build_rev"] = buildinfo.GetBuildInfo().BuildRev
-	params["tunnel_whole_device"] = strconv.Itoa(config.TunnelWholeDevice)
 
 	// Blank parameters must be omitted.