Re: [sipcore] REMINDER: Re: WGLC: draft-ietf-sipcore-rfc4244bis

[Mary Barnes <[e-mail]>]

John,

I apologize for missing these in the round of -02 changes. Per the
other email thread, some of these are no longer relevant due to the
extent of changes and some were redundant with issues raised
(individually) in the tracker.   So, I'll just responsd to  those that
are still relevant [MB].  I will note that one thing that would help
me  to avoid losing document reviews in my mailbox is that if folks
could change the "Reminder" to (or prepend) the term "Review" (or
"Comments") so that the email is a bit easier to find when one's
mailbox is overflowing with issues from the issue tracker.

Thanks,
Mary.

On Tue, Aug 31, 2010 at 11:32 AM, Elwell, John
<[e-mail]> wrote:
> I have reviewed the document as far as section 8, before running out of time. Although the technical content of the document is reasonably stable, there are far too many minor problems that need fixing before it is ready to go. Specific comments:
>
> 1. In general, there are too many normative statements (MUST/SHOULD) written in the passive voice, or in the active voice with an inappropriate subject. I have pointed out a few of these in later comments, but all should be checked.
>
> 2. It nearly always uses the term "header" rather than "header field", although never, as far as I know, meaning the entire SIP header.
[MB] I'm not sure that I fully understand the concern. Can you point
out a specific sentence or section of concern?  The term "header" is
used when it is referring to the History-Info header field, but that
is done in other documents as well.  In general, I think it's
understood that it's referring to the header field when it's
qualified. Perhaps you're referring to places where it is not
qualified?   The term hi-entry is used, as well.
[/MB]
>
> 3. Section 2:
> "The term "retarget" is used in this document to refer both to the
>   process of a Proxy Server/User Agent Client (UAC) changing a Uniform
>   Resource Identifier (URI) in a request based on the rules for
>   determining request targets as described in Section 16.5  of [RFC3261]
>   and the subsequent forwarding of that request as described in section
>   16.6 of [RFC3261]."
> Sections 16.5 and 16.6 of RFC 3261 are concerned only with proxies, so for the UAC case it is inappropriate to say the term applies as described in those sections.
>
> 4. Section 2:
> "Noting that  [RFC3261] uses the term ... "
> This sentence lacks a main clause.
>
> 5. Section 2:
> "for which is  not"
> Should be "for which it is not"
>
> 6. Section 3:
> "by locating the first  entry just prior to the last hi-entry marked "
> The word "first" is confusing. It seems to be redundant - unless it has some special meaning that I have failed to grasp.
>
> 7. Section 4.1:
> "and if the Request-URI was modified by a previous hop"
> What is a hop? I tend to think of a hop as being the "wire" between two SIP entities, but here it seems to be used to mean a SIP entity such as a proxy. If we really want to use this term, we should define it. Check for other instances too.
[MB] I will change this to "previous SIP hop". Note that the term
"hop" is used throught RFC 3263. If you would like, I could add a note
in the terminology that the term is consistent with that. [/MB]
>
> 8. Section 4.1:
> "A UAC that wants to ensure that privacy not
>   be applied to its identity  MUST include a Privacy header with a priv-
>   value of "none.""
> Privacy associated with a UAC's identity is outside the scope of History-Info, which in general does not reveal a UAC's identity.
>
> 9. Section 4.1:
> "reason  header"
> Capitalize - should be "Reason".
[MB] I'll fix that. It's now in section 5.1. [/MB]
>
> 10. "the reason  header  MUST be associated with the hi-
>   targeted-to-uri in the previous (last) hi-entry, as described in
>   Section 6.3.3"
> I don't think 6.3.3 really tells me how to achieve this association - it tells me when I must associate. But see also comments on 6.3.3.
>
> 11. Section 4.1:
> "A new hi-entry MUST then be added for the URI from
>   the Contact header (which becomes the new Request-URI)."
> This seems to be a rather convoluted way of saying that you use the new URI from the new Request URI to populate the new hi-entry. Mentioning Contact header (field) is confusing, since the recursed request also contains a Contact header field.
[MB] I will just change that to: "The UAC MUST add an hi-entry using
the Request-URI of the request as the hi-targeted-to-uri."   [/MB]
>
> 12. Section 4.1:
> "If the URI in the
>   Contact header  contains a "hit" URI parameter"
> Make it clear this is the Contact header field from the 3xx response.
>
> 13. Section 4.1:
> "Prior to any application usage of the information  by the UAC"
> It is not clear what information this is talking about. Presumably H-I in responses (and not just 3xx responses mentioned in the previous paragraph).
>
> 14. Section 4.2.1:
> "The last hi-entry reflects the most recent
>   target and SHOULD contain  the Request-URI for the received request."
> We cannot place a normative requirement on hi-entry. There is no normative requirement on the UAS concerning receipt - the UAS is not in control of what it receives. Should rephrase, e.g., "will normally contain".
[MB] Agreed. [/MB]
>
> 15. Section 4.2.1:
> "the
>   UAS MUST insert an hi-entry ."
> How is this testable, other than by the fact that H-I will get reflected in the response? But that should be covered in 4.2.2, shouldn't it?
[MB] It's not testable on the wire. However, to applications at the
UAS expecting that entry to be in a request that they might receive,
it would seem appropriate that the field be added to the Request that
is presented to any application. Doing that also ensures it will be in
the response which is what the text currently states. I do not think
that applications should be screening the information it receives to
look for this situation. [/MB]
>
> 16. Section 4.2.1:
> "If privacy is required, the procedures of
>   Section 6.3.2 MUST be used."
> As far as I can see, 6.3.2 talks only about requests, whereas here we are talking about responses. I am not convinced considerations are quite the same.
[MB] We did update the privacy section to be more clear that this is
dealing with the privacy of the Request-URI for the UAS at which the
requrest terminated. I think we can qualify that to say "If privacy is
required for the response, ..."[/MB]
>
> 17. Section 4.2.2:
> "The information can also be useful for
>   implementations with B2BUAs that include the History-Info, received
>   in the incoming request, in the subsequent outgoing request."
> The words "implementations with B2BUAs" seem strange - why not just "B2BUAs"?
[MB] I don't see this text in section 4.2.2. of -01 nor 5.2.2 of -02.
 I think you are referring to section 4.2.1.  I'll change that it was
trying to qualify a B2BUA as something that was very implementation
specific and not standard SIP per se. [/MB]
>
> 18. Section 4.2.2:
> "the UAS MUST
>   include any History-Info received in the request  in the subsequent
>   response"
> Does this include 100 responses?
[MB] I would think so. Can you think of a reason why it shouldn't?  I
would think it could be useful for debug. [/MB]
>
> 19. Section 4.2.2:
> "The processing of History-Info in responses follows the methodology
>   described in Section 16.7 of [RFC3261 ]"
> Section 16.7 is concerned only with proxies, not UASs.
[MB] Yeah. I'm not quite sure when that strayed into that section.
It's duplicated in section 6.2. I think it was a cut 'n paste error,
since the general processing of HI is the same in responses whether
sent by a proxy or UAS. [/MB]
>
> 20. Section 5.1.1:
> "Upon receipt of an initial request for a dialog, or a standalone
>   request"
> This is a different formulation from that in 4.1: "in any request not associated with an established dialog".
[MB] The  relates to issue 21 which Dale wanted to qualify to include
early dialog's so I changed it to, "not associated with an early or
established dialog".  BTW, there's actually another occurrence in the
requirements section in Appendix A (req 6), which matches the change
that I made although the wording  in 5.1 is still different, so that
needs to be changed. [/MB]


>
> 21. Section 5.1.1:
> "The proxy MUST add a "hit" SIP/SIPS URI parameter
>      for the target URI that are determined"
> Singular/plural error.
>
> 22. Section 5.1.1:
> "If
>      privacy is required, the procedures of Section 6.3.2 MUST be used."
> In fact I think one needs to go to 6.3.2 to find out how to determine whether privacy is required, as well as for the procedures when it is found to be required.
[MB] I'll change that to:
  The procedures of
  section 7.3.1 MUST be followed to accomodate any privacy
  requirements.
[/MB]
>
> 23. Section 5.1.2:
> "When re-sending a request as a result of retargeting because of
>   failure"
> Should we make it clear that this doesn't apply in case of 3xx, i.e., which errors it does apply to.
[MB] It seems clear to me with the "i.e., either reception of error
responses or a timeout which
is considered to be an implicit 408 error response"  as 3xx responses
are not considered errors nor failures per RFC 3261 (i.e., the request
still has the potential to be successful). We could just change it to
read:
"   When re-sending a request as a result of retargeting because of
 either receipt of either an error response or a timeout which
is considered to be an implicit 408 error response, ..."

However, per the response to your comment #25, we may not need to
separate the 3xx responses. [/MB]
[/MB]
>
> 24. Section 5.1.2:
> "If the received error response did not include
>      any History-Info header fields, the proxy MUST use the same
>      History-Info header fields that were sent in the outgoing request
>      that failed to build the outgoing request."
> Hard to understand, because of the two instances of "outgoing request". Perhaps:
> "... MUST use the same History-Info header fields in the new outgoing request as those sent in the request that failed."
[MB] Yes - that is clearer. [/MB]
>
> 25. Sections 5.1.2 and 5.1.3. These are different, in that 5.1.2 allows for multiple branches failing, but 5.1.3 allows for only a single request encountering 3xx. Firstly, it is possible >1 request can return 3xx (the proxy is not forced to recurse immediately the first 3xx arrives). Secondly it is possible that some failure responses might arrive before the 3xx on which the proxy recurses. I think the new request should reflect H-I from all previous branches, not just the one leading to recursion. Also the tagging of the recursed request should take account of all previous branches.
[MB] Yes - the redirection case should be consistent.  And, really I
don't know that we need a separate case for redirection as I agree it
should send the aggregate information, as well.  [/MB]



>
> 26. Section 5.2:
> "A proxy that receives a request with the "histinfo" option tag in the
>   Supported header, SHOULD forward captured History-Info in subsequent,
>   provisional , and final responses "
> Does this include 100 responses?
[MB] That's the implication of "provisional" per RFC 3261. [/MB]
>
> 27. Section 5.2:
> "A proxy MAY anonymize any hi-entry whose domain corresponds to a
>   domain for which it is responsible (as per [RFC3323 ])."
> I don't think RFC 3323 tells me how to anonymize an hi-entry.
[MB]  RFC 3323 discusses anonymization and the role of an anonymizer,
etc. So, I'm not clear on your concern. [/MB]
>
> 28. Section 5.2:
> "The processing of History-Info in responses follows the methodology
>   described in Section 16.7 of [RFC3261], with the processing of
>   History-Info headers  adding an additional step, just before Step 9,
>   "Forwarding the Response"."
> What is meant by "processing of History-Info headers"? Does this refer to the procedures of the first two paragraphs?
[MB] Yes. [/MB]
>
> 29. Section 6.1:
> "The format for
>      this parameter is a string of digits, separated by dots to
>      indicate the number of forward hops and retargets"
> It is not clear from this whether the dots indicate the number of forward hops or the number of retargets. I know it is explained in detail later, but for somebody reading from front to back, this isn't very clear. Again, use of the word "hop" without definition is a concern.
[MB] As I stated above, the term hop is used throughout RFC 3263.  I
can qualify as a SIP hop. Each dot is a  hop and then each increment
is a retarget.  So, I could reword as:
        "The format for
         this parameter is a string of digits separated by dots. Each dot
         reflects a forward SIP hop and the value of the digit
         indicates the number retargets at each hop."

 [/MB]
>
> 30. Section 6.1:
> "By adding the
>      new entries in order (i.e., following existing entries per the
>      details in Section 5.1 ), "
> Section 5.1 doesn't really give details - in fact it refers forward to 6.3.4.
[MB] Okay. I'll change the reference. [/MB]
>
> 31. Section 6.1:
> "The latter case indicates that the
>      History-Info entries for the domain MUST  be anonymized prior to
>      forwarding, whereas the use of the Privacy header escaped in the
>      hi-targeted-to-uri means that a specific hi-entry MUST be
>      anonymized."
> Two instances of bad use of normative language. The active voice should be used, with the subject indicating to what the requirement applies (presumably to an entity forwarding a request or response in certain circumstances, but that surely is covered by the relevant procedures sections). I suspect we shouldn't be using normative language here.
>
> 32. Section 6.1:
> "The following summarizes the  syntax of the History-Info header"
> It looks to me like a formal definition, rather than a summary.
>
> 33. Section 6.1:
> "hi-index = "index" EQUAL 1*DIGIT *("." 1*DIGIT) "
> Would it be better to have a new definition index-value, which could then be reused by mp-param?
>
> 34. Section 6.1:
> "There MUST be no more than one hi-target parameter."
> Presumably this is per hi-entry.
>
> 35. Section 6.3.1:
> "SIP/SIPS URI target parameter for History-Info Header"
> A confusing title, because the parameter seems to used in the URI when the URI is NOT in a History-Info header field.
>
> 36. Section 6.3.1:
> "This
>   parameter is used to populate the hi-target parameter Section 6.3.5
>   when a Request-URI is first added in a hi-entry in the History-Info
>   header field ."
> I would say the parameter is used when an hi-entry is added containing the Request-URI value.
>
> 37. Section 6.3.1:
> ""mp": The Request-URI is a URI that represents another user.  This
>      occurs when a request is to be statically or dynamically
>      retargeted to another user. "
> So what if a request is retargeted to something that is neither a contact nor the AOR of another user, e.g., retargeting a dial string to a service URN?
>
> 38. Section 6.3.2:
> This whole section talks about requests. What about responses? For example, a request from domain A arrives at a proxy in domain B, which inserts an hi-entry requiring privacy before forwarding to the UAS. The UAS returns it in a response. I believe that hi-entry should be anonymized or removed before passing back to domain A, but I can't find anything requiring this
>
> 39. Section 6.3.2:
> "If the requestor has indicated a priv-
>   value of "session" or "header" in the request, all History-Info
>   entries MUST be anonymized  when the request leaves a domain for which
>   the intermediary is responsible."
> Another bad formulation of a normative statement. It should use the active voice, with a subject indicating the entity to which the requirement applies. Or is that better handled in section 5 (perhaps it is already covered - I haven't checked)?
>
> 40. Related to the above, I think the split of procedures between sections 4 and 5 on the one hand and section 6 on the other hand is rather confusing, and leads to a lot of forward and backward referencing and, in one or two places, duplication I think. Perhaps it is too late to do much about this, except to ensure consistency and avoid repetition.
[MB] RFC 4244 had the order of those two sections switched. One of the
co-authors preferred this approach. I personally preferred the former
approach (i.e., defining the protocol elements and then defining the
usage of such by each SIP entity).  It is a chicken and egg thing
really.  I don't know that one approach has distinct advantages over
others - I think the explanations need to be separate and that's
consistent with other SIP RFCs (which follow the protocol definition
first then the separate sections per SIP entity).  We did try to
remove as much duplication as possible, but I think sometimes it
doesn't hurt to re-iterate things, although certainly that increases
the burden in ensuring consistency.  [/MB]
>
> 41. Section 6.3.3:
> "For retargets that are the result of an explicit SIP response, a
>   Reason MUST be associated with the hi-targeted-to-uri ."
> Again this is passive voice. Also, what is meant by "associated with". I assume it means include a Reason in the hi-entry - why not say so?
[MB] That's fine - the verbage is original RFC 4244 language, but can
be changed as can the next two items (42,43). [/MB]
>
> 42. Section 6.3.2:
> "MUST be included  as the
>   Reason associated with the hi-targeted-to-uri that has been
>   retargeted."
> Again, the passive voice, and again I don't like the vague expression "associated with". Similarly in subsequent sentences in this paragraph.
>
> 43. Section 6.3.4:
> "an index MUST be  included along with the
>   Targeted-to-URI being captured"
> Passive. Who or what does this requirement apply to? Presumably to any entity inserting an hi-entry, but not to an entity merely passing one on, or a UAS reflecting an hi-entry back towards the UAC.
>
> 44. Section 6.3.4:
> "Within each level, the
>   integer reflects the number of peer entities to which the request has
>   been routed ."
> I don't think this is necessarily correct. For example, I could have two branches, each with a different target, but both resolving to the same next hop peer entity. Shouldn't it reflect the number of branches?
[MB] That does mean the number of branches.  Per your comment on
section 6.1, I can reword that differently - i.e., consistent with the
suggestion above. [/MB]
>
> 45. Section 6.3.4:
> "With the index for each new
>       branch calculated by incrementing the last/lowest  digit at the
>       current level"
> What does last/lowest mean? I can understand "last", but not "lowest". I spotted at least one other instance of this formulation.
[MB] "lowest" is the "lowest" part of the branch if you will.  Per
comment above, I can change the description to match the response to
your comment on 6.1. [/MB]
>
> 46. Section 6.3.4:
> "For example, if the index in the History-Info header of
>       the received request was 1.2, then the index in the History-Info
>       header field for the new hi-targeted- to-URI would be 1.3. "
> This doesn't make sense. If the received request had index 1.2, surely the index of the new hi-entry would be 1.2.x? Or should "received request" be "received 3xx response"? Even then I am not convinced that the index of the new hi-entry would necessarily be 1.3 - there may already have been a 1.3, in which case it would have to be 1.4, and so on.
>
> 47. Section 6.3.4:
> "Each index are  sequentially
>       assigned."
> Change "are" to "is".
>
> 48. Section 6.3.4:
> "Note that for each individual fork, only the entry corresponding
>       that  that fork is included "
> Repeated "that".
>
> 49. Section 6.3.5:
> "The hi-target attribute MUST  be added to the History-Info header if
>   the target to which the request is being forwarded contains a "hit"
>   URI parameter as defined in Section 6.3.1."
> Again passive voice - what is the subject? Assuming it is a UAC or a proxy, isn't such a requirement already covered by sections 4.1 and 5? We should not repeat normative statements in different sections.
>
> 50. Section 6.3.5:
> "If the value of the "hit" parameter is "mp", then the index of the
>   entry corresponding to the original target (i.e., the "mapped-from"
>   target) MUST  be added as a parameter to "mp"."
> Similar comment to above.
>
> 51. Section 6.3.5:
> "2.  Entry prior to last entry with hi-target of "rc" in the Request
>       received by a UAS - i.e., the Request URI associated with the
>       destination of the request was determined based on a Registered
>       Contact."
> I don't understand the i.e. part. The entry prior to the last entry with "rc" was not determined based on a registered contact - it is the next entry (the one containing rc) that was so determined.
>
> 52. Section 6.3.5:
> "4.  Entry prior to first entry with hi-target of "rc" in the Request
>       received by a UAS - i.e., the first Registered Contact to which
>       the request was targeted."
> Similar comment.
>
>
> John
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>> -----Original Message-----
>> From: [e-mail]
>> [mailto:[e-mail]] On Behalf Of Adam Roach
>> Sent: 25 August 2010 16:46
>> To: SIPCORE Chairs
>> Cc: SIPCORE (Session Initiation Protocol Core) WG
>> Subject: [sipcore] REMINDER: Re: WGLC: draft-ietf-sipcore-rfc4244bis
>>
>>   [as chair]
>>
>> As a reminder, we're just over halfway through this WGLC, and
>> have not
>> yet seen any comments. Please take some time to review this draft.
>>
>> /a
>>
>> On 8/16/10 4:29 PM, Adam Roach - SIPCORE Chair wrote:
>> >
>> > [as chair]
>> >
>> > A major author of draft-ietf-sipcore-rfc4244bis-01 believes
>> that the
>> > document has no remaining open issues, and is ready for evaluation.
>> > Today, we are starting a two-week working group last call
>> period. This
>> > last call period ends on Tuesday, August 31st.
>> >
>> > The latest version of the document can be retrieved here:
>> >
>> > http://tools.ietf.org/html/draft-ietf-sipcore-rfc4244bis
>> >
>> > Any comments on the document should be sent to the SIPCORE
>> mailing list.
>> >
>> > /a
>> >
>> > _______________________________________________
>> > sipcore mailing list
>> > [e-mail]
>> > https://www.ietf.org/mailman/listinfo/sipcore
>>
>> _______________________________________________
>> sipcore mailing list
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/sipcore
>>
> _______________________________________________
> sipcore mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/sipcore
>