From e4bfdccb571bb4309d5b7200cb4e0e1565d12b70 Mon Sep 17 00:00:00 2001 From: Varun Anand Date: Fri, 24 May 2019 16:29:22 -0700 Subject: [PATCH 1/2] Addressing comments for http://ag/7700679. (cherry picked from commit a229333986f95e48347fa8b653d7e1776019f84a) Note, that its in a separate CL so we could cherry-pick this CL to aosp. http://ag/7700679 is already in aosp (http://aosp/865073). Bug: 113122541 Bug: 120145746 Test: atest FrameworksNetTests Change-Id: Ic1767bc8bf1460e4223f86465fc72344428e6055 Merged-In: I7cfda226b4ed11b67002b83b38fba0f5caf96718 --- core/java/android/net/NetworkStats.java | 14 ++- .../android/server/ConnectivityService.java | 4 +- .../server/net/NetworkStatsServiceTest.java | 115 ++++++++++++++---- 3 files changed, 102 insertions(+), 31 deletions(-) diff --git a/core/java/android/net/NetworkStats.java b/core/java/android/net/NetworkStats.java index 27e0414961736..b27b1c057807a 100644 --- a/core/java/android/net/NetworkStats.java +++ b/core/java/android/net/NetworkStats.java @@ -1300,7 +1300,8 @@ public class NetworkStats implements Parcelable { } final Entry tmpEntry = new Entry(); - for (int i = 0; i < size; i++) { + final int origSize = size; + for (int i = 0; i < origSize; i++) { if (!Objects.equals(iface[i], tunIface)) { // Consider only entries that go onto the VPN interface. continue; @@ -1316,8 +1317,9 @@ public class NetworkStats implements Parcelable { tmpEntry.roaming = roaming[i]; tmpEntry.defaultNetwork = defaultNetwork[i]; - // In a first pass, compute each UID's total share of data across all underlyingIfaces. - // This is computed on the basis of the share of each UID's usage over tunIface. + // In a first pass, compute this entry's total share of data across all + // underlyingIfaces. This is computed on the basis of the share of this entry's usage + // over tunIface. // TODO: Consider refactoring first pass into a separate helper method. long totalRxBytes = 0; if (tunIfaceTotal.rxBytes > 0) { @@ -1394,9 +1396,11 @@ public class NetworkStats implements Parcelable { * perInterfaceTotal[j].operations / underlyingIfacesTotal.operations; } - + // tmpEntry now contains the migrated data of the i-th entry for the j-th underlying + // interface. Add that data usage to this object. combineValues(tmpEntry); if (tag[i] == TAG_NONE) { + // Add the migrated data to moved so it is deducted from the VPN app later. moved[j].add(tmpEntry); // Add debug info tmpEntry.set = SET_DBG_VPN_IN; @@ -1412,8 +1416,8 @@ public class NetworkStats implements Parcelable { @NonNull String[] underlyingIfaces, @NonNull Entry[] moved) { for (int i = 0; i < underlyingIfaces.length; i++) { - // Add debug info moved[i].uid = tunUid; + // Add debug info moved[i].set = SET_DBG_VPN_OUT; moved[i].tag = TAG_NONE; moved[i].iface = underlyingIfaces[i]; diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index cdab8c6ce0917..f25bb592033a5 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -4375,7 +4375,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // the underlyingNetworks list. if (underlyingNetworks == null) { NetworkAgentInfo defaultNai = getDefaultNetwork(); - if (defaultNai != null && defaultNai.linkProperties != null) { + if (defaultNai != null) { underlyingNetworks = new Network[] { defaultNai.network }; } } @@ -4383,7 +4383,7 @@ public class ConnectivityService extends IConnectivityManager.Stub List interfaces = new ArrayList<>(); for (Network network : underlyingNetworks) { LinkProperties lp = getLinkProperties(network); - if (lp != null) { + if (lp != null && !TextUtils.isEmpty(lp.getInterfaceName())) { interfaces.add(lp.getInterfaceName()); } } diff --git a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java index 07b5ba46245ef..80875732cf3ca 100644 --- a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java @@ -930,6 +930,65 @@ public class NetworkStatsServiceTest { assertUidTotal(sTemplateImsi1, UID_TETHERING, 1920L, 14L, 384L, 2L, 0); } + @Test + public void vpnRewriteTrafficThroughItself() throws Exception { + // WiFi network is connected and VPN is using WiFi (which has TEST_IFACE). + expectDefaultSettings(); + NetworkState[] networkStates = new NetworkState[] {buildWifiState(), buildVpnState()}; + VpnInfo[] vpnInfos = new VpnInfo[] {createVpnInfo(new String[] {TEST_IFACE})}; + expectNetworkStatsUidDetail(buildEmptyStats()); + expectBandwidthControlCheck(); + + mService.forceUpdateIfaces( + new Network[] {WIFI_NETWORK, VPN_NETWORK}, + vpnInfos, + networkStates, + getActiveIface(networkStates)); + // create some traffic (assume 10 bytes of MTU for VPN interface and 1 byte encryption + // overhead per packet): + // + // 1000 bytes (100 packets) were sent, and 2000 bytes (200 packets) were received by UID_RED + // over VPN. + // 500 bytes (50 packets) were sent, and 1000 bytes (100 packets) were received by UID_BLUE + // over VPN. + // + // VPN UID rewrites packets read from TUN back to TUN, plus some of its own traffic + // (100 bytes). + incrementCurrentTime(HOUR_IN_MILLIS); + expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 5) + .addValues(TUN_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 2000L, 200L, 1000L, 100L, 1L) + .addValues(TUN_IFACE, UID_BLUE, SET_DEFAULT, TAG_NONE, 1000L, 100L, 500L, 50L, 1L) + // VPN rewrites all the packets read from TUN + 100 additional bytes of VPN's + // own traffic. + .addValues(TUN_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 0L, 0L, 1600L, 160L, 2L) + // VPN sent 1760 bytes over WiFi in foreground (SET_FOREGROUND) i.e. 1600 + // bytes (160 packets) + 1 byte/packet overhead (=160 bytes). + .addValues(TEST_IFACE, UID_VPN, SET_FOREGROUND, TAG_NONE, 0L, 0L, 1760L, 176L, 1L) + // VPN received 3300 bytes over WiFi in background (SET_DEFAULT) i.e. 3000 bytes + // (300 packets) + 1 byte/packet encryption overhead (=300 bytes). + .addValues(TEST_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 3300L, 300L, 0L, 0L, 1L)); + + forcePollAndWaitForIdle(); + + // Verify increased TUN usage by UID_VPN does not get attributed to other apps. + NetworkStats tunStats = + mService.getDetailedUidStats(new String[] {TUN_IFACE}); + assertValues( + tunStats, TUN_IFACE, UID_RED, SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL, + DEFAULT_NETWORK_ALL, 2000L, 200L, 1000L, 100L, 1); + assertValues( + tunStats, TUN_IFACE, UID_BLUE, SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL, + DEFAULT_NETWORK_ALL, 1000L, 100L, 500L, 50L, 1); + assertValues( + tunStats, TUN_IFACE, UID_VPN, SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL, + DEFAULT_NETWORK_ALL, 0L, 0L, 1600L, 160L, 2); + + // Verify correct attribution over WiFi. + assertUidTotal(sTemplateWifi, UID_RED, 2000L, 200L, 1000L, 100L, 1); + assertUidTotal(sTemplateWifi, UID_BLUE, 1000L, 100L, 500L, 50L, 1); + assertUidTotal(sTemplateWifi, UID_VPN, 300L, 0L, 260L, 26L, 2); + } + @Test public void vpnWithOneUnderlyingIface() throws Exception { // WiFi network is connected and VPN is using WiFi (which has TEST_IFACE). @@ -946,25 +1005,29 @@ public class NetworkStatsServiceTest { getActiveIface(networkStates)); // create some traffic (assume 10 bytes of MTU for VPN interface and 1 byte encryption // overhead per packet): - // 1000 bytes (100 packets) were sent/received by UID_RED over VPN. - // 500 bytes (50 packets) were sent/received by UID_BLUE over VPN. - // VPN sent/received 1650 bytes (150 packets) over WiFi. - // Of 1650 bytes over WiFi, expect 1000 bytes attributed to UID_RED, 500 bytes attributed to - // UID_BLUE, and 150 bytes attributed to UID_VPN for both rx/tx traffic. + // 1000 bytes (100 packets) were sent, and 2000 bytes (200 packets) were received by UID_RED + // over VPN. + // 500 bytes (50 packets) were sent, and 1000 bytes (100 packets) were received by UID_BLUE + // over VPN. + // VPN sent 1650 bytes (150 packets), and received 3300 (300 packets) over WiFi. + // Of 1650 bytes sent over WiFi, expect 1000 bytes attributed to UID_RED, 500 bytes + // attributed to UID_BLUE, and 150 bytes attributed to UID_VPN. + // Of 3300 bytes received over WiFi, expect 2000 bytes attributed to UID_RED, 1000 bytes + // attributed to UID_BLUE, and 300 bytes attributed to UID_VPN. incrementCurrentTime(HOUR_IN_MILLIS); expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 3) - .addValues(TUN_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 1000L, 100L, 1000L, 100L, 1L) - .addValues(TUN_IFACE, UID_BLUE, SET_DEFAULT, TAG_NONE, 500L, 50L, 500L, 50L, 1L) - // VPN received 1650 bytes over WiFi in background (SET_DEFAULT). - .addValues(TEST_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 1650L, 150L, 0L, 0L, 1L) + .addValues(TUN_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 2000L, 200L, 1000L, 100L, 1L) + .addValues(TUN_IFACE, UID_BLUE, SET_DEFAULT, TAG_NONE, 1000L, 100L, 500L, 50L, 1L) + // VPN received 3300 bytes over WiFi in background (SET_DEFAULT). + .addValues(TEST_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 3300L, 300L, 0L, 0L, 1L) // VPN sent 1650 bytes over WiFi in foreground (SET_FOREGROUND). .addValues(TEST_IFACE, UID_VPN, SET_FOREGROUND, TAG_NONE, 0L, 0L, 1650L, 150L, 1L)); forcePollAndWaitForIdle(); - assertUidTotal(sTemplateWifi, UID_RED, 1000L, 100L, 1000L, 100L, 1); - assertUidTotal(sTemplateWifi, UID_BLUE, 500L, 50L, 500L, 50L, 1); - assertUidTotal(sTemplateWifi, UID_VPN, 150L, 0L, 150L, 0L, 2); + assertUidTotal(sTemplateWifi, UID_RED, 2000L, 200L, 1000L, 100L, 1); + assertUidTotal(sTemplateWifi, UID_BLUE, 1000L, 100L, 500L, 50L, 1); + assertUidTotal(sTemplateWifi, UID_VPN, 300L, 0L, 150L, 0L, 2); } @Test @@ -1068,24 +1131,28 @@ public class NetworkStatsServiceTest { getActiveIface(networkStates)); // create some traffic (assume 10 bytes of MTU for VPN interface and 1 byte encryption // overhead per packet): - // 1000 bytes (100 packets) were sent/received by UID_RED over VPN. - // VPN sent/received 660 bytes (60 packets) over WiFi and 440 bytes (40 packets) over Cell. - // For UID_RED, expect 600 bytes attributed over WiFi and 400 bytes over Cell for both - // rx/tx. - // For UID_VPN, expect 60 bytes attributed over WiFi and 40 bytes over Cell for both rx/tx. + // 1000 bytes (100 packets) were sent, and 500 bytes (50 packets) received by UID_RED over + // VPN. + // VPN sent 660 bytes (60 packets) over WiFi and 440 bytes (40 packets) over Cell. + // And, it received 330 bytes (30 packets) over WiFi and 220 bytes (20 packets) over Cell. + // For UID_RED, expect 600 bytes attributed over WiFi and 400 bytes over Cell for sent (tx) + // traffic. For received (rx) traffic, expect 300 bytes over WiFi and 200 bytes over Cell. + // + // For UID_VPN, expect 60 bytes attributed over WiFi and 40 bytes over Cell for tx traffic. + // And, 30 bytes over WiFi and 20 bytes over Cell for rx traffic. incrementCurrentTime(HOUR_IN_MILLIS); expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 3) - .addValues(TUN_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 1000L, 100L, 1000L, 100L, 2L) - .addValues(TEST_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 660L, 60L, 660L, 60L, 1L) - .addValues(TEST_IFACE2, UID_VPN, SET_DEFAULT, TAG_NONE, 440L, 40L, 440L, 40L, 1L)); + .addValues(TUN_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 500L, 50L, 1000L, 100L, 2L) + .addValues(TEST_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 330L, 30L, 660L, 60L, 1L) + .addValues(TEST_IFACE2, UID_VPN, SET_DEFAULT, TAG_NONE, 220L, 20L, 440L, 40L, 1L)); forcePollAndWaitForIdle(); - assertUidTotal(sTemplateWifi, UID_RED, 600L, 60L, 600L, 60L, 1); - assertUidTotal(sTemplateWifi, UID_VPN, 60L, 0L, 60L, 0L, 1); + assertUidTotal(sTemplateWifi, UID_RED, 300L, 30L, 600L, 60L, 1); + assertUidTotal(sTemplateWifi, UID_VPN, 30L, 0L, 60L, 0L, 1); - assertUidTotal(buildTemplateMobileWildcard(), UID_RED, 400L, 40L, 400L, 40L, 1); - assertUidTotal(buildTemplateMobileWildcard(), UID_VPN, 40L, 0L, 40L, 0L, 1); + assertUidTotal(buildTemplateMobileWildcard(), UID_RED, 200L, 20L, 400L, 40L, 1); + assertUidTotal(buildTemplateMobileWildcard(), UID_VPN, 20L, 0L, 40L, 0L, 1); } @Test From 4e4efe42a1a1fbde776f4872b7470a836f9b12c4 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Wed, 29 May 2019 19:02:41 +0900 Subject: [PATCH 2/2] Add one more test for VPN usage stats. (cherry picked from commit 1419b0b20e4b079bfa66662d042b4dc35fb22d78) Covers the case where the majority of traffic through the VPN is caused by the VPN app itself, and ensures that that traffic is correctly attributed to the VPN app as opposed to spread between the other apps that use the VPN. Bug: 120145746 Test: atest NetworkStatsServiceTest Change-Id: Ibd7646dc088fa4180abd696e89c3148ff34ce190 Merged-In: Iffd3f95fc2e11d311691a797b010edb38d2ef3c6 --- .../server/net/NetworkStatsServiceTest.java | 46 ++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java index 80875732cf3ca..31df9f3bc9c51 100644 --- a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java @@ -61,7 +61,6 @@ import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; -import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -1030,6 +1029,51 @@ public class NetworkStatsServiceTest { assertUidTotal(sTemplateWifi, UID_VPN, 300L, 0L, 150L, 0L, 2); } + @Test + public void vpnWithOneUnderlyingIfaceAndOwnTraffic() throws Exception { + // WiFi network is connected and VPN is using WiFi (which has TEST_IFACE). + expectDefaultSettings(); + NetworkState[] networkStates = new NetworkState[] {buildWifiState(), buildVpnState()}; + VpnInfo[] vpnInfos = new VpnInfo[] {createVpnInfo(new String[] {TEST_IFACE})}; + expectNetworkStatsUidDetail(buildEmptyStats()); + expectBandwidthControlCheck(); + + mService.forceUpdateIfaces( + new Network[] {WIFI_NETWORK, VPN_NETWORK}, + vpnInfos, + networkStates, + getActiveIface(networkStates)); + // create some traffic (assume 10 bytes of MTU for VPN interface and 1 byte encryption + // overhead per packet): + // 1000 bytes (100 packets) were sent, and 2000 bytes (200 packets) were received by UID_RED + // over VPN. + // 500 bytes (50 packets) were sent, and 1000 bytes (100 packets) were received by UID_BLUE + // over VPN. + // Additionally, the VPN sends 6000 bytes (600 packets) of its own traffic into the tun + // interface (passing that traffic to the VPN endpoint), and receives 5000 bytes (500 + // packets) from it. Including overhead that is 6600/5500 bytes. + // VPN sent 8250 bytes (750 packets), and received 8800 (800 packets) over WiFi. + // Of 8250 bytes sent over WiFi, expect 1000 bytes attributed to UID_RED, 500 bytes + // attributed to UID_BLUE, and 6750 bytes attributed to UID_VPN. + // Of 8800 bytes received over WiFi, expect 2000 bytes attributed to UID_RED, 1000 bytes + // attributed to UID_BLUE, and 5800 bytes attributed to UID_VPN. + incrementCurrentTime(HOUR_IN_MILLIS); + expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 3) + .addValues(TUN_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 2000L, 200L, 1000L, 100L, 1L) + .addValues(TUN_IFACE, UID_BLUE, SET_DEFAULT, TAG_NONE, 1000L, 100L, 500L, 50L, 1L) + .addValues(TUN_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 5000L, 500L, 6000L, 600L, 1L) + // VPN received 8800 bytes over WiFi in background (SET_DEFAULT). + .addValues(TEST_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 8800L, 800L, 0L, 0L, 1L) + // VPN sent 8250 bytes over WiFi in foreground (SET_FOREGROUND). + .addValues(TEST_IFACE, UID_VPN, SET_FOREGROUND, TAG_NONE, 0L, 0L, 8250L, 750L, 1L)); + + forcePollAndWaitForIdle(); + + assertUidTotal(sTemplateWifi, UID_RED, 2000L, 200L, 1000L, 100L, 1); + assertUidTotal(sTemplateWifi, UID_BLUE, 1000L, 100L, 500L, 50L, 1); + assertUidTotal(sTemplateWifi, UID_VPN, 5800L, 500L, 6750L, 600L, 2); + } + @Test public void vpnWithOneUnderlyingIface_withCompression() throws Exception { // WiFi network is connected and VPN is using WiFi (which has TEST_IFACE).