From 01ca51a90067a9e79d2f11cec84c70816a952e03 Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Fri, 15 May 2020 03:50:20 +0000 Subject: [PATCH 1/2] Add methods for IKEv2/IPsec test mode profiles This change adds the ability for IKEv2/IPsec VPN profiles to run on Test Networks. If enabled, the IKEv2/IPsec VPN profiles will request ONLY test networks. Bug: 148582947 Test: FrameworksNetTests passing Test: Added for testing Change-Id: I2511b39b27a1e29ae97907cdb12728d13fb1628f Merged-In: I2511b39b27a1e29ae97907cdb12728d13fb1628f (cherry picked from commit 9e18eeb4a676ef3afc4bc510f18f81213b8edc55) --- .../android/internal/net/VpnProfileTest.java | 47 ++++++++++++++++--- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/tests/net/java/com/android/internal/net/VpnProfileTest.java b/tests/net/java/com/android/internal/net/VpnProfileTest.java index ceca6f0288..e5daa71c30 100644 --- a/tests/net/java/com/android/internal/net/VpnProfileTest.java +++ b/tests/net/java/com/android/internal/net/VpnProfileTest.java @@ -33,7 +33,9 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import java.util.ArrayList; import java.util.Arrays; +import java.util.List; /** Unit tests for {@link VpnProfile}. */ @SmallTest @@ -41,6 +43,9 @@ import java.util.Arrays; public class VpnProfileTest { private static final String DUMMY_PROFILE_KEY = "Test"; + private static final int ENCODED_INDEX_AUTH_PARAMS_INLINE = 23; + private static final int ENCODED_INDEX_RESTRICTED_TO_TEST_NETWORKS = 24; + @Test public void testDefaults() throws Exception { final VpnProfile p = new VpnProfile(DUMMY_PROFILE_KEY); @@ -67,10 +72,11 @@ public class VpnProfileTest { assertFalse(p.isMetered); assertEquals(1360, p.maxMtu); assertFalse(p.areAuthParamsInline); + assertFalse(p.isRestrictedToTestNetworks); } private VpnProfile getSampleIkev2Profile(String key) { - final VpnProfile p = new VpnProfile(key); + final VpnProfile p = new VpnProfile(key, true /* isRestrictedToTestNetworks */); p.name = "foo"; p.type = VpnProfile.TYPE_IKEV2_IPSEC_USER_PASS; @@ -116,7 +122,7 @@ public class VpnProfileTest { @Test public void testParcelUnparcel() { - assertParcelSane(getSampleIkev2Profile(DUMMY_PROFILE_KEY), 22); + assertParcelSane(getSampleIkev2Profile(DUMMY_PROFILE_KEY), 23); } @Test @@ -159,14 +165,41 @@ public class VpnProfileTest { assertNull(VpnProfile.decode(DUMMY_PROFILE_KEY, tooManyValues)); } + private String getEncodedDecodedIkev2ProfileMissingValues(int... missingIndices) { + // Sort to ensure when we remove, we can do it from greatest first. + Arrays.sort(missingIndices); + + final String encoded = new String(getSampleIkev2Profile(DUMMY_PROFILE_KEY).encode()); + final List parts = + new ArrayList<>(Arrays.asList(encoded.split(VpnProfile.VALUE_DELIMITER))); + + // Remove from back first to ensure indexing is consistent. + for (int i = missingIndices.length - 1; i >= 0; i--) { + parts.remove(missingIndices[i]); + } + + return String.join(VpnProfile.VALUE_DELIMITER, parts.toArray(new String[0])); + } + @Test public void testEncodeDecodeInvalidNumberOfValues() { - final VpnProfile profile = getSampleIkev2Profile(DUMMY_PROFILE_KEY); - final String encoded = new String(profile.encode()); - final byte[] tooFewValues = - encoded.substring(0, encoded.lastIndexOf(VpnProfile.VALUE_DELIMITER)).getBytes(); + final String tooFewValues = + getEncodedDecodedIkev2ProfileMissingValues( + ENCODED_INDEX_AUTH_PARAMS_INLINE, + ENCODED_INDEX_RESTRICTED_TO_TEST_NETWORKS /* missingIndices */); - assertNull(VpnProfile.decode(DUMMY_PROFILE_KEY, tooFewValues)); + assertNull(VpnProfile.decode(DUMMY_PROFILE_KEY, tooFewValues.getBytes())); + } + + @Test + public void testEncodeDecodeMissingIsRestrictedToTestNetworks() { + final String tooFewValues = + getEncodedDecodedIkev2ProfileMissingValues( + ENCODED_INDEX_RESTRICTED_TO_TEST_NETWORKS /* missingIndices */); + + // Verify decoding without isRestrictedToTestNetworks defaults to false + final VpnProfile decoded = VpnProfile.decode(DUMMY_PROFILE_KEY, tooFewValues.getBytes()); + assertFalse(decoded.isRestrictedToTestNetworks); } @Test From 80aec1d4ff092adb4c7c8406b168e3964dab441d Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Fri, 22 May 2020 00:28:02 +0000 Subject: [PATCH 2/2] Forward unknown Data Stall types to Connectivity Diagnostics. This CL forwards suspected Data Stall events detected with unknown detection methods to ConnectivityDiagnostics. Currently, ConnectivityService drops any data stall events with unknown detection methods, which leads to false negatives for Connectivity Diagnostics registrants. This change ensures that registrants will still be notified as NetworkStack is updated to use new detection methods. The documentation for ConnectivityDiagnosticsManager#DataStallReport is also updated to reflect that the detection methods included in the report are a bit mask of detection methods used. Implicitly, this means that data stalls detected via unknown methods will have an empty bit mask (0x00). Bug: 156294356 Test: atest ConnectivityDiagnosticsManager Change-Id: I62d0bf91fcc17c7921afd519c72551399906bd6b Merged-In: I62d0bf91fcc17c7921afd519c72551399906bd6b (cherry picked from commit a1d9d811a05bf3447ebb90a39343b53eee79f0db) --- .../net/ConnectivityDiagnosticsManager.java | 6 +-- .../android/server/ConnectivityService.java | 51 +++++++++++-------- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/core/java/android/net/ConnectivityDiagnosticsManager.java b/core/java/android/net/ConnectivityDiagnosticsManager.java index 9086d49231..275e38c744 100644 --- a/core/java/android/net/ConnectivityDiagnosticsManager.java +++ b/core/java/android/net/ConnectivityDiagnosticsManager.java @@ -437,7 +437,7 @@ public class ConnectivityDiagnosticsManager { */ private long mReportTimestamp; - /** The detection method used to identify the suspected data stall */ + /** A bitmask of the detection methods used to identify the suspected data stall */ @DetectionMethod private final int mDetectionMethod; /** LinkProperties available on the Network at the reported timestamp */ @@ -499,9 +499,9 @@ public class ConnectivityDiagnosticsManager { } /** - * Returns the detection method used to identify this suspected data stall. + * Returns the bitmask of detection methods used to identify this suspected data stall. * - * @return The detection method used to identify the suspected data stall + * @return The bitmask of detection methods used to identify the suspected data stall */ public int getDetectionMethod() { return mDetectionMethod; diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 7c61ba2e7a..1e9a9d85e0 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -3102,30 +3102,24 @@ public class ConnectivityService extends IConnectivityManager.Stub } private void notifyDataStallSuspected(DataStallReportParcelable p, int netId) { + log("Data stall detected with methods: " + p.detectionMethod); + final PersistableBundle extras = new PersistableBundle(); - switch (p.detectionMethod) { - case DETECTION_METHOD_DNS_EVENTS: - extras.putInt(KEY_DNS_CONSECUTIVE_TIMEOUTS, p.dnsConsecutiveTimeouts); - break; - case DETECTION_METHOD_TCP_METRICS: - extras.putInt(KEY_TCP_PACKET_FAIL_RATE, p.tcpPacketFailRate); - extras.putInt(KEY_TCP_METRICS_COLLECTION_PERIOD_MILLIS, - p.tcpMetricsCollectionPeriodMillis); - break; - default: - // TODO(b/156294356): update for new data stall detection methods - log("Unknown data stall detection method, ignoring: " + p.detectionMethod); - return; + int detectionMethod = 0; + if (hasDataStallDetectionMethod(p, DETECTION_METHOD_DNS_EVENTS)) { + extras.putInt(KEY_DNS_CONSECUTIVE_TIMEOUTS, p.dnsConsecutiveTimeouts); + detectionMethod |= DETECTION_METHOD_DNS_EVENTS; + } + if (hasDataStallDetectionMethod(p, DETECTION_METHOD_TCP_METRICS)) { + extras.putInt(KEY_TCP_PACKET_FAIL_RATE, p.tcpPacketFailRate); + extras.putInt(KEY_TCP_METRICS_COLLECTION_PERIOD_MILLIS, + p.tcpMetricsCollectionPeriodMillis); + detectionMethod |= DETECTION_METHOD_TCP_METRICS; } - notifyDataStallSuspected(p.detectionMethod, netId, p.timestampMillis, extras); - } - - private void notifyDataStallSuspected(int detectionMethod, int netId, long timestampMillis, - @NonNull PersistableBundle extras) { final Message msg = mConnectivityDiagnosticsHandler.obtainMessage( ConnectivityDiagnosticsHandler.EVENT_DATA_STALL_SUSPECTED, detectionMethod, netId, - timestampMillis); + p.timestampMillis); msg.setData(new Bundle(extras)); // NetworkStateTrackerHandler currently doesn't take any actions based on data @@ -3134,6 +3128,10 @@ public class ConnectivityService extends IConnectivityManager.Stub mConnectivityDiagnosticsHandler.sendMessage(msg); } + private boolean hasDataStallDetectionMethod(DataStallReportParcelable p, int detectionMethod) { + return (p.detectionMethod & detectionMethod) != 0; + } + private boolean networkRequiresPrivateDnsValidation(NetworkAgentInfo nai) { return isPrivateDnsValidationRequired(nai.networkCapabilities); } @@ -8189,6 +8187,19 @@ public class ConnectivityService extends IConnectivityManager.Stub + "creators"); } - notifyDataStallSuspected(detectionMethod, network.netId, timestampMillis, extras); + final DataStallReportParcelable p = new DataStallReportParcelable(); + p.timestampMillis = timestampMillis; + p.detectionMethod = detectionMethod; + + if (hasDataStallDetectionMethod(p, DETECTION_METHOD_DNS_EVENTS)) { + p.dnsConsecutiveTimeouts = extras.getInt(KEY_DNS_CONSECUTIVE_TIMEOUTS); + } + if (hasDataStallDetectionMethod(p, DETECTION_METHOD_TCP_METRICS)) { + p.tcpPacketFailRate = extras.getInt(KEY_TCP_PACKET_FAIL_RATE); + p.tcpMetricsCollectionPeriodMillis = extras.getInt( + KEY_TCP_METRICS_COLLECTION_PERIOD_MILLIS); + } + + notifyDataStallSuspected(p, network.netId); } }