From 778327e54816f742b2726bda2e6eff29bb0c92f9 Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Thu, 15 Mar 2018 19:41:41 -0700 Subject: [PATCH 1/4] Add support for auth-only transforms Kernel limitations prevent auth-only SAs from being created. Explicitly request a null encryption algorithm instead of omitting the algorithm to comply with the kernel requirement for ESP. Bug: 75049573 Test: CTS tests for auth-only, crypt-only transforms added for all combinations of (UDP, TCP, IPv4, IPv6, UDP-encap) Also added unit tests to ensure correct triggering of NULL_CRYPT usage. Merged-In: Ia9a5cfee9c7786412846bc039f326420f6211c08 Change-Id: Ia9a5cfee9c7786412846bc039f326420f6211c08 (cherry picked from commit bf013a3820c69348e034c6340b28e95f3441ebe8) --- core/java/android/net/IpSecAlgorithm.java | 7 +++++++ services/core/java/com/android/server/IpSecService.java | 9 ++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/core/java/android/net/IpSecAlgorithm.java b/core/java/android/net/IpSecAlgorithm.java index c69a4d4c0b..f4b328e412 100644 --- a/core/java/android/net/IpSecAlgorithm.java +++ b/core/java/android/net/IpSecAlgorithm.java @@ -37,6 +37,13 @@ import java.util.Arrays; public final class IpSecAlgorithm implements Parcelable { private static final String TAG = "IpSecAlgorithm"; + /** + * Null cipher. + * + * @hide + */ + public static final String CRYPT_NULL = "ecb(cipher_null)"; + /** * AES-CBC Encryption/Ciphering Algorithm. * diff --git a/services/core/java/com/android/server/IpSecService.java b/services/core/java/com/android/server/IpSecService.java index 45a4dfb91b..d3f1a7bbb5 100644 --- a/services/core/java/com/android/server/IpSecService.java +++ b/services/core/java/com/android/server/IpSecService.java @@ -1467,6 +1467,13 @@ public class IpSecService extends IIpSecService.Stub { IpSecAlgorithm crypt = c.getEncryption(); IpSecAlgorithm authCrypt = c.getAuthenticatedEncryption(); + String cryptName; + if (crypt == null) { + cryptName = (authCrypt == null) ? IpSecAlgorithm.CRYPT_NULL : ""; + } else { + cryptName = crypt.getName(); + } + mSrvConfig .getNetdInstance() .ipSecAddSecurityAssociation( @@ -1481,7 +1488,7 @@ public class IpSecService extends IIpSecService.Stub { (auth != null) ? auth.getName() : "", (auth != null) ? auth.getKey() : new byte[] {}, (auth != null) ? auth.getTruncationLengthBits() : 0, - (crypt != null) ? crypt.getName() : "", + cryptName, (crypt != null) ? crypt.getKey() : new byte[] {}, (crypt != null) ? crypt.getTruncationLengthBits() : 0, (authCrypt != null) ? authCrypt.getName() : "", From 97c3c945d714b89339be5f2dc325885ee2863ebb Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Thu, 1 Mar 2018 18:53:07 -0800 Subject: [PATCH 2/4] Added implementation for VTI add/remove address This change adds implementation details for add/remove addresses onto a VTI. Bug: 73675031 Test: New tests added, passing on Walleye Merged-In: Idde9d943a5285d2c13c5c6b0f7b8a9faf718e6a5 Change-Id: Idde9d943a5285d2c13c5c6b0f7b8a9faf718e6a5 (cherry picked from commit ecc9f7cc08804e3fa15fea04ae94ea1bc74edbfe) --- core/java/android/net/IIpSecService.aidl | 5 +- core/java/android/net/IpSecManager.java | 16 +++-- .../java/com/android/server/IpSecService.java | 60 +++++++++++++------ 3 files changed, 57 insertions(+), 24 deletions(-) diff --git a/core/java/android/net/IIpSecService.aidl b/core/java/android/net/IIpSecService.aidl index 3ce0283d7f..3a3ddcc483 100644 --- a/core/java/android/net/IIpSecService.aidl +++ b/core/java/android/net/IIpSecService.aidl @@ -16,6 +16,7 @@ package android.net; +import android.net.LinkAddress; import android.net.Network; import android.net.IpSecConfig; import android.net.IpSecUdpEncapResponse; @@ -48,11 +49,11 @@ interface IIpSecService void addAddressToTunnelInterface( int tunnelResourceId, - String localAddr); + in LinkAddress localAddr); void removeAddressFromTunnelInterface( int tunnelResourceId, - String localAddr); + in LinkAddress localAddr); void deleteTunnelInterface(int resourceId); diff --git a/core/java/android/net/IpSecManager.java b/core/java/android/net/IpSecManager.java index fbf305633a..4e1f83430a 100644 --- a/core/java/android/net/IpSecManager.java +++ b/core/java/android/net/IpSecManager.java @@ -656,10 +656,14 @@ public final class IpSecManager { * tunneled traffic. * * @param address the local address for traffic inside the tunnel - * @throws IOException if the address could not be added * @hide */ - public void addAddress(LinkAddress address) throws IOException { + public void addAddress(LinkAddress address) { + try { + mService.addAddressToTunnelInterface(mResourceId, address); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } } /** @@ -668,10 +672,14 @@ public final class IpSecManager { *

Remove an address which was previously added to the IpSecTunnelInterface * * @param address to be removed - * @throws IOException if the address could not be removed * @hide */ - public void removeAddress(LinkAddress address) throws IOException { + public void removeAddress(LinkAddress address) { + try { + mService.removeAddressFromTunnelInterface(mResourceId, address); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } } private IpSecTunnelInterface(@NonNull IIpSecService service, diff --git a/services/core/java/com/android/server/IpSecService.java b/services/core/java/com/android/server/IpSecService.java index 45a4dfb91b..45e9481c22 100644 --- a/services/core/java/com/android/server/IpSecService.java +++ b/services/core/java/com/android/server/IpSecService.java @@ -36,6 +36,7 @@ import android.net.IpSecTransform; import android.net.IpSecTransformResponse; import android.net.IpSecTunnelInterfaceResponse; import android.net.IpSecUdpEncapResponse; +import android.net.LinkAddress; import android.net.Network; import android.net.NetworkUtils; import android.net.TrafficStats; @@ -618,10 +619,8 @@ public class IpSecService extends IIpSecService.Stub { spi, mConfig.getMarkValue(), mConfig.getMarkMask()); - } 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); + } catch (RemoteException | ServiceSpecificException e) { + Log.e(TAG, "Failed to delete SA with ID: " + mResourceId, e); } getResourceTracker().give(); @@ -681,10 +680,8 @@ public class IpSecService extends IIpSecService.Stub { .getNetdInstance() .ipSecDeleteSecurityAssociation( mResourceId, mSourceAddress, mDestinationAddress, mSpi, 0, 0); - } 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 SPI reservation with ID: " + mResourceId); + } catch (ServiceSpecificException | RemoteException e) { + Log.e(TAG, "Failed to delete SPI reservation with ID: " + mResourceId, e); } mSpi = IpSecManager.INVALID_SECURITY_PARAMETER_INDEX; @@ -829,15 +826,13 @@ public class IpSecService extends IIpSecService.Stub { 0, direction, wildcardAddr, wildcardAddr, mark, 0xffffffff); } } - } catch (ServiceSpecificException e) { - // FIXME: get the error code and throw is at an IOException from Errno Exception - } catch (RemoteException e) { + } catch (ServiceSpecificException | RemoteException e) { Log.e( TAG, "Failed to delete VTI with interface name: " + mInterfaceName + " and id: " - + mResourceId); + + mResourceId, e); } getResourceTracker().give(); @@ -1319,7 +1314,9 @@ public class IpSecService extends IIpSecService.Stub { * from multiple local IP addresses over the same tunnel. */ @Override - public synchronized void addAddressToTunnelInterface(int tunnelResourceId, String localAddr) { + public synchronized void addAddressToTunnelInterface( + int tunnelResourceId, LinkAddress localAddr) { + enforceNetworkStackPermission(); UserRecord userRecord = mUserResourceTracker.getUserRecord(Binder.getCallingUid()); // Get tunnelInterface record; if no such interface is found, will throw @@ -1327,8 +1324,21 @@ public class IpSecService extends IIpSecService.Stub { TunnelInterfaceRecord tunnelInterfaceInfo = userRecord.mTunnelInterfaceRecords.getResourceOrThrow(tunnelResourceId); - // TODO: Add calls to netd: - // Add address to TunnelInterface + try { + // We can assume general validity of the IP address, since we get them as a + // LinkAddress, which does some validation. + mSrvConfig + .getNetdInstance() + .interfaceAddAddress( + tunnelInterfaceInfo.mInterfaceName, + localAddr.getAddress().getHostAddress(), + 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); + } } /** @@ -1337,7 +1347,8 @@ public class IpSecService extends IIpSecService.Stub { */ @Override public synchronized void removeAddressFromTunnelInterface( - int tunnelResourceId, String localAddr) { + int tunnelResourceId, LinkAddress localAddr) { + enforceNetworkStackPermission(); UserRecord userRecord = mUserResourceTracker.getUserRecord(Binder.getCallingUid()); // Get tunnelInterface record; if no such interface is found, will throw @@ -1345,8 +1356,21 @@ public class IpSecService extends IIpSecService.Stub { TunnelInterfaceRecord tunnelInterfaceInfo = userRecord.mTunnelInterfaceRecords.getResourceOrThrow(tunnelResourceId); - // TODO: Add calls to netd: - // Remove address from TunnelInterface + try { + // We can assume general validity of the IP address, since we get them as a + // LinkAddress, which does some validation. + mSrvConfig + .getNetdInstance() + .interfaceDelAddress( + tunnelInterfaceInfo.mInterfaceName, + localAddr.getAddress().getHostAddress(), + 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); + } } /** From 49cd8d72686cc007ee0a0c5088100e169c9aa310 Mon Sep 17 00:00:00 2001 From: Nathan Harold Date: Tue, 20 Mar 2018 12:26:10 -0700 Subject: [PATCH 3/4] Expose add/removeAddress for IpSecInterfaces When exposing the APIs, these were missed. The outer structure is exposed, so this exposes the addAddress and removeAddress methods. Bug: 75234273 Test: compilation Merged-In: I79911434f9baa660e4d8564cc59d80da4a710c42 Change-Id: I79911434f9baa660e4d8564cc59d80da4a710c42 (cherry picked from commit a83601a511c3f11470109d78d1a736acdb9c6bd8) --- core/java/android/net/IpSecManager.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/java/android/net/IpSecManager.java b/core/java/android/net/IpSecManager.java index 4e1f83430a..cb4299ef69 100644 --- a/core/java/android/net/IpSecManager.java +++ b/core/java/android/net/IpSecManager.java @@ -658,7 +658,8 @@ public final class IpSecManager { * @param address the local address for traffic inside the tunnel * @hide */ - public void addAddress(LinkAddress address) { + @SystemApi + public void addAddress(LinkAddress address) throws IOException { try { mService.addAddressToTunnelInterface(mResourceId, address); } catch (RemoteException e) { @@ -674,7 +675,8 @@ public final class IpSecManager { * @param address to be removed * @hide */ - public void removeAddress(LinkAddress address) { + @SystemApi + public void removeAddress(LinkAddress address) throws IOException { try { mService.removeAddressFromTunnelInterface(mResourceId, address); } catch (RemoteException e) { From fdde4d633f837c44b127f0c73d3954f4a7d8a226 Mon Sep 17 00:00:00 2001 From: Nathan Harold Date: Tue, 27 Feb 2018 19:19:40 -0800 Subject: [PATCH 4/4] Check mOwnedByTransform to avoid DELSA on SPI The owned by transform flag prevents the removal of an SPI from accidentally deleting an associated SA in the kernel. That flag wasn't actually being checked, so deleting an SPI would result in the transform being removed. The existing code already guarantees that the SA is deleted when the transform is deleted Bug: 73258845 Test: runtest frameworks-net Merged-In: I4c26aea7af817a5d9e54da5db1cdf4f943bcae06 Change-Id: I4c26aea7af817a5d9e54da5db1cdf4f943bcae06 (cherry picked from commit 22795302be4ec35449908cf566aa7c16945df836) --- .../core/java/com/android/server/IpSecService.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/IpSecService.java b/services/core/java/com/android/server/IpSecService.java index 45e9481c22..89f599b17f 100644 --- a/services/core/java/com/android/server/IpSecService.java +++ b/services/core/java/com/android/server/IpSecService.java @@ -676,10 +676,12 @@ public class IpSecService extends IIpSecService.Stub { @Override public void freeUnderlyingResources() { try { - mSrvConfig - .getNetdInstance() - .ipSecDeleteSecurityAssociation( - mResourceId, mSourceAddress, mDestinationAddress, mSpi, 0, 0); + if (!mOwnedByTransform) { + mSrvConfig + .getNetdInstance() + .ipSecDeleteSecurityAssociation( + mResourceId, mSourceAddress, mDestinationAddress, mSpi, 0, 0); + } } catch (ServiceSpecificException | RemoteException e) { Log.e(TAG, "Failed to delete SPI reservation with ID: " + mResourceId, e); }