On 24/05/2017 03:19, Mirja Kühlewind wrote:
...
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> 1) Use of transport protocols is not sufficiently defined

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

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

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

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

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

> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------

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

    Brian
 
> 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!)!
> 
> 
> 

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

Reply via email to