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

[Alvaro Retana <[e-mail]>]

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]