Merge "Rework Exception Handling for IpSecManager" into pi-dev am: 16c671dc9a
am: 86238ee312
Change-Id: Ic1f560070d12f3bdeb5c07316aad7ebed9719f6f
diff --git a/core/java/android/net/IpSecManager.java b/core/java/android/net/IpSecManager.java
index a61ea50..1145d5b 100644
--- a/core/java/android/net/IpSecManager.java
+++ b/core/java/android/net/IpSecManager.java
@@ -20,12 +20,16 @@
import android.annotation.IntDef;
import android.annotation.NonNull;
import android.annotation.RequiresPermission;
+import android.annotation.SystemApi;
import android.annotation.SystemService;
import android.annotation.TestApi;
import android.content.Context;
import android.os.Binder;
import android.os.ParcelFileDescriptor;
import android.os.RemoteException;
+import android.os.ServiceSpecificException;
+import android.system.ErrnoException;
+import android.system.OsConstants;
import android.util.AndroidException;
import android.util.Log;
@@ -172,11 +176,16 @@
public void close() {
try {
mService.releaseSecurityParameterIndex(mResourceId);
- mResourceId = INVALID_RESOURCE_ID;
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
+ } catch (Exception e) {
+ // On close we swallow all random exceptions since failure to close is not
+ // actionable by the user.
+ Log.e(TAG, "Failed to close " + this + ", Exception=" + e);
+ } finally {
+ mResourceId = INVALID_RESOURCE_ID;
+ mCloseGuard.close();
}
- mCloseGuard.close();
}
/** Check that the SPI was closed properly. */
@@ -227,7 +236,6 @@
throw new RuntimeException(
"Invalid Resource ID returned by IpSecService: " + status);
}
-
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
@@ -239,6 +247,17 @@
public int getResourceId() {
return mResourceId;
}
+
+ @Override
+ public String toString() {
+ return new StringBuilder()
+ .append("SecurityParameterIndex{spi=")
+ .append(mSpi)
+ .append(",resourceId=")
+ .append(mResourceId)
+ .append("}")
+ .toString();
+ }
}
/**
@@ -261,7 +280,11 @@
mService,
destinationAddress,
IpSecManager.INVALID_SECURITY_PARAMETER_INDEX);
+ } catch (ServiceSpecificException e) {
+ throw rethrowUncheckedExceptionFromServiceSpecificException(e);
} catch (SpiUnavailableException unlikely) {
+ // Because this function allocates a totally random SPI, it really shouldn't ever
+ // fail to allocate an SPI; we simply need this because the exception is checked.
throw new ResourceUnavailableException("No SPIs available");
}
}
@@ -274,8 +297,8 @@
*
* @param destinationAddress the destination address for traffic bearing the requested SPI.
* For inbound traffic, the destination should be an address currently assigned on-device.
- * @param requestedSpi the requested SPI, or '0' to allocate a random SPI. The range 1-255 is
- * reserved and may not be used. See RFC 4303 Section 2.1.
+ * @param requestedSpi the requested SPI. The range 1-255 is reserved and may not be used. See
+ * RFC 4303 Section 2.1.
* @return the reserved SecurityParameterIndex
* @throws {@link #ResourceUnavailableException} indicating that too many SPIs are
* currently allocated for this user
@@ -289,7 +312,11 @@
if (requestedSpi == IpSecManager.INVALID_SECURITY_PARAMETER_INDEX) {
throw new IllegalArgumentException("Requested SPI must be a valid (non-zero) SPI");
}
- return new SecurityParameterIndex(mService, destinationAddress, requestedSpi);
+ try {
+ return new SecurityParameterIndex(mService, destinationAddress, requestedSpi);
+ } catch (ServiceSpecificException e) {
+ throw rethrowUncheckedExceptionFromServiceSpecificException(e);
+ }
}
/**
@@ -424,6 +451,8 @@
// constructor takes control and closes the user's FD when we exit the method.
try (ParcelFileDescriptor pfd = ParcelFileDescriptor.dup(socket)) {
mService.applyTransportModeTransform(pfd, direction, transform.getResourceId());
+ } catch (ServiceSpecificException e) {
+ throw rethrowCheckedExceptionFromServiceSpecificException(e);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
@@ -482,6 +511,8 @@
public void removeTransportModeTransforms(@NonNull FileDescriptor socket) throws IOException {
try (ParcelFileDescriptor pfd = ParcelFileDescriptor.dup(socket)) {
mService.removeTransportModeTransforms(pfd);
+ } catch (ServiceSpecificException e) {
+ throw rethrowCheckedExceptionFromServiceSpecificException(e);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
@@ -575,6 +606,13 @@
mResourceId = INVALID_RESOURCE_ID;
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
+ } catch (Exception e) {
+ // On close we swallow all random exceptions since failure to close is not
+ // actionable by the user.
+ Log.e(TAG, "Failed to close " + this + ", Exception=" + e);
+ } finally {
+ mResourceId = INVALID_RESOURCE_ID;
+ mCloseGuard.close();
}
try {
@@ -583,7 +621,6 @@
Log.e(TAG, "Failed to close UDP Encapsulation Socket with Port= " + mPort);
throw e;
}
- mCloseGuard.close();
}
/** Check that the socket was closed properly. */
@@ -600,6 +637,17 @@
public int getResourceId() {
return mResourceId;
}
+
+ @Override
+ public String toString() {
+ return new StringBuilder()
+ .append("UdpEncapsulationSocket{port=")
+ .append(mPort)
+ .append(",resourceId=")
+ .append(mResourceId)
+ .append("}")
+ .toString();
+ }
};
/**
@@ -627,7 +675,11 @@
if (port == 0) {
throw new IllegalArgumentException("Specified port must be a valid port number!");
}
- return new UdpEncapsulationSocket(mService, port);
+ try {
+ return new UdpEncapsulationSocket(mService, port);
+ } catch (ServiceSpecificException e) {
+ throw rethrowCheckedExceptionFromServiceSpecificException(e);
+ }
}
/**
@@ -650,7 +702,11 @@
@NonNull
public UdpEncapsulationSocket openUdpEncapsulationSocket()
throws IOException, ResourceUnavailableException {
- return new UdpEncapsulationSocket(mService, 0);
+ try {
+ return new UdpEncapsulationSocket(mService, 0);
+ } catch (ServiceSpecificException e) {
+ throw rethrowCheckedExceptionFromServiceSpecificException(e);
+ }
}
/**
@@ -665,6 +721,7 @@
* to create Network objects which are accessible to the Android system.
* @hide
*/
+ @SystemApi
public static final class IpSecTunnelInterface implements AutoCloseable {
private final String mOpPackageName;
private final IIpSecService mService;
@@ -691,11 +748,14 @@
* @param prefixLen length of the InetAddress prefix
* @hide
*/
+ @SystemApi
@RequiresPermission(android.Manifest.permission.MANAGE_IPSEC_TUNNELS)
public void addAddress(@NonNull InetAddress address, int prefixLen) throws IOException {
try {
mService.addAddressToTunnelInterface(
mResourceId, new LinkAddress(address, prefixLen), mOpPackageName);
+ } catch (ServiceSpecificException e) {
+ throw rethrowCheckedExceptionFromServiceSpecificException(e);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
@@ -710,11 +770,14 @@
* @param prefixLen length of the InetAddress prefix
* @hide
*/
+ @SystemApi
@RequiresPermission(android.Manifest.permission.MANAGE_IPSEC_TUNNELS)
public void removeAddress(@NonNull InetAddress address, int prefixLen) throws IOException {
try {
mService.removeAddressFromTunnelInterface(
mResourceId, new LinkAddress(address, prefixLen), mOpPackageName);
+ } catch (ServiceSpecificException e) {
+ throw rethrowCheckedExceptionFromServiceSpecificException(e);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
@@ -767,11 +830,16 @@
public void close() {
try {
mService.deleteTunnelInterface(mResourceId, mOpPackageName);
- mResourceId = INVALID_RESOURCE_ID;
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
+ } catch (Exception e) {
+ // On close we swallow all random exceptions since failure to close is not
+ // actionable by the user.
+ Log.e(TAG, "Failed to close " + this + ", Exception=" + e);
+ } finally {
+ mResourceId = INVALID_RESOURCE_ID;
+ mCloseGuard.close();
}
- mCloseGuard.close();
}
/** Check that the Interface was closed properly. */
@@ -788,6 +856,17 @@
public int getResourceId() {
return mResourceId;
}
+
+ @Override
+ public String toString() {
+ return new StringBuilder()
+ .append("IpSecTunnelInterface{ifname=")
+ .append(mInterfaceName)
+ .append(",resourceId=")
+ .append(mResourceId)
+ .append("}")
+ .toString();
+ }
}
/**
@@ -805,13 +884,18 @@
* @throws ResourceUnavailableException indicating that too many encapsulation sockets are open
* @hide
*/
+ @SystemApi
@NonNull
@RequiresPermission(android.Manifest.permission.MANAGE_IPSEC_TUNNELS)
public IpSecTunnelInterface createIpSecTunnelInterface(@NonNull InetAddress localAddress,
@NonNull InetAddress remoteAddress, @NonNull Network underlyingNetwork)
throws ResourceUnavailableException, IOException {
- return new IpSecTunnelInterface(
- mContext, mService, localAddress, remoteAddress, underlyingNetwork);
+ try {
+ return new IpSecTunnelInterface(
+ mContext, mService, localAddress, remoteAddress, underlyingNetwork);
+ } catch (ServiceSpecificException e) {
+ throw rethrowCheckedExceptionFromServiceSpecificException(e);
+ }
}
/**
@@ -831,6 +915,7 @@
* layer failure.
* @hide
*/
+ @SystemApi
@RequiresPermission(android.Manifest.permission.MANAGE_IPSEC_TUNNELS)
public void applyTunnelModeTransform(@NonNull IpSecTunnelInterface tunnel,
@PolicyDirection int direction, @NonNull IpSecTransform transform) throws IOException {
@@ -838,6 +923,8 @@
mService.applyTunnelModeTransform(
tunnel.getResourceId(), direction,
transform.getResourceId(), mContext.getOpPackageName());
+ } catch (ServiceSpecificException e) {
+ throw rethrowCheckedExceptionFromServiceSpecificException(e);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
@@ -853,4 +940,44 @@
mContext = ctx;
mService = checkNotNull(service, "missing service");
}
+
+ private static void maybeHandleServiceSpecificException(ServiceSpecificException sse) {
+ // OsConstants are late binding, so switch statements can't be used.
+ if (sse.errorCode == OsConstants.EINVAL) {
+ throw new IllegalArgumentException(sse);
+ } else if (sse.errorCode == OsConstants.EAGAIN) {
+ throw new IllegalStateException(sse);
+ } else if (sse.errorCode == OsConstants.EOPNOTSUPP) {
+ throw new UnsupportedOperationException(sse);
+ }
+ }
+
+ /**
+ * Convert an Errno SSE to the correct Unchecked exception type.
+ *
+ * This method never actually returns.
+ */
+ // package
+ static RuntimeException
+ rethrowUncheckedExceptionFromServiceSpecificException(ServiceSpecificException sse) {
+ maybeHandleServiceSpecificException(sse);
+ throw new RuntimeException(sse);
+ }
+
+ /**
+ * Convert an Errno SSE to the correct Checked or Unchecked exception type.
+ *
+ * This method may throw IOException, or it may throw an unchecked exception; it will never
+ * actually return.
+ */
+ // package
+ static IOException rethrowCheckedExceptionFromServiceSpecificException(
+ ServiceSpecificException sse) throws IOException {
+ // First see if this is an unchecked exception of a type we know.
+ // If so, then we prefer the unchecked (specific) type of exception.
+ maybeHandleServiceSpecificException(sse);
+ // If not, then all we can do is provide the SSE in the form of an IOException.
+ throw new ErrnoException(
+ "IpSec encountered errno=" + sse.errorCode, sse.errorCode).rethrowAsIOException();
+ }
}
diff --git a/core/java/android/net/IpSecTransform.java b/core/java/android/net/IpSecTransform.java
index 62f7996..23c8aa3 100644
--- a/core/java/android/net/IpSecTransform.java
+++ b/core/java/android/net/IpSecTransform.java
@@ -22,12 +22,14 @@
import android.annotation.IntDef;
import android.annotation.NonNull;
import android.annotation.RequiresPermission;
+import android.annotation.SystemApi;
import android.content.Context;
import android.os.Binder;
import android.os.Handler;
import android.os.IBinder;
import android.os.RemoteException;
import android.os.ServiceManager;
+import android.os.ServiceSpecificException;
import android.util.Log;
import com.android.internal.annotations.VisibleForTesting;
@@ -136,6 +138,8 @@
mResourceId = result.resourceId;
Log.d(TAG, "Added Transform with Id " + mResourceId);
mCloseGuard.open("build");
+ } catch (ServiceSpecificException e) {
+ throw IpSecManager.rethrowUncheckedExceptionFromServiceSpecificException(e);
} catch (RemoteException e) {
throw e.rethrowAsRuntimeException();
}
@@ -180,6 +184,10 @@
stopNattKeepalive();
} catch (RemoteException e) {
throw e.rethrowAsRuntimeException();
+ } catch (Exception e) {
+ // On close we swallow all random exceptions since failure to close is not
+ // actionable by the user.
+ Log.e(TAG, "Failed to close " + this + ", Exception=" + e);
} finally {
mResourceId = INVALID_RESOURCE_ID;
mCloseGuard.close();
@@ -249,6 +257,7 @@
*
* @hide
*/
+ @SystemApi
public static class NattKeepaliveCallback {
/** The specified {@code Network} is not connected. */
public static final int ERROR_INVALID_NETWORK = 1;
@@ -279,6 +288,7 @@
*
* @hide
*/
+ @SystemApi
@RequiresPermission(anyOf = {
android.Manifest.permission.MANAGE_IPSEC_TUNNELS,
android.Manifest.permission.PACKET_KEEPALIVE_OFFLOAD
@@ -321,6 +331,7 @@
*
* @hide
*/
+ @SystemApi
@RequiresPermission(anyOf = {
android.Manifest.permission.MANAGE_IPSEC_TUNNELS,
android.Manifest.permission.PACKET_KEEPALIVE_OFFLOAD
@@ -473,6 +484,7 @@
* @throws IOException indicating other errors
* @hide
*/
+ @SystemApi
@NonNull
@RequiresPermission(android.Manifest.permission.MANAGE_IPSEC_TUNNELS)
public IpSecTransform buildTunnelModeTransform(
@@ -502,4 +514,13 @@
mConfig = new IpSecConfig();
}
}
+
+ @Override
+ public String toString() {
+ return new StringBuilder()
+ .append("IpSecTransform{resourceId=")
+ .append(mResourceId)
+ .append("}")
+ .toString();
+ }
}
diff --git a/services/core/java/com/android/server/IpSecService.java b/services/core/java/com/android/server/IpSecService.java
index cd90e3f..33ca02f 100644
--- a/services/core/java/com/android/server/IpSecService.java
+++ b/services/core/java/com/android/server/IpSecService.java
@@ -1101,9 +1101,11 @@
new RefcountedResource<SpiRecord>(
new SpiRecord(resourceId, "", destinationAddress, spi), binder));
} catch (ServiceSpecificException e) {
- // TODO: Add appropriate checks when other ServiceSpecificException types are supported
- return new IpSecSpiResponse(
- IpSecManager.Status.SPI_UNAVAILABLE, INVALID_RESOURCE_ID, spi);
+ if (e.errorCode == OsConstants.ENOENT) {
+ return new IpSecSpiResponse(
+ IpSecManager.Status.SPI_UNAVAILABLE, INVALID_RESOURCE_ID, spi);
+ }
+ throw e;
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
@@ -1115,7 +1117,6 @@
*/
private void releaseResource(RefcountedResourceArray resArray, int resourceId)
throws RemoteException {
-
resArray.getRefcountedResourceOrThrow(resourceId).userRelease();
}
@@ -1315,15 +1316,12 @@
releaseNetId(ikey);
releaseNetId(okey);
throw e.rethrowFromSystemServer();
- } catch (ServiceSpecificException e) {
- // FIXME: get the error code and throw is at an IOException from Errno Exception
+ } catch (Throwable t) {
+ // Release keys if we got an error.
+ releaseNetId(ikey);
+ releaseNetId(okey);
+ throw t;
}
-
- // If we make it to here, then something has gone wrong and we couldn't create a VTI.
- // Release the keys that we reserved, and return an error status.
- releaseNetId(ikey);
- releaseNetId(okey);
- return new IpSecTunnelInterfaceResponse(IpSecManager.Status.RESOURCE_UNAVAILABLE);
}
/**
@@ -1352,9 +1350,6 @@
localAddr.getPrefixLength());
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
- } catch (ServiceSpecificException e) {
- // If we get here, one of the arguments provided was invalid. Wrap the SSE, and throw.
- throw new IllegalArgumentException(e);
}
}
@@ -1384,9 +1379,6 @@
localAddr.getPrefixLength());
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
- } catch (ServiceSpecificException e) {
- // If we get here, one of the arguments provided was invalid. Wrap the SSE, and throw.
- throw new IllegalArgumentException(e);
}
}
@@ -1590,12 +1582,7 @@
dependencies.add(refcountedSpiRecord);
SpiRecord spiRecord = refcountedSpiRecord.getResource();
- try {
- createOrUpdateTransform(c, resourceId, spiRecord, socketRecord);
- } catch (ServiceSpecificException e) {
- // FIXME: get the error code and throw is at an IOException from Errno Exception
- return new IpSecTransformResponse(IpSecManager.Status.RESOURCE_UNAVAILABLE);
- }
+ createOrUpdateTransform(c, resourceId, spiRecord, socketRecord);
// SA was created successfully, time to construct a record and lock it away
userRecord.mTransformRecords.put(
@@ -1642,23 +1629,15 @@
c.getMode() == IpSecTransform.MODE_TRANSPORT,
"Transform mode was not Transport mode; cannot be applied to a socket");
- try {
- mSrvConfig
- .getNetdInstance()
- .ipSecApplyTransportModeTransform(
- socket.getFileDescriptor(),
- resourceId,
- direction,
- c.getSourceAddress(),
- c.getDestinationAddress(),
- info.getSpiRecord().getSpi());
- } catch (ServiceSpecificException e) {
- if (e.errorCode == EINVAL) {
- throw new IllegalArgumentException(e.toString());
- } else {
- throw e;
- }
- }
+ mSrvConfig
+ .getNetdInstance()
+ .ipSecApplyTransportModeTransform(
+ socket.getFileDescriptor(),
+ resourceId,
+ direction,
+ c.getSourceAddress(),
+ c.getDestinationAddress(),
+ info.getSpiRecord().getSpi());
}
/**
@@ -1670,13 +1649,9 @@
@Override
public synchronized void removeTransportModeTransforms(ParcelFileDescriptor socket)
throws RemoteException {
- try {
- mSrvConfig
- .getNetdInstance()
- .ipSecRemoveTransportModeTransform(socket.getFileDescriptor());
- } catch (ServiceSpecificException e) {
- // FIXME: get the error code and throw is at an IOException from Errno Exception
- }
+ mSrvConfig
+ .getNetdInstance()
+ .ipSecRemoveTransportModeTransform(socket.getFileDescriptor());
}
/**