Re: [sipcore] Alexey Melnikov's Discuss on draft-ietf-sipcore-sip-push-21: (with DISCUSS and COMMENT)

[Christer Holmberg <[e-mail]>]

Hi Alexey,

Thank You for the review! Please see inline.

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

>I am generally excited about addition of push notifications to SIP.
>I have a couple of comments (and a few less serious ones) that I would like to discuss before recommending approval of this document:
>
>10.  pn-provider, pn-param and pn-prid URI Parameters for Apple Push
>     Notification service
>
>   When the Apple Push Notification service (APNs) is used, the PNS-
>   related SIP URI parameters are set as described below.
>
>   The value of the pn-provider URI parameter is "apns".
>
>   Example: pn-provider = apns
>
> Spaces are not allowed in URIs unencoded, so your example is misleading. I suggest you change it to "pn-provider=apns" (i.e. delete space before and after "=").
>
> Similar comment about 2 other parameter examples defined in this section.

Good catch. I will fix it (by deleting the spaces).

---

> 10.  pn-provider, pn-param and pn-prid URI Parameters for Apple Push
>     Notification service
>
>   The value of the pn-param URI parameter is a string that is composed
>   by two values, separated by a period (.): Team ID and Topic.  The
>   Team ID is provided by Apple and is unique to a development team.
>
> I assume it doesn't contain any periods?
>
>   The Topic consists of the Bundle ID, which uniquelly identifies an
>   appliciation, and a service value that identifies a service
>   associated with the application, separated by a period (.).  For VoIP
>   applications the service value is "voip".
>
> How many periods are used in the value? If your example below is correct, can you clarify that Bundle ID itself contains periods?
>
>   Example: pn-param = DEF123GHIJ.com.yourcompany.yourexampleapp.voip

The Bundle ID part of the Topic typically contains periods. The Team ID, and the service part of the Topic, do not contain periods.

Note that the Topic is actually created by Apple, and I don't even know whether the Apple PNS needs to separate the Bundle ID part from the service part. And, as far as I know, we don't need to separate the parts in our proxy implementation. We do separate the Team ID from the Topic, but we know the first period is the separator (as the Team Id does not contain periods).

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

>I am agreeing with Adam's DISCUSS.
>
>In 8.2:
>
>     pns-list        = pns *(COMMA pns)
>
> My understanding is that COMMA (defined in RFC 3261) allows folding whitespace, in particular CRLF. Do you really want this in values? It is Ok if you do, I just wanted to double check.

In the SIP syntax we are less strict than in some other protocols when it comes to folding whitespaces etc. The syntax above is very common in SIP.

---

>8.7.  SIP URI Parameters
>
>     COLON = <COLON defined in [RFC3261]>
>
> COLON is not used in this section or anywhere else in the document.

I have no idea how it got there, because as far as I can remember we haven't used COLON in any version of the draft.

I will remove it.

---

>14.4.1.  sip.pnsreg
>
>      Security considerations: This media feature tag does not introduce
>         new security considerations, as it simply indicates support for
>         a basic SIP feature. If an attacker manages to remove the media
>         feature tag, push notifications towards the client will be
>
> Is "not" missing before "be"?
>
>        requested.

Correct. I will add "not".

Regards,

Christer