From a0989622b1c7cc6fe2c6ce06f8c20bfb06e2268c Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Wed, 25 Jul 2018 13:06:29 -0700 Subject: [PATCH] Use UID as requestID This change makes all requestIDs use the UID of the creator, ensuring that rekeys always use the same requestID. This also has the nice property of separating app's resources from each other, and allowing for identification of which app/UID allocated the resources from command-line dumps (eg ip xfrm state show) Bug: 111841561 Test: Updated tests & passing taimen Change-Id: I4f1eadcdb795766ae4682b15e41727359c52fa38 --- .../java/com/android/server/IpSecService.java | 34 ++++++++------- .../server/IpSecServiceParameterizedTest.java | 43 ++++++++----------- 2 files changed, 37 insertions(+), 40 deletions(-) diff --git a/services/core/java/com/android/server/IpSecService.java b/services/core/java/com/android/server/IpSecService.java index 01e81525d5b5b..380f6a7e581e5 100644 --- a/services/core/java/com/android/server/IpSecService.java +++ b/services/core/java/com/android/server/IpSecService.java @@ -612,7 +612,7 @@ public class IpSecService extends IIpSecService.Stub { mSrvConfig .getNetdInstance() .ipSecDeleteSecurityAssociation( - mResourceId, + uid, mConfig.getSourceAddress(), mConfig.getDestinationAddress(), spi, @@ -679,7 +679,7 @@ public class IpSecService extends IIpSecService.Stub { mSrvConfig .getNetdInstance() .ipSecDeleteSecurityAssociation( - mResourceId, mSourceAddress, mDestinationAddress, mSpi, 0, 0); + uid, mSourceAddress, mDestinationAddress, mSpi, 0, 0); } } catch (ServiceSpecificException | RemoteException e) { Log.e(TAG, "Failed to delete SPI reservation with ID: " + mResourceId, e); @@ -821,13 +821,13 @@ public class IpSecService extends IIpSecService.Stub { for (int selAddrFamily : ADDRESS_FAMILIES) { netd.ipSecDeleteSecurityPolicy( - 0, + uid, selAddrFamily, IpSecManager.DIRECTION_OUT, mOkey, 0xffffffff); netd.ipSecDeleteSecurityPolicy( - 0, + uid, selAddrFamily, IpSecManager.DIRECTION_IN, mIkey, @@ -1083,7 +1083,8 @@ public class IpSecService extends IIpSecService.Stub { } checkNotNull(binder, "Null Binder passed to allocateSecurityParameterIndex"); - UserRecord userRecord = mUserResourceTracker.getUserRecord(Binder.getCallingUid()); + int callingUid = Binder.getCallingUid(); + UserRecord userRecord = mUserResourceTracker.getUserRecord(callingUid); final int resourceId = mNextResourceId++; int spi = IpSecManager.INVALID_SECURITY_PARAMETER_INDEX; @@ -1096,7 +1097,7 @@ public class IpSecService extends IIpSecService.Stub { spi = mSrvConfig .getNetdInstance() - .ipSecAllocateSpi(resourceId, "", destinationAddress, requestedSpi); + .ipSecAllocateSpi(callingUid, "", destinationAddress, requestedSpi); Log.d(TAG, "Allocated SPI " + spi); userRecord.mSpiRecords.put( resourceId, @@ -1264,7 +1265,8 @@ public class IpSecService extends IIpSecService.Stub { // TODO: Check that underlying network exists, and IP addresses not assigned to a different // network (b/72316676). - UserRecord userRecord = mUserResourceTracker.getUserRecord(Binder.getCallingUid()); + int callerUid = Binder.getCallingUid(); + UserRecord userRecord = mUserResourceTracker.getUserRecord(callerUid); if (!userRecord.mTunnelQuotaTracker.isAvailable()) { return new IpSecTunnelInterfaceResponse(IpSecManager.Status.RESOURCE_UNAVAILABLE); } @@ -1285,7 +1287,7 @@ public class IpSecService extends IIpSecService.Stub { for (int selAddrFamily : ADDRESS_FAMILIES) { // Always send down correct local/remote addresses for template. netd.ipSecAddSecurityPolicy( - 0, // Use 0 for reqId + callerUid, selAddrFamily, IpSecManager.DIRECTION_OUT, localAddr, @@ -1294,7 +1296,7 @@ public class IpSecService extends IIpSecService.Stub { okey, 0xffffffff); netd.ipSecAddSecurityPolicy( - 0, // Use 0 for reqId + callerUid, selAddrFamily, IpSecManager.DIRECTION_IN, remoteAddr, @@ -1532,7 +1534,7 @@ public class IpSecService extends IIpSecService.Stub { mSrvConfig .getNetdInstance() .ipSecAddSecurityAssociation( - resourceId, + Binder.getCallingUid(), c.getMode(), c.getSourceAddress(), c.getDestinationAddress(), @@ -1623,13 +1625,14 @@ public class IpSecService extends IIpSecService.Stub { @Override public synchronized void applyTransportModeTransform( ParcelFileDescriptor socket, int direction, int resourceId) throws RemoteException { - UserRecord userRecord = mUserResourceTracker.getUserRecord(Binder.getCallingUid()); + int callingUid = Binder.getCallingUid(); + UserRecord userRecord = mUserResourceTracker.getUserRecord(callingUid); checkDirection(direction); // Get transform record; if no transform is found, will throw IllegalArgumentException TransformRecord info = userRecord.mTransformRecords.getResourceOrThrow(resourceId); // TODO: make this a function. - if (info.pid != getCallingPid() || info.uid != getCallingUid()) { + if (info.pid != getCallingPid() || info.uid != callingUid) { throw new SecurityException("Only the owner of an IpSec Transform may apply it!"); } @@ -1643,7 +1646,7 @@ public class IpSecService extends IIpSecService.Stub { .getNetdInstance() .ipSecApplyTransportModeTransform( socket.getFileDescriptor(), - resourceId, + callingUid, direction, c.getSourceAddress(), c.getDestinationAddress(), @@ -1675,7 +1678,8 @@ public class IpSecService extends IIpSecService.Stub { enforceTunnelPermissions(callingPackage); checkDirection(direction); - UserRecord userRecord = mUserResourceTracker.getUserRecord(Binder.getCallingUid()); + int callingUid = Binder.getCallingUid(); + UserRecord userRecord = mUserResourceTracker.getUserRecord(callingUid); // Get transform record; if no transform is found, will throw IllegalArgumentException TransformRecord transformInfo = @@ -1717,7 +1721,7 @@ public class IpSecService extends IIpSecService.Stub { mSrvConfig .getNetdInstance() .ipSecUpdateSecurityPolicy( - 0, // Use 0 for reqId + callingUid, selAddrFamily, direction, tunnelInterfaceInfo.getLocalAddress(), diff --git a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java index 102cb7c77055e..99a5a69213fa5 100644 --- a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java +++ b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java @@ -41,9 +41,9 @@ import android.net.Network; import android.net.NetworkUtils; import android.os.Binder; import android.os.ParcelFileDescriptor; -import android.test.mock.MockContext; import android.support.test.filters.SmallTest; import android.system.Os; +import android.test.mock.MockContext; import java.net.Socket; import java.util.Arrays; @@ -121,6 +121,7 @@ public class IpSecServiceParameterizedTest { IpSecService.IpSecServiceConfiguration mMockIpSecSrvConfig; IpSecService mIpSecService; Network fakeNetwork = new Network(0xAB); + int mUid = Os.getuid(); private static final IpSecAlgorithm AUTH_ALGO = new IpSecAlgorithm(IpSecAlgorithm.AUTH_HMAC_SHA256, AUTH_KEY, AUTH_KEY.length * 4); @@ -181,7 +182,7 @@ public class IpSecServiceParameterizedTest { verify(mMockNetd) .ipSecDeleteSecurityAssociation( - eq(spiResp.resourceId), + eq(mUid), anyString(), anyString(), eq(TEST_SPI), @@ -189,8 +190,7 @@ public class IpSecServiceParameterizedTest { anyInt()); // Verify quota and RefcountedResource objects cleaned up - IpSecService.UserRecord userRecord = - mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); + IpSecService.UserRecord userRecord = mIpSecService.mUserResourceTracker.getUserRecord(mUid); assertEquals(0, userRecord.mSpiQuotaTracker.mCurrent); try { userRecord.mSpiRecords.getRefcountedResourceOrThrow(spiResp.resourceId); @@ -209,8 +209,7 @@ public class IpSecServiceParameterizedTest { mIpSecService.allocateSecurityParameterIndex( mDestinationAddr, TEST_SPI, new Binder()); - IpSecService.UserRecord userRecord = - mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); + IpSecService.UserRecord userRecord = mIpSecService.mUserResourceTracker.getUserRecord(mUid); IpSecService.RefcountedResource refcountedRecord = userRecord.mSpiRecords.getRefcountedResourceOrThrow(spiResp.resourceId); @@ -218,7 +217,7 @@ public class IpSecServiceParameterizedTest { verify(mMockNetd) .ipSecDeleteSecurityAssociation( - eq(spiResp.resourceId), + eq(mUid), anyString(), anyString(), eq(TEST_SPI), @@ -270,7 +269,7 @@ public class IpSecServiceParameterizedTest { verify(mMockNetd) .ipSecAddSecurityAssociation( - eq(createTransformResp.resourceId), + eq(mUid), anyInt(), anyString(), anyString(), @@ -305,7 +304,7 @@ public class IpSecServiceParameterizedTest { verify(mMockNetd) .ipSecAddSecurityAssociation( - eq(createTransformResp.resourceId), + eq(mUid), anyInt(), anyString(), anyString(), @@ -361,13 +360,12 @@ public class IpSecServiceParameterizedTest { IpSecTransformResponse createTransformResp = mIpSecService.createTransform(ipSecConfig, new Binder(), "blessedPackage"); - IpSecService.UserRecord userRecord = - mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); + IpSecService.UserRecord userRecord = mIpSecService.mUserResourceTracker.getUserRecord(mUid); assertEquals(1, userRecord.mSpiQuotaTracker.mCurrent); mIpSecService.releaseSecurityParameterIndex(ipSecConfig.getSpiResourceId()); verify(mMockNetd, times(0)) .ipSecDeleteSecurityAssociation( - eq(createTransformResp.resourceId), + eq(mUid), anyString(), anyString(), eq(TEST_SPI), @@ -389,7 +387,7 @@ public class IpSecServiceParameterizedTest { verify(mMockNetd, times(1)) .ipSecDeleteSecurityAssociation( - eq(createTransformResp.resourceId), + eq(mUid), anyString(), anyString(), eq(TEST_SPI), @@ -397,8 +395,7 @@ public class IpSecServiceParameterizedTest { anyInt()); // Verify quota and RefcountedResource objects cleaned up - IpSecService.UserRecord userRecord = - mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); + IpSecService.UserRecord userRecord = mIpSecService.mUserResourceTracker.getUserRecord(mUid); assertEquals(0, userRecord.mTransformQuotaTracker.mCurrent); assertEquals(1, userRecord.mSpiQuotaTracker.mCurrent); @@ -433,8 +430,7 @@ public class IpSecServiceParameterizedTest { IpSecTransformResponse createTransformResp = mIpSecService.createTransform(ipSecConfig, new Binder(), "blessedPackage"); - IpSecService.UserRecord userRecord = - mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); + IpSecService.UserRecord userRecord = mIpSecService.mUserResourceTracker.getUserRecord(mUid); IpSecService.RefcountedResource refcountedRecord = userRecord.mTransformRecords.getRefcountedResourceOrThrow( createTransformResp.resourceId); @@ -443,7 +439,7 @@ public class IpSecServiceParameterizedTest { verify(mMockNetd) .ipSecDeleteSecurityAssociation( - eq(createTransformResp.resourceId), + eq(mUid), anyString(), anyString(), eq(TEST_SPI), @@ -477,7 +473,7 @@ public class IpSecServiceParameterizedTest { verify(mMockNetd) .ipSecApplyTransportModeTransform( eq(pfd.getFileDescriptor()), - eq(resourceId), + eq(mUid), eq(IpSecManager.DIRECTION_OUT), anyString(), anyString(), @@ -509,8 +505,7 @@ public class IpSecServiceParameterizedTest { createAndValidateTunnel(mSourceAddr, mDestinationAddr, "blessedPackage"); // Check that we have stored the tracking object, and retrieve it - IpSecService.UserRecord userRecord = - mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); + IpSecService.UserRecord userRecord = mIpSecService.mUserResourceTracker.getUserRecord(mUid); IpSecService.RefcountedResource refcountedRecord = userRecord.mTunnelInterfaceRecords.getRefcountedResourceOrThrow( createTunnelResp.resourceId); @@ -530,8 +525,7 @@ public class IpSecServiceParameterizedTest { IpSecTunnelInterfaceResponse createTunnelResp = createAndValidateTunnel(mSourceAddr, mDestinationAddr, "blessedPackage"); - IpSecService.UserRecord userRecord = - mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); + IpSecService.UserRecord userRecord = mIpSecService.mUserResourceTracker.getUserRecord(mUid); mIpSecService.deleteTunnelInterface(createTunnelResp.resourceId, "blessedPackage"); @@ -551,8 +545,7 @@ public class IpSecServiceParameterizedTest { IpSecTunnelInterfaceResponse createTunnelResp = createAndValidateTunnel(mSourceAddr, mDestinationAddr, "blessedPackage"); - IpSecService.UserRecord userRecord = - mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); + IpSecService.UserRecord userRecord = mIpSecService.mUserResourceTracker.getUserRecord(mUid); IpSecService.RefcountedResource refcountedRecord = userRecord.mTunnelInterfaceRecords.getRefcountedResourceOrThrow( createTunnelResp.resourceId);