Merge "Don't throw missing payload exception if Auth is rejected with error notify"

2 files changed, 96 insertions, 51 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 3429c8a0..2955b038 100644
--- a/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java
+++ b/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java
@@ -35,9 +35,14 @@ import static com.android.internal.net.ipsec.ike.message.IkeNotifyPayload.NOTIFY
 import static com.android.internal.net.ipsec.ike.message.IkeNotifyPayload.NOTIFY_TYPE_NAT_DETECTION_DESTINATION_IP;
 import static com.android.internal.net.ipsec.ike.message.IkeNotifyPayload.NOTIFY_TYPE_NAT_DETECTION_SOURCE_IP;
 import static com.android.internal.net.ipsec.ike.message.IkeNotifyPayload.NOTIFY_TYPE_REKEY_SA;
+import static com.android.internal.net.ipsec.ike.message.IkePayload.PAYLOAD_TYPE_AUTH;
 import static com.android.internal.net.ipsec.ike.message.IkePayload.PAYLOAD_TYPE_CP;
 import static com.android.internal.net.ipsec.ike.message.IkePayload.PAYLOAD_TYPE_DELETE;
+import static com.android.internal.net.ipsec.ike.message.IkePayload.PAYLOAD_TYPE_EAP;
 import static com.android.internal.net.ipsec.ike.message.IkePayload.PAYLOAD_TYPE_NOTIFY;
+import static com.android.internal.net.ipsec.ike.message.IkePayload.PAYLOAD_TYPE_SA;
+import static com.android.internal.net.ipsec.ike.message.IkePayload.PAYLOAD_TYPE_TS_INITIATOR;
+import static com.android.internal.net.ipsec.ike.message.IkePayload.PAYLOAD_TYPE_TS_RESPONDER;
 import static com.android.internal.net.ipsec.ike.message.IkePayload.PAYLOAD_TYPE_VENDOR;
 import static com.android.internal.net.ipsec.ike.utils.IkeAlarmReceiver.ACTION_DELETE_CHILD;
 import static com.android.internal.net.ipsec.ike.utils.IkeAlarmReceiver.ACTION_DELETE_IKE;
@@ -3208,12 +3213,11 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine {
                             "Expected EXCHANGE_TYPE_IKE_AUTH but received: " + exchangeType);
                 }
 
+                validateIkeAuthResp(ikeMessage);
+
                 List<IkePayload> childReqList =
                         extractChildPayloadsFromMessage(mRetransmitter.getMessage());
-
                 if (mUseEap) {
-                    validateIkeAuthRespWithEapPayload(ikeMessage);
-
                     // childReqList needed after EAP completed, so persist to IkeSessionStateMachine
                     // state.
                     mFirstChildReqList = childReqList;
@@ -3221,12 +3225,12 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine {
                     IkeEapPayload ikeEapPayload =
                             ikeMessage.getPayloadForType(
                                     IkePayload.PAYLOAD_TYPE_EAP, IkeEapPayload.class);
-
+                    if (ikeEapPayload == null) {
+                        throw new AuthenticationFailedException("Missing EAP payload");
+                    }
                     deferMessage(obtainMessage(CMD_EAP_START_EAP_AUTH, ikeEapPayload));
                     transitionTo(mCreateIkeLocalIkeAuthInEap);
                 } else {
-                    validateIkeAuthRespWithChildPayloads(ikeMessage);
-
                     notifyIkeSessionSetup(ikeMessage);
 
                     performFirstChildNegotiation(
@@ -3339,40 +3343,12 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine {
             return buildIkeAuthReqMessage(payloadList);
         }
 
-        private void validateIkeAuthRespWithEapPayload(IkeMessage respMsg)
-                throws IkeProtocolException {
-            IkeEapPayload ikeEapPayload =
-                    respMsg.getPayloadForType(IkePayload.PAYLOAD_TYPE_EAP, IkeEapPayload.class);
-            if (ikeEapPayload == null) {
-                throw new AuthenticationFailedException("Missing EAP payload");
-            }
-
-            // TODO: check that we don't receive any ChildSaRespPayloads here
-
-            List<IkePayload> nonEapPayloads = new LinkedList<>();
-            nonEapPayloads.addAll(respMsg.ikePayloadList);
-            nonEapPayloads.remove(ikeEapPayload);
-            validateIkeAuthResp(nonEapPayloads);
-        }
-
-        private void validateIkeAuthRespWithChildPayloads(IkeMessage respMsg)
-                throws IkeProtocolException {
-            // Extract and validate existence of payloads for first Child SA setup.
-            List<IkePayload> childSaRespPayloads = extractChildPayloadsFromMessage(respMsg);
-
-            List<IkePayload> nonChildPayloads = new LinkedList<>();
-            nonChildPayloads.addAll(respMsg.ikePayloadList);
-            nonChildPayloads.removeAll(childSaRespPayloads);
-
-            validateIkeAuthResp(nonChildPayloads);
-        }
-
-        private void validateIkeAuthResp(List<IkePayload> payloadList) throws IkeProtocolException {
+        private void validateIkeAuthResp(IkeMessage authResp) throws IkeProtocolException {
             // Validate IKE Authentication
             IkeAuthPayload authPayload = null;
             List<IkeCertPayload> certPayloads = new LinkedList<>();
 
-            for (IkePayload payload : payloadList) {
+            for (IkePayload payload : authResp.ikePayloadList) {
                 switch (payload.payloadType) {
                     case IkePayload.PAYLOAD_TYPE_ID_RESPONDER:
                         mRespIdPayload = (IkeIdPayload) payload;
@@ -3394,7 +3370,18 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine {
                     case IkePayload.PAYLOAD_TYPE_NOTIFY:
                         IkeNotifyPayload notifyPayload = (IkeNotifyPayload) payload;
                         if (notifyPayload.isErrorNotify()) {
-                            throw notifyPayload.validateAndBuildIkeException();
+                            if (notifyPayload.isNewChildSaNotify()
+                                    && authResp.getPayloadForType(
+                                                    PAYLOAD_TYPE_AUTH, IkeAuthPayload.class)
+                                            != null) {
+                                // If error is for creating Child and Auth payload is included, try
+                                // to do authentication first and let ChildSessionStateMachine
+                                // handle the error later.
+                                continue;
+                            } else {
+                                throw notifyPayload.validateAndBuildIkeException();
+                            }
+
                         } else {
                             // Unknown and unexpected status notifications are ignored as per
                             // RFC7296.
@@ -3404,6 +3391,12 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine {
                                             + notifyPayload.notifyType);
                         }
                         break;
+                    case PAYLOAD_TYPE_SA: // Will be handled separately; fall through
+                    case PAYLOAD_TYPE_CP: // Will be handled separately; fall through
+                    case PAYLOAD_TYPE_TS_INITIATOR: // Will be handled separately; fall through
+                    case PAYLOAD_TYPE_TS_RESPONDER: // Will be handled separately; fall through
+                    case PAYLOAD_TYPE_EAP: // Will be handled separately
+                        break;
                     default:
                         logw(
                                 "Received unexpected payload in IKE AUTH response. Payload"
@@ -3689,18 +3682,11 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine {
                             "Expected EXCHANGE_TYPE_IKE_AUTH but received: " + exchangeType);
                 }
 
-                // Extract and validate existence of payloads for first Child SA setup.
-                List<IkePayload> childSaRespPayloads = extractChildPayloadsFromMessage(ikeMessage);
-
-                List<IkePayload> nonChildPayloads = new LinkedList<>();
-                nonChildPayloads.addAll(ikeMessage.ikePayloadList);
-                nonChildPayloads.removeAll(childSaRespPayloads);
-
-                validateIkeAuthRespPostEap(nonChildPayloads);
-
+                validateIkeAuthRespPostEap(ikeMessage);
                 notifyIkeSessionSetup(ikeMessage);
 
-                performFirstChildNegotiation(mFirstChildReqList, childSaRespPayloads);
+                performFirstChildNegotiation(
+                        mFirstChildReqList, extractChildPayloadsFromMessage(ikeMessage));
             } catch (IkeProtocolException e) {
                 // Notify the remote because they may have set up the IKE SA.
                 sendEncryptedIkeMessage(buildIkeDeleteReq(mCurrentIkeSaRecord));
@@ -3717,11 +3703,10 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine {
             handleIkeFatalError(ikeException);
         }
 
-        private void validateIkeAuthRespPostEap(List<IkePayload> payloadList)
-                throws IkeProtocolException {
+        private void validateIkeAuthRespPostEap(IkeMessage authResp) throws IkeProtocolException {
             IkeAuthPayload authPayload = null;
 
-            for (IkePayload payload : payloadList) {
+            for (IkePayload payload : authResp.ikePayloadList) {
                 switch (payload.payloadType) {
                     case IkePayload.PAYLOAD_TYPE_AUTH:
                         authPayload = (IkeAuthPayload) payload;
@@ -3729,7 +3714,18 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine {
                     case IkePayload.PAYLOAD_TYPE_NOTIFY:
                         IkeNotifyPayload notifyPayload = (IkeNotifyPayload) payload;
                         if (notifyPayload.isErrorNotify()) {
-                            throw notifyPayload.validateAndBuildIkeException();
+                            if (notifyPayload.isNewChildSaNotify()
+                                    && authResp.getPayloadForType(
+                                                    PAYLOAD_TYPE_AUTH, IkeAuthPayload.class)
+                                            != null) {
+                                // If error is for creating Child and Auth payload is included, try
+                                // to do authentication first and let ChildSessionStateMachine
+                                // handle the error later.
+                                continue;
+                            } else {
+                                throw notifyPayload.validateAndBuildIkeException();
+                            }
+
                         } else {
                             // Unknown and unexpected status notifications are ignored as per
                             // RFC7296.
@@ -3739,6 +3735,11 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine {
                                             + notifyPayload.notifyType);
                         }
                         break;
+                    case PAYLOAD_TYPE_SA: // Will be handled separately; fall through
+                    case PAYLOAD_TYPE_CP: // Will be handled separately; fall through
+                    case PAYLOAD_TYPE_TS_INITIATOR: // Will be handled separately; fall through
+                    case PAYLOAD_TYPE_TS_RESPONDER: // Will be handled separately; fall through
+                        break;
                     default:
                         logw(
                                 "Received unexpected payload in IKE AUTH response. Payload"
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 66c2517c..6fe4aada 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
@@ -18,7 +18,9 @@ package com.android.internal.net.ipsec.ike;
 
 import static android.net.ipsec.ike.IkeSessionConfiguration.EXTENSION_TYPE_FRAGMENTATION;
 import static android.net.ipsec.ike.IkeSessionParams.IKE_OPTION_EAP_ONLY_AUTH;
+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_INTERNAL_ADDRESS_FAILURE;
 import static android.net.ipsec.ike.exceptions.IkeProtocolException.ERROR_TYPE_INVALID_SYNTAX;
 import static android.net.ipsec.ike.exceptions.IkeProtocolException.ERROR_TYPE_NO_ADDITIONAL_SAS;
 import static android.net.ipsec.ike.exceptions.IkeProtocolException.ERROR_TYPE_NO_PROPOSAL_CHOSEN;
@@ -2528,6 +2530,48 @@ public final class IkeSessionStateMachineTest extends IkeSessionTestBase {
     }
 
     @Test
+    public void testAuthHandlesIkeErrorNotify() throws Exception {
+        mockIkeInitAndTransitionToIkeAuth(mIkeSessionStateMachine.mCreateIkeLocalIkeAuth);
+        verifyRetransmissionStarted();
+        resetMockIkeMessageHelper();
+
+        // Mock rejecting IKE AUTH with Authenticatio Failure notification
+        ReceivedIkePacket mockAuthFailPacket =
+                makeIkeAuthRespWithoutChildPayloads(
+                        Arrays.asList(new IkeNotifyPayload(ERROR_TYPE_AUTHENTICATION_FAILED)));
+        mIkeSessionStateMachine.sendMessage(CMD_RECEIVE_IKE_PACKET, mockAuthFailPacket);
+        mLooper.dispatchAll();
+
+        // Verify IKE Session is closed properly
+        assertNull(mIkeSessionStateMachine.getCurrentState());
+        verify(mMockIkeSessionCallback)
+                .onClosedExceptionally(any(AuthenticationFailedException.class));
+    }
+
+    @Test
+    public void testAuthHandlesCreateChildErrorNotify() throws Exception {
+        mockIkeInitAndTransitionToIkeAuth(mIkeSessionStateMachine.mCreateIkeLocalIkeAuth);
+        verifyRetransmissionStarted();
+        resetMockIkeMessageHelper();
+
+        // Mock rejecting IKE AUTH with a Create Child error notification
+        ReceivedIkePacket mockAuthFailPacket =
+                makeIkeAuthRespWithoutChildPayloads(
+                        Arrays.asList(new IkeNotifyPayload(ERROR_TYPE_INTERNAL_ADDRESS_FAILURE)));
+        mIkeSessionStateMachine.sendMessage(CMD_RECEIVE_IKE_PACKET, mockAuthFailPacket);
+        mLooper.dispatchAll();
+
+        // Verify IKE Session is closed properly
+        assertNull(mIkeSessionStateMachine.getCurrentState());
+
+        ArgumentCaptor<IkeProtocolException> captor =
+                ArgumentCaptor.forClass(IkeProtocolException.class);
+        verify(mMockIkeSessionCallback).onClosedExceptionally(captor.capture());
+        IkeProtocolException exception = captor.getValue();
+        assertEquals(ERROR_TYPE_INTERNAL_ADDRESS_FAILURE, exception.getErrorType());
+    }
+
+    @Test
     public void testAuthPskHandleRespWithParsingError() throws Exception {
         mockIkeInitAndTransitionToIkeAuth(mIkeSessionStateMachine.mCreateIkeLocalIkeAuth);
         verifyRetransmissionStarted();