Thanks again for the careful review. All changes made other than those listed below.
--Paul HOffman At 11:06 PM +0200 1/24/10, Yaron Sheffer wrote: >2.21.: EAP Failure cases are missing altogether. Also, the first paragraph >says that if an auth failure occurs at the responder, AUTHENTICATION_FAILED is >included in the protected response (to IKE_AUTH), while the last paragraph >says it's a separate Informational exchange. Please open a tracker issue for this. >2.23: "The recipient of either the NAT_DETECTION_SOURCE_IP or >NAT_DETECTION_DESTINATION_IP notification MAY compare the supplied value to a >SHA-1 hash of the SPIs, source IP address, and port, and if they don't match >it SHOULD enable NAT traversal. In the case there is a mismatch of the >NAT_DETECTION_SOURCE_IP hash with all of the NAT_DETECTION_SOURCE_IP payloads >received, the recipient MAY reject the connection attempt if NAT traversal is >not supported." In the first sentence, the comparison is not just with the >source address/port. Also, while NAT-T is a MAY, once you decide that you >implement NAT-T, this comparison becomes at least a SHOULD. "If they don't >match it SHOULD enable NAT-T" contradicts an earlier MUST: "if a NAT is >detected, both devices MUST send UDP encapsulated packets". And the second >sentence doesn't make sense as pointed out before - if you recognize these >payloads, then obviously you support NAT-T. Overall, the normative part of >this section needs to b! e reworked. Please open a tracker issue for this. >2.23: replace the second-last bullet with a pointer to the subsequent >subsection, on Transport Mode and NAT. Added a forward reference instead. >2.23: the last bullet (automatically updating the peer's IP address) is too >important to be only mentioned as a side note. We should at least mention it >in Sec. 2.6, following the sentence: "Incoming IKE packets are mapped to an >IKE SA only using the packet's SPI, not using (for example) the source IP >address of the packet." Please propose specific wording for that insertion to 2.6. >2.24: what's this section (ECN) doing here? It is 100% IPsec, nothing to do >with IKE. If there's a reason, let's explain it in the text. Disagree. This section lists new mandatory actions that were not in IKEv1. >3.2, "Next Payload": we say that the Encrypted payload gets a different Next >Payload value. But then, we should also say (here or where describing the >Encrypted payload) that the last *contained* payload gets a zero Next Payload >value. This is significant. Please propose changes in a new tracker issue. >3.3.5: "Attributes described as fixed length MUST NOT be encoded using the >variable-length encoding." This cannot be correct, a new 4-octet attribute >will have to be encoded as variable-length. It won't fit into the other format. Please open a tracker issue for this. >3.6: "DNS Signed Key" is marked Unspecified. Is it not RFC 4025? It's not used >anywhere, but still... RFC 4025 only specifies how to format those keys in the DNS and in display, not in IKE. You can guess that you should use the RDATA format, but that's only a guess. >3.6: [Hash and URL] "makes IKE less subject to denial of service attacks that >become easier to mount when IKE messages are large enough to require IP >fragmentation." This statement is hard to take seriously. When the certificate >is NOT cached, this encoding eliminates a difficult-to-perform DOS attack, >replacing it with a much easier one: stop the endpoint from obtaining the cert. See the IPsec WG archives. If you really think this subjective rathole is worth opening again, open a tracker issue. >3.10.1: INVALID_SELECTORS is underspecified. It should be rate limited (I >suppose), also, how long is the packet fragment included in the notification? >In addition, Sec. 2.21.2 implies that it is sent during Child SA negotiation, >which is not what 3.10.1 is saying. Please open a tracker issue for this. >3.15.1: "With IPv6, a requestor MAY supply the low-order address octets it >wants to use." This means to me that you *don't* provide all 16 octets. But >according to the table, the field is a fixed-length 16 octets! Please open a tracker issue for this. >3.16: the text here, "...this field MAY be set to any value" implies that the >Identifier field can be constant, e.g. always zero. But this would contradict >RFC 3748, that says: > >In order to avoid confusion between new Requests and retransmissions, the >Identifier value chosen for each new Request need only be different from the >previous Request, but need not be unique within the conversation. One way to >achieve this is to start the Identifier at an initial value and increment it >for each new Request. Initializing the first Identifier with a random number >rather than starting from zero is recommended, since it makes sequence attacks >somewhat more difficult. Please open a tracker issue for this. >5: what do we mean by "leaves all SAs vulnerable to ... overrun of either >endpoint"? Please open a tracker issue for this. >5: unfortunately we have to revise the text about group strengths - or remove >it altogether. It is non-normative, so it's probably best to eliminate it. Why? >B.1 (Group 1): We may want to add: "Use of this group is NOT RECOMMENDED." Please open a tracker issue for this. Even though this is obvious, it is a tad late to be suggesting new normative language. >C: I suggest to repeat at the top of this appendix this text from 2.5: >"Although new payload types may be added in the future and may appear >interleaved with the fields defined in this specification, implementations >SHOULD send the payloads defined in this specification in the order shown in >the figures in Sections 1 and 2; implementations MUST NOT reject as invalid a >message with those payloads in any other order." If the appendix is as "purely informative" as it says, then putting normative language in it (even as duplication) is inappropriate. _______________________________________________ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec
