-----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.
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. 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.
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/gbVH2WHz32zLPiwKSY4axjmWoEvw7dsgBhfdDWKbsTH
abgRnlxF0CbvNgoUi8d9BENCvZXkcTewIJdEpsV5oQ1w9wKjJX75XG/ggdrvrclh
pGY1+Jglqv7ZKeTzGnHNaC0mVgMcJPK1mdr8lUrwn9M+Pmfi8Mk7nHy0XnvdkXa0
NG4bt/9pWKc92zhVIHatu+61+oWu1b8VHOK/5i6+Ajl439efNMJZS3qYup5gReFF
zVMyDAcPKEMNAZq/+gbNgkpeH3VARAsDTM6NhrDo4w3vrxkV/NL/TC7v5bDVVJz0
wE+6d+B2l3oKnHAqPXv5vpVLDg/0DeI8A620kIEsMRPdcvLm/oej1KQnDgtS5NMW
3flVKJ4vzWzzDe8TH4V4JEttj9BsbV+3eyI49Cu4WEKnaJAFBRz1pjGiyHs8hxap
/9LhpimxDhWQ9X9ly5duLeF0hnIgFKDVqndAMIFMvAxeIl7rgX4OOUBnsUmONdsp
5Frva8NdWwCHYnY+ocjZ1gPzePjIyVvYiPX5bLl3ATcpw3PrN4d6lBWnZKMMvcdS
qH+9BUIdxVxh6haXf6Q6YD2QkTfaQs8c6357LIyvbED3z9X9yiFV4fHtSQCKogD1
0B1wQs3SHrsm6+QOd2NB
=R959
-----END PGP SIGNATURE-----
_______________________________________________
P2PSIP mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/p2psip
