Subject: [sipcore] Minor comments on draft-ietf-sipcore-sip-push-06

From: Yehoshua Gev <[email-masked]>
Date: Mon, 26 Feb 2018 11:03:41 +0200

Hi,

Some minor comments:

1. First paragraph of section 4:
    SIP URI parameter in the SIP Contact
  Should be:
    SIP URI parameters in the SIP Contact

2. 4th paragraph of section 5.3.1:
  I believe the 'sip.pns' should indicate the PNS of the pn-provider.

3. 4th paragraph of section 5.3.2:
  I think a timeout should be defined for the case the re-registration
doesn't occur.

4. Section 7.2:
  "The sip.pns feature-capability indicator is used in a SIP request" - can
it appear in any other request except REGISTER?

5. Section 12.3.1:
  I'm not sure how detailed the description should be, but currently it
lacks the possibility of sip.pns appearing on the REGISTER request.

Thanks,
Yehoshua

---

From: Christer Holmberg <[email-masked]>
Date: Mon, 26 Feb 2018 10:46:47 +0000

Hi Yehoshua,

Thank You for your comments! Please see inline.

>    SIP URI parameters in the SIP Contact

Are you sure? Note that there is an “A” in front of each parameter.

>  I believe the 'sip.pns' should indicate the PNS of the pn-provider.

The purpose is only to indicate to other proxies that the proxy will trigger push notifications.

>  I think a timeout should be defined for the case the re-registration doesn't occur.

Not sure I understood your comment (are you sure you used the correct paragraph/section?), but if the re-registration doesn’t occur then the registration will eventually expire.

>4. Section 7.2:
>  "The sip.pns feature-capability indicator is used in a SIP request" - can it appear in any other request except REGISTER?

The usage and semantics are only defined for REGISTER. If you want to use it with other SIP methods, you would have to specify such usage.

>5. Section 12.3.1:
>  I'm not sure how detailed the description should be, but currently it lacks the possibility of sip.pns appearing on the REGISTER request.

Good catch! I will fix that.

Regards,

Christer

---

From: Yehoshua Gev <[email-masked]>
Date: Mon, 26 Feb 2018 17:10:11 +0200

See Inline.

Thanks.

> Are you sure? Note that there is an “A” in front of each parameter.

I'm not sure :-), IIRC the plural is omitted only if hyphens are used, like
"a pn-prid- and pn-param-SIP-URI-parameter".
Maybe the sentence can be rephrased to:
 "The UA MUST include the following SIP URI parameters in the SIP Contact
header field URI of the REGISTER request: pn-provider, pn-prid and pn-param
(if required for the specific PNS)."

> trigger push notifications.

I understand, but the proposed ABNF for sip.pns requires a value.

> Not sure I understood your comment (are you sure you used the correct
> paragraph/section?), but if the re-registration doesn’t occur then the
> registration will eventually expire.
>

I meant that that paragraph handles the case of a re-registration that
fails with non-2xx response, but doesn't handle a case that the
re-registration never begins (or at least the proxy doesn't receive it).
For handling this case, I suggest adding a timeout (maybe with
implementation-specific time), and if it expires reject the request with
404.

> >4. Section 7.2:
> >  "The sip.pns feature-capability indicator is used in a SIP request" -
> can it appear in any other request except REGISTER?
>
> The usage and semantics are only defined for REGISTER. If you want to use
> it with other SIP methods, you would have to specify such usage.
>
> >5. Section 12.3.1:

---

From: Christer Holmberg <[email-masked]>
Date: Mon, 26 Feb 2018 16:57:01 +0000

Hi,

From: Yehoshua Gev [mailto:[email-masked]] 
Sent: 26 February 2018 17:10
To: Christer Holmberg <[email-masked]>
Cc: sipcore <[email-masked]>
Subject: Re: [sipcore] Minor comments on draft-ietf-sipcore-sip-push-06

See Inline.

Thanks.

Hi Yehoshua,

> I'm not sure :-), IIRC the plural is omitted only if hyphens are used, like "a pn-prid- and pn-param-SIP-URI-parameter".
> "The UA MUST include the following SIP URI parameters in the SIP Contact header field URI of the REGISTER request: 
> pn-provider, pn-prid and pn-param (if required for the specific PNS)."

If people think that's better I have no problem to change it.

> I understand, but the proposed ABNF for sip.pns requires a value.
 
Aaah... true. I guess it doesn't hurt if the proxy indicates the PNS.
 
>> Not sure I understood your comment (are you sure you used the correct paragraph/section?), but 
>>if the re-registration doesn’t occur then the registration will eventually expire.
>
> I meant that that paragraph handles the case of a re-registration that fails with non-2xx response, but 
> doesn't handle a case that the re-registration never begins (or at least the proxy doesn't receive it).
> For handling this case, I suggest adding a timeout (maybe with implementation-specific time), and if it expires reject the request with 404.

Aaah... I get it. There is a request to forward, but the re-registration REGISTER request never arrives. I will add text about that.

Regards,

Christer

---

From: Christer Holmberg <[email-masked]>
Date: Mon, 26 Feb 2018 19:29:13 +0000

Hi,

...

>>>paragraph/section?), but if the re-registration doesn’t occur then the registration will eventually expire.
>>
>> fails with non-2xx response, but doesn't handle a case that the re-registration never begins (or at least the proxy doesn't receive it).

Something like this:

"If the proxy doesn't receive the REGISTER request within a given time, it MUST reject the request with a 404 (Not Found) response. The time value is set based on local policy."

Regards,

Christer

---

From: Paul Kyzivat <[email-masked]>
Date: Mon, 26 Feb 2018 14:36:25 -0500

> "If the proxy doesn't receive the REGISTER request within a given time, it MUST reject the request with a 404 (Not Found) response. The time value is set based on local policy."

Why not go ahead and forward the request? It will probably time out, but 
there is an off chance that it might work.

	Thanks,
	Paul

---

From: Roman Shpount <[email-masked]>
Date: Mon, 26 Feb 2018 14:39:25 -0500

On Mon, Feb 26, 2018 at 2:36 PM, Paul Kyzivat <[email-masked]> wrote:

>>> Aaah... I get it. There is a request to forward, but the re-registration
>>> REGISTER request never arrives. I will add text about that.
>>>
>>
>> Something like this:
>>
>> "If the proxy doesn't receive the REGISTER request within a given time,
>> it MUST reject the request with a 404 (Not Found) response. The time value
>> is set based on local policy."
>>
>

In the modern deployment (SIPS with SIP Outbound) there is nowhere to
forward the request. Request can only be forwarded if there is an existing
TLS connection from the client to the proxy. This connection is typically
closed when client is suspended and is waiting for the SIP Push.

Regards,
_____________
Roman Shpount

---

From: Christer Holmberg <[email-masked]>
Date: Mon, 26 Feb 2018 20:03:47 +0000

Hi Paul,

What you suggest is already allowed, as described in the last section of 5.3.2. But, that only applies in special cases, where the proxy has “knowledge”.

For the general case, since we specify that the proxy has to wait for the REGISTER request, AND the associated response, I think we should stick to that.

Regards,

Christer

From: sipcore [mailto:[email-masked]] On Behalf Of Roman Shpount
Sent: 26 February 2018 21:39
To: Paul Kyzivat <[email-masked]>
Cc: SIPCORE <[email-masked]>
Subject: Re: [sipcore] Minor comments on draft-ietf-sipcore-sip-push-06

On Mon, Feb 26, 2018 at 2:36 PM, Paul Kyzivat <[email-masked]<mailto:[email-masked]>> wrote:
On 2/26/18 2:29 PM, Christer Holmberg wrote:
Hi,

...

Something like this:

Why not go ahead and forward the request? It will probably time out, but there is an off chance that it might work.

In the modern deployment (SIPS with SIP Outbound) there is nowhere to forward the request. Request can only be forwarded if there is an existing TLS connection from the client to the proxy. This connection is typically closed when client is suspended and is waiting for the SIP Push.

Regards,
_____________
Roman Shpount