Re: [Gen-art] Genart last call review of draft-ietf-ipsecme-ikev2-auth-announce-06

[Valery Smyslov <[e-mail]>]

Hi Reese,

thank you for your review. Please see inline.

> Reviewer: Reese Enghardt
> Review result: Ready with Nits
> 
> I am the assigned Gen-ART reviewer for this draft. The General Area Review
> Team (Gen-ART) reviews all IETF documents being processed by the IESG for the
> IETF Chair.  Please treat these comments just like any other last call comments.
> 
> For more information, please see the FAQ at
> 
> <https://wiki.ietf.org/en/group/gen/GenArtFAQ>.
> 
> Document: draft-ietf-ipsecme-ikev2-auth-announce-06
> Reviewer: Reese Enghardt
> Review Date: 2024-03-29
> IETF LC End Date: 2024-03-31
> IESG Telechat date: Not scheduled for a telechat
> 
> Summary: The document is clear and concise, and it is ready for publication. I
> have a few minor suggestion to make the document more comprehensible to non-
> expert readers.
> 
> Major issues: None.
> 
> Minor issues:
> 
> Abstract:
> 
> As a non-expert reader, the following phrases appear to contradict each other:
> "indicate the list of supported authentication methods" sounds like different
> algorithms, "configured with multiple different credentials  to authenticate each
> other" sounds like different certificates but potentially using the same algorithm. If
> this proposal is most applicable to scenarios where IKEv2 partners use different
> algorithms or methods, please consider saying so explicitly in the abstract.

I'm a bit confused here. My interpretation is that "different credentials" includes the situation 
when these credentials can only be used with specific authentication methods
(e.g., user may have a certificate and a PSK). So, I believe that the current text
is adequate, but as a non-native speaker I might have missed some nuances.
If this is the case, then can you propose a better wording?

> Section 1:
> 
> If there are several credentials, why not just try them one by one - is it just
> performance reasons (key exchange takes longer), or is there another
> reason? 

This would be a major change in the protocol, which doesn't seem appropriate.
And yes, performance also matters.

> Please consider adding a sentence as additional motivation for this new
> mechanism.

I've added the following sentence to the Introduction:

   Since IKEv2 doesn't allow to use multiple
   authentication methods and doesn't provide means for peers to
   indicate to the other side which authentication methods they support,
   it is possible that in these situations the peer which supports wider
   range of authentication methods (or authentication token formats)
   improperly selects the method (or format) which is not supported by
   the other side.

> Section 3.1:
> 
> "it includes a new status type notify SUPPORTED_AUTH_METHODS in the
> IKE_SA_INIT response message" Have a hard time parsing this sentence. Is
> "notify" part of the name of the new status type? Please consider rephrasing this
> sentence.

I've changed this to:

"it includes a new notification SUPPORTED_AUTH_METHODS in the IKE_SA_INIT response message".

Actually, the fact that the type of this notification is "status" (and not "error")
is pointed out in the IANA considerations section.

> "If the following conditions are met:"
> Either of the conditions or both of the conditions?

Changed to:

" If both of the following conditions are met:"

> "[…] then the responder may choose not to send actual list of the supported
> authentication methods in the IKE_SA_INIT exchange and instead ask the initiator
> to start the IKE_INTERMEDIATE exchange for the list to be sent in" Is this a MAY?
> Why is it not a MUST, as above it says "the responder […] must take care that the
> size of the response message wouldn't grow too much so that IP fragmentation
> takes place"?

I agree that s/may/MAY may make sense here (done). But is definitely not MUST,
since there are a number of considerations that should be taken into.
Even if both conditions are met, the responder might have known for sure
that no IP fragmentation would take place (e.g. TCP transport is used)
or it is not an issue (e.g. provider's network doesn't drop IP fragmented UDP datagrams).
In these cases there is no point to do an additional round trip.

> Why does using the IKE_INTERMEDIATE exchange fix the problem of IP
> fragmentation? Please consider adding a brief explanation here.

Changed the text to:

   [...] then the responder MAY choose not to send actual list of the
   supported authentication methods in the IKE_SA_INIT exchange and
   instead ask the initiator to start the IKE_INTERMEDIATE exchange for
   the list to be sent in.  This would allow to use IKE fragmentation
   [RFC7383] for long messages (which cannot be used in the IKE_SA_INIT
   exchange), thus avoiding IP fragmentation.

> Section 5:
> 
> "Note, that this is not a real attack, since NULL authentication should be allowed by
> local security policy." Why is it not a real attack then? If NULL authentication is
> allowed among other methods, surely downgrading to NULL authentication is still a
> problem? Or should the second sentence instead say "NULL authentication should
> NOT be allowed by local security policy"?

There is no negotiation of the authentication method to be used in IKEv2,
thus this is not a "downgrade". If your local policy allows peers to not authenticate
on their discretion, then it is your choice. If they use NULL authentication
in this case, they don't violate your policy, thus this is not an real attack.

> Nits/editorial comments:
> 
> Section 1:
> 
> "The problem may arise when there are several credentials of different type
> configured on one peer" "type" -> "types"?

Fixed.

Regards,
Valery.