Re: [IPsec] I-D Action: draft-ietf-ipsecme-rfc4307bis-04.txt

["Valery Smyslov" <[e-mail]>]

Hi,

> <chair hat on>
> 
> This version has many significant changes from the previous draft. 
> Please review it soon so we don't have a lot of surprises in WG Last 
> Call.
> 
> --Paul Hoffman

here is my review of the draft. 

Most issues are editorial. In a few places text needs to be improved,
Section 4.1. needs more explanation text. Otherwise the document looks good.



1. No period at the end of Abstract.

2. Section 1.

   The IKE protocol itself is
   also protected by encryption which is negotiated between the two
   endpoints using IKE.

    This text is not accurate, since it doesn't mention authentication (MAC),
    that also protects IKE SA.

3. Section 1.

    In the text

   To
   ensure interoperability, a set of "mandatory-to-implement" IKE
   encryption algorithms is defined.

    s/encryption algorithms/cryptographic algorithms

4. Section 1.1

   The algorithm implementation requirements and usage guidance may need
   to change over time to adapt to the changing world.  For this reason,
   the selection of mandatory-to-implement algorithms was removed from
   the main IKEv2 specification and placed in this document.

    Isn't it better to change:

    s/in this document/in a separate document

    (because the guidelines were not only in _this_ document, but also in previous documents)
    
5. Section 3.1

   The algorithms in the below table are negotiated in the SA payload
   and used for the Encrypted Payload.  

    Isn't "in the table below" more grammatically correct? The same in Section 3.3.

6. Section 3.1

       AES-GCM with a 16 octet ICV was not considered as in RFC4307.

    Shouldn't "as" be removed from grammar point of view?

7. Descriptions for AES-GCM and ENCR_AES_CCM_8 demonstrate some
    inconsistency. While the former claims that the advantage of using
    shorter than 16 bytes ICV are minimal, the latter claims that 
    8 bytes ICV is enough for IKE SA. Sure, the latter is for IoT use case,
    but I don't expect that the amount of data transferred over IKE SA
    in IoT and non-IoT cases is many magnitudes different.
    I think some other reasons must be given to justify this selection.
    For example: the majority of existing IoT devices implement
    ENCR_AES_CCM_8, so we decided to make it mandatory in IKE
    (well, I don't know if it is true, I'm not familiar with IoT world).

8. Section 3.2

   Transform Type 2 Algorithms are pseudo-random functions used to
   generate random values when needed.

    s/Algorithms/algorithms
    s/random/pseudorandom

9. Section 3.2

   If an algorithm is selected as the integrity algorithm, it SHOULD
   also be used as the PRF.  When using an AEAD cipher, a choice of PRF
   needs to be made.

    This text is not clear for me. What do you mean? The PRF needs always
    be negotiated in IKEv2, regardless of AEAD/non-AEAD cipher.
    Or do you mean that when non-AEAD cipher is used then
    PRF and ICV transforms SHOULD be based on the same underlying algorithm?
    Is it justified somewhere? Well, I can imagine some reasons for it, but I don't
    like uppervase "SHOULD" here unless you provide some reasoning.

10. Section 3.2

   PRF_HMAC_SHA2_256 was not mentioned in RFC4307, as no SHA2 based
   authentication was mentioned.  

    s/authentication/transforms

11. Section 3.2

   PRF_HMAC_SHA2_512 SHOULD be implemented as a future replacement for
   PRF_HMAC_SHA2_256 or for when stronger security is required.

    Shouldn't "for when" be "when" from grammatical point of view? The same in Section 3.3.

12. Section 3.2

    PRF_AES128_CBC is not in the the IKEv2 IANA registry. There is PRF_AES128_XCBC instead.

13. Section 3.3

    There is some disconnect between Sections 3.1 and 3.3. Section 3.1 lists
    ENCR_AES_CCM_8 as the only preferred choice for IoT. However,
    ENCR_AES_CCM_8 is AEAD cipher, so if it is used then no separate
    authentication transform is needed. Why then AUTH_AES_XCBC_96
    is listed in Section 3.3 for IoT use case? What encryption transform
    it is intended to be used with in IoT? 

14. Section 3.4

    Note that
   it is critical to enforce a secure Diffie Hellman exchange as this
   exchange provides encryption for the session.  

    s/encryption/keys
    s/Diffie Hellman/Diffie-Hellman

15. Section 3.4

    If an attacker can
   retrieve the private numbers (for example a, b) (and? or?) the public
   values (for example g**a, g**b), then the attacker can compute the
   secret and the keys used and decrypt the exchange.  

    What is (and? or?)? Is it some leftover? Shouldn't it be just "and"?

16. Section 4.1

   RSA Digital Signature is widely deployed and therefor kept for
   interoperability.  

    s/therefor/therefore

17.Section 4.2

    Recommendations for when a hash function is involved in a signature:

    Shouldn't it be rephrased for more smooth reading?

18. Section 4.2

    The last table contains unexplained comment "?SHOULD".

19. Section 4.2

    In general this section leaves an impression that it is incomplete
    and contains disconnects. For example, it states that 
    RSASSA-PSS MUST be implemented, however a few lines
    below in a table I read: RSASSA-PSS with SHA-256 - SHOULD.
    I think more explanation text should be added.

Regards,
Valery Smyslov.