Re: [Gen-art] Gen-art LC review: draft-holmerg-dispatch-iotl-03.txt

[Jari Arkko <[e-mail]>]

Thanks for the review, Robert, and for the changes & list discussion Christer.

Robert, do you believe these changes are sufficient?

Jari

On 19 Dec 2014, at 17:10, Christer Holmberg <[e-mail]> wrote:

> Hi Robert,
> 
> Thanks for your review. Please see inline.
> 
>> I am the assigned Gen-ART reviewer for this draft. For background on
>> Gen-ART, please see the FAQ at
>> 
>> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
>> 
>> Please resolve these comments along with any other Last Call comments
>> you may receive.
>> 
>> Document: draft-holmberg-dispatch-iotl-03
>> Reviewer: Robert Sparks
>> Review Date: 18-Dec-2014
>> IETF LC End Date: 8-Jan-2015
>> IESG Telechat date: Not on an upcoming telechat agenda
>> 
>> Summary: Almost ready for publication as a Proposed Standard but has 
>> issues that need to be addressed
>> 
>> There are only a few issues to address:
>> 
>> Major Issue 1: It makes no sense to publish a standards track RFC that 
>> says behavior on general networks is undefined. I've reviewed all of the 
>> list traffic about this document, and I understand how the text that's 
>> currently in the document got there, but it's not the best way to deal 
>> with the concern that caused it to be introduced. The original concern 
>> was that the document didn't provide enough detail to tell what the 
>> presence or absence of the values meant, and whether there was an 
>> associate d change to the semantics of the protocol. While the 
>> description is thin, I think there has been enough shown to know that 
>> the semantics of the protocol are not being changed.
>> 
>> So, instead, the document should just recognize that devices that don't 
>> implement this specification will do what RFC 3261 requires them to do 
>> with unknown URI parameters: ignore them. This document sufficiently 
>> describes what the values it defines means to elements that _do_ 
>> implement this specification. They provide additional information to 
>> upper layers (ultimately, transaction users as 3261 defines them), and 
>> those upper layers might make forwarding decisions using it, just like 
>> they can use _anything_ at their disposal. The basic semantics of the 
>> SIP protocol are unaffected.
>> 
>> To resolve this issue, I suggest removing the text that occurs in 
>> several places saying that this is applicable only to 3gpp networks, and 
>> add a short sentence reminding the reader that RFC3261 expects new URI 
>> parameters to be standardized and defines how unknown URI parameters are 
>> handled.
> 
> I saw that you have raised this issue on the DISPATCH list, so I'll get back to it later.
> 
> -------------
> 
>> Major Issue 2: The document suggests that implementations violate what 
>> RFC3261 requires them to do. Specifically, it says "An entity that 
>> understands the 'iotl' parameter MUST NOT, from a SIP request, remove 
>> 'iotl' parameters from SIP URIs associated with other entities, unless 
>> the entity has means to determine that the 'iotl' parameter does not 
>> represent a valid traffic leg." RFC3261 requires that MUST NOT, and it 
>> does not allow the "unless" clause. It is not ok for an entity to change 
>> some other entities URIs in, say, Route, Service-Route, Path, and 
>> similar places under any circumstances.
>> 
>> My suggested resolution is to remove the unless clause, and change the 
>> first part of the sentence to note that this is what 3261 requires.
> 
> I suggest the following modified paragraph text:
> 
> 	"As defined in [RFC3261], a SIP entity must not modify remove uri parameters 
> 	from SIP URIs associated with other entities. This also applies to the
> 	'iotl' parameter."
> 
> -------------
> 
>> Major issue 3: The Security considerations section is incomplete. Please 
>> discuss the ramifications of something maliciously providing an 
>> incorrect value in the parameter. What are the ramifications if someone 
>> does violate protocol and changes or removes a value in transit?
> 
> I suggest the following modified paragraph text:
> 
>   "The information SHOULD only be used for making policy decisions based
>   on the role by nodes within the same trust domain [RFC3325].  In
>   addition, there MUST exist an agreement between the operators for
>   usage of the traffic leg information. When a SIP URI "iotl" parameter is
>   received from outside the trust domain, the parameter MUST be ignored, in order
>   to avoid to erroneous policy decisions that can impact charging, the handling of 
>   media, etc. The same can occur if an entity outside the trust domain is able to remove 
>   the parameter from a SIP URI"
> 
> -------------
> 
>> Minor issue 1: It is unclear where you expect these URIs to occur. I 
>> have a good feel only after reading the list traffic. I suggest you be 
>> explicit in 5.1 that you expect these to be placed in Service-Route and 
>> Path header field values, hence to occur in Route header field values, 
>> and Request-URIs.
> 
> The draft currently has the following paragraph:
> 
> 	"For routing of a SIP request, a SIP entity can add the 'iotl'
>   	parameter to the SIP URI of the Request-URI [RFC3261], or to the SIP
>   	URI of a Route header field [RFC3261], of an initial request for a
>   	dialog, or of an stand-alone request."
> 
> I suggest to add the following sentence to the end of that paragraph:
> 
> 	"SIP entities can add the 'iotl' parameter to the SIP URI of a Path header field [REF] or a Service-Route header 
> 	field [REF], in order for the parameter to occur in a Route header field."
> 
> -------------
> 
>> Minor issue 2: Why do you say "This document does not specify the usage 
>> of the 'iotl' parameter within a SIP URI of a Record-Route header field. 
>> Would it create an interoperability problem if someone put one there? 
>> Because if they do, it will end up in Route header fields later. If 
>> that's ok, please strike the sentence. If it's not, then you need to say 
>> MUST NOT place the parameter in URIs in Record-Route header field values.
> 
> It doesn't cause any interoperability problems - there simply aren't any procedures defined.
> 
> So, I suggest to remove the sentence.
> 
> -------------
> 
> Nits/editorial comments:
> 
>> Since you are providing an extension point for other values, someone 
>> will ask if you need a registry for those values. I suggest explicitly 
>> saying we are not creating a registry at this time but expect to do so 
>> if the extension point is ever used to head that conversation off.
> 
> I could add the following text to the Syntax/General section:
> 
> 	"This specification does not create an IANA registry for 'iotl' parameter values.
> 	If new parameters values are defined in the future, such registry needs to be 
> 	created."
> 
> Or, do you think it should be somewhere else?
> 
> -------------
> 
>> "dialogue" appears in a couple of places. Since we're talking about the 
>> 3261 term, I suggest using "dialog" consistently.
> 
> I'll fix that.
> 
> -------------
> 
>> The sentence (which occurs in the abstract and introduction) "The 
>> directionality in traffic legs relates to a SIP request creating a 
>> dialogue and stand-alone SIP request." does not parse. What is it trying 
>> to say, and why is it important?
> 
> The sentence is not needed (and it doesn't belong in the Abstract to begin with), so I suggest to remove it.
> 
> -------------
> 
> Thanks!
> 
> Regards,
> 
> Christer
> _______________________________________________
> Gen-art mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/gen-art