Subject: [Anima] Mirja Kühlewind's Discuss on draft-ietf-anima-grasp-12: (with DISCUSS and COMMENT)

From: Mirja Kühlewind <[email-masked]>
Date: Tue, 23 May 2017 08:19:22 -0700

Mirja Kühlewind has entered the following ballot position for
draft-ietf-anima-grasp-12: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-anima-grasp/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

1) Use of transport protocols is not sufficiently defined
Especially the following text in section 3.5.3 seems not to reflect later
assumptions correctly; it seems to be assumed that TCP is used for all
messages other than the discovery and therefore reliable transport is
provided for these message (see sections 3.5.5, 3.8.4 and 3.8.5):
"All other GRASP messages are unicast and could in principle run over
   any transport protocol.  An implementation MUST support use of TCP.
   It MAY support use of another transport protocol but the details are
   out of scope for this specification.  However, GRASP itself does not
   provide for error detection or retransmission.  Use of an unreliable
   transport protocol is therefore NOT RECOMMENDED."

In general the usage of the transport protocols is not well enough
specified, see also Spencer's comments and this part of Martin's tsv-art
review (Thanks!):
"* Usage of UDP: This document is not discussing any of the aspects in
RFC 8085. Every usage of UDP is required by IETF consensus to review RFC
8085 and to address at least the applicable subset of issues listed in
RFC 8085 (or the predecessor RFC 5405).

* Starting with UDP and switching to TCP for the data transfer looks like
the right do. However, UDP should be really only used to discover other
devices, but not piggy back further protocol mechanics. However, this
document is not really specific on how to make use of TCP, for instance,
how long are TCP connections kept open or closed down after a protocol
exchange (persistent vs temporary connections). What happens if a TCP
connection is shutdown by one end or is forcefully closed, e.g., by a
reset?"

I would recommend, as assumed in the rest of the document, to update
section 3.5.3 to only use UDP for the initial recovery message and open a
TCP connection for the discovery response and require that all other
messages to be sent over TCP (also removing any option to use any other
reliable transport because TCP seems to be the right choice here.)
Further, additional guidance is needed when to open and close a TCP
connection (or keep it alive for later use) and what to do if the
connection is interrupted.

2) Time-out handling
section 3.5.4.4: "Since the relay device is unaware of the timeout set by
the original
   initiator it SHOULD set a timeout at least equal to
GRASP_DEF_TIMEOUT
   milliseconds."
Should a relay really maintain an own time-out? Wouldn't it be sufficient
to just relay again if another discovery message is received. Otherwise
this can lead to an amplification, when the own time-out expires and
another relay message is sent when another discovery message is received
due to the time-out of the originating peer.

Further in relation to the point about, this should be more specific:
section 3.5.4.4: "Also, it MUST limit the total rate at which it relays
   discovery messages to a reasonable value, in order to mitigate
   possible denial of service attacks. "

3) Version and extensibility:
section 3.5.4.5: "A possible future extension
   is to allow multiple objectives in rapid mode for greater
efficiency."
How can this extension be defined if there is no version mechanism?

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Other mostly editorial comments:
- ASA needs to be spelled out in the intro.
- I would recommend to move section 2 and 3.3 into the appendix
- section 3.5.4.2: "A neighbor with multiple interfaces will respond with
a cached discovery response if any."
   "cached response" is explained in the next section and not clear in
this paragraph.
- section 3.5.4.3: "After a GRASP device successfully discovers a locator
for a Discovery
   Responder supporting a specific objective, it MUST cache this
   information, including the interface index via which it was
   discovered.  This cache record MAY be used for future negotiation or
   synchronization, and the locator SHOULD be passed on when
appropriate
   as a Divert option to another Discovery Initiator."
   Not sure why the first is a MUST and the later is a SHOULD. I guess a
SHOULD for caching would be sufficient.
- section 3.8.6 "If a node receives a Request message for an objective
for which no
   ASA is currently listening, it MUST immediately close the relevant
   socket to indicate this to the initiator."
   How is that indicated? Should really be further clarified
- Also section 3.8.6: "In case of a clash, it MUST discard the Request
message, in
   which case the initiator will detect a timeout."
   Why don't you send an error message instead? How does the initiator
know that is should retry (assuming there is a TCP connection underneath
that provides reliable transport)?
- Also section 3.8.9: "If not, the initiator MUST abandon or restart the
negotiation
   procedure, to avoid an indefinite wait."
  How does the initiator decide for abandoning or restarting instead?
Needs clarification!
- Could be useful to include an optional reasoning field in the Invalid
Message and make copying the received message up to the maximum message
size of this message a SHOULD (section 3.8.12.).
- Not sure I fully understand the purpose of the No Operation Message
(section 3.8.13.). If you just want to open a socket for probing, you
perform a TCP handshake and send a RST right after. No need for further
application layer interactions. And should there also be an optional
reasoning phrase?
- Not sure why the objectives flag is needed. I assume that unknown
objectives are ignored anyway and if a objective is known the receiver
should know if that objective is valid for the respective message type
(section 3.10.2).
- section 3.10.4: "An issue requiring particular attention is that GRASP
itself is a stateless protocol."
   It's not. It caches information and needs to remember previous
messages sent to reply correctly.
- section 5: "Generally speaking, no personal information is expected to
be
      involved in the signaling protocol, so there should be no direct
impact on personal privacy."
   I don't think this is true because the protocol is so generic that you
cannot say anything about the services it is used for.
Please see also further comments from Martin's tsv-art review (Thanks
again!)!

---

From: Brian E Carpenter <[email-masked]>
Date: Wed, 24 May 2017 08:34:54 +1200

On 24/05/2017 03:19, Mirja Kühlewind wrote:
...

At this point I think we are only talking about wording; the
intention of the -12 version is pretty much what you say below,
following some earlier IESG comments. (Martin reviewed the -11 version.)

> due to the time-out of the originating peer.

Yes, there is an issue in this area, which some very recent testing of the
prototype showed up. I will propose text next week.

> How can this extension be defined if there is no version mechanism?

Simply by adding it - the other end sends back M_INVALID if it doesn't
understand. That goes for all potential extensions.

Will deal with the rest next week when back from vacation.

    Brian

---

From: Michael Richardson <[email-masked]>
Date: Wed, 24 May 2017 12:52:11 -0400

Mirja Kühlewind <[email-masked]> wrote:
    > a TCP connection for the discovery response and require that all other

Some parts of GRASP are really begging for SCTP :-)
I wish it was more clearly an option.... no firewalls in the ACP to screw it up.

    > 3) Version and extensibility: section 3.5.4.5: "A possible future
    > extension is to allow multiple objectives in rapid mode for greater
    > efficiency."  How can this extension be defined if there is no version
    > mechanism?

:-)

Try the new extension and get back M_INVALID if not suppored.
I insisted upon M_INVALID for this reason.

-- 
Michael Richardson <[email-masked]>, Sandelman Software Works
 -= IPv6 IoT consulting =-

---

From: Brian E Carpenter <[email-masked]>
Date: Thu, 25 May 2017 07:53:12 +1200

> I wish it was more clearly an option.... no firewalls in the ACP to screw it up.

I tend to give the same answer as for UDP (or COAP): let's get the basic spec
out and then discuss whether to add GRASP-over-foo specs.

If GRASP succeeds, that will be an obvious next step.

   Brian

> _______________________________________________
> Anima mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/anima
>

---

From: Brian E Carpenter <[email-masked]>
Date: Tue, 30 May 2017 14:45:28 +1200

Responding to Mirja's comments (her DISCUSS points were
already discussed in other messages):

It is ;-)

> - I would recommend to move section 2 and 3.3 into the appendix

Moving the requirements (s2): already asked the WG about this.

"High Level Design Choices" (s3.3): But this is a description
of the design. Are people confused by the word "choice" perhaps?
Just in case that's the issue, I will simply delete it and the
section will be "High Level Design".

>    "cached response" is explained in the next section and not clear in

It's hard to give an overview without deferring some details. We'll try
to simplify the text.

> SHOULD for caching would be sufficient.

Fair enough, although not implementing a cache would be a bad idea IMHO.

>    How is that indicated? Should really be further clarified

That's covered by one of Martin's comments - a transport session
failure indicates a failed negotiation or synchronization.
[BTW I'm impressed that you & Martin picked this up. I discovered
it experimentally with the prototype and we have a couple of API
error codes for it, but I failed to think about adding it to
the text until I saw Martin's comments.]

> that provides reliable transport)?
First of all, this is an incredibly rare event, even given the birthday
paradox: it's a collision of two random 32 bit numbers. So it really
isn't worth any extra machinery.

Second, an ASA always needs to be ready to retry anything in case of
failure. That's kind of the First Law of Autonomics: never give up.

>   How does the initiator decide for abandoning or restarting instead?

It's irrelevant here whether the initiator abandons or retries. All
this means is that when the timer pops, the negotiation has failed.
We will reword this accordingly.

> size of this message a SHOULD (section 3.8.12.).

By making the body ?any we certainly allow this. Also, ?any
could be a CBOR array like [reason, rcvd-message]

Personally I think we might leave this undefined until we have some
experience. This is another example where CBOR's flexibility is
a great asset.

> application layer interactions. And should there also be an optional

I hope you can agree it's harmless. (Actually your laptop probably
received some of these in Chicago, because we were running GRASP
sessions on the IETF network quite often.) The reason I found it
essential was to discover the port number assigned by the O/S
to a multicast socket, which you can only get by sending a
multicast and then using getsockname(). It seemed more civilised
to use a defined no-op than to just send a null packet.

> should know if that objective is valid for the respective message type

Discovery-only can be useful and the "dry run" flag is dynamic. Also
of course it is another hook for extensibility.

> messages sent to reply correctly.

Right, but it isn't transaction-safe (ACID) which I guess is what we meant.

> cannot say anything about the services it is used for.

Well, we can say what we *expect*, but you're correct. Since we are
requiring crypto, we should be OK anyway. Will reword slightly.

> Please see also further comments from Martin's tsv-art review (Thanks

Thanks
   Brian