Hi, On 06.02.2013 05:21, The IESG wrote: > The IESG has received a request from the Peer-to-Peer Session Initiation > Protocol WG (p2psip) to consider the following document: > - 'REsource LOcation And Discovery (RELOAD) Base Protocol' > <draft-ietf-p2psip-base-24.txt> as Proposed Standard > > The IESG plans to make a decision in the next few weeks, and solicits > final comments on this action. Please send substantive comments to the > ietf@xxxxxxxx mailing lists by 2013-02-19. Exceptionally, comments may be > sent to iesg@xxxxxxxx instead. In either case, please retain the > beginning of the Subject line to allow automated sorting. Sorry for the late comments, but I re-read the whole draft which took some time. There are still some clarifications necessary as well as some inconsistencies left. Unfortunately, my comments from June, 19th were not addressed yet. https://www.ietf.org/mail-archive/web/p2psip/current/msg06229.html Polina Goltsman also found many issues listed below while working on an implementation. The list is long, but the good news is that it's mainly clarifications that are missing. Major issues: 1) sec. 6.3.2: length field in Forwarding Header covers what exactly? it is not really clear whether the length field counts the whole message or in case of fragmentation only the fragment size. When reading Sec 6.7 is seems that the former is meant, but the definition could and should be clearer. Similarly, sec. 6.7 should be clear about this, e.g., describing that all Forwarding Headers are identical for fragments of the same message with exception of the fragment field. 2) sec. 7.4.1.1: replica number handling is unclear it is unclear how replica numbers are incremented (e.g., per peer) and what receiving peers should actually do with this number. Is it important to store the value or would a boolean be sufficient so that the peer knows that it's a replica? 3) sec. 6.5.2: transport protocol set for AppAttach Applications may require use of other transport protocols than those defined in OverlayLinkType (TLS/DTLS, what about plain UDP, SCTP, DCCP, etc.), but currently, this seems to be not possible (do the considerations of sec. 6.5.1.6 apply here?). 4) sec. 6.3.4: How can a different signature algorithm be used if not all implementations support it? There is no possibility to provide the feedback, that the signature algorithm is not acceptable at a particular node. 5) sec. 10.5: handling of parallel JOIN requests and use of peer_ready it is not clear what should happen if two JNs try to join at the same time at the same AP (can they be processed in parallel or should they be processed in sequence). Furthermore, when MUST/SHOULD a JN send peer_ready Update - in step 9? Minor issues: citations are put first, followed by comments starting with # sec. 1.1 ======== old: storage rather than for bulk storage of large objects. Records are stored under numeric addresses which occupy the same space as node new: storage rather than for bulk storage of large objects. Records are stored under numeric addresses, called Resource-IDs, which occupy the same space as node # Resource-IDs are used in 1.2.2 but not introduced sec. 1.2 ======== Message Transport: Handles end-to-end reliability, manages request ... # What are the interactions with Forwarding and Link Management and the Topology Plugin old: Forwarding and Link Management Layer: Stores and implements the routing table by providing packet forwarding services between nodes. It also handles establishing new links between nodes, including setting up connections across NATs using ICE. # It may be confusing to say "routing table" here, since this # is usually associated with the topology plugin. So IMHO # it is the Connection Table, not the routing table. # Furthermore, I propose the following change: old: including setting up connections across NATs using ICE. new: including setting up connections for overlay links across NATs using ICE. old: directly between nodes. TLS [RFC5246] and DTLS [RFC6347] are the currently defined "link layer" protocols used by RELOAD for hop- new: currently defined "overlay link layer" protocols used by RELOAD for hop- # avoids confusion with the classic ISO/OSI link layer (layer 2) old: In addition to the above components, nodes communicate with a central new: In addition to the above components, nodes may communicate with a central # while it may be the default case, it is not strictly required sec. 1.3 ======== old: RELOAD also provides an optional shared secret based admission control feature using shared secrets and TLS-PSK. In order to form a new: RELOAD also provides an optional shared secret based admission control feature using shared secrets and TLS-PSK/TLS-SRP. In order to # TLS-SRP should be mentioned here, too sec. 2 ====== old: Terms used in this document are defined inline when used and are also new: Terms in this document are defined inline when used and are also # avoid double "used" old: Bootstrap Node: A network node used by Joining Nodes to help locate the Admitting Peer. new: Bootstrap Node: A network node used by Joining Nodes to help accessing the overlay by forwarding messages to peers. # The bootstrap node does not locate the Admitting Peer, but the JN locates # the AP by routing a message to its own Resource-ID old: Connection Table: The set of nodes to which a node is directly connected, which include nodes that are not yet available for routing. new: Connection Table: Contains connection information for the set of nodes to which a node is directly connected, which include nodes that are not yet available for routing. # it is a data structure ... Node-ID: A value of fixed but configurable length that uniquely identifies a node. Node-IDs of all 0s and all 1s are reserved and are invalid Node-IDs. A value of zero is not used in the wire protocol but can be used to indicate an invalid node in implementations and APIs. The Node-ID of all 1s is used on the wire protocol as a wildcard. # what means "invalid" exactly? A wildcard can be used at least on # the wire so it is not invalid being used as destination Node-IDs. # So the above definition is slightly contradictory. old: Responsible Peer: The peer that is responsible for a specific resource, as defined by the plugin algorithm. new: Responsible Peer: The peer that is responsible for a specific resource, as defined by the topology plugin algorithm. old: Usage: An usage is the definition of a set of data structures (data Kinds) that an application wants to store in the overlay. An usage may also define a set of network protocols (application IDs) new: Usage: A usage is the definition of a set of data structures (data Kinds) that an application wants to store in the overlay. A usage may also define a set of network protocols (application IDs) # Typo: 2x An usage -> A usage sec. 3.1 ======== old: o To determine its position in the overlay topology (if the overlay is structured; topology plugins do not need to be structured). new: o To determine its position in the overlay topology (if the overlay is structured; overlays do not need to be structured). # a structured topology plugin is not the same as a structured overlay o To determine the set of resources for which the node is responsible. # this isn't necessarily true for unstructured overlays? old: The general principle here is that the security mechanisms (TLS at new: The general principle here is that the security mechanisms ((D)TLS at sec. 3.2 ======== entity. From the perspective of a peer, a client is a node that has connected to the overlay, but has not yet taken steps to insert # an additional reference to the Connection Table would be nice sec. 3.2.1 ========== clients that choose this option need to process Update messages # Update messages are not introduced yet, so a forward reference to # section 6.4.2.3 would be helpful performing an Attach. A client wishing to connect using this mechanism with a certificate with multiple Node-IDs can use a Ping (Section 6.5.3) to probe the Node-ID of the node to which it is connected before doing the Attach (Section 6.5.1). # At this point it is not really clear why this step is necessary. # Certificates with multiple Node-IDs were not explained yet. # Furthermore, the reference to Section 6.5.1 should be moved # to the previous sentence. sec. 3.3 ======== old: This section will discuss the capabilities of RELOAD's routing layer, new: This section discusses the capabilities of RELOAD's routing layer, old: Resource-based routing: RELOAD supports routing messages based solely on the name of the resource. Such messages are delivered new: Resource-based routing: RELOAD supports routing messages based solely on the name of the resource or Resource-ID. Such messages old: Clients: RELOAD supports requests from and to clients that do not participate in overlay routing, located via either of the mechanisms described above. new: Clients: RELOAD supports requests from and to clients that do not participate in overlay routing. # the addition is not really necessary and may be confusing old: Destination Lists: While in principle it is possible to just inject a message into the overlay with a single Node-ID as the new: Destination Lists: While in principle it is possible to just inject a message into the overlay with a single Node-ID or a single Resource-ID as the # Resource-ID is also a possible destination old: The basic routing mechanism used by RELOAD is Symmetric Recursive. new: The basic routing mode used by RELOAD is Symmetric Recursive Routing (SRR, cf. Section 6.2) # I would prefer to use the term "mode" (as on p. 28) and it should be # consistent with the text in section 6.2 old: opaque ID X1 which maps internally to [A, B] (perhaps by being an encryption of [A, B] and forwards to Z with only X1 as the via list. new: opaque ID X1 which maps internally to [A, B] (perhaps by being an encryption of [A, B]) and forwards to Z with only X1 as the via list. # simple typo old: RELOAD also supports a basic Iterative "routing" mode (where the new: RELOAD also supports a basic Iterative routing mode (where the old: Iterative "routing" is implemented using the RouteQuery method, which new: Iterative routing is implemented using the RouteQuery method, which old: requests this behavior. Note that iterative "routing" is selected new: requests this behavior. Note that Iterative routing is selected sec. 3.4 ======== pairs. The result is a connection between A and B. At this point, A and B MAY send messages directly between themselves without going through other overlay peers. In other words, A and B are on each other's connection tables. They MAY then execute an Update process, # MAY is RFC 2119 terminology, which is defined in section 5 # Besides Update process a Join process is also possible. order to support this case, some small number of "bootstrap nodes" typically need to be publicly accessible so that new peers can # what "publicly accessible" means exactly is not defined # typically need to be publicly accessible (i.e., not behind a NAT or # firewall) ... old: The second case is when a client connects to a peer at an arbitrary IP address, rather than to its responsible peer, as described in the new: The second case is when a client connects to a peer at an arbitrary node-ID, rather than to its responsible peer, as described in the # since responsible peer may depend on the overlay topology, node-ID # seems to be a better fit here sec. 3.5.2 ========== old: When a new peer wishes to join the Overlay Instance, it will need a Node-ID that it is allowed to use and a set of credentials which new: When a new peer wishes to join the Overlay Instance, it needs a Node-ID that it is allowed to use and a set of credentials which # not really sure about this change as non-native speaker match that Node-ID. When an enrollment server is used, the Node-ID used is the Node-ID found in the certificate received from the # the mode with self-signed certificates is missing and should be # mentioned also here "bootstrap node". Because this is the first connection the peer makes, these nodes will need public IP addresses so that they can be # may also work if the bootstrap node is directly reachable, e.g., # in the same domain # in this paragraph and the following paragraph "Once a peer" # is used three times past adjacencies which have public IP address and attempt to use them # inconsistent use of term "adjacencies"/adjacent within the document # different meanings as follows: # 1.) all directly connected nodes (i.e., all nodes in the Connection Table), e.g., sec. 3.5.2 and 6.4.2.3 # 2.) all peers in the routing table # 3.) adjacent according to the overlay topology, e.g. sec. 6.4.2.1 sec. 4 ====== limits on size, on the values which may be stored. For many Kinds, the set may be restricted to a single value; some sets may be allowed # what set? sec. 4.1.2. =========== old: responsibility if the responsible peer fail [Chord]. new: responsibility if the responsible peer fails [Chord]. sec. 6 ====== old: messages. We then describe the symmetric recursive routing model, which is RELOAD's default routing algorithm. We then define the new: messages. We then describe the symmetric recursive routing mode, which is RELOAD's default routing mode. We then define the # IMHO the term mode fits best, the routing algorithm is defined # within the topology plugin sec. 6.1. ========= old: peer SHOULD generate an appropriate error but local policy can new: peer SHOULD generate an appropriate error message but local policy can Once the peer has determined that the message is correctly formatted # what does "correctly formatted" mean exactly? sec. 6.1.1. =========== this node so it MUST verify the signature as described in Section 7.1 and MUST pass it up to the upper layers. "Upper layers" is used here to mean the components above the "Overlay Link Service Boundary" line in the figure in Section 1.2. # this is somewhat confusing. The text describes what the # Forwarding and link management component does, but what # other components are meant here? state, e.g., by unpacking any opaque IDS. # I think any is incorrect, since other IDs are # not inserted by this node and so it cannot "unpack" those, # but only its own opaque IDs sec. 6.1.2. =========== the first entry on the destination list is in the peer's connection table, then it MUST forward the message to that peer directly. # This is probably motivated by clients. A hint to this fact # may help. old: destination list, it would detect that I is a opaque ID, recover the new: destination list, it would detect that I is an opaque ID, recover the # Typo fix old: called List Compression. Possibilities for a opaque ID include a new: called List Compression. Possibilities for an opaque ID include a # Typo fix An intermediate node receiving a request from another node MUST return a response to this request with a destination list equal to the concatenation of the Node-ID of the node that sent the request with the via list in the request. The intermediate node normally # 1.) unclear why an _intermediate_ peer should return a response # (if it is not the destination node), # 2.) the via list must be reversed before concatenating the Node-ID # so: old: with the via list in the request. The intermediate node normally new: with the reversed via list in the request. The intermediate node normally sec. 6.1.3. =========== old: compressed via list), the peer MUST replace that entry with the original via list that it replaced and then re-examine the new: compressed via list), the peer MUST replace that entry with the reversed original via list that it replaced and then re-examine the # the via list must be reversed for responses... sec. 6.2. ========= old: This Section defines RELOAD's Symmetric Recursive Routing (SRR) algorithm, which is the default algorithm used by nodes to route messages through the overlay. All implementations MUST implement this routing algorithm. An overlay MAY be configured to use alternative routing algorithms, and alternative routing algorithms MAY be selected on a per-message basis. I.e., a node in an overlay which supports SRR and some other routing algorithm called XXX might use SRR some of the time and XXX some of the time. new: This Section defines RELOAD's Symmetric Recursive Routing (SRR) mode, which is the default mode used by nodes to route messages through the overlay. All implementations MUST implement this routing mode. An overlay MAY be configured to use alternative routing modes, and alternative routing modes MAY be selected on a per-message basis. I.e., a node in an overlay which supports SRR and some other routing mode called XXX might use SRR some of the time and XXX some of the time. # better use mode for consistency (algorithm is contained in the # topology plugin sec. 6.2.1. =========== old: node MAY also construct a more complicated destination list for source routing. new: node MAY also construct a more complicated destination list for (loose) source routing. Once the message is constructed, the node sends the message to some adjacent peer. If the first entry on the destination list is directly connected, then the message MUST be routed down that connection. Otherwise, the topology plugin MUST be consulted to determine the appropriate next hop. # adjacent peer should be adjacent node, since it may be a client # directly connected means: a valid entry in the Connection Table # exists? this should be mentioned here... sec. 6.2.2. =========== # a hint that the same Transaction-ID as in the request MUST be used # could be added. sec. 6.3.1.1. ============= Unless a given structure that uses a select explicitly allows for unknown types in the select, any unknown type SHOULD be treated as an # How is that allowance specified in the specification? Is it by the comment # /* This structure can be extended */ sec. 6.3.2. =========== receive message with a TTL greater than the current value of initial-ttl (or the 100 default) MUST discard the message and send an "Error_TTL_Exceeded" error. # what if the initial-ttl is larger than 100 and the TTL is >100 but # < initial-ttl? The condition "or the 100 default" holds old: used to indicate the fragment offset; see Section 6.7. new: used to indicate the fragment offset in bytes; see Section 6.7. old: length: The count in bytes of the size of the message, including the header. new: length: The count in bytes of the size of the whole unfragmented message, including the header. # as already mentioned in the beginning this should be more precise destinations which the message should pass through. The destination list is constructed by the message originator. The # is it allowed that intermediate peers add destinations? if not, please # state so old: next. The list shrinks as the message traverses each listed peer. new: next. The list may shrink as the message traverses each listed peer. # it need not be always the case that the list shrinks with each traversed peer sec. 6.3.2.2. ============= old: structure with a DestinationType of opaque_id_type and a opaque_id new: structure with a DestinationType of opaque_id_type and an opaque_id # typo fix old: opaque A compressed list of Node-IDs and an eventual Resource-ID. Because this value was compressed by one of the peers, it is only meaningful to that peer and cannot be decoded by other peers. Thus, it is represented as an opaque string. resource The Resource-ID of the resource which is desired. This type MUST only appear in the final location of a destination list and MUST NOT appear in a via list. It is meaningless to try to route through a resource. new: resource The Resource-ID of the resource which is desired. This type MUST only appear in the final location of a destination list and MUST NOT appear in a via list. It is meaningless to try to route through a resource. opaque_id_type A compressed list of Node-IDs and an eventual Resource-ID. Because this value was compressed by one of the peers, it is only meaningful to that peer and cannot be decoded by other peers. Thus, it is represented as an opaque string. # 1.) match the order in the select # 2.) it must be opaque_id_type, not opaque sec. 6.3.2.3 ============ flags Three flags are defined FORWARD_CRITICAL(0x01), DESTINATION_CRITICAL(0x02), and RESPONSE_COPY(0x04). These flags MUST NOT be set in a response. If the FORWARD_CRITICAL flag is # What is the correct reaction if these flags are set in a response? # (returning an Error_Invalid_Message or ignore?) sec. 6.3.3.1 ============ old: A node processing a request MUST return its status in the message_code field. If the request was a success, then the message new: A node processing a request MUST return its status in the message_code field of a response. If the request was a success, then the message # clarification? Error_Request_Timeout: A response to the request has not been received in a suitable amount of time. The requesting node MAY resend the request at a later time. # not clear when this will ever be used. Which node should send # this error message? All RELOAD messages MUST be signed. Intermediate nodes do not verify signatures. Upon receipt (and fragment reassembly if needed) the destination node MUST verify the signature and the authorizing certificate. If the signature fails, the implementation SHOULD simply drop the message and MUST NOT process it. This check provides # What happens if none {0,0} is given? Then no Node-ID is present... sec. 6.4.2. =========== What happens if these messages (like Join, Leave) are accidentally sent to a Client? Do the send an Invalid Message back? A new peer (but one that already has credentials) uses the JoinReq message to join the overlay. The JoinReq is sent to the responsible peer depending on the routing mechanism described in the topology Is the destination address now the Resource-ID or the Node-ID of the "responsible peer"? Because joins may only be executed between nodes which are directly adjacent, receiving peers MUST verify that any JoinReq they receive # 1.) should be peers rather than nodes # 2.) directly adjacent means here: directly adjacent in the overlay # topology (could otherwise be misunderstood as being directly # connected) # 3.) what must happen if the verification fails? adjacent, receiving peers MUST verify that any LeaveReq they receive arrives from a transport channel that is bound to the Node-ID to be # what happens if that verification fails? old: assumed by the leaving peer.) This also prevents replay attacks new: assumed by the leaving peer. This also prevents replay attacks # Typo fix sec. 6.4.2.3 ============ the state change. In general, peers send Update messages to all their adjacencies whenever they detect a topology shift. # A hint to the Connection Table and Clients would clarify sec. 6.4.2.4. ============= old: X. A RouteQuery can also request that the receiving peer initiate an new: X. A RouteQuery can also request that the receiving peer initiates an old: One important use of the RouteQuery request is to support iterative routing. The sender selects one of the peers in its routing table # add reference to Section 3.3. sec. 6.4.2.4.1 ============== destination The destination which the requester is interested in. This may be any valid destination object, including a Node-ID, opaque ID, or Resource-ID. # Does opaque ID make sense here? sec. 6.5.1 ========== A node sends an Attach request when it wishes to establish a direct TCP or UDP connection to another node for the purpose of sending # TCP/TLS or DTLS? old: node A has Attached to node B, but not received any Updates from B, new: node A has attached to node B, but not received any Updates from B, # Typo channel but MUST NOT route messages through B to other peers via that channel. The process of Attaching is separate from the process of # Is that also true for clients? sec. 6.5.1.1 ============ old: } AttachReqAns; The values contained in AttachReqAns are: new: } AttachReq; The values contained in AttachReq are: old: A single AttachReqAns MUST NOT include both candidates whose new: A single AttachReq MUST NOT include both candidates whose # consistency! sec. 6.5.1.2. ============= old: 6.5.1.2. Response Definition new: 6.5.1.2. Response Definition The AttachAns message hast the same format as the AttachReq message. #s/AttachReqAns/AttachAns/ in the whole paragraph. sec. 6.5.1.3. ============= An agent follows the ICE specification as described in [RFC5245] with # agent was not defined in the RELOAD context so far. sec. 6.5.4.2. ============= old: o The configuration document is correctly digitally signed (see Section 11 for details on signatures. new: o The configuration document is correctly digitally signed (see Section 11 for details on signatures). old: one listed in the current configuration file). Details on kind- signer field in the configuration file is described in Section 11.1. new: one listed in the current configuration file). Details on kind- signer field in the configuration file are described in Section 11.1. # typos sec. 6.6.2 ========== old: Each connection has it own sequence number space. Initially the new: Each connection and direction has it own sequence number space. Initially the sec. 6.6.3.1 ============ A node MUST NOT have more than one unacknowledged message on the DTLS connection at a time. Note that because retransmissions of the same message are given new sequence numbers, there may be multiple unacknowledged sequence numbers in use. # Since retransmissions violate the first sentence, it may be better # to use: old: A node MUST NOT have more than one unacknowledged message on the DTLS connection at a time. Note that because retransmissions of the same new: A node MUST NOT have more than one unacknowledged message on the DTLS connection at a time, except for retransmissions. Note that because from the routing table. The link MAY be restored to the routing table if ACKs resume before the connection is closed, as described # this should read Connection Table twice? sec. 6.7 ========= and fragments it, each fragment has a full copy of the Forwarding # 1.) clarification would be good that the length field is covering the # total msg length and which field are different in the "copies" # (at least the fragment field) # 2.) what happens if overlapping fragments are received? Feedback on sections 7 and greater will follow in a separate mail later today. Regards, Roland