Subject: [P2PSIP] Review of draft-ietf-p2psip-sip-06

From: Marc Petit-Huguenin <[email-masked]>
Date: Mon, 15 Aug 2011 17:23:30 -0700

1. Section 3: SipRegistration structure

I would suggest to use "SipRegistration Resource Record" instead, to match the
terminology in -concepts.

Same thing for SipRegistration PDU in the same section.

2. Section 3: 1st paragraph

s/This uses the SIP-REGISTRATION Kind-ID/This uses the SIP-REGISTRATION Kind/

3. Section 3: opaque contact_prefs<0..2^16-1>;

The format of the contact_prefs field is not described.  Is it RFC 3840 or RFC
2533 (better in my opinion) or something else?

4. Section 3: Destination destination_list<0..2^16-1>;

I think it would be better to have a list of destination_list instead, to be
able to register different paths to the destination.  This could be useful if a
node decides to register as a client to multiple peers, using the procedure in
the second bullet of section 3.2.1 of -base.

5. Section 3, second bullet of last list.

It says that the storing peer must check that "[t]he certificate contains a
Node-ID that is the same as the dictionary key it is being stored at.", which is
not exactly true when the certificate contains multiple Node-ID.  It would be
better to say:

"The Node-ID of the signer is the same as the dictionary key it is being stored at."

6. Section 4, first item in list

"Check to see if the domain part of the AOR matches the domain name of an
overlay of which he is a member."

The way I understand this sentence, the "sip:[email-masked]" AOR
matches an overlay named "overlay.implementers.org".  Was that the intent of
this rule, or was the intent that the "sip:[email-masked]" AOR only
matches the "implementers.org" overlay?

7. Section 5.

"If certificate-based authentication is in use, the responding peer MUST present
a certificate with a Node-ID matching the terminal entry in the route list."

If this sentence is about the AppAttach itself (and not about the connection
created by the AppAttach), isn't that rule already part of -base?

8. Section 6. Second sentence.

I do not understand what this sentence means.

9. Section 8.1. "In this section we discuss security issues that are likely to
be relevant to any usage of RELOAD."

There is no security issues discussed in section 8.1.

10. Section 9

Please change "TBD" by the code point that was already assigned (1, I think), so
early implementations can do interoperability testing.

Nits
====

- - Abstract

s/(RELOAD),/(RELOAD)./

- - Section 2

s/consiered/considered/

- -- 
Marc Petit-Huguenin
Personal email: [email-masked]
Professional email: [email-masked]
Blog: http://blog.marc.petit-huguenin.org