Re: [lisp] AD Review of draft-ietf-lisp-pubsub-09

["Alberto Rodriguez-Natal (natal)" <[e-mail]>]

Hi Alvaro,

Thanks a lot for your review! We’ll parse your comments and get back to you.

One quick reply regarding the intended status. As discussed in Vienna, it seems that this one could be moved into Standards Track. The plan for the next iteration is to update the intended status to Proposed Standard.

Thanks,
Alberto

From: Alvaro Retana <[e-mail]>
Date: Tuesday, April 26, 2022 at 12:01 AM
To: [e-mail] <[e-mail]>
Cc: Luigi Iannone <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>
Subject: AD Review of draft-ietf-lisp-pubsub-09
Dear authors:

Thank you for your work on this document!  I have several comments
inline (see below).  I will wait for a revised ID before moving this
document forward.

Please reply inline to this message to expedite my review of any updates.


This document currently has an intended status of Experimental.  What
is the experiment?  How would you know if the experiment has been
successful?  When deploying this extension, what would you want
operators to experiment with?

Thanks!

Alvaro.


[Line numbers from idnits.]


...
129     3.  Deployment Assumptions

131        The specification described in this document makes the following
132        deployment assumptions:

134        (1)  A unique 128-bit xTR-ID (plus a 64-bit Site-ID) identifier is
135             assigned to each xTR.

137        (2)  Map-Servers are configured in proxy-reply mode, i.e., they are
138             solicited to generate and send Map-Reply messages for the
139             mappings they are serving.

[major] This is the only place, including rfc6830bis/rfc6833bis, where
"proxy-reply mode" is used.  Is this operation specified anywhere,
maybe using a different name?  This seems to be related to the
Map-Server being able to offer non-authoritative Map-Replies -- please
be specific.


[major] What happens if one of these assumptions is not met?  If the
rest of the specification is followed (setting the I and N bits, etc.)
what are the "failure scenarios" if the conditions are not met?


141        The distribution of xTR-IDs (and Site-IDs) are out of the scope of
142        this document.

[minor] s/distribution/configuration


144     4.  Map-Request PubSub Additions
...
190           xTR-ID bit (I-bit): This bit is set to 1 to indicate that a 128
191           bit xTR-ID and a 64 bit Site-ID fields are present at the end of
192           the Map-Request message.  For PubSub operation, an xTR MUST be
193           configured with an xTR-ID and Site-ID, and it MUST set the I bit
194           to 1 and include its xTR-ID and Site-ID in the Map-Request
195           messages it generates.

[nit] s/I bit/I-bit


[major] "MUST set the I bit to 1 and include its xTR-ID and Site-ID"

What should the receiver do if the I bit is set but the ID fields are
not included?


[major] IANA hasn't assigned this bit yet.  Please include a note with
a forward reference to §10.


197           Notification-Requested bit (N-bit): The N-bit of an EID-record is
198           set to 1 to specify that the xTR wants to be notified of updates
199           for that mapping record.

[nit] This text, and others, talk about notification, which is
correct.  However, the document is called "pubsub", so it might be
nice to remain consistent throughout can call this a "subscription".


201           xTR-ID field: xTR-ID is a 128 bit field at the end of the Map-
202           Request message, starting after the final Record in the message
203           (or the Map-Reply Record, if present).  The xTR-ID is used to
204           uniquely identify the sender of a Map-Request message.  The xTR-ID
205           is defined in Section 5.6 of [I-D.ietf-lisp-rfc6833bis]

[minor] Some of the description already exists in rfc6833bis.  Please
don't duplicate the specification.

NEW>
   xTR-ID field: If the I-bit is set, this field if added at the end
   of the Map-Request message, starting after the final Record in the
   message (or the Map-Reply Record, if present).  The xTR-ID is
   specified in Section 5.6 of [I-D.ietf-lisp-rfc6833bis].


207           Site-ID field: Site-ID is a 64 bit field at the end of the Map-
208           Request message, following the xTR-ID.  Site-ID is used by the
209           Map-Server receiving the Map-Request message to identify which
210           xTRs belong to the same site.  The Site-ID is defined in
211           Section 5.6 of [I-D.ietf-lisp-rfc6833bis]

[minor] Same as above.

NEW>
   Site-ID field: If the I-bit is set, this field is added at the end
   of the Map-Request message, following the xTR-ID.  The Site-ID is
   defined in Section 5.6 of [I-D.ietf-lisp-rfc6833bis].


213     5.  Mapping Request Subscribe Procedures

215        The xTR subscribes for changes for a given EID-prefix by sending a
216        Map-Request to the Mapping System with the N-bit set on the EID-
217        Record.  The xTR builds a Map-Request according to Section 5.3 of
218        [I-D.ietf-lisp-rfc6833bis] but also does the following:

220        (1)  The xTR MUST set the I-bit to 1 and append its xTR-ID and Site-
221             ID to the Map-Request.  The xTR-ID uniquely identifies the xTR.

223        (2)  The xTR MUST set the N-bit to 1 for each EID-Record to which the
224             xTR wants to subscribe.

[minor] If I understand correctly, it is ok for a Map-Request to have
the I-bit set (+ xTR-ID + Site-ID), but include no records with the
N-bit set.  Is that right?  If so, the mention of the N-bit in the
intro paragraph makes that a little confusing.


...
241        Notify message back to the xTR to acknowledge the successful
242        subscription.  The Map-Server MUST follow the specification in
243        Section 5.7 of [I-D.ietf-lisp-rfc6833bis] to build the Map-Notify
244        with the following considerations:

[major] The "MUST...with the following considerations" construct reads
as exceptions to an absolute requirement.  There's no need to require
using the base spec -- it is assumed!  Also, there are some
recommendations (SHOULDs) in rfc6833bis which end up in a Normative
conflict with a requirement.

OLD>
   The Map-Server MUST follow the specification in Section 5.7
   of [I-D.ietf-lisp-rfc6833bis] to build the Map-Notify with
   the following considerations:

NEW>
   The Map-Server builds the Map-Notify according to Section 5.7
   of [I-D.ietf-lisp-rfc6833bis] with the following considerations:


...
253        (3)  The Map-Server MUST send the Map-Notify to one of the ITR-RLOCs
254             received in the Map-Request (which one is implementation
255             specific).

[major] Does this need to be specified here?  Where are Map-Notify
messages sent to?  I couldn't find a specific answer, but it seems to
me that choosing a destination address should be pretty "basic"; i.e.
something that should have been specified in the base spec.


257        When the xTR receives a Map-Notify with a nonce that matches one in
258        the list of outstanding Map-Request messages sent with an N-bit set,
259        it knows that the Map-Notify is to acknowledge a successful
260        subscription.  The xTR processes this Map-Notify as described in
261        Section 5.7 of [I-D.ietf-lisp-rfc6833bis] with the following
262        considerations.  The xTR MUST use its security association with the
263        Map-Server (see Section 7.1) to validate the authentication data on
264        the Map-Notify.  The xTR MUST use the Map-Notify to populate its map-
265        cache with the returned EID-prefix and RLOC-set.

[minor] "MUST use its security association"

I know that the first mention was related to the Map-Server and that
now the same phrase is used with respect to the xTR.  It might be
better if the security statement (for the relationship) was made only
once.


267        The subscription of an xTR-ID to the list of subscribers for the EID-
268        Record may fail for a number of reasons.  For example, because of
269        local configuration policies (such as accept and drop lists of
270        subscribers), or because the Map-Server has exhausted the resources
271        to dedicate to the subscription of that EID-Record (e.g., the number
272        of subscribers excess the capacity of the Map-Server).

[nit] s/The subscription of an xTR-ID to the list of subscribers for
the EID-Record may fail for a number of reasons./The subscription of
an xTR-ID may fail for a number of reasons.


...
279        If an xTR-ID is successfully added to the list of subscribers for an
280        EID-Record, the Map-Server MUST extract the nonce and ITR-RLOCs
281        present in the Map-Request, and store the association between the
282        EID-Record, xTR-ID, ITR-RLOCs and nonce.  Any already present state
283        regarding ITR-RLOCs and/or nonce for the same xTR-ID MUST be
284        overwritten.

[major] ...and a Map-Notify MUST be sent, right?   The specification
of the subscription seems incomplete.


...
296     6.  Mapping Notification Publish Procedures
...
303        When a mapping stored in a Map-Server is updated (e.g., via a Map-
304        Register from an ETR), the Map-Server MUST notify the subscribers of
305        that mapping via sending Map-Notify messages with the most updated
306        mapping information.  The Map-Notify message sent to each of the
307        subscribers as a result of an update event MUST follow the exact
308        encoding and logic defined in Section 5.7 of
309        [I-D.ietf-lisp-rfc6833bis] for Map-Notify, except for the following:

[major] "MUST...except for" is a Normative conflict.

OLD>
   The Map-Notify message sent to each of the subscribers as a
   result of an update event MUST follow the exact encoding and
   logic defined in Section 5.7 of [I-D.ietf-lisp-rfc6833bis] for
   Map-Notify, except for the following:

NEW>
   The Map-Notify message sent to each of the subscribers as a
   result of an update event follows the encoding and logic
   defined in Section 5.7 of [I-D.ietf-lisp-rfc6833bis] for
   Map-Notify, except for the following:


311        (1)  The Map-Notify MUST be sent to one of the ITR-RLOCs associated
312             with the xTR-ID of the subscriber (which one is implementation
313             specific).

[major] Please see the related comment in §5.  Also, please specify
behavior only once.


...
331        When the xTR receives a Map-Notify with an EID not local to the xTR,
332        the xTR knows that the Map-Notify has been received to update an
333        entry on its map-cache.  Processing of unsolicited Map-Notify
334        messages MUST be explicitly enabled via configuration at the xTR.
335        The xTR MUST keep track of the last nonce seen in a Map-Notify
336        received as a publication from the Map-Server for the EID-Record.  If
337        a Map-Notify received as a publication has a nonce value that is not
338        greater than the saved nonce, the xTR drops the Map-Notify message
339        and logs the fact a replay attack could have occurred.  To compare
340        two nonces, the xTR uses the serial number arithmetic defined in
341        [RFC1982] with SERIAL_BITS = 64.  The nonce field space (64 bits) is
342        considered large enough to not be depleted during normal operation of
343        the protocol (e.g., assuming a fast publication rate of one Map-
344        Notify per EID-Record per Map-Server per second, the nonce field
345        space will not be depleted in 0.5 trillion years).  The same
346        considerations discussed in Section 5.6 of [I-D.ietf-lisp-rfc6833bis]
347        regarding storing Map-Register nonces apply here for Map-Notify
348        nonces.

[major] "Processing of unsolicited Map-Notify messages MUST be
explicitly enabled via configuration at the xTR."

rfc6833bis added the Map-Notify-Ack, but it doesn't require
configuration anywhere to process unsolicited Map-Notify messages.
IOW, this requirement is not in line with rfc6833bis.


[major] "has a nonce value that is not greater than the saved nonce"

#2 above says that the "Map-Server increments the nonce by one every
time".  Does this mean that the received nonce value has to be
greater, but just by one?


[major] "To compare two nonces, the xTR uses the serial number
arithmetic defined in [RFC1982] with SERIAL_BITS = 64."

This sounds like something that should be specified in the base
specification.  I found at least one place in rfc6833bis (§5.6) where
the nonce is also incremented and needs to be compared, but no
specification of how.  Maybe a pointer to §5.6 is enough.

BTW, I know that some of the text was added in response to the SecDir
review.  I appreciate that, but, again, the text belongs in the base
specification, not here.


350        The xTR processes the received Map-Notify as specified in Section 5.7
351        of [I-D.ietf-lisp-rfc6833bis], with the following considerations.
352        The xTR MUST use its security association with the Map-Server (see
353        Section 7.1) to validate the authentication data on the Map-Notify.
354        The xTR MUST use the mapping information carried in the Map-Notify to
355        update its internal map-cache.  The xTR MUST acknowledge the Map-
356        Notify by sending back a Map-Notify-Ack (specified in Section 5.7 of
357        [I-D.ietf-lisp-rfc6833bis]), with the nonce from the Map-Notify, to
358        the Map-Server.  If after a configurable timeout, the Map-Server has
359        not received back the Map-Notify-Ack, it can try to send the Map-
360        Notify to a different ITR-RLOC for that xTR-ID.  If the Map-Server
361        tries all the ITR-RLOCs without receiving a response, it may stop
362        trying to send the Map-Notify.

[major] "The xTR MUST acknowledge the Map-Notify by sending back a
Map-Notify-Ack..."

This action is already specified in §5.7/rfc6833bis, but without the
same normative language.  In any case, please don't respecify
behaviors here.


[major] "If after a configurable timeout, the Map-Server has not
received back the Map-Notify-Ack..."   §5.7/rfc6833bis talks about
exponentially backed-off retransmissions.  You shouldn't change the
behavior here.


364     7.  Security Considerations
...
369        In the particular case of PubSub, cache poisoning via malicious Map-
370        Notify messages is avoided by the use of nonce and the security
371        association between the ITRs and the Map-Servers.

[?] Does all this apply to xTRs or just ITRs?


373     7.1.  Security Association between ITR and MS

[minor] I think this is the first place where "MS" is used.  Please
keep using the expanded form to avoid confusion.


...
424     7.2.  DDoS Attack Mitigation

426        Misbehaving nodes may send massive subscription requests which may
427        lead to exhaust the resources of Map-Servers.  Furthermore,
428        frequently changing the state of a subscription may also be
429        considered as an attack vector.  To mitigate such issues, xTRs SHOULD
430        rate-limit Map-Requests and Map-Servers SHOULD rate-limit Map-
431        Notifies.  Rate-limiting Map-Requests is discussed in Section 5.3 of

433        [I-D.ietf-lisp-rfc6833bis] and the same guidelines apply here.  To
434        rate-limit Map-Notifies, a Map-Server MUST NOT send more than one
435        Map-Notify per second to a particular xTR-ID.  This parameter MUST be
436        configurable.  Note that when the Map-Notify rate-limit threshold is
437        met for a particular xTR-ID, the Map-Server will silently discard
438        additional subscription requests from that xTR-ID.  Similarly, for
439        pending mapping updates that need to be notified to that xTR-ID, the
440        Map-Server will combine them into a single Map-Notify (with multiple
441        EID-records) which it will send when the rate-limit mechanism allows
442        it to transmit again Map-Notifies to that xTR-ID.

[major] "SHOULD rate-limit Map-Requests...discussed in Section 5.3 of
[I-D.ietf-lisp-rfc6833bis]"

§5.3/rfc6833bis requires rate-limiting, so the recommendation here is
not in line with that.  Please don't attempt to specify things here
again.


[major] "SHOULD rate-limit Map-Notifies...a Map-Server MUST NOT send
more than one Map-Notify per second to a particular xTR-ID."

§5.7/rfc6833bis already talks about sending unsolicited Map-Notify
message only in conformance with rfc8085.  I see no reason to specify
somehting different for this case.  Is there one?


[nit] There's an extra space between lines 431 and 433.


[major] "when the Map-Notify rate-limit threshold is met...the
Map-Server will silently discard additional subscription requests from
that xTR-ID."

Is meeting the threshold considered a subscription failure?  It sounds
like one to me!  §5 talks about what to do it the subscription fails,
but it is not to "silently discard additional subscription requests".
I'm assuming that a Map-Reply should be sent in this case, right?


[major] While it's a good idea to rate limit, a misbehaving node will
not necessarily follow these recommendations either.  A better
mitigation technique would be to rate limit the accepted messages, not
the sent ones.  [No need to include anything -- just an observation.]


...
482     10.  IANA Considerations

484        This document requests IANA to assign a new bit from the "LISP
485        Control Plane Header Bits: Map-Request" sub-registry under the
486        "Locator/ID Separation Protocol (LISP) Parameters" registry available
487        at [IANA-LISP].  The position of this bit in the Map-Request message
488        can be found in Figure 1.

490             +-----------+---------------+--------------+-------------+
491             | Spec Name | IANA Name     | Bit Position | Description |
492             +-----------+---------------+--------------+-------------+
493             | I         | map-request-I | 11           | xTR-ID Bit  |
494             +-----------+---------------+--------------+-------------+

[major] IANA hasn't yet assigned this bit yet.  Please make it clear
that this is the suggested value.


...
498        This document also requests the creation of a new sub-registry
499        entitled "LISP Map-Request Record Bits" under the "Locator/ID
500        Separation Protocol (LISP) Parameters" registry available at
501        [IANA-LISP].

[] The other registries are all called "LISP Control Plane Header
Bits: xxx".  Following that, here's a suggestion for this new
registry: "LISP Control Plane Header Bits: Map-Request-Record”.


...
513        Bits in position 2-8 are for future assignment.

[minor] s/Bits in position 2-8 are for future assignment./The
remaining bits are Unassigned.


515        The policy for allocating new bits from this sub-registry is
516        Specification Required (Section 4.6 of [RFC8126]).

[major] "Specification Required" required the review of a Designated
Expert.  Please provide any specific instructions that the DEs should
take into account when assigning values from this registry.


518     11.  Normative References
...
537        [IANA-LISP]
538                   IANA, "Locator/ID Separation Protocol (LISP) Parameters",
539                   <https://www.iana.org/assignments/lisp-parameters/lisp-
540                   parameters.xhtml>.

[minor] This reference can be Informative.

[EoR -09]