[Swan-dev] [Swan-commit] ikev2: allow Protocol ID IKE in Notify

[Antony Antony]

I think fix f9fada7234b is worth a closer look.

Yesterday, when Tuomo noticed this issue and when I fixed it, the issue 
appeared a simple bug.
On a closer look I think 14e07ddcf2f5 was very delibrate to block

"Notify with  Protocol ID IKE(1)"

Now we know that out in the wild Cisco would send Protocol ID IKE(1).
Commits f9fada7234b and 83ea2c2f2 allow this.
I think we should accept it, be more relaxed in what we accept.

I am wondering why we decided to block Protocol ID IKE(1).
Andrew would you please share your thoughts?

Extra details:
pluto send with "Notify with Protocol (0)".
Wireshark packet disector call 
isakmp.notify.protoid Protocol ID: RESERVED (0)

Pluto call it IKEv2_SEC_PROTO_NONE and according to the comment it is also 
delibrate decision to deviate from IANA registry

 * The value '0' is a little odd.  While IANA lists it as Reserved, a
 * notify payload must use that value for notifications that do not
 * include an SPI.  Hence 'NONE' is used.

So far I noticed two comments hinting IKE should not be allowed. I found the 
second one this morning. I think there is one more in the ocde about syncing
Notify Protocol IDs between IKEv1 and IKEv2. I am going to fix the second 
one. 

-extern enum_names ikev2_notify_protocol_id_names;      /* NONE=0, 2=AH, 3=ESP; NOT IKE
+extern enum_names ikev2_notify_protocol_id_names;      /* NONE=0, 1=IKE, 2=AH, 3=ESP;

Before I commit this fix, I have a question.
Should we re-visit my fixes or even revert them?

iPhone send Protocol ID: RESERVED (0). So far Cisco is the only outliever we 
know of.

regards,
-antony

On Fri, Oct 16, 2020 at 02:36:20PM +0000, Antony Antony wrote:
> New commits:
> commit f9fada7234b69d069d00d22163229bfe071ef70e
> Author: Antony Antony <antony at phenome.org>
> Date:   Fri Oct 16 14:21:43 2020 +0200
> 
>     ikev2: allow Protocol ID IKE in Notify
>     
>     Cisco send Protocol ID IKE(1) in notifications in IKEv2 IKE_INIT.
>     
>     Commit 14e07ddcf2f5 would not allow "1" and drop the message.
>     
>     RFC7296 #section-3.10 is a bit vauge.
>     "If the SPI field is empty, this field MUST be sent as zero and
>      MUST be ignored on receipt."
>     
>     In this case SPI is empty. May be Cisco should not send "1"?
>     For now lets allow "1".
>     
>     Pluto log message
>     
>     | ***parse IKEv2 Vendor ID Payload:
>     |    next payload type: ISAKMP_NEXT_v2N (0x29)
>     
>     | Now let's proceed with payload (ISAKMP_NEXT_v2N)
>     Protocol ID of IKEv2 Notify Payload has an unknown value: 1 (0x1)
>     "westnet-eastnet-ipv4-psk-ikev2" #1: malformed payload in packet
>     | processing: STOP state #0 (in process_md() at demux.c:275)
>     
>     Wireshark disect.
>     
>        Payload: Vendor ID (43) : Cisco Copyright
>             Next payload: Notify (41)
>     
>         Payload: Notify (41) - NAT_DETECTION_SOURCE_IP
>             Next payload: Notify (41)
>             0... .... = Critical Bit: Not Critical
>             .000 0000 = Reserved: 0x00
>             Payload length: 28
>             Protocol ID: IKE (1)
>     
>     ->>> above is what pluto refused
>     
>             SPI Size: 0
>             Notify Message Type: NAT_DETECTION_SOURCE_IP (16388)
>             Notification DATA: 158a0910b35701b6831b2f3ff90993cd57b993ff
>         Payload: Notify (41) - NAT_DETECTION_DESTINATION_IP
>     
>     Fixes: 14e07ddcf2f5 ("constants: organize Security Protocol ID name tables inline with IETF")
>