Subject: Comments to the draft-ietf-ipsec-ikev2-11.txt

From: Tero Kivinen <[email-masked]>
Date: Sun, 9 Nov 2003 05:34:54 +0200

I now managed to find out enough time to completely read through the
draft-ietf-ipsec-ikev2-11.txt draft. Here are some comments to the
draft. I also verified at the same time that all issues in the issue
tracker which we accepted should now be in draft itself. 

> 1 Introduction
...
>    IKE performs mutual authentication between two parties and
>    establishes an IKE security association that includes shared secret
>    information that can be used to efficiently establish SAs for ESP
>    [RFC2406] and/or AH [RFC2402] and a set of cryptographic algorithms
     ^^^^^^^^^           ^^^^^^^^^
>    to be used to protect the SAs.  In this document, the term "suite" or

Should we use the ESPbis and AHbis references here, as the IKEv2
requres support for the extended sequence numbers etc which are not
defined in the RFC2406 etc.

This draft already have normative reference to the RFC2401bis, thus we
have to wait it anyways, so updating the references should not cause
extra delays. 

> 2.5 Version Numbers and Forward Compatibility
...
>    While new payload types may be added in the future and may appear
>    interleaved with the fields defined in this specification,
>    implementations MUST send the payloads defined in this specification
>    in the order shown in the figures in section 2 and implementations
>    SHOULD reject as invalid a message with those payloads in any other
>    order.

We have a small problem here. This text say that implementations MUST
send the payloads in the order shown in the figures in section 2. The
problem is that not all optional payloads are mentioned in the
figures. For example the most of the status notifications are not
listed in the pictures. Only notifications mentioned are the N(cookie)
and N(REKEY_SA).

Others like INITIAL_CONTACT, SET_WINDOW_SIZE, ADDITIONAL_TS_POSSIBLE,
IPCOMP_SUPPORTED, NAT_DETECTION_SOURCE_IP,
NAT_DETECTION_DESTINATION_IP, USE_TRANSPORT_MODE, and
HTTP_CERT_LOOKUP_SUPPORTED notifications are not mentioned.

For some there are also some restrictions to which exchanges to put
them. There are couple of categories here:

      1) child SA and TSi/TSr related:
	 - ADDITIONAL_TS_POSSIBLE
	 - IPCOMP_SUPPORTED
	 - USE_TRANSPORT_MODE
	 Only in those packets which contain child SA and TSi/TSr
	 payloads. Immediately after TSr payload, or
	 ADDITIONAL_TS_POSSIBLE after TSr and IPCOMP_SUPPORTED and
	 USE_TRANSPORT_MODE after child SA payload?

      2) IKE_SA_INIT related:
         - NAT_DETECTION_SOURCE_IP
	 - NAT_DETECTION_DESTINATION_IP
	 Only in IKE_INIT_SA request and reply, propably immediately
	 after Nonce, before any other optional payloads (certreq). 

      3) IKE_SA related:
         - INITIAL_CONTACT
         - SET_WINDOW_SIZE
	 Propably either in the IKE_AUTH exchange or separate
	 INFORMATIONAL exchange. 

      4) Certificate related:
         - HTTP_CERT_LOOKUP_SUPPORTED
	 On the same packet as where CERTREQ payload is (immediately
	 after it)

Other payloads which are not mentioned at all are vendor id payloads,
i.e where are they allowed, and what is their position in the packet?
Should we have some generic rules here explaining where to put the
payloads, or what should we do for it.

My persional opinion is that allowing free order of payloads is easier
from the protocol text point of view, and it makes the sending of
packets little bit easier, and does not make the processing the
received packet that much harder (i.e the first pass of the packet
will parse the generic payload headers, and mark where each payload
is, thus finding the specific payload from the packet is easy). 

IKE_INIT_SA in certreq 3.7

> 2.9 Traffic Selector Negotiation
...
>    The first of the two TS payloads is known as TSi (Traffic Selector-
>    initiator).  The second is known as TSr (Traffic Selector-responder).

This might be little misleading, as the TSi and TSr have different
payload numbers now, there are no more two TS payloads, but one TSi
and TSr payload. I.e the text could say:

The TSi payload is sent first (Traffic Selector-initiator) and the TSr
is send after it (Traffic Selector-responder).

>    TSi specifies the source address of traffic forwarded from (or the
>    destination address of traffic forwarded to) the initiator of the
...
>    acceptable but their union is not, Bob MUST accept some subset and
>    MAY include a Notify payload of type ADDITIONAL_TS_POSSIBLE to
>    indicate that Alice might want to try again. This case will only

Should we say something about that if Bob returns
ADDITIONAL_TS_POSSIBLE, then it MUST next time select different subset
from the traffic selectors if similar set is proposed again.

I.e the initiator will always send same TSi/TSr set and the responder
will select new different subset and return that.

Other possibility is that the initiator will propose different TSi/TSr
next time, by removing the TSi/TSr already returned in the previous
negotiation.

If we take an example where the initiator configured the source range
of (10.0.0.0-10.255.255.255), and the responder have following ranges
(10.0.0.0-10.0.0.255, 10.0.2.0-10.0.2.255, 10.0.4.0-10.0.255.255)
configured (I ignore port, protocol and the destination addresses in
this examples). 

If we use the first option then the negotiation goes like this:

Initiator					Responder
---------					---------
TSi(10.0.0.0-10.255.255.255) -> 
			     <- TSi(10.0.0.0-10.0.0.255),
				N(ADDITIONAL_TS_POSSIBLE)

TSi(10.0.0.0-10.255.255.255) -> 
			     <- TSi(10.0.2.0-10.0.2.255),
				N(ADDITIONAL_TS_POSSIBLE)

TSi(10.0.0.0-10.255.255.255) -> 
			     <- TSi(10.0.4.0-10.0.255.255)

If we use the second option then the negotiation goes like this:

Initiator					Responder
---------					---------
TSi(10.0.0.0-10.255.255.255) -> 
			     <- TSi(10.0.0.0-10.0.0.255),
				N(ADDITIONAL_TS_POSSIBLE)

TSi(10.0.1.0-10.255.255.255) -> 
			     <- TSi(10.0.2.0-10.0.2.255),
				N(ADDITIONAL_TS_POSSIBLE)

TSi(10.0.1.0-10.0.1.255,10.0.3.0-10.255.255.255) -> 
			     <- TSi(10.0.4.0-10.0.255.255)

We must select one of the options, as otherwise we might end up in the
interoperability problem, as if the initiator will always offer the
same range (it is using first option) and responder assumes that
initiator will change its range and always return the first suitable
range (i.e using second option and always returns
10.0.0.0-10.0.0.255).

I think the second option would be better because initiator have more
information about why it is doing the negotiation again, i.e it might
be because it got some packet with different QoS parameters and wanted
to create separate SA for it. 

> 2.11 Address and Port Agility
> 
>    IKE runs over UDP ports 500 and 4500, and implicitly sets up ESP and
>    AH associations for the same IP addresses it runs over. The IP

Francis Dupont pointed out that this is not actually clear enough. I.e
which IP-addresses are used. The IP-addresses of the IKE_SA_INIT
exchange. The IP-addresses of the IKE_AUTH exchange. The IP-addresses
of the CREATE_CHILD_SA exchange. As we do not store information in the
IKE_SA_INIT phase, I think the best is to say that we do not allow
IP-addresses to change during the IKE_AUTH exchanges, and we use the
IP-addresses of the IKE_AUTH exchange.

I.e when the IKE_AUTH exchange is done (all packets using same
IP-address) we tie that IP-address to the IKE SA and use it as the
IP-address for the ESP and AH associations.

This address might then be updated later if there is NAT between and
the other end is behind NAT (see NAT-T section), otherwise it will
stay the same for the lifetime of the IKE SA. Of course we still reply
packets back to the address from where we received them. 

> 2.15 Authentication of the IKE_SA
...
>    the second message.  Appended to this (for purposes of computing the
>    signature) are the initiator's nonce Ni (just the value, not the
>    payload containing it), and the value prf(SK_ar,IDr') where IDr' is
>    the responder's ID payload excluding the fixed header. Note that
					  ^^^^^^^^^^^^^^^^
>    neither the nonce Ni nor the value prf(SK_ar,IDr') are transmitted.
>    Similarly, the initiator signs the first message, starting with the
>    first octet of the first SPI in the header and ending with the last
>    octet of the last payload.  Appended to this (for purposes of
>    computing the signature) are the responder's nonce Nr, and the value
>    prf(SK_ai,IDi'). In the above calculation, IDi' and IDr' are the
>    entire ID payloads excluding the fixed header.  It is critical to the
				  ^^^^^^^^^^^^^^^^
>    security of the exchange that each side sign the other side's nonce
>    (see [SIGMA]).

I think we should use the "generic payload header (see section 3.2)"
instead of the fixed header. Otherwise someone might interpret that
"fixed header" to also include the ID type and RESERVED fields in the
beginning of the ID payload (they are fixed for all ID payload types).

...
>    terminated. The shared secret can be variable length. The pad string
>    is added so that if the shared secret is derived from a password, the
>    IKE implementation need not store the password in cleartext, but
>    rather can store the value prf(Shared Secret,"Key Pad for IKEv2"),
				^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>    which could not be used as a password equivalent for protocols other
>    than IKEv2.  As noted above, deriving the shared secret from a

As we use the negotiated PRF there, we either might need to store
multiple different copies of the secret, or we might need to restrict
the PRFs to be used to the one we are using when storing the result of
the PRF to the database.

I think we should say something about it here too. 

> 2.22 IPComp
...
>    Negotiation of IP compression is separate from the negotiation of
>    cryptographic parameters associated with a CHILD_SA. A node
>    requesting a CHILD_SA MAY advertise its support for one or more
>    compression algorithms though one or more Notify payloads of type
>    IPCOMP_SUPPORTED. The response MAY indicate acceptance of a single
>    compression algorithm with a Notify payload of type IPCOMP_SUPPORTED.
>    These payloads MAY ONLY occur in the same messages that contain SA
>    payloads.

This says that the notifications can only occur on the same packet as
SA payloads, but it does not mention the exact location inside the
packet (i.e after which payload it should appear). Changing the last
two lines to:

"These payloads MAY ONLY occur in the same messages that contain SA
 payloads, immediately after the SA payload."

would fix the problem.

> 2.23 NAT Traversal
...

I already sent another email about this section. 

> 3.3.5 Transform Attributes
...
>    Note that only a single attribute type (Key Length) is defined, and
>    it is fixed length. The variable length encoding specification is
>    included only for future extensions.  The only algorithms defined in
>    this document that accept attributes are the AES based encryption,
>    integrity, and pseudo-random functions, which require a single
>    attribute specifying key width.

This isn't exactly true. The other ciphers having variable length keys
defined in this document can also use the Key Length attribute. This
includes RC5, Cast, Blowfish etc. 

>    Attributes described as basic MUST NOT be encoded using the variable
			     ^^^^^
>    length encoding.  Variable length attributes MUST NOT be encoded as
>    basic even if their value can fit into two octets. NOTE: This is a
     ^^^^^
>    change from IKEv1, where increased flexibility may have simplified
>    the composer of messages but certainly complicated the parser.

The word "basic" is leftover from the IKEv1, and isn't used anywhere
else in the document. It should be changed to "Type/Value" or "TV" as
in the table below. 

>          Attribute Type                 value        Attribute Format
>       --------------------------------------------------------------
>       RESERVED                           0-13
>       Key Length (in bits)               14                 TV
>       RESERVED                           15-17
...
> 3.5 Identification Payloads
...
>    The following table lists the assigned values for the Identification
>    Type field, followed by a description of the Identification Data
>    which follows:
> 
>       ID Type                           Value
>       -------                           -----
>       RESERVED                            0
>       ID_IPV4_ADDR                        1
>       ID_FQDN                             2
>       ID_RFC822_ADDR                      3
>       ID_IPV6_ADDR                        5
>       ID_DER_ASN1_DN                      9
>       ID_DER_ASN1_GN                      10
>       ID_KEY_ID                           11

The table above does not have the numbers not used in IKEv2 marked as
RESERVED, and it does not have range reserved to IANA etc. Also
whether those numbers are actually shared between IKEv1 and IKEv2,
should be mentioned here. 

> 3.7 Certificate Request Payload
> 
>    The Certificate Request Payload, denoted CERTREQ in this memo,
>    provides a means to request preferred certificates via IKE and can
>    appear in the IKE_INIT_SA response and/or the IKE_AUTH request.
		   ^^^^^^^^^^^^

Typo, should be IKE_SA_INIT instead. 

...
>    The Certificate Encoding field has the same values as those defined
>    in section 3.6. The Certification Authority field contains an
>    indicator of trusted authorities for this certificate type.  The
>    Certification Authority value is a concatenated list of SHA-1 hashes
>    of the public keys of trusted CAs.  Each is encoded as the SHA-1 hash
>    of the Subject Public Key Info element (see section 4.1.2.7 of [RFC
>    3280]) from each Trust Anchor certificate.  The twenty-octet hashes
>    are concatenated and included with no other formatting.

Which encodings are actually using this format? I assume at least
Kerberos Token etc are not using this format :-)

So this should mention that X.509 certificates, CRL, ARL, Hash and URL
formats, and possibly PKCS#7 are using this format (i.e everything
having PKIX CA certificates).

> 3.10.1 Notify Message Types
...
>         NOTIFY MESSAGES - STATUS TYPES           Value
>         ------------------------------           -----
> 
>         INITIAL_CONTACT                          16384
> 
>             This notification asserts that this IKE_SA is the only
>             IKE_SA currently active between the authenticated
>             identities. It MAY be sent when an IKE_SA is established
>             after a crash, and the recipient MAY use this information to
>             delete any other IKE_SAs it has to the same authenticated
>             identity without waiting for a timeout.  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).

Where is this notification sent. Inside the IKE_AUTH exchange? As a
separate INFORMATIONAL exchange? That should be clarified. The IKEv1
left that open, and we did see implementors implementing this
differently. 

>         SET_WINDOW_SIZE                          16385
> 
>             This notification asserts that the sending endpoint is
>             capable of keeping state for multiple outstanding exchanges,
>             permitting the recipient to send multiple requests before
>             getting a response to the first. The data associated with a
>             SET_WINDOW_SIZE notification MUST be 4 octets long and
>             contain the big endian representation of the number of
>             messages the sender promises to keep. Window size is always
>             one until the initial exchanges complete.

When is this notification sent? Again inside IKE_AUTH exchange or as
separate INFORMATIONAL exchange? Are nodes allowed to change their
window size freely, i.e can they send this multiple times. Are they
allowed to make their window smaller. What happens to the exchanges
which are in progress at that time (and might be violating the new
window size)? What happens to the window size after rekey, is it
dropped back to 1 when IKE SA is rekeyed? Is it per IKE SA or per
host. 

I would say the most simple solution is to say, that you can only send
this as separate INFORMATIONAL exchange, and you can only do this
once per IKE SA (after IKE SA rekey the window size is set back to 1),
and it is per IKE SA. 

>         ADDITIONAL_TS_POSSIBLE                   16386
> 
>             This notification asserts that the sending endpoint narrowed
>             the proposed traffic selectors but that other traffic
>             selectors would also have been acceptable, though only in a
>             separate SA (see section 2.9). There is no data associated
>             with this Notify type. It may only be sent as an additional
>             payload in a message including accepted TSs.

Exact location of this payload inside the packet? Immediate after TS*
payloads (i.e after TSr)? 

>         IPCOMP_SUPPORTED                         16387
> 
>             This notification may only be included in a message
>             containing an SA payload negotiating a CHILD_SA and
>             indicates a willingness by its sender to use IPComp on this
>             SA. The data associated with this notification includes a

Exact location? Immediately after SA payload?

>         NAT_DETECTION_SOURCE_IP                  16388
>         NAT_DETECTION_DESTINATION_IP             16389

Location? After Nonce in the IKE_SA_INIT payloads?

>         COOKIE                                   16390

Perhaps we should repeat the location of this, i.e the first payload
of the IKE_SA_INIT. 

>         USE_TRANSPORT_MODE                       16391
>             This notification MAY be included in a request message that
>             also includes an SA requesting a CHILD_SA. It requests that

Again location? After SA payload? If both IPCOMP_SUPPORTED and this,
which comes first? No ordering requirement for them?

>         HTTP_CERT_LOOKUP_SUPPORTED               16392
>             This notification MAY be included in any message that can
>             include a CERTREQ payload and indicates that the sender is
>             capable of looking up certificates based on an HTTP-based
>             URL (and hence presumably would prefer to receive
>             certificate specifications in that format).

Exact location? Immediately after CERTREQ?

>         REKEY_SA                                 16393
>             This notification MUST be included in a CREATE_CHILD_SA
>             exchange if the purpose of the exchange is to replace an
>             existing ESP or AH SA. The SPI field identifies the SA being
>             rekeyed. There is no data.

Repeat exact location. First payload inside the encrypted payload. 

> 3.12 Vendor ID Payload
...
>    A Vendor ID payload may be sent as part of any message.  Reception of

Is there any payload ordering restrictions on the Vendor IDs?

> 3.15.1 Configuration Attributes
> 
>                            1                   2                   3
>        0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       !R|         Attribute Type      !            Length             |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |                                                               |
>       ~                             Value                             ~
>       |                                                               |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
>                Figure 23:  Configuration Attribute Format
> 
>    o  Reserved (1 bit) - This bit MUST be set to zero and MUST be
>       ignored on receipt.

What is the reason for the 1 bit reserved field in the attribute type
field? I think it would be better to make attribute type 16 bit field
and remove this. 

>    o  Attribute Type (7 bits) - A unique identifier for each of the
		        ^^^^^^
>       Configuration Attribute Types.

Attribute type field is 15 bits, not 7. 

>                                       Multi-
>         Attribute Type          Value Valued Length
>         ======================= ===== ====== ==================
>          INTERNAL_IP6_SUBNET     15    NO    17 octets
					       ^^^^^^^^^

That should be 0 or 17 octects as in other types too (0 = request). 

>       o  INTERNAL_IP4_SUBNET - The protected sub-networks that this
>          edge-device protects.  This attribute is made up of two fields;
>          the first being an IP address and the second being a netmask.

It is actually little funny, that the IKEv2 uses IP-address ranges in
all other places, but here it uses two different ways to express
subnet + netmask. I.e for IPv4 it is netmask as bitmask, and in IPv6
it is prefix length. 

>       o  INTERNAL_IP6_SUBNET - The protected sub-networks that this
>          edge-device protects.  This attribute is made up of two fields;
>          the first being a 16 octet IPv6 address the second being a one
>          octet prefix-length as defined in [ADDRIPV6].  Multiple

I think it would be cleaner design to use IP-address ranges in both
IPv4 and IPv6 cases here. Especially when our TSi/TSr policies allows
us to express everything as ranges, and we might not be able to
convert that range to the this kind netmask format.

Actually the whole reason for INTERNAL_IP*_SUBNET is little unclear
for me. I think we should be able to express same thing with TSi/TSr
payloads. I.e the client requests TSr of 0.0.0.0/0, and gets list of
subnets behind SGW back. Do we need another mechanism for that, i.e
can we remove this?

> 6 IANA Considerations
> 
>    This document contains many "magic numbers" to be maintained by the
>    Internet Assigned Numbers Authority (IANA). While in many cases the
>    values were chosen so as to avoid collisions with other
>    specifications, these should be considered a new IANA registry for
>    IKEv2. The tables to be maintained are:
> 
>    Payload Types
>    Transform Types
>    For each Transform Type defined, the assigned Transform values
>    Authentication Method
>    Security Protocol ID
>    Error types
>    Status types
>    IPComp Transform IDs
>    Configuration request types
>    Configuration attribute types

Is this list up to date? I think we should collect the initial IANA
registry as a separate draft, so the IANA will have much easier task
to create all these new registries. 

> 9.1 Normative References
> 
>    [ESPCBC]   Pereira, R., Adams, R., "The ESP CBC-Mode Cipher
      ^^^^^^
>               Algorithms", RFC 2451, November 1998.

That should be [RFC2451] as it is used in the text section 3.3.2.
ESPCBC is not used anywhere in the text. 

> 9.2 Informative References
...
>    [Hutt02]   Huttunen, A. et. al., "UDP Encapsulation of IPsec
>               Packets", draft-ietf-ipsec-udp-encaps-05.txt, December
>               2002.

Shouldn't this be normative reference, as if you want to implement
NAT-T then you need to read the keepalive etc protocol from here?

>    [Ker01]    Keromytis, A., Sommerfeld, B., "The 'Suggested ID'
>               Extension for IKE", draft-keromytis-ike-id-00.txt, 2001

There is no references to this draft anywhere in the text, and it is
already expired. Perhaps we can delete this. 

Ok, that ends my comments to the draft-ietf-ipsec-ikev2-11.txt for
now, hopefully we do not see another such message later anymore :-) 
-- 
[email-masked]
SSH Communications Security                  http://www.ssh.fi/
SSH IPSEC Toolkit                            http://www.ssh.fi/ipsec/