[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
rushed comments on IKEv2 draft
I just read draft-ietf-ipsec-ikev2-00.txt for the first time. Here
are my detailed comments on what I read.
Disclaimers: I read and wrote too quickly. I haven't read others
comments. I'm rushing to get this out before the IETF sessions that
will deal with this draft.
I liked much about the draft, but I'm not going to say anything about
the parts that I agree with. So this is full of complaints.
This document is ordered to match the text it comments on. The
complaints are very mixed in importance.
Hugh Redelmeier
hugh@mimosa.com voice: +1 416 482-8253
IKEv2 is not close enough to IKE v1 to be considered just a version
change. Many changes are nice but not important.
On the other hand, it carries a lot of baggage from IKE v1. A
blank-page design could be a lot cleaner and simpler.
I'd say that it falls between these two stools. As such, I would not
support its adoption until it goes one way or the other.
Global:
- I think all occurrences of "byte" should be replaced with "octet".
Somewhere:
- I don't see anything to address the well-known race condition between
setting up IPsec SAs and using them. A third message in Phase 2 serves
admirably. I am not consciously arguing for a commit bit :-)
- [3.1? 4.1? 7.4?] The draft should spell out how bignums are to be
represented as octets. Big-endian is obvious. Not so obvious is
that the number of octets to be used does not depend on the length
of the value but on the length of the modulus. This makes a
difference when converting values to octets for keying material and
hashes.
- in IKEv1, there are a few constraints on payload order. In our
interop testing, we think all implementations sent payloads in the
order used in the RFC. So this was useless freedom, causing
more test cases, ones that have probably never been exercised!
I suggest that the payload order be fully specified.
1.1 The IKE Protocol
- viewing all IKE messages as fitting into request/response
pairs may be a straightjacket. Each message that is neither the
first nor the last could be designed as as a response to the
previous message AND a request for the next. On the other hand, the
pure request/response view may be easier to understand.
- As described in draft-spencer-ipsec-ike-implementation-01.txt
section 3.2 I think that a more symmetric retransmission logic is
better. It does not depend on who is the Initiator and who is the
Responder:
(1) if a reply is expected, retransmit only if it does not appear
before a timeout
(2) if a reply is not expected (last message of the exchange),
retransmit only on receiving a retransmission of the previous
message.
If DOS evasion is being attempted, and we just sent the second
message, (1) should be suppressed -- part of being stateless.
- The description of the anti-DOS steps could be improved by making
them more formal. An example of a question not answered by the
text: may the Responder delay the Initiator indefinitely with
a series of IKE_SA_init_reject messages?
1.2 Changes from IKEv1
- I'd like to add two goals:
+ Eliminate race conditions in setting up or replacing an IPsec SA
group (those SAs created by a single exchange).
+ to standardise rekeying (replacement of SAs).
2.1 Use of Retransmission Timers
- see comments on 1.1. Interesting that this is covered two places in
the document.
2.2 Use of Sequence Numbers for Message ID
- In IKEv1, (non-zero) message IDs are used only after Phase 1.
Am I right in interpreting this section as saying all messages
Have message IDs, and only the first pair of Phase 1 messages
have zero message IDs? This isn't completely clear.
"In the case where the IKE_SA_init is rejected (e.g. in order to
require a cookie), the second IKE_SA_init message will begin the
sequence over with Message #0."
Does an the second IKE_SA_init message start both sides Message ID
at zero? Surely yes, but this isn't clear.
When an IKE SA is rekeyed, does the Message ID start at 0 again?
In summary, I think that the counter should be local to the IKE SA
and should start at 0 for each new IKE SA. I am not sure that
this is what is being proposed.
2.3 Window Size for overlapping requests
- "An IKE endpoint SHOULD be capable of processing incoming requests out
of order to maximize performance in the event of network failures or
packet reordering."
Does this make sense if request n logically requires n-1?
- Are there race conditions to be considered? For example:
Since child-SAs are inherited by the replacing IKE SA,
would it be legal for an apparently-subsequent delete notification
to come under the protection of the superseded IKE SA?
2.6 Cookies
"Unlike ESP and AH where only the recipient's SA identifier appears in
the message, in IKE, the sender's IKE SA identifier is also sent in
every message. In IKEv1 the IKE-SA identifier consisted of the pair
(Initiator cookie, Responder cookie), whereas in IKEv2, the SA is
uniquely defined by the recipient's SA identifier even though both
are included in the IKEv2 header."
- since only the recipient's cookie matters, why not expand it to
fill the whole space formerly used for the pair of cookies?
A larger cookie might have advantages and there would seem to
be no worse compatibility problems.
- The SA is uniquely identified (not defined) by the recipient's SA
identifier AND IP address, I think. This is analogous to the IPsec
case. If not "IP address", some system identifier -- we cannot assume
that the SA identifier is globally unique.
2.8 Rekeying
- Negotiation of a new IKE SA within an IKE SA is not well described
and that description seems to be in the wrong place.
+ 2.8 says:
To rekey an IKE-SA, establish a new equivalent IKE-SA (see section 4
and 4.2 below) with the peer to whom the old IKE-SA is shared using a
Phase 2 negotiation within the existing IKE-SA. An IKE-SA so created
inherits all of the original IKE-SA's child SAs. Use the new IKE-SA
for all control messages needed to maintain the child-SAs created by
the old IKE-SA, and delete the old IKE-SA.
+ IKE SA creation is not mentioned in section 4. It only talks
about child SAs -- I don't think that an IKE SA is a child.
Besides, the TS payloads (mandatory) have no meaning for an IKE
SA. Ahh: the syntax described in 4 is wrong for 4.2, but
no diagram appears in 4.2. Somehow, 4 should be generalized
to cover IKE and IPsec SA negotiation; perhaps it needs to be
split.
+ The title of 4.2 "Generating Keying Material for IKE-SAs from a
create-child exchange" is misleading. It should be called
something like "Replacing [rekeying?] an IKE SA using a
create-child exchange". More than generating keying material is
going on: SPIs are being changed, msgids are starting over (I think),
lifetimes are probably being reset. Apparently Phase 1 IDs are
not being changed.
2.9 Traffic Selector Negotiation
- the description doesn't suggest that Traffic Selectors can include
protocol as a selector. Surely they can.
- "The Responder is allowed to narrow the choices by selecting a subset
of the traffic, for instance by eliminating one or more members of
the set of traffic selectors provided the set does not become the
NULL set."
This concerns me. I don't know that this negotiation can be
guaranteed to satisfy the Initiators needs. Yet the Initiator
cannot express this. I don't know what negotiation makes sense here
-- I don't have a counterproposal.
3 The Phase 1 Exchange
- "identities are hidden from eavesdroppers." It would be good to
point out that an active attack can reveal identities.
- HDR*, ID, AUTH, [CERT,]
SA, TSi, TSr -->
This diagram, as I read it, is wrong: the "SA, TSi, TSr" is optional.
I suggest:
HDR*, ID, AUTH, [CERT,]
[ SA, TSi, TSr ] -->
Similar adjustment is needed to the response syntax.
It would be good to actually write the two messages/response
pairs separately since the optional payloads in the first message
determine whether the optional payloads are forbidden or required
in the response.
- what happens if the IKE SA negotiation would succeed but the
piggybacked IPsec SA group negotiation would fail? How is
that communicated?
4 The CREATE-CHILD-SA (Phase 2) Exchange
- "A phase 2 exchange is one request/response pair, and can be used to
create or delete a child-SA, delete the IKE-SA, or deliver
information such as error conditions."
I think that it can be used to create a new IKE SA too.
- there are two kinds of PFS. This should be discussed somewhere -- here?
- If the Initiator includes a KE, must the Responder?
If the Initiator does not include a KE, must the Responder not include a KE?
I don't think that this is specified.
4.1 Generating Keying Material for IPsec SAs
- "A single child-SA negotiation results in two security associations--
one inbound and one outbound."
This is not always true -- there may be as many as 6. This is an
example of why I wish to introduce a term for the collection of
IPSEC SAs arising from a single negotiation. For now, I use
"group".
5 Informational (Phase 2) Exchange
- There needs to be a name for the collection of IPsec SAs negotiated
at once. With this name, the concept can be recognized as
important. I will use "group" for now.
- A delete should delete a whole group at once. Nothing else makes
sense. How to refer to the group? By the message-ID of the Phase 2
that negotiated the group (there should be an equivalent for
piggyback-negotiated groups). No fuss, no muss (no problem with
some but not all SAs of a group being deleted, no problem with
referring to an ipcomp SA, no problem with implied reference to
outbound SAs).
No delete should refer to an SPI.
To delete an IKE SA, you must issue the delete under the protection
of that SA. So the SA can be implicit, as long as the protocol 0
is specified.
So what protocol should be used for the IPsec SA group? Why not 1
(the old DOI)?
- "ESP, AH, and IPcomp SAs always exist in pairs, with one SA in each
direction." This should be qualified by "created through IKE".
- "When SAs are nested" I think that this would be more accurately
described using the "group" concept.
6 Error Handling
- "There is a trade-off between wanting to be helpful in diagnosing a
problem and responding to it and wanting to avoid being a dupe in a
denial of service attack based on forged messages."
There is also a risk of disclosing compromising information.
7.1 The IKE Header
- the HDR is unprotected (unauthenticated, unencrypted). Everything
that can be moved out should be moved out to be protected:
+ next payload
+ exchange type
+ flags (except *possibly* E(ncrypted))
+ message ID
+ length
The explicit IV makes it possible to hide more than would otherwise
be the case. The SPI aka cookies ought to be enough.
If IKEv1 is going to be catered to, make up a plausible header,
but don't trust or use it.
- "Payloads are processed in the order in which they appear in an IKE
message"
It isn't clear what this is meant to say. Is this only talking
about parsing the message, or is something more going on?
- "It is therefore possible for a single instance of IKE to multiplex
distinct sessions with multiple peers."
I think that each session involves only one pair of peers. The sentence
suggests otherwise.
7.2 Generic Payload Header
- "critical" bit should have the other sense. Unless a payload is
marked "ignore if you don't understand", it should be treated as
critical. This default (critical) should be represented as 0.
7.3 Security Association Payload
- SA payload description is opaque. My I suggest BNF-like notation as
a supplement? But not just the description is complex -- the actual
structure is complex. Much more should be said about this, but I'm
not yet up to speed on it.
- does it make sense to combine IKE and other protocols in one
SA Payload? Is that how the IKE SA's encryption and authentication
is negotiated? How does the context (kind of SA being negotiated)
affect the SA Proposal?
7.3.1 Proposal Substructure
- "the first four bytes of the Proposal structure are designed to look
somewhat like the header of a Payload"
In what sense "somewhat"? Are they not identical?
- Why are there two bullets about "Proposal Length"?
Why are there two bullets about "Proposal #"?
Why are there two bullets about "RESERVED"?
7.3.2 Transform Substructure
- "For Transform Type 7 (Window Size), the Transform-ID specifies the
window size a peer is contracting to support to handle overlapping
requests (see section 2.3)."
This seems a little ugly when every other Transform-ID is essentially
an enumerated type.
7.3.4 Mandatory Transform-IDs
- "Some transforms are mandatory to support" What does that mean?
If they are proposed, must they be accepted (assuming nothing
else is proposed)? Must they be proposed?
- AES is good. With it comes a need for stronger D-H group, hash, and
PRF, I think.
7.3.4 Transform Attributes
- the number 7.3.4 is used twice.
- "Attribute Length (2 bytes) - Length in bytes of the Attribute
Value. When the AF bit is a one (1), the Attribute Value is only 2
bytes and the Attribute Length field is not present."
This is clear, and appropriate, but a surprise is lurking. In payloads,
the length includes the header. In attributes, the length excludes
the header. It would be a kindness to point this out to the reader.
7.3.5 Attribute Negotiation
- "implementers of this protocol SHOULD accept values which they deem
to supply greater security."
I think that the implementation may wish to say no to prevent
overloading. I think that "SHOULD" is too strong.
7.5 Identification Payload
- If Phase 2 IDs have any meaning, they probably need their own
authentication. Unless they are only selector specifications,
in which case they should be moved into Traffic Selector.
Just what is the semantic content of a Phase 2 ID?
- how does one specify a Phase 2 ID when the IPsec negotiation
has been piggybacked within the Phase 1 negotiation (section 3)?
- should the strings in id payloads be ASCII? UTF8? Reference
an RFC about FQDNs and user@FQDNS?
7.8 Authentication Payload
- "initial unprotected IKE-SA-INIT" I presume that this means the
second pair if DOS avoidance happened.
- is the Signature Payload representation the same as in IKEv1?
Both use PKCS#1, but I'm not sure that they are using it the
same way.
- "using the cryptographic hash and signature algorithms chosen by the
signer". If I've understood, there is a common hash, but the
signature algorithms may differ because they reflect the respective
public keys.
7.10.1 Notify Message Types
- [about INVALID-MESSAGE-ID] "Sent when either an IKE MESSAGE-ID more
that ten greater than the highest acknowledged MESSAGE-ID."
Where does the magic number 10 come from?
- [about ADDRESS-NOTIFICATION] "Don't understand."
I don't understand this description.
- [about INITIAL_CONTACT] "This notification MUST NOT be sent by an
entity that may be replicated (e.g. a roaming user's credentials
where the user is allowed to connect to the corporate firewall from
two remote systems at the same time)." How can this be enforced?
How can one tell if an entity may be replicated? The problem goes
the other way too: it should not be sent to an entity that may be
replicated.
7.11 Delete Payload
- "NOTE: What's the deal with IPcomp SAs. This mechanism is probably not
appropriate for deleting them!!"
One example of why deleting a group makes sense.
7.13 Traffic Selector Payload
- the SUBNET forms of TS are redundant: the RANGE forms can handle
the same choice. I think that this redundancy adds complexity with
no benefit.
8 Diffie-Hellman Groups
- group 1 and DES are described as being "for historic purposes only".
For use by Civil War Re-enactors? We should not have them in a
yet-to-be-adopted standard.
- I think that there are patents that affect support for EC2N D-H
groups. This should be noted.
- "nor is there anything which will dilute the strength obtained
from stronger groups"
I am not a cryptographer, but I think that the width of the PRF
will limit the strength obtained from stronger groups.
Appendix A
- "Attributes described as basic MUST NOT be encoded as variable.
Variable length attributes MUST NOT be encoded as basic even if their
value can fit into two bytes. NOTE: This is a change from IKEv1,
where increased flexibility may have simplified the composer of
messages but certainly complicated the parser."
I didn't fine that it complicated my parser.
The new rule is a bit awkward since I will feel compelled
to enforce it. But only for v2.
Follow-Ups: