Re: [sipcore] AD Evaluation of draft-ietf-sipcore-sip-push-20 - Ben's technical comments

[Christer Holmberg <[e-mail]>]

Hi,

Thanks for the comments! Please see inline.
    
>    §1, paragraph 5: "Note that this mechanism necessarily adds delay to
>    responding to non-INVITE requests requiring push notification."
>    
>    Technically, it adds delay for any transaction. Non-INVITE transactions are just more delay sensitive.

I suggest to say: "adds delay to responding to requests".

---
    
> §3,
>    - first paragraph:
>    Please state this in terms of a scope assumption, not a global statement. That is, limit the scope of this 
>    document is limited to PNSs that work this way, rather than state that all PNSs work this way.
    
I suggest to say: "The mechanism defined in this document assumes that, when a SIP UA registers with a PNS, it..."

>    - 2nd paragraph: I think it's safe to say "The format of the PRID varies..." rather than "... may vary...".

I will fix as suggested.

---
    
> §4.1
>    
>    - General: When we speak of multiple bindings, and doing something to all of those bindings (e.g. refreshing 
> them on a push-notification), is it allowable for a UA to have some bindings that use a PNS and some that do not? 
> Or perhaps some that use different PNSs? I realize that may not make sense for the mobile device use case that 
> seems centric to this draft, but I can imagine a situation where a complex UA might have different PNS configurations f
> or different network paths and/or different SIP services.
>   
> If you want to assume that the PNS configuration is the same for all contacts, please say so explicitly. If not, does it 
> make sense to scope PNS-triggered refreshes to just those bindings that use the same PNS configuration?
   
I don't think there is any technical reason to mandate usage of the same PNS (or using PNS to begin with) for all contacts, so I don't think we need to forbid it.

We could say something like: "The procedures in this section apply to bindings associated with a given PNS."

I don't think we need to say more than that.
 
>    -2nd paragraph:
>    I don't think we can assume that all UAs will always learn all the bindings they need to to setup at the same time. 
>   For example, a UA might have changes in network configuration that require them to add or remove bindings. This 
>   makes it impractical to register them at the same time and always use the same expirations, unless we ask them to 
>   refresh all existing bindings whenever they add a new one. (Or perhaps ask them to set the expiration of any new 
>  binding to match the remaining time for existing ones, but that seems like a more difficult approach.)
    
I don't think the text assumes that the UA is aware of all bindings at the same time - it only says that it should use the same 'expires' value for each binding.

>-6th paragraph: "REGISTER request will create NAT bindings..."
>    
>    If we talk about creating NAT bindings as one of the purposes of the push-triggered REGISTER, we may also need to 
>    think about the lifetime of those nat bindings, and how that interacts with the REGISTER expires value. I'm not sure we 
>   want to go there; would it make sense to remove the mention?
    
I am not sure that is needed in the context of PUSH, since the NAT bindings only need to be alive in time for the request that triggered the push notification to reach the UA. Next time there is an inbound SIP request, the REGISTER triggered by the push notification associated with the request will create a NAT binding (if the previous one has expired) for that request.

>    - 7th paragraph:
>    --  The first sentence seems to still assume the REGISTER has as single contact. Should it say to insert the tag into each 
> contact header field? (but see general §4.1 comment above.)
   
What sentence are you referring to?

 >   -- "if the response to the REGISTER request contains a ’sip.pnsreg’
 >   feature-capability indicator with an indicator value, the UA MUST
 >   send REGISTER requests prior to the registration expires."
 >   
 >   This seems to duplicate a MUST from normal SIP. Would it make sense to just say "... the UA refreshes the binding 
>    according to normal SIP procedures".
    
I will fix as suggested.

---

>    §5.3.1:
>    - 4th paragraph: Section 4.1 talks about the UA paying attention to sip.pns in a 2XX response. It does not mention 
>   it for other result codes. If the UA needs to pay attention to it in 423 (or in any other response), it should be mentioned there.
   
There are no push-specific UA actions for non-2xx responses.

If the UA receives a 423, it follows normal SIP procedures. Even if the proxy has inserted 'sip.pns' feature-capability indicator in the 423 response it only provides information about the PNS(s) supported by the proxy. It does not mandate push-specific UA actions.

>    -7th paragraph, "the proxy SHOULD insert a Feature-Caps header
>    field with a ’sip.pns’ feature-capability indicator": What if the response already contains it?

The text covers the case where the proxy generates the 423 response.
  
---

> §5.3.2, 2nd paragraph: Please mention that the R-URI will only contain these parameters after it is retargeted. This mostly 
> matters if the push-proxy is colocated with a retargeting proxy; retargeting must happen before this procedure.

Section 1 says:

   "The proxy MUST be in the signalling path of REGISTER requests sent by
   the UA towards the registrar, and of SIP requests (for a new dialog
   or a stand-alone) forwarded by the proxy responsible for the UA's
   domain (sometimes referred to as home proxy, S-CSCF, etc) towards the
   UA."

> Along those lines, what is the PUSH proxy expected to do with inbound SIP requests that do not contain the parameters 
> in the R-URI? Route them normally? Reject them?

I think that is an implementation issue. If the proxy also handles non-PNS calls, it would forward the request using normal procedures.

---

>    §6: Is support for push-notifications on mid-dialog requests optional? If so, please state that up front.

If one wants to support longlived SIP dialogs, one obviously will have to implement section 6. I am not sure we need to say something.
    
---

>    §6.1.1: Does the UA indicate support on a per-dialog basis? That is, it can support the mechanism for some dialogs but not others?

It could be per-dialog basis. The text says: "if the UA is willing to receive push notifications triggered by incoming mid-dialog requests".
    
---

>    §8.1, 2nd paragraph: Should that say "only defined for ... responses", or "... and is not defined for use in request messages."?
 
I suggest to say "only defined for".

---
   
>    §9, 1st paragraph: We are talking about a specification that defines the parameter usage for the given PNS, not the PNS in it's entirety, right?

Yes. I suggest to say: "defines the usage of the associated PNS". Because, the parameter itself only identifies the PNS.

---

>    §13, 3rd paragraph: The paragraph as written basically says "Operators must ensure SIP signaling is secured unless they are 
>    sure it's secured", which is circular logic. Would it make sense to say they must use TLS (or SIPS) unless they are sure that 
>    it is protected by other mechanisms?

Sounds good. I will fix as suggested.    

---

> §16.2: I think the reference to RFC 3891 should be normative.

I will make it normative.


I will address the editorial comments in a separate e-mail.

Regards,

Christer