Push Notification with the Session Initiation Protocol (SIP)
draft-ietf-sipcore-sip-push-29

I agree with the DISCUSSes, but don't have enough SIP knowledge to add anything.

Thanks for addressing my discuss points. Although I find the solution
for SUBSCRIBE dialogs particularly unsatisifying, I'm not going to get
in the way of working group consensus here. The original discuss points
(and original comments) are detailed below.

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

Thanks to everyone who is put work into defining this mechanism. I think it will
be very useful to have a solution for integrating push mechanisms into SIP
networks. I've identified three issues that I think need to be addressed in the
current document before it can move forward, and a fourth serious flaw that I
call out in my comments below.

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

It's nice that this document has considered the impact of inbound mid-dialog
messages in long-lived dialogs. However, it appears to have a major hole in it:
handing of outbound messages for the purpose of maintaining soft-state in those
dialogs isn't handled correctly.

In particular: networks that deploy this mechanism will cause SUBSCRIBE
dialogs to time out and be destroyed while they are still in use.
Additionally, if RFC 4028 (session timers) is negotiated, then INVITE dialogs
will suffer the same fate.

I can think of a couple of ways for these situations to be handled, but they
need explicit text in the document.

One approach would be to specify that the User Agent selects its requested
"Expires" value in its registration to be the smallest value before its set of
subscriptions and session timers needs to be refreshed. This would cause a push
notification to happen to prevent registration timeout, and the client could
refresh the other soft state at that time. Complications arise if the registrar
responds with a 483 (Interval too Brief), and we'd need to find a solution for
that.

Another approach would be for the clients to refresh all soft state whenever
they send a registration, and set the timeout for that soft state to be equal to
or greater than the registration timeout. A complication could arise if the
notifier or the peer in an invite dialog shortens the requested time, and we'd
need to find a solution for that.

A third approach would be getting the proxy involved in some way -- either by
requiring it to observe subscription and session timer timeouts and requiring it
to send push notifications prior to their expiration, or by an explicit
communication between the UA and the proxy that indicates when the next push
notification should be scheduled. If the latter approach is taken, I would
suggest that it needs to be taken for REGISTER messages as well.

I really don't think this mechanism is feasibly deployable without a solution to
this problem.

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

§4.1:

>  For privacy and security reasons, the UA MUST NOT include the SIP URI
>  parameters defined in this document in non-REGISTER request, to
>  prevent the PNS information associated with the UA from reaching the
>  remote peer.

This seems to ignore the availability of Contact URI parameters via RFC 3680
subscriptions. This would seem to impose a requirement on Registrars to strip
this information before making it available to any other party (which, in
turn, requires that the system have explicit Registrar *and* Proxy support).
As far as I can tell, the system so far has not required explicit proxy support
(and there's certainly no mechanism built-in that ensures that a proxy has any
special handling regarding this mechanism).

If the PNS information is sensitive, and cannot be allowed to leak out to
other users, then we need some way to ensure the registrar has provided
positive confirmation that it will not do so before allowing it to come into
possession of them.

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

§4.2:

>  The UA can do so by only including the pn-provider
>  SIP URI parameter in the SIP Contact header field URI of the REGISTER
>  request, but without including the pn-prid SIP URI parameter.

Unless I'm mistaken, this is a barrier to interoperation.

It's not 100% clear, but I suspect the intended implication is that the
pn-provider parameter here will contain a single value? The syntax of the
parameter certainly seems to imply that. This seems to be a pretty big
problem, since it presupposes that the client will know which PNSes the Proxy
supports.  Consider, for example, an iOS client that can use any of RFC 8030,
FCM, and APN (cf https://firebase.google.com/docs/cloud-messaging/ios/client).
If the client doesn't know a priori what the proxy supports (and this entire
section only makes sense if that's true), then it won't know which of those
three services to indicate in its REGISTER request. If it guesses wrong, this
mechanism simply fails.

I think you need a different discovery mechanism here -- either have one that
has the client offering multiple PNS protocols and the proxy responding with
one, or have one in which the proxy indicates all of its supported services in
a response, and the client chooses one to use in its next REGISTER message.

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

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.

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

§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.

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

§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"

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.

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

§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?

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.

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

§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..."

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

§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."

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

§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..."

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

§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.

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

§5.3.2:

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

Same comment as above.

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

§6.1.1:

>  When the UA sends in initial request for a dialog, or a 2xx response

Nit: "...sends an initial request..."

>  purr' SIP URI parameter in the Contact header of the request or

Nit: "...Contact header field..."

>  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..."

>  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..."

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

§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.

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


§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.

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

§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?

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

§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.

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

§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..."

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

§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..."

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

§§10-12:

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

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

§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"

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

§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

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

§11:

>  [RFC8292] defines a mechanism which allows a proxy to identity itself

Nit: "...mechanism that allows..."

Thank you for addressing my DISCUSSes and comments.

Section 13: "MUST ... unless" is a construct worth avoiding.

Thank you for addressing my DISCUSS and the extensive editorial rework.

One question on the new PNS registry: 
I don't quite understand why the policy is "Specification Required". I guess you need some description of the values for pn-provider, pn-param and pn-prid. However, you could just add these fields to the registry directly (with the option to just provide a reference in at least the param and prid fields if the description is rather extensive). However, for e.g. RFC8030 is would easily be possible to describe everyhing you need within the registry. For apns and fcm you can simply add a pointer to this new RFC if you think the description is too long to be included in the registry directly. But this could make registration for new simple services easier.

And one minor comment on some language:
Should these sentence in 3.1  and 5.2 maybe be normative as well:
"If a SIP UA
   receives a push notification that contains a payload the UA can
   discard the payload, but the UA will still send a binding-refresh
   REGISTER request."
e.g NEW
"If a SIP UA
   receives a push notification that contains a payload the UA can
   discard the payload, but the UA MUST still send a binding-refresh
   REGISTER request."

"A proxy
   will not request push notifications towards a UA that has not
   provided a pn-prid SIP URI parameter."
e.g. NEW
"A proxy
   MUST NOT request push notifications towards a UA that has not
   provided a pn-prid SIP URI parameter."

It seems like these sentences are on purpose not normative (maybe because as they are part of a note...?), however, I think it would be more clear to state those requirements normatively as well. Otherwise if you think these requirement are described normatively somewhere else in the doc, I think a forward reference would be good (I didn't double-check myself).

nits: 
- maybe s/consider the interval too small/consider the interval too short/
- maybe s/response Section 8.1/response (see Section 8.1)/
- s/proxy receives a receives a SIP/proxy receives a SIP/