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

Marc Petit-Huguenin <marc@petit-huguenin.org> Sun, 07 March 2021 15:49 UTC

Return-Path: <marc@petit-huguenin.org>
X-Original-To: asap@ietfa.amsl.com
Delivered-To: asap@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 102DE3A1816; Sun, 7 Mar 2021 07:49:35 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.9
X-Spam-Level:
X-Spam-Status: No, score=-1.9 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id oK5BDVPqxeYI; Sun, 7 Mar 2021 07:49:32 -0800 (PST)
Received: from implementers.org (implementers.org [92.243.22.217]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id ED4A83A181B; Sun, 7 Mar 2021 07:49:28 -0800 (PST)
Received: from [IPv6:2601:648:8400:8e7d:d250:99ff:fedf:93cd] (unknown [IPv6:2601:648:8400:8e7d:d250:99ff:fedf:93cd]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "Marc Petit-Huguenin", Issuer "implementers.org" (verified OK)) by implementers.org (Postfix) with ESMTPS id 776DBAE21E; Sun, 7 Mar 2021 16:49:24 +0100 (CET)
To: draft-kinamdar-dispatch-sip-auto-peer.authors@ietf.org
From: Marc Petit-Huguenin <marc@petit-huguenin.org>
Cc: "asap@ietf.org" <asap@ietf.org>
Message-ID: <ced4f399-3fe9-9117-6fc0-f3820e5a6318@petit-huguenin.org>
Date: Sun, 07 Mar 2021 07:49:22 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"; format="flowed"
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Archived-At: <https://mailarchive.ietf.org/arch/msg/asap/stfBV5TrKFLFhmP4zQiFoI2kRR4>
Subject: [Asap] Review of draft-kinamdar-dispatch-sip-auto-peer-05
X-BeenThere: asap@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Automatic SIP trunking And Peering WG <asap.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/asap>, <mailto:asap-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/asap/>
List-Post: <mailto:asap@ietf.org>
List-Help: <mailto:asap-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/asap>, <mailto:asap-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sun, 07 Mar 2021 15:49:35 -0000

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: marc@petit-huguenin.org
Blog: https://marc.petit-huguenin.org
Profile: https://www.linkedin.com/in/petithug