LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Block unpriviledged apps which create keepalives with null fd

Browse Source 
Currently, socketKeepalive implementation is accepting null fd
due to backward compatibility with legacy packet keepalive API.
However, due to lack of the fd, the service cannot guarantee the
port is not reused by another app if the caller release the port
for any reason.

Thus, grant the null fd access only for priviledged apps.

This commit also address some comments from aosp/918533.

Bug: 126699232
Test: atest FrameworksNetTests
Change-Id: I0baf582ff4ca8af6082c3754e8dfbcd867f39792
This commit is contained in:
junyulai
2019-03-27 11:00:37 +08:00
parent 8b381dd91b
commit 828dad188c
3 changed files with 27 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
core/java/android/net/ConnectivityManager.java
View File

@@ -1934,6 +1934,8 @@ public class ConnectivityManager {
@NonNull Callback callback) {
ParcelFileDescriptor dup;
try {
// Dup is needed here as the pfd inside the socket is owned by the IpSecService,
// which cannot be obtained by the app process.
dup = ParcelFileDescriptor.dup(socket.getFileDescriptor());
} catch (IOException ignored) {
// Construct an invalid fd, so that if the user later calls start(), it will fail with
@@ -1975,6 +1977,7 @@ public class ConnectivityManager {
@NonNull Callback callback) {
ParcelFileDescriptor dup;
try {
// TODO: Consider remove unnecessary dup.
dup = pfd.dup();
} catch (IOException ignored) {
// Construct an invalid fd, so that if the user later calls start(), it will fail with

17
services/core/java/com/android/server/connectivity/KeepaliveTracker.java
View File

@@ -154,12 +154,19 @@ public class KeepaliveTracker {
// keepalives are sent cannot be reused by another app even if the fd gets closed by
// the user. A null is acceptable here for backward compatibility of PacketKeepalive
// API.
// TODO: don't accept null fd after legacy packetKeepalive API is removed.
try {
if (fd != null) {
mFd = Os.dup(fd);
} else {
Log.d(TAG, "uid/pid " + mUid + "/" + mPid + " calls with null fd");
Log.d(TAG, toString() + " calls with null fd");
if (!mPrivileged) {
throw new SecurityException(
"null fd is not allowed for unprivileged access.");
}
if (mType == TYPE_TCP) {
throw new IllegalArgumentException(
"null fd is not allowed for tcp socket keepalives.");
}
mFd = null;
}
} catch (ErrnoException e) {
@@ -531,7 +538,8 @@ public class KeepaliveTracker {
try {
ki = new KeepaliveInfo(cb, nai, packet, intervalSeconds,
KeepaliveInfo.TYPE_NATT, fd);
} catch (InvalidSocketException e) {
} catch (InvalidSocketException | IllegalArgumentException | SecurityException e) {
Log.e(TAG, "Fail to construct keepalive", e);
notifyErrorCallback(cb, ERROR_INVALID_SOCKET);
return;
}
@@ -570,7 +578,8 @@ public class KeepaliveTracker {
try {
ki = new KeepaliveInfo(cb, nai, packet, intervalSeconds,
KeepaliveInfo.TYPE_TCP, fd);
} catch (InvalidSocketException e) {
} catch (InvalidSocketException | IllegalArgumentException | SecurityException e) {
Log.e(TAG, "Fail to construct keepalive e=" + e);
notifyErrorCallback(cb, ERROR_INVALID_SOCKET);
return;
}

17
tests/net/java/com/android/server/ConnectivityServiceTest.java
View File

@@ -4047,8 +4047,6 @@ public class ConnectivityServiceTest {
// TODO: 1. Move this outside of ConnectivityServiceTest.
// 2. Make test to verify that Nat-T keepalive socket is created by IpSecService.
// 3. Mock ipsec service.
// 4. Find a free port instead of a fixed port.
final int srcPort = 12345;
final InetAddress myIPv4 = InetAddress.getByName("192.0.2.129");
final InetAddress notMyIPv4 = InetAddress.getByName("192.0.2.35");
final InetAddress myIPv6 = InetAddress.getByName("2001:db8::1");
@@ -4059,7 +4057,8 @@ public class ConnectivityServiceTest {
final int invalidKaInterval = 9;
final IpSecManager mIpSec = (IpSecManager) mContext.getSystemService(Context.IPSEC_SERVICE);
final UdpEncapsulationSocket testSocket = mIpSec.openUdpEncapsulationSocket(srcPort);
final UdpEncapsulationSocket testSocket = mIpSec.openUdpEncapsulationSocket();
final int srcPort = testSocket.getPort();
LinkProperties lp = new LinkProperties();
lp.setInterfaceName("wlan12");
@@ -4179,6 +4178,7 @@ public class ConnectivityServiceTest {
// Check that keepalive slots start from 1 and increment. The first one gets slot 1.
mWiFiNetworkAgent.setExpectedKeepaliveSlot(1);
int srcPort2 = 0;
try (SocketKeepalive ka = mCm.createSocketKeepalive(
myNet, testSocket, myIPv4, dstIPv4, executor, callback)) {
ka.start(validKaInterval);
@@ -4186,7 +4186,8 @@ public class ConnectivityServiceTest {
// The second one gets slot 2.
mWiFiNetworkAgent.setExpectedKeepaliveSlot(2);
final UdpEncapsulationSocket testSocket2 = mIpSec.openUdpEncapsulationSocket(6789);
final UdpEncapsulationSocket testSocket2 = mIpSec.openUdpEncapsulationSocket();
srcPort2 = testSocket2.getPort();
TestSocketKeepaliveCallback callback2 = new TestSocketKeepaliveCallback(executor);
try (SocketKeepalive ka2 = mCm.createSocketKeepalive(
myNet, testSocket2, myIPv4, dstIPv4, executor, callback2)) {
@@ -4204,6 +4205,10 @@ public class ConnectivityServiceTest {
}
}
// Check that there is no port leaked after all keepalives and sockets are closed.
assertFalse(isUdpPortInUse(srcPort));
assertFalse(isUdpPortInUse(srcPort2));
mWiFiNetworkAgent.disconnect();
waitFor(mWiFiNetworkAgent.getDisconnectedCV());
mWiFiNetworkAgent = null;
@@ -4292,7 +4297,6 @@ public class ConnectivityServiceTest {
}
private void doTestNattSocketKeepalivesFdWithExecutor(Executor executor) throws Exception {
final int srcPort = 12345;
final InetAddress myIPv4 = InetAddress.getByName("192.0.2.129");
final InetAddress anyIPv4 = InetAddress.getByName("0.0.0.0");
final InetAddress dstIPv4 = InetAddress.getByName("8.8.8.8");
@@ -4311,7 +4315,8 @@ public class ConnectivityServiceTest {
// Prepare the target file descriptor, keep only one instance.
final IpSecManager mIpSec = (IpSecManager) mContext.getSystemService(Context.IPSEC_SERVICE);
final UdpEncapsulationSocket testSocket = mIpSec.openUdpEncapsulationSocket(srcPort);
final UdpEncapsulationSocket testSocket = mIpSec.openUdpEncapsulationSocket();
final int srcPort = testSocket.getPort();
final ParcelFileDescriptor testPfd =
ParcelFileDescriptor.dup(testSocket.getFileDescriptor());
testSocket.close();