Merge changes from topic "ipsec-resource-id-cleanup" am: 2b36299c73

am: d7319df4d0

Change-Id: If6ab575e2f30fc70d73e6f8787c87f3c7a2ab09d

core/java/android/net/IpSecConfig.java

@@ -102,17 +102,11 @@ public final class IpSecConfig implements Parcelable {
 
     /** Set the local IP address for Tunnel mode */
     public void setLocalAddress(String localAddress) {
-        if (localAddress == null) {
-            throw new IllegalArgumentException("localAddress may not be null!");
-        }
         mLocalAddress = localAddress;
     }
 
     /** Set the remote IP address for this IPsec transform */
     public void setRemoteAddress(String remoteAddress) {
-        if (remoteAddress == null) {
-            throw new IllegalArgumentException("remoteAddress may not be null!");
-        }
         mRemoteAddress = remoteAddress;
     }
 

core/java/android/net/IpSecManager.java

@@ -69,7 +69,7 @@ public final class IpSecManager {
     }
 
     /** @hide */
-    public static final int INVALID_RESOURCE_ID = 0;
+    public static final int INVALID_RESOURCE_ID = -1;
 
     /**
      * Thrown to indicate that a requested SPI is in use.
@@ -128,7 +128,7 @@ public final class IpSecManager {
         private final InetAddress mRemoteAddress;
         private final CloseGuard mCloseGuard = CloseGuard.get();
         private int mSpi = INVALID_SECURITY_PARAMETER_INDEX;
-        private int mResourceId;
+        private int mResourceId = INVALID_RESOURCE_ID;
 
         /** Get the underlying SPI held by this object. */
         public int getSpi() {
@@ -146,6 +146,7 @@ public final class IpSecManager {
         public void close() {
             try {
                 mService.releaseSecurityParameterIndex(mResourceId);
+                mResourceId = INVALID_RESOURCE_ID;
             } catch (RemoteException e) {
                 throw e.rethrowFromSystemServer();
             }
@@ -501,7 +502,7 @@ public final class IpSecManager {
     public static final class UdpEncapsulationSocket implements AutoCloseable {
         private final ParcelFileDescriptor mPfd;
         private final IIpSecService mService;
-        private final int mResourceId;
+        private int mResourceId = INVALID_RESOURCE_ID;
         private final int mPort;
         private final CloseGuard mCloseGuard = CloseGuard.get();
 
@@ -554,6 +555,7 @@ public final class IpSecManager {
         public void close() throws IOException {
             try {
                 mService.closeUdpEncapsulationSocket(mResourceId);
+                mResourceId = INVALID_RESOURCE_ID;
             } catch (RemoteException e) {
                 throw e.rethrowFromSystemServer();
             }

core/java/android/net/IpSecTransform.java

@@ -347,6 +347,9 @@ public final class IpSecTransform implements AutoCloseable {
          */
         public IpSecTransform.Builder setSpi(
                 @TransformDirection int direction, IpSecManager.SecurityParameterIndex spi) {
+            if (spi.getResourceId() == INVALID_RESOURCE_ID) {
+                throw new IllegalArgumentException("Invalid SecurityParameterIndex");
+            }
             mConfig.setSpiResourceId(direction, spi.getResourceId());
             return this;
         }
@@ -381,6 +384,9 @@ public final class IpSecTransform implements AutoCloseable {
         public IpSecTransform.Builder setIpv4Encapsulation(
                 IpSecManager.UdpEncapsulationSocket localSocket, int remotePort) {
             mConfig.setEncapType(ENCAP_ESPINUDP);
+            if (localSocket.getResourceId() == INVALID_RESOURCE_ID) {
+                throw new IllegalArgumentException("Invalid UdpEncapsulationSocket");
+            }
             mConfig.setEncapSocketResourceId(localSocket.getResourceId());
             mConfig.setEncapRemotePort(remotePort);
             return this;
@@ -426,6 +432,9 @@ public final class IpSecTransform implements AutoCloseable {
         public IpSecTransform buildTransportModeTransform(InetAddress remoteAddress)
                 throws IpSecManager.ResourceUnavailableException,
                         IpSecManager.SpiUnavailableException, IOException {
+            if (remoteAddress == null) {
+                throw new IllegalArgumentException("Remote address may not be null or empty!");
+            }
             mConfig.setMode(MODE_TRANSPORT);
             mConfig.setRemoteAddress(remoteAddress.getHostAddress());
             // FIXME: modifying a builder after calling build can change the built transform.
@@ -447,8 +456,12 @@ public final class IpSecTransform implements AutoCloseable {
          */
         public IpSecTransform buildTunnelModeTransform(
                 InetAddress localAddress, InetAddress remoteAddress) {
-            // FIXME: argument validation here
+            if (localAddress == null) {
-            // throw new IllegalArgumentException("Natt Keepalive requires UDP Encapsulation");
+                throw new IllegalArgumentException("Local address may not be null or empty!");
+            }
+            if (remoteAddress == null) {
+                throw new IllegalArgumentException("Remote address may not be null or empty!");
+            }
             mConfig.setLocalAddress(localAddress.getHostAddress());
             mConfig.setRemoteAddress(remoteAddress.getHostAddress());
             mConfig.setMode(MODE_TUNNEL);

services/core/java/com/android/server/IpSecService.java

@@ -102,8 +102,14 @@ public class IpSecService extends IIpSecService.Stub {
     /* Binder context for this service */
     private final Context mContext;
 
-    /** Should be a never-repeating global ID for resources */
+    /**
-    private static AtomicInteger mNextResourceId = new AtomicInteger(0x00FADED0);
+     * The next non-repeating global ID for tracking resources between users, this service,
+     * and kernel data structures. Accessing this variable is not thread safe, so it is
+     * only read or modified within blocks synchronized on IpSecService.this. We want to
+     * avoid -1 (INVALID_RESOURCE_ID) and 0 (we probably forgot to initialize it).
+     */
+    @GuardedBy("IpSecService.this")
+    private int mNextResourceId = 1;
 
     interface IpSecServiceConfiguration {
         INetd getNetdInstance() throws RemoteException;
@@ -856,7 +862,7 @@ public class IpSecService extends IIpSecService.Stub {
         checkNotNull(binder, "Null Binder passed to allocateSecurityParameterIndex");
 
         UserRecord userRecord = mUserResourceTracker.getUserRecord(Binder.getCallingUid());
-        int resourceId = mNextResourceId.getAndIncrement();
+        final int resourceId = mNextResourceId++;
 
         int spi = IpSecManager.INVALID_SECURITY_PARAMETER_INDEX;
         String localAddress = "";
@@ -979,7 +985,7 @@ public class IpSecService extends IIpSecService.Stub {
 
         int callingUid = Binder.getCallingUid();
         UserRecord userRecord = mUserResourceTracker.getUserRecord(callingUid);
-        int resourceId = mNextResourceId.getAndIncrement();
+        final int resourceId = mNextResourceId++;
         FileDescriptor sockFd = null;
         try {
             if (!userRecord.mSocketQuotaTracker.isAvailable()) {
@@ -1102,7 +1108,7 @@ public class IpSecService extends IIpSecService.Stub {
             IpSecConfig c, IBinder binder) throws RemoteException {
         checkIpSecConfig(c);
         checkNotNull(binder, "Null Binder passed to createTransportModeTransform");
-        int resourceId = mNextResourceId.getAndIncrement();
+        final int resourceId = mNextResourceId++;
 
         UserRecord userRecord = mUserResourceTracker.getUserRecord(Binder.getCallingUid());