Re: [Asap] Review of draft-kinamdar-dispatch-sip-auto-peer-05

[Marc Petit-Huguenin <[e-mail]>]

Thanks Sreekanth and Kaustubh.

Looks good, see inline for a few comments.

On 6/12/21 10:02 PM, Kaustubh Inamdar (kinamdar) wrote:
> Thanks for your comments Marc...please find responses below from both Sreekanth & I.
> 
> 
>    A. Major General Comments
>        A.1. The document refers to the SIP Connect profile. and my understanding of SIP Connect is that it is US centric -- e.g., UK's NICC certainly has a different understanding on how Private-Asserted-Identity is used.  So I think that the capability set should be based first on a choice of profiles, not just implicitly on one (and BTW, which SIP Connect profile? there is 3 of them).  After a profile is selected, there seems to be two different set of properties that can be attached to it:
>        1) Choices that are part of the profile (e.g. DNS servers, Registration Mode vs Static Mode, any choice related to MAY keyword, etc..)
>        2) Properties that override the profile MUST/MUST NOT and SHOULD/SHOULD NOT.  E.g., if the provider does not support TCP (which is a MUST) there should be a property that overrides that implicit value.  It could also be used to add stuff that is not existing in the profile, like the SCTP transport for SIP.
>        To complement that, I would permit profile-less capabilities, where all the properties have to be explicit.
> Not applied, I'd like to know the reasoning here.
> KI:The SIP Connect technical recommendations isn’t US centric, it is applicable globally. There is a possibility of course of certain countries and geographies deferring to other, more localised, technical recommendations. The capability set document has been designed to include elements, which when populated correctly by the ITSPs, convey enough information to provide successful peering. The capability set isn’t using SIP Connect as a template, rather, it was designed looking at common ITSP peering issues across the globe. Having said that, there should ideally be a section added about caller identity (which currently isn’t there) - specifically around how caller ID is formatted (E.164 v/s non E.164 formatting) and which header field values must be included to convey called ID. For example, whether it is required to include the PAID along with the From header field value. We will add a section on Caller ID in an upcoming version of the draft.

OK.  After these modifications a review by someone familiar with different jurisdictions may still be useful.

> 
>        A.2. There may be a need to provide a new capability set ahead of time, to ensure a smooth transition when properties change.  That can be done by permitting multiple capability sets in a document, and adding a "not-before" date.
> Not applied, I'd like to know the reasoning here.
> 
> KI: We actually wanted to have the group weigh in on this one. Below is the text we sent in response to the new version notification:
> “Also, it might be useful to have the SIP service provider being capable of providing a capability set document that is applicable at a future date. However, perhaps, providing multiple capability sets in a document might not be the cleanest approach. The following approaches might be more tenable -
> 
> A) Use WebFinger to find out if an new capability set document is available.
> B) Enhance the YANG model to include elements that accommodate the specifics of a future applicable version of the capability set document. For example:
> 
>    <revision>
>       <notBefore>Some_future_date</notBefore>
>       <location>URL to fetch future version</location>
>     </revision>
> 
> Option B above might be cleaner as it removes any dependencies on WebFinger.”

Yes, Option B seems simpler.

Thanks.

> 
> B.4. The first sentence of Section 7 should be moved to section 1 "Introduction" as it is not normative.
> Not applied, please elaborate.
> 
> KI: The introduction section mentions about the capability set, however, it doesn’t include any details about the elements within the capability set. The placement of the section on state deltas is incorrect and ideally should go after the section on “Processing the Capability Set Response” and provides a natural, logical flow to the sequence of events. We will change this in a new version.
> 
>   B.3. Similarly section 10 and 11 could be moved as subsections of a new section "Examples", with some text explaining that the examples are informative.
> 
> Seems applied, but for the informative admonition.
> SN: Will correct the text in the new version
> 
>   B.5. I would move section 6.6 "Identifying the Request Target" before 6.5 "Generating the Response", so the text follows the messages flow (client->server->client).
> 
> Section 6.6. seems to have disappear.
> SN: This is now section 4.5
> 
>   B.6. In the same spirit I would add a section after section 9 that explains the processing of the response from the client point of view.
> 
> I think that's the new section 8, correct?
> SN: Yes this is now section 8.
> 
>   B.7. Section 6.3 should probably add some text about verifying the TLS hostname.
> 
> I see some new text in section 4.3, but that text is AFAICT incorrect. Verifying the TLS hostname is not an alternate way of authenticating the client, but a way of validating the server. If you are looking for alternate ways of authenticating the client, client certificates or OAUTH can be added to the list.
> SN: I agree. The text in this section can be a little clearer. Will correct this in the new version.
>   C.2 The Yang model in section 9.2, the example of extension in section9.4, and the examples in section 10.1 do not fit in a 72 column page, which makes them difficult to read and review.
> 
> There is still some places were lines are too long. Converting the xml2rfc v3 file in text format lists these places.
> SN: The lines in the yang model definition will be corrected in the new version. There were a few lines that were longer in the Example section (xml representation of the model) as we didn't want to break the line. We will break the line in the new version.
>   C.3 The references bibliography does not seem to follow the standard used by most RFC, namely that the URI in in the bibliographic item, not on a separate list.  Also the DOI is missing.
> 
> Thanks for doing that, it is better now. That said I think that putting the URL together with the citation label is a bit distracting when reading the text version of the draft.
> SN: Ack. Will rework this issue in the next version.
> ________________________________
> From: Marc Petit-Huguenin <[e-mail]>
> Sent: Sunday, March 7, 2021 9:19 PM
> To: [e-mail] <[e-mail]>
> Cc: [e-mail] <[e-mail]>
> Subject: Review of draft-kinamdar-dispatch-sip-auto-peer-05
> 
> Hi,
> 
> Gonzalo asked me if I could do a review of this draft.  You will find a preliminary review below -- there is still stuff in that draft I need to think about, mostly around sections 9.2 and 9.3.
> 
> A. Major General Comments
> 
> A.1. The document refers to the SIP Connect profile. and my understanding of SIP Connect is that it is US centric -- e.g., UK's NICC certainly has a different understanding on how Private-Asserted-Identity is used.  So I think that the capability set should be based first on a choice of profiles, not just implicitly on one (and BTW, which SIP Connect profile? there is 3 of them).  After a profile is selected, there seems to be two different set of properties that can be attached to it:
> 
> 1) Choices that are part of the profile (e.g. DNS servers, Registration Mode vs Static Mode, any choice related to MAY keyword, etc..)
> 
> 2) Properties that override the profile MUST/MUST NOT and SHOULD/SHOULD NOT.  E.g., if the provider does not support TCP (which is a MUST) there should be a property that overrides that implicit value.  It could also be used to add stuff that is not existing in the profile, like the SCTP transport for SIP.
> 
> To complement that, I would permit profile-less capabilities, where all the properties have to be explicit.
> 
> A.2. There may be a need to provide a new capability set ahead of time, to ensure a smooth transition when properties change.  That can be done by permitting multiple capability sets in a document, and adding a "not-before" date.
> 
> A.3. I'd really like to see the mechanism described in his draft used to also retrieve properties related to STIR/SHAKEN.  E.g. the ACME server to use to get a delegated certificate from the provider acting as a CA (which could serve as an alternate way to retrieve the list of DIDs).
> 
> 
> B. Minor General Comments
> 
> B.1. I would move section 2 "Conventions and Terminology" after section 5 "Overview of Operations" as a way to indicate where the Informative part ends, and where the Normative part starts.  Also you should use BCP 14 instead of just RFC 2119.
> 
> B.2. Section 3 "Reference Architecture" and section 4 "Configuration Workflow" should probably be moved as subsections of section 5 "Overview of Transport".  That may help look at redundancies, like the last paragraph of section 5 that says more or less the same thing than the first paragraph of section 3.
> 
> B.3. Similarly section 10 and 11 could be moved as subsections of a new section "Examples", with some text explaining that the examples are informative.
> 
> B.4. The first sentence of Section 7 should be moved to section 1 "Introduction" as it is not normative.
> 
> B.5. I would move section 6.6 "Identifying the Request Target" before 6.5 "Generating the Response", so the text follows the messages flow (client->server->client).
> 
> B.6. In the same spirit I would add a section after section 9 that explains the processing of the response from the client point of view.
> 
> B.7. Section 6.3 should probably add some text about verifying the TLS hostname.
> 
> 
> C. Presentation Comments
> 
> C.1. It would be great to submit the xml2rfc v3 file instead of the txt file so more convenient reviewing tools can be used.
> 
> C.2 The Yang model in section 9.2, the example of extension in section9.4, and the examples in section 10.1 do not fit in a 72 column page, which makes them difficult to read and review.
> 
> C.3 The references bibliography does not seem to follow the standard used by most RFC, namely that the URI in in the bibliographic item, not on a separate list.  Also the DOI is missing.
> 
> 
> D. Nits
> 
> D.1. Section 5, 1st paragraph: s/HTTPs"HTTPS/
> 
> D.2. Section 6.2: s/https uri/HTTPS URI/
> 
> D.3. Section 6.4, last paragraph:  Use MUST and MUST NOT.
> 
> D.4. Section 9.3, "signallingForking" item: s/Boolen/Boolean/
> 
> D.5. Section 9.3, "version" item: s/provide/provider/
> 
> D.11. Section 11, first example: s!GET //!GET /!
> 

-- 
Marc Petit-Huguenin
Email: [e-mail]
Blog: https://marc.petit-huguenin.org
Profile: https://www.linkedin.com/in/petithug