I have been selected as the General Area Review Team (Gen-ART)
reviewer for this draft (for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).
Please resolve these comments along with any other Last Call comments
you may receive.
Document: draft-ietf-ipsecme-ikev2bis-08.txt
Reviewer: Elwyn Davies
Review Date: 18 March 2010
IETF LC End Date: 18 March 2010
IESG Telechat date: (if known) -
Summary:
Not ready. The document contains a lot of minor niggles and nits plus a major
item that I am not sure the IETF should support: this is the removal of all
mention of mandatory to implement security suites from the document. I
appreciate the difficulty of keeping up to the minute, but it seems to me that
this is outweighed by the difficulty of guaranteeing interoperability. If the
security landscape is so unstable, we have a bigger problem perhaps. Whether
this change is acceptable to the IAB, the IESG and the wider IETF is not
something I can resolve.
One niggle that I felt represented a rather lackadaisical approach, was the
retaining of the publication date of RFC 4306 as a frozen point in time for the
purpose of documenting the state of the world as regards lists of configurable
lists. I would have preferred the losts to be up to date at the publication of
*this* RFC.
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. Is this acceptable?
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.2: Maybe should mention that retransmissions MUST use the same Message ID.
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.8.1, para 2: The use of 'lexicographical' in the comparison algorithm adds
confusion IMO. It implies that there is some language related total ordering
on the values stored in each octet that might or might not be the natural
integer ordering. Presumably the intent is that corresponding octets should be
treated as 8 bit unsigned integers (in network bit order?) and compared as such.
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? NOTIFY payload?
s2.21.4: More requests to limit message rates without specific means.
s2.23, para 7 and 8 (first bullet point): I understood from earlier that the
first requirement (port 4500 and responding to source address/port) applied to
*all* implementations and not just those specifically supporting NAT traversal.
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.6, next to last para: 'also MUST be capable of being configured to send and
accept the two Hash and URL formats (with HTTP URLs).'
^^^^^^^^^^^^^^^^^^^^^^^^
Only one format appears to be defined in this section. Is there another
somewhere else?
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, para 4:
The pair is called an "exchange". We call the first
messages establishing an IKE SA IKE_SA_INIT and IKE_AUTH exchanges
and subsequent IKE exchanges CREATE_CHILD_SA or INFORMATIONAL
exchanges.
The start of the second sentence is confusing on a quick read: A better form
might be:
We call the first exchanges of messages that establish an IKE SA IKE_SA_INIT
and IKE_AUTH and subsequent IKE...
s1.2 (conventions): The use of [] to denote optional items in exchanges can be
confusing with references. But we will probably have to live with it.
s1.2
The keys used for the
encryption and integrity protection are derived from SKEYSEED and are
I assume *how* the derivation is done is described later (I think sa2.13/2.14
is the right answer). A forward reference would help - both to show that you
shouldn't worry about how just now and to point to where it is defined.
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.3.1: The term Message ID has not been discussed on defined before its
appearance here.
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.
s2.13, para 3: I think the statement 'The preferred key size is used as the
length..' ought to be strengthened to 'The preferred key size MUST be used as
the length..'.
s2.14, para 1: A reference back to Section 2.13 for the key length discussion
would be useful.
s2.15, para 1: IDr' is defined twice - once on its own and once in concert with
IDi'.
s2.17, para 7: the term ROHC_INTEG is not defined - it almost certainly needs a
reference.
s2.17, last bullet: s/protocol specification/the protocol specification/,
s/For ESP and/For ESP and/
s2.20, para 2: Is the term 'trolling' sufficiently well-known not to need
definition?
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, para 2: The term 'real Internet' is not well defined. The pejorative
language in para 1 and here is unhelpful.
s2.23, para 6: How is IPsec expected to 'discover' a NAT? (it appears that I am
told a few paras later, but it isn't obvious that that is the case here!)
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.
s2.23.1, para 11: s/known NATs outer addresses/known NAT's outer addresses/
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.1, Response Flag: s2.21.2 has a (sort of) exception to the 'no response to a
response rule'.
s3.1, Message ID and Length: Should be specified as unsigned integers.
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, 'Proposal #': The usage of # as shorthand for number is US specific
jargon and should be defined. This issue also occurs in '# of transforms' in
s3.3.1.
s3.3 onwards, last para: Repeating the payload type number here makes for a
double maintenance problem and isn't useful within the payload.
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.5, para after ID Field types table: 'IPv6-only implementations MAY be
configurable to send only ID_IPV6_ADDR instead of ID_IPV6_ADDR for IP
addresses.' s/ID_IPV6_ADDR instead of ID_IPV6_ADDR/ID_IPV6_ADDR instead of
ID_IPV4_ADDR/
s3.6: Probably needs a reference for ASN.1 specification.
s3.9, last para: s/The size the/The size of the/
s3.16, para 1: 'The full set of acceptable values for the payload is defined
elsewhere,' Where is not specified, but maybe RFC 3748. It should be
explicitly stated that Type and Type_data are taken from RFC 3748 (and maybe
other documents).
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.
_______________________________________________
Ietf mailing list
Ietf(_at_)ietf(_dot_)org
https://www.ietf.org/mailman/listinfo/ietf