Last Call Review of draft-ietf-sipcore-locparam-06
review-ietf-sipcore-locparam-06-opsdir-lc-chown-2020-02-17-00

Request Review of draft-ietf-sipcore-locparam
Requested rev. no specific revision (document currently at 06)
Type Last Call Review
Team Ops Directorate (opsdir)
Deadline 2020-01-27
Requested 2020-01-13
Authors James Winterbottom, Roland Jesske, Bruno Chatras, Andrew Hutton
Draft last updated 2020-02-17
Completed reviews Genart Last Call review of -04 by Ines Robles (diff)
Secdir Last Call review of -04 by Rich Salz (diff)
Opsdir Last Call review of -06 by Tim Chown
Assignment Reviewer Tim Chown 
State Completed
Review review-ietf-sipcore-locparam-06-opsdir-lc-chown-2020-02-17
Posted at https://mailarchive.ietf.org/arch/msg/ops-dir/JFpzSLXz0vstXfsfKtZziffwKUk
Reviewed rev. 06
Review result Has Issues
Review completed: 2020-02-17

Review
review-ietf-sipcore-locparam-06-opsdir-lc-chown-2020-02-17

Hi,


I have reviewed this document as part of the Operational directorate's ongoing effort to review all IETF documents being processed by the IESG.  These comments were written with the intent of improving the operational aspects of the IETF drafts. Comments that are not addressed in last call may be included in AD reviews during the IESG review.  Document editors and WG chairs should treat these comments just like any other last call comments.

The draft describes an extension to RFC6442 to support the inclusion of a “loc-src” parameter to the SIP Geolocation field such that when multiple locationValues are present the recipient can make a better informed decision about which locationValue it wishes to use.

For the context of my comments, I am not that familiar with SIP-related protocols. 

The rationale for the addition of this parameter appears sound, and its definition seems appropriate. However, the document is written in such a way that the rationale is split over two sections, and the mechanism for including the parameter is split across three sections, leaving the document a little clumsy for readers to follow.

Overall, the document is one that should ultimately be published, but as is it has issues, and in my view needs some restructuring to make it more concise and simpler to follow, so is not (yet) ready for publication.

Main comments:

The definition of the parameter itself is fine.

The first issue is around the rationale.  This seems perfectly sound, but the document splits the reasoning between most of the Introduction and the first two paragraphs of Section 3.  It would be better to have the rationale in a Rationale section, without repetition. 

The more confusing issue is the way the mechanism is described over Sections 3 (paragraphs 3,4,5), 4 (paragraph 3) and 7 (all paragraphs).  There is repetition, some clashes, and some ambiguity.  Rewriting the text to have a single mechanism section would make the text far clearer, in my view.

Here is how I would suggest the text is restructured:

Introduction 
Largely paras 1 and 2 of the current intro.

Rationale 
Largely para 3 of the intro, and paras 1 and 2 of the existing Rationale section.

Loc-src parameter specification
Paras 1-2 of the Mechanism section, with the Figure, and para 6 of the Rationale section.

Mechanism
A rewrite of paras 3-5 of the current Rationale section, para 3 of the current Mechanism section, and pretty much all of the Security section.

Example
Can stay as is

Privacy Considerations
Fine

Security Considerations
Just one para mentioning the trust issue and the protections defined in the Mechanism section would be fine.  Don’t split the guidance as it currently is across three sections - put it all in one pace where - a new Mechanisms section as above - it can be much more easily followed and implemented.


Other comments:

Section 3
Para 2 says the document makes no comment on the rationale, but the document explains the rationale.  Perhaps just delete this.
Para 5 talks about another networks with no trust; is the issue messages sent to (as stated) or also received from?  I think when you pull all the mechanism text into one place you can make this clearer.

Section 4
Para 3 - what is the difference between a UA and UE from the point of view of an end node adding (or not adding) a loc-src parameter?  It says a UA MUST NOT add one, but in Section 5 states that a UE might provide it?  
The last sentence talks of removing loc-src for certain case(s) but this is also included in Sections 3 and 7.  It really needs to be stated clearly in one place.

Section 5
Para 1  - RFC 4483 or RFC 2392?  RFC6442 cites 2392 for cid.

Section 7
Para 1 - What is a “location recipient”?  A receiver which is inspecting a header with multiple locationValues?  Perhaps define this, or rewrite.
Paras 2 and 3 - “strongly recommended” should be “RECOMMENDED” ?
I think much of this section is repeating what has been said before, e.g. in Section 4 it says “MUST remove” and here it says removing it is (only) strongly recommended.  

Bets wishes,
Tim