Subject: [RAI] RAI-ART review of draft-ietf-p2psip-base-10

From: Eric Burger <[email-masked]>
Date: Tue, 7 Sep 2010 17:05:26 -0400

I am the assigned RAI-ART reviewer for draft-ietf-p2psip-base-10

For background on RAI-ART, please see the FAQ at
<http://www.softarmor.com/rai/art/FAQ.html>.

Please resolve these comments along with any other comments you may receive.

Summary: This draft is well written and just about ready to go. These comments are mostly clarifying. However, there are some substantive comments the authors must address, either by clarifying the text or from further discussion.

SUBSTANTIVE COMMENTS
Section 5.3.2.1, Processing Configuration Sequence Numbers
The configuration sequence number is a monotonically increasing, 16-bit integer. Age is determined modulo 65536. That is, 3 is typically considered newer than 65530. RELOAD uses the sequence number to determine if a received configuration is newer than the current configuration. However, the value 65535 is special, in that the recipient must accept the configuration. This is great, unless a peer is at configuration sequence 65534. Let us say the peer is disconnected, and missed the next two updates. Now the global sequence number is 1. During normal operations, 1 is newer than 65534. However, the peer will send out its configuration with the next number in the sequence, 65535. Unfortunately, this is the special, must accept sequence number, meaning the out-of-date peer's configuration will overwrite the new configuration.

I would suggest adding language similar to the following at the end of the last paragraph in the section: "Since 65535 is a special value, peers sending a new configuration when the configuration sequence is currently 65534 MUST set the configuration sequence number to 0 when they send out a new configuration."

Section 5.3.2.2, Destination and Via Lists
How is it possible to enforce "the encoding MUST include a generation counter..."?  I didn't do that.  Nanny nanny boo boo.  The protocol police in the black helicopters will get you if you don't!

Section 5.4.2.5, Probe
Are there any security or privacy issues exposed by willy-nilly probe requests, or is the nature of the p2p network to be trusting?  The latter is OK; this is a peer network after all.

CLARIFYING COMMENTS
Section 1.2, Architecture
Someone unfamiliar with P2P technologies may wonder, "Why are we defining a message routing protocol? What is wrong with IP? What is the relation between RELOAD and, for example, MANET? Is all this routing for NAT traversal? Won't IPv6 make this obsolete?" To be honest, it was not until I reviewed Section 9 on Chord did message routing make some sense to me. A bit of clarifying text here or a forward reference to Section 9 might head off knee-jerk reactions of "we are not going to standardize IP over P2P over IP."

Section 2, Terminology, Resource Name
The text states the resource name is human readable. In this case, it must have some indication of language and character set.

Section 2, Terminology, Connection Table and Routing Table
I read the words in the definitions for these terms, but they made absolutely no sense to me on the first reading. They make perfect sense on the second reading. I would almost offer we have some, "There are circular definitions here; if these definitions do not make sense to you now, they will after you finish reading this document in its entirety." That also has the benefit of encouraging people to actually read the document.

Section 5.1.3, Private ID
On my first pass, I said to myself, "It looks like a Private ID could clash with a Node ID." It was not until I ready the pseudo code defining a Destination that I saw these IDs have their own name spaces. It would be helpful to say something in 5.1 or 5.1.3 similar to, "Private IDs have their own, distinct namespace with Node IDs and Resource IDs."

Section 5.2.1, Request Origination, request timeout
Why a 3 second timer? Satellite IP can have up to 2.5 seconds of one-way latency. Is this arbitrary? If so, I would think 3 micro-fortnights would be a better timer (apologies to Dave Oran).  On the serious side, should there not be a relationship between this timeout and the RTO of Section 5.6.3.1.1? For example, if a satellite RTO is five sections, an implementation should be able to adjust this time accordingly.

Section 5.2.1, Request Origination, write requests
Assume X and Y have write permissions for a resource. X does a write that gets lost, then Y does a write, then X retries the write. Who wins?

Section 5.3.1, Presentation Language, style issue
WAY too many adjectives and adverbs that do not add any value. I would offer it is easy and very familiar to write straightforward and clear language.

Section 5.3.2.1, Forwarding Options [Joke Issue]
What, no reference, even informative, to RFC 3459?  Your reviewer is put out <g> <wink>.

Section 5.3.2.1, Forwarding Options [Real Issue]
If RESPONSE_COPY is set, I understand copying the option from the request to the response.  However, the text goes on to say, "and clear the ... flags."  The expected behavior is not clear to me.

Section 5.3.3, Message Contents Format
Is there a reason for the alternating request / response codes? If it is to deal with overlapping responses, then you cannot have multiple requests with the same point code outstanding.  If one can have multiple requests with the same point codes, then you will need sequence numbers.  If you have sequence numbers, then you do not need to correlate requests with responses via the request / response code.  The concept of odd / even request / response codes seems a bit unnecessarily complex.  I do have this comment in the CLARIFYING section as I am sure there is an engineering reason to do odd / even codes.  If there is no reason to do it, than I would put this comment in the SUBSTANTIVE comment section.

Section 5.3.3, Message Contents Format
If critical = False, the recipient SHOULD process the message, *UNLESS*????  Why would the peer chose not to process the message?  Or, is this really a MAY?

Section 5.3.3.1, Response Codes and Response Errors, Error_Request_Timeout
The text does not make it clear *who* received what, when?  Which end times out?  Which end sends the message?

Section 5.3.3.1, discussion of Error_Config_Too_New and Error_Unknown_Kind
It took me three readings to figure out the text says what it means, but we need better text.  The text talks about responses to messages, which is a message that gets sent.  These two response codes tell the implementor to send this message, then send a Config_Update message.  That is just what we want to do.  However, it took me two readings to understand that Config_Update was not an error message.  The problem is there are way too many "messages" floating around.  In SIP terms, we would say something like send a Config_Update method.  In the context of this document, could we say something like "A node which generates this response MUST send a Config_Update request containing the appropriate kind definition after sending the response."?

Section 5.5.1.2
Under what circumstances would a peer not process an Attach request?  If you cannot think if it, then a peer MUST process an Attach request.  The peer can always reject the request.  If you can think of it, like if the peer is under a DoS attack and is purposely dropping Attach requests, say so.  Otherwise, peers MAY process Attach requests.  Also, this paragraph looks like it has a lot of editing shrapnel.  For example, it says the peer needs to start ICE checks, and then start ICE checks.  ICE is so slow as it is, so we do not need to do it twice.

Section 5.5.1.4, last paragraph
The text does not describe under what circumstances would a peer not select a new stun server from the same group?  Are you really saying that a peer MUST select a new STUN server from the same group, unless there are no STUN servers remaining?  Are you really saying that a peer can pick a STUN server from anywhere, but it is RECOMMENDED to pick a server from the same group? 

Section 5.5.1.6, last bullet
"For example, the DTLS/UDP with SR overlay link protocol." is not a sentence.  Where is the verb?

Section 5.5.2.2 - but really a general comment
Most RFC's have the structure Clients do This; Servers do That.  I get this is a peer-to-peer protocol.  However, even in a peer protocol, someone sends a request, and the other end sends a response.  This section, which is where a responder would go, has lots of directions for the requestor as well.  This is begging for implementor confusion.  A constructive comment would be to either break this out and have the normative language for the requestor to be in section 5.5.2.1 or have a comment up front warning implementors to basically look everywhere for the normative protocol definition.

Section 5.5.3.1, Ping Request Definition
Since a Ping can be sent to a ResourceID, should that not also be in the PingReq?  Or will the header take care of ResourceID versus NodeID addressing?

Section 6.4.4.1, page 87, first full paragraph after the structure definition, last sentence.
Should the last line read "and, if not, the peer MUST reject the request"?

Section 6.4.1.2
The peer returns the StoreAns, and the text says the response includes a list of peers where the data will be replicated.  This is where passive voice does not work (by the way, passive voice never works!): WHO replicates the data?

Section 6.1.4.2, KindId definition
The definition is "KindId  unknown_kinds<2^8-1>".  What does this mean?  Is unknown_kinds an array of fixed length of 127?  Is it a variable length array of zero to 127 entries, "KindId  unknown_kinds<0..2^8-1>"?  If the latter, this is a typo.  If the former, the syntax definition does not talk about scalar vectors.

Section 6.4.4
Is the statement "interactively fetching R_n+1=nearest(1 + R_n)" meaningful to the community?

Section 9, third from last bullet
Is 2^128 nodes really a limit? Is that not something like making every quark in the universe a potential peer?  Or am I exaggerating and this is really just the number of atoms in the solar system?

Section 10.1, definition of turn-density
Just checking, but if the default value is 1, my read of the document is that every node is a TURN server. 1/1 = 1 = 100%.  Along those lines, what if you wanted, for example in a closed LAN or enterprise, to have no TURN servers. How would you represent that?  A very large number so 1/large-number is almost zero?

Section 10.1, definition of bad-node
My read of this section is there can be only one bad-node listed at a time.  If I read that wrong, it needs to be more clear that more than one bad-node can get listed.  However, I have another question.  Since NodeID's get allocated or are generated, what if a node gets NodeID X, the network later says X is a bad-node, and much later a new node gets assigned NodeID X?  Is X forever taken out of the NodeID name space?  If so, that could make for an interesting denial-of-service attack, that of removing NodeIDs.  Conversely, if bad-node has a life time, the text should talk about it.  Given the large NodeID name space, and that they are not human readable, one could have a compromise of black listing for a "long, but finite" time.

Section 10.3, Credentials
The text says the Node-IDs need to be "unpredictable to the requesting user."  Is this a redundant requirement to the Node-ID being cryptographically random?  Since the reason for it being unpredictable is given in the security section, I would either clarify with text like "The enrollment server must chose Node-IDs such that they are unpredictable to the requesting user.  Thus, the Node-IDs MUST be cryptographically random [RFC4086]."  If I am wrong, and cryptographically random is not sufficient as to what "unpredictable to the requesting user" means, then the text must describe what "unpredictable" means.

Section 12.3
Something I am clueless on.  The user name looks like a FQDN, like [email-masked].  If it looks like a FQDN, can one resolve the address from outside the peer network?  If one can, how?  If one cannot, why do we have a string of characters that looks like a FQDN?  This is a clarifying question, in that I can get it, but my brain hurts.

TYPOS
Section 2, Kind
s/Each Kind is identied iwht/Each Kind is identified with/
s/called a Kind-ID ./called a Kind-ID./

Section 5.3.2.4, last paragraph starting "This option can only be used"
Lots of typos in this paragraph.  What I found reading (not in a grammar checker) is below.  I would suggest running this paragraph (if not the document) through a grammar checker to find this nitty stuff.
Line 6:
s/as if it had send and Attache request/as if it had sent an Attach request/
Line 7:
s/This cases/This causes/
Last line of Page 47:
s/noot/not/

Section 5.3.3.1, Error_Incompatible_with_Overlay
s/overlayalgorithm/overlay algorithm/

Section 5.4.2.5.2, first paragraph after the struct definitions:
s/This structure allows for unknown options/This structure allows for unknown option/

Section 5.5.1.1, struct definitions
The text consistently uses C comments, /* mumble */.  This definition has some XML comments, <!-- mumble -->.  Besides being stylistically troubling, since the definition language uses angle brackets, I would STRONGLY suggest using ONLY C-style comments in all struct definitions.

Section 5.5.1.13, first sentence
s/The procedures of Section 11 apply/The procedures of Section 11 of ICE apply/

Section 5.6.5, second paragraph, third line
s/using the framing header/use the framing header/

Section 10.1, last line on page 117
s/starting form/starting from/

Section 13.5, Data Kind-ID
/Code points in the range 0xf0000001 to 0xffffffff/Code points in the range 0xf0000001 to 0xfffffffe/  Twice in the paragraph.
0xffffffff is a reserved code point.

Something for the Bottom Where Few Will Look
Has anyone proactively pinged for IPR?  The only IPR declared so far is by Qualcomm, and that is only for devices with wireless WAN interfaces.  Given the common ownership of JoltID and Skype, can one of the Skype employees or consultants prod JoltID to declare if they have IPR here?  A reasonable person would expect such IPR rules would apply.

---

From: Cullen Jennings <[email-masked]>
Date: Tue, 21 Sep 2010 19:42:33 -0600

Eric, Thank you very much for the careful read. We will be working these into the next version. 

On Sep 7, 2010, at 3:05 PM, Eric Burger wrote:

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

Cullen Jennings
For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html