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

[Donald Eastlake <[e-mail]>]

Hi Luigi,

Thanks for all the changes. It looks acceptable to me now.

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


On Thu, Jun 2, 2022 at 8:29 AM Luigi Iannone <[e-mail]> wrote:

> Thanks Donald
>
> Revision -13 includes the text suggested by Alvaro and the fix in section
> A.3
>
> Ciao
>
> L.
>
> On 2 Jun 2022, at 04:49, Donald Eastlake <[e-mail]> wrote:
>
> Hi Luigi,
>
> I think -12 is pretty good. The change suggested by Alvaro should be
> included. And I think the last sentence in Section A.3 in -12 should
> probably be a separate paragraph and the start should be "Note that ..."
> rather than "To be noted that ...". With those changes, I am OK with the
> draft.
>
> Thanks,
> Donald
> ===============================
>  Donald E. Eastlake 3rd   [phone] (cell)
>  2386 Panoramic Circle, Apopka, FL 32703 USA
>  [e-mail]
>
>
> On Wed, Jun 1, 2022 at 8:16 AM Luigi Iannone <[e-mail]> wrote:
>
>> Hi Donald,
>>
>> A new revision of the drafts has been submitted.
>> Here is the link to the rfcdiff:
>> https://www.ietf.org/rfcdiff?url2=draft-ietf-lisp-6834bis-12.txt
>>
>> Let  me know if this revision does not address your concerns.
>>
>> Thanks
>>
>> Ciao
>>
>> L.
>>
>> On 1 Jun 2022, at 10:45, Luigi Iannone <[e-mail]> wrote:
>>
>>
>>
>> On 1 Jun 2022, at 05:29, Donald Eastlake <[e-mail]> wrote:
>>
>> 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.
>>
>>
>> This last sentence is great. I will put it in section 7.
>>
>>
>>
>> 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?
>>
>>
>> This is similar to the comment made by Paul.
>> His suggestion is to change the SHOULD in a MUST. Would this work for you
>> as well?
>>
>>
>>
>>
>>> 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."
>>
>>
>> Thanks a lot, it reads much better.
>>
>>
>> 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."
>>
>>
>> Yes, that is again absolutely correct. I will add the sentence.
>>
>>
>> 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.
>>
>>
>> Thanks again for the feedback.
>>
>> Ciao
>>
>> L.
>>
>>
>>
>>
>> <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]
>>>
>>>
>>
>