Locator/ID Separation Protocol (LISP) Map-Versioning
draft-ietf-lisp-6834bis-14

# Internet AD comments for {draft-ietf-lisp-6834bis-11}
CC @ekline

## Comments

### S6

* Consider adding a reference to RFC 1982, which goes into this at
  greater length.

Thanks for the detailed discussion and the updates. I've cleared my DISCUSS. I'm not going to reply to the various individual messages because the net of the whole thing is "LGTM".

A few small nits on -14:

1. §A.1:

   The ETR checks only the Dest Map-Version number, ignoring the Source
   Map-Version number as specified in the final sentence of Section 7.2,
   ignoring the Source Map-Version number.

The source map-version number is getting double ignored, it must feel sad. :-) Probably should be:

   The ETR checks only the Dest Map-Version number, ignoring the Source
   Map-Version number as specified in the final sentence of Section 7.2.

2. §A.2:

   Map-Versioning is compatible with the LISP interworking between LISP
   and non-LISP sites as defined in [RFC6832].  LISP interworking
   defines three techniques to allow communication LISP sites and non-

Insert "between" between "communication" and "LISP sites", so:

   Map-Versioning is compatible with the LISP interworking between LISP
   and non-LISP sites as defined in [RFC6832].  LISP interworking
   defines three techniques to allow communication between LISP sites and non-

--

Original DISCUSS:

This spec makes liberal use of the approach of dropping any packet received with an unloved Map-Version number, for example (but not limited to)

   2.  The packet arrives with a Dest Map-Version number newer (as
       defined in Section 6) than the one stored in the EID-to-RLOC
       Database.  Since the ETR is authoritative on the mapping, meaning
       that the Map-Version number of its mapping is the correct one,
       this implies that someone is not behaving correctly with respect
       to the specifications.  In this case, the packet carries a
       version number that is not valid and packet MUST be silently
       dropped.

Isn’t it the case that by definition the packet has arrived at a valid ETR for the mapping (since as the text says, “the ETR is authoritative”)? Isn’t the map-version more in the nature of a hint than a critical-for-correctness field? What bad behavior is being protected against by silently dropping this traffic, that has arrived at a correct endpoint albeit with an incorrect hint?

At various points in the document there's a kind of vague assertion that incorrect map-versions could be an attack. While I don't deny that, the assertion isn't supported or elaborated on anywhere that I saw, which is worrying and also makes it less convincing. Shouldn't the Security Considerations talk about this? I did also go have a look at the Security Considerations in draft-ietf-lisp-rfc6833bis-31, which also didn't help me. RFC 7835 §3.3 does touch on this, suggesting that maybe an attacker could use a spoofed Map-Version to trigger a DoS attack. But this, too, is an unsatisfying rationale, since as you take pains to point out, rate limiting of Map-Requests and such is required. Furthermore, if triggering Map-Requests is the concern, couldn't the packet still be delivered, without triggering a Map-Request?

When this was an Experimental protocol this kind of thing was probably less crucial to justify and explain, but I would have expected the experiment to produce results that could be fed into this document. At the moment, the "drop any packet that doesn't comply with expectations" design feels arbitrary and potentially brittle. I would appreciate some discussion of this design choice, thanks in advance.

(I do acknowledge that security matters can be subtle, and I'm not a SEC AD after all... but all the more reason for the document to be explicit about what the security concerns are instead of just gesturing toward them and leaving the reader to guess.)

Original COMMENT:

I support Roman Danyliw's DISCUSS position.

I have a number of further questions and comments --

1. In §6.1:

           If an ETR receives LISP-encapsulated packets with the V-bit
   set, when the original mapping in the EID-to-RLOC Database has the
   version number set to the Null Map-Version value, then those packets
   MUST be silently dropped.

What does “original” mean in this context? Couldn’t the mapping in the db once have had a value but in a later revision, had its value changed to the null value? Presumably in such a situation packets would be lost until the ITR decided to issue a new map-request. 


2. In §7.1 (3):

                                                    According to rate
       limitation policy defined in [I-D.ietf-lisp-rfc6833bis] for Map-
       Request messages, after 10 retries Map-Requests are sent every 30
       seconds, if in the meantime the Dest Map-Version number in the
       packets is not updated, the ETR SHOULD drop packets with a stale
       Map-Version number. 

What exactly is “the meantime”? Does that mean “after 10 retries”? After 30 seconds? Basically, what precisely is the grace period extended to the ITR have to come into compliance before being blocked? This seems important to be clear about -- even if the clarity is in the form of "it's implementation-dependent".


3. In §7.1, final paragraph:

   LISP-encapsulated packets cannot transport a Dest Map-Version number
   equal to the Null Map-Version number, because in this case the ETR is
   signaling that Map-Version numbers are not used for the mapping of
   the destination EID (see Section 6.1).

Considering that the Null Map-Version number is just the distinguished value 0, the first clause is prima facie wrong -- it's possible to encode 0 in that field. I think what you mean is something more along the lines of 

   It is a protocol violation for LISP-encapsulated packets to contain a 
   Dest Map-Version number equal to the Null Map-Version number, see 
   Section 6.1.
   
Please don't try to explain it again in-line as you've done, it just confuses the reader (well, it confused me!). Instead, refer them back to the place where you specified the rule. 

It does seem unfortunate that in this case, it's not possible to include a Source Map-Version number, even if that would be helpful to do, since the V bit is required to be set to 0 and covers both Source and Dest. 


4. §7.1 (3), nit: s/The packets arrive/The packet arrives/


5. In §7.1 and §7.2:

                             A check on this version number SHOULD be
   done, where the following cases can arise:
   
and

                             If the ETR has an entry in its EID-to-RLOC
   Map-Cache for the source EID, then a check SHOULD be performed and
   the following cases can arise:

What are the cases under which the check can be omitted? Please consider adding discussion about those cases. Alternately, consider making the SHOULD a MUST if there are no such cases.


6. In §7.2:

   3.  The packet arrives with a Source Map-Version number smaller
       (i.e., older) than the one stored in the local EID-to-RLOC Map-
       Cache.  Such a case is not valid with respect to the
       specifications.

The final sentence ("not valid") seems like it must be wrong: consider for example the case of out-of-order packets. Other scenarios also exist, such as transient non-synchronization between ETRs during convergence. I notice that §9 talks about the lack of synchronization mechanisms in LISP, other than diligent consistency of configuration. So, I guess there's a good chance that "convergence" means "someone updating mapping configurations by hand" and so version skew could exist for human-scale periods of time. Of greatest concern is if "human-scale periods of time" means "hours or days" in the case where a mistake with operational procedures leaves the hand-configured databases on two ETRs out of sync with one another.

I guess a minimum fix would be to simply cut the wrong sentence and slightly re-word, e.g.:

   3.  The packet arrives with a Source Map-Version number smaller
       (i.e., older) than the one stored in the local EID-to-RLOC Map-
       Cache.  Note that if the mapping is already present in the
       EID-to-RLOC Map-Cache, this means that an explicit Map-Request
       has been sent and a Map-Reply has been received from an
       authoritative source.  In this situation, the packet SHOULD be
       silently dropped.  Operators can configure exceptions to this
       recommendation, which are outside the scope of this document.


7. In §7.2:

   If the ETR does not have an entry in the EID-to-RLOC Map-Cache for
   the source EID, then the Source Map-Version number MUST be ignored.

I think it would be nice to have an xref to §A.1, where the reason for this is explained. Otherwise it seems rather arbitrary.


8. In §8:

I see that in -12 you cut the text that in -11 used to say

   Map-Versioning MUST NOT be used over the public Internet and SHOULD
   only be used in trusted and closed deployments.

I note that the requirement continues to exist however, since normative reference draft-ietf-lisp-rfc6830bis-38 §4.1 says

   Several of the mechanisms in this document are intended for
   deployment in controlled, trusted environments, and are insecure for
   use over the public Internet.  In particular, on the public internet
   xTRs:
...
   *  MUST NOT use Gleaning or Locator-Status-Bits and Map-Versioning,
      as described in Section 13 to update the EID-to-RLOC Mappings.
      Instead relying solely on control-plane methods.

Thus it still seems to me that the questions others raised about this requirement may be relevant.

So, I question whether cutting the text is the right way to fix the concerns. It makes sense in an Experimental document, but perhaps not in a Standards Track one.


9. In §9:

   LISP requires ETRs to provide the same mapping for the same EID-
   Prefix to a requester.

What does this mean? Same as what? I guess maybe what you mean here is "LISP requires multiple ETRs within the same site to provide identical mappings for a given EID-Prefix"? If so, please say that (or something else clearer than what's there now). If not, please help?


10. In §A.1:

   The ETR checks only the Dest Map-Version number as described in
   Section 7, ignoring the Source Map-Version number.

I would rewrite as

   The ETR checks only the Dest Map-Version number, 
   ignoring the Source Map-Version number as specified in
   the final sentence of Section 7,.


11. In §A.2:

                                                LISP interworking
   defines three techniques to make LISP sites and non-LISP sites,
   namely Proxy-ITR, LISP-NAT, and Proxy-ETR. 

This isn't a complete sentence. I guess what you mean is something like "LISP interworking defines three techniques to allow communication between LISP and non-LISP sites"?


12. In §A.2.1:

   With this setup, LISP Domain A is able to check whether the PITR is
   using the latest mapping.

First, how does Domain A check this? Second, the latest mapping for what? I suppose you might mean something like "Domain A is able to check whether the PITR is using the latest mapping for the destination EID, by inspecting the Destination Map-Version as detailed in Section 7.1"?


13. In §A.2.3:

   With this setup, the Proxy-ETR, by looking at the Source Map-Version
   Number, is able to check whether the mapping has changed.

Again, what mapping, and how? I guess it must be the source EID. (The version 12 text, which I've quoted here, makes that clearer, although it would still be even clearer to write "... check whether the Source EID-to-RLOC mapping has changed.") Why does the ETR care about that? I guess there's the assumption it might be an ITR/ETR passing traffic bidirectionally, in which case the source EID might be useful, but if that's the reason then some words to that effect would help.

The shepherd writeup says:

> It is the proper type of RFC since it provides updates to RFC 6834 Locator/ID
> Separation Protocol (LISP) Map-Versioning, which was an experimental document.

I don't follow.  Only a Proposed Standard can update an Experimental?

The SHOULD at the top of sections 7.1 and 7.2 are curious.  Does the protocol interoperate properly if both of those Map-Version checks are skipped, which the SHOULD allows in each case?  I would suggest including some guidance around when an implementation might legitimately choose not to do them.  Or should these be MUSTs or MAYs instead?

DISCUSS resolved by version 13/14. Old DISCUSS copied below:



Changed my comments to a DISCUSS, as Donald Eastlake also pointed these out in his secdir review, and I am now convinced we need better text to address this.

#1  map-version rollover is defined (to skip the 0 version) but I also see:

The packet arrives with a Dest Map-Version number greater (i.e.,
       newer) than the one stored in the EID-to-RLOC Database.  Since
       the ETR is authoritative on the mapping, meaning that the Map-
       Version number of its mapping is the correct one

This would imply rollover to a smaller number is not expected to occur ?

#2 MUST NOT or SHOULD ?

Map-Versioning MUST NOT be used over the public Internet and SHOULD only be used in trusted and closed deployments.

This sentence seems to contradict itself. I would turn the SHOULD into a MUST

Thank you to Donald Eastlake for the SECDIR review.

Thank you for addressing my DISCUSS feedback.

Thanks for working on this specification. Thanks to Yoshifumi Nishida for a very good TSVART review.

# Éric Vyncke, INT AD, review of draft-ietf-lisp-6834bis-11

Thank you for the work put into this document. Updated ballot as I failed to clean up the template, i.e., this one does not contain the empty DISCUSS section ;-) Sorry about that.

Please find below some non-blocking COMMENT points (but replies would be appreciated even if only for my own education), and some nits.

Special thanks to Padma Pillay-Esnault for the shepherd's write-up including the WG consensus and the intended status. 

I hope that this helps to improve the document,

Regards,

-éric

## COMMENTS

### Section 6

Just wondering why having an algorithm defined for 'N' while the versions are always on 12 bits.

### Section 8

```
Map-Versioning MUST NOT be used over the public Internet and SHOULD
   only be used in trusted and closed deployments.
```

An explanation of why and how would be welcome. Feel free to ignore this comment though as this is the usual recommendation for any tunneling mechanism w/o authentication/confidentiality.

## NITS  

### Section 6

s/MUST consist in an increment by one the older/MUST consist in an increment by one of the older/ ? Moreover, 'increment' is usually understood as 'add 1' so no need to add 'by one' in the sentence

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. 

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments

I support the discuss positions from Roman, Paul and John which I think cover the issues that I see in this document as well.

# GEN AD review of draft-ietf-lisp-6834bis-10

CC @larseggert

## Comments

### Section 6, paragraph 8
```
     1.  V1 = V2 : The Map-Version numbers are the same.

     2.  V2 > V1 : if and only if

           V2 > V1 AND (V2 - V1) <= 2**(N-1)

           OR

           V1 > V2 AND (V1 - V2) > 2**(N-1)

     3.  V1 > V2 : otherwise.
```
Shouldn't this include cases for if either V1 or V2 is the Null Map-Version?

### Section 6.1, paragraph 0
```
  6.1.  The Null Map-Version
```
It might have been cleaner to actually define a one-bit "Null
Map-Version" flag and use an 11-bit number space, instead of
overloading the 0x0000 version. That would have eliminated the
need for a lot of special-casing in the arithmetic.

### Inclusive language

Found terminology that should be reviewed for inclusivity; see
https://www.rfc-editor.org/part2/#inclusive_language for background and more
guidance:

 * Term `invalid`; alternatives might be `not valid`, `unenforceable`, `not
   binding`, `inoperative`, `illegitimate`, `incorrect`, `improper`,
   `unacceptable`, `inapplicable`, `revoked`, `rescinded`

## Nits

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

### Boilerplate

Document still refers to the "Simplified BSD License", which was corrected in
the TLP on September 21, 2021. It should instead refer to the "Revised BSD
License".

### Grammar/style

#### Section 6.1, paragraph 5
```
C Map-Cache for the source EID is up to date. If one or both of the above pre
                                  ^^^^^^^^^^
```
It appears that hyphens are missing in the adjective "up-to-date".

#### "A.1.", paragraph 2
```
 LISP Domain A is able to check whether or not the PITR is using the latest m
                                ^^^^^^^^^^^^^^
```
Consider shortening this phrase to just "whether". It is correct though if you
mean "regardless of whether".

#### "A.2.1.", paragraph 2
```
 the Proxy-ETR is able to check whether or not the mapping has changed. A.3.
                                ^^^^^^^^^^^^^^
```
Consider shortening this phrase to just "whether". It is correct though if you
mean "regardless of whether".

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT].

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments
[IRT]: https://github.com/larseggert/ietf-reviewtool

Thanks for a concise and clearly written document, and to Yoshi for the TSVART review.