Hi Paul, 

Thanks for the review. From your response [1] I understand we can proceed to 
the adoption of the document if no changes are performed on ESP. This condition 
is fully stated in the abstract [2] as well as in the charter [3].  As a 
result, I believe the draft is ready for adoption. Of course we welcome further 
reviews!

Please find inline my responses to the current review. If the changes address 
your concern, I am happy to update the draft with the proposed text. I am happy 
to take more reviews as you volunteered to provided further reviews. 😉

For full transparency, my understanding of the current reviews is that they 
will result in minor -- thought useful -- changes to the current draft. The 
concerns you expressed are in my opinion mostly due to a confusion with another 
draft "ESP Header Compression and Diet-ESP" (draft-mglt-ipsecme-diet-esp). This 
draft is discussed in ipsecme not in lwig and so are sort of out of scope. All 
comments are answered below.

Yours, 
Daniel

[1]
"""
If the document is defining a minimum/battery optimized ESP configuartion, I 
have no problems with it and I will review further text and welcome adoption. 
If it makes changes to the ESP protocol, then I think there should be more 
discussion before adoption.
"""

[2]
"""
  This document describes a minimal implementation of the IP
   Encapsulation Security Payload (ESP) defined in RFC 4303.  
[...]
  This document does not update or modify RFC 4303, but provides a
   compact description of how to implement the minimal version of the
   protocol.  If this document and RFC 4303 conflicts then RFC 4303 is
   the authoritative description.
"""

[3]
"""
"LWIG is chartered to provide guidance on lightweight implementations. 
Almost all drafts in this WG are informational in nature and we don't intend 
to change existing standards or define new ones.
"""

[4] https://tools.ietf.org/html/draft-mglt-ipsecme-diet-esp-06

-----Original Message-----
From: Paul Wouters <[email protected]> 
Sent: Wednesday, February 13, 2019 1:09 PM
To: Daniel Migault <[email protected]>
Cc: Mohit Sethi M <[email protected]>; Tero Kivinen <[email protected]>; 
Heinrich Singh <[email protected]>; [email protected]
Subject: Re: [Lwip] [IPsec] The LWIG WG has placed draft-mglt-lwig-minimal-esp 
in state "Call For Adoption By WG Issued"

On Tue, 12 Feb 2019, Daniel Migault wrote:

> I am wondering what is currently the status of the draft. Feel free to let me 
> know if I something is expected on my side.

I cannot answer that question, I'll leave that to the chairs, but I do have 
several strong reservations on the document. Especiallt with the complex 
framework to reduce a number of bytes.

<mglt>
I understand that the strong reservations concern the modification of ESP as 
well as the compression framework. Such reservations do not apply here. I 
believe this statement is clearly mentioned in the abstract [1] as well as in 
the lwig charter[2].  I think what you're referring to is another draft in 
ipsecme "ESP Header Compression" (draft-mglt-ipsecme-diet-esp) [3] which does 
not apply here.  We think the abstract of the document states that clearly, but 
we're happy to state this point stronger if you feel that's necessary.

To be clear:
* draft-mglt-lwig-minimal-esp does not modify ESP
* draft-mglt-lwig-minimal-esp  does not reduce any bytes of ESP
* draft-mglt-lwig-minimal-esp does not describe any framework.

[1]
"""
  This document describes a minimal implementation of the IP
   Encapsulation Security Payload (ESP) defined in RFC 4303.
[...]
  This document does not update or modify RFC 4303, but provides a
   compact description of how to implement the minimal version of the
   protocol.  If this document and RFC 4303 conflicts then RFC 4303 is
   the authoritative description.
"""

[2]
"""
If the document is defining a minimum/battery optimized ESP configuartion, I 
have no problems with it and I will review further text and welcome adoption. 
If it makes changes to the ESP protocol, then I think there should be more 
discussion before adoption.
"""

[3] https://tools.ietf.org/html/draft-mglt-ipsecme-diet-esp-06
</mglt>

In section 3 there is a discussion about not using randomness for a SPI.
It argues why this is not needed. My issue is that there I can see three ways 
of a system needing to generate an IPsec SPI:

1) It has an IKEv2 stack
2) It has nother (eg 3GPP??) protocol that does not provide a SPI, but
    handles all other keying.
3) Manual keying

In the case of 1) the implementation clearly needs to generate randomness 
anyway, at least for the IKE SPIs which really must be strong random. (It saved 
us from an IKEv1 attack 2 years ago!)

In the case of 2), why isn't the other protocol dictating the SPI?

As for case 3), well manual keying is strongly discouraged outside of using a 
keying protocol that provides the same safeguards as IKE.

So I don't know which implementations would actually qualify for using 
non-random static like SPI's

Perhaps you can clarify this with some use cases where this would apply?

<mglt>
I believe the current text [4] already provides some use cases and obviously we 
do not expect these device to have IKEv2 or keying negotiation facilities 
implemented. Again, the use of ESP does not mandate IKEv2 or equivalent even 
though this is recommended strongly. We believe that very constraint device 
would benefit from having fixed SPI rather than no security. The current draft 
documents how such this category of device can secure their communications.

Note also that mandating random SPI would represent a change in the ESP 
specification which is again not intended by the current document nor the 
charter. In addition, I believe the text is clear enough that random SPI is 
recommended whenever it is possible and that non random SPI are reserved for 
very constraint nodes. If the text does not provide this impression, feel free 
to let us know how to make it clearer.

[4]
"""
   As the general recommendation is to randomly generate the SPI,
   constraint device that will use a limited number of fix SPI are
   expected to be very constraint devices with very limited
   capabilities, where the use of randomly generated SPI may prevent
   them to implement IPsec.  In this case the ability to provision non
   random SPI enables these devices to secure their communications.
   These devices, due to there limitations, are expected to provide
   limited information and how the use of non random SPI impacts privacy
   requires further analysis.  Typically temperature sensors, wind
   sensors, used outdoor do not leak privacy sensitive information.
   When used indoor, the privacy information is stored in the encrypted
   data and as such does not leak privacy.
"""
</mglt>

Section 4 is about the sequence number. While in principle i see nothing wrong 
with relaing the requirement for increasing by 1 and using another source for 
increasing numbers, I do find the reasoning weak. For example, with this 
counter you would also check the number of bytes encrypted with that key to 
ensure you are not encrypting more than 2^20 or 2^32 packets. While I guess 
these devices likely wouldn't ever get there, shouldn't there still be a formal 
check in the code to prevent this anyway?

<mglt>
The document does not provide any recommendations on how to handle the SN. It 
just states that generally the counter is incremented by one but other means 
may be used. 

"""
  Usually, SN is generated by incrementing a counter for each packet
   sent.  A constraint device may avoid maintaining this context and use
   another source that is known to always increase.  
"""

The comment provides an additional example of counter that could be re-used. I 
propose to add that example at the end of the paragraph with the following 
sentence.

""" Similarly, the SN make be handled as a counter used for other purposes. For 
example, an implementation may also use the number of bytes encrypted as the SN 
in order to enforce the management of the key lifetime of the key. The SN may 
also be used to generate the IV when AES-CTR [RFC3686] AES-GCM [RFC4106], 
AES-CCM [RFC4309] or Chacha20-Poly1035 [RFC7634] is used. Note that in this 
later both SN and IV are sent explicitly in the ESP packet. 
[draft-ipsecme-implicit-iv] defines transforms where the IV is derived from the 
SN, which avoids the IV to be sent on the wire.    
"""

You are correct that this parameter is already part of the SA and for device 
with no key management facilities, we hardly expect them to reach this limit 
nor to be able to rekey. 
</mglt>

Additionally, if resources matter that much, you are using AES-GCM or 
CHACHA20POLY1305, meaning you need a counter anyway. So you can just re-use 
that one anyway. So I'm not convinced this is actually a needed use case.

Section 5, while TFC deployment might be used a lot (yet?), it is part of many 
stacks, so the claim about not being widely adopted for standard ESP traffic is 
partially true.

I am not sure what you are trying to say with this paragraph:

    As a result, TFC cannot not be enabled with minimal, and
    communication protection that were relying on TFC will be more
    sensitive to traffic shaping.  This could expose the application as
    well as the devices used to a passive monitoring attacker.  Such
    information could be used by the attacker in case a vulnerability is
    disclosed on the specific device.  In addition, some application use
    - such as health applications - may also reveal important privacy
    oriented informations.

Are you suggesting to obsolete TFC ?

<mglt>
No. we are saying that not enabling TFC with minimal implementation is correct. 
TFC is hard to manage and devices that are looking at minimizing the number of 
bytes sent over the air will not enabled it.  The sentence has a nit and should 
be:

OLD:
    As a result, TFC cannot not be enabled with minimal, and

NEW:
    As a result, TFC is not enabled with minimal ESP, and

</mglt>

Section 5 does not actually propose a change that I can see. It implies padding 
support and any and all kind of padding should be able to be omited?

<mglt>
To remain compliant padding is performed in a given way and other paddings are 
accepted. This is to remain compliant with ESP and avoid minimal ESP 
implementation to perform padding in alternative ways as defined by ESP. 
</mglt>

Similarly, section 6 does not seem to propose a change to strip out the Next 
Header, though it is suggesting it. I think BEET is not a real consideration, 
as I dont think many implementations support it and I don't know anyone using 
it? But I'm not convinced this 1 byte is really a goal we should consider.

<mglt>
Stripping the next header would change ESP. This is out of scope of the draft 
and have no intention to suggest this. I do not find any such suggestion in the 
text. Please let us know what make you think so, so that we clarify this.
</mglt>

This sentence is confusing:

        ESP can be used to authenticate only or to encrypt the communication.

Since IPsec-v2 allowed ESP without authentication, and IPsec-v3 only has 
authenticated ESP. It's better to say ESP allows null-encryption and not 
mention authentication (which always happens)

<mglt>
I would argue the current text is clearer than the proposed text, but I am 
happy to change it. I believe this is clearer to say that we are willing to 
authenticate the communications than to say we are encrypting it with the NULL 
encryption. Null encryption is the way to performed authentication only - not 
the intent. The following sentence insists also on providing authentication all 
the time. Again English is not my natural language and I am happy to be told 
otherwise. 

"""
      ESP can
       be used to authenticate only or to encrypt the communication.  In
       the later case, authenticated encryption must always be
       considered [RFC8221].
"""
</mglt>

It talks about "Cipher Suites" which is really a TLS term.

<mglt>
We can change "Cipher Suites" to a more appropriated term. Would you suggest 
instead the use of Encryption Algorithm Transform ? 
</mglt>

        For example if the device
        benefits from AES hardware modules and uses AES-CTR, it may
        prefer AUTH_AES-XCBC for its authentication.

Note that AES-XCBC is not a FIPS approves integrity algorithm :) But more 
importantly, you do not want a non-AEAD at all if battery usage is so 
important. use AES-CCM or AES-GCM or when not having AES HW support, 
chacha20-poly1305.

<mglt>
The idea was to replace the usage of hashing functions by a function using HW 
support. However, the draft should not provide specific algorithms and instead 
rely on specific recommendations (RFC 8221) that will be updated over time. If 
following the up-to-date cryptographic recommendations is not clear, please 
feel free to let us know how to clarify this.
</mglt>



All in all, I think the document should more clearly seperate the issues of a 
minimal ESP implementation, and any proposed modifications to ESP.
And if that is done, the protocol shouldn't be ESP but something new, unless it 
is completely backwards compatible (like IPsec-v2 to->
IPsec-v3 was)
<mglt>
We are proposing NO CHANGES to ESP. We are then completely backwards compatible 
as well. I believe this has been clearly stated in the abstract. 
</mglt>

If the document is defining a minimum/battery optimized ESP configuartion, I 
have no problems with it and I will review further text and welcome adoption. 
If it makes changes to the ESP protocol, then I think there should be more 
discussion before adoption.

<mglt>
There is no changes to ESP. Any change to ESP would have been discussed in 
ipsecme. We more welcome your feed back to improve the text.. 
</mglt>

Paul
_______________________________________________
Lwip mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/lwip

Reply via email to