Subject: [Int-dir] INT Dir review: draft-ietf-lwig-ikev2-minimal-03

From: Dave Thaler <[email-masked]>
Date: Thu, 24 Sep 2015 23:01:26 +0000

Hi,

I am an assigned INT directorate reviewer for draft-ietf-lwig-ikev2-minimal-03.

These comments were written primarily for the benefit of the Internet

Area Directors.  Document editors and shepherd(s) should treat these

comments just like they would treat comments from any other IETF

contributors and resolve them along with any other Last Call comments

that have been received. For more details on the INT Directorate,

see http://www.ietf.org/iesg/directorate.html.

A marked up copy with my comments in line is at
http://research.microsoft.com/~dthaler/draft-ietf-lwig-ikev2-minimal-03.pdf

Possibly technical:

1)      Page 5 discusses cryptographic algorithms but does not state, nor reference

any other document that states, what algorithm(s) are mandatory to

implement in a minimal implementation.  Without that, implementations

may not interoperate.

2)      Page 6 talks about agreeing on a PRF.   But it does not state, nor reference

anywhere else that states, how such is agreed upon or what is mandatory

to implement.

3)      I am not familiar with IKEv2 protocol details, but the second paragraph

of section 2.1 implies that only initiators send requests, and only

responders send responses, and only requests are retransmitted.  However,

much of the rest of the document, including parts of section 2.1, imply

that initiators (especially minimal implementations of an initiator)

receive requests such as CREATE_CHILD_SA, etc.   This contradiction

should be addressed.

4)      Several places talk about what a "typical" minimal implementation would

do, or what a minimal implementation would "usually" do.  However, it's

not clear what the alternative(s) are.  Since an implementation of this draft

is not RFC 7296 compliant, it's unclear whether there are other atypical

things that are also non-compliant but legal in this document, or whether

the only alternatives are to do exactly what RFC 7296 says.

5)      The security considerations section needs work.  Since this document

removes functionality that is mandatory in RFC 7296, it needs to discuss

the considerations resulting from lacking that support.  For example, if

that support was there to close security holes, then this document would

introduce new security vulnerabilities, or at least narrow the cases where

this is safe to use.  E.g., is a minimal implementation lacking that other

functionality dangerous to put on the public Internet?  Why or why not?

Editorial:

6)      Question: What if a doc comes along that Updates RFC 7296?   Is such a doc

expected to Update this document too or not?  How do we ensure that such

authors are aware of this document since this one doesn't Update RFC 7296

itself?

7)      The document needs a number of English grammar fixes throughout.

I propose editorial fixes using tracked changes in place in the PDF

above for most such cases where I could tell what was intended.
There were a number of cases where I could not tell what the authors
intended though, and so called those out with comments.

8)      A number of terms are used without definition or reference, at least

before they're used.  I've called out a number of such cases in the marked

up copy.  Examples include IKE_SA_INIT in section 1, "home area network

server" in section 1.1, "child SA" in section 2.1, etc.  If a reader is expected

to know these terms before reading this document, it would be good for

the Introduction to say what is expected pre-reading.   Another example

is the use of the term "server" which is unclear as to whether it's

synonymous with "responder", since the term "server" is undefined.

If they are synonymous, it would be better to use consistent terminology.

9)      [IPSECARCH] is cited on Page 12 but is not in the references section.

Dave

---

From: Dave Thaler <[email-masked]>
Date: Mon, 28 Sep 2015 19:57:39 +0000

> -----Original Message-----
> From: Tero Kivinen [mailto:[email-masked]]
> Sent: Monday, September 28, 2015 5:39 AM
> To: Dave Thaler <[email-masked]>
> Cc: [email-masked]; [email-masked]; draft-ietf-lwig-ikev2-
> [email-masked]; [email-masked]
> Subject: INT Dir review: draft-ietf-lwig-ikev2-minimal-03
> 
> Dave Thaler writes:
> > Possibly technical:
> >
> > 1) Page 5 discusses cryptographic algorithms but does not state, nor
> > reference any other document that states, what algorithm(s) are
> > mandatory to implement in a minimal implementation. Without that,
> > implementations may not interoperate.
> 
> In IPsec and IKE we have decided to specify the mandatory to implement
> algorithms in separate document, not in the base specification.
> 
> This way the mandatory to implement algorithms can be changed without need
> to modify the base specification.

Then you should cite the separate document.

> 
> As this document has same mandatory to implement algorithms as base IKEv2,
> it uses same documents for those. This means that for IKEv2 the mandatory to
> implement algorithms are specified in the RFC4307 and for IPsec they are
> specified in the RFC7321.
> 
> In practice most of the minimal implementations will implement whatever is
> usable in the use scenario where they are to be used. I.e common case is that
> the server end is full IKEv2 implementation and it will support wide range of
> algorithms. The minimal implementation will then implement some algorithm
> supported by the other end.
> 
> The RFC4307 is bit old and would need to be updated, as it still specifies as
> 3DES as only mandatory to implement encryption algorithm and 1024-bit
> MODP group as only mandatory to implement IKEv2 Diffie-Hellman Group.
> Both of those are bit questionable for the security point of view. The
> PRF_HMAC_SHA1 and AUTH_HMAC_SHA1_96 should still be ok.
> 
> I would expect minimal implementations reusing existing crypto algorithms
> they have in their system, for example the 802.15.4 hardware usually have
> hardware support for AES-CCM, so reusing that in the minimal IKE level would
> be smart and make the code smaller.
> 
> Anyways this document does not change what IKEv2 already specifies for the
> mandatory to use algorithms, and I want to keep it that way.
> 
> Updating the RFC4307 belongs to the IPsecME WG.
> 
> > 2) Page 6 talks about agreeing on a PRF. But it does not state, nor
> > reference anywhere else that states, how such is agreed upon or what
> > is mandatory to implement.
> 
> The cryptographic algorithms are negotiated by the IKE_SA_INIT exchange, i.e.
> the initiator sends SAi1 which "states the cryptographic algorithms the initiator
> supports for the IKE SA", and then the responder replies with SAr1 which
> "contains chosen cryptographic suite from initiator's offered choices.".
> 
> I.e. one of those cryptographic algorithms agreed during that exhcange is
> Transform Type 2, Pseudorandom Function (PRF). For more information of that
> see section A.3.2 for actual transforms and A.3 in general.

Then add a reference from page 6 to those appendix sections.  I.e. don't just 
answer me in email, put the answer into the relevant text.

> 
> > 3) I am not familiar with IKEv2 protocol details, but the second
> > paragraph of section 2.1 implies that only initiators send requests,
> > and only responders send responses, and only requests are
> > retransmitted.
> 
> Yes. Initiator == the one who initiated the exchange. There is also term called
> "original initiator" which specifies who was the original initiator of the
> IKE_SA_INIT which created the IKE SA.
> 
> > However, much of the rest of the document, including parts of section
> > 2.1, imply that initiators (especially minimal implementations of an
> > initiator) receive requests such as CREATE_CHILD_SA, etc. This
> > contradiction should be addressed.
> 
> In 2.1 the initiator = initiator of the IKE_SA_INIT and IKE_AUTH == minimal
> implementation.
> 
> The section 2.2 is wrong, as it should not contain the words "Initiator" and
> "Responder" in the figures, as in that case the Minimal implementation will be
> acting as responder, and the other end will act as initiator. I will change those
> to to be "Minimal implementation" and "Other end".
> 
> > 4) Several places talk about what a “typical” minimal implementation
> > would do, or what a minimal implementation would “usually” do.
> > However, it’s not clear what the alternative(s) are.  Since an
> > implementation of this draft is not RFC 7296 compliant, it’s unclear
> 
> Implementations of this draft is mostly RFC 7296 complient, only thing left out
> is the certificate authentication.
> 
> > whether there are other atypical things that are also non-compliant
> > but legal in this document, or whether the only alternatives are to do
> > exactly what RFC 7296 says.
> 
> The words "typical" and "usually" are used to specify which is the minimal way
> implementations can do and still be conforming to the RFC
> 7296 for that part. For example:
> 
>       As minimal implementation usually has only one host where it
>       connects, and that means it has only one shared secret. This
>       means it does not need to care about IDr payload that much. If
>       the other end sends AUTH payload which initiator can verify
>       using the shared secret it has, then it knows the other end is
>       the peer it was configured to talk to.
> 
> The text "usually has only one host" means that it is completely valid by
> RFC7296 to only have one host to which it can connect, there is no requirement
> in RFC7296 to be able to talk to multiple hosts at same time. And if minimal
> implementation can only talk to one host, it does not need to care about IDr, as
> it does not need that to identify to whom it is talking to, as it has only one host
> it can be talking to.
> If other end knows how to authenticate himself, that is enough.
> 
> On the other hand there are use cases where the minimal implementation is
> not so minimal anymore, but instead wants to talk to for example two devices
> instead of one. Then it cannot use this optimization.

Your answer cannot be found in the document.  I recommend adding text
framing the context.  Maybe once early on explaining how "usually/typically"
should be interpreted?
 
> > removes functionality that is mandatory in RFC 7296, it needs to
> > discuss the considerations resulting from lacking that support. For
> > example, if that support was there to close security holes, then this
> > document would introduce new security vulnerabilities, or at least
> > narrow the cases where this is safe to use. E.g., is a minimal
> > implementation lacking that other functionality dangerous to put on
> > the public Internet? Why or why not?
> 
> This document is conforming to the RFC7296, so it is inherits all same security
> properties from it. It uses one of the mandatory to implement authentication
> methods, which is already covered in the RFC7296 security considerations
> section. The RFC7296 did already consider pre-shared keys authentication
> being safe enough to be used in general case. This does not change this.
> 
> There are lots of implementations out there which do use pre-shared keys as
> their authentication method in the internet already, so this document does not
> change that. We do cover the case where shared secret authentication is not
> good enough, i.e. in the Appendix B.2 where we discuss about the raw public
> keys.
> 
> The RFC7296 security considerations section warns about pre-shared keys as
> follows:
> 
>    When using pre-shared keys, a critical consideration is how to assure
>    the randomness of these secrets.  The strongest practice is to ensure
>    that any pre-shared key contain as much randomness as the strongest
>    key being negotiated.  Deriving a shared secret from a password,
>    name, or other low-entropy source is not secure.  These sources are
>    subject to dictionary and social-engineering attacks, among others.

That doesn't address my comment at all.
If you have a device which is incapable of being configured with
certificate auth, then it cannot be used in any cases where pre-shared keys
are less secure (than certs) for whatever reason.   As such, you're 
potentially introducing new security considerations by precluding such
support in devices.  That needs to be addressed in the sec. considerations
section, it's not sufficient to just say it's "same as RFC 7296".

> 
> > Editorial:
> >
> > 6) Question: What if a doc comes along that Updates RFC 7296? Is such
> > a doc expected to Update this document too or not? How do we ensure
> > that such authors are aware of this document since this one doesn’t
> > Update RFC 7296 itself?
> 
> If document which updates 7296 changes something in such way that it is no
> longer conformant with 7296 (and to this document) then most likely this
> document would need to be updated too. On the other hand
> RFC4306 was updated to RFC5996 and that was updated to RFC7296, and only
> change that would have affected this document was the raw RSA support vs
> raw public key support in Appendix B.2.
> 
> IPsec community is quite small, and I do not expect there to be problems with
> authors being aware of this document...
>
> I will checked the pdf and made the editorial changes. Some of the text had
> already changed, so didn't apply all the changes directly.
> 
> You also commented several sentences which were taken directly from the
> RFC7296 (in section 2.1). I kept those as they were in the RFC7296.
> 
> Some replies:
> 
> DT4: Yes, it is intended to be able to understand those terms while you go
> foward. No need to read 7296 for those.
> 
> DT10, DT12: Both sentences are from 7296.
> 
> DT16: Directly from 7296. Reason being that IDi, IDr, TSi, and TSr, have
> separate payload numbers, but Ni and Nr have same number, i.e.
> the payload number for Ni and Nr are same.
> 
> DT17: From 7296. How they are generated or chosen is left to the
> implementations. Some use hashes, some, use some kind counters, some could
> even use the memory address of the IKE SA structure etc.
> 
> DT19: I only use the MUST/SHOULD etc when I get text directly from RFC7296,
> so I can be sure the meaning is same. Thats why I used "needs to" here as this
> text is not from 7296. Also if minimal implementation only supports one set of
> algorithms, there is actually no need to check whether other end accepted his
> proposal or not, as other end cannot trick him to use usecure algorithms, if he
> only supports one.
> 
> DT20: prf+ was defined few lines aboce in the figure.
> 
> DT21: I think Diffie-Hellman is known widely enough, and adding reference to
> paper from 1977 does not really help anybody.
> 
> DT22, DT23: From 7296.
> 
> DT24, DT25: Yes (left as they are).
> 
> DT28: If implemenation wants to have support for multiple algorithms, then
> they can implement that, and then they need to add support for that too.
> 
> DT31, DT32: may be ignored would also work, but I try to avoid 2119 language
> unless it comes from the 7296.
> 
> DT33: Most likely implementations will send that notify, as it does not cost
> them that much, but can help the server to clean up state. On the other hand if
> they are really minimal, then they might leave it out as it do cost some bytes to
> be transmitted, and few bytes to add that set of bytes to the message.
> 
> DT39: Sending some other error code, fully implementing CREATE_CHILD_SA
> etc.
> 
> DT40, 41: From 7296.
> 
> DT43: I hate the TOFU term. I do not trust the other end at the first use, I might
> be willing to accept the risk on first use, i.e. leap of fate describes that you are
> willing to take risk and assume other end is who it claims to be.
> 
> DT44: Baby duck will imprint to first being it sees. Not sure if there is any good
> reference for that. It is different from Leap of Fate, as it does not know who he
> is talking to, but just assumes that first one who answers is the one who he
> should be talking to. I do define it in the same sentence.
> 
> 
> > 8) A number of terms are used without definition or reference, at
> > least before they’re used. I’ve called out a number of such cases in
> > the marked up copy. Examples include IKE_SA_INIT in section 1, “home
> > area network server” in section 1.1, “child SA” in section 2.1, etc.
> > If a reader is expected to know these terms before reading this
> > document, it would be good for the Introduction to say what is
> > expected pre-reading.
> 
> You should be able to read this without reading anything else, but I agree that
> there are some cases where we first use term, and then come to define it only
> few paragraph below. Adding references to such cases would be unnecessarely
> at least for my point of view.
> 
> > Another example is the use of the term “server” which is unclear as to
> > whether it’s synonymous with “responder”, since the term “server”
> > is undefined. If they are synonymous, it would be better to use
> > consistent terminology.
> 
> Changed most of the uses of "server" in the section 2 to responder, as that is
> correct term. It is still used in Use cases, Implementation Status and Raw Public
> Keys sections
> 
> That text is direct copy from the RFC7296 and there are some other similar
> ones [IDNA], [EAI], and [PKCS1].
> 
> I could simply remove those references as I do not think they are needed for
> this document, but I left them in as they were in the RFC7296, and I did not
> want to modify that text I copied from there too much.
> --
> [email-masked]