Re: [Lwip] Review of draft-ietf-lwig-minimal-esp-00

[Daniel Migault <[e-mail]>]

Hi Valery,

Thank you very much for the review. Your review was very helpful to improve
the draft and we believe the new version has taken the reviews into
account. Please find a more detailed description of the updates led by your
reviews.

Yours,
 Daniel

On Tue, Dec 3, 2019 at 8:08 AM Valery Smyslov <[e-mail]>
wrote:

> Hi,
>
> I reviewed draft-ietf-lwig-minimal-esp-00. In general I think that the
> document
> provides useful guidelines on how ESP can be implemented on constrained
> devices.
>
> General comment: the draft uses RFC2119 requirement language in several
> places,
> and it is not always clear whether it is just a repetition of the RFC4301
> requirements
> or a new requirement imposed by this document. In general, I think it's
> better not to include
> the existing RFC4301/4303 requirements in the draft or make it absolutely
> clear that these are not
> new requirements if you need to mention them (adding a reference to a
> corresponding section
> in the RFCs).
>

<mglt>
We initially took this text as an editorial choice to quote or refer to
useful original text. I understand it brings some confusion - unless we
read it very carefully. I am tempted to agree that it might be clearer now
that we removed the quoted text. In fact it is just commented so it
would be easy to move back, if that would raise any concerns.

</mglt>

>
> Another general comment: sections 3-7 discuss how the corresponding ESP
> packet fields
> can be tweaked to deal with low resource devices. I think that for the
> sake of clarity
> the suggested measures must be summarized in each of these sections.
> Currently
> these sections contain quite a lot of discussion and no clear conclusions
> what
> is OK to do and in what situations. I think document will be more clear if
> such
> conclusions are put at the end of each section (currently some advises are
> spread
> over them).
>
> <mglt>
We tried to clarify the text. One difficulty is that recommendations can
easily change over the use cases.
For section 3, we mentioned that tweak applies to constraint devices for
which handling 32 bit random SPI would cause an issue. I think that is now
clearer. We also re-order some of the paragraphs so the discussion of a
specific use cases remains localized.
For section 4 we mentioned the SN that has an always increasing source may
take advantage of it. We clarified that other recommendations concern all
devices.
For section 5 - 6 -  7 seems relatively generic.
If you believe we could improve this, feel free to let us know.

</mglt>

>
> Specific comments:
>
> Section 3.
>
>    When fix SPI are used,
>    it is RECOMMENDED the constraint node has as many SPI values as ESP
>    session per host IP address, and that SA lookup includes the IP
>    addresses.
>
> This is probably wrong if we take into considerations that SA may be
> rekeyed.
> Some words should be added either prohibiting rekeying ESP SAs in this case
> or discussing that in case of rekey one will consume additional SPI values
> for in fact the same SA.
>
> <mglt>
I agree. We now clearly mention that the number of SPI a peer needs to
handle depends on the number of session and the rekying of these SA. We
also moved the discussion of fixed SPIs to something more generic in order
to provide more generic guidances. We reinforced also the need to rekey -
and managed the keys in the security consideration section.


SPI section has been updated as follows:

"""
  However, for some constrained nodes, generating and handling 32 bit
   random SPI may consume too much resource, in which case SPI can be
   generated using predictable functions or end up in a using a subset
   of the possible values for SPI.  In fact, the SPI does not
   necessarily need to be randomly generated.  A node provisioned with
   keys by a third party - e.g. that does not generate them - and that
   uses a transform that does not needs random data may not have such
   random generators.  However, non random SPI and restricting their
   possible values MAY lead to privacy and security concerns.  As a
   result, this alternative should be considered for devices that would
   be strongly impacted by the generation of a random SPI and after
   understanding the privacy and security impact of generating non
   random SPI.

   When a constrained node limits the number of possible SPIs this limit
   should both consider the number of inbound SAs - possibly per IP
   addresses - as well as the ability for the node to rekey.  SPI can
   typically be used to proceed to clean key update and the SPI value
   may be used to indicate which key is being used.  This can typically
   be implemented by a SPI being encoded with the SAD entry on a subset
   of bytes (for example 3 bytes), while the remaining byte is left to
   indicate the rekey index.
"""
The security consideration has been updated as follows:

"""
  The security of a communication provided by ESP is closely related to
   the security associated to the management of that key.  This usually
   include mechanisms to prevent a nonce to repeat for example.  When a
   node is provisioned with a session key that is used across reboot,
   the implementer MUST ensure that the mechanisms put in place remain
   valid across reboot as well.

   It is RECOMMENDED to use ESP in conjunction of key management
   protocols such as for example IKEv2 [RFC7296] or minimal IKEv2
   [RFC7815].  Such mechanisms are responsible to negotiate fresh
   session keys as well as prevent a session key being use beyond its
   life time.  When such mechanisms cannot be implemented and the
   session key is, for example, provisioned, the nodes SHOULD ensure
   that keys are not used beyond their life time and that the
   appropriated use of the key remains across reboots.

   When a node generates its key or when random value such as nonces are
   generated, the random generation MUST follow [RFC4086].
"""

</mglt>

>    When used indoor, the privacy information is stored in the encrypted
>    data and as such does not leak privacy.
>
> I cannot parse this :-)
>
> <mglt>
Not entirely sure what was the initial intent, but the current text
mentions that privacy implication should also consider the exposure of the
communication. Typically, an indoor sensor with local IP address may leak
little privacy sensitive information with its SPI. Though this is not
entirely no information. The current text is now:

"""
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, privacy leakage outside the local network may be
   limited.
"""

</mglt>

>     Such packet will not be rejected due to an SPI mismatch, but instead
>    after the signature check which requires more resource and thus make
>    DoS more efficient, especially for devices powered by batteries.
>
> I think this a very good argument against fixed (predictable) SPIs.
> In fact, after reading through this section it seems to me that the
> conclusion must be - predictable (fixed) SPIs SHOULD NOT be used.
>
> <mglt>
The current text does not mention fixed SPI anymore, but considers SPIs
that are non randomly generated.
We also clarify the text to say. Generate randomly the SPI when you can.
Only if you really cannot consider the alternative. The reason I would not
like to have SHOULD NOT is that it is still better to have IPsec with non
randomly generated SPIs than no security. That said, we do not want to
encourage non randomly generated SPIs.
</mglt>

   Values 0-255 SHOULD NOT be used.
>
> I believe these values MUST NOT be used with IPsec ESP, no?
> Why "SHOULD NOT"? The values from this range are reserved
> for other protocols utilizing ESP (e.g. SPI (1) was used in SKIP).
>
> <mglt>
This has been changed as well. Originally we had SHOULD NOT as not to
prevent future usage with the reserved values. We set it to MUST NOT.
</mglt>

> Section 4.
>
> I see no discussion regarding using ESN. I think there is generally no
> point
> to use ESN for constrained devices, but it can be useful if clock is used
> to generate (E)SN, as suggested in the draft. In this case 64-bit numbers
> can make sure that no two packets will be sent with the same ESN
> (provided the clock has high resolution).
>
> <mglt>
This is correct that the former version did not provide indications on
the use of ESN. This has been added in the new version. It is hard to
recommend one or the other but what we would like is to emphasize is that
incrementing the life time of the session with ESN is generally a bad idea
and that providing a rekey mechanism is probably a better option. here is
the text we come to:

"""
 SN can be encoded over 32 bits or 64 bits - known as Extended
   Sequence Number (ESN).  As per [RFC4303], the support ESN is not



Migault & Guggemos         Expires May 1, 2021                  [Page 6]
^L
Internet-Draft                 Minimal ESP                  October 2020


   mandatory.  The determination of the use of ESN is based on the
   largest possible value a SN can take over a session.  When SN is
   incremented for each packet, the number of packets sent over the life
   time of a session may be considered.  However, when the SN is
   incremented differently - such as when time is used - the maximum
   value SN needs to be considered instead.  Note that the limit of
   messages being sent is primary determined by the security associated
   to the key rather than the SN.  The security of the key used to
   encrypt decreases with the each message being sent and a node MUST
   ensure the limit is not reached - even though the SN would permit it.
   In a constrained environment, it is likely that the implementation of a
   rekey mechanism is preferred over the use of ESN.
"""


</mglt>


> Section 5.
>
>    The purpose of padding is to respect the 32 bit alignment of ESP.
>
> It is not an accurate statement, the padding is also used when encryption
> transform requires the input to be a multiple of some number of bytes.
> Many modern transforms (based on GCM, CCM, CTR modes) don't have such
> a requirement, but some (e.g. based on CBC mode) do have.
>
> <mglt>
Correct. We fixed with the following text:

"""
 The purpose of padding is to respect the 32 bit alignment of ESP or block
size expected by an encryption transform - such as AES-CBC for example.
"""
</mglt>

> Section 6.
>
>    For interoperability, it is RECOMMENDED a minimal ESP
>    implementation discards dummy packets.
>
> I'm puzzled by this sentence. What else can the receiver do with dummy
> packets other than discard? RFC4303 leaves him only this option :-)
>
> <mglt>
Your are correct. My main concern when writing these lines was that the
implementation handled properly the dummy packets. In a word, do not get
upset by them. One case I am thinking of is when ESP is used in transport
mode an IP communication believes a dummy packet should be discarded rather
than sent to the next transport layer. In tunnel mode things seem different
as the next header is likely an IP header in which case everything else
should be discarded.

</mglt>

>
> Nits:
> Throughout the text:
> s/fix SPI/fixed SPI
>
<mglt>
Actually we removed the fixed SPI.
</mglt>

> s/constraint node/constrained node
> s/algorythm/algorithm
>
<mglt>
fixed
</mglt>


>
> Section 2:
>
> s/IPsec suite protocol/IPsec protocol suite
>
> <mglt>
fixed
</mglt>

> Section 3:
>
>    However, for some constraint nodes, generating a random SPI may
>    consume to much resource, in which case SPI can be generated using
>    predictable functions or even a fix value.
>
> s/to/too
>
> <mglt>
fixed
</mglt>

>    When a constraint node uses fix value for SPIs, it imposes some
>    limitations on the number of inbound SA.  This limitation can be
>    alleviate by how the SA look up is performed.  When fix SPI are used,
>    it is RECOMMENDED the constraint node has as many SPI values as ESP
>    session per host IP address, and that SA lookup includes the IP
>    addresses.
>
> s/alleviate/alleviated
> s/RECOMMENDED the/RECOMMENDED that the
>
>    More specifically, traffic pattern MAY leak sufficient information in
>    itself.  In other words, privacy leakage is a complex and the use of
>    random SPI is unlikely to be sufficient.
>
> s/MAY/may
>
>    In addition,
>    predictable SPI enable an attacker to forge packets with a valid SPI.
>
> s/enable/enables
>
> <mglt>
fixed
</mglt>

> Section 5:
>
>    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.
>
> s/cannot not/cannot
> s/minimal/minimal ESP
> s/were relying/rely
>
> Section 7:
>
>    Currently recommended
>    [RFC8221] only recommend crypto-suites with an ICV which makes the
>    ICV a mandatory field.
>
> s/recommend/recommends
>
> <mglt>
fixed
</mglt>

> Section 8:
>
>    The recommended suites to use are expect to evolve over time
>    and implementer SHOULD follow the recommendations provided by
>    [RFC8221] and updates.
>
> s/expect/expected
> s/implementer/implementers
>
> <mglt>
fixed
</mglt>


>        Note that it
>        is not because a encryption algorithm transform is widely
>        deployed that is secured.
>
> s/a/an
>
<mglt>
fixed
</mglt>

>
> Regards,
> Valery Smyslov.
>
>
>

-- 
Daniel Migault
Ericsson