From 02c7abac856c3e94f4a2714d673cefb65c55efb7 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Thu, 16 Oct 2014 00:55:07 +0900 Subject: Don't make lockdown VPN source firewall rules over-broad. Currently, the lockdown VPN adds firewall allow rules matching the whole subnet that the server assigned, so for example if the VPN server assigns it the IP address 10.1.23.5/8, it will allow the whole of 10.0.0.0/8 to pass the firewall. This is needlessly overbroad and has a particularly bad corner case where if the prefix length is 0, everything is allowed. Bug: 17695048 Change-Id: Idbec4b3aea0f72f9bdfd26dcd72d6a97d026fb12 --- .../core/java/com/android/server/net/LockdownVpnTracker.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'services') diff --git a/services/core/java/com/android/server/net/LockdownVpnTracker.java b/services/core/java/com/android/server/net/LockdownVpnTracker.java index e9c7751..cf0aba4 100644 --- a/services/core/java/com/android/server/net/LockdownVpnTracker.java +++ b/services/core/java/com/android/server/net/LockdownVpnTracker.java @@ -190,7 +190,7 @@ public class LockdownVpnTracker { mNetService.setFirewallInterfaceRule(iface, true); for (LinkAddress addr : sourceAddrs) { - mNetService.setFirewallEgressSourceRule(addr.toString(), true); + setFirewallEgressSourceRule(addr, true); } mErrorCount = 0; @@ -277,7 +277,7 @@ public class LockdownVpnTracker { } if (mAcceptedSourceAddr != null) { for (LinkAddress addr : mAcceptedSourceAddr) { - mNetService.setFirewallEgressSourceRule(addr.toString(), false); + setFirewallEgressSourceRule(addr, false); } mAcceptedSourceAddr = null; } @@ -286,6 +286,14 @@ public class LockdownVpnTracker { } } + private void setFirewallEgressSourceRule( + LinkAddress address, boolean allow) throws RemoteException { + // Our source address based firewall rules must only cover our own source address, not the + // whole subnet + final String addrString = address.getAddress().getHostAddress(); + mNetService.setFirewallEgressSourceRule(addrString, allow); + } + public void onNetworkInfoChanged() { synchronized (mStateLock) { handleStateChangedLocked(); -- cgit v1.1