Subject: Re: [sipcore] Adam Roach's Discuss on draft-ietf-sipcore-sip-push-21: (with DISCUSS and COMMENT) - the COMMENT issues

From: Christer Holmberg <[email-masked]>
Date: Wed, 9 Jan 2019 18:49:01 +0000

Hi,

In this reply I will address the COMMENT issues.

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

>Figure 1:
>
> This diagram includes both a ladder diagram and an example REGISTER message.
> While both of these are useful at this point in the document, neither of them is an "Architecture," and they're really two different things. 
> I suggest breaking this into two Figures, entitled "Typical Information Flow" and "Example REGISTER Message" (or similar), respectively.

I can do that, but one of the WGLC/AD comments said that it should be clear that the Example REGISTER message shows an example of the REGISTER message in the flow. 

---------------------------------------------------------------------------

§4.1:

>>  As long as the UA wants to receive push notifications (requested by  
>> the proxy), the UA MUST include a pn-provider, pn-prid and a pn-param  
>> (if required for the specific PNS provider) SIP URI parameter in each  
>> binding-refresh REGISTER request.
>
> Please be clear that these parameters appear in the Contact URI.

I will clarify that.

---------------------------------------------------------------------------

§5.2:

>>  In order to request push notifications towards a SIP UA that will  
>> trigger the UA to send binding-refresh SIP REGISTER requests, the SIP  
>> proxy needs to have information about when a registration will  
>> expire.  The proxy needs to be able to retrieve the information from  
>> the registrar using some mechanism.  Such mechanisms are outside the  
>> scope of this document, but could be implemented e.g., using the  
>> SIPregistration event package mechanism [RFC3680].
>
> Nit: "SIP registration"

Will fix.

> This mechanism seems to be predicated on an architecture in which the proxy must be on the traffic path. Although it's problematic 
> from a "what proxies are expected to do" perspective, it seems like the proxy could glean this information from the 200 response to 
> the REGISTER request. (I mean, the proxy is already reading parameters out of the contact header field, so the behavior of acting on 
> registration information is already assumed). The proxy would, of course, need to run its own timers, but that seems to be a pretty 
> minor requirement, given everything else involved here.

One way to implement the "retrieve the information from the registrar" could of course be using own timers. But, I don't think we should specify such behavior.

Also, in some cases the proxy might be co-located with the "home proxy", or some other network node,  where the interface with the registrar already exists.

---------------------------------------------------------------------------

§5.3.1:

> This is a major comment, and one that has a sufficient impact on interop that I had a long debate with myself about whether it should be a DISCUSS. Please take it very seriously.
>
>  Otherwise, if the pn-provider SIP URI parameter identifies a type of  
> PNS that the proxy does not support, or if the REGISTER request does  
> not contain all additional information required for the specific type  
> of PNS, the proxy MUST either forward the request (e.g., if the proxy  
> knows that another proxy that will receive the REGISTER request  
> supports the type of PNS)...
>
> How would the proxy know this?

"Local configuration"

> Features like suppressing PNS behavior when a "sip.pns" feature capability is present in the REGISTER imply that the various nodes 
> in the network discover capabilities of their neighbors automatically (which is consistent with the way SIP does features), while this 
> provision seems to require provisioned knowledge. This is going to be operationally very confusing.
>
> At the very least, the document needs to call out how this "knowledge" is obtained. In the more general sense, since a client cannot 
> rely on the proxy supporting *any* kind of PNS, the use of 555 in the way specified here is at best an optimization -- and, since the configured 
> information can conflict with actual reality, it's an optimization that can actually break things unless extreme care is exercised. (e.g., if the proxy 
> is configured to "know" that the next proxy cannot provide push service, but this knowledge is wrong, then the network is unnecessarily broken.)
>
> My rather strong recommendation is to remove this code, and let the client rely on the lack of indication of support in the response indicate that the feature is not supported.

I would like to keep the code, because there are cases where one wants the register to be rejected (rather than accepted, but without indication of supported PNS) if the PNS is not supported.

Also, there are already deployments of 555, so I'd prefer to keep it, as it doesn't break anything.

Regarding a proxy knowing what other proxies support, I agree it could be clarified. We could even make it more clear that sending of 555 the default behavior. Something like:

"...the proxy SHOULD send a SIP 555 (Push Notification Service Not Supported)
response to the REGISTER request, unless the proxy knows (based on local configuration)
that another proxy that will receive the REGISTER request supports the type of PNS, 
in which case it MAY forward the request." 

I would even be ok to remove the text about the proxy knowing. Because, as I indicated in my reply to Paul, I don't think there will be multiple proxies in the path handling push (if there are, the REGISTER can be forwarded as an implementation feature).

---------------------------------------------------------------------------

§5.3.1:

>  o  if the proxy received a 'sip.pnsreg' media feature tag in the
>     REGISTER request, the proxy SHOULD include a 'sip.pnsreg' feature-
>     capability indicator with an indicator value bigger than 120 in
>     the response, unless the proxy always want to request push
>
> Nit: "...wants..."

Will fix.

---------------------------------------------------------------------------

§5.3.2:

>  If the contact of the REGISTER request does not match  the 
> Request-URI of the SIP request to be forwarded, or if the contact  was 
> not present in the REGISTER response, the proxy MUST reject the  SIP 
> request with a 404 (Not Found) response.
>
> This seems to be a very strong requirement ("MUST") when, e.g., many or most networks may want to return a 480 instead of a 
> 404. I think what you want to say is something like "...MUST reject the SIP request. Response codes of either
> 404 (Not Found) or 480 (Temporarily Unavailable) are RECOMMENDED, but other appropriate codes may be used as well."

I will modify as suggested.

---------------------------------------------------------------------------

§5.3.2:

>  The reason the proxy needs to wait for the REGISTER response before  
> forwarding the SIP request is to make sure that the REGISTER request  
> has been accepted by the registrar, and that the UA which initiated
>
> Nit: "...the UA that initiated..."

Will fix.

---------------------------------------------------------------------------

§5.3.2:

>  In case of non-2xx response to the REGISTER request, the proxy MUST  
> reject the SIP request with a 404 (Not Found) response.
>
>  If the push notification request fails (see PNS-specific  
> documentation for details), the proxy MUST reject the SIP request  
> with a 480 (Temporarily Unavailable) response.
>
>  If the proxy does not receive the REGISTER request from the UA within  
> a given time after the proxy has requested the push notification, the  
> proxy MUST reject the request with a 480 (Temporarily Unavailable)  
> response.  The time value is set based on local policy.
>
> As above, this seems way too restrictive in terms of mandating specific error codes. 
> It may well be the case that a network would want to treat these situations identically from 
> the perspective of the calling party.

I will do the same modification as earlier.

---------------------------------------------------------------------------

§5.3.2:

>  Otherwise the
>  proxy MUST reject the SIP request with a 480 (Temporarily
>  Unavailable) response.
>
> Same comment as above.

Same fix.

---------------------------------------------------------------------------

§6.1.1:

>  When the UA sends in initial request for a dialog, or a 2xx response
>
> Nit: "...sends an initial request..."

Will fix.

>  purr' SIP URI parameter in the Contact header of the request or
>
> Nit: "...Contact header field..."

Will fix.

>  NOTE: As the 'pn-purr' SIP URI parameter only applies to a give  
> dialog, the UA needs to include a 'pn-purr' parameter in the Contact
>
> Nit: "...given dialog..."

Will fix.

>  dialog, the UA needs to include a 'pn-purr' parameter in the Contact  
> header of the request or response for each dialog in which the UA is
>
> Nit: "...Contact header field..."

Will fix.

---------------------------------------------------------------------------

§6.1.1:

>  The UA MUST include a parameter value identical to the the  last 
> 'sip.pnspurr' feature-capability indicator that it received in a  
> REGISTER response.
>
> This is incredibly confusing, as the "sip.pnspurr" indicator has not been mentioned in the document so far. Please revise as something along the lines of:
>
>  The UA MUST include a parameter value identical to the the last  
> 'sip.pnspurr' feature-capability indicator (see Section 6.2.1) that it  
> received in a REGISTER response.
>
> Alternately, reverse the order of sections 6.1 and 6.2 so that the PURR concept is properly introduced before protocol procedures are defined for it.

I will take a look.

---------------------------------------------------------------------------

§6.1.1:

>  NOTE: As the 'pn-purr' SIP URI parameter only applies to a give  
> dialog, the UA needs to include a 'pn-purr' parameter in the Contact  
> header of the request or response for each dialog in which the UA is  
> willing to receive push notifications triggered by incoming mid-  
> dialog requests.
>
> Similar to the above, this is a very confusing introduction of the "pn-purr" URI parameter.

Same fix.

---------------------------------------------------------------------------

§6.2.2:

>  Contact header field with a PURR value that the proxy has generated  
> Section 6.2.2, the proxy MUST add a Record-Route header to the  
> request, to insert itself in the dialog route [RFC3261].
>
> This "Section 6.2.2" makes the sentence impossible to read. Is it intended to be in parentheses?

Correct. Will fix.

---------------------------------------------------------------------------

§6.2.3:

>  As described in Section 5.3.2, while waiting for the push  
> notification request to succeed, and the associated REGISTER request  
> to arrive from the SIP UA, the proxy needs to take into consideration  
> that the transaction associated with the NOTIFY request will  
> eventually time out at the sender of the request (UAC), and the  
> sender will consider the transaction a failure.
>
> I think this needs some discussion about the impact of the error codes selected by the proxy on the dialog and its usages. For example:
>
> * If the proxy sends a 404 to prevent a transaction timeout, it will destroy the
>  dialog and all of its usages
>
> * If the proxy sends a 480 to prevent a transaction timeout, it will destroy the
>  usage, but not other usages in the dialog (if any)
>
> * If the proxy sends a 504 to prevent a transaction timeout, it will keep the
>  dialog and the usage alive, and allow the client to re-try at a later time.
>  Simply allowing the transaction to time out will have the same effect, albeit
>  with more message retransmissions.
>
> Pointing to RFC 5057 might be useful here.

Perhaps saying something like:

"When the proxy rejects a mid-dialog request, it needs to select a response code that
only impacts the transaction associated with the request. See [RFC50579] for more
information."
 
---------------------------------------------------------------------------

§8.7:

>    Parameter value characters that are not part of pvalue needs to be
>    escaped, as defined in RFC 3261.
>
> Nit: "...need to be escaped..."

Will fix.

---------------------------------------------------------------------------

§9:

>  When a new value is registered to the PNS Sub-registry, a reference  
> to a specification which describes the usage of the PNS associated
>
> Nit: "...specification that describes..."

Will fix.

---------------------------------------------------------------------------

§§10-12:

> Nit: It seems like these should all be subsections of section 9.

The idea was that the PNS registrations are in separate main sections, as they could even be in a separate document.

---------------------------------------------------------------------------

§10:

>  The Topic consists of the Bundle ID, which uniquelly identifies an  
> appliciation, and a service value that identifies a service
>
>Nit: "uniquely"
> Nit: "application"

Will fix.

---------------------------------------------------------------------------

§10:

>  Example: pn-param = DEF123GHIJ.com.yourcompany.yourexampleapp.voip
>
> Please use an RFC 2026 domain for this example; e.g.:
>
>  Example: pn-param = DEF123GHIJ.com.example.yourexampleapp.voip

Will fix as suggested.

---------------------------------------------------------------------------

§11:

>  [RFC8292] defines a mechanism which allows a proxy to identity itself
>
> Nit: "...mechanism that allows..."

Will fix.

Regards,

Christer

---

From: Christer Holmberg <[email-masked]>
Date: Thu, 10 Jan 2019 14:15:59 +0000

Hi,

Based on Adam's COMMENTs, I have updated the pull request used for the gen-art/sec-dir/ops-dir/IESG reviews.

The commit can be found at:

https://github.com/cdh4u/draft-sip-push/pull/31/commits/4392202c287bc2b24a31c99de819aa050fcdd6b0

Regards,

Christer