Subject: Re: [ppsp] Ben Campbell's Discuss on draft-ietf-ppsp-base-tracker-protocol-10: (with DISCUSS and COMMENT)

From: "Huangyihong (Rachel)" <[email-masked]>
Date: Tue, 27 Oct 2015 02:58:05 +0000

Hi Ben,

Thank you for the detailed comments. We're so sorry for the late replies. Please see them inline.

BR,
Rachel

> -----Original Message-----
> From: Ben Campbell [mailto:[email-masked]]
> Sent: Wednesday, October 14, 2015 10:07 AM
> To: The IESG
> Cc: [email-masked]; [email-masked];
> Zongning; [email-masked]
> Subject: Ben Campbell's Discuss on draft-ietf-ppsp-base-tracker-protocol-10:
> (with DISCUSS and COMMENT)
> 
> Ben Campbell has entered the following ballot position for
> draft-ietf-ppsp-base-tracker-protocol-10: 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-ppsp-base-tracker-protocol/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> I think this draft needs privacy considerations. The protocol sensitive
> information in the form of IP addresses and location. If I understand correctly it
> may also identify transferred content, which can be highly sensitive under some
> circumstances. Encryption is optional.  It think it needs stronger guidance on
> privacy implications and the use of HTTPS.

[Rachel]: How about adding a privacy subsection in Section 6 as following?

NEW
"
Section 6.5  Privacy for Peers

The PPSP-TP provides mechanisms in which the peers can send message containing IP addresses, ports and other information to the tracker. A tracker or a third party who is able to intercept such messages can store and process the obtained information in order to analyze peers' behaviors and communication patterns. Such analysis can lead to privacy risks. For example, an unauthorized party may snoops on the data transmission from the peer to a tracker in order to introduce some corrupted chunks. 

The PPSP peer protocol [RFC7574] has already included some mechanisms to protect the streamed content, see section 12.3 and 12.4 of [RFC7574]. For PPSP-TP, peer implementations as well as tracker implementations MUST support the "https" URI scheme [RFC2818] and Transport Layer Security (TLS) [RFC5246]. In addition, a peer should be cognizant about potential tracker tracking through queries of peers, e.g., by using HTTP cookies. The PPSP-TP protocol specified in this document does not rely on HTTP cookies. Thus, peers MAY decide not to return cookies received from the tracker, in order to making some additional tracking more difficult.
"

> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> - 1.1, definitions of "LEECH" and "SEEDER"
> 
> These don't seem to match the  use of the terms elsewhere in the document.
> In particular, these seems to describe transient states, while the protocol
> requires the peer to declare itself to be a leech or seed when it joins a swarm.
> Additionally, these definitions don't seem to consider a peer that may have
> received part but not all of some content, but still makes the chunks it has
> available.

[Rachel]: Maybe I should check all the descriptions in the document. A LEECH is a peer which hasn't finished downloading the whole content yet, but it is also contributes its downloaded content with other LEECH. A SEEDER is a peer which has the whole content. It just contributes the content to others, and it does not download. 

> 
> - 1.1, LIVE STREAMING:
> 
> Is there an expectation of syncing reception between receivers?

[Rachel]: For live streaming, receivers are all receiving the same content. The out of sync is caused by different network transmission time. So the lag is quite small, and usually does no harm to individual user's watching experience. So currently, it is not widely considered in streaming protocols.

> 
> - 1.1, VIDEO-ON-DEMAND
> Aren't the main points that different audiences can watch the same part at
> different times, or different parts at the same time?

[Rachel]: Yes. You're right. How about changing to the following:

"
It refers to a scenario where different audience may watch different parts of the same recorded streaming with downloaded content at the same time, or may watch the same part of the recorded streaming at different times. 
"

> 
> - 1.2, 2nd to last paragraph:
> 
> Wouldn't a distributed tracker have implications for discover and connection
> management? It also seems worth some discussion in the operations section.
> 

[Rachel]: Currently , a management of a distributed tracker is out of scope of this document. The tracker in this document is treated as a single logical entity. Maybe we should add clarification words in this part.

> - 1.2.2, first paragraph:
> 
> Aren't there at least some implied requirements for peer-ids? E.g.
> uniqueness requirements? Requirements for elements to treat them as
> opaque strings?

[Rachel]: Please see the definition of PEER ID in Section 1.1 terminology part. It is quoted as following

"
   PEER ID:  The identifier of a Peer such that other Peers, or the
   Tracker, can refer to the Peer by using its ID.  The Peer ID is
   mandatory, can take the form of a universal unique identifier (UUID),
   defined in [RFC4122], and can be bound to a network address of the
   Peer, i.e., an IP address, or a uniform resource identifier/locator
   (URI/URL) that uniquely identifies the corresponding Peer in the
   network.  The Peer ID and any required security certificates are
   obtained from an offline enrollment server.
"

> 
> -2.1: "The messaging model of PPSP-TP aligns with HTTP protocol..."
> 
> Is that the same as saying it uses HTTP as a substrate? (If so, the references to
> HTTP need to be normative.)

[Rachel]: Yes. Will change the reference in the next version.

> 
> - 2.2, 3rd paragraph: "Requests are sent, and responses returned to these
> requests."
> The use of passive voice obscures the actors. What does each of these things?

[Rachel]: How about deleting this paragraph?

> 
> - Figure 5: Is there a matching peer state machine (i.e. that models the internal
> state of a peer?)

[Rachel]: No, peer doesn't have to keep a state machine. It's all in the server side.

> 
> - 2.3.2, general:
> 
> The section is inconsistent in specifying result codes. Also, it's not clear
> whether these are HTTP result codes, or PPSP-TP result codes.

[Rachel]: It's PPSP-TP result codes. They are specified in Section 4.2 and 4.3. I will add some clarification in this section to clarify this.

> 
> -2.3.2: "Peers MUST NOT generate protocol elements that are invalid."
> 
> That's an odd use of "MUST NOT".

[Rachel]: How about changing to "SHOULD NOT".

> 
> ability_nat: What's the use case for one peer caring what the NAT status of
> other peers might be, as long as it gets addresses that it can reach?

[Rachel]: It's an optional attribute. A possible use case I can imagine is that maybe a peer that may not have any NAT traversal ability just wants some public addresses. Thus, tracker can return some eligible address accordingly.

> 
> - 3.2.4: "connection":
> That list will continuously become obsolete. I suggest some classification that
> describes the attributes you care about, rather than the current-as-of-now set
> of network technologies.

[Rachel]: Okay. Will do some modification.

> 
> - table 3:
> 
> How is "priority" encoded and interpreted?

[Rachel]: "priority" is an integer. It may be determined by network topology preference, operator policy preference, etc. How to create a priority is outside of the scope. The larger the value, the higher the priority. I'll add some clarification to it.

> 
> -3.3.4:
> 
> "Normally, swarm action result
>    elements SHOULD be set and error_code MUST be set to 0 when
>    response_type is 0x00."
> 
> What does it mean for swarm action results element to be set? Since that's a
> structure, I assume you don't mean set as in "true", or set to zero like the
> error_code. Do you mean they should be _included_ or _present_?

[Rachel]: Yes. "set" should be changed to "present".

> 
> "A swarm action result element is the result information for a peer to
>    request the tracker to have some actions towards the swarm."
> 
> I'm a bit confused about the causality here. This is the result of a request that
> has already happened? An indication that the pier should request something? A
> reciprocal request and the result of the previous request?

[Rachel]: Sorry for causing the confusion. It's the result of a request from the peer to the tracker. It includes the information that the tracker responds to the request.

> 
> -4, general:
> 
> The protocol specification sections are inconsistent in the way they use
> 2119 keywords. It seems somewhat random whether any given protocol
> requirement uses 2119 keywords or not. I suggest the authors re-review this
> section for 2119 consistency. In general, you don't need 2119 words to describe
> every detail of the the basic flow of the protocol; rather they are more useful
> when there's a decision to make, or a place where an implementor is likely to
> get things wrong.

[Rachel]: Thanks for the suggestions. Will do it in the next version.

> 
> -4, 2nd paragraph:
> 
> Can you offer any guidance on when one should actually _use_ HTTPS? (See
> DISCUSS comment)

[Rachel]: Please see the reply to the DISCUSS comment.

> 
> -4, paragraph 4:
> 
> This seems to be describing the general working of HTTP. I'm not sure why you
> need to restated it here, especially with 2119 words.

[Rachel]: Agree. Will delete this paragraph.
> 
> -4.1.1, paragraph 4:
> 
> Since the mentioned elements are "optional", is there any guidance for the
> tracker? For example, can the peer assume that the tracker (or other
> peers) will never require these elements to operate properly? Or if they may be
> required by the tracker (or other peers), how would the peer know it needed to
> include them?

[Rachel]: It is optional because they are not mandated conditions for a tracker to search for a peer list. It is possible that the tracker in a implementation may never require these elements. But in some implementations, the tracker may have some filter algorithms for searching peers. Thus, the peers in those implementations can include corresponding attributes in the request, so that the tracker can manage the information of all peers, and search for appropriate set of peers for a requesting peer.

> 
> - 4.1.1, paragraph 6:
> 
> "... MAY... start by pre-processing the peer authentication information"
> needs elaboration. I assume the "MAY" relates to the previously mentioned
> "preferred" use of TLS client certificate authentication?

[Rachel]: Yes. So it's better to change to "SHOULD"?

> 
> -4.1.1., 3 paragraphs from end:
> 
> Can you elaborate on "STUN-like function"? I gather the tracker can reflect
> back the observed source address/port of the peer in the response? If so, I'm
> not sure readers will understand that from "STUN-like function".

[Rachel]: Here we mean that the tracker is also a STUN server. As you said the tracker can perform STUN-like function by putting peer's address it observes in the response. Will add some clarifications to help reader to understand this.

> 
> -4.1.1.1:
> 
> I'd love to see the example use HTTPS, just to set a good example.

[Rachel]: Ok. Will reflect it in the next version.
> 
> In the example of a peer leaving one swarm and joining another, second
> swarm_action:
> 
> s/@peer_mode/peer_mode

[Rachel]: Ok.
> 
> -4.3, 1st paragraph:
> 
> Why would the peer fail to read the response? Because the response is invalid?
> Not received? This all runs over TCP right?

[Rachel]: Yes. It is assumed that some invalid response types are appeared. 

> 
> -4.3, 3rd paragraph:
> 
> Why only a should? What are the consequences of not being prepared?

[Rachel]: Will change to "MUST".

> 
> I assume the errors in this section are PPSP-TP errors, not HTTP result codes. I
> think the choice of 4XX codes will confuse people into thinking these match the
> HTTP 4XX client-error result codes. Especially since many seem to have similar
> meanings--but rearranged.

[Rachel]: You're right. Will fix it in the next version. 

> 
> -6.1:
> 
> It would probably be worth mentioning the need for an enrollment service in
> the operations section.

[Rachel]: Ok.
> 
> The 3rd paragraph would work better in the previous paragraph where you
> mention the HTTP security considerations, and perhaps cast in terms of HTTPS.
> Also, please consider a reference to RFC 7525.

[Rachel]: Ok.

> 
> 
> OAuth might should've been mentioned back when you discussed digest
> authentication and HTTPS client certificates

[Rachel]: Ok.
> 
> -6.2:
> 
> This seems to mean that the peers in a swarm must agree on an integrity
> protection mechanism. Can you reference something? On the other hand isn't
> this an issue for the actual transport protocol rather than the tracker protocol?

[Rachel]: The integrity verification schemes mentioned in Section 6.2 are considered in peer protocol, which can reference section 6.1 of RFC7574. 

> 
> -7:
> 
> The 3rd paragraph seems to forbid adding new elements, as described in the
> next section.
> 
> The 2119 words in the 4th paragraph are questionable. It's hard to put MUST
> level requirements on people, and the MAY is a statement of fact.

[Rachel]: Right. Will fix it.

> 
> In the last two paragraphs:
> 
> Internet-Drafts are not appropriate places to document things, other than
> ephemerally on their ways to become RFCs. I-Ds expire.
> 
> Saying extensions MAY be documented in RFCs doesn't add anything; anything
> may be documented in RFCs. I assume therefore you mean at least SHOULD. If
> so, what kind(s) of RFCs would be appropriate?

[Rachel]: These should be proposed standard RFCs.

> 
> The suggestion that private extensions need not be documented in RFCs can be
> harmful to long-term interoperability. So-called private extensions have a way
> of migrating to public networks.

[Rachel]: So maybe we should delete the last paragraph.

> 
> -7.2:
> 
> Wouldn't non-backward compatible extensions require a new version number?
> Doesn't this violate the requirements in the parent section?

[Rachel]: Maybe rewording is needed here. How about changing to the following?
OLD
"
   -  Backward Compatibility:  One of the most important issues to
      consider is whether the new extension is backward compatible with
      the base PPST-TP.
"

NEW
"
-  Backward Compatibility: The new extension MUST be backward compatible with the base PPSP-TP specified in this document.
"

> 
> -8:
> 
> I am surprised not to see registries for methods and result codes, especially
> with the considerable text about extensibility. Do you expect them to be
> extended?
> 
> 10.2:
> The references to 2616, 2818,  and 5246 probably need to be normative.
> Also, 2616 is obsolete.

[Rachel]: Right. Will fix them.

> 
> Editorial and nits:
> -------------------
> 
> - IDNits points out a number of citations without matching references
> 
> -1.1: VIDEO-ON-DEMAND and LIVE-STREAMING
> 
> I suggest changing "It refers to" to "VIDEO-ON-DEMAND refers to..." and
> "LIVE-STREAMING refers to..."
> 
> - 2.2, definition of CONNECT: "... Peer
>       registers in the Tracker (or if already registered) to notify it..."
> 
> What is the antecedent of "it"? (peer or tracker?)

[Rachel]: the tracker.

> 
> - 2.3.1, 4):
> s/"is responded with"/ "is responded to with" (Or even better, "The tracker
> responds to the request with...")

[Rachel]: Ok.

> 
> - 3.1 : "... turning the definitions for
>    JSON objects extensible."
> 
> I don't understand the phrase. Do you mean "making the
> definitions...extensible"?

[Rachel]: This sentence should be deleted.

> 
> - 3.2.2, 1st paragraph:
> 
> Paragraph seems out of place, and seems redundant with the rest of the
> section. The placement here makes it look like NAT status selection is the
> primary use, and I don't think that's the intent.

[Rachel]: Right. Should be deleted.
> 
> s/"inform the tracker on the preferred type"/"inform the tracker of the
> preferred type"
> 
> - 3.2.5, first paragraph:
> 
> s/can be related with/can be related to
> 
> -3.3.4, response element:
> 
> s/ppsp_tp_interger_t/ppsp_tp_integer_t

[Rachel]: Ok.

> 
> - 4.1.1, 3rd paragraph after table 6
> 
> The first sentence is convoluted and hard to parse. I suggest breaking it into
> simpler sentences.

[Rachel]: Sure. How about the following change

OLD
"
The element peer_num indicates to the tracker the number of peers to
   be returned in a list corresponding to the indicated properties,
   being ability_nat for NAT traversal (considering that PPSP-ICE NAT
   traversal techniques may be used), and optionally concurrent_links,
   online_time and upload_bandwidth_level for the preferred
   capabilities.
"

NEW
"
The element peer_num indicates the maximum number of peers to be returned in a list from the tracker. The returned peer list can be optionally filtered by some indicated properties, such as ability_nat for NAT traversal, and concurrent_links, online_time and upload_bandwidth_level for the preferred capabilities.
"

> 
> 
> -4.1.2, 4th paragraph, "... with regard to the possibility of
>    connecting with other IETF efforts such as ALTO [RFC7285]."
> 
> I don't understand the intent of this phrase.

[Rachel]: How about changing it as to the following:
OLD
"
The tracker may take network location information into
   consideration as well, to express network topology preferences or
   Operators' policy preferences, with regard to the possibility of
   connecting with other IETF efforts such as ALTO [RFC7285].
"

NEW
"
The tracker may take network location information into consideration as well, to express network topology preferences or operators' policy preferences. It can implement other IETF efforts like ALTO[RFC7285], which is out of the scope of this document.
"

> 
> -4.1.2, 5th paragraph:
> 
> The structure of the sentence is confusing. Do you mean to say that included
> elements MUST include public addresses?

[Rachel]: No. It seems this paragraph is redundant with the 6th paragraph. So it can be deleted.

> 
> -4.1.2, 3rd to last paragraph:
> 
> The paragraph seems redundant to the previous paragraph.
> 
> -6, first paragraph:
> 
> s/"impersonating to be another valid participant"/"impersonate a valid
> participant"

[Rachel]: Will fix it.

> 
> -7.2, security issues: "Being security an important component of any
>       protocol..."
> 
> I don't understand this phrase.

[Rachel]: Sorry. It should be modified as following:

OLD
"
   -  Security Issues:  Being security an important component of any
      protocol, designers of PPSP-TP extensions need to carefully
      consider security requirements, namely authorization requirements
      and requirements for end-to-end integrity.
"

NEW
"
-  Security Issues: As security is an important component of any protocol, designers of PPSP-TP extensions need to carefully consider security requirements, e.g.,  authorization requirements and requirements for end-to-end integrity.
"