From 2cde5cd10333f8d8fb6510825475ef4fc0c7131e Mon Sep 17 00:00:00 2001 From: evitayan Date: Sun, 26 Apr 2020 22:14:08 -0700 Subject: Use #hasTransport to check if test network is used Fix the bug that using hasCapability to check if the network is a test network Bug: 148689509 Test: atest FrameworksIkeTests Test: manually tested Change-Id: Iadde414a0cc4575666d65a1002c6ab8b58ac9d86 --- .../com/android/internal/net/ipsec/ike/utils/RandomnessFactory.java | 5 ++--- .../android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java | 4 ++-- .../java/com/android/internal/net/ipsec/ike/IkeSessionTestBase.java | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/java/com/android/internal/net/ipsec/ike/utils/RandomnessFactory.java b/src/java/com/android/internal/net/ipsec/ike/utils/RandomnessFactory.java index 9ebb057a..2e5d1703 100644 --- a/src/java/com/android/internal/net/ipsec/ike/utils/RandomnessFactory.java +++ b/src/java/com/android/internal/net/ipsec/ike/utils/RandomnessFactory.java @@ -31,7 +31,7 @@ import java.security.SecureRandom; public class RandomnessFactory implements EapRandomFactory { // This constant is mirrored of android.net.NetworkCapabilities.TRANSPORT_TEST due to lack of // @TestApi guarantees in mainline modules - public static final int NETWORK_CAPABILITY_TRANSPORT_TEST = 7; + public static final int TRANSPORT_TEST = 7; private final boolean mIsTestModeEnabled; @@ -41,8 +41,7 @@ public class RandomnessFactory implements EapRandomFactory { NetworkCapabilities networkCapabilities = connectManager.getNetworkCapabilities(network); mIsTestModeEnabled = - networkCapabilities != null - && networkCapabilities.hasCapability(NETWORK_CAPABILITY_TRANSPORT_TEST); + networkCapabilities != null && networkCapabilities.hasTransport(TRANSPORT_TEST); } /** diff --git a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java index 8fb4ebba..a41ceca7 100644 --- a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java +++ b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java @@ -1268,7 +1268,7 @@ public final class IkeSessionStateMachineTest extends IkeSessionTestBase { public void testEnableTestMode() throws Exception { doReturn(true) .when(mMockNetworkCapabilities) - .hasCapability(RandomnessFactory.NETWORK_CAPABILITY_TRANSPORT_TEST); + .hasTransport(RandomnessFactory.TRANSPORT_TEST); IkeSessionStateMachine ikeSession = makeAndStartIkeSession(buildIkeSessionParams()); @@ -1281,7 +1281,7 @@ public final class IkeSessionStateMachineTest extends IkeSessionTestBase { public void testDisableTestMode() throws Exception { doReturn(false) .when(mMockNetworkCapabilities) - .hasCapability(RandomnessFactory.NETWORK_CAPABILITY_TRANSPORT_TEST); + .hasTransport(RandomnessFactory.TRANSPORT_TEST); IkeSessionStateMachine ikeSession = makeAndStartIkeSession(buildIkeSessionParams()); diff --git a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionTestBase.java b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionTestBase.java index 3728f368..174cd7cb 100644 --- a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionTestBase.java +++ b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionTestBase.java @@ -102,6 +102,6 @@ public abstract class IkeSessionTestBase { .getNetworkCapabilities(any(Network.class)); doReturn(false) .when(mMockNetworkCapabilities) - .hasCapability(RandomnessFactory.NETWORK_CAPABILITY_TRANSPORT_TEST); + .hasTransport(RandomnessFactory.TRANSPORT_TEST); } } -- cgit v1.2.3 From 8cf110e3a1d3291b5aff9fc72ad8f42afefd9e6a Mon Sep 17 00:00:00 2001 From: Dheeraj Shetty Date: Tue, 28 Apr 2020 17:20:50 -0700 Subject: Do not throw exception for wrong protocol ID As per RFC 7296, protocol ID field must be ignored when when SPI field is empty. Test: atest FrameworksIkeTest Test: On device test, to check if IKE INIT fails on wrong protocol ID. Bug: 154554731 Change-Id: I65d72ee4194823615d27d85f56c732dcd98047e9 Change-Id: I28c930a66d56c1153d911cceed105ce0052764e4 --- .../internal/net/ipsec/ike/message/IkeNotifyPayload.java | 6 ++++-- .../net/ipsec/ike/message/IkeNotifyPayloadTest.java | 13 ------------- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/src/java/com/android/internal/net/ipsec/ike/message/IkeNotifyPayload.java b/src/java/com/android/internal/net/ipsec/ike/message/IkeNotifyPayload.java index b9087777..30145338 100644 --- a/src/java/com/android/internal/net/ipsec/ike/message/IkeNotifyPayload.java +++ b/src/java/com/android/internal/net/ipsec/ike/message/IkeNotifyPayload.java @@ -16,6 +16,7 @@ package com.android.internal.net.ipsec.ike.message; +import static android.net.ipsec.ike.IkeManager.getIkeLog; import static android.net.ipsec.ike.exceptions.IkeProtocolException.ERROR_TYPE_AUTHENTICATION_FAILED; import static android.net.ipsec.ike.exceptions.IkeProtocolException.ERROR_TYPE_CHILD_SA_NOT_FOUND; import static android.net.ipsec.ike.exceptions.IkeProtocolException.ERROR_TYPE_FAILED_CP_REQUIRED; @@ -74,6 +75,8 @@ import java.util.Set; * Version 2 (IKEv2) */ public final class IkeNotifyPayload extends IkeInformationalPayload { + private static final String TAG = IkeNotifyPayload.class.getSimpleName(); + @Retention(RetentionPolicy.SOURCE) @IntDef({ NOTIFY_TYPE_ADDITIONAL_TS_POSSIBLE, @@ -267,8 +270,7 @@ public final class IkeNotifyPayload extends IkeInformationalPayload { private void validateNotifyPayloadForIkeAndNewChild() throws InvalidSyntaxException { if (protocolId != PROTOCOL_ID_UNSET) { - throw new InvalidSyntaxException( - "Expected Procotol ID unset: Protocol ID is " + protocolId); + getIkeLog().w(TAG, "Expected Procotol ID unset: Protocol ID is " + protocolId); } if (notifyType == ERROR_TYPE_INVALID_SELECTORS diff --git a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/message/IkeNotifyPayloadTest.java b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/message/IkeNotifyPayloadTest.java index 884c76eb..0b3c3583 100644 --- a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/message/IkeNotifyPayloadTest.java +++ b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/message/IkeNotifyPayloadTest.java @@ -84,19 +84,6 @@ public final class IkeNotifyPayloadTest { assertArrayEquals(new byte[0], payload.notifyData); } - @Test - public void testDecodeNotifyPayloadThrowException() throws Exception { - byte[] inputPacket = - TestUtils.hexStringToByteArray(NOTIFY_NAT_DETECTION_PAYLOAD_BODY_HEX_STRING); - // Change Protocol ID to ESP - inputPacket[PROTOCOL_ID_OFFSET] = (byte) (IkePayload.PROTOCOL_ID_ESP & 0xFF); - try { - IkeNotifyPayload payload = new IkeNotifyPayload(false, inputPacket); - fail("Expected InvalidSyntaxException: Protocol ID should not be ESP"); - } catch (InvalidSyntaxException expected) { - } - } - @Test public void testGenerateNatDetectionData() throws Exception { long initiatorIkeSpi = Long.parseLong(IKE_INITIATOR_SPI_HEX_STRING, 16); -- cgit v1.2.3 From 99f67e8fb6f5198e400c02793d0d6014d1fbfee7 Mon Sep 17 00:00:00 2001 From: Anton Hansson Date: Thu, 30 Apr 2020 17:43:44 +0100 Subject: Rename module dist files This makes the filenames of the disted artifacts (api txts and stubs) match the module name of the modules they're from. This matches the naming scheme used by java_sdk_library, which should make the future transition to this build rule easier. Bug: 149293194 Test: lunch sdk_phone_armv7 && m sdk dist && find out/dist/apistubs Change-Id: If0e181349b8b263e22e424640ea3e956b944f460 Merged-In: If0e181349b8b263e22e424640ea3e956b944f460 (cherry picked from commit 2619608a0695d130d844965e7f09885dc3484fbf) --- Android.bp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Android.bp b/Android.bp index 8e7717aa..f954c1cb 100644 --- a/Android.bp +++ b/Android.bp @@ -98,6 +98,7 @@ droidstubs { "framework-module-api-defaults-module_libs_api", "ike-stubs-defaults", ], + dist: { dest: "android.net.ipsec.ike.txt" }, } droidstubs { @@ -112,4 +113,5 @@ java_library { name: "android.net.ipsec.ike.stubs.module_libs_api", srcs: [":android.net.ipsec.ike.stubs.sources.module_libs_api"], defaults: ["framework-module-stubs-lib-defaults-module_libs_api"], + dist: { dest: "android.net.ipsec.ike.jar" }, } -- cgit v1.2.3 From 16de8ea2c6af9fadbaa59b267af8c09e5109bed4 Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Thu, 30 Apr 2020 10:53:40 -0700 Subject: Switch IKE Sockets to use a poll loop This commit switches IKE sockets from the packet reader to using a poll loop for the following reasons: 1. PacketReader FD_EVENT notifications were sometimes not fired 2. This results in flaky behavior of IKE socket reads 2. a. Even in real life, it appears as network packet loss, and may end up missing all retransmitted responses. Once packet reader is fixed, this patch should be reverted. Bug: 148689509 Test: CTS tests passing Change-Id: Iaa999cf2094768d9fd61b90ceb11fd7a23d501fc --- .../net/ipsec/ike/IkeSessionStateMachine.java | 3 +- .../android/internal/net/ipsec/ike/IkeSocket.java | 68 +++++++++++++++++++++- .../internal/net/ipsec/ike/IkeUdpEncapSocket.java | 23 +++----- .../internal/net/ipsec/ike/IkeUdpSocket.java | 13 +---- .../net/ipsec/ike/IkeSessionStateMachineTest.java | 16 ++--- .../net/ipsec/ike/IkeUdpEncapSocketTest.java | 22 +++---- 6 files changed, 93 insertions(+), 52 deletions(-) diff --git a/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java b/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java index 87eab421..5fbcda89 100644 --- a/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java +++ b/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java @@ -2952,7 +2952,8 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { IkeUdpEncapSocket.getIkeUdpEncapSocket( mIkeSessionParams.getNetwork(), mIpSecManager, - IkeSessionStateMachine.this); + IkeSessionStateMachine.this, + getHandler().getLooper()); switchToIkeSocket(initIkeSpi, newSocket); } catch (ErrnoException | IOException | ResourceUnavailableException e) { handleIkeFatalError(e); diff --git a/src/java/com/android/internal/net/ipsec/ike/IkeSocket.java b/src/java/com/android/internal/net/ipsec/ike/IkeSocket.java index 8553f30d..1dbc9c1f 100644 --- a/src/java/com/android/internal/net/ipsec/ike/IkeSocket.java +++ b/src/java/com/android/internal/net/ipsec/ike/IkeSocket.java @@ -27,11 +27,13 @@ import android.util.LongSparseArray; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.net.ipsec.ike.message.IkeHeader; -import com.android.internal.net.ipsec.ike.utils.PacketReader; import java.io.FileDescriptor; +import java.io.FileInputStream; +import java.io.IOException; import java.net.InetAddress; import java.net.InetSocketAddress; +import java.util.Arrays; import java.util.HashSet; import java.util.Set; @@ -53,7 +55,7 @@ import java.util.Set; * IkeSessionStateMachine}. Since all {@link IkeSessionStateMachine}s run on the same working * thread, there will not be concurrent modification problems. */ -public abstract class IkeSocket extends PacketReader implements AutoCloseable { +public abstract class IkeSocket implements AutoCloseable { private static final String TAG = "IkeSocket"; /** Non-udp-encapsulated IKE packets MUST be sent to 500. */ @@ -63,6 +65,7 @@ public abstract class IkeSocket extends PacketReader implements AutoCloseable { // Network this socket bound to. private final Network mNetwork; + private final Handler mHandler; // Map from locally generated IKE SPI to IkeSessionStateMachine instances. @VisibleForTesting @@ -74,7 +77,7 @@ public abstract class IkeSocket extends PacketReader implements AutoCloseable { protected final Set mAliveIkeSessions = new HashSet<>(); protected IkeSocket(Network network, Handler handler) { - super(handler); + mHandler = handler; mNetwork = network; } @@ -107,6 +110,53 @@ public abstract class IkeSocket extends PacketReader implements AutoCloseable { } } + /** Starts the packet reading poll-loop. */ + public void start() { + // Start background reader thread + new Thread( + () -> { + try { + // Loop will exit and thread will quit when the retrieved fd is closed. + // Receiving either EOF or an exception will exit this reader loop. + // FileInputStream in uninterruptable, so there's no good way to ensure that + // that this thread shuts down except upon FD closure. + while (true) { + byte[] intercepted = receiveFromFd(); + if (intercepted == null) { + // Exit once we've hit EOF + return; + } else if (intercepted.length > 0) { + // Only save packet if we've received any bytes. + getIkeLog() + .d( + this.getClass().getSimpleName(), + "Received packet"); + mHandler.post( + () -> { + handlePacket(intercepted, intercepted.length); + }); + } + } + } catch (IOException ignored) { + // Simply exit this reader thread + return; + } + }).start(); + } + + private byte[] receiveFromFd() throws IOException { + FileInputStream in = new FileInputStream(getFd()); + byte[] inBytes = new byte[4096]; + int bytesRead = in.read(inBytes); + + if (bytesRead < 0) { + return null; // return null for EOF + } else if (bytesRead >= 4096) { + throw new IllegalStateException("Too big packet. Fragmentation unsupported"); + } + return Arrays.copyOf(inBytes, bytesRead); + } + /** * Returns the port that this IKE socket is listening on (bound to). */ @@ -117,6 +167,12 @@ public abstract class IkeSocket extends PacketReader implements AutoCloseable { protected abstract FileDescriptor getFd(); + protected FileDescriptor createFd() { + return getFd(); + } + + protected abstract void handlePacket(byte[] recvbuf, int length); + /** * Return Network this socket bound to * @@ -176,6 +232,12 @@ public abstract class IkeSocket extends PacketReader implements AutoCloseable { stop(); } + /** Stops the packet reading loop */ + public void stop() { + // No additional cleanup at this time. Subclasses are in charge of closing their sockets, + // which will result in the packet reading poll loop exiting. + } + /** * IPacketReceiver provides a package private interface for handling received packet. * diff --git a/src/java/com/android/internal/net/ipsec/ike/IkeUdpEncapSocket.java b/src/java/com/android/internal/net/ipsec/ike/IkeUdpEncapSocket.java index 2eb10104..11bb9340 100644 --- a/src/java/com/android/internal/net/ipsec/ike/IkeUdpEncapSocket.java +++ b/src/java/com/android/internal/net/ipsec/ike/IkeUdpEncapSocket.java @@ -26,6 +26,7 @@ import android.net.IpSecManager.ResourceUnavailableException; import android.net.IpSecManager.UdpEncapsulationSocket; import android.net.Network; import android.os.Handler; +import android.os.Looper; import android.system.ErrnoException; import android.system.Os; import android.util.LongSparseArray; @@ -81,7 +82,10 @@ public final class IkeUdpEncapSocket extends IkeSocket { * @return an IkeUdpEncapSocket instance */ public static IkeUdpEncapSocket getIkeUdpEncapSocket( - Network network, IpSecManager ipsecManager, IkeSessionStateMachine ikeSession) + Network network, + IpSecManager ipsecManager, + IkeSessionStateMachine ikeSession, + Looper looper) throws ErrnoException, IOException, ResourceUnavailableException { IkeUdpEncapSocket ikeSocket = sNetworkToIkeSocketMap.get(network); if (ikeSocket == null) { @@ -92,7 +96,7 @@ public final class IkeUdpEncapSocket extends IkeSocket { Os.fcntlInt(fd, F_SETFL, SOCK_DGRAM | SOCK_NONBLOCK); network.bindSocket(fd); - ikeSocket = new IkeUdpEncapSocket(udpEncapSocket, network, new Handler()); + ikeSocket = new IkeUdpEncapSocket(udpEncapSocket, network, new Handler(looper)); // Create and register FileDescriptor for receiving IKE packet on current thread. ikeSocket.start(); @@ -112,21 +116,10 @@ public final class IkeUdpEncapSocket extends IkeSocket { return mUdpEncapSocket; } + /** Get FileDescriptor for use with the packet reader polling loop */ @Override protected FileDescriptor getFd() { - return createFd(); // Returns cached FD - } - - /** - * Get FileDescriptor of mUdpEncapSocket. - * - *

PacketReader registers a listener for this file descriptor on the thread where - * IkeUdpEncapSocket is constructed. When there is a read event, this listener is invoked and - * then calls {@link handlePacket} to handle the received packet. - */ - @Override - protected FileDescriptor createFd() { - return mUdpEncapSocket.getFileDescriptor(); + return mUdpEncapSocket.getFileDescriptor(); // Returns cached FD } /** Package private */ diff --git a/src/java/com/android/internal/net/ipsec/ike/IkeUdpSocket.java b/src/java/com/android/internal/net/ipsec/ike/IkeUdpSocket.java index a0bb29ad..b6130ad5 100644 --- a/src/java/com/android/internal/net/ipsec/ike/IkeUdpSocket.java +++ b/src/java/com/android/internal/net/ipsec/ike/IkeUdpSocket.java @@ -46,20 +46,9 @@ public abstract class IkeUdpSocket extends IkeSocket { mSocket = socket; } + /** Get FileDescriptor for use with the packet reader polling loop */ @Override protected FileDescriptor getFd() { - return createFd(); // Returns cached FD - } - - /** - * Get FileDescriptor for use with the Packet Receiver. - * - *

PacketReader registers a listener for this file descriptor on the thread where - * IkeUdpSocket is constructed. When there is a read event, this listener is invoked and then - * calls {@link handlePacket} to handle the received packet. - */ - @Override - protected FileDescriptor createFd() { return mSocket; } diff --git a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java index 8fb4ebba..f5f6765d 100644 --- a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java +++ b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java @@ -755,13 +755,11 @@ public final class IkeSessionStateMachineTest extends IkeSessionTestBase { ikeSession.mLocalAddress = LOCAL_ADDRESS; assertEquals(REMOTE_ADDRESS, ikeSession.mRemoteAddress); - // Get socket instances used by the IkeSessionStateMachine by virtue of the caching. - mSpyIkeUdp4Socket = spy(IkeUdp4Socket.getInstance(mMockDefaultNetwork, ikeSession)); - mSpyIkeUdp6Socket = spy(IkeUdp6Socket.getInstance(mMockDefaultNetwork, ikeSession)); - mSpyIkeUdpEncapSocket = - spy( - IkeUdpEncapSocket.getIkeUdpEncapSocket( - mMockDefaultNetwork, mIpSecManager, ikeSession)); + // Setup socket instances used by the IkeSessionStateMachine + // TODO: rename these from spy to mock. + mSpyIkeUdp4Socket = mock(IkeUdp4Socket.class); + mSpyIkeUdp6Socket = mock(IkeUdp6Socket.class); + mSpyIkeUdpEncapSocket = mock(IkeUdpEncapSocket.class); doNothing().when(mSpyIkeUdp4Socket).sendIkePacket(any(), any()); doNothing().when(mSpyIkeUdp6Socket).sendIkePacket(any(), any()); @@ -1182,7 +1180,6 @@ public final class IkeSessionStateMachineTest extends IkeSessionTestBase { mLooper.dispatchAll(); verify(mSpyCurrentIkeSocket).releaseReference(eq(mIkeSessionStateMachine)); - verify(mSpyCurrentIkeSocket).close(); } @Test @@ -1331,9 +1328,6 @@ public final class IkeSessionStateMachineTest extends IkeSessionTestBase { // Validate socket switched assertTrue(mIkeSessionStateMachine.mIkeSocket instanceof IkeUdpEncapSocket); verify(mSpyIkeUdp4Socket).unregisterIke(anyLong()); - - // Validate keepalive has started - verify(mMockSocketKeepalive).start(anyInt()); } @Ignore diff --git a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeUdpEncapSocketTest.java b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeUdpEncapSocketTest.java index 8c0bf4d5..439fa27e 100644 --- a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeUdpEncapSocketTest.java +++ b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeUdpEncapSocketTest.java @@ -118,12 +118,12 @@ public final class IkeUdpEncapSocketTest extends IkeSocketTestBase { IkeUdpEncapSocket ikeSocketOne = IkeUdpEncapSocket.getIkeUdpEncapSocket( - mMockNetwork, mSpyIpSecManager, mockIkeSessionOne); + mMockNetwork, mSpyIpSecManager, mockIkeSessionOne, Looper.myLooper()); assertEquals(1, ikeSocketOne.mAliveIkeSessions.size()); IkeUdpEncapSocket ikeSocketTwo = IkeUdpEncapSocket.getIkeUdpEncapSocket( - mMockNetwork, mSpyIpSecManager, mockIkeSessionTwo); + mMockNetwork, mSpyIpSecManager, mockIkeSessionTwo, Looper.myLooper()); assertEquals(2, ikeSocketTwo.mAliveIkeSessions.size()); assertEquals(ikeSocketOne, ikeSocketTwo); @@ -154,12 +154,12 @@ public final class IkeUdpEncapSocketTest extends IkeSocketTestBase { IkeUdpEncapSocket ikeSocketOne = IkeUdpEncapSocket.getIkeUdpEncapSocket( - mockNetworkOne, mSpyIpSecManager, mockIkeSessionOne); + mockNetworkOne, mSpyIpSecManager, mockIkeSessionOne, Looper.myLooper()); assertEquals(1, ikeSocketOne.mAliveIkeSessions.size()); IkeUdpEncapSocket ikeSocketTwo = IkeUdpEncapSocket.getIkeUdpEncapSocket( - mockNetworkTwo, mSpyIpSecManager, mockIkeSessionTwo); + mockNetworkTwo, mSpyIpSecManager, mockIkeSessionTwo, Looper.myLooper()); assertEquals(1, ikeSocketTwo.mAliveIkeSessions.size()); assertNotEquals(ikeSocketOne, ikeSocketTwo); @@ -192,7 +192,10 @@ public final class IkeUdpEncapSocketTest extends IkeSocketTestBase { // Send IKE packet IkeUdpEncapSocket ikeSocket = IkeUdpEncapSocket.getIkeUdpEncapSocket( - mMockNetwork, mSpyIpSecManager, mMockIkeSessionStateMachine); + mMockNetwork, + mSpyIpSecManager, + mMockIkeSessionStateMachine, + Looper.myLooper()); ikeSocket.sendIkePacket(mDataOne, IPV4_LOOPBACK); byte[] receivedData = receive(mDummyRemoteServerFd); @@ -227,7 +230,8 @@ public final class IkeUdpEncapSocketTest extends IkeSocketTestBase { IkeUdpEncapSocket.getIkeUdpEncapSocket( mMockNetwork, mSpyIpSecManager, - mMockIkeSessionStateMachine)); + mMockIkeSessionStateMachine, + mIkeThread.getLooper())); createLatch.countDown(); Log.d("IkeUdpEncapSocketTest", "IkeUdpEncapSocket created."); } catch (ErrnoException @@ -253,20 +257,18 @@ public final class IkeUdpEncapSocketTest extends IkeSocketTestBase { sendToIkeUdpEncapSocket(mDummyRemoteServerFd, mDataOne, IPV4_LOOPBACK); receiveLatch.await(); - assertEquals(1, ikeSocket.numPacketsReceived()); assertArrayEquals(mDataOne, packetReceiver.mReceivedData); // Send second packet. sendToIkeUdpEncapSocket(mDummyRemoteServerFd, mDataTwo, IPV4_LOOPBACK); receiveLatch.await(); - assertEquals(2, ikeSocket.numPacketsReceived()); assertArrayEquals(mDataTwo, packetReceiver.mReceivedData); // Close IkeUdpEncapSocket. TestCountDownLatch closeLatch = new TestCountDownLatch(); - ikeSocket - .getHandler() + mIkeThread + .getThreadHandler() .post( () -> { ikeSocket.releaseReference(mMockIkeSessionStateMachine); -- cgit v1.2.3 From 1192340fda45d31cacebfc49523cb3b7c0706a6e Mon Sep 17 00:00:00 2001 From: Dheeraj Shetty Date: Tue, 28 Apr 2020 11:30:25 -0700 Subject: Add DH groups from child session params DH group is absent from SA proposal during rekey of first child. In this case add it from the child session params. Test: atest FrameworksIkeTest Bug: 138965374 Change-Id: I0284516b555917f723d8bbed50beb9f23869ed1b Merged-In: I0284516b555917f723d8bbed50beb9f23869ed1b --- .../android/net/ipsec/ike/ChildSaProposal.java | 10 +++ .../net/ipsec/ike/ChildSessionStateMachine.java | 42 +++++++++- .../ipsec/ike/ChildSessionStateMachineTest.java | 91 ++++++++++++++++++---- 3 files changed, 127 insertions(+), 16 deletions(-) diff --git a/src/java/android/net/ipsec/ike/ChildSaProposal.java b/src/java/android/net/ipsec/ike/ChildSaProposal.java index 4d3ea284..bf55e8fa 100644 --- a/src/java/android/net/ipsec/ike/ChildSaProposal.java +++ b/src/java/android/net/ipsec/ike/ChildSaProposal.java @@ -107,6 +107,16 @@ public final class ChildSaProposal extends SaProposal { && isTransformSelectedFrom(mEsns, ((ChildSaProposal) reqProposal).mEsns); } + /** @hide */ + public boolean isNegotiatedFromExceptDhGroup(SaProposal saProposal) { + return getProtocolId() == saProposal.getProtocolId() + && isTransformSelectedFrom( + getEncryptionTransforms(), saProposal.getEncryptionTransforms()) + && isTransformSelectedFrom( + getIntegrityTransforms(), saProposal.getIntegrityTransforms()) + && isTransformSelectedFrom(mEsns, ((ChildSaProposal) saProposal).mEsns); + } + /** * This class is used to incrementally construct a ChildSaProposal. ChildSaProposal instances * are immutable once built. diff --git a/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachine.java b/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachine.java index 8697ab50..72663a85 100644 --- a/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachine.java +++ b/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachine.java @@ -104,8 +104,10 @@ import java.net.InetAddress; import java.security.GeneralSecurityException; import java.util.ArrayList; import java.util.Arrays; +import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; +import java.util.Set; import java.util.concurrent.Executor; /** @@ -221,6 +223,7 @@ public class ChildSessionStateMachine extends AbstractSessionStateMachine { @VisibleForTesting final State mRekeyChildRemoteCreate = new RekeyChildRemoteCreate(); @VisibleForTesting final State mRekeyChildLocalDelete = new RekeyChildLocalDelete(); @VisibleForTesting final State mRekeyChildRemoteDelete = new RekeyChildRemoteDelete(); + @VisibleForTesting boolean mIsFirstChild; /** * Builds a new uninitialized ChildSessionStateMachine @@ -361,6 +364,7 @@ public class ChildSessionStateMachine extends AbstractSessionStateMachine { this.mUdpEncapSocket = udpEncapSocket; this.mIkePrf = ikePrf; this.mSkD = skD; + mIsFirstChild = true; int spi = registerProvisionalChildAndGetSpi(respPayloads); sendMessage( @@ -391,6 +395,7 @@ public class ChildSessionStateMachine extends AbstractSessionStateMachine { this.mUdpEncapSocket = udpEncapSocket; this.mIkePrf = ikePrf; this.mSkD = skD; + mIsFirstChild = false; sendMessage(CMD_LOCAL_REQUEST_CREATE_CHILD); } @@ -1264,13 +1269,18 @@ public class ChildSessionStateMachine extends AbstractSessionStateMachine { @Override public void enterState() { try { + ChildSaProposal saProposal = mSaProposal; + if (mIsFirstChild) { + saProposal = addDhGroupsFromChildSessionParamsIfAbsent(); + } + // Build request with negotiated proposal and TS. mRequestPayloads = CreateChildSaHelper.getRekeyChildCreateReqPayloads( mRandomFactory, mIpSecSpiGenerator, mLocalAddress, - mSaProposal, + saProposal, mLocalTs, mRemoteTs, mCurrentChildSaRecord.getLocalSpi(), @@ -1307,8 +1317,8 @@ public class ChildSessionStateMachine extends AbstractSessionStateMachine { switch (createChildResult.status) { case CREATE_STATUS_OK: try { - // Do not need to update the negotiated proposal and TS because they - // are not changed. + // Do not need to update TS because they are not changed. + mSaProposal = createChildResult.negotiatedProposal; ChildLocalRequest rekeyLocalRequest = makeRekeyLocalRequest(); @@ -1405,6 +1415,29 @@ public class ChildSessionStateMachine extends AbstractSessionStateMachine { } } + private ChildSaProposal addDhGroupsFromChildSessionParamsIfAbsent() { + // DH groups are excluded for the first child. Add dh groups from child session params in + // this case. + if (mSaProposal.getDhGroups().size() != 0) { + return mSaProposal; + } + + Set dhGroupSet = new LinkedHashSet<>(); + for (SaProposal saProposal : mChildSessionParams.getSaProposals()) { + if (!mSaProposal.isNegotiatedFromExceptDhGroup(saProposal)) continue; + dhGroupSet.addAll(Arrays.asList(saProposal.getDhGroupTransforms())); + } + + DhGroupTransform[] dhGroups = new DhGroupTransform[dhGroupSet.size()]; + dhGroupSet.toArray(dhGroups); + + return new ChildSaProposal( + mSaProposal.getEncryptionTransforms(), + mSaProposal.getIntegrityTransforms(), + dhGroups, + mSaProposal.getEsnTransforms()); + } + /** * RekeyChildRemoteCreate represents the state where Child Session receives a Rekey Child * request. @@ -1448,6 +1481,9 @@ public class ChildSessionStateMachine extends AbstractSessionStateMachine { IkeSaPayload reqSaPayload = IkePayload.getPayloadForTypeInProvidedList( PAYLOAD_TYPE_SA, IkeSaPayload.class, reqPayloads); + + // TODO: Handle first child remote rekey request with KE payload. + byte respProposalNumber = reqSaPayload.getNegotiatedProposalNumber(mSaProposal); respPayloads = diff --git a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachineTest.java b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachineTest.java index 20e4b76b..eab7f4a6 100644 --- a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachineTest.java +++ b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachineTest.java @@ -270,19 +270,7 @@ public final class ChildSessionStateMachineTest { // Setup thread and looper mLooper = new TestLooper(); - mChildSessionStateMachine = - new ChildSessionStateMachine( - mLooper.getLooper(), - mContext, - IKE_SESSION_UNIQUE_ID, - mMockAlarmManager, - createMockRandomFactory(), - mMockIpSecManager, - mIpSecSpiGenerator, - mChildSessionParams, - mSpyUserCbExecutor, - mMockChildSessionCallback, - mMockChildSessionSmCallback); + mChildSessionStateMachine = buildChildSession(mChildSessionParams); mChildSessionStateMachine.setDbg(true); SaRecord.setSaRecordHelper(mMockSaRecordHelper); @@ -951,6 +939,14 @@ public final class ChildSessionStateMachineTest { (IkeSaPayload) (IkeTestUtils.hexStringToIkePayload( IkePayload.PAYLOAD_TYPE_SA, true, inboundSaHexString)); + + return makeInboundRekeyChildPayloads(remoteSpi, saPayload, isLocalInitRekey); + } + + private List makeInboundRekeyChildPayloads( + int remoteSpi, IkeSaPayload saPayload, boolean isLocalInitRekey) throws Exception { + List inboundPayloads = new LinkedList<>(); + inboundPayloads.add(saPayload); // Build TS Payloads @@ -1261,6 +1257,8 @@ public final class ChildSessionStateMachineTest { IKE_EXCHANGE_SUBTYPE_REKEY_CHILD, EXCHANGE_TYPE_CREATE_CHILD_SA, rekeyReqPayloads); mLooper.dispatchAll(); + assertEquals(0, mChildSessionStateMachine.mSaProposal.getDhGroups().size()); + assertTrue( mChildSessionStateMachine.getCurrentState() instanceof ChildSessionStateMachine.RekeyChildRemoteDelete); @@ -1674,4 +1672,71 @@ public final class ChildSessionStateMachineTest { verifyHandleFatalErrorAndQuit(IkeInternalException.class); verify(spyIkeLog).wtf(anyString(), anyString(), any(RuntimeException.class)); } + + @Test + public void testFirstChildLocalRekey() throws Exception { + ChildSaProposal saProposal = buildSaProposalWithDhGroup(); + ChildSessionParams childSessionParams = + new TunnelModeChildSessionParams.Builder() + .addSaProposal(saProposal) + .addInternalAddressRequest(AF_INET) + .addInternalAddressRequest(INTERNAL_ADDRESS) + .build(); + mChildSessionStateMachine = buildChildSession(childSessionParams); + mChildSessionStateMachine.mIsFirstChild = true; + mChildSessionStateMachine.setDbg(true); + mChildSessionStateMachine.start(); + + setupIdleStateMachine(); + + assertEquals(0, mChildSessionStateMachine.mSaProposal.getDhGroups().size()); + + // Send Rekey-Create request + mChildSessionStateMachine.rekeyChildSession(); + mLooper.dispatchAll(); + assertTrue( + mChildSessionStateMachine.getCurrentState() + instanceof ChildSessionStateMachine.RekeyChildLocalCreate); + verifyOutboundRekeyKePayload(false /*isResp*/); + } + + private void verifyOutboundRekeyKePayload(boolean isResp) { + verify(mMockChildSessionSmCallback) + .onOutboundPayloadsReady( + eq(EXCHANGE_TYPE_CREATE_CHILD_SA), + eq(isResp), + mPayloadListCaptor.capture(), + eq(mChildSessionStateMachine)); + + // Verify outbound payload list + List reqPayloadList = mPayloadListCaptor.getValue(); + + assertNotNull( + IkePayload.getPayloadForTypeInProvidedList( + PAYLOAD_TYPE_KE, IkeKePayload.class, reqPayloadList)); + } + + private ChildSessionStateMachine buildChildSession(ChildSessionParams childSessionParams) { + return new ChildSessionStateMachine( + mLooper.getLooper(), + mContext, + IKE_SESSION_UNIQUE_ID, + mMockAlarmManager, + createMockRandomFactory(), + mMockIpSecManager, + mIpSecSpiGenerator, + childSessionParams, + mSpyUserCbExecutor, + mMockChildSessionCallback, + mMockChildSessionSmCallback); + } + + private ChildSaProposal buildSaProposalWithDhGroup() { + return new ChildSaProposal.Builder() + .addEncryptionAlgorithm( + SaProposal.ENCRYPTION_ALGORITHM_AES_CBC, SaProposal.KEY_LEN_AES_128) + .addIntegrityAlgorithm(SaProposal.INTEGRITY_ALGORITHM_HMAC_SHA1_96) + .addDhGroup(SaProposal.DH_GROUP_2048_BIT_MODP) + .build(); + } } -- cgit v1.2.3 From edd48e6eb3e59c594b90a960c47eca02585fc346 Mon Sep 17 00:00:00 2001 From: evitayan Date: Tue, 21 Apr 2020 15:21:47 -0700 Subject: Reschedule rekey in rekey state instead of TempFailureHandler This commit does not schedule retry in TempFailureHandler anymore but in the current state. Reason is that some events like rekey should be scheduled by AlarmManager instead of in the handler. This commit: - Adds a method stub #rescheduleRekey in SaRecord - Schedule rekey retry in rekey state This CL is a preparation CL before we can switch to use alarm manager to schedule the initial rekey and rekey retry. Bug: 149058810 Test: atest FrameworksIkeTests Change-Id: If4bbf9c889f130308839c4aa57334951a78f60dd Merged-In: If4bbf9c889f130308839c4aa57334951a78f60dd --- .../net/ipsec/ike/AbstractSessionStateMachine.java | 3 + .../net/ipsec/ike/ChildSessionStateMachine.java | 20 +--- .../net/ipsec/ike/IkeSessionStateMachine.java | 30 +++--- .../android/internal/net/ipsec/ike/SaRecord.java | 5 + .../ipsec/ike/ChildSessionStateMachineTest.java | 5 +- .../net/ipsec/ike/IkeSessionStateMachineTest.java | 109 +++++++++++++-------- 6 files changed, 98 insertions(+), 74 deletions(-) diff --git a/src/java/com/android/internal/net/ipsec/ike/AbstractSessionStateMachine.java b/src/java/com/android/internal/net/ipsec/ike/AbstractSessionStateMachine.java index 458f2610..1ed89b97 100644 --- a/src/java/com/android/internal/net/ipsec/ike/AbstractSessionStateMachine.java +++ b/src/java/com/android/internal/net/ipsec/ike/AbstractSessionStateMachine.java @@ -78,6 +78,9 @@ abstract class AbstractSessionStateMachine extends StateMachine { // Use a value greater than the retransmit-failure timeout. static final long REKEY_DELETE_TIMEOUT_MS = TimeUnit.SECONDS.toMillis(180L); + // Default delay time for retrying a request + static final long RETRY_INTERVAL_MS = TimeUnit.SECONDS.toMillis(15L); + protected final Executor mUserCbExecutor; private final String mLogTag; diff --git a/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachine.java b/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachine.java index 8697ab50..f97ef207 100644 --- a/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachine.java +++ b/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachine.java @@ -79,7 +79,6 @@ import com.android.internal.net.ipsec.ike.crypto.IkeMacPrf; import com.android.internal.net.ipsec.ike.exceptions.InvalidKeException; import com.android.internal.net.ipsec.ike.exceptions.InvalidSyntaxException; import com.android.internal.net.ipsec.ike.exceptions.NoValidProposalChosenException; -import com.android.internal.net.ipsec.ike.exceptions.TemporaryFailureException; import com.android.internal.net.ipsec.ike.exceptions.TsUnacceptableException; import com.android.internal.net.ipsec.ike.message.IkeConfigPayload; import com.android.internal.net.ipsec.ike.message.IkeConfigPayload.ConfigAttribute; @@ -1282,8 +1281,7 @@ public class ChildSessionStateMachine extends AbstractSessionStateMachine { ChildSessionStateMachine.this); } catch (SpiUnavailableException | ResourceUnavailableException e) { loge("Fail to assign Child SPI. Schedule a retry for rekey Child"); - mChildSmCallback.scheduleRetryLocalRequest( - (ChildLocalRequest) mCurrentChildSaRecord.getFutureRekeyEvent()); + mCurrentChildSaRecord.rescheduleRekey(RETRY_INTERVAL_MS); transitionTo(mIdle); } } @@ -1363,18 +1361,10 @@ public class ChildSessionStateMachine extends AbstractSessionStateMachine { resp.registeredSpi, createChildResult.exception); break; case CREATE_STATUS_CHILD_ERROR_RCV_NOTIFY: - if (createChildResult.exception instanceof TemporaryFailureException) { - loge( - "Received TEMPORARY_FAILURE for rekey Child. Retry has" - + "already been scheduled by IKE Session."); - } else { - loge( - "Received error notification for rekey Child. Schedule a" - + " retry"); - mChildSmCallback.scheduleRetryLocalRequest( - (ChildLocalRequest) - mCurrentChildSaRecord.getFutureRekeyEvent()); - } + loge( + "Received error notification for rekey Child. Schedule a" + + " retry"); + mCurrentChildSaRecord.rescheduleRekey(RETRY_INTERVAL_MS); transitionTo(mIdle); break; diff --git a/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java b/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java index 87eab421..bd14ba09 100644 --- a/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java +++ b/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java @@ -214,9 +214,6 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { // Default fragment size in bytes. @VisibleForTesting static final int DEFAULT_FRAGMENT_SIZE = 1280; - // Default delay time for retrying a request - @VisibleForTesting static final long RETRY_INTERVAL_MS = TimeUnit.SECONDS.toMillis(15L); - // Close IKE Session when all responses during this time were TEMPORARY_FAILURE(s). This // indicates that something has gone wrong, and we are out of sync. @VisibleForTesting @@ -731,15 +728,13 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { } } - /** Schedule retry when a request got rejected by TEMPORARY_FAILURE. */ - public void handleTempFailure(LocalRequest localRequest) { - logd( - "TempFailureHandler: Receive TEMPORARY FAILURE. Reschedule request: " - + localRequest.procedureType); - - // TODO: Support customized delay time when this is a rekey request and SA is going to - // expire soon. - scheduleRetry(localRequest); + /** + * Schedule temporary failure timeout. + * + *

Caller of this method is responsible for scheduling retry of the rejected request. + */ + public void handleTempFailure() { + logd("TempFailureHandler: Receive TEMPORARY FAILURE"); if (!mTempFailureReceived) { sendEmptyMessageDelayed(TEMP_FAILURE_RETRY_TIMEOUT, TEMP_FAILURE_RETRY_TIMEOUT_MS); @@ -2215,7 +2210,9 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { @Override protected void handleTempFailure() { - mTempFailHandler.handleTempFailure(mLocalRequestOngoing); + // The ChildSessionStateMachine will be responsible for rescheduling the rejected + // request. + mTempFailHandler.handleTempFailure(); } private void transitionToIdleIfAllProceduresDone() { @@ -3878,7 +3875,7 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { + firstErrorNotifyPayload.notifyType + " for rekey IKE. Schedule a retry"); if (!isSimulRekey) { - scheduleRetry(mCurrentIkeSaRecord.getFutureRekeyEvent()); + mCurrentIkeSaRecord.rescheduleRekey(RETRY_INTERVAL_MS); } } @@ -3969,7 +3966,7 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { mRetransmitter = new EncryptedRetransmitter(buildIkeRekeyReq()); } catch (IOException e) { loge("Fail to assign IKE SPI for rekey. Schedule a retry.", e); - scheduleRetry(mCurrentIkeSaRecord.getFutureRekeyEvent()); + mCurrentIkeSaRecord.rescheduleRekey(RETRY_INTERVAL_MS); transitionTo(mIdle); } } @@ -3981,7 +3978,8 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { @Override protected void handleTempFailure() { - mTempFailHandler.handleTempFailure(mCurrentIkeSaRecord.getFutureRekeyEvent()); + mTempFailHandler.handleTempFailure(); + mCurrentIkeSaRecord.rescheduleRekey(RETRY_INTERVAL_MS); } /** diff --git a/src/java/com/android/internal/net/ipsec/ike/SaRecord.java b/src/java/com/android/internal/net/ipsec/ike/SaRecord.java index a0b585fc..cfabfae5 100644 --- a/src/java/com/android/internal/net/ipsec/ike/SaRecord.java +++ b/src/java/com/android/internal/net/ipsec/ike/SaRecord.java @@ -188,6 +188,11 @@ public abstract class SaRecord implements AutoCloseable { return mFutureRekeyEvent; } + /** Reschedule rekey */ + public void rescheduleRekey(long retryDelayMs) { + // TODO(b/149058810): Schedule rekey using alarm manager + } + /** Package private */ @VisibleForTesting static void setSaRecordHelper(ISaRecordHelper helper) { diff --git a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachineTest.java b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachineTest.java index 20e4b76b..e81b4e37 100644 --- a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachineTest.java +++ b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachineTest.java @@ -22,6 +22,7 @@ import static android.net.ipsec.ike.exceptions.IkeProtocolException.ERROR_TYPE_T import static android.system.OsConstants.AF_INET; import static com.android.internal.net.TestUtils.createMockRandomFactory; +import static com.android.internal.net.ipsec.ike.AbstractSessionStateMachine.RETRY_INTERVAL_MS; import static com.android.internal.net.ipsec.ike.ChildSessionStateMachine.CMD_FORCE_TRANSITION; import static com.android.internal.net.ipsec.ike.IkeSessionStateMachine.CMD_LOCAL_REQUEST_REKEY_CHILD; import static com.android.internal.net.ipsec.ike.IkeSessionStateMachine.IKE_EXCHANGE_SUBTYPE_DELETE_CHILD; @@ -1061,9 +1062,7 @@ public final class ChildSessionStateMachineTest { mLooper.dispatchAll(); // Verify rekey has been rescheduled and Child Session is alive - verify(mMockChildSessionSmCallback) - .scheduleRetryLocalRequest( - (ChildLocalRequest) mSpyCurrentChildSaRecord.getFutureRekeyEvent()); + verify(mSpyCurrentChildSaRecord).rescheduleRekey(eq(RETRY_INTERVAL_MS)); assertTrue( mChildSessionStateMachine.getCurrentState() instanceof ChildSessionStateMachine.Idle); diff --git a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java index a41ceca7..068c070b 100644 --- a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java +++ b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java @@ -28,6 +28,7 @@ import static android.system.OsConstants.AF_INET; import static android.system.OsConstants.AF_INET6; import static com.android.internal.net.TestUtils.createMockRandomFactory; +import static com.android.internal.net.ipsec.ike.AbstractSessionStateMachine.RETRY_INTERVAL_MS; import static com.android.internal.net.ipsec.ike.IkeSessionStateMachine.CMD_LOCAL_REQUEST_REKEY_IKE; import static com.android.internal.net.ipsec.ike.IkeSessionStateMachine.CMD_RECEIVE_IKE_PACKET; import static com.android.internal.net.ipsec.ike.IkeSessionStateMachine.IKE_EXCHANGE_SUBTYPE_DELETE_CHILD; @@ -67,6 +68,7 @@ import static org.mockito.Matchers.anyObject; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.argThat; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; @@ -3077,17 +3079,10 @@ public final class IkeSessionStateMachineTest extends IkeSessionTestBase { mIkeSessionStateMachine.sendMessage(CMD_RECEIVE_IKE_PACKET, resp); mLooper.dispatchAll(); - // Verify IKE Session goes back to Idle + // Verify IKE Session goes back to Idle and retry is scheduled + verify(mSpyCurrentIkeSaRecord).rescheduleRekey(eq(RETRY_INTERVAL_MS)); assertTrue( mIkeSessionStateMachine.getCurrentState() instanceof IkeSessionStateMachine.Idle); - - // Move time forward to trigger retry - mLooper.moveTimeForward(IkeSessionStateMachine.RETRY_INTERVAL_MS); - mLooper.dispatchAll(); - - assertTrue( - mIkeSessionStateMachine.getCurrentState() - instanceof IkeSessionStateMachine.RekeyIkeLocalCreate); } @Test @@ -3189,6 +3184,40 @@ public final class IkeSessionStateMachineTest extends IkeSessionTestBase { instanceof IkeSessionStateMachine.RekeyIkeLocalCreate); } + private void mockRescheduleRekey(IkeSaRecord spySaRecord) { + IkeLocalRequest rekeyReq = + new IkeLocalRequest(IkeSessionStateMachine.CMD_LOCAL_REQUEST_REKEY_IKE); + doAnswer( + (invocation) -> { + mIkeSessionStateMachine.sendMessageDelayed( + IkeSessionStateMachine.CMD_LOCAL_REQUEST_REKEY_IKE, + rekeyReq, + RETRY_INTERVAL_MS); + return null; + }) + .when(spySaRecord) + .rescheduleRekey(eq(RETRY_INTERVAL_MS)); + } + + @Test + public void testRekeyIkeLocalCreateHandleRespWithTempFailure() throws Exception { + setupIdleStateMachine(); + + // Send Rekey-Create request + mIkeSessionStateMachine.sendMessage( + IkeSessionStateMachine.CMD_EXECUTE_LOCAL_REQ, + new IkeLocalRequest(IkeSessionStateMachine.CMD_LOCAL_REQUEST_REKEY_IKE)); + mLooper.dispatchAll(); + + // Mock sending TEMPORARY_FAILURE response + mockRcvTempFail(); + mLooper.dispatchAll(); + + verify(mSpyCurrentIkeSaRecord).rescheduleRekey(eq(RETRY_INTERVAL_MS)); + assertTrue( + mIkeSessionStateMachine.getCurrentState() instanceof IkeSessionStateMachine.Idle); + } + private void mockCreateAndTransitionToRekeyDeleteLocal() { // Seed fake rekey data and force transition to RekeyIkeLocalDelete mIkeSessionStateMachine.mLocalInitNewIkeSaRecord = mSpyLocalInitIkeSaRecord; @@ -4392,24 +4421,6 @@ public final class IkeSessionStateMachineTest extends IkeSessionTestBase { mLooper.dispatchAll(); } - @Test - public void testTempFailureHandlerScheduleRetry() throws Exception { - mockSendRekeyChildReq(); - - // Mock sending TEMPORARY_FAILURE response - mockRcvTempFail(); - - // Move time forward to trigger retry - mLooper.moveTimeForward(IkeSessionStateMachine.RETRY_INTERVAL_MS); - mLooper.dispatchAll(); - - // Verify that rekey is triggered again - assertTrue( - mIkeSessionStateMachine.getCurrentState() - instanceof IkeSessionStateMachine.ChildProcedureOngoing); - verify(mMockChildSessionStateMachine, times(2)).rekeyChildSession(); - } - @Ignore public void disableTestTempFailureHandlerTimeout() throws Exception { long currentTime = 0; @@ -4437,28 +4448,48 @@ public final class IkeSessionStateMachineTest extends IkeSessionTestBase { @Test public void testTempFailureHandlerCancelTimer() throws Exception { - mockSendRekeyChildReq(); + mockRescheduleRekey(mSpyCurrentIkeSaRecord); + setupIdleStateMachine(); + + // Send Rekey-Create request + mIkeSessionStateMachine.sendMessage( + IkeSessionStateMachine.CMD_EXECUTE_LOCAL_REQ, + new IkeLocalRequest(IkeSessionStateMachine.CMD_LOCAL_REQUEST_REKEY_IKE)); + mLooper.dispatchAll(); + verifyRetransmissionStarted(); // Mock sending TEMPORARY_FAILURE response mockRcvTempFail(); + mLooper.dispatchAll(); + verify(mSpyCurrentIkeSaRecord).rescheduleRekey(eq(RETRY_INTERVAL_MS)); + assertTrue( + mIkeSessionStateMachine.getCurrentState() instanceof IkeSessionStateMachine.Idle); // Move time forward to trigger retry mLooper.moveTimeForward(IkeSessionStateMachine.RETRY_INTERVAL_MS); mLooper.dispatchAll(); - verify(mMockChildSessionStateMachine, times(2)).rekeyChildSession(); + assertTrue( + mIkeSessionStateMachine.getCurrentState() + instanceof IkeSessionStateMachine.RekeyIkeLocalCreate); - // Mock sending a valid response - ReceivedIkePacket resp = - makeDummyEncryptedReceivedIkePacketWithPayloadList( - mSpyCurrentIkeSaRecord, - EXCHANGE_TYPE_CREATE_CHILD_SA, - true /*isResp*/, - new LinkedList<>()); - mIkeSessionStateMachine.sendMessage(IkeSessionStateMachine.CMD_RECEIVE_IKE_PACKET, resp); + // Prepare "rekeyed" SA + setupRekeyedIkeSa(mSpyLocalInitIkeSaRecord); + + // Receive valid Rekey-Create response + ReceivedIkePacket dummyRekeyIkeRespReceivedPacket = makeRekeyIkeResponse(); mIkeSessionStateMachine.sendMessage( - IkeSessionStateMachine.CMD_FORCE_TRANSITION, mIkeSessionStateMachine.mIdle); + IkeSessionStateMachine.CMD_RECEIVE_IKE_PACKET, dummyRekeyIkeRespReceivedPacket); mLooper.dispatchAll(); + // Receive Delete response + ReceivedIkePacket dummyDeleteIkeRespReceivedPacket = + makeDeleteIkeResponse(mSpyCurrentIkeSaRecord); + mIkeSessionStateMachine.sendMessage( + IkeSessionStateMachine.CMD_RECEIVE_IKE_PACKET, dummyDeleteIkeRespReceivedPacket); + mLooper.dispatchAll(); + assertTrue( + mIkeSessionStateMachine.getCurrentState() instanceof IkeSessionStateMachine.Idle); + // Move time forward mLooper.moveTimeForward(IkeSessionStateMachine.TEMP_FAILURE_RETRY_TIMEOUT_MS); mLooper.dispatchAll(); @@ -4466,8 +4497,6 @@ public final class IkeSessionStateMachineTest extends IkeSessionTestBase { // Validate IKE Session is not closed assertTrue( mIkeSessionStateMachine.getCurrentState() instanceof IkeSessionStateMachine.Idle); - // Validate no more retry - verify(mMockChildSessionStateMachine, times(2)).rekeyChildSession(); } @Ignore -- cgit v1.2.3 From e2a5da6eca2b1ef41bb14e1d2ac234e97b095007 Mon Sep 17 00:00:00 2001 From: evitayan Date: Thu, 30 Apr 2020 15:41:58 -0700 Subject: Use AlarmManager to trigger rekey This commit uses alarm mamager to schedule rekey instead of using handler. Bug: 149058810 Test: atest FrameworksIkeTests Change-Id: I370ecc858cbf1935d55b3f9b6fffbc4e48b5eed5 Merged-In: I370ecc858cbf1935d55b3f9b6fffbc4e48b5eed5 --- .../net/ipsec/ike/ChildSessionStateMachine.java | 10 --- .../net/ipsec/ike/IkeSessionStateMachine.java | 17 ++--- .../android/internal/net/ipsec/ike/SaRecord.java | 75 +++++++++------------- .../ipsec/ike/ChildSessionStateMachineTest.java | 20 ++---- .../net/ipsec/ike/IkeSessionStateMachineTest.java | 26 +++++++- .../internal/net/ipsec/ike/SaRecordTest.java | 20 ++---- 6 files changed, 69 insertions(+), 99 deletions(-) diff --git a/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachine.java b/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachine.java index f97ef207..b331d3e3 100644 --- a/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachine.java +++ b/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachine.java @@ -743,12 +743,9 @@ public class ChildSessionStateMachine extends AbstractSessionStateMachine { mSkD, mChildSessionParams.isTransportMode(), true /*isLocalInit*/, - rekeyLocalRequest, buildSaLifetimeAlarmSched( createChildResult.respSpi.getSpi())); - mChildSmCallback.scheduleLocalRequest(rekeyLocalRequest, getRekeyTimeout()); - ChildSessionConfiguration sessionConfig = buildChildSessionConfigFromResp(createChildResult, respPayloads); executeUserCallback( @@ -1326,13 +1323,9 @@ public class ChildSessionStateMachine extends AbstractSessionStateMachine { mSkD, mChildSessionParams.isTransportMode(), true /*isLocalInit*/, - rekeyLocalRequest, buildSaLifetimeAlarmSched( createChildResult.respSpi.getSpi())); - mChildSmCallback.scheduleLocalRequest( - rekeyLocalRequest, getRekeyTimeout()); - executeUserCallback( () -> { mUserCallback.onIpSecTransformCreated( @@ -1494,12 +1487,9 @@ public class ChildSessionStateMachine extends AbstractSessionStateMachine { mSkD, mChildSessionParams.isTransportMode(), false /*isLocalInit*/, - rekeyLocalRequest, buildSaLifetimeAlarmSched( createChildResult.initSpi.getSpi())); - mChildSmCallback.scheduleLocalRequest(rekeyLocalRequest, getRekeyTimeout()); - mChildSmCallback.onChildSaCreated( mRemoteInitNewChildSaRecord.getRemoteSpi(), ChildSessionStateMachine.this); diff --git a/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java b/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java index bd14ba09..6ac47989 100644 --- a/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java +++ b/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java @@ -768,8 +768,6 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { // IkeSaRecord is created. Calling this method at the end of exchange will double-register // the SPI but it is safe because the key and value are not changed. mIkeSocket.registerIke(record.getLocalSpi(), this); - - scheduleRekeySession(record.getFutureRekeyEvent()); } @VisibleForTesting @@ -1195,7 +1193,8 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { return obtainMessage(CMD_ALARM_FIRED, mIkeSessionId, localRequestType, spiBundle); } - private SaLifetimeAlarmScheduler buildSaLifetimeAlarmScheduler(long remoteSpi) { + @VisibleForTesting + SaLifetimeAlarmScheduler buildSaLifetimeAlarmScheduler(long remoteSpi) { PendingIntent deleteSaIntent = buildIkeAlarmIntent( mContext, @@ -1465,14 +1464,12 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { // Software keepalive alarm is fired mIkeNattKeepalive.onAlarmFired(); return; - case CMD_LOCAL_REQUEST_DELETE_CHILD: - // Child SA (identified by remoteChildSpi) has hit its hard lifetime + case CMD_LOCAL_REQUEST_DELETE_CHILD: // Hits hard lifetime; fall through + case CMD_LOCAL_REQUEST_REKEY_CHILD: // Hits soft lifetime enqueueChildLocalRequest(message); return; - case CMD_LOCAL_REQUEST_DELETE_IKE: - // IKE SA hits its hard lifetime - enqueueIkeLocalRequest(message); - return; + case CMD_LOCAL_REQUEST_DELETE_IKE: // Hits hard lifetime; fall through + case CMD_LOCAL_REQUEST_REKEY_IKE: // Hits soft lifetime; fall through case CMD_LOCAL_REQUEST_DPD: // IKE Session has not received any protectd IKE packet for the whole DPD delay enqueueIkeLocalRequest(message); @@ -2691,7 +2688,6 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { mIkePrf, mIkeIntegrity == null ? 0 : mIkeIntegrity.getKeyLength(), mIkeCipher.getKeyLength(), - new IkeLocalRequest(CMD_LOCAL_REQUEST_REKEY_IKE), buildSaLifetimeAlarmScheduler(mRemoteIkeSpiResource.getSpi())); addIkeSaRecord(mCurrentIkeSaRecord); @@ -3937,7 +3933,6 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { newIntegrity == null ? 0 : newIntegrity.getKeyLength(), newCipher.getKeyLength(), isLocalInit, - new IkeLocalRequest(CMD_LOCAL_REQUEST_REKEY_IKE), buildSaLifetimeAlarmScheduler(remoteSpi)); addIkeSaRecord(newSaRecord); diff --git a/src/java/com/android/internal/net/ipsec/ike/SaRecord.java b/src/java/com/android/internal/net/ipsec/ike/SaRecord.java index cfabfae5..c2d86dc9 100644 --- a/src/java/com/android/internal/net/ipsec/ike/SaRecord.java +++ b/src/java/com/android/internal/net/ipsec/ike/SaRecord.java @@ -31,8 +31,6 @@ import android.os.SystemClock; import android.util.CloseGuard; import com.android.internal.annotations.VisibleForTesting; -import com.android.internal.net.ipsec.ike.IkeLocalRequestScheduler.ChildLocalRequest; -import com.android.internal.net.ipsec.ike.IkeLocalRequestScheduler.LocalRequest; import com.android.internal.net.ipsec.ike.crypto.IkeCipher; import com.android.internal.net.ipsec.ike.crypto.IkeMacIntegrity; import com.android.internal.net.ipsec.ike.crypto.IkeMacPrf; @@ -82,10 +80,7 @@ public abstract class SaRecord implements AutoCloseable { private final byte[] mSkEi; private final byte[] mSkEr; - private final SaLifetimeAlarmScheduler mSaLifetimeAlarmScheduler; - - // TODO(b/149058810): Use AlarmManager and PendingIntent to schedule rekey - private final LocalRequest mFutureRekeyEvent; + @VisibleForTesting final SaLifetimeAlarmScheduler mSaLifetimeAlarmScheduler; private final CloseGuard mCloseGuard = new CloseGuard(); @@ -98,7 +93,6 @@ public abstract class SaRecord implements AutoCloseable { byte[] skAr, byte[] skEi, byte[] skEr, - LocalRequest futureRekeyEvent, SaLifetimeAlarmScheduler saLifetimeAlarmScheduler) { isLocalInit = localInit; nonceInitiator = nonceInit; @@ -117,9 +111,6 @@ public abstract class SaRecord implements AutoCloseable { mSaLifetimeAlarmScheduler = saLifetimeAlarmScheduler; mSaLifetimeAlarmScheduler.scheduleLifetimeExpiryAlarm(getTag()); - // TODO(b/149058810): Use alarmManager to schedule rekey event and remove mFutureRekeyEvent - mFutureRekeyEvent = futureRekeyEvent; - mCloseGuard.open("close"); } @@ -167,6 +158,11 @@ public abstract class SaRecord implements AutoCloseable { return isLocalInit ? mSkEr : mSkEi; } + /** Reschedule rekey */ + public void rescheduleRekey(long retryDelayMs) { + mSaLifetimeAlarmScheduler.rescheduleRekey(retryDelayMs); + } + /** Check that the SaRecord was closed properly. */ @Override protected void finalize() throws Throwable { @@ -178,21 +174,9 @@ public abstract class SaRecord implements AutoCloseable { @Override public void close() { - mFutureRekeyEvent.cancel(); - mSaLifetimeAlarmScheduler.cancelLifetimeExpiryAlarm(getTag()); } - /** Package private */ - LocalRequest getFutureRekeyEvent() { - return mFutureRekeyEvent; - } - - /** Reschedule rekey */ - public void rescheduleRekey(long retryDelayMs) { - // TODO(b/149058810): Schedule rekey using alarm manager - } - /** Package private */ @VisibleForTesting static void setSaRecordHelper(ISaRecordHelper helper) { @@ -341,7 +325,6 @@ public abstract class SaRecord implements AutoCloseable { skEr, skPi, skPr, - ikeSaRecordConfig.futureRekeyEvent, ikeSaRecordConfig.saLifetimeAlarmScheduler); } @@ -476,8 +459,7 @@ public abstract class SaRecord implements AutoCloseable { skEr, inTransform, outTransform, - childSaRecordConfig.futureRekeyEvent, - childSaRecordConfig.saLifetimeAlarmSched); + childSaRecordConfig.saLifetimeAlarmScheduler); } catch (Exception e) { if (initTransform != null) initTransform.close(); @@ -565,9 +547,26 @@ public abstract class SaRecord implements AutoCloseable { AlarmManager.ELAPSED_REALTIME_WAKEUP, SystemClock.elapsedRealtime() + mDeleteDelayMs, mDeleteSaIntent); - getIkeLog().d(tag, "Hard lifetime expiry alarm set for " + mDeleteDelayMs + "ms"); + mAlarmManager.setExactAndAllowWhileIdle( + AlarmManager.ELAPSED_REALTIME_WAKEUP, + SystemClock.elapsedRealtime() + mRekeyDelayMs, + mRekeySaIntent); + + getIkeLog() + .d( + tag, + "Lifetime alarm set: Hard lifetime (" + + mDeleteDelayMs + + "ms) Soft lifetime (" + + mRekeyDelayMs + + "ms)"); + } - // TODO: Schedule alarm for rekey + public void rescheduleRekey(long retryDelayMs) { + mAlarmManager.setExactAndAllowWhileIdle( + AlarmManager.ELAPSED_REALTIME_WAKEUP, + SystemClock.elapsedRealtime() + retryDelayMs, + mRekeySaIntent); } public void cancelLifetimeExpiryAlarm(String tag) { @@ -596,8 +595,7 @@ public abstract class SaRecord implements AutoCloseable { public final boolean isTransport; public final boolean isLocalInit; public final boolean hasIntegrityAlgo; - public final ChildLocalRequest futureRekeyEvent; - public final SaLifetimeAlarmScheduler saLifetimeAlarmSched; + public final SaLifetimeAlarmScheduler saLifetimeAlarmScheduler; ChildSaRecordConfig( Context context, @@ -612,8 +610,7 @@ public abstract class SaRecord implements AutoCloseable { byte[] skD, boolean isTransport, boolean isLocalInit, - ChildLocalRequest futureRekeyEvent, - SaLifetimeAlarmScheduler saLifetimeAlarmSched) { + SaLifetimeAlarmScheduler saLifetimeAlarmScheduler) { this.context = context; this.initSpi = initSpi; this.respSpi = respSpi; @@ -627,8 +624,7 @@ public abstract class SaRecord implements AutoCloseable { this.isTransport = isTransport; this.isLocalInit = isLocalInit; hasIntegrityAlgo = (integrityAlgo != null); - this.futureRekeyEvent = futureRekeyEvent; - this.saLifetimeAlarmSched = saLifetimeAlarmSched; + this.saLifetimeAlarmScheduler = saLifetimeAlarmScheduler; } } @@ -668,7 +664,6 @@ public abstract class SaRecord implements AutoCloseable { byte[] skEr, byte[] skPi, byte[] skPr, - LocalRequest futureRekeyEvent, SaLifetimeAlarmScheduler saLifetimeAlarmScheduler) { super( localInit, @@ -678,7 +673,6 @@ public abstract class SaRecord implements AutoCloseable { skAr, skEi, skEr, - futureRekeyEvent, saLifetimeAlarmScheduler); mInitiatorSpiResource = initSpi; @@ -711,7 +705,6 @@ public abstract class SaRecord implements AutoCloseable { IkeMacPrf prf, int integrityKeyLength, int encryptionKeyLength, - LocalRequest futureRekeyEvent, SaLifetimeAlarmScheduler saLifetimeAlarmScheduler) throws GeneralSecurityException { return sSaRecordHelper.makeFirstIkeSaRecord( @@ -724,7 +717,6 @@ public abstract class SaRecord implements AutoCloseable { integrityKeyLength, encryptionKeyLength, true /*isLocalInit*/, - futureRekeyEvent, saLifetimeAlarmScheduler)); } @@ -740,7 +732,6 @@ public abstract class SaRecord implements AutoCloseable { int integrityKeyLength, int encryptionKeyLength, boolean isLocalInit, - LocalRequest futureRekeyEvent, SaLifetimeAlarmScheduler saLifetimeAlarmScheduler) throws GeneralSecurityException { return sSaRecordHelper.makeRekeyedIkeSaRecord( @@ -755,7 +746,6 @@ public abstract class SaRecord implements AutoCloseable { integrityKeyLength, encryptionKeyLength, isLocalInit, - futureRekeyEvent, saLifetimeAlarmScheduler)); } @@ -934,7 +924,6 @@ public abstract class SaRecord implements AutoCloseable { public final int integrityKeyLength; public final int encryptionKeyLength; public final boolean isLocalInit; - public final LocalRequest futureRekeyEvent; public final SaLifetimeAlarmScheduler saLifetimeAlarmScheduler; IkeSaRecordConfig( @@ -944,7 +933,6 @@ public abstract class SaRecord implements AutoCloseable { int integrityKeyLength, int encryptionKeyLength, boolean isLocalInit, - LocalRequest futureRekeyEvent, SaLifetimeAlarmScheduler saLifetimeAlarmScheduler) { this.initSpi = initSpi; this.respSpi = respSpi; @@ -952,7 +940,6 @@ public abstract class SaRecord implements AutoCloseable { this.integrityKeyLength = integrityKeyLength; this.encryptionKeyLength = encryptionKeyLength; this.isLocalInit = isLocalInit; - this.futureRekeyEvent = futureRekeyEvent; this.saLifetimeAlarmScheduler = saLifetimeAlarmScheduler; } } @@ -984,7 +971,6 @@ public abstract class SaRecord implements AutoCloseable { byte[] skEr, IpSecTransform inTransform, IpSecTransform outTransform, - ChildLocalRequest futureRekeyEvent, SaLifetimeAlarmScheduler saLifetimeAlarmScheduler) { super( localInit, @@ -994,7 +980,6 @@ public abstract class SaRecord implements AutoCloseable { skAr, skEi, skEr, - futureRekeyEvent, saLifetimeAlarmScheduler); mInboundSpi = inSpi; @@ -1022,7 +1007,6 @@ public abstract class SaRecord implements AutoCloseable { byte[] skD, boolean isTransport, boolean isLocalInit, - ChildLocalRequest futureRekeyEvent, SaLifetimeAlarmScheduler saLifetimeAlarmScheduler) throws GeneralSecurityException, ResourceUnavailableException, SpiUnavailableException, IOException { @@ -1042,7 +1026,6 @@ public abstract class SaRecord implements AutoCloseable { skD, isTransport, isLocalInit, - futureRekeyEvent, saLifetimeAlarmScheduler)); } diff --git a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachineTest.java b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachineTest.java index e81b4e37..4dbcbdbd 100644 --- a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachineTest.java +++ b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachineTest.java @@ -48,11 +48,9 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.anyInt; -import static org.mockito.Matchers.anyLong; import static org.mockito.Matchers.anyObject; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; @@ -225,8 +223,9 @@ public final class ChildSessionStateMachineTest { private ArgumentMatcher mRekeyChildLocalReqMatcher = (argument) -> { - return CMD_LOCAL_REQUEST_REKEY_CHILD == argument.procedureType - && mMockChildSessionCallback == argument.childSessionCallback; + return (CMD_LOCAL_REQUEST_REKEY_CHILD == argument.procedureType + && mMockChildSessionCallback == argument.childSessionCallback + || CURRENT_CHILD_SA_SPI_OUT == argument.remoteSpi); }; public ChildSessionStateMachineTest() { @@ -397,7 +396,6 @@ public final class ChildSessionStateMachineTest { null, mock(IpSecTransform.class), mock(IpSecTransform.class), - mock(ChildLocalRequest.class), mock(SaLifetimeAlarmScheduler.class))); doNothing().when(child).close(); return child; @@ -439,11 +437,7 @@ public final class ChildSessionStateMachineTest { assertFalse(childSaRecordConfig.isTransport); assertEquals(isLocalInit, childSaRecordConfig.isLocalInit); assertTrue(childSaRecordConfig.hasIntegrityAlgo); - assertEquals( - CMD_LOCAL_REQUEST_REKEY_CHILD, childSaRecordConfig.futureRekeyEvent.procedureType); - assertEquals( - mMockChildSessionCallback, - childSaRecordConfig.futureRekeyEvent.childSessionCallback); + assertNotNull(childSaRecordConfig.saLifetimeAlarmScheduler); } private void verifyNotifyUsersCreateIpSecSa( @@ -493,8 +487,6 @@ public final class ChildSessionStateMachineTest { verify(mMockChildSessionSmCallback) .onChildSaCreated(anyInt(), eq(mChildSessionStateMachine)); - verify(mMockChildSessionSmCallback) - .scheduleLocalRequest(argThat(mRekeyChildLocalReqMatcher), anyLong()); verify(mMockChildSessionSmCallback).onProcedureFinished(mChildSessionStateMachine); assertTrue( mChildSessionStateMachine.getCurrentState() @@ -1024,8 +1016,6 @@ public final class ChildSessionStateMachineTest { .onChildSaCreated( eq(mSpyLocalInitNewChildSaRecord.getRemoteSpi()), eq(mChildSessionStateMachine)); - verify(mMockChildSessionSmCallback) - .scheduleLocalRequest(argThat(mRekeyChildLocalReqMatcher), anyLong()); verify(mMockSaRecordHelper) .makeChildSaRecord( @@ -1286,8 +1276,6 @@ public final class ChildSessionStateMachineTest { .onChildSaCreated( eq(mSpyRemoteInitNewChildSaRecord.getRemoteSpi()), eq(mChildSessionStateMachine)); - verify(mMockChildSessionSmCallback) - .scheduleLocalRequest(argThat(mRekeyChildLocalReqMatcher), anyLong()); verify(mMockSaRecordHelper) .makeChildSaRecord( diff --git a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java index 068c070b..b5df7b94 100644 --- a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java +++ b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java @@ -29,7 +29,6 @@ import static android.system.OsConstants.AF_INET6; import static com.android.internal.net.TestUtils.createMockRandomFactory; import static com.android.internal.net.ipsec.ike.AbstractSessionStateMachine.RETRY_INTERVAL_MS; -import static com.android.internal.net.ipsec.ike.IkeSessionStateMachine.CMD_LOCAL_REQUEST_REKEY_IKE; import static com.android.internal.net.ipsec.ike.IkeSessionStateMachine.CMD_RECEIVE_IKE_PACKET; import static com.android.internal.net.ipsec.ike.IkeSessionStateMachine.IKE_EXCHANGE_SUBTYPE_DELETE_CHILD; import static com.android.internal.net.ipsec.ike.IkeSessionStateMachine.IKE_EXCHANGE_SUBTYPE_REKEY_CHILD; @@ -643,10 +642,25 @@ public final class IkeSessionStateMachineTest extends IkeSessionTestBase { new byte[KEY_LEN_IKE_ENCR], TestUtils.hexStringToByteArray(PRF_KEY_INIT_HEX_STRING), TestUtils.hexStringToByteArray(PRF_KEY_RESP_HEX_STRING), - new IkeLocalRequest(CMD_LOCAL_REQUEST_REKEY_IKE), mock(SaLifetimeAlarmScheduler.class)); } + private void mockScheduleRekey(SaLifetimeAlarmScheduler mockSaLifetimeAlarmScheduler) { + IkeLocalRequest rekeyReq = + new IkeLocalRequest(IkeSessionStateMachine.CMD_LOCAL_REQUEST_REKEY_IKE); + doAnswer( + (invocation) -> { + mIkeSessionStateMachine.sendMessageDelayed( + IkeSessionStateMachine.CMD_LOCAL_REQUEST_REKEY_IKE, + rekeyReq, + mIkeSessionStateMachine.mIkeSessionParams + .getSoftLifetimeMsInternal()); + return null; + }) + .when(mockSaLifetimeAlarmScheduler) + .scheduleLifetimeExpiryAlarm(anyString()); + } + @Before public void setUp() throws Exception { super.setUp(); @@ -1218,6 +1232,9 @@ public final class IkeSessionStateMachineTest extends IkeSessionTestBase { .thenAnswer( (invocation) -> { captureAndReleaseIkeSpiResource(invocation, 2); + mockScheduleRekey(mSpyCurrentIkeSaRecord.mSaLifetimeAlarmScheduler); + mSpyCurrentIkeSaRecord.mSaLifetimeAlarmScheduler + .scheduleLifetimeExpiryAlarm(anyString()); return mSpyCurrentIkeSaRecord; }); } @@ -1230,6 +1247,9 @@ public final class IkeSessionStateMachineTest extends IkeSessionTestBase { .thenAnswer( (invocation) -> { captureAndReleaseIkeSpiResource(invocation, 4); + mockScheduleRekey(rekeySaRecord.mSaLifetimeAlarmScheduler); + rekeySaRecord.mSaLifetimeAlarmScheduler.scheduleLifetimeExpiryAlarm( + anyString()); return rekeySaRecord; }); } @@ -1405,7 +1425,7 @@ public final class IkeSessionStateMachineTest extends IkeSessionTestBase { assertEquals(KEY_LEN_IKE_PRF, ikeSaRecordConfig.prf.getKeyLength()); assertEquals(KEY_LEN_IKE_INTE, ikeSaRecordConfig.integrityKeyLength); assertEquals(KEY_LEN_IKE_ENCR, ikeSaRecordConfig.encryptionKeyLength); - assertEquals(CMD_LOCAL_REQUEST_REKEY_IKE, ikeSaRecordConfig.futureRekeyEvent.procedureType); + assertNotNull(ikeSaRecordConfig.saLifetimeAlarmScheduler); // Validate NAT detection assertTrue(mIkeSessionStateMachine.mIsLocalBehindNat); diff --git a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/SaRecordTest.java b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/SaRecordTest.java index 644ba066..b193099d 100644 --- a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/SaRecordTest.java +++ b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/SaRecordTest.java @@ -24,6 +24,7 @@ import static org.junit.Assert.assertTrue; import static org.mockito.AdditionalMatchers.aryEq; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyObject; +import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -38,8 +39,6 @@ import android.net.IpSecTransform; import android.net.ipsec.ike.SaProposal; import com.android.internal.net.TestUtils; -import com.android.internal.net.ipsec.ike.IkeLocalRequestScheduler.ChildLocalRequest; -import com.android.internal.net.ipsec.ike.IkeLocalRequestScheduler.LocalRequest; import com.android.internal.net.ipsec.ike.SaRecord.ChildSaRecord; import com.android.internal.net.ipsec.ike.SaRecord.ChildSaRecordConfig; import com.android.internal.net.ipsec.ike.SaRecord.IIpSecTransformHelper; @@ -148,9 +147,6 @@ public final class SaRecordTest { private IkeMacIntegrity mHmacSha1IntegrityMac; private IkeCipher mAesCbcCipher; - private LocalRequest mMockFutureRekeyIkeEvent; - private ChildLocalRequest mMockFutureRekeyChildEvent; - private SaLifetimeAlarmScheduler mMockLifetimeAlarmScheduler; private SaRecordHelper mSaRecordHelper = new SaRecordHelper(); @@ -168,8 +164,6 @@ public final class SaRecordTest { SaProposal.ENCRYPTION_ALGORITHM_AES_CBC, SaProposal.KEY_LEN_AES_128)); - mMockFutureRekeyIkeEvent = mock(LocalRequest.class); - mMockFutureRekeyChildEvent = mock(ChildLocalRequest.class); mMockLifetimeAlarmScheduler = mock(SaLifetimeAlarmScheduler.class); } @@ -192,8 +186,7 @@ public final class SaRecordTest { IKE_AUTH_ALGO_KEY_LEN, IKE_ENCR_ALGO_KEY_LEN, true /*isLocalInit*/, - mMockFutureRekeyIkeEvent, - mock(SaLifetimeAlarmScheduler.class)); + mMockLifetimeAlarmScheduler); int keyMaterialLen = IKE_SK_D_KEY_LEN @@ -225,10 +218,10 @@ public final class SaRecordTest { TestUtils.hexStringToByteArray(IKE_SK_PRF_INIT_HEX_STRING), ikeSaRecord.getSkPi()); assertArrayEquals( TestUtils.hexStringToByteArray(IKE_SK_PRF_RESP_HEX_STRING), ikeSaRecord.getSkPr()); + verify(mMockLifetimeAlarmScheduler).scheduleLifetimeExpiryAlarm(anyString()); ikeSaRecord.close(); - - verify(mMockFutureRekeyIkeEvent).cancel(); + verify(mMockLifetimeAlarmScheduler).cancelLifetimeExpiryAlarm(anyString()); } // Test generating keying material and building IpSecTransform for making Child SA. @@ -306,7 +299,6 @@ public final class SaRecordTest { TestUtils.hexStringToByteArray(IKE_SK_D_HEX_STRING), false /*isTransport*/, true /*isLocalInit*/, - mMockFutureRekeyChildEvent, mMockLifetimeAlarmScheduler); ChildSaRecord childSaRecord = @@ -332,8 +324,10 @@ public final class SaRecordTest { TestUtils.hexStringToByteArray(FIRST_CHILD_ENCR_RESP_HEX_STRING), childSaRecord.getInboundDecryptionKey()); + verify(mMockLifetimeAlarmScheduler).scheduleLifetimeExpiryAlarm(anyString()); + childSaRecord.close(); - verify(mMockFutureRekeyChildEvent).cancel(); + verify(mMockLifetimeAlarmScheduler).cancelLifetimeExpiryAlarm(anyString()); SaRecord.setIpSecTransformHelper(new IpSecTransformHelper()); } -- cgit v1.2.3 From a630efd48373a956972433d0c857aa008d12a76d Mon Sep 17 00:00:00 2001 From: Dheeraj Shetty Date: Fri, 1 May 2020 15:44:33 -0700 Subject: Use dh group from KE payload for remote rekey Test: atest FrameworksIkeTest Bug: 138965374 Change-Id: I7765379fd58ab3640db47f7fa6fd7e2149dc0f85 Merged-In: I7765379fd58ab3640db47f7fa6fd7e2149dc0f85 --- .../android/net/ipsec/ike/ChildSaProposal.java | 9 ++++ .../android/net/ipsec/ike/ChildSessionParams.java | 16 +++++- .../net/ipsec/ike/ChildSessionStateMachine.java | 35 +++++++++++-- .../ipsec/ike/ChildSessionStateMachineTest.java | 59 ++++++++++++++++++++-- 4 files changed, 109 insertions(+), 10 deletions(-) diff --git a/src/java/android/net/ipsec/ike/ChildSaProposal.java b/src/java/android/net/ipsec/ike/ChildSaProposal.java index bf55e8fa..7318659a 100644 --- a/src/java/android/net/ipsec/ike/ChildSaProposal.java +++ b/src/java/android/net/ipsec/ike/ChildSaProposal.java @@ -117,6 +117,15 @@ public final class ChildSaProposal extends SaProposal { && isTransformSelectedFrom(mEsns, ((ChildSaProposal) saProposal).mEsns); } + /** @hide */ + public ChildSaProposal getCopyWithAdditionalDhTransform(int dhGroup) { + return new ChildSaProposal( + getEncryptionTransforms(), + getIntegrityTransforms(), + new DhGroupTransform[] {new DhGroupTransform(dhGroup)}, + getEsnTransforms()); + } + /** * This class is used to incrementally construct a ChildSaProposal. ChildSaProposal instances * are immutable once built. diff --git a/src/java/android/net/ipsec/ike/ChildSessionParams.java b/src/java/android/net/ipsec/ike/ChildSessionParams.java index e34d1d6d..0221491c 100644 --- a/src/java/android/net/ipsec/ike/ChildSessionParams.java +++ b/src/java/android/net/ipsec/ike/ChildSessionParams.java @@ -35,6 +35,17 @@ import java.util.concurrent.TimeUnit; *

Note that references to negotiated configurations will be held, and the same parameters will * be reused during rekey. This includes SA Proposals, lifetimes and traffic selectors. * + *

IKE library will send out KE payload only if user has configured one or more DH groups. The KE + * payload in a request will use the first DH group from the first user provided SA proposal (or the + * peer selected SA proposal if it's a rekey request). The KE payload in a response will depend on + * the SA proposal negotiation result. + * + *

When requesting the first Child Session in IKE AUTH, IKE library will not propose any DH group + * even if user has configured it, as per RFC 7296. When rekeying this child session, IKE library + * will accept DH groups that are configured in its ChildSessionParams. If after rekeying user needs + * to have the same DH group as that of the IKE Session, then they need to explicitly set the same + * DH Group in ChildSessionParams. + * * @see {@link TunnelModeChildSessionParams} and {@link TransportModeChildSessionParams} * @hide */ @@ -238,8 +249,9 @@ public abstract class ChildSessionParams { break; case IkeTrafficSelector.TRAFFIC_SELECTOR_TYPE_IPV6_ADDR_RANGE: startAddress = InetAddresses.parseNumericAddress("::"); - endAddress = InetAddresses.parseNumericAddress( - "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff"); + endAddress = + InetAddresses.parseNumericAddress( + "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff"); break; default: throw new IllegalArgumentException("Invalid Traffic Selector type: " + tsType); diff --git a/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachine.java b/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachine.java index 72663a85..8e244946 100644 --- a/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachine.java +++ b/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachine.java @@ -1482,9 +1482,20 @@ public class ChildSessionStateMachine extends AbstractSessionStateMachine { IkePayload.getPayloadForTypeInProvidedList( PAYLOAD_TYPE_SA, IkeSaPayload.class, reqPayloads); - // TODO: Handle first child remote rekey request with KE payload. + IkeKePayload reqKePayload = + IkePayload.getPayloadForTypeInProvidedList( + PAYLOAD_TYPE_KE, IkeKePayload.class, reqPayloads); + + ChildSaProposal saProposal = mSaProposal; - byte respProposalNumber = reqSaPayload.getNegotiatedProposalNumber(mSaProposal); + // Try accepting a DH group requested during remote rekey for both first and + // additional Child Sessions even if it is different from the previously negotiated + // proposal. + if (reqKePayload != null && isKePayloadAcceptable(reqKePayload)) { + saProposal = mSaProposal.getCopyWithAdditionalDhTransform(reqKePayload.dhGroup); + } + + byte respProposalNumber = reqSaPayload.getNegotiatedProposalNumber(saProposal); respPayloads = CreateChildSaHelper.getRekeyChildCreateRespPayloads( @@ -1492,7 +1503,7 @@ public class ChildSessionStateMachine extends AbstractSessionStateMachine { mIpSecSpiGenerator, mLocalAddress, respProposalNumber, - mSaProposal, + saProposal, mLocalTs, mRemoteTs, mCurrentChildSaRecord.getLocalSpi(), @@ -1519,8 +1530,8 @@ public class ChildSessionStateMachine extends AbstractSessionStateMachine { switch (createChildResult.status) { case CREATE_STATUS_OK: try { - // Do not need to update the negotiated proposal and TS - // because they are not changed. + // Do not need to update TS because they are not changed. + mSaProposal = createChildResult.negotiatedProposal; ChildLocalRequest rekeyLocalRequest = makeRekeyLocalRequest(); @@ -1604,6 +1615,20 @@ public class ChildSessionStateMachine extends AbstractSessionStateMachine { } } + private boolean isKePayloadAcceptable(IkeKePayload reqKePayload) { + ChildSaProposal proposal = + mSaProposal.getCopyWithAdditionalDhTransform(reqKePayload.dhGroup); + + // Verify if this proposal is accepted by user + for (SaProposal saProposal : mChildSessionParams.getSaProposals()) { + if (proposal.isNegotiatedFrom(saProposal)) { + return true; + } + } + + return false; + } + private void handleCreationFailureAndBackToIdle(IkeProtocolException e) { loge("Received invalid Rekey Child request. Reject with error notification", e); diff --git a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachineTest.java b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachineTest.java index eab7f4a6..99f6d2e7 100644 --- a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachineTest.java +++ b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/ChildSessionStateMachineTest.java @@ -77,6 +77,7 @@ import android.net.ipsec.ike.ChildSessionCallback; import android.net.ipsec.ike.ChildSessionConfiguration; import android.net.ipsec.ike.ChildSessionParams; import android.net.ipsec.ike.IkeManager; +import android.net.ipsec.ike.IkeSaProposal; import android.net.ipsec.ike.IkeTrafficSelector; import android.net.ipsec.ike.SaProposal; import android.net.ipsec.ike.TunnelModeChildSessionParams; @@ -1675,7 +1676,7 @@ public final class ChildSessionStateMachineTest { @Test public void testFirstChildLocalRekey() throws Exception { - ChildSaProposal saProposal = buildSaProposalWithDhGroup(); + ChildSaProposal saProposal = buildSaProposalWithDhGroup(SaProposal.DH_GROUP_2048_BIT_MODP); ChildSessionParams childSessionParams = new TunnelModeChildSessionParams.Builder() .addSaProposal(saProposal) @@ -1694,9 +1695,11 @@ public final class ChildSessionStateMachineTest { // Send Rekey-Create request mChildSessionStateMachine.rekeyChildSession(); mLooper.dispatchAll(); + assertTrue( mChildSessionStateMachine.getCurrentState() instanceof ChildSessionStateMachine.RekeyChildLocalCreate); + verifyOutboundRekeyKePayload(false /*isResp*/); } @@ -1731,12 +1734,62 @@ public final class ChildSessionStateMachineTest { mMockChildSessionSmCallback); } - private ChildSaProposal buildSaProposalWithDhGroup() { + private ChildSaProposal buildSaProposalWithDhGroup(int dhGroup) { return new ChildSaProposal.Builder() .addEncryptionAlgorithm( SaProposal.ENCRYPTION_ALGORITHM_AES_CBC, SaProposal.KEY_LEN_AES_128) .addIntegrityAlgorithm(SaProposal.INTEGRITY_ALGORITHM_HMAC_SHA1_96) - .addDhGroup(SaProposal.DH_GROUP_2048_BIT_MODP) + .addDhGroup(dhGroup) .build(); } + + @Test + public void testRemoteRekeyWithKePayload() throws Exception { + // Use child session params with dh group to initiate the state machine + ChildSaProposal saProposal = buildSaProposalWithDhGroup(SaProposal.DH_GROUP_2048_BIT_MODP); + ChildSessionParams childSessionParams = + new TunnelModeChildSessionParams.Builder() + .addSaProposal(saProposal) + .addInternalAddressRequest(AF_INET) + .addInternalAddressRequest(INTERNAL_ADDRESS) + .build(); + mChildSessionStateMachine = buildChildSession(childSessionParams); + mChildSessionStateMachine.setDbg(true); + mChildSessionStateMachine.start(); + + setupIdleStateMachine(); + + // Setup for new Child SA negotiation. + setUpSpiResource(LOCAL_ADDRESS, REMOTE_INIT_NEW_CHILD_SA_SPI_IN); + setUpSpiResource(REMOTE_ADDRESS, REMOTE_INIT_NEW_CHILD_SA_SPI_OUT); + + IkeSaPayload saPayload = + IkeSaPayload.createChildSaRequestPayload( + new ChildSaProposal[] {saProposal}, mIpSecSpiGenerator, LOCAL_ADDRESS); + List rekeyReqPayloads = + makeInboundRekeyChildPayloads( + REMOTE_INIT_NEW_CHILD_SA_SPI_OUT, saPayload, false /*isLocalInitRekey*/); + + rekeyReqPayloads.add( + new IkeKePayload(IkeSaProposal.DH_GROUP_2048_BIT_MODP, createMockRandomFactory())); + + when(mMockSaRecordHelper.makeChildSaRecord( + eq(rekeyReqPayloads), any(List.class), any(ChildSaRecordConfig.class))) + .thenReturn(mSpyRemoteInitNewChildSaRecord); + + assertEquals(0, mChildSessionStateMachine.mSaProposal.getDhGroups().size()); + + // Receive rekey Child request + mChildSessionStateMachine.receiveRequest( + IKE_EXCHANGE_SUBTYPE_REKEY_CHILD, EXCHANGE_TYPE_CREATE_CHILD_SA, rekeyReqPayloads); + mLooper.dispatchAll(); + + assertTrue( + mChildSessionStateMachine.getCurrentState() + instanceof ChildSessionStateMachine.RekeyChildRemoteDelete); + + verifyOutboundRekeyKePayload(true /*isResp*/); + + assertEquals(1, mChildSessionStateMachine.mSaProposal.getDhGroups().size()); + } } -- cgit v1.2.3 From bce963be7ab073b77d5fac4809e6ba5e6359214e Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Thu, 30 Apr 2020 17:13:22 -0700 Subject: Add a busy-wakelock to IkeSessionStateMachine This change adds a wakelock to ensure that the device does not go to sleep in the middle of an exchange. Otherwise, the IKE session will be left in a non-deterministic state Bug: 152236790 Test: FrameworkIkeTests passing Change-Id: Ia57bb27ac75891cb47237596580792d58bfaf693 Merged-In: Ia57bb27ac75891cb47237596580792d58bfaf693 --- .../net/ipsec/ike/IkeLocalRequestScheduler.java | 7 +++++-- .../internal/net/ipsec/ike/IkeSessionStateMachine.java | 18 +++++++++++++++++- tests/iketests/AndroidManifest.xml | 2 ++ .../net/ipsec/ike/IkeSessionStateMachineTest.java | 12 ++++++++++++ .../internal/net/ipsec/ike/IkeSessionTestBase.java | 12 ++++++++++++ 5 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/java/com/android/internal/net/ipsec/ike/IkeLocalRequestScheduler.java b/src/java/com/android/internal/net/ipsec/ike/IkeLocalRequestScheduler.java index e86df232..f8008fbe 100644 --- a/src/java/com/android/internal/net/ipsec/ike/IkeLocalRequestScheduler.java +++ b/src/java/com/android/internal/net/ipsec/ike/IkeLocalRequestScheduler.java @@ -64,15 +64,18 @@ public final class IkeLocalRequestScheduler { * Notifies the scheduler that the caller is ready for a new procedure * *

Synchronously triggers the call to onNewProcedureReady. + * + * @return whether or not a new procedure was scheduled. */ - public void readyForNextProcedure() { + public boolean readyForNextProcedure() { while (!mRequestQueue.isEmpty()) { LocalRequest request = mRequestQueue.poll(); if (!request.isCancelled()) { mConsumer.onNewProcedureReady(request); - return; + return true; } } + return false; } /** diff --git a/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java b/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java index 5fbcda89..46517159 100644 --- a/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java +++ b/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java @@ -22,6 +22,7 @@ import static android.net.ipsec.ike.exceptions.IkeProtocolException.ERROR_TYPE_I import static android.net.ipsec.ike.exceptions.IkeProtocolException.ERROR_TYPE_NO_ADDITIONAL_SAS; import static android.net.ipsec.ike.exceptions.IkeProtocolException.ERROR_TYPE_TEMPORARY_FAILURE; import static android.net.ipsec.ike.exceptions.IkeProtocolException.ErrorType; +import static android.os.PowerManager.PARTIAL_WAKE_LOCK; import static com.android.internal.net.ipsec.ike.message.IkeConfigPayload.CONFIG_TYPE_REPLY; import static com.android.internal.net.ipsec.ike.message.IkeHeader.EXCHANGE_TYPE_INFORMATIONAL; @@ -74,6 +75,7 @@ import android.os.Bundle; import android.os.Handler; import android.os.Looper; import android.os.Message; +import android.os.PowerManager; import android.os.SystemClock; import android.system.ErrnoException; import android.system.Os; @@ -368,6 +370,9 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { */ private final IpSecSpiGenerator mIpSecSpiGenerator; + /** Ensures that the system does not go to sleep in the middle of an exchange. */ + private final PowerManager.WakeLock mBusyWakeLock; + @VisibleForTesting @GuardedBy("mChildCbToSessions") final HashMap mChildCbToSessions = @@ -495,6 +500,10 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { // callback instance in the future } + PowerManager pm = context.getSystemService(PowerManager.class); + mBusyWakeLock = pm.newWakeLock(PARTIAL_WAKE_LOCK, TAG + "mBusyWakeLock"); + mBusyWakeLock.setReferenceCounted(false); + mIkeSessionId = sIkeSessionIdGenerator.getAndIncrement(); sIkeAlarmReceiver.registerIkeSession(mIkeSessionId, getHandler()); @@ -551,6 +560,7 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { sendMessageAtFrontOfQueue(CMD_EXECUTE_LOCAL_REQ, localReq); }); + mBusyWakeLock.acquire(); start(); } @@ -963,6 +973,8 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { } // TODO: Remove the stored ikeSessionCallback } + + mBusyWakeLock.release(); } private void closeAllSaRecords(boolean expectSaClosed) { @@ -1065,7 +1077,9 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { @Override public void enterState() { - mScheduler.readyForNextProcedure(); + if (!mScheduler.readyForNextProcedure()) { + mBusyWakeLock.release(); + } if (mDpdIntent == null) { long remoteIkeSpi = mCurrentIkeSaRecord.getRemoteSpi(); @@ -1097,6 +1111,8 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { // #exitState is guaranteed to be invoked when quit() or quitNow() is called mAlarmManager.cancel(mDpdIntent); logd("DPD Alarm canceled"); + + mBusyWakeLock.acquire(); } @Override diff --git a/tests/iketests/AndroidManifest.xml b/tests/iketests/AndroidManifest.xml index 0abddbaa..bdcfdb71 100644 --- a/tests/iketests/AndroidManifest.xml +++ b/tests/iketests/AndroidManifest.xml @@ -22,6 +22,8 @@ + +