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

Resolve the bug that when receiving error notify and no auth payload
in IKE Auth, IKE library would still look for payloads for
authentication and will throw exception saying missing payload if
not found. This commit make IKE library throw the received error to
user in this case

Bug: 148287674
Test: atest FrameworksIkeTests(new tests added)
Test: testIkeAuthHandlesAuthFailNotification
      testIkeAuthHandlesFirstChildCreationFail
Change-Id: I7fb20fee8fdf649ccbbaef838f87b2d7cec70bc1

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 ea496f81..85d61291 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;
@@ -3204,12 +3209,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;
@@ -3217,12 +3221,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(
@@ -3335,40 +3339,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;
@@ -3390,7 +3366,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.
@@ -3400,6 +3387,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"
@@ -3685,18 +3678,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));
@@ -3713,11 +3699,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;
@@ -3725,7 +3710,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.
@@ -3735,6 +3731,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();