From ab4f831229ea368def923521373febdee1ab7dd1 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 8031acad0e8059020df23a8258b7c2e0ab2bbd8b) 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 --- .../android/server/ConnectivityService.java | 4 +- .../server/net/NetworkStatsServiceTest.java | 115 ++++++++++++++---- 2 files changed, 93 insertions(+), 26 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index cdab8c6ce0..f25bb59203 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 07b5ba4624..80875732cf 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 e13b3791a0bcaeaa474452ecc2f073e4e8187dfd 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 54bb4f4608d91f9c6f4bb1cbfeaa9ac589560cd9) 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 80875732cf..31df9f3bc9 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).