Hi Marc, Thank you very much for the feedback. See inline.
> -----Original Message----- > From: [email protected] [mailto:[email protected]] On Behalf Of > Marc Petit-Huguenin > Sent: Wednesday, September 26, 2012 12:52 AM > To: Rosen, Brian > Cc: P2PSIP WG > Subject: Re: [P2PSIP] WGLC for draft-ietf-p2psip-diagnostics-09 > > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA256 > > I carefully read this I-D, and I found a lot of nits, which make it difficult > to read. I think that this draft is ready to be submitted to the IESG, but I > would like to start a discussion on one specific point, security. > Thanks, the document will double checked and updated. > The draft is very light on the discussion about who is authorized to do what. > Obviously some of the information that can be requested are sensitive, and I > am afraid that without having a mechanism for authorization in the document, > implementers will be reluctant to add it to their implementation. Good point! >I think > especially of SOFTWARE_VERSION, but there can be future Diagnostics Kinds that > are not for general availability. So I would suggest to define in the spec an > additional mechanism to restrict the access to some Kinds: > > The idea would be to add an extension in the configuration file to authorize > access to these Diagnostics Kinds only to a specific list of Node-IDs (in a > similar way that signer node-IDs are authorized to sign kind and configuration > elements in signer and king-signer elements. > This should work. Could this lead to the over dynamic of the configuration file? Another option is for each Kind, just stored these signed IDs as a value in the overlay. BR, -Haibin > > Nits > ==== > > - - The title, abstract and section 1 uses the acronym "P2PSIP", but in fact > what is described is not limited to P2PSIP (i.e. the usage of > draft-ietf-p2psip-sip) but can be applied to any usage of RELOAD. So I would > recommend to change P2PSIP to P2P. > > - - Section 4: Path_Track > > This is used inconsistently in the whole document. I would suggest to change > it to PathTrack everywhere, with the request structure named PathTrackReq and > the answer PathTrackAns. The message code names (path_track_req and > path_track_ans) are correct. > > - - The meaning of peer, node and client is well defined in > draft-ietf-p2psip-base, but the usage of these words is not consistent in this > document. I would suggest to check every instance and change them if > needed. > For example in Section 4, last paragraph, "... must be only provided to > authorized peers." should be "... must be only provided to authorized > nodes.", and so on... > > - - Section 4.2, last paragraph: "There have been proposal on the list..." > > This kind of language probably does not have its place in a normative section. > Move this to an annex. > > - - Section 4.3: Move to an Annex (see above) > > - - Section 5.1.1, title: > > s/DiagnosticRequest/DiagnosticsRequest/ > > - - Section 5.1.1, structure: > > I think that this would make it clearer that there can be more than one > DiagnosticExtension: > > enum { (2^16-1) } DiagnosticExtensionRequestType; > > struct { > DiagnosticExtensionRequestType type; > opaque diagnostic_extension_contents<0..2^32-1>; > } DiagnosticExtension; > > struct { > uint64 expiration; > uint64 timestamp_initiated; > uint64 dMFlags; > DiagnosticExtension diagnostic_extensions<0..2^32-1>; > } DiagnosticsRequest; > > - - Section 5.1.1, 7th paragraph "the flags can be extended by standard > action." > > Add a cross reference to the correct subsection in section 9 (IANA > Considerations). > > - - Section 5.1.2, title: > > s/DiagnosticResponse/DiagnosticsResponse/ > > - - Section 5.1.3, second paragraph: > > s/and other bits Must be cleared/nd other bits must be cleared/ > > - - Section 5.1.3: > > I would suggest to provide the structure (in the presentation language defined > by RELOAD section 6.3.1) for all the types defined in this section, if > interoperability between implementations is wanted. > > - - Section 5.1.3: SOFTWARE_VERSION: I would suggest to change the example > to > something more anonymous. As for the format itself, this draft may be of some > help: > > http://www.ietf.org/internet-drafts/draft-karcz-uuas-00.txt > > - - Section 5.1.3: MACHINE_UPTIME > > s/the nodes has been/the node has been/ > > - - Section 5.1.3: MEMORY-FOOTPRINT > > s/32 -bit/32-bit/ > > - - Section 5.1.3: INSTANCES_STORED > > s/The array is index by/The array is indexed by/ > > - - Section 5.1.3: UNDERLAY_HOP > > s/for diagnostics purpose./for diagnostics purpose)./ > > - - Section 5.1.4, 2nd paragraph > > s/Editor's Note: The self-tuning/The self-tuning/ > > - - Section 5.2, second paragraph > > s/Section 5.3.3/Section 6.3.3/ > > - - Section 5.2, third paragraph: "If a peer does not extend the > extension,..." > > Does not make sense. > > - - Section 5.3, second paragraph > > s/Route_Query/RouteQuery/ > > - - Section 5.3.1 > > s/compressed id/opaque ID/ > > - - Section 5.5.1, first paragraph, "The sender MUST include the dMFlags > field" > > This field is not longer optional, so this text is useless. > > - - Section 5.5.1, first paragraph, "...but MUST include the field" > > Same than above. > > - - Section 5.5.1, second paragraph > > s/diagnostic ping/diagnostic Ping/ > > - - Section 5.5.1, 2nd and 3rd paragraphs: > > s/NodeId/Node-ID/ > s/ResourceID/Resource-ID/ > > - - Section 5.5.2, second paragraph "contain the Error code followed by the > subcode..." > > What is this subcode? > > - - Section 5.5.3, 3rd paragraph "will represent 100-hops" > > Where does this 100-hops come from? > > - - Section 5.5.3, 5th paragraph "subcode and description" > > What is this subcode? > > - - Section 5.5.3, 5th paragraph > > s/with the Error Code 1 "Error_Unauthorized"./with the Error Code 2 > Error_Forbidden./ > > - - Section 5.5.4, 2nd paragraph: > > Remove this paragraph. > > - - Section 5.5.4, 3rd paragraph: > > s/Inspect/PathTrack/ > > - - Section 6.1 > > s/WEMA_BYTES_RCVD/EWMA_BYTES_RCVD/ > > - - Section 6.2 > > s/StatusInfo/STATUS_INFO/ > > - - Section 6.3 > > s/Inspect/PathTrack/ > s/InspectReq/PathTrackReq/ > > - - Section 9 > > I find the way the registries are organized makes them difficult to > understand. > > It would be better to have one registry merging 9.1, 9.2 and 9.6, with the > following columns: > > - - Diagnostic Kind Type (from 9.2, same than diagnostic information from 9.6) > - - diagnostic flag (from 9.6, can be empty) > - - Code (from 9.2) > - - Diagnostic Extension Name (from 9.1, can be empty) > - - Code (from 9.1, empty if the previous cell is empty) > - - Specification (from 9.1, 9.2 and 9.6) > > Another alternative is to have one registry for the dMFlags kinds, and another > for the extensions Kinds, and perhaps use different RFC 5226 requirements. > > Private and experimental uses should also be documented. > > > On 09/17/2012 10:18 AM, Rosen, Brian wrote: > > We are issuing a two week Working Group Last Call for > > draft-ietf-p2psip-diagnostics-09. > > > > Please send comments to the list, including any statements you may have > > about the readiness of this document to be published. > > > > WGLC will close on October 2. > > > > - -- > Marc Petit-Huguenin > Email: [email protected] > Blog: http://blog.marc.petit-huguenin.org > Profile: http://www.linkedin.com/in/petithug > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.12 (GNU/Linux) > > iQIcBAEBCAAGBQJQYeFKAAoJECnERZXWan7E68kQAJamA99TW0I/MdPwrOEzjEBI > kX+qcAiDr6UTWwMaUfwq/gbVH2WHz32zLPiwKSY4axjmWoEvw7dsgBhfdDWKbs > TH > abgRnlxF0CbvNgoUi8d9BENCvZXkcTewIJdEpsV5oQ1w9wKjJX75XG/ggdrvrclh > pGY1+Jglqv7ZKeTzGnHNaC0mVgMcJPK1mdr8lUrwn9M+Pmfi8Mk7nHy0XnvdkXa0 > NG4bt/9pWKc92zhVIHatu+61+oWu1b8VHOK/5i6+Ajl439efNMJZS3qYup5gReFF > zVMyDAcPKEMNAZq/+gbNgkpeH3VARAsDTM6NhrDo4w3vrxkV/NL/TC7v5bDVVJ > z0 > wE+6d+B2l3oKnHAqPXv5vpVLDg/0DeI8A620kIEsMRPdcvLm/oej1KQnDgtS5NMW > 3flVKJ4vzWzzDe8TH4V4JEttj9BsbV+3eyI49Cu4WEKnaJAFBRz1pjGiyHs8hxap > /9LhpimxDhWQ9X9ly5duLeF0hnIgFKDVqndAMIFMvAxeIl7rgX4OOUBnsUmONds > p > 5Frva8NdWwCHYnY+ocjZ1gPzePjIyVvYiPX5bLl3ATcpw3PrN4d6lBWnZKMMvcdS > qH+9BUIdxVxh6haXf6Q6YD2QkTfaQs8c6357LIyvbED3z9X9yiFV4fHtSQCKogD1 > 0B1wQs3SHrsm6+QOd2NB > =R959 > -----END PGP SIGNATURE----- > _______________________________________________ > P2PSIP mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/p2psip _______________________________________________ P2PSIP mailing list [email protected] https://www.ietf.org/mailman/listinfo/p2psip
