From e1549ae364cc368641994f478959b8736a183f8e Mon Sep 17 00:00:00 2001 From: Robert Greenwalt Date: Fri, 7 Feb 2014 03:52:12 -0800 Subject: 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: I80912cc08ffa1e4b63008c94630006cf316e7a64 --- core/java/android/net/LinkProperties.java | 20 +++++++++++ services/java/com/android/server/WifiService.java | 12 +++++++ wifi/java/android/net/wifi/WifiConfiguration.java | 43 +++++++++++++++++++++++ wifi/java/android/net/wifi/WifiStateMachine.java | 3 ++ 4 files changed, 78 insertions(+) diff --git a/core/java/android/net/LinkProperties.java b/core/java/android/net/LinkProperties.java index 75646fdf9090..bf411cccd5c9 100644 --- a/core/java/android/net/LinkProperties.java +++ b/core/java/android/net/LinkProperties.java @@ -112,6 +112,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); } @@ -127,6 +137,16 @@ public class LinkProperties implements Parcelable { return Collections.unmodifiableCollection(mRoutes); } + /** + * 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/WifiService.java b/services/java/com/android/server/WifiService.java index 98794c923f7c..eba365acbc6b 100644 --- a/services/java/com/android/server/WifiService.java +++ b/services/java/com/android/server/WifiService.java @@ -43,6 +43,7 @@ import android.net.wifi.WpsInfo; import android.net.wifi.WpsResult; import android.net.ConnectivityManager; import android.net.DhcpInfo; +import android.net.LinkProperties; import android.net.NetworkInfo; import android.net.NetworkInfo.State; import android.net.NetworkInfo.DetailedState; @@ -762,6 +763,17 @@ public 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 c4fe1b4b35fb..4b5aa9cbcab3 100644 --- a/wifi/java/android/net/wifi/WifiConfiguration.java +++ b/wifi/java/android/net/wifi/WifiConfiguration.java @@ -16,11 +16,16 @@ 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 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 @@ -615,6 +620,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 040ff248f59e..6deda5b11c42 100644 --- a/wifi/java/android/net/wifi/WifiStateMachine.java +++ b/wifi/java/android/net/wifi/WifiStateMachine.java @@ -1606,9 +1606,11 @@ public class WifiStateMachine extends StateMachine { private void configureLinkProperties() { if (mWifiConfigStore.isUsingStaticIp(mLastNetworkId)) { mLinkProperties = mWifiConfigStore.getLinkProperties(mLastNetworkId); + mLinkProperties = WifiConfiguration.stripUndisplayableConfig(mLinkProperties); } else { synchronized (mDhcpInfoInternal) { mLinkProperties = mDhcpInfoInternal.makeLinkProperties(); + mLinkProperties = WifiConfiguration.stripUndisplayableConfig(mLinkProperties); } mLinkProperties.setHttpProxy(mWifiConfigStore.getProxyProperties(mLastNetworkId)); } @@ -1816,6 +1818,7 @@ public class WifiStateMachine extends StateMachine { //DHCP renewal in connected state LinkProperties linkProperties = dhcpInfoInternal.makeLinkProperties(); linkProperties.setHttpProxy(mWifiConfigStore.getProxyProperties(mLastNetworkId)); + linkProperties = WifiConfiguration.stripUndisplayableConfig(linkProperties); linkProperties.setInterfaceName(mInterfaceName); if (!linkProperties.equals(mLinkProperties)) { if (DBG) { -- cgit v1.2.3-59-g8ed1b