[lisp] AD Review of draft-ietf-lisp-sec-25

[Alvaro Retana <[e-mail]>]

Dear authors:

Thank you for the work on this document!

I put detailed comments inline below.

As we have a constrained timeline for this document, I would like to
start the IETF Last-Call in no more than a couple of weeks.  I don't
think my comments will be hard to address.  In fact, there are
comments that I repeat in different sections, and some overlap.

Please reply inline to this message to expedite my review of any
updates.  Also, if you think talking would clear things up faster,
feel free to find time on my calendar:
https://doodle.com/mm/alvaroretana/book-a-time

Thanks!

Alvaro.


[Line numbers from idnits.]

...
15	Abstract

17	   This memo specifies LISP-SEC, a set of security mechanisms that
18	   provides origin authentication, integrity and anti-replay protection
19	   to LISP's EID-to-RLOC mapping data conveyed via mapping lookup
20	   process.  LISP-SEC also enables verification of authorization on EID-
21	   prefix claims in Map-Reply messages.

[nit] s/via mapping lookup process/via the mapping lookup process/g


...
100	1.  Introduction
...
120	   This memo specifies LISP-SEC, a set of security mechanisms that
121	   provides origin authentication, integrity and anti-replay protection
122	   to LISP's EID-to-RLOC mapping data conveyed via mapping lookup
123	   process.  LISP-SEC also enables verification of authorization on EID-
124	   prefix claims in Map-Reply messages, ensuring that the sender of a
125	   Map-Reply that provides the location for a given EID-prefix is
126	   entitled to do so according to the EID prefix registered in the
127	   associated Map-Server.  Map-Register/Map-Notify security, including
128	   the right for a LISP entity to register an EID-prefix or to claim
129	   presence at an RLOC, is out of the scope of LISP-SEC as those
130	   protocols are protected by the security mechanisms specified in
131	   [I-D.ietf-lisp-rfc6833bis].  However, LISP-SEC extends the Map-
132	   Register message to allow an ITR to securely downgrade to non LISP-
133	   SEC Map-Requests.  Additional security considerations are described
134	   in Section 6.

[major] "securely downgrade to non LISP-SEC Map-Requests"

To "securely downgrade" to no security doesn't sound right to me.  See
more comments in §6.7.


...
176	4.  LISP-SEC Threat Model

178	   LISP-SEC addresses the control plane threats, described in section
179	   3.7 and 3.8 of [RFC7835], that target EID-to-RLOC mappings, including
180	   manipulations of Map-Request and Map-Reply messages, and malicious
181	   ETR EID prefix overclaiming.  LISP-SEC makes two main assumptions:
182	   (1) the LISP mapping system is expected to deliver a Map-Request
183	   message to their intended destination ETR as identified by the EID,
184	   and (2) no man-in-the-middle (MITM) attack can be mounted within the
185	   LISP Mapping System.  How the Mapping System is protected from MITM
186	   attacks depends from the particular Mapping System used, and is out
187	   of the scope of this memo.  Furthermore, while LISP-SEC enables
188	   detection of EID prefix overclaiming attacks, it assumes that Map-
189	   Servers can verify the EID prefix authorization at registration time.

[] As part of the efforts to make the language in IETF documents more
inclusive, consider using "on-path attack" instead of MITM.  This term
in already in use in some parts of this document.

https://www.ietf.org/about/groups/iesg/statements/on-inclusive-language/


...
202	5.  Protocol Operations
...
210	   LISP-SEC builds on top of the security mechanisms defined in
211	   [I-D.ietf-lisp-rfc6833bis] to address the threats described in
212	   Section 4 by leveraging the trust relationships existing among the
213	   LISP entities participating to the exchange of the Map-Request/Map-
214	   Reply messages.  Those trust relationships are used to securely
215	   distribute, as described in Section 8.4, a per-message One-Time Key
216	   (OTK) that provides origin authentication, integrity and anti-replay
217	   protection to mapping data conveyed via the mapping lookup process,
218	   and that effectively prevent overclaiming attacks.  The processing of
219	   security parameters during the Map-Request/Map-Reply exchange is as
220	   follows:

[nit] s/participating to/participating in


...
245	   1.  The ITR, upon needing to transmit a Map-Request message,
246	       generates and stores an OTK (ITR-OTK).  This ITR-OTK is included
247	       into the Encapsulated Control Message (ECM) that contains the
248	       Map-Request sent to the Map-Resolver.  ITR-OTK confidentiality
249	       and integrity protection MUST be provided in the path between the
250	       ITR and the Map-Resolver.  This can be achieved either by
251	       encrypting the ITR-OTK with the pre-shared secret known to the
252	       ITR and the Map-Resolver (as specified in Section 6.5), or by
253	       enabling DTLS between the ITR and the Map-Resolver.

[major] "protection MUST be provided"

Please specify things only once.  In this case, because this section
just "describes the flow", there's no need to specify anything in it,
or go into some of the details.  The protection part is properly
covered later in the document and is not necessary to be called out at
this point.


255	   2.  The Map-Resolver decapsulates the ECM message, decrypts the ITR-
256	       OTK, if needed, and forwards through the Mapping System the
257	       received Map-Request and the ITR-OTK, as part of a new ECM
258	       message.  The LISP Mapping System delivers the ECM to the
259	       appropriate Map-Server, as identified by the EID destination
260	       address of the Map-Request.  As mentioned in Section 4, how the
261	       Mapping System is protected from MITM attacks depends from the
262	       particular Mapping Systems used, and is out of the scope of this
263	       memo.

[minor] Anything "as mentioned in" can be left out of this description
to avoid duplication.


...
275	   4.  The Map-Server derives a new OTK, the MS-OTK, by applying a Key
276	       Derivation Function (KDF) to the ITR-OTK.  This MS-OTK is
277	       included in the Encapsulated Control Message that the Map-Server
278	       uses to forward the Map-Request to the ETR.  MS-OTK
279	       confidentiality and integrity protection MUST be provided in the
280	       path between the Map-Server and the ETR.  This can be achieved
281	       either by encrypting the MS-OTK with the pre-shared secret known
282	       to the Map-Server and the ETR (as specified in Section 6.5), or
283	       by enabling DTLS between the Map-Server and the ETR.

[major] "protection MUST be provided":  Same comment as above: no need
for this part here.


...
303	   8.  The ITR, upon receiving the Map-Reply, uses the locally stored
304	       ITR-OTK to verify the integrity of the EID-prefix authorization
305	       data included in the Map-Reply by the Map-Server.  The ITR
306	       computes the MS-OTK by applying the same KDF (as specified in the
307	       ECM encapsulated Map-Reply) used by the Map-Server, and verifies
308	       the integrity of the Map-Reply.  If the integrity checks fail,
309	       the Map-Reply MUST be discarded.  Also, if the EID-prefixes
310	       claimed by the ETR in the Map-Reply are not equal or more
311	       specific than the EID-prefix authorization data inserted by the
312	       Map-Server, the ITR MUST discard the Map-Reply.

[major] "...MUST..."

These details are not needed here.


...
322	6.1.  Encapsulated Control Message LISP-SEC Extensions
...
358	      V: Key Version bit.  This bit is toggled when the sender switches
359	      to a new OTK wrapping key

[] I don't understand how this works.  If the OTK doesn't change then
this bit's value shouldn't change, is that it?

[major - If so..]  If so, what should a receiver do if the V bit
didn't change but the OTK did?  What if the OTK didn't change, but the
V bit did?


...
363	      Requested HMAC ID: The HMAC algorithm, that will be used to
364	      protect the mappings, requested by the ITR.  See Section 6.4 for
365	      details, and Section 8.3 for HMAC IDs that MUST be supported.

[major] §8.3 says this:

   AUTH-HMAC-SHA-1-96 MUST be supported, AUTH-HMAC-SHA-256-128 SHOULD be
   supported.

However, (1) it is not good practice to include specifications in the
IANA Considerations section (only instructions to IANA should be
included there), and, (2) the MUST/SHOULD combination doesn't match
the MUST used here.

Instead, please move the text (above + references to rfc2104/rfc6234)
from §8.3 to §6.4, and eliminate the reference to §8.3.

Note that similar text is used in multiple places.  Please update all.


...
377	      OTK Wrapping ID: The identifier of the key derivation function and
378	      of the key wrapping algorithm used to encrypt the One-Time-Key.
379	      See Section 6.5 for more details, and Section 8.4 for Wrapping IDs
380	      that MUST be supported.

[minor] The figure uses ("OTK Wrap. ID").  I know that's just an
abbreviation, but please include it here for completeness:

s/OTK Wrapping ID:/OTK Wrapping ID (OTK Wrap. ID):


[major] "and Section 8.4 for Wrapping IDs that MUST be supported."

Same comment as above: please move the text from §8.4 to §6.5.

BTW, §6.5 already has come text about requiring
AES-KEY-WRAP-128+HKDF-SHA256, but not NULL-KEY-WRAP-128: this last
algorithm is mentioned, but the text doesn't require it.

Also, AES-KEY-WRAP-128 (without "+HKDF-SHA256") is mentioned
separately, but not listed in the table in §8.4.


382	      One-Time-Key Preamble: set to 0 if the OTK is not encrypted.  When
383	      the OTK is encrypted, this field MAY carry additional metadata
384	      resulting from the key wrapping operation.  When a 128-bit OTK is
385	      sent unencrypted by Map-Resolver, the OTK Preamble is set to
386	      0x0000000000000000 (64 bits).  See Section 6.5.1 for details.

[nit] s/by Map-Resolver/by a Map-Resolver


...
391	      EID-AD Length: length (in bytes) of the EID Authentication Data
392	      (EID-AD).  The ITR MUST set EID-AD Length to 4 bytes, as it only
393	      fills the KDF ID field, and all the remaining fields part of the
394	      EID-AD are not present.  An EID-AD MAY contain multiple EID-
395	      records.  Each EID-record is 4-byte long plus the length of the
396	      AFI-encoded EID-prefix.

[nit] s/set EID-AD Length/set the EID-AD Length


[major] "ITR MUST set EID-AD Length to 4 bytes"

What should the receiver do if the length is set to anything else?

For messages not originated by the ITR, the length has to be more then
4.  In fact, it has to be 12 + the length of EID-prefix (multiple may
be present) + length of EID HMAC.  What should a receiver do if the
length is not appropriate?

I don't remember seeing anything in rfc6833bis about error handling or
what to do about mismatched lengths in general.  Did I miss it?


398	      KDF ID: Identifier of the Key Derivation Function used to derive
399	      the MS-OTK.  The ITR MAY use this field to indicate the
400	      recommended KDF algorithm, according to local policy.  The Map-
401	      Server can overwrite the KDF ID if it does not support the KDF ID
402	      recommended by the ITR.  See Section 5.4 for more details, and
403	      Section 8.5 for KDF IDs that MUST be supported.

[major] "Map-Server can overwrite the KDF ID if it does not support
the KDF ID recommended by the ITR"

Specify things only once.  The text in §6.7.1 is similar.  See comments there.


[minor] s/5.4/6.4/g
§5.4 doesn't exist.


[major] "Section 8.5 for KDF IDs that MUST be supported"

Same comment as above: please move the text from §8.5 to §6.4.


405	      Record Count: The number of records in this Map-Request message.
406	      A record is comprised of the portion of the packet that is labeled
407	      'Rec' above and occurs the number of times equal to Record Count.

[major] Should there at least be one, or is it ok if the value is 0?
If at least one, what should a receiver do if no records are included?


409	      E: ETR-Cant-Sign bit.  This bit is set to 1 to signal to the ITR
410	      that at least one of the ETRs authoritative for the EID prefixes
411	      of this Map-Reply has not enabled LISP-SEC.  This allows the ITR
412	      to securely downgrade to non LISP-SEC requests, as specified in
413	      Section 6.7, if so desired.

[major] If I understand correctly, this bit should only ever be set by
a Map-Server.  Are there other cases where setting it would be valid?

If this bit is set other than by a Map-Server, what should the receiver do.

Assuming my understanding is correct, please say something here about
the Map-Server being the only one that can set the bit.  §6.7 is about
Map-Server processing, but the fact that it can set the bit doesn't
explicitly preclude other from doing so.


[minor] s/This allows the ITR to securely downgrade to non LISP-SEC
requests, as specified in Section 6.7, if so desired./See Section 6.7
for more details.


...
417	      EID HMAC ID: Identifier of the HMAC algorithm used to protect the
418	      integrity of the EID-AD.  This field is filled by Map-Server that
419	      computed the EID-prefix HMAC.  See Section 5.4 for more details,
420	      and Section 8.3 for HMAC IDs that MUST be supported.

[nit] s/by Map-Server/by the Map-Server


[major] "Section 8.3 for HMAC IDs that MUST be supported"

Same comment as above: please move the text from §8.3 to §6.4.


422	      EID mask-len: Mask length for EID-prefix.

424	      EID-AFI: Address family of EID-prefix according to [AFN].

426	      EID-prefix: The Map-Server uses this field to specify the EID-
427	      prefix that the destination ETR is authoritative for, and is the
428	      longest match for the requested EID.

[major] These 3 fields, which are part of the "Rec", are the same as
what is specified in §5.2/rfc6833bis.  But the descriptions are
different, and the EID-AFI names doesn't match EID-Prefix-AFI.  Please
point at the definitions in rfc6833bis:

   EID mask-len: As defined in §5.2/rfc6833bis.
   ...


430	      EID HMAC: HMAC of the EID-AD computed and inserted by Map-Server.
431	      Before computing the HMAC operation the EID HMAC field MUST be set
432	      to 0.  The HMAC MUST cover the entire EID-AD.

[major] s/by Map-Server/by a Map-Server


[major] "Before computing the HMAC operation the EID HMAC field MUST
be set to 0.  The HMAC MUST cover the entire EID-AD."

§6.7.1 describes the same operation, but without using Normative language:

   The scope of the HMAC operation covers the entire EID-AD, from the
   EID-AD Length field to the EID HMAC field, which must be set to 0
   before the computation.

Please specify things only once!  It seems to me that it may be more
appropriate to include the specification in §6.7.1 (Generating a
LISP-SEC Protected Encapsulated Map-Request).


434	6.2.  Map-Reply LISP-SEC Extensions
...
464	      MR AD Type: 1 (LISP-SEC Authentication Data).  See Section 8.

[] Just wondering...  Why are separate registries used for ECM AD Type
and MR AD Type?  Are you expecting so many extensions that a single
space won't be enough?


466	      EID-AD Length: length (in bytes) of the EID-AD.  An EID-AD MAY
467	      contain multiple EID-records.  Each EID-record is 4-byte long plus
468	      the length of the AFI-encoded EID-prefix.

[] It looks like you're mostly redefining the same fields as in the
last section, at least for the EID-AD and Rec.  Please point at the
definitions there instead of redefining again here.  If there are
exceptions/variations that are east to call out then just mention
those here.

Otherwise, the comments from the last section apply here too.


470	      KDF ID: Identifier of the Key Derivation Function used to derive
471	      MS-OTK.  See Section 6.7 for more details, and Section 8.5 for KDF
472	      IDs that MUST be supported.

[minor] §6.7 doesn't talk about the KFD ID.


...
480	      EID HMAC ID: Identifier of the HMAC algorithm used to protect the
481	      integrity of the EID-AD.  See Section 6.7 for more details, and
482	      Section 8.3 for HMAC IDs that MUST be supported.

[minor] §6.7 doesn't talk about the EID HMAC ID.


484	      EID mask-len: Mask length for EID-prefix.

486	      EID-AFI: Address family of EID-prefix according to [RFC8060].

488	      EID-prefix: This field contains an EID-prefix that the destination
489	      ETR is authoritative for, and is the longest match for the
490	      requested EID.

[major] Same comment as in §6.1: These 3 fields, which are part of the
"Rec", are the same as what is specified in §5.2/rfc6833bis.


...
499	      PKT HMAC ID: Identifier of the HMAC algorithm used to protect the
500	      integrity of the Map-Reply.  See Section 8.3 for HMAC IDs that
501	      MUST be supported.

[major] As mentioned before, please take Normative specifications out
of the IANA section and move it somewhere more appropriate.


503	      PKT HMAC: HMAC of the whole Map-Reply packet, including the LISP-
504	      SEC Authentication Data.  The scope of the authentication goes
505	      from the Map-Reply Type field to the PKT HMAC field included.
506	      Before computing the HMAC operation the PKT HMAC field MUST be set
507	      to 0.  See Section 6.8 for more details.

[major] These two sentences don't seem to say the same thing:

   HMAC of the whole Map-Reply packet, including the LISP-SEC
   Authentication Data.  The scope of the authentication goes
   from the Map-Reply Type field to the PKT HMAC field included.

Does it include the Map-Reply (first sentence) or just the data
defined in this document (second sentence)?  The description in §6.8
is slightly clearer, but not by much:

   The PKT-AD contains the HMAC of the whole Map-Reply packet...
   The scope of the HMAC operation covers the entire PKT-AD, from the
   Map-Reply Type field to the PKT HMAC field...

It would be better if the specification was made only once.  A pointer
to §6.8 should be enough here.


[major] "Before computing the HMAC operation the PKT HMAC field MUST
be set to 0."

§6.8 describes the same operation, but without using Normative
language: "...to the PKT HMAC field, which must be set to 0 before the
computation."

Please specify things only once!  It seems to me that it may be more
appropriate to include the specification in §6.8.


509	6.3.  Map-Register LISP-SEC Extentions

[] This section is not needed -- see below.

511	   This memo is allocating one of the bits marked as Unassigned in the
512	   Map-Register message defined in [I-D.ietf-lisp-rfc6833bis].  More
513	   precisely, the second bit after the Type field in a Map-Register
514	   message is allocated as the S bit.  The S bit indicates to the Map-
515	   Server that the registering ETR is LISP-SEC enabled.  An ETR that
516	   supports LISP-SEC MUST set the S bit in its Map-Register messages.

[major] The S-bit is already allocated and defined in rfc6833bis, so
the first two sentences are not needed.


[major] rfc6833bis is not as explicit as the last two sentences, but
it seems to me that it already covers them.  This is from
§5.6/rfc6833bis (Map-Register Message Format):

   S: This is the security-capable bit.  When set, the procedures from
      [I-D.ietf-lisp-sec] are supported.

This text doesn't explicitly require the ETR to set the bit.  If you
want to make that explicit, then we should update the text in
rfc633bis.


518	6.4.  ITR Processing: Generating a Map-Request

520	   Upon creating a Map-Request, the ITR generates a random ITR-OTK that
521	   is stored locally (until the corresponding Map-Reply is received),
522	   together with the nonce generated as specified in
523	   [I-D.ietf-lisp-rfc6833bis].

[minor] "until the corresponding Map-Reply is received"

Please include a forward reference to §6.9.


...
531	   The Map-Request MUST be encapsulated in an ECM, with the S-bit set to
532	   1, to indicate the presence of Authentication Data.

[major] "Map-Request MUST be encapsulated in an ECM"

This part is confusing to me -- along with the many parts that talk
about an "encapsulated Map-Request".

What do you mean by an "encapsulated Map-Request"?  I see 3 options:

(1) The ECM Authentication Data (from Figure 1) carried in the ECM
(§5.8/rfc6833bis) with the S-bit set.  This is what I've been assuming
because §6.1 talks about it being a Map-Request when it describes the
Record Count.

If so, at minimum, the text is confusing because the content of the
ECM Authentication Data is different from the Map-Request from
§5.2/rfc6833bis, but the names are the same.  Also, the functionality
from rfc6833bis is different.


(2) A Map-Request (§5.2/rfc6833bis) encapsulated as the LCM in the ECM
as described in §5.8/rfc6833bis.  In this case, setting the S-bit, as
required above would indicate that the ECM Authentication Data (from
Figure 1) would also be there.

If so, this document is not clear about the interaction between the
two Map-Requests.  Should the Records match, what if they don't,
etc..?


(3) A Map-Request (§5.2/rfc6833bis) encapsulated as the LCM in the ECM
as described in §5.8/rfc6833bis.  The S-bit is not set.

I don't think this interpretation makes sense because then it wouldn't
be protected at all.


Please clarify here, and update the descriptions elsewhere as needed.


534	   ITR-OTK is wrapped with the algorithm specified by the OTK Wrapping
535	   ID field.  See Section 6.5 for further details on OTK encryption.  If
536	   the NULL-KEY-WRAP-128 algorithm is selected and DTLS is not enabled
537	   in the path between the ITR and the Map-Resolver, the Map-Request
538	   MUST be dropped and an appropriate log action SHOULD be taken.

[nit] s/ITR-OTK/The ITR-OTK


540	   The Requested HMAC ID field contains the suggested HMAC algorithm to
541	   be used by the Map-Server and the ETR to protect the integrity of the
542	   ECM Authentication data and of the Map-Reply.  A HMAC ID Value of
543	   NONE (0), MAY be used to specify that the ITR has no preferred HMAC
544	   ID.

[nit] s/NONE (0), MAY/NONE (0) MAY


546	   The KDF ID field specifies the suggested key derivation function to
547	   be used by the Map-Server to derive the MS-OTK.  A KDF Value of NONE
548	   (0), MAY be used to specify that the ITR has no preferred KDF ID.

[major] s/Value of NONE (0), MAY be used/Value of NONE (0) may be used

Even though it is optional to use 0, the optional use of the KDF ID
field by the ITR was already specified in §6.1, so this is just a
statement of fact.


...
554	6.4.1.  PITR Processing

[minor] Please expand PITR on first use.


556	   The processing performed by a PITR is equivalent to the processing of
557	   an ITR.  However, if the PITR is directly connected to a Mapping
558	   System such as LISP+ALT [RFC6836], the PITR performs the functions of
559	   both the ITR and the Map-Resolver forwarding the Map-Request
560	   encapsulated in an ECM header that includes the Authentication Data
561	   fields as described in Section 6.6.

[?] This description of a PITR having multiple colocated functionality
is not specific to the PITR, right?  Also, the description is not
specific to LISP+ALT, the same would happen with any Mapping System,
right?   It seems to me that this section is not needed.


563	6.5.  Encrypting and Decrypting an OTK
...
594	   Implementations of this specification MUST support OTK Wrapping ID
595	   AES-KEY-WRAP-128+HKDF-SHA256 that specifies the use of the HKDF-
596	   SHA256 Key Derivation Function specified in[RFC4868] to derive a per-
597	   message encryption key (per-msg-key), as well as the AES-KEY-WRAP-128
598	   Key Wrap algorithm used to encrypt a 128-bit OTK, according to
599	   [RFC3394].

[nit] s/in[RFC4868]/in [RFC4868]


601	   The key wrapping process for OTK Wrapping ID AES-KEY-WRAP-128+HKDF-
602	   SHA256 is described below:

604	   1.  The KDF algorithm is identified by the field 'OTK Wrapping ID'
605	       according to the table in Section 8.4.

607	   2.  The Key Wrap algorithm is identified by the field 'OTK Wrapping
608	       ID' according to the table in Section 8.4.

[minor] Can we merge these two steps?  Also, given that other
algorithms may be defined later, let's separate the process from the
table itself.

Suggestion>
   The KDF and Key Wrap algorithms are identified by the value of
   the 'OTK Wrapping ID' field.  The initial values are documented
   in Table #x.


610	   3.  If the NULL-KEY-WRAP-128 algorithm (defined in (Section 8.4)) is
611	       selected and DTLS is not enabled, the Map-Request MUST be dropped
612	       and an appropriate log action SHOULD be taken.

[minor] s/8.4/6.5


...
640	6.5.1.  Unencrypted OTK

642	   MS-OTK confidentiality and integrity protection MUST be provided in
643	   the path between the Map-Server and the ETR.  Similarly, ITR-OTK
644	   confidentiality and integrity protection MUST be provided in the path
645	   between the ITR and the Map-Resolver.

[major] This same specification is also present in §6.5.  Please
specify the bahavior only once.


...
676	6.7.  Map-Server Processing

678	   Upon receiving an ECM encapsulated Map-Request with the S-bit set to
679	   1, the Map-Server process the Map-Request according to the value of
680	   the security-capable S-bit and of the proxy map-reply P-bit contained
681	   in the Map-Register sent by the ETRs authoritative for that prefix
682	   during registration.

[minor] This is a long sentence...my first read got me confused about
what was meant by "the value of the security-capable S-bit" if it had
just been pointed that it is set to 1.

Please come up with a shortcut for "ECM encapsulated Map-Request with
the S-bit set to 1".  Refer back to the comments on §6.4.  Suggestion:
"secure Map-Request".

Suggestion>
   Upon receiving a "secure Map-Request", the Map-Server precesses
   it according to the setting of the S and P-bits in the Map-Register
   received from the authoritative ETRs for the corresponding prefix,
   as described below.


...
731	   In this way the ITR that sent a LISP-SEC protected Map-Request always
732	   receives a LISP-SEC protected Map-Reply.  However, the ETR-Cant-Sign
733	   E-bit set to 1 specifies that a non LISP-SEC Map-Request might reach
734	   additional ETRs that have LISP-SEC disabled.  This mechanism allows
735	   the ITR to securely downgrade to non LISP-SEC requests, if so
736	   desired.

[major] "This mechanism allows the ITR to securely downgrade to non
LISP-SEC requests, if so desired."

Besides the fact that "securely downgrade to non LISP-SEC" sounds like
a contradiction, the "if so desired" part leaves the operation up in
the air.  When/why would an ITR desire to disable security?  What are
the use cases where it may be ok?  What are the risks that should be
considered?

As I see it, downgrading is risky because it can open the door to
overclaiming, which list-sec closes (rfc6833bis).  IOW, a rogue ETR
can decide not to set the S-bit (even if it did support lisp-sec) and
eliminate its benefits.  rfc6833bis also says this:

   3.  LISP-SEC [I-D.ietf-lisp-sec] MUST be implemented.  Network
       operators should carefully weight how the LISP-SEC threat model
       applies to their particular use case or deployment.  If they
       decide to ignore a particular recommendation, they should make
       sure the risk associated with the corresponding threats is well
       understood.


Please add a "Deployment/Operational Considerations" section to help
operators in making the decision above, particularly about when an ITR
may desire to downgrade.  Please include (as appropriate) information
as described in §2/rfc5706.

FWIW, I see the E-bit as useful as a transition mechanism: while not
all the ETRs may have been upgraded then it seems ok to downgrade to
maintain the operation of the network.  However, I'm having a hard
time seeing the value afterwards.


738	6.7.1.  Generating a LISP-SEC Protected Encapsulated Map-Request
...
745	   The Map-Server updates the OTK-AD by deriving a new OTK (MS-OTK) from
746	   the ITR-OTK received with the Map-Request.  MS-OTK is derived
747	   applying the key derivation function specified in the KDF ID field.
748	   If the algorithm specified in the KDF ID field is not supported, the
749	   Map-Server uses a different algorithm to derive the key and updates
750	   the KDF ID field accordingly.

[major]

   If the algorithm specified in the KDF ID field is not supported, the
   Map-Server uses a different algorithm to derive the key and updates
   the KDF ID field accordingly.

See the comment below (after line 775) about the future existence of
multiple required/recommended algorithms.


752	   MS-OTK confidentiality and integrity protection MUST be provided in
753	   the path between the Map-Server and the ETR.  This can be achieved
754	   either by enabling DTLS between the Map-Server and the ETR, or by
755	   encrypting the MS-OTK with the pre-shared secret known to the Map-
756	   Server and the ETR.

[major] This is specified in §6.5.  Please specify behaviors only once.


...
767	   The Map-Server includes in the EID-AD the longest match registered
768	   EID-prefix for the destination EID, and an HMAC of this EID-prefix.
769	   The HMAC is keyed with the ITR-OTK contained in the received ECM
770	   Authentication Data, and the HMAC algorithm is chosen according to
771	   the Requested HMAC ID field.  If the Map-Server does not support this
772	   algorithm, the Map-Server uses a different algorithm and specifies it
773	   in the EID HMAC ID field.  The scope of the HMAC operation covers the
774	   entire EID-AD, from the EID-AD Length field to the EID HMAC field,
775	   which must be set to 0 before the computation.

[major]

   ...the HMAC algorithm is chosen according to the Requested HMAC ID
   field.  If the Map-Server does not support this algorithm, the Map-
   Server uses a different algorithm and specifies it in the EID HMAC ID
   field.

This document defines one required and one recommended algorithms.
The logic above then works.  What happens when other algorithms are
added -- but are only recommended?  It seems to me that if the ITR
requests one of the recommended algorithms, and the Map-Server doesn't
support it then another recommended algorithm, in this case not
supported by the ITR, might be selected.

How do the different LISP nodes know/learn what is supported by the
other nodes?  If the "uses a different algorithm" part results in a
required algorithm then we should be ok as long as future documents
Update the set of required and recommended algorithms in the future.


[major] "does not support this algorithm"

It's possible for the ITR to set the value to 0.  That case needs to
be covered here too.


[] See the related comment about the last sentence in §6.1.


...
782	6.7.2.  Generating a Proxy Map-Reply

784	   LISP-SEC proxy Map-Reply are generated according to
785	   [I-D.ietf-lisp-rfc6833bis], with the Map-Reply S-bit set to 1.  The
786	   Map-Reply includes the Authentication Data that contains the EID-AD,
787	   computed as specified in Section 6.7.1, as well as the PKT-AD
788	   computed as specified in Section 6.8.

[major] "Map-Reply...as specified in Section 6.7.1"

§6.7.1 talks about Map-Requests, not Map-Replies.  It looks like
theres a section missing.


790	6.8.  ETR Processing
...
802	   The EID-AD is copied from the Authentication Data of the received
803	   encapsulated Map-Request.

[major] The KFD ID may have to be updated so the EID-AD can't simply
be copied.  And the EID HMAC needs to be recalculated.

Note that §6.1 says that the EID HMAC is "computed and inserted by
Map-Server."  IOW, it doesn't take into account that it may have to be
updated.


805	   The PKT-AD contains the HMAC of the whole Map-Reply packet, keyed
806	   with the MS-OTK and computed using the HMAC algorithm specified in
807	   the Requested HMAC ID field of the received encapsulated Map-Request.
808	   If the ETR does not support the Requested HMAC ID, it uses a
809	   different algorithm and updates the PKT HMAC ID field accordingly.
810	   The scope of the HMAC operation covers the entire PKT-AD, from the
811	   Map-Reply Type field to the PKT HMAC field, which must be set to 0
812	   before the computation.

[] See the related comment in §6.2.


...
817	6.9.  ITR Processing: Receiving a Map-Reply

819	   In response to an encapsulated Map-Request that has the S-bit set, an
820	   ITR MUST receive a Map-Reply with the S-bit set, that includes an
821	   EID-AD and a PKT-AD.  If the Map-Reply does not include both ADs, the
822	   ITR MUST discard it.  In response to an encapsulated Map-Request with
823	   S-bit set to 0, the ITR expects a Map-Reply with S-bit set to 0, and
824	   the ITR SHOULD discard the Map-Reply if the S-bit is set.

[major] "ITR MUST receive a Map-Reply with the S-bit set"

The style of the second part of the paragraph is clearer on what the action is.

OLD>
   In response to an encapsulated Map-Request that has the S-bit set, an
   ITR MUST receive a Map-Reply with the S-bit set, that includes an
   EID-AD and a PKT-AD.  If the Map-Reply does not include both ADs, the
   ITR MUST discard it.

NEW>
   In response to a "secure Map-Request", an ITR expects a Map-Reply
   with the S-bit set to 1 including an EID-AD and a PKT-AD.  The ITR
   MUST discard the Map-Reply otherwise.


[major] "ITR SHOULD discard the Map-Reply if the S-bit is set"

When is it ok for the the ITR not to discard the Map-Reply?  Why is
this a recommendation and not a requirement?


...
831	   The integrity of the EID-AD is verified using the ITR-OTK (stored
832	   locally for the duration of this exchange) to re-compute the HMAC of
833	   the EID-AD using the algorithm specified in the EID HMAC ID field.
834	   If the EID HMAC ID field does not match the Requested HMAC ID the ITR
835	   SHOULD discard the Map-Reply and send, at the first opportunity it
836	   needs to, a new Map-Request with a different Requested HMAC ID field,
837	   according to ITR's local policy.  The scope of the HMAC operation
838	   covers the entire EID-AD, from the EID-AD Length field to the EID
839	   HMAC field.

[major] "If the EID HMAC ID field does not match the Requested HMAC ID
the ITR SHOULD discard the Map-Reply and send, at the first
opportunity it needs to, a new Map-Request with a different Requested
HMAC ID field, according to ITR's local policy.

§6.8 says that the ETR can use a different algorithm.

When is ok for the ITR to not discard the Map-Reply?  Why is this a
recommendation and not a requirement?


...
843	   To verify the integrity of the PKT-AD, first the MS-OTK is derived
844	   from the locally stored ITR-OTK using the algorithm specified in the
845	   KDF ID field.  This is because the PKT-AD is generated by the ETR
846	   using the MS-OTK.  If the KDF ID in the Map-Reply does not match the
847	   KDF ID requested in the Map-Request, the ITR SHOULD discard the Map-
848	   Reply and send, at the first opportunity it needs to, a new Map-
849	   Request with a different KDF ID, according to ITR's local policy.
850	   Without consistent configuration of involved entities, extra delays
851	   may be experienced.  However, since HKDF-SHA1-128 is specified as
852	   mandatory to implement in Section 8.5, the process will eventually
853	   converge.

[major] "If the KDF ID in the Map-Reply does not match the KDF ID
requested in the Map-Request, the ITR SHOULD discard the Map-Reply and
send, at the first opportunity it needs to, a new Map-Request with a
different KDF ID, according to ITR's local policy."

§6.1 says that the ETR can use a different algorithm.

When is ok for the ITR to not discard the Map-Reply?  Why is this a
recommendation and not a requirement?


[minor] s/Section 8.5/Section 6.4


855	   The derived MS-OTK is then used to re-compute the HMAC of the PKT-AD
856	   using the Algorithm specified in the PKT HMAC ID field.  If the PKT
857	   HMAC ID field does not match the Requested HMAC ID the ITR SHOULD
858	   discard the Map-Reply and send, at the first opportunity it needs to,
859	   a new Map-Request with a different Requested HMAC ID according to
860	   ITR's local policy or until all HMAC IDs supported by the ITR have
861	   been attempted.

[major] "If the PKT HMAC ID field does not match the Requested HMAC ID
the ITR SHOULD discard the Map-Reply..."

Same comments as above...


...
873	   [I-D.ietf-lisp-rfc6833bis] allows ITRs to send solicited Map-Requests
874	   directly to the ETRs that send the SMR message in the first place.
875	   However, in order to receive a secure Map-Reply, ITRs SHOULD send SMR
876	   triggered Map-Requests over the mapping system using the
877	   specifications of this memo.  If an ITR accepts piggybacked Map-
878	   Replies, it SHOULD also send a Map-Request over the mapping system in
879	   order to verify the piggybacked Map-Reply with a secure Map-Reply.

[minor] Expand SMR on first use.


[major] "in order to receive a secure Map-Reply, ITRs SHOULD
send...Map-Requests...using the specifications of this memo."

The only way to receive a secure Map-Reply is to use this
specification, so the "SHOULD" seems out of place because it is a
statement of fact.

Note however that rfc6833bis already requires the behavior (from §6.1):

   An SMR message is simply a bit set in a Map-Request message.  An ITR
   or PITR will send a Map-Request (SMR-invoked Map-Request) when they
   receive an SMR message.  While the SMR message is sent through the
   data-plane, the SMR-invoked Map-Request MUST be sent through the
   Mapping System (not directly).

IOW, the sentence is not needed.  Specify things only once.


[major] What are "piggybacked Map-Replies"?  Where are they specified?
 When is it ok for the ITR to not verify the information?  Why is it a
recommendation and not a requirement?


881	6.9.1.  Map-Reply Record Validation
...
917	   The EID-record with EID-prefix 2001:db8:102::/48 is not eligible to
918	   be used by the ITR since it is not included in any of the EID-ADs
919	   signed by the Map-Server.  A log action MUST be taken.

[major] "A log action MUST be taken."

This action is specified as part of an example.  Please generalize the
specification, include the text before the example, and don't use
normative language in an example.

Suggestion>
   If any EID-prefix in the payload of the Map-Reply is not
   included in the EID-AD signed by the Map-Server a log action
   MUST be taken.


...
925	   The EID-record with EID-prefix 2001:db8:200::/40 is not eligible to
926	   be used by the ITR since it is not included in any of the EID-ADs
927	   signed by the Map-Server.  A log action MUST be taken.  In this last
928	   example the ETR is trying to over claim the EID-prefix
929	   2001:db8:200::/40, but the Map-Server authorized only
930	   2001:db8:203::/48, hence the EID-record is discarded.

[major] "A log action MUST be taken."  See comment above.


...
934	7.1.  Mapping System Security

936	   The LISP-SEC threat model described in Section 4, assumes that the
937	   LISP Mapping System is working properly and eventually delivers Map-
938	   Request messages to a Map-Server that is authoritative for the
939	   requested EID.

[minor] "eventually" ??  Maybe that word is not needed.


...
949	7.2.  Random Number Generation

951	   The ITR-OTK MUST be generated by a properly seeded pseudo-random (or
952	   strong random) source.  See [RFC4086] for advice on generating
953	   security-sensitive random data

[nit] s/data/data.


...
963	7.4.  Deploying LISP-SEC

965	   This memo is written according to [RFC2119].  Specifically, the use
966	   of the key word SHOULD "or the adjective 'RECOMMENDED', mean that
967	   there may exist valid reasons in particular circumstances to ignore a
968	   particular item, but the full implications must be understood and
969	   carefully weighed before choosing a different course".

[major] §2 already includes the rfc2119/rfc8174 template.  There's no
need to include this text here, or the last paragraph.

I wonder the intent of putting it in this section: there are no
SHOULDs in it.  Should there be any? [I don't think so.]

Also, I expect the specification to call out some of the
considerations to be taken into account -- and not just mention that
considerations may exist.  [I made sure to ask about the SHOULDs in
other parts of the text.]


...
977	   As an example, in certain closed and controlled deployments, it is
978	   possible that the threat associated with a MITM between the xTR and
979	   the Mapping System is very low, and after carfeul consideration it
980	   may be decided to allow a NULL key wrapping algorithm while carrying
981	   the OTKs between the xTR and the Mapping System.

[nit] s/carfeul/careful


...
992	7.5.  Shared Keys Provisioning

994	   Provisioning of the keys shared between the ITR and the Map-Resolver
995	   as well as between the ETR and the Map-Server should be performed via
996	   an orchestration infrastructure and it is out of the scope of this
997	   memo.  It is recommended that both shared keys are refreshed at
998	   periodical intervals to address key aging or attackers gaining
999	   unauthorized access to the shared keys.  Shared keys should be
1000	   unpredictable random values.

[] You will be asked about LISP management and the YANG model in
particular.  It is up to you whether or not you want to include an
Informative reference to draft-ietf-lisp-yang.


1002	7.6.  Replay Attacks
...
1010	   In case of replayed Map-Request, the Map-Server, Map-Resolver and ETR
1011	   will have to do a LISP-SEC computation.  This is equivalent to a
1012	   valid LISP-SEC computation and an attacker does not obtain any
1013	   benefit.

[major] You mean "equivalent to a valid LISP-SEC computation" in terms
of resources, right?

It might be a good idea to indicate why "an attacker does not obtain
any benefit".  Is it because the Map-Request will be discarded?  Why?

There's some text in the Security Considerations of rfc6833bis about
this topic and rate limiting.  It might be worth referencing it.

BTW, please include a paragraph at the start of this section (before
§7.1) that explicitly says that the Security Considerations of
rfc6833bis apply to this document too.


1015	7.7.  Message Privacy

1017	   DTLS [RFC6347] SHOULD be used to provide communication privacy and to
1018	   prevent eavesdropping, tampering, or message forgery to the messages
1019	   exchanged between the ITR, Map-Resolver, Map-Server, and ETR.

[major] "DTLS [RFC6347] SHOULD be used"

When is it ok to not use DTLS?  Why is this a recommendation and not a
requirement?

There are many places in this document that say this:

   ...confidentiality and integrity protection MUST be provided
   in the path between [A] and [B].  This can be achieved either
   by enabling DTLS between [A] and [B], or by encrypting...

I can see how enabling DLTS is not required if the *-OTK is encrypted.
Is this the conditioning statement that leads to a SHOULD?  Please
write it out here.


...
1030	8.  IANA Considerations

[] The comments in §8.1 apply to all the sections.


1032	8.1.  ECM AD Type Registry

1034	   IANA is requested to create the "ECM Authentication Data Type"
1035	   registry with values 0-255, for use in the ECM LISP-SEC Extensions
1036	   Section 6.1.  The registry MUST be initially populated with the
1037	   following values:

[major] Where should this registry be located?  I assume you would
want it to be a sub-registry of the "Locator/ID Separation Protocol
(LISP) Parameters" registry.
(https://www.iana.org/assignments/lisp-parameters/lisp-parameters.xhtml)


[major] "registry MUST be initially populated"

Normative keywords should only be used for interoperability reasons,
not to give instructions to IANA:

Suggestion>
   Table x shows the initial allocations made to this new registry.


1039	             Name                     Value        Defined In
1040	             -------------------------------------------------
1041	             Reserved                 0             This memo
1042	             LISP-SEC-ECM-EXT         1             This memo

[minor] Please add a table number -- to all the tables in this section.


1044	   Values 2-255 are unassigned.  They are to be assigned according to
1045	   the "Specification Required" policy defined in [RFC8126].

[major] The Specification Required policy requires the review of a
Designated Expert.  Please provide instructions for the experts?  What
factors should they take into account when approving a new value?


...
1126	10.1.  Normative References
...
1141	   [RFC4086]  Eastlake 3rd, D., Schiller, J., and S. Crocker,
1142	              "Randomness Requirements for Security", BCP 106, RFC 4086,
1143	              DOI 10.17487/RFC4086, June 2005,
1144	              <https://www.rfc-editor.org/info/rfc4086>.

[minor] This reference can be Informative.


...
1164	10.2.  Informational References
...
1170	   [I-D.ietf-lisp-rfc6830bis]
1171	              Farinacci, D., Fuller, V., Meyer, D., Lewis, D., and A.
1172	              Cabellos, "The Locator/ID Separation Protocol (LISP)",
1173	              Work in Progress, Internet-Draft, draft-ietf-lisp-
1174	              rfc6830bis-36, 18 November 2020,
1175	              <https://www.ietf.org/archive/id/draft-ietf-lisp-
1176	              rfc6830bis-36.txt>.

[major] This reference must be Normative.


1178	   [RFC2104]  Krawczyk, H., Bellare, M., and R. Canetti, "HMAC: Keyed-
1179	              Hashing for Message Authentication", RFC 2104,
1180	              DOI 10.17487/RFC2104, February 1997,
1181	              <https://www.rfc-editor.org/info/rfc2104>.

[major] This reference must be Normative.


1183	   [RFC3394]  Schaad, J. and R. Housley, "Advanced Encryption Standard
1184	              (AES) Key Wrap Algorithm", RFC 3394, DOI 10.17487/RFC3394,
1185	              September 2002, <https://www.rfc-editor.org/info/rfc3394>.

[major] This reference must be Normative.


1187	   [RFC5869]  Krawczyk, H. and P. Eronen, "HMAC-based Extract-and-Expand
1188	              Key Derivation Function (HKDF)", RFC 5869,
1189	              DOI 10.17487/RFC5869, May 2010,
1190	              <https://www.rfc-editor.org/info/rfc5869>.

[major] This reference must be Normative.


1192	   [RFC6234]  Eastlake 3rd, D. and T. Hansen, "US Secure Hash Algorithms
1193	              (SHA and SHA-based HMAC and HKDF)", RFC 6234,
1194	              DOI 10.17487/RFC6234, May 2011,
1195	              <https://www.rfc-editor.org/info/rfc6234>.

[major] This reference must be Normative.


...
1202	   [RFC7835]  Saucez, D., Iannone, L., and O. Bonaventure, "Locator/ID
1203	              Separation Protocol (LISP) Threat Analysis", RFC 7835,
1204	              DOI 10.17487/RFC7835, April 2016,
1205	              <https://www.rfc-editor.org/info/rfc7835>.

[major] This reference must be Normative as "LISP-SEC addresses the
control plane threats, described in 3.7 and 3.8 of [RFC7835]".

[EoR -25]