Subject: Review of draft-ietf-nsis-ntlp-11-txt

From: "Adrian Farrel" <[email-masked]>
Date: Sat, 23 Sep 2006 17:08:17 +0100

Hi Ross,

Here is my review of draft-ietf-nsis-ntlp-11.txt.

Although I have some doubts about the motivation for the
work, I have tried to concentrate on technical issues with
the protocol and the documentation. The concerns are
broken into technical and editorial in the lists below.

My review has been very quick which accounts for the
relatively small number of comments.

Please feel free to share these thoughts with the IESG or
the authors.

No doubt many of these issues have been discussed by the
nsis working group and satisfactory conclusions have
already been reached. I don't think there is a need to
enter into a debate on the points if that is the case,
but I would be happy to clarify any of my comments on
request.

Cheers,
Adrian

=========
Technical issues

----
Non-path-coupled signaling - Section 1 para 2
"concentrates mainly on path-coupled signaling". You raised
my hopes :-) But I think the document concentrates entirely
on path-coupled signaling.
We could note that there are only a few places where this
is actually enforced. For example, the Query, Response and
Confirm could easily be allowed to use an existing MA
(possibly set up on demand by a D-mode Query) and so out-
of-band signaling would be supported.
----
Section 1.1 - Multicast flows
You say:
   Multicast:  GIST does not handle multicast flows.
      This includes classical IP multicast and any of the
      small group multicast schemes recently proposed.
I think you might say that this is for future investigation
rather than say it is not supported. You may find that GIST
is perfectly capable of supporting multicast and is rather
a useful tool where all replication points are
GIST-capable.
----
Section 2 - Definitions
The definitions of routing and transport don't appear until
section 3.1. Given that these terms have subtle context-
specific meanings in nsis, and that readers familiar with
the current nature of the Internet may become confused, you
might consider moving these into section 2.

It would be helpful to define "routing state" in the
context of GIST.

The definition of "[data] flow" seems to be lacking. "A set
of packets" is pretty vague.
----
Section 3.1 - Routing
Does routing as described in this document determine how to
reach the adjacent GIST peer along the data path?
Surely either:
- routing (conventional IP routing) determines how packets
  are forwarded along the data path
or
- GIST routing as described in this document determines
  which is the adjacent GIST peer by probing nodes along
  the data path.
----
Section 3.2 - Connection mode
The motivation "fast state setup in the face of packet
loss" is suspect. What makes you think that you will be
able to set up and maintain a MA more rapidly in the
face of message loss than you would be able to squeeze
through a single UDP message?
----
Section 3.2 - D-mode encapsulation
You say that UDP is the only encapsulation which does not
require per-message shared state to be maintained between
peers. Raw IP seems to do this job as well. So your point
is that UDP is the only encapsulation with other properties
that you desire that...
----
Router Alert interception
I am concerned about this process as specified.
Perhaps my memory of IP is a little rusty?
And perhaps I am telling you how to suck eggs, but the
current text on this is very scattered and unclear.

RFC 2711 defines the Router Alert option for IPv6.
The option carries a two byte field that indicates that the
datagram includes a message of a particular protocol. The
IANA maintains a registry of values for this field, and
new NSLPs could certainly be added to this registry.
The RFC states that:
   The router's interest and the actions taken by employing
   Router Alert MUST be specified in the RFC of the
   protocol that mandates or allows the use of Router Alert
That means that each NSLP specification must describe the
processing - i.e. it is out of scope for GIST unless (and
this might be sensible) a single new value is defined to
indicate that the datagram carries a GIST message.
Obviously, if you do this you will need to make a new
request in section 9.

However, note well that a datagram that is intercepted
because of the Router Alert option will be delivered to the
component for processing the payload protocol - in this
case UDP, and UDP will deliver the GIST message to the
destination port on the local host. So you need to be
really careful in IPv6: if you register with your local
IPv6 stack that you want to receive intercepted Router
Alert-flagged datagrams, you need to make sure that the
GIST port is enabled or the UDP stack will drop/reject
packets.

IPv4 is slightly less friendly than IPv6. Although the
Router Alert option defined in RFC 2113 also has a two byte
field, only one value (zero) is defined and there is no
IANA registry of other values. In IPv4, an IP stack decides
to intercept a Router Alert-flagged datagram "shall examine
packets carrying it more closely (check the IP Protocol
field, for example) to determine whether or not further
processing is necessary." I think that you would be well-
advised to define the precise examination that you are
requiring in the case of GIST. Are you asking that all
flagged packets are delivered to UDP (as marked by their
IP Protocol field) even though it may be that many cannot
be delivered - perhaps they are not even GIST messages. Or
are you asking that the IP stack checks such flagged
datagrams for:
- IP protocol = UDP
- destination port = GIST default port

Bottom line:
- You must recall that there will be other users of
  Router Alert active in the network at the same time as
  GIST
- You must take care that if a datagram is intercepted
  and delivered to UDP, UDP will know what to do with it.
  UDP is not a forwarding protocol.
- You must define the correct processing for any IPv6
  Router Alert options in the "owning specification" or
  define a new codepoint for GIST.
----
Section 3.4
I think it would be worth discussing somewhere how the
upstream node knows when state has been successfully
installed at the downstream node if the downstream node
requested the use of Confirm.
In fact the answer is that it doesn't. It just goes ahead
and tries a message and if it doesn't get an error back
everything must be OK.
This is probably within the spirit of not optimising for
error conditions, but you might want to make it a bit
clearer.
----
Section 3.5 SIDs
The statement about whether NSIS implementations could
provide common functionality to generate SIDs is clearly
out of scope. I suggest removing it.
You need to add a couple of clear statements in this
section:
- Is the SID maintained end-to-end or does each NSLP
  that handles the message insert its own SID?
- What is the scope of uniqueness of the SID? Is it per
  node, or is it per NSLP ID?
----
Section 4
Is this section normative? It includes some 2119 language
(e.g., 4.3.4) and some text that appears to be definitive
(e.g. the bullets in 4.3.2). However, the GIS service
interface (section 4.1) is clearly not normative.
It would be helpful if the text made clear what is and is
not normative. In particular, some text in 4.1 like:
  This abstract service interface definition in no way
  constrains implementations, is non-normative, and is
  only provided as in formation to help understand how
  GIST might be used.
----
Section 4.1.2 Security
This section should discuss where the keys come from and
how they are distributed (or ref 8.2).
----
Section 4.2.1 para 1
Says: "The primary key" without reference to any other
keys.
----
Section 4.2.1
"All of this information is learned from prior GIST
exchanges". Well, once I had read the rest of the document,
I understood this!
Can you clarify that there are specific GIST exchanges that
are used to set up Message Routing State and that those
exchanges distribute this information.
----
Section 4.3.1 D-mode Normal encapsulation
"Each datagram contains a single message"
Didn't I see something about bundling somewhere?
----
Section 4.3.1 Q-mode
"usually with an IP router alert option"
How so "usually"?
----
Section 4.3.2 para 2
You say that the interaction is with signaling application
policy, but I think that the interaction is with the
signaling application, itself. The application is
responsible for applying policy. That is, GIST does not
interact with a policy component at this stage.
----
Section 4.3.2 para 2
(Perhaps a nit)
You say that GIST MUST propagate the Query. But consider
that the local node may be the destination of the Query.
----
Section 4.3.2 bullets
Given how these look to be normative, it might be best to
put bullet 3 before bullet 2. Otherwise there is a small
security hole because bullet 2 can be used to discover that
a given MRI/SID/NSLP-ID combination exists on a target node
----
Section 4.3.3 bullet 2
Worth saying why.
"...to preserve fragment ordering"
----
Section 4.3.4 - GIST Hop Count
A.4.4.2 shows clearly that an error is generated by a GIST
node if it decrements the hop count to zero on receipt. But
the definition in A.1 says:
   GIST hops (8 bits):  A hop count for the number of
   GIST-aware nodes this message can still pass through.
This is inconsistent. A message sent with a hop count of 1
will have the count decremented to zero on receipt
resulting in an error. So the value 1 did not represent the
number of nodes that the message could pass through.
In fact, there is no point in sending a message with hop
count 1.

In 4.3.4 you have
   2.  The GIST hop count has reached zero.  The error
       "Hop Limit Exceeded" (Appendix A.4.4.2) SHOULD be
       returned to the sender, which MAY retry with a
       larger initial hop count if it is clear that a
       loop has not been formed.
It is not explained how a receiver of a "Hop Limit
Exceeded" error can tell whether a loop has not been
formed. And it is not explained how the initiator of
such an error message could know, either.

Additionally, the statement (in several places, e.g.,
4.3.2) that the GIST hop count prevents looping is not
completely accurate. It prevents indefinite looping, but
it does not prevent looping per se.
----
Section 4.3.4 point 3
Is there a valid race where this can happen? For example,
when a signaling application crashes.
Suggest you add "during normal processing", or "this is an
error condition", or remove "...this should never happen"
as you go on to describe how to handle it anyway.
----
Section 4.4
This section appears to say that an MA cannot exist without
some Routing State. Is this the case, and if so why?
Surely it would be helpful to prevent MA flap to allow an
implementation to retain an MA without Routing State in
case a subsequent usage became apparent.
Confusingly, section 4.4.2 describes MA "Re-use", but this
is meant to describe "multi-use" because the MA is assumed
to already be in use. Perhaps you could change the title
and text in 4.4.2?
Note that a receiver that does not install routing state
until it receives a Confirm, may have an MA set up with no
associated routing state. And such an MA could be assigned
for use for other routing state.
----
Section 4.4.1 page 29
"about the node itself"
Which node?
----
Section 4.4.2 Peer-Identity
s/uniqueness between peers/uniqueness across the set of
all potential peers/

Should you suggest a source of this value since the
uniqueness will depend on all implementations selecting
a value in the same way. Perhaps you could suggest the
use of the Router ID, or some-such.
----
Section 4.4.2 bullet 1
"This will only fail..."
This is a subjective measure of failure.
What you actually mean is "This will only happen..."
But if this can happen (and if it can be done by an on-path
attacker) you need to describe how to recover/protect.
----
Section 4.4.2 bullet 2
"These should be rare events..."
Not only is "rare" subjective, but I don't see how you can
say that hijack attempts should be rare. So the protection
against this type of attack seems to be that you allow a
new MA to be set up and then let it time out.
----
Section 4.4.3 Soft State Concerns for Routing State

You should think carefully about the soft state scalability
concerns of RSVP (see RFC 2961). The Routing State
maintenance process that you describe here and expand on in
Sections 6.2 and 6.3 scale linearly with the number of
Routing States. Discussing rate limiting here is
interesting: how many Routing States would be needed before
rate limiting caused some state not to be refreshed?

The text at the end of 4.4.3 about refresh timing is good,
but conflicts slightly with paragraph 2 where:
   The Querying node MUST generate a Query before this
   timer expires
since what you really mean is that the Query node must act
to ensure the delivery of a Query before the time expires
on the Responder.

Given the impact of UDP rate limiting on top of unreliable
message delivery, you should probably be stronger in your
recommendation of suitable timer values.
----
Section 4.4.3 Leveraging Soft State NSLP for MA keepalive
In the event that the NSLP is a soft state protocol or
contains keep-alive messages, you may want to consider
leveraging those transactions in the setting of the
MA-Hold-Time. This would require that a suitable value for
the MA-Hold-Time was suggested by the NSLP application.
----
Section 4.4.3 TCP failure detection
One use of the MA-Hello may be the converse of what is
proposed. As currently stated, the message keeps alive an
MA that would otherwise be closed down. But another
possibility is that the TCP connection that underlies the
MA fails without being reported to the GIST implementation.
In this case, the implementation would not become aware of
the fact until it next tried to send, or until the hold
time expired.
Although not fatal, such a state of affairs delays delivery
of NSLP messages until the MA can be re-established.
Therefore MA-Hello could be used to detect MA failures. But
that would place quite a different timescale on the rate of
sending.
----
Parallel MAs
Not sure that I saw any comment on the correct use of
parallel MAs. I assume that the rules are that all messages
associated with one flow must be sent on the same MA to
avoid message re-ordering.
Can you make sure that this is stated somewhere.
----
Section 5.1 Confirm
   Confirm: A Confirm may be sent in D-mode, or C-mode if a
   messaging association has been re-used.
The implication of this text is that I can send the Confirm
and pipeline setting up the MA.
But the last para of 5.7.1 says that the Confirm is sent
over the MA.
The table in 5.5 supports 5.1, not 5.7.1.
----
Section 5.1 MA-Hello
A bit obvious, but you may want to recommend (or must) that
an MA-Hello sent in response to an MA-Hello that contains
R=1 does not itself contain R=1.
----
Section 5.2.2
Helpful to state here whether the presence of a duplicate
object is to be ignored or treated as an error.
The former would be better for forward extensibility, but
either would be acceptable.
----
Section 5.3.3
The final paragraphs describe good techniques for
implementations to protect the network. Are there also
mechanisms for nodes to protect themselves? Such as rate
limiting on the receive side.
----
Section 5.6
How will we handle an error introduced by a GIST forwarding
node? Will the error be reported to the forwarding node or
to the originator? If the latter, is this:
- helpful
- a security weakness
----
Section 6.1 and Section 8.4
A DoS attack can come from any node in the network.
Such a node can send a Q-mode Query message as though it
was forwarding it at the GIST level. The result, from any
GIST capable node that intercepts it, may be a D-mode
Response direct to the (spoofed) Query node. The Query
node will handle the Response (according to Rule 2) by
sending a "No Routing State" error message.
Please consider whether a Responder node should be
recommended to keep track of such events and rate limit
processing of new Query messages under such circumstances.
----
Section 6.2
Can I receive er_NoRSM in Awaiting Refresh state?
I think so because my sent Query refresh may have been
lost, the peer may have timed out the state, and I may
have sent Data.
----
Section 6.2
to_Inactive_QNode is not possible in Awaiting Response
state because the timer is not running.
----
Section 6.2
What about the receipt of other errors? Especially fatal
errors.
----
Section 6.2 Rule 3
This action needs to check to see if Confirm was requested.
Should not send a Confirm if one was never requested.

What if the er_NoRSM is sent in response to the Confirm?
This is opening a tight loop! (Such could arise if the
peer has 'lost' the Routing State.

Also, if you have sent several (say 100) Data messages
immediately after a lost Confirm, you will receive 100
er_NoRSM messages and (according to this state machine)
you will send 100 new Confirm messages.

This is not just a mater of implementation optimisation
because you are impacting the network and the adjacent
nodes. Further, this state machine is presented as
normative so variations from it will be tested by
certification labs.
----
Section 6.3 Rule 1
Say "set R=1"
----
Section 6.3 Rule 5
Say "set R=0"
----
Section 6.3
Can I change my mind about whether I want a Confirm
while I am waiting for one?
For example, in Awaiting Confirm state I may receive a
new Query. Must I also set the R flag on the Response
or can I clear the flag and move to Established state?

Similarly, during the refresh processing (since the MA
is now set up and I no longer need to see the Confirm)
can I clear the R flag on refresh Responses? I think the
state machine allows this. But once I am in Awaiting
Refresh state, can I change my mind and send R=0 on my next
Response?
----
Section 6.3
I can receive a Confirm in Established state
----
Section 6.3
to_Expire_RNode is not possible in Awaiting Confirm
state because the timer is not running.
----
Section 6.4
Probably need to support tg_RawData in Connected state :-)
----
Section 6.4
er_MAFailure is not possible in Awaiting Connection state.
----
Section 6.4 Rule 8
Should there be some feedback to the Message State machines
or can we rely on them to fail to find an MA next time they
try?
What about potentially lost messages?
----
Section 7.1.1
"On the assumption"
Can you include the reference, please.  [26]?
----
Section 7.1.4
As demonstrated by Figure 9 and the text in 7.1.1, rapid
local repair may not be such a good idea. It will cause the
GIST state to thrash around and new MAs to be set up for no
purpose because the new flow path is actually an entirely
different route from the head-end.
Thus, in the figure, any action taken immediately by (say)
D1 is a waste of time because the new flow actually does
not touch D1.
Shouldn't you let the flow routing protocol (IGP?) converge
first and let the NSLP drive the repair through its
retransmissions/refreshes/explicit messages?
----
Section 9
The use of reserved blocks within the codepoint registries
appears non-standard. The word "Reserved" does not appear
in RFC 2434.
I would suggest the use of "IETF consensus" with an
additional note saying that the block is reserved for
future expansion. If you do not do this, IANA will not know
under what circumstances to open up the block for
allocation.
----
Section 9 - Object Types etc.
There is a note (using RFC 2119 language, which is not
appropriate in the IANA section) about what must be defined
if a new object type (etc.) is defined. You need to say
where those definitions are expected to be lodged. It reads
like you  are asking IANA to keep the definitions in the
registries.
----
Section 9 - Error classes
IANA will find it helpful if you make clearer that the
2-byte error code is split into an error class and an error
code. You are asking them to keep a registry of the 1-byte
error code as listed in A.4.4 and to track the error class
as a parameter of that registry.
As currently presented, IANA might reasonably assume that
the error codes 0x0101 and 0x0201 are different error
values
----
Section 11.1
draft-loughney-nsis-ext-02.txt seems to have expired.
Do you really want this RFC to be gated on the development
of a personal I-D?
If material in that I-D is normative to GIST (and it
appears that that is the case) you should consider moving
that material into the main GIST specification.
----
Section 11.2
A reference to draft-ietf-nsis-ntlp-statemachine-02.txt
would be a good idea.
----
Section A.2.1
The extensibility flags are very sensible. But the
definition of 10 is not clear. If the message is to be
delivered to an NSLP application, how are objects of this
type handled?
Note that I think the processing described in this section
is normative to the protocol and should appear in the body
of the document.
----
Section A.3.1.1 - Flow Label
You define a "may only be set" for the F flag without
describing how to handle the Flow label (presumably ignore)
if the F flag is set.
----
Section A.3.1.1 - S flag
You say that the S flag can only be set if P=1. How to
process the SPI if P=0?
----
Section A.3.1.1 - A/B flags
You say that these flags can only be set if P=1. How to
process the ports if P=0?
----
Section A.3.3 - Routing state validity time
Did you consider allowing zero for "always valid"?
----
Section A.3.4
Need to say what happens in Prof-Count = 0
The current text says:
  "If there are no profiles (i.e. all bytes are null),"
Should this say "if there are no protocol layers in a
profile"?
The description of the padding is fine, but I presume a
receiver MUST ignore all bytes beyond the length indicated
by the layer count field.
----
Section A.3.5
Can MPO-Count be zero? Should say so for clarity.
Did you consider allowing zero for "hold open indefinitely"
Did you consider that different hold times might be
applicable to different MAs according to the protocols in
use?
----
Section A.3.8 - List of translated objects
Please say that the elements of this list are the object
type values defined for use in section A.2.
Please indicate whether the A, B and reserved flags are
relevant.
----
Section A.3.9
Please state padding rules.
----
Section A.4.1
I suspect it is the intention that the Additional
Information objects are encoded as sub-objects (i.e. as
TLVs) since otherwise, parsing the data is impossible
and future extensibility is compromised.
But where is the list of object type values, and shouldn't
IANA manage them?
----
Section A.4.3
Do you want to distinguish between protocol errors (i.e.
messages received at the wrong time) and message
formatting errors?
----
Section A.4.7
I should have thought that the class of this error is
a Permanent-Failure. Or do you plan that a Response will
also be sent even though the NSLP ID cannot be matched?
If no response, then according to Figure 7, the receiver
state machine is stuck. If the error class is Informational
then according to figure 6, the sender state machine is
stuck.
----
Section A.4.9 - Error sub-code 2: Missing Object
How will you identify which object was missing?

=========
Editorial issues

----
Abstract
s/universal/common/
----
Section 2 - Session Identifier
The word "long" is a bit too subjective for inclusion in a
protocol spec. Can you find a better term?
----
Section 3
There is various 2119 notation in this section. I think the
section is not a normative protocol definition so all uses
should go to lower case. But if I am wrong, then the many
lower case uses need to be changed.
----
Section 3.2 - Connection mode
s/objects/messages/
----
Section 3.3 page 13 3rd para
   These updates may be done in separate documents as is
   the case for the base GIST specification, as described
   in Section 7.2.2.
s/base/NAT traversal/
s/7.2.2./7.2.2 and [40]./
----
Section 3.3 page 13 4th para
s/enforce/allow/
----
Section 3.4
   The Data message is used purely to encapsulate signaling
   application data.
Not to deliver it? :-)
----
Section 3.7 point 3
s/towards/to/
----
Section 3.7 point 8
There is a reference to step 6.B, but step 6 has no such
labeling.
----
Section 4, para 1
Delete "in the first place"
----
Section 4.1.1, para 2
Delete "directly"
----
Section 4.1.2 para 1
s/security related/security-related/
----
Section 4.3.4
s/MUST never/MUST NOT/
----
Section 4.4.1
Missing reference in empty square brackets "[]"
----
Section 4.4.2 bullet 1
"appropriate" should be replaced with something more
specific.
----
Section 4.4.3 bullet 2, para 2
s/retain/create/
----
Section 5.2.1
It might be nice to order the fields described here in the
same order as they appear in the message. See A.1
----
Section 5.2.2 GIST-Error-Data
s/determine/report/
----
Section 5.8.1.2
s/vital/essential/
----
Section 6.3 Rule 2 and 8
s/piggybacked/NSLP/
----
Section 9 - MA Protocol IDs
You say "upper layer protocol", but I think "transport
protocol" would be more appropriate.
----
Section 11.2
I think [26] and [27] might usefully be normative.
----
Section A.2.1
s/containing and/containing an/
----
Section A.3.1.1 - P flag
s/IP Protocol/Protocol field/
---- 
Section A.3.1.2 - D flag
Need to back-reference A.3.1.1 for the definition
----
Section A.3.3
Odd ordering to the field descriptions.
----
Section A.4.10 para 1
s/packet/message/
----
Section D.
Suggest adding an RFC Editor note to have this appendix
deleted before publication as an RFC.

---

From: Ross Callon <[email-masked]>
Date: Sat, 23 Sep 2006 22:37:39 -0400

Adrian;

Thanks for the very careful detailed comments (although I
suppose that per page of spec there aren't really all *that*
many comments). I have entered these in the ID tracker
(as part of the "DISCUSS" vote that I was about to enter
even before seeing these comments).

I will be on vacation and probably largely out of email
contact for the next week, but of course you might hear
from the authors before I get back.

Thanks, Ross

At 05:08 PM 9/23/2006 +0100, Adrian Farrel wrote:
>---- Section A.3.1.2 - D flag