Fix NullPointerException in handling Cookie payload
When IKE library receives a Cookie payload in an IKE
INIT response, it should retry sending IKE INIT
request with the same SPI, and expect receiving
another IKE INIT response that containing payloads
for SA setup.
It also means that after IKE library has received
the Cookie payload, it should maintain the
allocated SPI resources, because the SPI resources
will be used in SA setup later.
The cause of NullPointerException is IKE library
closed and nulled out the SPI resources after it
received a Cookie payload. And then when IKE library
received the second IKE INIT response, it tried to
use the null-value SPI resources to set up the SA
by calling IkeSaRecord.makeFirstIkeSaRecord.
Bug: 179776241
Test: FrameworksIkeTests (new tests added)
Change-Id: Ia49b65051ca2576f74218b5e1a9ee3834b22a2ff
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 132c519..15ad260 100644
--- a/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java
+++ b/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java
@@ -3042,7 +3042,13 @@
@Override
protected void handleResponseIkeMessage(IkeMessage ikeMessage) {
+ // IKE_SA_INIT exchange and IKE SA setup succeed
boolean ikeInitSuccess = false;
+
+ // IKE INIT is not finished. IKE_SA_INIT request was re-sent with Notify-Cookie,
+ // and the same INIT SPI and other payloads.
+ boolean ikeInitRetriedWithCookie = false;
+
try {
int exchangeType = ikeMessage.ikeHeader.exchangeType;
if (exchangeType != IkeHeader.EXCHANGE_TYPE_IKE_SA_INIT) {
@@ -3059,6 +3065,7 @@
buildReqWithCookie(mRetransmitter.getMessage(), outCookiePayload);
sendRequest(initReq);
+ ikeInitRetriedWithCookie = true;
return;
}
@@ -3111,7 +3118,7 @@
handleIkeFatalError(e);
} finally {
- if (!ikeInitSuccess) {
+ if (!ikeInitSuccess && !ikeInitRetriedWithCookie) {
if (mLocalIkeSpiResource != null) {
mLocalIkeSpiResource.close();
mLocalIkeSpiResource = null;
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 268b589..9c3a8e8 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
@@ -217,9 +217,11 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Random;
+import java.util.Set;
import java.util.concurrent.Executor;
public final class IkeSessionStateMachineTest extends IkeSessionTestBase {
@@ -1478,6 +1480,13 @@
SaProposal.DH_GROUP_2048_BIT_MODP, mIkeSessionStateMachine.mPeerSelectedDhGroup);
}
+ private ReceivedIkePacket getIkeInitRespWithCookie() throws Exception {
+ IkeNotifyPayload inCookieNotify = new IkeNotifyPayload(NOTIFY_TYPE_COOKIE, COOKIE_DATA);
+ List<IkePayload> payloads = new ArrayList<>();
+ payloads.add(inCookieNotify);
+ return makeDummyReceivedIkeInitRespPacket(payloads);
+ }
+
@Test
public void testCreateIkeLocalIkeInitReceivesCookie() throws Exception {
setupFirstIkeSa();
@@ -1494,10 +1503,7 @@
resetMockIkeMessageHelper();
// Send back a Notify-Cookie
- IkeNotifyPayload inCookieNotify = new IkeNotifyPayload(NOTIFY_TYPE_COOKIE, COOKIE_DATA);
- List<IkePayload> payloads = new ArrayList<>();
- payloads.add(inCookieNotify);
- ReceivedIkePacket resp = makeDummyReceivedIkeInitRespPacket(payloads);
+ ReceivedIkePacket resp = getIkeInitRespWithCookie();
mIkeSessionStateMachine.sendMessage(IkeSessionStateMachine.CMD_RECEIVE_IKE_PACKET, resp);
mLooper.dispatchAll();
@@ -1518,6 +1524,28 @@
}
@Test
+ public void testCreateIkeLocalIkeInitRcvRespAfterRcvCookie() throws Exception {
+ setupFirstIkeSa();
+
+ mIkeSessionStateMachine.sendMessage(IkeSessionStateMachine.CMD_LOCAL_REQUEST_CREATE_IKE);
+ mLooper.dispatchAll();
+
+ // Receive IKE INIT response with Cookie
+ mIkeSessionStateMachine.sendMessage(
+ IkeSessionStateMachine.CMD_RECEIVE_IKE_PACKET, getIkeInitRespWithCookie());
+
+ // Receive IKE INIT response
+ mIkeSessionStateMachine.sendMessage(
+ IkeSessionStateMachine.CMD_RECEIVE_IKE_PACKET, makeIkeInitResponse());
+ mLooper.dispatchAll();
+
+ assertTrue(
+ mIkeSessionStateMachine.getCurrentState()
+ instanceof IkeSessionStateMachine.CreateIkeLocalIkeAuth);
+ verifyIkeSaNegotiationResult();
+ }
+
+ @Test
public void testCreateIkeLocalIkeInitSwitchesToEncapPorts() throws Exception {
setupFirstIkeSa();
mIkeSessionStateMachine.sendMessage(IkeSessionStateMachine.CMD_LOCAL_REQUEST_CREATE_IKE);
@@ -1603,7 +1631,10 @@
mIkeSessionStateMachine.getCurrentState()
instanceof IkeSessionStateMachine.CreateIkeLocalIkeAuth);
verifyRetransmissionStarted();
+ verifyIkeSaNegotiationResult();
+ }
+ private void verifyIkeSaNegotiationResult() throws Exception {
// Validate negotiated SA proposal.
IkeSaProposal negotiatedProposal = mIkeSessionStateMachine.mSaProposal;
assertNotNull(negotiatedProposal);
@@ -1631,7 +1662,7 @@
// Validate NAT detection
assertTrue(mIkeSessionStateMachine.mLocalNatDetected);
- assertFalse(mIkeSessionStateMachine.mRemoteNatDetected);
+ assertTrue(mIkeSessionStateMachine.mRemoteNatDetected);
assertTrue(mIkeSessionStateMachine.mSupportNatTraversal);
// Validate vendor IDs
@@ -1646,9 +1677,11 @@
mIkeSessionStateMachine.mEnabledExtensions);
// Validate Signature Hash Algorithms received in IKE INIT response
- assertEquals(
- Arrays.asList(IkeAuthDigitalSignPayload.ALL_SIGNATURE_ALGO_TYPES),
- mIkeSessionStateMachine.mPeerSignatureHashAlgorithms);
+ Set<Short> expectedHashAlgos = new HashSet<Short>();
+ for (short algo : IkeAuthDigitalSignPayload.ALL_SIGNATURE_ALGO_TYPES) {
+ expectedHashAlgos.add(algo);
+ }
+ assertEquals(expectedHashAlgos, mIkeSessionStateMachine.mPeerSignatureHashAlgorithms);
}
private void setIkeInitResults() throws Exception {
