Subject: Re: [sipcore] Short WGLC on SIP Push

From: Christer Holmberg <[email-masked]>
Date: Fri, 1 Feb 2019 10:32:07 +0000

I have created a pull request commit based on Ben's comments:

https://github.com/cdh4u/draft-sip-push/pull/35/commits/2a4d771f3e753f043e72cec64afbb2dd7c6de4e3

Regards,

Christer

On 30/01/2019, 17.54, "sipcore on behalf of Christer Holmberg" <[email-masked] on behalf of [email-masked]> wrote:

    Hi Ben,
    
    Thank You for the comments! Please see inline.
        
    > I have reviewed version 24. I mainly reviewed the diff from version 21. For the most part, I am only making substantive 
    > comments in this email. However, I think that the new text needs more wordsmithing and proofreading.
    
    I am fine with that. However, in that case I do expect people to let me know what changes they want. 
    
    ---
    
    >  §4.1.2: In the case where the UA cannot remove its registration, I assume this is all soft-state and 
    >  will get cleaned up when the bindings expire. If correct, it would be worth mentioning that here. 
    >  I agree the UA needs to remove the binding if it can
        
    What about:
    
       "When a UA wants to disable previously requested push notifications,
       the UA SHOULD remove the binding [RFC3261], unless the UA is no
       longer able to perform SIP procedures (e.g., due to a forced shutdown
       of the UA) in which case the registrar will remove the binding once it 
       expires."
    
    ---
    
    > §5.2: Is this really a queue in the formal sense? That is, a given queued request doesn’t have to wait for other 
    > requests that are ahead in the queue, does it?
       
    No.
    
    Please suggest a better word? "Bucket"? :)
    
    ---
    
    >    §5.3:
    >    
    >    I think this section needs to be careful about URI matching for the purposes of generating a push request, 
    >    and URI matching for the purposes of message routing.
    >  
    >   The 3261 URI comparison rules say that a parameter that exists in one URI but not the other is ignored. 
    >   That means a URI with the push-related parameters will match one without them. Is that the intent? For example, 
    >   is an inbound request that has a request-URI that matches the registered contact for the push-enabled UA but does 
    >   not have the push parameters expected to trigger a push request?
    
    You are correct, I missed that part.
    
    So, I guess we have to put back the old text in the first paragraph, together with a clarification that it only applies when comparing a REGISTER contact with a request R-URI:
    
       "By default, a SIP proxy uses the URI comparison rules defined 
       in [RFC3261]. However, when a SIP proxy compares the Contact 
       header field URI of a 2xx response to a REGISTER request with a Request-URI of a SIP
       request in the SIP Request Push Queue (Section 5.2), the proxy uses
       the URI comparison rules with the following additions:
       the pn-prid, pn-provider and pn-param SIP URI parameters MUST also 
       match. If a pn- parameter is present in one of the compared URIs but 
       not in the other URI there is no match.
        
    ---
    
    >    In the second paragraph, do I understand that we are saying that a URI that would not have matched if we weren’t 
    >    using the PNS will now suddenly match for purposes of routing the request to the UA? What do we expect the UAS to 
    >    do with a request that doesn’t match any of its bindings? Is it also allowed to match just on the push parameters? 
    >   (I wonder if there are subtle security considerations here.)
    >    
    > That same race condition can occur in non-push networks, where a UAS changes a binding between the time a UAC sends 
    > an request and the time that request reaches the responsible proxy. We don’t expect that to work. Why do we expect this to 
    > be different for push-enabled networks? Is it because the window for the race condition is larger?  (It can’t be _much_ longer, 
    > or the user experience will be pretty bad.)
    
    Initially the idea was that the proxy would not forward a request with only a "partial" match. But, Adam also commented that 
    the race condition can occur in non-push environments, so a UAS needs to be able to handle it. A UAS can choose to reject such request, 
    but the comment was that the proxy should not make that decision.
    
    Note that function wise this hasn't changed in version 24.
    
    ---
    
    >    §5.6.2:
    >    
    >    - “The proxy checks whether the SIP Request Push queue contains a SIP request associated with the 
    >   REGISTER transaction”: Please be precise about what “associated” means in this context. I initially assumed 
    >   it meant “has an R-URI that matches one of the contacts in the REGISTER request”, but the following step 
    >   checks for that.
    
    There seems to be some mixing of old and new text. I suggest to modify the paragraph:
     
    OLD:
    
         "The proxy checks whether the SIP Request Push Queue contains a SIP
          request associated with the REGISTER transaction.  If the queue
          contains such request the proxy compares (Section 5.3) the Contact
          header field URI in the REGISTER response with the Request-URIs of
          the SIP requests in the queue.  If there is a match, the proxy
          MUST remove the SIP request from the waiting queue and forward it
          towards the UA."
    
    NEW:
    
          The proxy checks whether the SIP Request Push Queue contains a SIP
          request associated with the REGISTER transaction, by comparing (Section 5.3) 
          the Contact header field URI in the REGISTER response with the Request-URIs
          of the SIP requests in the queue. If there is a match, the proxy
          MUST remove the SIP request from the waiting queue and forward it
          towards the UA."
        
    >    - In the paragraph about what happens if the push-triggered REGISTER does not show up in time, it’s 
    >   not clear which transaction timer is being discussed. I think this means some special push-specific timer 
    >  set by the proxy, not the UAC's normal SIP transaction timers? (If we are talking about the UAC’s timer, then 
    >  it seems tardy for the proxy to send an error _after_ the UAC’s timer has expired.) If so, then some text mentioning 
    > the proxy needs to keep such timers in the first place would be helpful.
       
    It is a push-specific timer, and I agree that needs to be clarified.
    
    I suggest to modify the first paragraph of section 5.2:
    
    OLD:
    
      "When a SIP proxy receives a SIP request, addressed towards a UA, that
       will trigger the proxy to request that a push notification is sent to
       the UA, the proxy will place the request in a queue referred to as
       the SIP Request Push Queue.  A SIP request is removed from the queue
       once the proxy either forwards the request towards the UA or when the
       proxy sends an error response to the request.  The detailed
       procedures are described in the sections below."
    
    NEW:
    
       "When a SIP proxy receives a SIP request, addressed towards a UA, that
       will trigger the proxy to request that a push notification is sent to
       the UA, the proxy will place the request in a queue referred to as
       the SIP Request Push Queue.  The proxy starts a timer (referred to as queue 
       timer) associated with the transaction. A SIP request is removed from the queue 
       once the proxy forwards the request towards the UA, when the
       proxy sends an error response to the request, or when the timer times out.  
       The detailed procedures are described in the sections below."
    
    ...and, to modify the paragraph in section 5.6.2:
    
    OLD:
    
      "When the proxy has requested that a push notification is sent to a
       UA, if the proxy does not receive a REGISTER response with a Contact
       URI that matches the Request-URI of the SIP request, the transaction
       timer of the SIP request will eventually time out.  When that happens
       the proxy MUST remove the SIP request from the queue and send a 480
       (Temporarily Unavailable) response.  The timer expiration value is
       set based on local policy, taking the guidelines below into
       consideration."
    
    NEW:
    
      "When the proxy has requested that a push notification is sent to a
       UA, if the proxy does not receive a REGISTER response with a Contact
       URI that matches the Request-URI of the SIP request, the queue
       timer (Section 5.2) associated with the SIP request will eventually time out.  
       When that happens the proxy MUST remove the SIP request from the queue 
       and send a 480 (Temporarily Unavailable) response.  The queue timer expiration 
       value is set based on local policy, taking the guidelines below into consideration."
    
    >    - " If the proxy is able to authorize the sender of the REGISTER…”
    >    Does this mean “authenticate”? If you really mean “authorize”, then “is able to” doesn’t seem to make 
    >    sense. Maybe “If the proxy is responsible for authorizing…”?
    
    My suggestion is to use the original text, but replace "authorize" with "authenticate".
    
    ---
        
    >  §6.2.3: Do I understand correctly that a mid-dialog request with an incorrect R-URI will still work if the PURR matches? 
    > What do we expect the UAS to do with a mid-dialog request with an incorrect r-uri?
       
    Not sure I understand what you mean by "incorrect r-uri".
    
    The r-uri is created using the normal 3261 route set creating rules, and I don't think the proxy needs to do anything in addition to what a non-push SIP proxy would do.
    
    ---
    
    >  §13:
    >  - I suggest removing the language suggesting that malicious push-notifications have no security-related impacts.
    > Running down batteries and triggering extra messaging can have their own security-related impacts.
        
    What about modifying the sentence as:
    
    "Such push notifications will impact the battery life of the UA and will trigger unnecessary SIP traffic."
    
    > - TLS doesn’t, by itself, secure the network from malicious _endpoints_, if you mean endpoint in the sense of a SIP UA. 
    >  That requires authentication and authorization of the UA, which TLS does not do unless you want to require 
    >  mutual-authentication.  I suspect you meant to talk about attacks from other parties?
        
    My understanding from Ekr's comment was that "endpoints" would cover both UAs and network entities. However, I do agree that we could use another word here. What about "malicious network entities"?
    
    Regards,
    
    Christer

    
    _______________________________________________
    sipcore mailing list
    [email-masked]
    https://www.ietf.org/mailman/listinfo/sipcore