Subject: [sipcore] Review of sip-push-10

From: Martin Thomson <[email-masked]>
Date: Thu, 29 Mar 2018 17:17:22 +1100

I really don't know what to make of this draft.  On the one hand, it's
clearly a sensible addition to the protocol based on the current
environment.  But then it's also clearly doing nothing but further
entrenching the two dominant proprietary push service platforms.

I also found the prose a little hard to follow.  Some restructuring
might go a long way to improving clarity.

The description of RFC 8292 integration isn't particularly helpful.
Does the UA register once, discover that the Proxy/Registrar supports
vapid, then re-register immediately?  None of that is well-described.

The document talks in passing about its assumptions about the push
message, but never actually states the requirements on that message.
Please add a clear description of what the message is.  For instance,
is any message sufficient?  Or does the message have to be empty?
What would it mean to receive a message with a payload?

An important limitation of APNS and FCM here is that a single app
can't hold multiple contexts, therefore any message it receives will
cause re-registration even if the message was for other purposes
("Scarves, 5% off until Monday!", bam).  You should document that.

Section 4:
Can a UA include Feature-Caps on its own?  Maybe it doesn't want to do
all the push service stuff until it knows that the server supports the
features it needs (vapid for example).

Section 5.3.1:
I don't understand the relevance of all the 423-related text here.
Maybe it's a lack of clarity about the expected processing model.
FWIW, the only thing of value in this very long paragraph is that
sip.pns is not to be included with a 423.

However, I would argue the opposite and that sip.pns is most valuable
in a 423.  For example, a UA knows that it only stays alive for a
short time, so it conservatively creates registrations with a short
duration.  That's not very useful, but the UA might expect to only be
available while the app is in the foreground where it is able to
refresh frequently.  However, if it receives a signal from the
proxy/registrar that indicates push support (in a 423 perhaps), it
might be inspired to extend the registration and rely on push.

How would a proxy decide to send a 555 response or ignore the push
information in the REGISTER request?

Why would a proxy add Feature-Caps instead of just stripping out the
push parameters?  (That's probably a consequence of my ignorance of
REGISTER procedures, but I am curious.)  Why does the Feature-Caps
need a value?  The downstream proxy only needs to know that push is
taken care of, not which push is in use.  Is it just for symmetry with
the downstream notification?  What value does the parameter take?  The
pn-provider label?

Section 5.3.2:
Why include so much text here?  If the proxy considers the UA to be
registered - and only challenged with respect to active connectivity -
then normal SIP processing applies with only one tweak: send a push
message and wait until the registration is restored.

The idea that the proxy might be able to correlate a REGISTER request
that includes a different contact URI with this new transaction/dialog
is a little baffling - either the registration is restored with the
right contact, or the registration doesn't happen and message delivery
will ultimately time out and fail.  In this regard, the only text you
need is the timeout text you have near the end of this section.

Section 6 contains no information.  If anything, you can use NAT as
further justification for use of a push notification service, but it
doesn't warrant an entire section.

Section 7.5:
I don't understand the purpose behind sip.pnsreg is defined here.  How
is this different than, say, the usual registration expiration logic?
Maybe this goes back to the basic registration logic.

Nits:
Section 7.6: typo: indciates

Section 11: "When Firebase..." seems to be copy-pasta
s/push URI/push subscription URI/
I don't think that you need to include a URI for RFC 8030.