From 5a19b9500d69352aaa6105c7a274cf80f65d117e Mon Sep 17 00:00:00 2001 From: Nathan Harold Date: Fri, 5 Jan 2018 19:25:13 -0800 Subject: [PATCH 1/3] Make Transforms Unidirectional Convert the IpSecTransform from being a bi-directional pair of SAs to a unidirectional single SA. This CL also removes the concept of "direction from SAs meaning that a IpSecTransform may now be applied to a socket in either direction. -Make transforms unidirectional -Add Convert allocateSpi() to use destination rather than direction and remote address -Remove directionality from builders for IpSecTransform -Change applyTransportModeTransform() to take a direction in which to apply the transform object. -Additional minor naming updates -Restrict IpSecConfig to only print keys on eng builds -Move DIRECTION constants to IpSecManager -Add sourceAddress parameter to IpSecTransform to provide additional guarantees about the source address of data; (explicit failure rather than implicit failure). -Move SPI to the build() method of IpSecTransform Bug: 71717213 Test: runtest frameworks-net, CTS - IpSecManagerTest Change-Id: I0824b37f443f4b8c62536d9801238c63ed8f2a1c --- core/java/android/net/IIpSecService.aidl | 6 +- core/java/android/net/IpSecAlgorithm.java | 8 +- core/java/android/net/IpSecConfig.java | 183 ++++------ core/java/android/net/IpSecManager.java | 119 +++--- core/java/android/net/IpSecTransform.java | 155 +++----- .../java/com/android/server/IpSecService.java | 338 ++++++++---------- 6 files changed, 371 insertions(+), 438 deletions(-) diff --git a/core/java/android/net/IIpSecService.aidl b/core/java/android/net/IIpSecService.aidl index d9b57db180..3fe531fd79 100644 --- a/core/java/android/net/IIpSecService.aidl +++ b/core/java/android/net/IIpSecService.aidl @@ -31,7 +31,7 @@ import android.os.ParcelFileDescriptor; interface IIpSecService { IpSecSpiResponse allocateSecurityParameterIndex( - int direction, in String remoteAddress, int requestedSpi, in IBinder binder); + in String destinationAddress, int requestedSpi, in IBinder binder); void releaseSecurityParameterIndex(int resourceId); @@ -43,7 +43,7 @@ interface IIpSecService void deleteTransportModeTransform(int transformId); - void applyTransportModeTransform(in ParcelFileDescriptor socket, int transformId); + void applyTransportModeTransform(in ParcelFileDescriptor socket, int direction, int transformId); - void removeTransportModeTransform(in ParcelFileDescriptor socket, int transformId); + void removeTransportModeTransforms(in ParcelFileDescriptor socket, int transformId); } diff --git a/core/java/android/net/IpSecAlgorithm.java b/core/java/android/net/IpSecAlgorithm.java index 7d752e89e6..c69a4d4c0b 100644 --- a/core/java/android/net/IpSecAlgorithm.java +++ b/core/java/android/net/IpSecAlgorithm.java @@ -256,13 +256,19 @@ public final class IpSecAlgorithm implements Parcelable { return getName().equals(AUTH_CRYPT_AES_GCM); } + // Because encryption keys are sensitive and userdebug builds are used by large user pools + // such as beta testers, we only allow sensitive info such as keys on eng builds. + private static boolean isUnsafeBuild() { + return Build.IS_DEBUGGABLE && Build.IS_ENG; + } + @Override public String toString() { return new StringBuilder() .append("{mName=") .append(mName) .append(", mKey=") - .append(Build.IS_DEBUGGABLE ? HexDump.toHexString(mKey) : "") + .append(isUnsafeBuild() ? HexDump.toHexString(mKey) : "") .append(", mTruncLenBits=") .append(mTruncLenBits) .append("}") diff --git a/core/java/android/net/IpSecConfig.java b/core/java/android/net/IpSecConfig.java index f54ceb5c14..80b0af3373 100644 --- a/core/java/android/net/IpSecConfig.java +++ b/core/java/android/net/IpSecConfig.java @@ -32,59 +32,29 @@ public final class IpSecConfig implements Parcelable { // MODE_TRANSPORT or MODE_TUNNEL private int mMode = IpSecTransform.MODE_TRANSPORT; - // Needs to be valid only for tunnel mode // Preventing this from being null simplifies Java->Native binder - private String mLocalAddress = ""; + private String mSourceAddress = ""; // Preventing this from being null simplifies Java->Native binder - private String mRemoteAddress = ""; + private String mDestinationAddress = ""; // The underlying Network that represents the "gateway" Network // for outbound packets. It may also be used to select packets. private Network mNetwork; - /** - * This class captures the parameters that specifically apply to inbound or outbound traffic. - */ - public static class Flow { - // Minimum requirements for identifying a transform - // SPI identifying the IPsec flow in packet processing - // and a remote IP address - private int mSpiResourceId = IpSecManager.INVALID_RESOURCE_ID; + // Minimum requirements for identifying a transform + // SPI identifying the IPsec SA in packet processing + // and a destination IP address + private int mSpiResourceId = IpSecManager.INVALID_RESOURCE_ID; - // Encryption Algorithm - private IpSecAlgorithm mEncryption; + // Encryption Algorithm + private IpSecAlgorithm mEncryption; - // Authentication Algorithm - private IpSecAlgorithm mAuthentication; + // Authentication Algorithm + private IpSecAlgorithm mAuthentication; - // Authenticated Encryption Algorithm - private IpSecAlgorithm mAuthenticatedEncryption; - - @Override - public String toString() { - return new StringBuilder() - .append("{mSpiResourceId=") - .append(mSpiResourceId) - .append(", mEncryption=") - .append(mEncryption) - .append(", mAuthentication=") - .append(mAuthentication) - .append(", mAuthenticatedEncryption=") - .append(mAuthenticatedEncryption) - .append("}") - .toString(); - } - - static boolean equals(IpSecConfig.Flow lhs, IpSecConfig.Flow rhs) { - if (lhs == null || rhs == null) return (lhs == rhs); - return (lhs.mSpiResourceId == rhs.mSpiResourceId - && IpSecAlgorithm.equals(lhs.mEncryption, rhs.mEncryption) - && IpSecAlgorithm.equals(lhs.mAuthentication, rhs.mAuthentication)); - } - } - - private final Flow[] mFlow = new Flow[] {new Flow(), new Flow()}; + // Authenticated Encryption Algorithm + private IpSecAlgorithm mAuthenticatedEncryption; // For tunnel mode IPv4 UDP Encapsulation // IpSecTransform#ENCAP_ESP_*, such as ENCAP_ESP_OVER_UDP_IKE @@ -100,36 +70,37 @@ public final class IpSecConfig implements Parcelable { mMode = mode; } - /** Set the local IP address for Tunnel mode */ - public void setLocalAddress(String localAddress) { - mLocalAddress = localAddress; + /** Set the source IP addres for this IPsec transform */ + public void setSourceAddress(String sourceAddress) { + mSourceAddress = sourceAddress; } - /** Set the remote IP address for this IPsec transform */ - public void setRemoteAddress(String remoteAddress) { - mRemoteAddress = remoteAddress; + /** Set the destination IP address for this IPsec transform */ + public void setDestinationAddress(String destinationAddress) { + mDestinationAddress = destinationAddress; } - /** Set the SPI for a given direction by resource ID */ - public void setSpiResourceId(int direction, int resourceId) { - mFlow[direction].mSpiResourceId = resourceId; + /** Set the SPI by resource ID */ + public void setSpiResourceId(int resourceId) { + mSpiResourceId = resourceId; } - /** Set the encryption algorithm for a given direction */ - public void setEncryption(int direction, IpSecAlgorithm encryption) { - mFlow[direction].mEncryption = encryption; + /** Set the encryption algorithm */ + public void setEncryption(IpSecAlgorithm encryption) { + mEncryption = encryption; } - /** Set the authentication algorithm for a given direction */ - public void setAuthentication(int direction, IpSecAlgorithm authentication) { - mFlow[direction].mAuthentication = authentication; + /** Set the authentication algorithm */ + public void setAuthentication(IpSecAlgorithm authentication) { + mAuthentication = authentication; } - /** Set the authenticated encryption algorithm for a given direction */ - public void setAuthenticatedEncryption(int direction, IpSecAlgorithm authenticatedEncryption) { - mFlow[direction].mAuthenticatedEncryption = authenticatedEncryption; + /** Set the authenticated encryption algorithm */ + public void setAuthenticatedEncryption(IpSecAlgorithm authenticatedEncryption) { + mAuthenticatedEncryption = authenticatedEncryption; } + /** Set the underlying network that will carry traffic for this transform */ public void setNetwork(Network network) { mNetwork = network; } @@ -155,28 +126,28 @@ public final class IpSecConfig implements Parcelable { return mMode; } - public String getLocalAddress() { - return mLocalAddress; + public String getSourceAddress() { + return mSourceAddress; } - public int getSpiResourceId(int direction) { - return mFlow[direction].mSpiResourceId; + public int getSpiResourceId() { + return mSpiResourceId; } - public String getRemoteAddress() { - return mRemoteAddress; + public String getDestinationAddress() { + return mDestinationAddress; } - public IpSecAlgorithm getEncryption(int direction) { - return mFlow[direction].mEncryption; + public IpSecAlgorithm getEncryption() { + return mEncryption; } - public IpSecAlgorithm getAuthentication(int direction) { - return mFlow[direction].mAuthentication; + public IpSecAlgorithm getAuthentication() { + return mAuthentication; } - public IpSecAlgorithm getAuthenticatedEncryption(int direction) { - return mFlow[direction].mAuthenticatedEncryption; + public IpSecAlgorithm getAuthenticatedEncryption() { + return mAuthenticatedEncryption; } public Network getNetwork() { @@ -209,17 +180,13 @@ public final class IpSecConfig implements Parcelable { @Override public void writeToParcel(Parcel out, int flags) { out.writeInt(mMode); - out.writeString(mLocalAddress); - out.writeString(mRemoteAddress); + out.writeString(mSourceAddress); + out.writeString(mDestinationAddress); out.writeParcelable(mNetwork, flags); - out.writeInt(mFlow[IpSecTransform.DIRECTION_IN].mSpiResourceId); - out.writeParcelable(mFlow[IpSecTransform.DIRECTION_IN].mEncryption, flags); - out.writeParcelable(mFlow[IpSecTransform.DIRECTION_IN].mAuthentication, flags); - out.writeParcelable(mFlow[IpSecTransform.DIRECTION_IN].mAuthenticatedEncryption, flags); - out.writeInt(mFlow[IpSecTransform.DIRECTION_OUT].mSpiResourceId); - out.writeParcelable(mFlow[IpSecTransform.DIRECTION_OUT].mEncryption, flags); - out.writeParcelable(mFlow[IpSecTransform.DIRECTION_OUT].mAuthentication, flags); - out.writeParcelable(mFlow[IpSecTransform.DIRECTION_OUT].mAuthenticatedEncryption, flags); + out.writeInt(mSpiResourceId); + out.writeParcelable(mEncryption, flags); + out.writeParcelable(mAuthentication, flags); + out.writeParcelable(mAuthenticatedEncryption, flags); out.writeInt(mEncapType); out.writeInt(mEncapSocketResourceId); out.writeInt(mEncapRemotePort); @@ -231,22 +198,15 @@ public final class IpSecConfig implements Parcelable { private IpSecConfig(Parcel in) { mMode = in.readInt(); - mLocalAddress = in.readString(); - mRemoteAddress = in.readString(); + mSourceAddress = in.readString(); + mDestinationAddress = in.readString(); mNetwork = (Network) in.readParcelable(Network.class.getClassLoader()); - mFlow[IpSecTransform.DIRECTION_IN].mSpiResourceId = in.readInt(); - mFlow[IpSecTransform.DIRECTION_IN].mEncryption = + mSpiResourceId = in.readInt(); + mEncryption = (IpSecAlgorithm) in.readParcelable(IpSecAlgorithm.class.getClassLoader()); - mFlow[IpSecTransform.DIRECTION_IN].mAuthentication = + mAuthentication = (IpSecAlgorithm) in.readParcelable(IpSecAlgorithm.class.getClassLoader()); - mFlow[IpSecTransform.DIRECTION_IN].mAuthenticatedEncryption = - (IpSecAlgorithm) in.readParcelable(IpSecAlgorithm.class.getClassLoader()); - mFlow[IpSecTransform.DIRECTION_OUT].mSpiResourceId = in.readInt(); - mFlow[IpSecTransform.DIRECTION_OUT].mEncryption = - (IpSecAlgorithm) in.readParcelable(IpSecAlgorithm.class.getClassLoader()); - mFlow[IpSecTransform.DIRECTION_OUT].mAuthentication = - (IpSecAlgorithm) in.readParcelable(IpSecAlgorithm.class.getClassLoader()); - mFlow[IpSecTransform.DIRECTION_OUT].mAuthenticatedEncryption = + mAuthenticatedEncryption = (IpSecAlgorithm) in.readParcelable(IpSecAlgorithm.class.getClassLoader()); mEncapType = in.readInt(); mEncapSocketResourceId = in.readInt(); @@ -260,10 +220,10 @@ public final class IpSecConfig implements Parcelable { strBuilder .append("{mMode=") .append(mMode == IpSecTransform.MODE_TUNNEL ? "TUNNEL" : "TRANSPORT") - .append(", mLocalAddress=") - .append(mLocalAddress) - .append(", mRemoteAddress=") - .append(mRemoteAddress) + .append(", mSourceAddress=") + .append(mSourceAddress) + .append(", mDestinationAddress=") + .append(mDestinationAddress) .append(", mNetwork=") .append(mNetwork) .append(", mEncapType=") @@ -274,10 +234,14 @@ public final class IpSecConfig implements Parcelable { .append(mEncapRemotePort) .append(", mNattKeepaliveInterval=") .append(mNattKeepaliveInterval) - .append(", mFlow[OUT]=") - .append(mFlow[IpSecTransform.DIRECTION_OUT]) - .append(", mFlow[IN]=") - .append(mFlow[IpSecTransform.DIRECTION_IN]) + .append("{mSpiResourceId=") + .append(mSpiResourceId) + .append(", mEncryption=") + .append(mEncryption) + .append(", mAuthentication=") + .append(mAuthentication) + .append(", mAuthenticatedEncryption=") + .append(mAuthenticatedEncryption) .append("}"); return strBuilder.toString(); @@ -299,17 +263,18 @@ public final class IpSecConfig implements Parcelable { public static boolean equals(IpSecConfig lhs, IpSecConfig rhs) { if (lhs == null || rhs == null) return (lhs == rhs); return (lhs.mMode == rhs.mMode - && lhs.mLocalAddress.equals(rhs.mLocalAddress) - && lhs.mRemoteAddress.equals(rhs.mRemoteAddress) + && lhs.mSourceAddress.equals(rhs.mSourceAddress) + && lhs.mDestinationAddress.equals(rhs.mDestinationAddress) && ((lhs.mNetwork != null && lhs.mNetwork.equals(rhs.mNetwork)) || (lhs.mNetwork == rhs.mNetwork)) && lhs.mEncapType == rhs.mEncapType && lhs.mEncapSocketResourceId == rhs.mEncapSocketResourceId && lhs.mEncapRemotePort == rhs.mEncapRemotePort && lhs.mNattKeepaliveInterval == rhs.mNattKeepaliveInterval - && IpSecConfig.Flow.equals(lhs.mFlow[IpSecTransform.DIRECTION_OUT], - rhs.mFlow[IpSecTransform.DIRECTION_OUT]) - && IpSecConfig.Flow.equals(lhs.mFlow[IpSecTransform.DIRECTION_IN], - rhs.mFlow[IpSecTransform.DIRECTION_IN])); + && lhs.mSpiResourceId == rhs.mSpiResourceId + && IpSecAlgorithm.equals(lhs.mEncryption, rhs.mEncryption) + && IpSecAlgorithm.equals( + lhs.mAuthenticatedEncryption, rhs.mAuthenticatedEncryption) + && IpSecAlgorithm.equals(lhs.mAuthentication, rhs.mAuthentication)); } } diff --git a/core/java/android/net/IpSecManager.java b/core/java/android/net/IpSecManager.java index 34cfa9b215..67d4fcac97 100644 --- a/core/java/android/net/IpSecManager.java +++ b/core/java/android/net/IpSecManager.java @@ -17,6 +17,7 @@ package android.net; import static com.android.internal.util.Preconditions.checkNotNull; +import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.SystemService; import android.annotation.TestApi; @@ -33,6 +34,8 @@ import dalvik.system.CloseGuard; import java.io.FileDescriptor; import java.io.IOException; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.net.DatagramSocket; import java.net.InetAddress; import java.net.Socket; @@ -52,6 +55,23 @@ import java.net.Socket; public final class IpSecManager { private static final String TAG = "IpSecManager"; + /** + * For direction-specific attributes of an {@link IpSecTransform}, indicates that an attribute + * applies to traffic towards the host. + */ + public static final int DIRECTION_IN = 0; + + /** + * For direction-specific attributes of an {@link IpSecTransform}, indicates that an attribute + * applies to traffic from the host. + */ + public static final int DIRECTION_OUT = 1; + + /** @hide */ + @IntDef(value = {DIRECTION_IN, DIRECTION_OUT}) + @Retention(RetentionPolicy.SOURCE) + public @interface PolicyDirection {} + /** * The Security Parameter Index (SPI) 0 indicates an unknown or invalid index. * @@ -125,7 +145,7 @@ public final class IpSecManager { */ public static final class SecurityParameterIndex implements AutoCloseable { private final IIpSecService mService; - private final InetAddress mRemoteAddress; + private final InetAddress mDestinationAddress; private final CloseGuard mCloseGuard = CloseGuard.get(); private int mSpi = INVALID_SECURITY_PARAMETER_INDEX; private int mResourceId = INVALID_RESOURCE_ID; @@ -164,14 +184,14 @@ public final class IpSecManager { } private SecurityParameterIndex( - @NonNull IIpSecService service, int direction, InetAddress remoteAddress, int spi) + @NonNull IIpSecService service, InetAddress destinationAddress, int spi) throws ResourceUnavailableException, SpiUnavailableException { mService = service; - mRemoteAddress = remoteAddress; + mDestinationAddress = destinationAddress; try { IpSecSpiResponse result = mService.allocateSecurityParameterIndex( - direction, remoteAddress.getHostAddress(), spi, new Binder()); + destinationAddress.getHostAddress(), spi, new Binder()); if (result == null) { throw new NullPointerException("Received null response from IpSecService"); @@ -216,25 +236,23 @@ public final class IpSecManager { } /** - * Reserve a random SPI for traffic bound to or from the specified remote address. + * Reserve a random SPI for traffic bound to or from the specified destination address. * *

If successful, this SPI is guaranteed available until released by a call to {@link * SecurityParameterIndex#close()}. * - * @param direction {@link IpSecTransform#DIRECTION_IN} or {@link IpSecTransform#DIRECTION_OUT} - * @param remoteAddress address of the remote. SPIs must be unique for each remoteAddress + * @param destinationAddress the destination address for traffic bearing the requested SPI. + * For inbound traffic, the destination should be an address currently assigned on-device. * @return the reserved SecurityParameterIndex - * @throws ResourceUnavailableException indicating that too many SPIs are currently allocated - * for this user - * @throws SpiUnavailableException indicating that a particular SPI cannot be reserved + * @throws {@link #ResourceUnavailableException} indicating that too many SPIs are + * currently allocated for this user */ - public SecurityParameterIndex allocateSecurityParameterIndex( - int direction, InetAddress remoteAddress) throws ResourceUnavailableException { + public SecurityParameterIndex allocateSecurityParameterIndex(InetAddress destinationAddress) + throws ResourceUnavailableException { try { return new SecurityParameterIndex( mService, - direction, - remoteAddress, + destinationAddress, IpSecManager.INVALID_SECURITY_PARAMETER_INDEX); } catch (SpiUnavailableException unlikely) { throw new ResourceUnavailableException("No SPIs available"); @@ -242,26 +260,27 @@ public final class IpSecManager { } /** - * Reserve the requested SPI for traffic bound to or from the specified remote address. + * Reserve the requested SPI for traffic bound to or from the specified destination address. * *

If successful, this SPI is guaranteed available until released by a call to {@link * SecurityParameterIndex#close()}. * - * @param direction {@link IpSecTransform#DIRECTION_IN} or {@link IpSecTransform#DIRECTION_OUT} - * @param remoteAddress address of the remote. SPIs must be unique for each remoteAddress + * @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 * @return the reserved SecurityParameterIndex - * @throws ResourceUnavailableException indicating that too many SPIs are currently allocated - * for this user - * @throws SpiUnavailableException indicating that the requested SPI could not be reserved + * @throws {@link #ResourceUnavailableException} indicating that too many SPIs are + * currently allocated for this user + * @throws {@link #SpiUnavailableException} indicating that the requested SPI could not be + * reserved */ public SecurityParameterIndex allocateSecurityParameterIndex( - int direction, InetAddress remoteAddress, int requestedSpi) + InetAddress destinationAddress, int requestedSpi) throws SpiUnavailableException, ResourceUnavailableException { if (requestedSpi == IpSecManager.INVALID_SECURITY_PARAMETER_INDEX) { throw new IllegalArgumentException("Requested SPI must be a valid (non-zero) SPI"); } - return new SecurityParameterIndex(mService, direction, remoteAddress, requestedSpi); + return new SecurityParameterIndex(mService, destinationAddress, requestedSpi); } /** @@ -269,14 +288,14 @@ public final class IpSecManager { * *

This applies transport mode encapsulation to the given socket. Once applied, I/O on the * socket will be encapsulated according to the parameters of the {@code IpSecTransform}. When - * the transform is removed from the socket by calling {@link #removeTransportModeTransform}, + * the transform is removed from the socket by calling {@link #removeTransportModeTransforms}, * unprotected traffic can resume on that socket. * *

For security reasons, the destination address of any traffic on the socket must match the * remote {@code InetAddress} of the {@code IpSecTransform}. Attempts to send traffic to any * other IP address will result in an IOException. In addition, reads and writes on the socket * will throw IOException if the user deactivates the transform (by calling {@link - * IpSecTransform#close()}) without calling {@link #removeTransportModeTransform}. + * IpSecTransform#close()}) without calling {@link #removeTransportModeTransforms}. * *

Rekey Procedure

* @@ -287,14 +306,15 @@ public final class IpSecManager { * in-flight packets have been received. * * @param socket a stream socket + * @param direction the policy direction either {@link #DIRECTION_IN} or {@link #DIRECTION_OUT} * @param transform a transport mode {@code IpSecTransform} * @throws IOException indicating that the transform could not be applied - * @hide */ - public void applyTransportModeTransform(Socket socket, IpSecTransform transform) + public void applyTransportModeTransform( + Socket socket, int direction, IpSecTransform transform) throws IOException { try (ParcelFileDescriptor pfd = ParcelFileDescriptor.fromSocket(socket)) { - applyTransportModeTransform(pfd, transform); + applyTransportModeTransform(pfd, direction, transform); } } @@ -303,14 +323,14 @@ public final class IpSecManager { * *

This applies transport mode encapsulation to the given socket. Once applied, I/O on the * socket will be encapsulated according to the parameters of the {@code IpSecTransform}. When - * the transform is removed from the socket by calling {@link #removeTransportModeTransform}, + * the transform is removed from the socket by calling {@link #removeTransportModeTransforms}, * unprotected traffic can resume on that socket. * *

For security reasons, the destination address of any traffic on the socket must match the * remote {@code InetAddress} of the {@code IpSecTransform}. Attempts to send traffic to any * other IP address will result in an IOException. In addition, reads and writes on the socket * will throw IOException if the user deactivates the transform (by calling {@link - * IpSecTransform#close()}) without calling {@link #removeTransportModeTransform}. + * IpSecTransform#close()}) without calling {@link #removeTransportModeTransforms}. * *

Rekey Procedure

* @@ -321,14 +341,14 @@ public final class IpSecManager { * in-flight packets have been received. * * @param socket a datagram socket + * @param direction the policy direction either DIRECTION_IN or DIRECTION_OUT * @param transform a transport mode {@code IpSecTransform} * @throws IOException indicating that the transform could not be applied - * @hide */ - public void applyTransportModeTransform(DatagramSocket socket, IpSecTransform transform) - throws IOException { + public void applyTransportModeTransform( + DatagramSocket socket, int direction, IpSecTransform transform) throws IOException { try (ParcelFileDescriptor pfd = ParcelFileDescriptor.fromDatagramSocket(socket)) { - applyTransportModeTransform(pfd, transform); + applyTransportModeTransform(pfd, direction, transform); } } @@ -337,14 +357,14 @@ public final class IpSecManager { * *

This applies transport mode encapsulation to the given socket. Once applied, I/O on the * socket will be encapsulated according to the parameters of the {@code IpSecTransform}. When - * the transform is removed from the socket by calling {@link #removeTransportModeTransform}, + * the transform is removed from the socket by calling {@link #removeTransportModeTransforms}, * unprotected traffic can resume on that socket. * *

For security reasons, the destination address of any traffic on the socket must match the * remote {@code InetAddress} of the {@code IpSecTransform}. Attempts to send traffic to any * other IP address will result in an IOException. In addition, reads and writes on the socket * will throw IOException if the user deactivates the transform (by calling {@link - * IpSecTransform#close()}) without calling {@link #removeTransportModeTransform}. + * IpSecTransform#close()}) without calling {@link #removeTransportModeTransforms}. * *

Rekey Procedure

* @@ -355,24 +375,27 @@ public final class IpSecManager { * in-flight packets have been received. * * @param socket a socket file descriptor + * @param direction the policy direction either DIRECTION_IN or DIRECTION_OUT * @param transform a transport mode {@code IpSecTransform} * @throws IOException indicating that the transform could not be applied */ - public void applyTransportModeTransform(FileDescriptor socket, IpSecTransform transform) + public void applyTransportModeTransform( + FileDescriptor socket, int direction, IpSecTransform transform) throws IOException { // We dup() the FileDescriptor here because if we don't, then the ParcelFileDescriptor() // constructor takes control and closes the user's FD when we exit the method // This is behaviorally the same as the other versions, but the PFD constructor does not // dup() automatically, whereas PFD.fromSocket() and PDF.fromDatagramSocket() do dup(). try (ParcelFileDescriptor pfd = ParcelFileDescriptor.dup(socket)) { - applyTransportModeTransform(pfd, transform); + applyTransportModeTransform(pfd, direction, transform); } } /* Call down to activate a transform */ - private void applyTransportModeTransform(ParcelFileDescriptor pfd, IpSecTransform transform) { + private void applyTransportModeTransform( + ParcelFileDescriptor pfd, int direction, IpSecTransform transform) throws IOException { try { - mService.applyTransportModeTransform(pfd, transform.getResourceId()); + mService.applyTransportModeTransform(pfd, direction, transform.getResourceId()); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } @@ -407,12 +430,11 @@ public final class IpSecManager { * @param socket a socket that previously had a transform applied to it * @param transform the IPsec Transform that was previously applied to the given socket * @throws IOException indicating that the transform could not be removed from the socket - * @hide */ - public void removeTransportModeTransform(Socket socket, IpSecTransform transform) + public void removeTransportModeTransforms(Socket socket, IpSecTransform transform) throws IOException { try (ParcelFileDescriptor pfd = ParcelFileDescriptor.fromSocket(socket)) { - removeTransportModeTransform(pfd, transform); + removeTransportModeTransforms(pfd, transform); } } @@ -430,12 +452,11 @@ public final class IpSecManager { * @param socket a socket that previously had a transform applied to it * @param transform the IPsec Transform that was previously applied to the given socket * @throws IOException indicating that the transform could not be removed from the socket - * @hide */ - public void removeTransportModeTransform(DatagramSocket socket, IpSecTransform transform) + public void removeTransportModeTransforms(DatagramSocket socket, IpSecTransform transform) throws IOException { try (ParcelFileDescriptor pfd = ParcelFileDescriptor.fromDatagramSocket(socket)) { - removeTransportModeTransform(pfd, transform); + removeTransportModeTransforms(pfd, transform); } } @@ -454,17 +475,17 @@ public final class IpSecManager { * @param transform the IPsec Transform that was previously applied to the given socket * @throws IOException indicating that the transform could not be removed from the socket */ - public void removeTransportModeTransform(FileDescriptor socket, IpSecTransform transform) + public void removeTransportModeTransforms(FileDescriptor socket, IpSecTransform transform) throws IOException { try (ParcelFileDescriptor pfd = ParcelFileDescriptor.dup(socket)) { - removeTransportModeTransform(pfd, transform); + removeTransportModeTransforms(pfd, transform); } } /* Call down to remove a transform */ - private void removeTransportModeTransform(ParcelFileDescriptor pfd, IpSecTransform transform) { + private void removeTransportModeTransforms(ParcelFileDescriptor pfd, IpSecTransform transform) { try { - mService.removeTransportModeTransform(pfd, transform.getResourceId()); + mService.removeTransportModeTransforms(pfd, transform.getResourceId()); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } diff --git a/core/java/android/net/IpSecTransform.java b/core/java/android/net/IpSecTransform.java index 102ba6d94f..7b9b483092 100644 --- a/core/java/android/net/IpSecTransform.java +++ b/core/java/android/net/IpSecTransform.java @@ -38,13 +38,11 @@ import java.lang.annotation.RetentionPolicy; import java.net.InetAddress; /** - * This class represents an IPsec transform, which comprises security associations in one or both - * directions. + * This class represents a transform, which roughly corresponds to an IPsec Security Association. * *

Transforms are created using {@link IpSecTransform.Builder}. Each {@code IpSecTransform} - * object encapsulates the properties and state of an inbound and outbound IPsec security - * association. That includes, but is not limited to, algorithm choice, key material, and allocated - * system resources. + * object encapsulates the properties and state of an IPsec security association. That includes, + * but is not limited to, algorithm choice, key material, and allocated system resources. * * @see RFC 4301, Security Architecture for the * Internet Protocol @@ -52,23 +50,6 @@ import java.net.InetAddress; public final class IpSecTransform implements AutoCloseable { private static final String TAG = "IpSecTransform"; - /** - * For direction-specific attributes of an {@link IpSecTransform}, indicates that an attribute - * applies to traffic towards the host. - */ - public static final int DIRECTION_IN = 0; - - /** - * For direction-specific attributes of an {@link IpSecTransform}, indicates that an attribute - * applies to traffic from the host. - */ - public static final int DIRECTION_OUT = 1; - - /** @hide */ - @IntDef(value = {DIRECTION_IN, DIRECTION_OUT}) - @Retention(RetentionPolicy.SOURCE) - public @interface TransformDirection {} - /** @hide */ public static final int MODE_TRANSPORT = 0; @@ -170,7 +151,7 @@ public final class IpSecTransform implements AutoCloseable { * *

Deactivating a transform while it is still applied to a socket will result in errors on * that socket. Make sure to remove transforms by calling {@link - * IpSecManager#removeTransportModeTransform}. Note, removing an {@code IpSecTransform} from a + * IpSecManager#removeTransportModeTransforms}. Note, removing an {@code IpSecTransform} from a * socket will not deactivate it (because one transform may be applied to multiple sockets). * *

It is safe to call this method on a transform that has already been deactivated. @@ -272,85 +253,49 @@ public final class IpSecTransform implements AutoCloseable { private IpSecConfig mConfig; /** - * Set the encryption algorithm for the given direction. - * - *

If encryption is set for a direction without also providing an SPI for that direction, - * creation of an {@code IpSecTransform} will fail when attempting to build the transform. + * Set the encryption algorithm. * *

Encryption is mutually exclusive with authenticated encryption. * - * @param direction either {@link #DIRECTION_IN} or {@link #DIRECTION_OUT} * @param algo {@link IpSecAlgorithm} specifying the encryption to be applied. */ - public IpSecTransform.Builder setEncryption( - @TransformDirection int direction, IpSecAlgorithm algo) { + public IpSecTransform.Builder setEncryption(@NonNull IpSecAlgorithm algo) { // TODO: throw IllegalArgumentException if algo is not an encryption algorithm. - mConfig.setEncryption(direction, algo); + Preconditions.checkNotNull(algo); + mConfig.setEncryption(algo); return this; } /** - * Set the authentication (integrity) algorithm for the given direction. - * - *

If authentication is set for a direction without also providing an SPI for that - * direction, creation of an {@code IpSecTransform} will fail when attempting to build the - * transform. + * Set the authentication (integrity) algorithm. * *

Authentication is mutually exclusive with authenticated encryption. * - * @param direction either {@link #DIRECTION_IN} or {@link #DIRECTION_OUT} * @param algo {@link IpSecAlgorithm} specifying the authentication to be applied. */ - public IpSecTransform.Builder setAuthentication( - @TransformDirection int direction, IpSecAlgorithm algo) { + public IpSecTransform.Builder setAuthentication(@NonNull IpSecAlgorithm algo) { // TODO: throw IllegalArgumentException if algo is not an authentication algorithm. - mConfig.setAuthentication(direction, algo); + Preconditions.checkNotNull(algo); + mConfig.setAuthentication(algo); return this; } /** - * Set the authenticated encryption algorithm for the given direction. + * Set the authenticated encryption algorithm. * - *

If an authenticated encryption algorithm is set for a given direction without also - * providing an SPI for that direction, creation of an {@code IpSecTransform} will fail when - * attempting to build the transform. - * - *

The Authenticated Encryption (AE) class of algorithms are also known as Authenticated - * Encryption with Associated Data (AEAD) algorithms, or Combined mode algorithms (as - * referred to in RFC 4301). + *

The Authenticated Encryption (AE) class of algorithms are also known as + * Authenticated Encryption with Associated Data (AEAD) algorithms, or Combined mode + * algorithms (as referred to in + * RFC 4301). * *

Authenticated encryption is mutually exclusive with encryption and authentication. * - * @param direction either {@link #DIRECTION_IN} or {@link #DIRECTION_OUT} * @param algo {@link IpSecAlgorithm} specifying the authenticated encryption algorithm to * be applied. */ - public IpSecTransform.Builder setAuthenticatedEncryption( - @TransformDirection int direction, IpSecAlgorithm algo) { - mConfig.setAuthenticatedEncryption(direction, algo); - return this; - } - - /** - * Set the SPI for the given direction. - * - *

Because IPsec operates at the IP layer, this 32-bit identifier uniquely identifies - * packets to a given destination address. To prevent SPI collisions, values should be - * reserved by calling {@link IpSecManager#allocateSecurityParameterIndex}. - * - *

If the SPI and algorithms are omitted for one direction, traffic in that direction - * will not be encrypted or authenticated. - * - * @param direction either {@link #DIRECTION_IN} or {@link #DIRECTION_OUT} - * @param spi a unique {@link IpSecManager.SecurityParameterIndex} to identify transformed - * traffic - */ - 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()); + public IpSecTransform.Builder setAuthenticatedEncryption(@NonNull IpSecAlgorithm algo) { + Preconditions.checkNotNull(algo); + mConfig.setAuthenticatedEncryption(algo); return this; } @@ -363,7 +308,8 @@ public final class IpSecTransform implements AutoCloseable { * @hide */ @SystemApi - public IpSecTransform.Builder setUnderlyingNetwork(Network net) { + public IpSecTransform.Builder setUnderlyingNetwork(@NonNull Network net) { + Preconditions.checkNotNull(net); mConfig.setNetwork(net); return this; } @@ -382,7 +328,8 @@ public final class IpSecTransform implements AutoCloseable { * encapsulated traffic. In the case of IKEv2, this should be port 4500. */ public IpSecTransform.Builder setIpv4Encapsulation( - IpSecManager.UdpEncapsulationSocket localSocket, int remotePort) { + @NonNull IpSecManager.UdpEncapsulationSocket localSocket, int remotePort) { + Preconditions.checkNotNull(localSocket); mConfig.setEncapType(ENCAP_ESPINUDP); if (localSocket.getResourceId() == INVALID_RESOURCE_ID) { throw new IllegalArgumentException("Invalid UdpEncapsulationSocket"); @@ -419,24 +366,33 @@ public final class IpSecTransform implements AutoCloseable { * will not affect any network traffic until it has been applied to one or more sockets. * * @see IpSecManager#applyTransportModeTransform - * @param remoteAddress the remote {@code InetAddress} of traffic on sockets that will use - * this transform + * @param sourceAddress the source {@code InetAddress} of traffic on sockets that will use + * this transform; this address must belong to the Network used by all sockets that + * utilize this transform; if provided, then only traffic originating from the + * specified source address will be processed. + * @param spi a unique {@link IpSecManager.SecurityParameterIndex} to identify transformed + * traffic * @throws IllegalArgumentException indicating that a particular combination of transform * properties is invalid - * @throws IpSecManager.ResourceUnavailableException indicating that too many transforms are - * active + * @throws IpSecManager.ResourceUnavailableException indicating that too many transforms + * are active * @throws IpSecManager.SpiUnavailableException indicating the rare case where an SPI * collides with an existing transform * @throws IOException indicating other errors */ - public IpSecTransform buildTransportModeTransform(InetAddress remoteAddress) + public IpSecTransform buildTransportModeTransform( + @NonNull InetAddress sourceAddress, + @NonNull IpSecManager.SecurityParameterIndex spi) throws IpSecManager.ResourceUnavailableException, IpSecManager.SpiUnavailableException, IOException { - if (remoteAddress == null) { - throw new IllegalArgumentException("Remote address may not be null or empty!"); + Preconditions.checkNotNull(sourceAddress); + Preconditions.checkNotNull(spi); + if (spi.getResourceId() == INVALID_RESOURCE_ID) { + throw new IllegalArgumentException("Invalid SecurityParameterIndex"); } mConfig.setMode(MODE_TRANSPORT); - mConfig.setRemoteAddress(remoteAddress.getHostAddress()); + mConfig.setSourceAddress(sourceAddress.getHostAddress()); + mConfig.setSpiResourceId(spi.getResourceId()); // FIXME: modifying a builder after calling build can change the built transform. return new IpSecTransform(mContext, mConfig).activate(); } @@ -445,26 +401,33 @@ public final class IpSecTransform implements AutoCloseable { * Build and return an {@link IpSecTransform} object as a Tunnel Mode Transform. Some * parameters have interdependencies that are checked at build time. * - * @param localAddress the {@link InetAddress} that provides the local endpoint for this + * @param sourceAddress the {@link InetAddress} that provides the source address for this * IPsec tunnel. This is almost certainly an address belonging to the {@link Network} * that will originate the traffic, which is set as the {@link #setUnderlyingNetwork}. - * @param remoteAddress the {@link InetAddress} representing the remote endpoint of this - * IPsec tunnel. + * @param spi a unique {@link IpSecManager.SecurityParameterIndex} to identify transformed + * traffic * @throws IllegalArgumentException indicating that a particular combination of transform * properties is invalid. + * @throws IpSecManager.ResourceUnavailableException indicating that too many transforms + * are active + * @throws IpSecManager.SpiUnavailableException indicating the rare case where an SPI + * collides with an existing transform + * @throws IOException indicating other errors * @hide */ public IpSecTransform buildTunnelModeTransform( - InetAddress localAddress, InetAddress remoteAddress) { - if (localAddress == null) { - throw new IllegalArgumentException("Local address may not be null or empty!"); + @NonNull InetAddress sourceAddress, + @NonNull IpSecManager.SecurityParameterIndex spi) + throws IpSecManager.ResourceUnavailableException, + IpSecManager.SpiUnavailableException, IOException { + Preconditions.checkNotNull(sourceAddress); + Preconditions.checkNotNull(spi); + if (spi.getResourceId() == INVALID_RESOURCE_ID) { + throw new IllegalArgumentException("Invalid SecurityParameterIndex"); } - 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); + mConfig.setSourceAddress(sourceAddress.getHostAddress()); + mConfig.setSpiResourceId(spi.getResourceId()); return new IpSecTransform(mContext, mConfig); } diff --git a/services/core/java/com/android/server/IpSecService.java b/services/core/java/com/android/server/IpSecService.java index 02cfe3dc75..9d228c3d0a 100644 --- a/services/core/java/com/android/server/IpSecService.java +++ b/services/core/java/com/android/server/IpSecService.java @@ -25,6 +25,7 @@ import static android.system.OsConstants.SOCK_DGRAM; import static com.android.internal.util.Preconditions.checkNotNull; import android.content.Context; +import android.net.ConnectivityManager; import android.net.IIpSecService; import android.net.INetd; import android.net.IpSecAlgorithm; @@ -62,7 +63,6 @@ import java.net.InetSocketAddress; import java.net.UnknownHostException; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.atomic.AtomicInteger; import libcore.io.IoUtils; @@ -83,7 +83,7 @@ public class IpSecService extends IIpSecService.Stub { private static final String NETD_SERVICE_NAME = "netd"; private static final int[] DIRECTIONS = - new int[] {IpSecTransform.DIRECTION_OUT, IpSecTransform.DIRECTION_IN}; + new int[] {IpSecManager.DIRECTION_OUT, IpSecManager.DIRECTION_IN}; private static final int NETD_FETCH_TIMEOUT_MS = 5000; // ms private static final int MAX_PORT_BIND_ATTEMPTS = 10; @@ -104,10 +104,10 @@ public class IpSecService extends IIpSecService.Stub { private final Context mContext; /** - * 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). + * 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; @@ -536,14 +536,14 @@ public class IpSecService extends IIpSecService.Stub { private final class TransformRecord extends KernelResourceRecord { private final IpSecConfig mConfig; - private final SpiRecord[] mSpis; + private final SpiRecord mSpi; private final EncapSocketRecord mSocket; TransformRecord( - int resourceId, IpSecConfig config, SpiRecord[] spis, EncapSocketRecord socket) { + int resourceId, IpSecConfig config, SpiRecord spi, EncapSocketRecord socket) { super(resourceId); mConfig = config; - mSpis = spis; + mSpi = spi; mSocket = socket; } @@ -551,29 +551,26 @@ public class IpSecService extends IIpSecService.Stub { return mConfig; } - public SpiRecord getSpiRecord(int direction) { - return mSpis[direction]; + public SpiRecord getSpiRecord() { + return mSpi; } /** always guarded by IpSecService#this */ @Override public void freeUnderlyingResources() { - for (int direction : DIRECTIONS) { - int spi = mSpis[direction].getSpi(); - try { - mSrvConfig - .getNetdInstance() - .ipSecDeleteSecurityAssociation( - mResourceId, - direction, - mConfig.getLocalAddress(), - mConfig.getRemoteAddress(), - spi); - } catch (ServiceSpecificException e) { - // FIXME: get the error code and throw is at an IOException from Errno Exception - } catch (RemoteException e) { - Log.e(TAG, "Failed to delete SA with ID: " + mResourceId); - } + int spi = mSpi.getSpi(); + try { + mSrvConfig + .getNetdInstance() + .ipSecDeleteSecurityAssociation( + mResourceId, + mConfig.getSourceAddress(), + mConfig.getDestinationAddress(), + spi); + } catch (ServiceSpecificException e) { + // FIXME: get the error code and throw is at an IOException from Errno Exception + } catch (RemoteException e) { + Log.e(TAG, "Failed to delete SA with ID: " + mResourceId); } getResourceTracker().give(); @@ -597,10 +594,8 @@ public class IpSecService extends IIpSecService.Stub { .append(super.toString()) .append(", mSocket=") .append(mSocket) - .append(", mSpis[OUT].mResourceId=") - .append(mSpis[IpSecTransform.DIRECTION_OUT].mResourceId) - .append(", mSpis[IN].mResourceId=") - .append(mSpis[IpSecTransform.DIRECTION_IN].mResourceId) + .append(", mSpi.mResourceId=") + .append(mSpi.mResourceId) .append(", mConfig=") .append(mConfig) .append("}"); @@ -609,23 +604,16 @@ public class IpSecService extends IIpSecService.Stub { } private final class SpiRecord extends KernelResourceRecord { - private final int mDirection; - private final String mLocalAddress; - private final String mRemoteAddress; + private final String mSourceAddress; + private final String mDestinationAddress; private int mSpi; private boolean mOwnedByTransform = false; - SpiRecord( - int resourceId, - int direction, - String localAddress, - String remoteAddress, - int spi) { + SpiRecord(int resourceId, String sourceAddress, String destinationAddress, int spi) { super(resourceId); - mDirection = direction; - mLocalAddress = localAddress; - mRemoteAddress = remoteAddress; + mSourceAddress = sourceAddress; + mDestinationAddress = destinationAddress; mSpi = spi; } @@ -646,7 +634,7 @@ public class IpSecService extends IIpSecService.Stub { mSrvConfig .getNetdInstance() .ipSecDeleteSecurityAssociation( - mResourceId, mDirection, mLocalAddress, mRemoteAddress, mSpi); + mResourceId, mSourceAddress, mDestinationAddress, mSpi); } catch (ServiceSpecificException e) { // FIXME: get the error code and throw is at an IOException from Errno Exception } catch (RemoteException e) { @@ -662,6 +650,10 @@ public class IpSecService extends IIpSecService.Stub { return mSpi; } + public String getDestinationAddress() { + return mDestinationAddress; + } + public void setOwnedByTransform() { if (mOwnedByTransform) { // Programming error @@ -689,12 +681,10 @@ public class IpSecService extends IIpSecService.Stub { .append(super.toString()) .append(", mSpi=") .append(mSpi) - .append(", mDirection=") - .append(mDirection) - .append(", mLocalAddress=") - .append(mLocalAddress) - .append(", mRemoteAddress=") - .append(mRemoteAddress) + .append(", mSourceAddress=") + .append(mSourceAddress) + .append(", mDestinationAddress=") + .append(mDestinationAddress) .append(", mOwnedByTransform=") .append(mOwnedByTransform) .append("}"); @@ -772,14 +762,17 @@ public class IpSecService extends IIpSecService.Stub { /** @hide */ @VisibleForTesting public IpSecService(Context context, IpSecServiceConfiguration config) { - this(context, config, (fd, uid) -> { - try{ - TrafficStats.setThreadStatsUid(uid); - TrafficStats.tagFileDescriptor(fd); - } finally { - TrafficStats.clearThreadStatsUid(); - } - }); + this( + context, + config, + (fd, uid) -> { + try { + TrafficStats.setThreadStatsUid(uid); + TrafficStats.tagFileDescriptor(fd); + } finally { + TrafficStats.clearThreadStatsUid(); + } + }); } /** @hide */ @@ -845,8 +838,8 @@ public class IpSecService extends IIpSecService.Stub { */ private static void checkDirection(int direction) { switch (direction) { - case IpSecTransform.DIRECTION_OUT: - case IpSecTransform.DIRECTION_IN: + case IpSecManager.DIRECTION_OUT: + case IpSecManager.DIRECTION_IN: return; } throw new IllegalArgumentException("Invalid Direction: " + direction); @@ -855,10 +848,8 @@ public class IpSecService extends IIpSecService.Stub { /** Get a new SPI and maintain the reservation in the system server */ @Override public synchronized IpSecSpiResponse allocateSecurityParameterIndex( - int direction, String remoteAddress, int requestedSpi, IBinder binder) - throws RemoteException { - checkDirection(direction); - checkInetAddress(remoteAddress); + String destinationAddress, int requestedSpi, IBinder binder) throws RemoteException { + checkInetAddress(destinationAddress); /* requestedSpi can be anything in the int range, so no check is needed. */ checkNotNull(binder, "Null Binder passed to allocateSecurityParameterIndex"); @@ -866,28 +857,21 @@ public class IpSecService extends IIpSecService.Stub { final int resourceId = mNextResourceId++; int spi = IpSecManager.INVALID_SECURITY_PARAMETER_INDEX; - String localAddress = ""; - try { if (!userRecord.mSpiQuotaTracker.isAvailable()) { return new IpSecSpiResponse( IpSecManager.Status.RESOURCE_UNAVAILABLE, INVALID_RESOURCE_ID, spi); } + spi = mSrvConfig .getNetdInstance() - .ipSecAllocateSpi( - resourceId, - direction, - localAddress, - remoteAddress, - requestedSpi); + .ipSecAllocateSpi(resourceId, "", destinationAddress, requestedSpi); Log.d(TAG, "Allocated SPI " + spi); userRecord.mSpiRecords.put( resourceId, new RefcountedResource( - new SpiRecord(resourceId, direction, localAddress, remoteAddress, spi), - binder)); + new SpiRecord(resourceId, "", destinationAddress, spi), binder)); } catch (ServiceSpecificException e) { // TODO: Add appropriate checks when other ServiceSpecificException types are supported return new IpSecSpiResponse( @@ -1032,27 +1016,27 @@ public class IpSecService extends IIpSecService.Stub { } @VisibleForTesting - void validateAlgorithms(IpSecConfig config, int direction) throws IllegalArgumentException { - IpSecAlgorithm auth = config.getAuthentication(direction); - IpSecAlgorithm crypt = config.getEncryption(direction); - IpSecAlgorithm aead = config.getAuthenticatedEncryption(direction); + void validateAlgorithms(IpSecConfig config) throws IllegalArgumentException { + IpSecAlgorithm auth = config.getAuthentication(); + IpSecAlgorithm crypt = config.getEncryption(); + IpSecAlgorithm aead = config.getAuthenticatedEncryption(); - // Validate the algorithm set - Preconditions.checkArgument( - aead != null || crypt != null || auth != null, - "No Encryption or Authentication algorithms specified"); - Preconditions.checkArgument( - auth == null || auth.isAuthentication(), - "Unsupported algorithm for Authentication"); - Preconditions.checkArgument( + // Validate the algorithm set + Preconditions.checkArgument( + aead != null || crypt != null || auth != null, + "No Encryption or Authentication algorithms specified"); + Preconditions.checkArgument( + auth == null || auth.isAuthentication(), + "Unsupported algorithm for Authentication"); + Preconditions.checkArgument( crypt == null || crypt.isEncryption(), "Unsupported algorithm for Encryption"); - Preconditions.checkArgument( - aead == null || aead.isAead(), - "Unsupported algorithm for Authenticated Encryption"); - Preconditions.checkArgument( - aead == null || (auth == null && crypt == null), - "Authenticated Encryption is mutually exclusive with other Authentication " - + "or Encryption algorithms"); + Preconditions.checkArgument( + aead == null || aead.isAead(), + "Unsupported algorithm for Authenticated Encryption"); + Preconditions.checkArgument( + aead == null || (auth == null && crypt == null), + "Authenticated Encryption is mutually exclusive with other Authentication " + + "or Encryption algorithms"); } /** @@ -1062,29 +1046,6 @@ public class IpSecService extends IIpSecService.Stub { private void checkIpSecConfig(IpSecConfig config) { UserRecord userRecord = mUserResourceTracker.getUserRecord(Binder.getCallingUid()); - if (config.getLocalAddress() == null) { - throw new IllegalArgumentException("Invalid null Local InetAddress"); - } - - if (config.getRemoteAddress() == null) { - throw new IllegalArgumentException("Invalid null Remote InetAddress"); - } - - switch (config.getMode()) { - case IpSecTransform.MODE_TRANSPORT: - if (!config.getLocalAddress().isEmpty()) { - throw new IllegalArgumentException("Non-empty Local Address"); - } - // Must be valid, and not a wildcard - checkInetAddress(config.getRemoteAddress()); - break; - case IpSecTransform.MODE_TUNNEL: - break; - default: - throw new IllegalArgumentException( - "Invalid IpSecTransform.mode: " + config.getMode()); - } - switch (config.getEncapType()) { case IpSecTransform.ENCAP_NONE: break; @@ -1103,11 +1064,36 @@ public class IpSecService extends IIpSecService.Stub { throw new IllegalArgumentException("Invalid Encap Type: " + config.getEncapType()); } - for (int direction : DIRECTIONS) { - validateAlgorithms(config, direction); + validateAlgorithms(config); - // Retrieve SPI record; will throw IllegalArgumentException if not found - userRecord.mSpiRecords.getResourceOrThrow(config.getSpiResourceId(direction)); + // Retrieve SPI record; will throw IllegalArgumentException if not found + SpiRecord s = userRecord.mSpiRecords.getResourceOrThrow(config.getSpiResourceId()); + + // If no remote address is supplied, then use one from the SPI. + if (TextUtils.isEmpty(config.getDestinationAddress())) { + config.setDestinationAddress(s.getDestinationAddress()); + } + + // All remote addresses must match + if (!config.getDestinationAddress().equals(s.getDestinationAddress())) { + throw new IllegalArgumentException("Mismatched remote addresseses."); + } + + // This check is technically redundant due to the chain of custody between the SPI and + // the IpSecConfig, but in the future if the dest is allowed to be set explicitly in + // the transform, this will prevent us from messing up. + checkInetAddress(config.getDestinationAddress()); + + // Require a valid source address for all transforms. + checkInetAddress(config.getSourceAddress()); + + switch (config.getMode()) { + case IpSecTransform.MODE_TRANSPORT: + case IpSecTransform.MODE_TUNNEL: + break; + default: + throw new IllegalArgumentException( + "Invalid IpSecTransform.mode: " + config.getMode()); } } @@ -1127,13 +1113,12 @@ public class IpSecService extends IIpSecService.Stub { UserRecord userRecord = mUserResourceTracker.getUserRecord(Binder.getCallingUid()); - // Avoid resizing by creating a dependency array of min-size 3 (1 UDP encap + 2 SPIs) - List dependencies = new ArrayList<>(3); + // Avoid resizing by creating a dependency array of min-size 2 (1 UDP encap + 1 SPI) + List dependencies = new ArrayList<>(2); if (!userRecord.mTransformQuotaTracker.isAvailable()) { return new IpSecTransformResponse(IpSecManager.Status.RESOURCE_UNAVAILABLE); } - SpiRecord[] spis = new SpiRecord[DIRECTIONS.length]; int encapType, encapLocalPort = 0, encapRemotePort = 0; EncapSocketRecord socketRecord = null; @@ -1149,51 +1134,46 @@ public class IpSecService extends IIpSecService.Stub { encapRemotePort = c.getEncapRemotePort(); } - for (int direction : DIRECTIONS) { - IpSecAlgorithm auth = c.getAuthentication(direction); - IpSecAlgorithm crypt = c.getEncryption(direction); - IpSecAlgorithm authCrypt = c.getAuthenticatedEncryption(direction); + IpSecAlgorithm auth = c.getAuthentication(); + IpSecAlgorithm crypt = c.getEncryption(); + IpSecAlgorithm authCrypt = c.getAuthenticatedEncryption(); - RefcountedResource refcountedSpiRecord = - userRecord.mSpiRecords.getRefcountedResourceOrThrow( - c.getSpiResourceId(direction)); - dependencies.add(refcountedSpiRecord); + RefcountedResource refcountedSpiRecord = + userRecord.mSpiRecords.getRefcountedResourceOrThrow(c.getSpiResourceId()); + dependencies.add(refcountedSpiRecord); + SpiRecord spiRecord = refcountedSpiRecord.getResource(); - spis[direction] = refcountedSpiRecord.getResource(); - int spi = spis[direction].getSpi(); - try { - mSrvConfig - .getNetdInstance() - .ipSecAddSecurityAssociation( - resourceId, - c.getMode(), - direction, - c.getLocalAddress(), - c.getRemoteAddress(), - (c.getNetwork() != null) ? c.getNetwork().getNetworkHandle() : 0, - spi, - (auth != null) ? auth.getName() : "", - (auth != null) ? auth.getKey() : new byte[] {}, - (auth != null) ? auth.getTruncationLengthBits() : 0, - (crypt != null) ? crypt.getName() : "", - (crypt != null) ? crypt.getKey() : new byte[] {}, - (crypt != null) ? crypt.getTruncationLengthBits() : 0, - (authCrypt != null) ? authCrypt.getName() : "", - (authCrypt != null) ? authCrypt.getKey() : new byte[] {}, - (authCrypt != null) ? authCrypt.getTruncationLengthBits() : 0, - encapType, - encapLocalPort, - encapRemotePort); - } catch (ServiceSpecificException e) { - // FIXME: get the error code and throw is at an IOException from Errno Exception - return new IpSecTransformResponse(IpSecManager.Status.RESOURCE_UNAVAILABLE); - } + try { + mSrvConfig + .getNetdInstance() + .ipSecAddSecurityAssociation( + resourceId, + c.getMode(), + c.getSourceAddress(), + c.getDestinationAddress(), + (c.getNetwork() != null) ? c.getNetwork().netId : 0, + spiRecord.getSpi(), + (auth != null) ? auth.getName() : "", + (auth != null) ? auth.getKey() : new byte[] {}, + (auth != null) ? auth.getTruncationLengthBits() : 0, + (crypt != null) ? crypt.getName() : "", + (crypt != null) ? crypt.getKey() : new byte[] {}, + (crypt != null) ? crypt.getTruncationLengthBits() : 0, + (authCrypt != null) ? authCrypt.getName() : "", + (authCrypt != null) ? authCrypt.getKey() : new byte[] {}, + (authCrypt != null) ? authCrypt.getTruncationLengthBits() : 0, + encapType, + encapLocalPort, + encapRemotePort); + } catch (ServiceSpecificException e) { + // FIXME: get the error code and throw is at an IOException from Errno Exception + return new IpSecTransformResponse(IpSecManager.Status.RESOURCE_UNAVAILABLE); } // Both SAs were created successfully, time to construct a record and lock it away userRecord.mTransformRecords.put( resourceId, new RefcountedResource( - new TransformRecord(resourceId, c, spis, socketRecord), + new TransformRecord(resourceId, c, spiRecord, socketRecord), binder, dependencies.toArray(new RefcountedResource[dependencies.size()]))); return new IpSecTransformResponse(IpSecManager.Status.OK, resourceId); @@ -1217,9 +1197,9 @@ public class IpSecService extends IIpSecService.Stub { */ @Override public synchronized void applyTransportModeTransform( - ParcelFileDescriptor socket, int resourceId) throws RemoteException { + ParcelFileDescriptor socket, int direction, int resourceId) throws RemoteException { UserRecord userRecord = mUserResourceTracker.getUserRecord(Binder.getCallingUid()); - + checkDirection(direction); // Get transform record; if no transform is found, will throw IllegalArgumentException TransformRecord info = userRecord.mTransformRecords.getResourceOrThrow(resourceId); @@ -1230,17 +1210,15 @@ public class IpSecService extends IIpSecService.Stub { IpSecConfig c = info.getConfig(); try { - for (int direction : DIRECTIONS) { - mSrvConfig - .getNetdInstance() - .ipSecApplyTransportModeTransform( - socket.getFileDescriptor(), - resourceId, - direction, - c.getLocalAddress(), - c.getRemoteAddress(), - info.getSpiRecord(direction).getSpi()); - } + 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()); @@ -1251,14 +1229,14 @@ public class IpSecService extends IIpSecService.Stub { } /** - * Remove a transport mode transform from a socket, applying the default (empty) policy. This - * will ensure that NO IPsec policy is applied to the socket (would be the equivalent of - * applying a policy that performs no IPsec). Today the resourceId parameter is passed but not - * used: reserved for future improved input validation. + * Remove transport mode transforms from a socket, applying the default (empty) policy. This + * ensures that NO IPsec policy is applied to the socket (would be the equivalent of applying a + * policy that performs no IPsec). Today the resourceId parameter is passed but not used: + * reserved for future improved input validation. */ @Override - public synchronized void removeTransportModeTransform(ParcelFileDescriptor socket, int resourceId) - throws RemoteException { + public synchronized void removeTransportModeTransforms( + ParcelFileDescriptor socket, int resourceId) throws RemoteException { try { mSrvConfig .getNetdInstance() From 3167625a152141abece506815fcad91553e5a623 Mon Sep 17 00:00:00 2001 From: Nathan Harold Date: Tue, 16 Jan 2018 12:08:43 -0800 Subject: [PATCH 2/3] Prevent Closure of Underlying Socket FDs The version of applyTransportModeTransform() and removeTransportModeTransform() that accepted Socket and DatagramSocket were closing the underlying FDs upon return. It's unclear whether this is due to a behavior change elsewhere in ParcelFileDescriptor, but either way, converting to using getFileDescriptor$ and then calling dup() explicitly rather than relying on ParcelFileDescriptor seems like a better idea anyway. Bug: 72047396 Test: CTS - IpSecManagerTest.testCreateTransform() Change-Id: Ia2f02564e1289f25bf113dbb861fcfd2240537a7 --- core/java/android/net/IpSecManager.java | 35 ++++--------------------- 1 file changed, 5 insertions(+), 30 deletions(-) diff --git a/core/java/android/net/IpSecManager.java b/core/java/android/net/IpSecManager.java index 67d4fcac97..2202df3baf 100644 --- a/core/java/android/net/IpSecManager.java +++ b/core/java/android/net/IpSecManager.java @@ -313,9 +313,7 @@ public final class IpSecManager { public void applyTransportModeTransform( Socket socket, int direction, IpSecTransform transform) throws IOException { - try (ParcelFileDescriptor pfd = ParcelFileDescriptor.fromSocket(socket)) { - applyTransportModeTransform(pfd, direction, transform); - } + applyTransportModeTransform(socket.getFileDescriptor$(), direction, transform); } /** @@ -347,9 +345,7 @@ public final class IpSecManager { */ public void applyTransportModeTransform( DatagramSocket socket, int direction, IpSecTransform transform) throws IOException { - try (ParcelFileDescriptor pfd = ParcelFileDescriptor.fromDatagramSocket(socket)) { - applyTransportModeTransform(pfd, direction, transform); - } + applyTransportModeTransform(socket.getFileDescriptor$(), direction, transform); } /** @@ -383,18 +379,8 @@ public final class IpSecManager { FileDescriptor socket, int direction, IpSecTransform transform) throws IOException { // We dup() the FileDescriptor here because if we don't, then the ParcelFileDescriptor() - // constructor takes control and closes the user's FD when we exit the method - // This is behaviorally the same as the other versions, but the PFD constructor does not - // dup() automatically, whereas PFD.fromSocket() and PDF.fromDatagramSocket() do dup(). + // constructor takes control and closes the user's FD when we exit the method. try (ParcelFileDescriptor pfd = ParcelFileDescriptor.dup(socket)) { - applyTransportModeTransform(pfd, direction, transform); - } - } - - /* Call down to activate a transform */ - private void applyTransportModeTransform( - ParcelFileDescriptor pfd, int direction, IpSecTransform transform) throws IOException { - try { mService.applyTransportModeTransform(pfd, direction, transform.getResourceId()); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); @@ -433,9 +419,7 @@ public final class IpSecManager { */ public void removeTransportModeTransforms(Socket socket, IpSecTransform transform) throws IOException { - try (ParcelFileDescriptor pfd = ParcelFileDescriptor.fromSocket(socket)) { - removeTransportModeTransforms(pfd, transform); - } + removeTransportModeTransforms(socket.getFileDescriptor$(), transform); } /** @@ -455,9 +439,7 @@ public final class IpSecManager { */ public void removeTransportModeTransforms(DatagramSocket socket, IpSecTransform transform) throws IOException { - try (ParcelFileDescriptor pfd = ParcelFileDescriptor.fromDatagramSocket(socket)) { - removeTransportModeTransforms(pfd, transform); - } + removeTransportModeTransforms(socket.getFileDescriptor$(), transform); } /** @@ -478,13 +460,6 @@ public final class IpSecManager { public void removeTransportModeTransforms(FileDescriptor socket, IpSecTransform transform) throws IOException { try (ParcelFileDescriptor pfd = ParcelFileDescriptor.dup(socket)) { - removeTransportModeTransforms(pfd, transform); - } - } - - /* Call down to remove a transform */ - private void removeTransportModeTransforms(ParcelFileDescriptor pfd, IpSecTransform transform) { - try { mService.removeTransportModeTransforms(pfd, transform.getResourceId()); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); From 0d483b76f339e2089f5b659e140f951544a60143 Mon Sep 17 00:00:00 2001 From: Nathan Harold Date: Wed, 17 Jan 2018 01:00:20 -0800 Subject: [PATCH 3/3] IpSec - API Tweak for removeTransportModeTransform Because IpSecTransforms are now unidirectional, and because the only mechanism for removing Transforms removes it from both directions, the API can no longer use the Transform parameter to meaningfully validate that the caller had applied a transform. Since that functionality was as-yet unimplemented and is now infeasible, the transform parameter is removed. Bug: 72079356 Test: cts - IpSecManagerTest; runtest frameworks-net Change-Id: If19b0d34bdc6daf31a40d6d62bff326dcbca08c0 --- core/java/android/net/IIpSecService.aidl | 2 +- core/java/android/net/IpSecManager.java | 30 ++++++++----------- .../java/com/android/server/IpSecService.java | 4 +-- 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/core/java/android/net/IIpSecService.aidl b/core/java/android/net/IIpSecService.aidl index 3fe531fd79..790c80b1d9 100644 --- a/core/java/android/net/IIpSecService.aidl +++ b/core/java/android/net/IIpSecService.aidl @@ -45,5 +45,5 @@ interface IIpSecService void applyTransportModeTransform(in ParcelFileDescriptor socket, int direction, int transformId); - void removeTransportModeTransforms(in ParcelFileDescriptor socket, int transformId); + void removeTransportModeTransforms(in ParcelFileDescriptor socket); } diff --git a/core/java/android/net/IpSecManager.java b/core/java/android/net/IpSecManager.java index 2202df3baf..2cda58c99a 100644 --- a/core/java/android/net/IpSecManager.java +++ b/core/java/android/net/IpSecManager.java @@ -405,62 +405,56 @@ public final class IpSecManager { /** * Remove an IPsec transform from a stream socket. * - *

Once removed, traffic on the socket will not be encrypted. This operation will succeed - * regardless of the state of the transform. Removing a transform from a socket allows the - * socket to be reused for communication in the clear. + *

Once removed, traffic on the socket will not be encrypted. Removing transforms from a + * socket allows the socket to be reused for communication in the clear. * *

If an {@code IpSecTransform} object applied to this socket was deallocated by calling * {@link IpSecTransform#close()}, then communication on the socket will fail until this method * is called. * * @param socket a socket that previously had a transform applied to it - * @param transform the IPsec Transform that was previously applied to the given socket * @throws IOException indicating that the transform could not be removed from the socket */ - public void removeTransportModeTransforms(Socket socket, IpSecTransform transform) + public void removeTransportModeTransforms(Socket socket) throws IOException { - removeTransportModeTransforms(socket.getFileDescriptor$(), transform); + removeTransportModeTransforms(socket.getFileDescriptor$()); } /** * Remove an IPsec transform from a datagram socket. * - *

Once removed, traffic on the socket will not be encrypted. This operation will succeed - * regardless of the state of the transform. Removing a transform from a socket allows the - * socket to be reused for communication in the clear. + *

Once removed, traffic on the socket will not be encrypted. Removing transforms from a + * socket allows the socket to be reused for communication in the clear. * *

If an {@code IpSecTransform} object applied to this socket was deallocated by calling * {@link IpSecTransform#close()}, then communication on the socket will fail until this method * is called. * * @param socket a socket that previously had a transform applied to it - * @param transform the IPsec Transform that was previously applied to the given socket * @throws IOException indicating that the transform could not be removed from the socket */ - public void removeTransportModeTransforms(DatagramSocket socket, IpSecTransform transform) + public void removeTransportModeTransforms(DatagramSocket socket) throws IOException { - removeTransportModeTransforms(socket.getFileDescriptor$(), transform); + removeTransportModeTransforms(socket.getFileDescriptor$()); } /** * Remove an IPsec transform from a socket. * - *

Once removed, traffic on the socket will not be encrypted. This operation will succeed - * regardless of the state of the transform. Removing a transform from a socket allows the - * socket to be reused for communication in the clear. + *

Once removed, traffic on the socket will not be encrypted. Removing transforms from a + * socket allows the socket to be reused for communication in the clear. * *

If an {@code IpSecTransform} object applied to this socket was deallocated by calling * {@link IpSecTransform#close()}, then communication on the socket will fail until this method * is called. * * @param socket a socket that previously had a transform applied to it - * @param transform the IPsec Transform that was previously applied to the given socket * @throws IOException indicating that the transform could not be removed from the socket */ - public void removeTransportModeTransforms(FileDescriptor socket, IpSecTransform transform) + public void removeTransportModeTransforms(FileDescriptor socket) throws IOException { try (ParcelFileDescriptor pfd = ParcelFileDescriptor.dup(socket)) { - mService.removeTransportModeTransforms(pfd, transform.getResourceId()); + mService.removeTransportModeTransforms(pfd); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } diff --git a/services/core/java/com/android/server/IpSecService.java b/services/core/java/com/android/server/IpSecService.java index 9d228c3d0a..46a35ec800 100644 --- a/services/core/java/com/android/server/IpSecService.java +++ b/services/core/java/com/android/server/IpSecService.java @@ -1235,8 +1235,8 @@ public class IpSecService extends IIpSecService.Stub { * reserved for future improved input validation. */ @Override - public synchronized void removeTransportModeTransforms( - ParcelFileDescriptor socket, int resourceId) throws RemoteException { + public synchronized void removeTransportModeTransforms(ParcelFileDescriptor socket) + throws RemoteException { try { mSrvConfig .getNetdInstance()