Document: draft-ietf-ipsecme-ikev2bis-10.txt Reviewer: Elwyn Davies Review Date: 4 May 2010 IETF LC End Date: 18 March 2010 IESG Telechat date: 6 May 2010 Summary: When I reviewed this document at IETF Last call, I discovered that compared to previous documents, it contains no mention of mandatory to implement ciphersuites. In a discussion with Paul Hoffmann, I was pointed at various other RFCs that specify ciphersuites, and informed that the IESG and WG had approved the current situation. However it seems to me that somebody looking at the IKE documents would be likely to come to this document first and would find it difficult to locate the auxiliary documents without considerable effort (and risk of missing some). I continue to believe that f there is not to be a list of appropriate ciphersuites here, there needs to be some pointers to places to look. A number of my comments have been implemented, but there are quite a few that I have seen no response to amd have not been implemented. The list below represents the remaining comments. I have left in a number of comments regarding the cut off time (publication date of RFC 3406) for updating of registration tables. It strikes me that it would be relatively little work and quite helpful to bring these tables up to date. There are also a number of relatively minor points where items and processes are explicitly not fully specified. Thus could lead to annoying corners where implementations are totally interoperable (e.g., what is the complete set of 'terminators' forbidden in IP_FQDNs? What is an acceptable 'sort of' email address/NAI on EAP?) Finally there are a number of points where it is recommended that network traffic needs to be limited but no concrete guidelines are given. I think that some sort of suggestions for the parameters to be applied (e.g., time constants, number of repeats, backoff algorithm) would be desirable. Major issues: s3.3.4: The draft states that the list of mandatory to implement suites has been removed due to evolution going too fast. However there are effectively some mandatory to implement suites; they are listed in other documents. There should be a way of finding them so that users and implmenters can find them easily. Minor issues: s1: At para 6 in s1 the draft states: > > The first request/response of an IKE session (IKE_SA_INIT) negotiates > > security parameters for the IKE SA, sends nonces, and sends Diffie- > > Hellman values. As a (not really) naive reader, I asked myself 'Why does this text suddenly mention that we need to send nonces and Diffie-Hellman values?' Looking back at the text so far I realized that the text has not introduced the techniques that it is going to use to establish the authenticated IKE channel etc. It is assumed that readers know that as soon as D-H is mentioned we are talking public key cryptography. A paragraph explaining the underlying ideas would be useful. s1.2, last para: > > Note that IKE_AUTH messages do not contain KEi/KEr or Ni/Nr payloads. > > Thus, the SA payloads in the IKE_AUTH exchange cannot contain > > Transform Type 4 (Diffie-Hellman Group) with any value other than > > NONE. Implementations SHOULD omit the whole transform substructure > > instead of sending value NONE. Does 'cannot contain' conflict with the 'SHOULD'? I am unclear what an implementer would do if s/he chose to do otherwise than omit the transform structure in the light of the previous statement. s2.1, last sentence: > > The retransmission policy for one-way messages is somewhat different > > from that for regular messages. Because no acknowledgement is ever > > sent, there is no reason to gratuitously retransmit one-way messages. > > Given that all these messages are errors, it makes sense to send them > > only once per "offending" packet, and only retransmit if further > > offending packets are received. Still, it also makes sense to limit > > retransmissions of such error messages. Does this need to be more precise? Some explicit policy for restricting retransmissions? Possibly in s2.21.4. s2.3: Should there be some discussion of the interaction of rekeying of the IKE_SA and windows? Presumably a rekey message should not be actioned until all previous messages have been responded to. Likewise receiving a Message ID with a sequence number bigger than that in the rekey message should be very suspect! Should the INVALID_MESSAGE_ID notification be sent in this case (and before or after the rekey?) There might be some knock on into s2.8 where rekeying is discussed. And maybe into s2.25? s2.4, para 2: > > The INITIAL_CONTACT notification, if sent, MUST > > be in the first IKE_AUTH request or response, not as a separate > > exchange afterwards; receiving parties MAY ignore it in other > > messages. What should receiving parties do if they *do* receive it and *don't* ignore it? Since it 'MUST be sent in the first IKE_AUTH' receiving at any other time is a protocol error and should cause some response (like dropping the IKE_SA perhaps). s2.4, para 4: > > Note > > that this places requirements on the failure modes of an IKE > > endpoint. An implementation MUST NOT continue sending on any SA if > > some failure prevents it from receiving on all of the associated SAs. > > If Child SAs can fail independently from one another without the > > associated IKE SA being able to send a delete message, then they MUST > > be negotiated by separate IKE SAs. I am fairly certain that is impossible for any implementation to guarantee the MUST NOT here. How does it absolutely know that it isn't able to receive anything? Absence of activity might mean failure or idleness. I *think* this is not just talking about IKE SAs but includes the Child SAs. This appears to imply that the Child SAs all have to be usable bidirectionally and have keepalives on them? And that IKE should know about the keepalives. What is the second condition saying? Does this impose some constraints on the use of Child SA deletion.. like that the IKE SA must support Child SA deletion? or is it just saying that if the IKE SA has failed you can't leave some Child SAs active (as implied by the next para)? The more I think about this the more confused I become! s2.6, next to last para: > > The initiator should limit the number of cookie exchanges it > > tries before giving up. Does anything need to be said abut exponential back-off? ss1.7/2.8: The changes documented in s1.7 state: > > In 2.8, changed "Note that, when rekeying, the new Child SA MAY have > > different traffic selectors and algorithms than the old one" to "Note > > that, when rekeying, the new Child SA SHOULD NOT have different > > traffic selectors and algorithms than the old one". This refers to para 3 of s2.8. Is the SHOULD NOT just there to cope with existing implementations? This is a binary choice - either they are the same or they aren't. If MAY is no good, I can't see that allowing the opposite choice at all makes any sense. I would argue that this ought to be MUST NOT. s2.15, definition of signed octets (both cases): What 'Length' is intended? And how should it be represented as a string for concatenation? Are the definitions of Nonce[IR]Payload intended to show how Nonce[IR]Data are derived ? They are not otherwise used. Similarly xxxIDPayload? How many octets of what value make up 'RESERVED' (or is it just the literal text? s2.21.2, last sentence: How would the peer expect to demonstrate understanding of extended error notifications? Protocol version number? [A suggestion of using the Vendor ID payload has been added. I am not sure if this adequately deals with the situation.] s2.21.4: More requests to limit message rates without specific means. s2.23: Should there be a discussion of NAT64? s3.1, Major Version: This definition is not entirely consistent with the major version support discussion earlier in the document. It should probably talk about implementations that support this version of the specification or any subsequent version that uses s higher version number. Reference back to s2.5 would be useful. This applies to the Minor Version also. s3.3.5, Key Length attribute: > > o The Key Length attribute MUST NOT be used with transforms that use > > a fixed length key. This includes, e.g., ENCR_DES, ENCR_IDEA, and > > all the Type 2 (Pseudo-random function) and Type 3 (Integrity > > Algorithm) transforms specified in this document. It is > > recommended that future Type 2 or 3 transforms do not use this > > attribute. I think the recommended is a RECOMMENDED here. s3.5, Identification Field table: The class of 'terminators' is not fully defined. s3.5, IP_FQDN: The specification refers to IDNA - does IDNAbis have any impact here? s3.5, last para: > > Responder implementations should not attempt to > > verify that the contents actually conform to the exact syntax given > > in [MAILFORMAT], but instead should accept any reasonable-looking > > NAI. How do I verify conformance to this statement please? s3.12: > > Writers of Internet-Drafts who wish to extend this protocol MUST > > define a Vendor ID payload to announce the ability to implement the > > extension in the Internet-Draft. It is expected that Internet-Drafts > > that gain acceptance and are standardized will be given "magic > > numbers" out of the Future Use range by IANA, and the requirement to > > use a Vendor ID will go away. > > I don't understand this paragraph. I suspect I need to eat some magic mushrooms in order to be able to see the magic numbers. Can I recommend swapping over the payload identification numbers with the Delete payload so that Vendor ID is #42? But seriously, this doesn't seem to be the way we should go about standardizing an extension. s8.2 [AEAD]: I think the reference in s3.14 makes this normative. s8.2 [MAILFORMAT]: I think the statements in s3.5 make this normative. ============================================ Nits/editorial comments: General: The version of the draft in the repository is unpaginated. General: The notation used for concatenation ('|'), raising to the power ('^'), etc could be usefully defined early on in one place. s1.3, para 3: the first sentence is redundant - it was said in para 1. s1.3: The section would be clearer if the last para was moved to be immediately after para 4 (the last para describes how and why the MAY in para 4 occurs). It would also help to change the order of the sentences in the last para, making the last sentence first. s1.5, para 5: the last sentence > > The Initiator > > flag is set, the Response bit is set to 0, and the version flags are > > set in the normal fashion. can be omitted from the general introduction here - the various items referred to in the sentence have not been introduced at this point in the document. s1.5, last para: The previous comment applies to the last sentence here also. s2, para 1: There should be references for the port number definitions 500 is in RFC 2408 and 4500 is in RFC 3947. Arguably port 500 should be reassigned to IKE by the IKE standards since ISAKMP is no longer a well-known protocol. s2.9, last para: I believe the 'SHOULD narrow' ought to be 'should narrow'. This is is not something the protocol controls. s2.13, para 2: HMAC needs a reference. 2.14, para 1: A reference back to Section 2.13 for the key length discussion would be useful. s2.22: The IPCOMP algorithm table ought to be updated to the date of publication of this draft as an RFC - note to RFC Editor. s2.23, bullet point 7: 'these payloads'? Which payloads are they? s2.23.1, para 5: This is the first 'real' use of PAD (as opposed to in the list of changes) - it would be useful to expand it and put in a forward reference. s2.23.1: There are several missing definite articles in this section. s3.1, Exchange Type: This should be updated to refer to the current document. AFAICS the table of exchange types is still valid as of today. There are several subsequent instances of the same issue. s3.2: The table of payload type identifiers could usefully have a forward reference to the section number where it is defined. s3.2, Payload length: Should be specified as unsigned integers. there are many other cases - it could be conveniently resolved by a global statement added to the byte ordering statement. s3.2, last para: 'Many payloads contain fields marked as "RESERVED".' So what? (well they should be treated just like you said two paras before, presumably!). s3.3, para 1: :-) 'Assembly of Security Association Payloads requires great peace of mind.' Hence any implementation must be capable of passing the Turing Test?????? s3.3.1, SPI Size: 'the SPI is obtained from the outer header': What does 'outer' mean here? s3.5, para after ID Field types table: 'Implementations SHOULD be capable of generating and accepting all of these types.' Does 'all' here mean the four types in the previous sentence or 'all' the types in the full table? s3.6: Probably needs a reference for ASN.1 specification. s4, para 2: 'There are a series of optional features that can easily be ignored by a particular implementation if it does not support that feature.' I found this hard to parse. I *think* it is saying that the following list of features are ones that are prime candidates for omission from minimal implementations. How about: 'The following features are a selection of those that can be omitted from a minimal implementation while leaving it capable of interoperating satisfactorily with others:' s7, last para: Most of the references here could be usefully attached to some of the tables in s3 where the relevant specifications are called out. This would be both useful and resolve the xml2rfc nit. Appendix B, para 2: > > The strength supplied by group one may not be sufficient for the > > mandatory-to-implement encryption algorithm and is here for historic > > reasons. There are no longer any mandatory to implement encryption algorithms (at present), so this statement needs to be revised.