I only commented to those cases where I disagreed with you (mostly I think we do not need to make the change). The other comments were ok for me.
Yaron Sheffer writes: > 1.3.2: for some reason we support negotiation of DH parameters when > rekeying the IKE SA. So we should say that the INVALID_KE_PAYLOAD > response is possible here, too. I think generic text in section 1.3 should be enough for it: If a CREATE_CHILD_SA exchange includes a KEi payload, at least one of the SA offers MUST include the Diffie-Hellman group of the KEi. The Diffie-Hellman group of the KEi MUST be an element of the group the initiator expects the responder to accept (additional Diffie-Hellman groups can be proposed). If the responder selects a proposal using a different Diffie-Hellman group (other than NONE), the responder MUST reject the request and indicate its preferred Diffie-Hellman group in the INVALID_KE_PAYLOAD Notification payload. There are two octets of data associated with this notification: the accepted D-H Group number in big endian order. In the case of such a rejection, the CREATE_CHILD_SA exchange fails, and the initiator will probably retry the exchange with a Diffie-Hellman proposal and KEi in the group that the responder gave in the INVALID_KE_PAYLOAD. and this covers all subsections of 1.3 including 1.3.2, so I think that is enough, and no additional text is needed. > 1.4: "The processing of an INFORMATIONAL exchange is determined by > its component payloads." Adding "The payloads are processed in > strict left-to-right order" would make it deterministic, and is > probably what people do today. There should be no need to make the processing deterministic, as at least I cannot think of case where the processing order would matter, and I do not want someone to start complaining if someone process them in "wrong" order. I do not think the addition should be done. > 2.1: either here or in 1.5 we should say something about allowing > limited retransmission of the rare one-way IKE messages, for > reliability. I have always assumed that you do not retransmit those, you simply rate limit sending of them, and if other end sends further packets which trigger sending new one-way IKE messages, then you will send next one-way IKE message. I.e. the retransmission is done automatically by the other end either retransmitting its message (IKE packets) or other end sending more ESP packets. If we add text, I would rather add text saying you MUST NOT retransmit those one-way IKE messages as they are one-way messages. You may send new (identical) one-way IKE messages in case the same situation is triggered again (rate limited) because other end sent another packet. This would be consistent that attacker sending only one IKE or ESP packet can cause only one one-way IKE message to be sent. If it wants to peer to send another, it again needs to send another packet (i.e. no amplification provided for attacker). > 2.3: do we allow to send SET_WINDOW_SIZE in the initial exchange? Yes. > We don't say we don't - although we do say that it would only be > effective starting with IKE_AUTH. But I believe we should not accept > it until both peers are fully authenticated. Regardless of the SET_WINDOW_SIZE the window size is always one until the initial exchanges completes and initial exchanges include IKE_INIT_SA and IKE_AUTH, thus after those have completed the parties have been authenticated, and as you cannot have any other exchanges while you have initial exchanges still in progess it does not really matter. I do not think this requires new text (except perhaps to clarify that SET_WINDOW_SIZE can be in the initial exchange (usually in IKE_AUTH)). We do send it in the IKE_AUTH (the same one having SA and TS payloads if multiple IKE_AUTH messages), in the IKE SA rekey, and in the next exchange (regardless of type) if it is changed on the fly. > 2.4: this sentence is inconsistent: "The INITIAL_CONTACT > notification, if sent, MUST be in the first IKE_AUTH request or > response, not as a separate exchange afterwards; however, > receiving parties MAY ignore it in other messages." If it MUST be > only at a certain location, then it should be ignored (or even > rejected) in others. No. The first one says where you MUST send it, the second part says what you do if someone does not follow that MUST, which tells you MAY ignore it. There is no inconsistancy there. The text is written like this as this was changed between RFC4306 and draft-ietf-ipsecme-ikev2. The RFC4306 does not specify the location, but IKEv2bis does, and because of this old implementations might send INITIAL_CONTACT notifications in wrong place, and the text says that you are free to ignore it in those places. Support for INITIAL_CONTACT is MAY anyways (both sending and processing it on receive). So no need to change anything there. > 2.8: this sentence "It should do so if there has been no traffic > since the last time the SA was rekeyed" is useless: it talks about > local policy for deletion, which we are not specifying. The policy > "delete the SA if there's been no traffic for the last 5 minutes" > would be just as valid as this one. Yes, but the local policy given in the IKEv2bis is very common and useful one. I do not think the text is useless, and as it does not mandate implementations doing anything, it just says what they might want to do, I do not see any reason to remove it. > 2.8: the sentence beginning "From a technical correctness and > interoperability perspective" is not strictly correct: there is > still a race condition between the first packets on the SA and the > CCSA response. Immediately starting to send is practical but not > "technically correct". For the protocol point of view it is techinally correct. I.e. after responder has sent the response to the CREATE_CHILD_SA, for its point of view the SA is ready and it can be used immediately. There is race condition built in to the protocol, which is why the text is there, i.e. it says it is techinally correct, but may still cause packets to be dropped. I do not see any need to change this text. > 2.8.2: we should add a sentence on what happens if the peer receives > TEMPORARY_FAILURE and does not understand it (because it's new > to this spec). There is text for that alread which says (section 3.10.1): Types in the range 0 - 16383 are intended for reporting errors. An implementation receiving a Notify payload with one of these types that it does not recognize in a response MUST assume that the corresponding request has failed entirely. So I do not see need to change the the 2.8.2. If change is needed perhaps adding pointer to the 3.10.1, but I think even that is not needed). -- kivi...@iki.fi _______________________________________________ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec
