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

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

Hi Alvaro,

Thanks again for your review and apologies for the delayed response!

Please find comments inline. There was some conversation on the list between you and Dino where some agreements were made. Below I refer to the outcome of that conversation as “Alvaro/Dino discussion” where appropriate.

Thanks!
Alberto

From: Alvaro Retana <[e-mail]>
Date: Monday, April25 2022 at 6:01 PM
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.

[AR] As per the Alvaro/Dino discussion, we’re updating the PubSub doc to be consistent with 9301(6833bis) regarding proxy Map-Reply.

[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?

[AR] If the xTR-ID is not included, the Map-Request will be processed as a regular one. If the Map-Server is not proxy-replying, the Map-Request will be forwarded to the ETR (as per 9301) and any PubSub details on the Map-Request will be ignored by the ETR. Following the Alvaro/Dino discussion, we’re adding the text: “If either assumption is not met, a subscription cannot be established, and the network will continue operating without this enhancement.”


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

[minor] s/distribution/configuration

[AR] Ack

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


[AR] Ack

[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?

[AR] As per the Alvaro/Dino discussion, we’re adding some text along the lines of: “If the I-bit is set but the Site-ID and/or xTR-ID are not included, a receiver can detect the error because after processing that last EID-record, there are no bytes left from processing the message. The receiver will log a malformed Map-Request and drop the message.”


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

[AR] Ack

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".

[AR] Both lowercase ‘s’ and uppercase ‘S’ were already taken in the Map-Request, that’s why the bit is called ‘N’ and hence the name of the bit is “Notification-Requested bit”.

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].


[AR] Ack

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].


[AR] Ack

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.


[AR] This is technically correct, a Map-Request could have the I-bit set (+ xTR-ID + Site-ID) and yet not include any records with the N-bit set. This is still conformant with the specification, if no N-bit is set in the Map-Request, the message will just be processed as a regular Map-Request according to 9301 (the I-bit and IDs will just be ignored).

...
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:


[AR] Ack, thanks!

...
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.


[AR] As mentioned in the Alvaro/Dino discussion, regular Map-Notifies as per 9301 are sent to the source IP address of the Map-Register that triggered them. Since PubSub extends LISP to specify that Map-Notifies can also be triggered by Map-Requests, we need to specify where these Map-Request-triggered Map-Notifies are sent to.

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.


[AR] Maybe we can move the second mention right after the first one?

OLD>
 (2)  The Map-Server MUST use its security association with the xTR
       (see Section 7.1) to compute the authentication data of the Map-
       Notify.

NEW>
 (2)  The Map-Server MUST use its security association with the xTR
        (see Section 7.1) to sign the authentication data of the Map-
        Notify. The xTR MUST use the security association to verify
        the received authentication data.

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.


[AR] Ack

...
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.


[AR] At this point the spec has already covered the part about sending the Map-Notify. I agree maybe this paragraph is not correctly located, maybe we can move it up, and relocate it right after the paragraph at line 233.

...
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:


[AR] Ack, thanks!

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.


[AR] As discussed in the related comment in §5, sending Map-Notifies to ITR-RLOCs extracted from a Map-Request is new behavior introduced by PubSub. Please also note that while §5 talks about Map-Notifies sent to ack a subscription, §6 here talks about Map-Notifies sent as publication. Probably better to be explicit here IMHO.

...
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.


[AR] Following Alvaro/Dino discussion, we’re removing this sentence from the doc.

[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?


[AR] As long as the nonce is greater it is fine. It doesn’t need to be just by one (e.g. to cover cases where Map-Notifies might get lost on their way to the xTR).

[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.


[AR] Sounds good, we’re updating the doc to simply point to §5.6 in 9301 as follows:

OLD>
                                                                                          […] To compare
two nonces, the xTR uses the serial number arithmetic defined in
[RFC1982] with SERIAL_BITS = 64.  The nonce field space (64 bits) is
considered large enough to not be depleted during normal operation of
the protocol (e.g., assuming a fast publication rate of one Map-
Notify per EID-Record per Map-Server per second, the nonce field
space will not be depleted in 0.5 trillion years).  The same
considerations discussed in Section 5.6 of [I-D.ietf-lisp-rfc6833bis]
regarding storing Map-Register nonces apply here for Map-Notify
nonces.

NEW>
The same considerations discussed in Section 5.6 of [RFC9301]
 regarding Map-Register nonces apply here for Map-Notify nonces.

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.


[AR] Agreed, we’re removing this sentence from the doc.

[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.


[AR] Please note that 9301 does not talk about trying to send a Map-Notify to a different ITR-RLOC, since in 9301 Map-Notifies are not sent to ITR-RLOCs, hence the need to define this 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?


[AR] Since ETR don’t have a cache, they are not subject to this. This only applies to ITRs (and other ITR-like entities, such as RTRs). In practice, it is rare to see an ETR that is not also an ITR, so it is fine to say xTR here, IMHO.

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.


[AR] Ack, good catch!

...
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.


[AR] Fair point, we’re updating the text, see next comment.


[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?


[AR] Following your comment here and above, we’re updating the text as follows:

OLD>
Misbehaving nodes may send massive subscription requests which may
lead to exhaust the resources of Map-Servers.  Furthermore,
frequently changing the state of a subscription may also be
considered as an attack vector.  To mitigate such issues, xTRs SHOULD
 rate-limit Map-Requests and Map-Servers SHOULD rate-limit Map-
 Notifies.  Rate-limiting Map-Requests is discussed in Section 5.3 of
 [I-D.ietf-lisp-rfc6833bis] and the same guidelines apply here.  To
 rate-limit Map-Notifies, a Map-Server MUST NOT send more than one
Map-Notify per second to a particular xTR-ID.  This parameter MUST be
 configurable.

NEW>
Misbehaving nodes may send massive subscription requests which may
lead to exhaust the resources of Map-Servers.  Furthermore,
frequently changing the state of a subscription may also be
considered as an attack vector.  To mitigate such issues, Section 5.3 of
[RFC9301] discusses rate-limiting Map-Requests and Section 5.7 of
 [RFC9301] discusses rate-limiting Map-Notifies.

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


[AR] Ack

[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?


[AR] Good point, how about this?

OLD>
Note that when the Map-Notify rate-limit threshold is
met for a particular xTR-ID, the Map-Server will silently discard
additional subscription requests from that xTR-ID.

NEW>
Note that when the Map-Notify rate-limit threshold is
met for a particular xTR-ID, the Map-Server will discard
additional subscription requests from that xTR-ID and will
fall back to [RFC9301] behavior when receiving a Map-Request
from that xTR-ID (i.e. the Map-Server will send a Map-Reply).

[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.]


[AR] Ack. Note that the text above implicitly limits the accepted messages (by falling back to RFC 9301)

...
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.


[AR] Ack

...
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”.


[AR] Ack, thanks!

...
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.


[AR] Ack

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.


[AR] We’re making this change:

OLD:
   The policy for allocating new bits from this sub-registry is
   Specification Required (Section 4.6 of [RFC8126]).
NEW:
   The policy for allocating new bits from this sub-registry is
   Specification Required (Section 4.6 of [RFC8126]).
   It is suggested that multiple designated experts be appointed for
   registry change requests.

   Criteria that should be applied by the designated experts include
   determining whether the proposed registration duplicates existing
   entries and whether the registration description is clear and fits
   the purpose of this registry. These criteria are considered in addition
   to those already provided in Section 4.6 of [RFC8126] (e.g.,
   the proposed registration must be documented in a permanent and readily
   available public specification).
   Registration requests are evaluated within a three-week review period on the advice of
   one or more designated experts.  Within the review period, the
   designated experts will either approve or deny the registration
   request, communicating this decision to IANA.
   Denials should include an explanation and, if applicable, suggestions
   as to how to make the request successful.

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.


[AR] Ack, thanks!

[EoR -09]

[AR] Thanks again for the great review!