Re: [lisp] Last Call SECDIR Review of draft-ietf-lisp-6834bis-11

[Donald Eastlake <[e-mail]>]

See below at <de>

On Mon, May 30, 2022 at 9:32 AM Luigi Iannone <[e-mail]> wrote:

> Hi Donald,
>
> Thank you very much for your review.
> I take this last updated review and provide some answers inline.
>
> On 30 May 2022, at 04:35, Donald Eastlake <[e-mail]> wrote:
>
> I have updated my review of the -10 version for -11 below.
> Comments/suggestions that I do not comment on still apply.
>
> On Wed, May 25, 2022 at 3:41 PM Donald Eastlake <[e-mail]> wrote:
>
>
> I have reviewed this document as part of the security directorate's
> ongoing effort to review all IETF documents being processed by the IESG.
> Document editors and WG chairs should treat these comments just like any
> other last call comments. Sorry this is a bit late.
>
>
> The summary of the review is Ready with Issues.
>
>
> Well, minor issues...
>
> ...
>
> SECURITY
>
> This draft appears to completely ignore the issue of Map Version Number
> advancing so far so quickly that an old version number is encountered that
> appears to be newer than or equal to the current version number. Why can't
> this happen? Or if it can, why doesn't that hurt?
>
> This is more of an operational point. If you update a mapping, the best
> would be to give sufficient time so that everybody updates and there is no
> such a risk.
> What about adding in section 7 “dealing with Map-Version Numbers” the
> following sentence.
>
> It is an operational question to make sure that Map-Version numbers are
> not updated so frequently as to create the risk that very old version
> numbers appear newer (because of the circular space).
>
> Would that address your issue?
>

<de> Not really. (1) I think the document needs to say what happens if the
numbers wrap around and overlap. (2) Assuming the answer to 1 is as bad as
I think, then it is not "an operational question" to avoid this but rather
"an operational requirement".  That is, there should be a statement
something like "Map Version Number incrementing and TTL MUST be managed so
that an old Version Numbers will not be confused as a new Version Number.

> Section 8, last paragraph: Says Map-Versioning can only be used in
> trusted, closed environments but Section 7.1 and 7.2 seem to talk about
> what to do about the Map-Version field without any reference to this, but
> mentioning private deployments for certain error conditions. For example,
> Section 7.2 point 3 says to discard a packet on an erroneous Map-Version
> value except perhaps in some private deployments. But if you MUST NOT use
> Map-Versioning on the open internet, shouldn't it be required to discard
> all LISP encapsulated packets with Map-Version numbering if received over
> the public Internet?
>
> Actually section 7.1 reads:
>
> Operators can configure exceptions to this
>        recommendation, which are outside the scope of this document.
>
>
> We should have done the same for 7.2, will do in a revision.
>
<de> Well, adding that to Section 7.2 would help. But the problem is the
following sentence in Section 8:

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


It seems to me that sentence says you can only use Map-Versioning in a
private network and that private network SHOULD be trusted. So I guess it
allows use in an untrusted private network... Is that what you want to say?


> Otherwise, the Security Considerations seem adequate although I think the
> 1st and 2nd paragraphs of Section 8 should be swapped.
>
> Yes, it may read better. Will be swapped in the next revision.
>
> OTHER ISSUES
>
> Section 6, right after equation 3: Isn't "(69 + 4096) mod 4096" the same
> as "69"? And isn't 69 equal to 69, not less than 69? Shouldn't it say
> "Map-Version numbers in the range [69 + 2049; 68] are smaller than 69"? Or
> actually "in the ranges [69 + 2049; 4095] and [1;68] are smaller than 69"?
>
> Wonderful catch. Should be
> [69 +  2049; (69 + 4095) mod 4096]
>
>
> Will fix.
>
> Section A.3: How is it possible to tell that no more traffic will be
> received? Should this instead be something like wait the TTL of the
> mappings to that RLOC plus estimated transit time and some margin for
> safety?
>
> Absolutely right, the sentence should be:
>
>    Upon updating the mapping, the RLOC will receive the less and less
>
>    traffic because remote LISP sites will get the updated mapping.
>
>    At least one TTL after the mapping was updated, it could be
>
>    considered safe to shut down the RLOC gracefully, because all
>
>    sites actively using the mapping should have updated
>
>    it.
>
>
> Sounds better?
>

<de> Yes, that's better. But I would suggest alternate wording such as the
following:

     "Upon updating the mapping, the RLOC will receive less and less

   traffic because remote LISP sites will request the updated

   mapping and see that it is disabled. At least one TTL, plus a

   little time for traffic transit, after the mapping is updated,

   it should be safe to shut down the RLOC gracefully, because

   all sites actively using the mapping should have been updated."

TYPOS/MINOR
>
> Should the document say anything about mapping changes possibly causing
> re-ordering?
>
> Not sure what do you mean by “re-ordering”, can you articulate?
>

<de> I was thinking about adding one sentence somewhere something like the
following:
"A change in map version resulting in a change in ETR for a flow can result
in the re-ordering of the packet in the flow just as any other routing
change could cause re-ordering."

> Section 1: I think the following should end with "ITR": "If this is not
> the case, the ETR can directly send a Map-Request containing the updated
> mapping to the ETR,"
>
>
> Above fixed in -11.
>
> Section 7.2, first sentence just after point 3: Suggest using "MAY" in
> "may be more restrictive."
>
>
> Will be changed in the next revision.
>
> Section A.2.3: "provider edge" pops up here with no other mention or
> explanation anywhere in the draft.
>
>
> Either drop the term or provide some sort of definition/explanation.
>
>
> Provider edge should actually be just “domain"
>

<de> OK.

> Section A.2.3: The last two sentences sound like they contradict each
> other. I assume the last sentence is refering to change in the Source
> mapping. Suggest "the mapping" -> "the Source mapping".
>
>
> Yes, is
>
> With this setup, the Proxy-ETR, by looking at the Source Map-Version Number,
>
> is able to check whether the mapping has changed.
>
>
<de> OK.

> EDITORIAL
>
> Section 1: "This operation is two-fold." -> "This information has two
> uses."
>
> New Section 6: "... MUST consist in an increment ..." -> "... MUST
> consist of an increment ..."
>
> New Section A.2.3: "uRPF" is used only once so the acronym should be
> dropped and only the expansion used.
>
> Will update as suggested in the next revision.
>

<de> OK.

<de> Thanks,
Donald
===============================
 Donald E. Eastlake 3rd   [phone] (cell)
 2386 Panoramic Circle, Apopka, FL 32703 USA
 [e-mail]

Thanks again for the review.
> Let me know if the proposed changes address your comments.
>
> Ciao
>
> L.
>
>
>
>
>
> Thanks,
> Donald
> ===============================
> Donald E. Eastlake 3rd   [phone] (cell)
> 2386 Panoramic Circle, Apopka, FL 32703 USA
> [e-mail]
>
>
>