From 950230663fb3a9af438ec2ee57605fc9e7a58428 Mon Sep 17 00:00:00 2001 From: Robert Greenwalt Date: Fri, 7 Feb 2014 03:52:12 -0800 Subject: [PATCH] DO NOT MERGE Sanitize WifiConfigs Do this both on input from apps (giving error) and between wifi and ConnectivityService (ignoring bad data). This means removing all addresses beyond the first and all routes but the first default and the implied direct-connect routes. We do this because the user can't monitor the others (no UI), their support wasn't intended, they allow redirection of all traffic without user knowledge and they allow circumvention of legacy VPNs. This should not move forward from JB as it breaks IPv6 and K has a more resilient VPN. Bug:12663469 Change-Id: I0d92db7efc30a1bb3e5b8c6e5595bdb9793a16f2 Conflicts: core/java/android/net/LinkProperties.java services/java/com/android/server/WifiService.java wifi/java/android/net/wifi/WifiStateMachine.java --- core/java/android/net/LinkProperties.java | 20 +++++++++ .../com/android/server/wifi/WifiService.java | 12 ++++++ .../android/net/wifi/WifiConfiguration.java | 43 +++++++++++++++++++ .../android/net/wifi/WifiStateMachine.java | 5 ++- 4 files changed, 79 insertions(+), 1 deletion(-) diff --git a/core/java/android/net/LinkProperties.java b/core/java/android/net/LinkProperties.java index 75f8b5948b815..dc9a54f8ea757 100644 --- a/core/java/android/net/LinkProperties.java +++ b/core/java/android/net/LinkProperties.java @@ -144,6 +144,16 @@ public class LinkProperties implements Parcelable { return Collections.unmodifiableCollection(mLinkAddresses); } + /** + * Replaces the LinkAddresses on this link with the given collection of addresses + */ + public void setLinkAddresses(Collection addresses) { + mLinkAddresses.clear(); + for (LinkAddress address: addresses) { + addLinkAddress(address); + } + } + public void addDns(InetAddress dns) { if (dns != null) mDnses.add(dns); } @@ -198,6 +208,16 @@ public class LinkProperties implements Parcelable { return routes; } + /** + * Replaces the RouteInfos on this link with the given collection of RouteInfos. + */ + public void setRoutes(Collection routes) { + mRoutes.clear(); + for (RouteInfo route : routes) { + addRoute(route); + } + } + public void setHttpProxy(ProxyProperties proxy) { mHttpProxy = proxy; } diff --git a/services/java/com/android/server/wifi/WifiService.java b/services/java/com/android/server/wifi/WifiService.java index a70978ee6c4b5..851c00f25c3ca 100644 --- a/services/java/com/android/server/wifi/WifiService.java +++ b/services/java/com/android/server/wifi/WifiService.java @@ -35,6 +35,7 @@ import android.net.wifi.WifiWatchdogStateMachine; import android.net.DhcpInfo; import android.net.DhcpResults; import android.net.LinkAddress; +import android.net.LinkProperties; import android.net.NetworkUtils; import android.net.RouteInfo; import android.os.Binder; @@ -470,6 +471,17 @@ public final class WifiService extends IWifiManager.Stub { */ public int addOrUpdateNetwork(WifiConfiguration config) { enforceChangePermission(); + // Until we have better UI so the user knows what's up we can't support undisplayable + // things (it's a security hole). Even when we can support it we probably need + // to lock down who can modify what. TODO - remove this when addOrUpdateNetwork + // restricts callers AND when the UI in settings lets users view the data AND + // when the VPN code is immune to specific routes. + if (config != null) { + LinkProperties lp = config.linkProperties; + if (lp == null || lp.equals(WifiConfiguration.stripUndisplayableConfig(lp)) == false) { + return -1; + } + } if (mWifiStateMachineChannel != null) { return mWifiStateMachine.syncAddOrUpdateNetwork(mWifiStateMachineChannel, config); } else { diff --git a/wifi/java/android/net/wifi/WifiConfiguration.java b/wifi/java/android/net/wifi/WifiConfiguration.java index b971fc3387352..ab4cfa5849aa7 100644 --- a/wifi/java/android/net/wifi/WifiConfiguration.java +++ b/wifi/java/android/net/wifi/WifiConfiguration.java @@ -16,12 +16,17 @@ package android.net.wifi; +import android.net.LinkAddress; import android.net.LinkProperties; +import android.net.RouteInfo; import android.os.Parcelable; import android.os.Parcel; import android.text.TextUtils; +import java.util.ArrayList; import java.util.BitSet; +import java.util.Collection; +import java.util.Iterator; /** * A class representing a configured Wi-Fi network, including the @@ -579,6 +584,44 @@ public class WifiConfiguration implements Parcelable { } } + /** + * We don't want to use routes other than the first default and + * correct direct-connect route, or addresses beyond the first as + * the user can't see them in the UI and malicious apps + * can do malicious things with them. In particular specific routes + * circumvent VPNs of this era. + * + * @hide + */ + public static LinkProperties stripUndisplayableConfig(LinkProperties lp) { + if (lp == null) return lp; + + LinkProperties newLp = new LinkProperties(lp); + Iterator i = lp.getLinkAddresses().iterator(); + RouteInfo directConnectRoute = null; + if (i.hasNext()) { + LinkAddress addr = i.next(); + Collection newAddresses = new ArrayList(1); + newAddresses.add(addr); + newLp.setLinkAddresses(newAddresses); + directConnectRoute = new RouteInfo(addr,null); + } + boolean defaultAdded = false; + Collection routes = lp.getRoutes(); + Collection newRoutes = new ArrayList(2); + for (RouteInfo route : routes) { + if (defaultAdded == false && route.isDefaultRoute()) { + newRoutes.add(route); + defaultAdded = true; + } + if (route.equals(directConnectRoute)) { + newRoutes.add(route); + } + } + newLp.setRoutes(newRoutes); + return newLp; + } + /** Implement the Parcelable interface {@hide} */ public void writeToParcel(Parcel dest, int flags) { dest.writeInt(networkId); diff --git a/wifi/java/android/net/wifi/WifiStateMachine.java b/wifi/java/android/net/wifi/WifiStateMachine.java index 6589ff5a83366..db8e39a72f741 100644 --- a/wifi/java/android/net/wifi/WifiStateMachine.java +++ b/wifi/java/android/net/wifi/WifiStateMachine.java @@ -1586,10 +1586,12 @@ public class WifiStateMachine extends StateMachine { private void configureLinkProperties() { if (mWifiConfigStore.isUsingStaticIp(mLastNetworkId)) { mLinkProperties = mWifiConfigStore.getLinkProperties(mLastNetworkId); + mLinkProperties = WifiConfiguration.stripUndisplayableConfig(mLinkProperties); } else { synchronized (mDhcpResultsLock) { if ((mDhcpResults != null) && (mDhcpResults.linkProperties != null)) { - mLinkProperties = mDhcpResults.linkProperties; + mLinkProperties = WifiConfiguration.stripUndisplayableConfig( + mDhcpResults.linkProperties); } } mLinkProperties.setHttpProxy(mWifiConfigStore.getProxyProperties(mLastNetworkId)); @@ -1830,6 +1832,7 @@ public class WifiStateMachine extends StateMachine { if (getNetworkDetailedState() == DetailedState.CONNECTED) { //DHCP renewal in connected state linkProperties.setHttpProxy(mWifiConfigStore.getProxyProperties(mLastNetworkId)); + linkProperties = WifiConfiguration.stripUndisplayableConfig(linkProperties); if (!linkProperties.equals(mLinkProperties)) { if (DBG) { log("Link configuration changed for netId: " + mLastNetworkId