From 3a0aaa681f213478ca3e5f2789d5a44143354cb7 Mon Sep 17 00:00:00 2001 From: Etan Cohen Date: Sun, 30 Dec 2018 17:59:59 -0800 Subject: [PATCH] [CS] Add an option to block sensitive network specifier Network specifiers are used for 2 purposes: - As part of network requests to specify more information on the type of requested networks. - On network agents to specify information about their networks. The network specifiers of the requests and agents are matched to each other. However, the agent network specifier may contain sensitive information which we do not want forwarded to any app. This CL adds an option to strip out this agent network specifier before the network capabilities are forwarded to the app. Bug: 161853197 Bug: 161370134 Test: atest ConnectivityServiceTest (frameworks/base/tests/net) Test: atest frameworks/base/tests/net Test: atest frameworks/opt/net/wifi/tests/wifitests Test: atest frameworks/opt/telephony/tests/telephonytests Test: atest frameworks/opt/net/ethernet/tests Test: atest android.net.cts - some flakiness! Test: act.py ThroughputTest Test: act.py DataPathTest Test: atest SingleDeviceTest (cts) Change-Id: I38ed3ff88532ef522ab167c88d87e6e82295ffc5 Merged-In: If08d312ff814bdde1147518f923199e6349503d5 --- .../android/server/ConnectivityService.java | 6 +- .../server/ConnectivityServiceTest.java | 109 ++++++++++++++++-- 2 files changed, 103 insertions(+), 12 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index b71fabae7f..5602d6f9ac 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1433,6 +1433,9 @@ public class ConnectivityService extends IConnectivityManager.Stub newNc.setUids(null); newNc.setSSID(null); } + if (newNc.getNetworkSpecifier() != null) { + newNc.setNetworkSpecifier(newNc.getNetworkSpecifier().redact()); + } return newNc; } @@ -5145,7 +5148,8 @@ public class ConnectivityService extends IConnectivityManager.Stub } switch (notificationType) { case ConnectivityManager.CALLBACK_AVAILABLE: { - putParcelable(bundle, new NetworkCapabilities(networkAgent.networkCapabilities)); + putParcelable(bundle, networkCapabilitiesRestrictedForCallerPermissions( + networkAgent.networkCapabilities, nri.mPid, nri.mUid)); putParcelable(bundle, new LinkProperties(networkAgent.linkProperties)); break; } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 6ceed53f4e..719ce01eca 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -113,7 +113,6 @@ import android.net.NetworkSpecifier; import android.net.NetworkState; import android.net.NetworkUtils; import android.net.RouteInfo; -import android.net.StringNetworkSpecifier; import android.net.UidRange; import android.net.VpnService; import android.net.captiveportal.CaptivePortalProbeResult; @@ -135,6 +134,7 @@ import android.support.test.InstrumentationRegistry; import android.support.test.filters.SmallTest; import android.support.test.runner.AndroidJUnit4; import android.test.mock.MockContentResolver; +import android.text.TextUtils; import android.util.ArraySet; import android.util.Log; @@ -2495,16 +2495,76 @@ public class ConnectivityServiceTest { return new NetworkRequest.Builder().addTransportType(TRANSPORT_WIFI); } + /** + * Verify request matching behavior with network specifiers. + * + * Note: this test is somewhat problematic since it involves removing capabilities from + * agents - i.e. agents rejecting requests which they previously accepted. This is flagged + * as a WTF bug in + * {@link ConnectivityService#mixInCapabilities(NetworkAgentInfo, NetworkCapabilities)} but + * does work. + */ @Test public void testNetworkSpecifier() { + // A NetworkSpecifier subclass that matches all networks but must not be visible to apps. + class ConfidentialMatchAllNetworkSpecifier extends NetworkSpecifier implements + Parcelable { + @Override + public boolean satisfiedBy(NetworkSpecifier other) { + return true; + } + + @Override + public int describeContents() { + return 0; + } + + @Override + public void writeToParcel(Parcel dest, int flags) {} + + @Override + public NetworkSpecifier redact() { + return null; + } + } + + // A network specifier that matches either another LocalNetworkSpecifier with the same + // string or a ConfidentialMatchAllNetworkSpecifier, and can be passed to apps as is. + class LocalStringNetworkSpecifier extends NetworkSpecifier implements Parcelable { + private String mString; + + LocalStringNetworkSpecifier(String string) { + mString = string; + } + + @Override + public boolean satisfiedBy(NetworkSpecifier other) { + if (other instanceof LocalStringNetworkSpecifier) { + return TextUtils.equals(mString, + ((LocalStringNetworkSpecifier) other).mString); + } + if (other instanceof ConfidentialMatchAllNetworkSpecifier) return true; + return false; + } + + @Override + public int describeContents() { + return 0; + } + @Override + public void writeToParcel(Parcel dest, int flags) {} + } + + NetworkRequest rEmpty1 = newWifiRequestBuilder().build(); NetworkRequest rEmpty2 = newWifiRequestBuilder().setNetworkSpecifier((String) null).build(); NetworkRequest rEmpty3 = newWifiRequestBuilder().setNetworkSpecifier("").build(); NetworkRequest rEmpty4 = newWifiRequestBuilder().setNetworkSpecifier( (NetworkSpecifier) null).build(); - NetworkRequest rFoo = newWifiRequestBuilder().setNetworkSpecifier("foo").build(); + NetworkRequest rFoo = newWifiRequestBuilder().setNetworkSpecifier( + new LocalStringNetworkSpecifier("foo")).build(); NetworkRequest rBar = newWifiRequestBuilder().setNetworkSpecifier( - new StringNetworkSpecifier("bar")).build(); + new LocalStringNetworkSpecifier("bar")).build(); TestNetworkCallback cEmpty1 = new TestNetworkCallback(); TestNetworkCallback cEmpty2 = new TestNetworkCallback(); @@ -2513,7 +2573,7 @@ public class ConnectivityServiceTest { TestNetworkCallback cFoo = new TestNetworkCallback(); TestNetworkCallback cBar = new TestNetworkCallback(); TestNetworkCallback[] emptyCallbacks = new TestNetworkCallback[] { - cEmpty1, cEmpty2, cEmpty3 }; + cEmpty1, cEmpty2, cEmpty3, cEmpty4 }; mCm.registerNetworkCallback(rEmpty1, cEmpty1); mCm.registerNetworkCallback(rEmpty2, cEmpty2); @@ -2522,6 +2582,9 @@ public class ConnectivityServiceTest { mCm.registerNetworkCallback(rFoo, cFoo); mCm.registerNetworkCallback(rBar, cBar); + LocalStringNetworkSpecifier nsFoo = new LocalStringNetworkSpecifier("foo"); + LocalStringNetworkSpecifier nsBar = new LocalStringNetworkSpecifier("bar"); + mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI); mWiFiNetworkAgent.connect(false); cEmpty1.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); @@ -2530,30 +2593,54 @@ public class ConnectivityServiceTest { cEmpty4.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); assertNoCallbacks(cFoo, cBar); - mWiFiNetworkAgent.setNetworkSpecifier(new StringNetworkSpecifier("foo")); + mWiFiNetworkAgent.setNetworkSpecifier(nsFoo); cFoo.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); for (TestNetworkCallback c: emptyCallbacks) { - c.expectCallback(CallbackState.NETWORK_CAPABILITIES, mWiFiNetworkAgent); + c.expectCapabilitiesLike((caps) -> caps.getNetworkSpecifier().equals(nsFoo), + mWiFiNetworkAgent); } - cFoo.expectCallback(CallbackState.NETWORK_CAPABILITIES, mWiFiNetworkAgent); + cFoo.expectCapabilitiesLike((caps) -> caps.getNetworkSpecifier().equals(nsFoo), + mWiFiNetworkAgent); + assertEquals(nsFoo, + mCm.getNetworkCapabilities(mWiFiNetworkAgent.getNetwork()).getNetworkSpecifier()); cFoo.assertNoCallback(); - mWiFiNetworkAgent.setNetworkSpecifier(new StringNetworkSpecifier("bar")); + mWiFiNetworkAgent.setNetworkSpecifier(nsBar); cFoo.expectCallback(CallbackState.LOST, mWiFiNetworkAgent); cBar.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); for (TestNetworkCallback c: emptyCallbacks) { - c.expectCallback(CallbackState.NETWORK_CAPABILITIES, mWiFiNetworkAgent); + c.expectCapabilitiesLike((caps) -> caps.getNetworkSpecifier().equals(nsBar), + mWiFiNetworkAgent); } - cBar.expectCallback(CallbackState.NETWORK_CAPABILITIES, mWiFiNetworkAgent); + cBar.expectCapabilitiesLike((caps) -> caps.getNetworkSpecifier().equals(nsBar), + mWiFiNetworkAgent); + assertEquals(nsBar, + mCm.getNetworkCapabilities(mWiFiNetworkAgent.getNetwork()).getNetworkSpecifier()); + cBar.assertNoCallback(); + + mWiFiNetworkAgent.setNetworkSpecifier(new ConfidentialMatchAllNetworkSpecifier()); + cFoo.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); + for (TestNetworkCallback c : emptyCallbacks) { + c.expectCapabilitiesLike((caps) -> caps.getNetworkSpecifier() == null, + mWiFiNetworkAgent); + } + cFoo.expectCapabilitiesLike((caps) -> caps.getNetworkSpecifier() == null, + mWiFiNetworkAgent); + cBar.expectCapabilitiesLike((caps) -> caps.getNetworkSpecifier() == null, + mWiFiNetworkAgent); + assertNull( + mCm.getNetworkCapabilities(mWiFiNetworkAgent.getNetwork()).getNetworkSpecifier()); + cFoo.assertNoCallback(); cBar.assertNoCallback(); mWiFiNetworkAgent.setNetworkSpecifier(null); + cFoo.expectCallback(CallbackState.LOST, mWiFiNetworkAgent); cBar.expectCallback(CallbackState.LOST, mWiFiNetworkAgent); for (TestNetworkCallback c: emptyCallbacks) { c.expectCallback(CallbackState.NETWORK_CAPABILITIES, mWiFiNetworkAgent); } - assertNoCallbacks(cEmpty1, cEmpty2, cEmpty3, cFoo, cBar); + assertNoCallbacks(cEmpty1, cEmpty2, cEmpty3, cEmpty4, cFoo, cBar); } @Test