Last Call Review of draft-ietf-ppsp-peer-protocol-09
review-ietf-ppsp-peer-protocol-09-secdir-lc-harrington-2014-07-03-00

Request Review of draft-ietf-ppsp-peer-protocol
Requested rev. no specific revision (document currently at 12)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2014-07-01
Requested 2014-06-05
Authors Arno Bakker, Riccardo Petrocco, Victor Grishchenko
Draft last updated 2014-07-03
Completed reviews Genart Last Call review of -10 by Christer Holmberg (diff)
Genart Last Call review of -12 by Christer Holmberg
Secdir Last Call review of -09 by David Harrington (diff)
Opsdir Last Call review of -09 by Tina Tsou (diff)
Assignment Reviewer David Harrington
State Completed
Review review-ietf-ppsp-peer-protocol-09-secdir-lc-harrington-2014-07-03
Reviewed rev. 09 (document currently at 12)
Review result Not Ready
Review completed: 2014-07-03

Review
review-ietf-ppsp-peer-protocol-09-secdir-lc-harrington-2014-07-03

I have reviewed this document as part of the security directorate's 
ongoing effort to review all IETF documents being processed by the 
IESG.  These comments were written primarily for the benefit of the 
security area directors.  Document editors and WG chairs should treat 
these comments just like any other last call comments.

Summary: 

I think this document is far from ready for publication as a standard., or for last-call reviews. The WG has review work to do before this document is ready for expert review.

I gave up reviewing this document after 10 pages, because it was so not ready for review.

I will say that, being both an opsdir and secdir reviewer, I was greatly encouraged by the table of contents, which 

shows

 that a lot of consideration was given to security and ops and mgmt. But then I 

started reading the text in the main part of the document, and was severely disappointed by what I found.

I chose to sample sections of the document to see if maybe it was just the opening text that was problematic. I don’t think so.

1)  Editorial: in the terminology section, it says

   content integrity protection scheme
       Scheme for protecting the integrity of the content while it is
       being distributed via the peer-to-peer network.  I.e. methods for
       receiving peers to detect whether a requested chunk has been
       maliciously modified by the sending peer.

I do not believe this protocol can detect intent, so to claim that it can detect whether a chunk has been maliciously modified is inaccurate. It can tell if a chunk has been modified.

2) Technical: in the definition  of swarm ID, there is a conditional that applies to VOD. It is ambiguous whether it also applies to live streaming. I recommend this wording be modified to make it clearer.

If Merkle trees are not used for integrity protection, what is the swarm ID for VOD?

3) Ed: s/

as it source/as its source/

4) Ed: "

   This message conveys protocol options, in particular, peer A includes

   the ID of the swarm as the destination peers can listen for multiple
   swarms on the same transport address.”

I couldn’t parse this sentence; is it missing punctuation?

I recommend rewording for clarity.

5) Ed: "

denotes what chunks of the content peer B, resp. C have.

”

I don

’

t know what word 

“resp.” is abbreviating.

Substituting words that start with resp, (e.g., response, respectively, etc.) I cannot make sense of this sentence.

6) tech: I feel uncomfortable with section 2 containing examples that describe the overall flow. Examples are non-normative text, usually contained in a non-normative appendix. These examples describe the order of messages, and it is 

7) in example 2.2, the integrity hash is provided by the peer that is providing the (potentially maliciously modified) content. Isn’t that like asking the fox to verify that the henhouse is safe?

8) in 3, paragraph 1, it says “In general,” which seems less specific than normally expected of standards language.

9) in 3, paragraph 1, it says “this behavior”, but I’m not sure which behavior it is referencing. It is unclear whether not sending error messages, or discarding messages, or stopping communication, or classifying peers is the behavior that allows a peer to deal with slow, crashed, or silent peers. I don’t understand HOW any of the behaviors mentioned would allow a peer to deal with slow, crashed, or silent peers. I do not understand on what basis peers are judged “good” or “bad”.

10) in 3, paragraph 2, it says, "

Multiple messages are multiplexed into a single datagram for

   transmission.  Messages in a single datagram MUST be processed in the
   strict order in which they appear in the datagram.” While this might be true for UDP, is this also accurate for TCP or other non-datagram transports? 

This is not written using RFC2119 keywords; should “are” be “MUST be” or “SHOULD be” or “MAY be”?

11) in 3, paragraph 3, the second sentence seems to contradict the first sentence, and since neither is written using RFC2119 keywords, it seems to really leave the whole question open to implementer interpretation.

[jumping forward to sample other places in the document]

12) section 8 is ambiguous:

"8.  UDP Encapsulation



   PPSPP implementations MUST use UDP as transport protocol and MUST use
   LEDBAT for congestion control [RFC6817].  Using LEDBAT enables PPSPP
   to serve the content after playback (seeding) without disrupting the
   user who may have moved to different tasks that use its network
   connection.  Future PPSPP versions can also run over other transport
   protocols, or use different congestion control algorithms.

“

I find “MUST use UDP” inconsistent with “”can also run over other transport protocols”, and “MUST use LEDBAT” inconsistent with “or use different congestion control protocols”.

[jumping ahead …]

In 8.3, there is a discussion of multiple swarms sharing a UDP port. I’m unsure whether this would work, but since UDP is stateless, it might. Do channels work for protocols like TCP? 

[jumping ahead …] 

"A SIGNED_INTEGRITY message (type 0x07) consists of a chunk
   specification, a 64-bit NTP timestamp [RFC5905] and a digital
   signature encoded as a Signature field in a RRSIG record in DNSSEC
   without the BASE-64 encoding [RFC4034].”

Can this work in an implementation with no NTP support?

[juming ahead …]

8.14 describes a keep alive message format, but no processing instructions.

Are receiving implementations required to do anything with this message? are they supposed to respond somehow?

How does the sender of a keep alive determine that the connection is still live?

There is no message type for keep alive. If the first octet of the keep alive happens to be 0x0a, how does the receiver know whether this is a keep alive or a CHOKE message?

Table 7 defines the messages types in decimal. But the text uses hex values. 

—

At this point, I will observe that I think this document is far from ready for publication as a standard. 

The WG, and the chairs and shepherd, really need to look at this document closely to ensure the English is clear and unambiguous, that RFC2119 keywords are used to specify the behaviors expected from implementers.

I think this document would be well served by describing the message types and formats and their processing in a transport-neutral manner, and then describing the transport-specific mapping details, so it is clear and unambiguous how the mapping of message sequences to datagrams is handled for UDP and DCCP, and how the mapping of message sequences should be handled for stream-based protocols like TCP. There are some message types, notably keep alive, that I am not sure apply in the same way to different transports.

Multiple messages are multiplexed in a datagram. How are the messages delimited? If there is any corruption in one message, how does the receiver find the end of the message and the start of the next message? If I understand correctly, invalid messages are discarded and no error code is sent. If one of the messages are found to be invalid, are all messages in that datagram discarded? or are all subsequent messages in that datagram discarded? or is it valid to process the remaining messages in the datagram after an invalid message is detected? If so, would that conflict with the rule that all messages must be processed in order?

How will messages be limited in stream-based transports? if 1000 messages are sent in a streaming protocol, and number 3 is invalid, what happens to the remaining stream of 997 messages?

Dave Harrington

ietfdbh at comcast.net