Subject: Re: [sipcore] Comments on draft-ietf-sipcore-keep-05

From: Christer Holmberg <[email-masked]>
Date: Wed, 13 Oct 2010 15:50:40 +0200

Hi,

I was informed that I hadn't address how/if Hadriel's comments have been addressed in the -06 version of the draft. 

-----------

>Technical:
>1) What happens if the two nodes indicate keepalives, but the 
>downstream doesn't receive them?  In Outbound, it would cause 
>a flow failure for the registration.  My guess is vendors 
>will do the same for draft-keep. (probably even for 
>dialog-based keepalives)  So to me that means (a) the 
>indicated timer value is a maximum time which the sender must 
>send them in, not just a "recommendation", and (b) failure to 
>receive a response to the keepalive should trigger a 
>re-Register, which needs to be indicated in text in the draft.

No changes were done in -06 due to this.

The draft uses the same terminology as the Outbound RFC, which talks about a recommended value.

Also, according to the procedures, a SIP entity must send keep-alives *at least* as often as the indicated recommended keep-alive frequency.

In addition, the draft does not mandate procedures if keep-alives are not received.

-----------

>2) Just to be clear, even if there are multiple Registrations 
>or dialogs on a given connection, only one keepalive 
>"instance" would be active on that connection, right?  I.e., 
>we're not talking about sending multiple keepalives, one per 
>dialog/registration, on the same connection.  So something 
>like this should be added to section 4.1:
>"Even though the keepalive negotiation is performed on a 
>registration or dialog basis, the keepalive mechanism is per 
>transport connection, meaning only one keepalive is sent from 
>the upstream to the downstream entity per TCP connection or 
>UDP port pairs, even if there are multiple dialogs and 
>registrations across them.  Since keepalive is negotiated in 
>each REGISTER or dialog transaction, the most recently 
>negotiated settings MUST be used for the whole connection."

No changes were done in -06 due to this.

Implementations can of course make sure that keep-alives are not negotiated for registrations/dialogs sharing the same connection, but in order to keep the mechanism itself simple the draft does not mandate entities to make sure that only one "set" of keep-alives are sent per connection.  

-----------

>There is, unfortunately, a complication around this stuff: a 
>proxy can be stateful for some calls and stateless for 
>others.  This means you can send a INVITE request downstream 
>and be told to do keepalives, while your next INVITE to it 
>does not, but you still need to do keepalives.  I'm not sure 
>how to handle this in text.
> 
> 
>3) Section 4.2.2 says:
>   Usage of keep-alives
>   can be negotiated when the registration is established, or later
>   during the lifetime of the registration.
> 
>I don't believe it's technically possible to re-negotiate it 
>during the lifetime, because the "lifetime" is the expiry 
>value of the Registration.  Once you send another REGISTER to 
>renegotiate it, it's a new "lifetime".  No?  

This was addressed in -06 by removing the "lifetime" terminology.

-----------

> 4) Section 4.3 has:
>    A SIP entity that has negotiated sending of keep-alives associated
>    with a registration MUST be prepared to receive, from the 
>    receiver of the keep-alives, SIP messages (requests and responses) associated
>    with the registration on the same IP port:address from where the
>    keep-alives are sent. If the SIP entity sends the keep-alives over a
>    connection-oriented transport protocol, it MUST be prepared to
>    receive SIP messages associated with the registration over the same
>    connection.  The same rules apply for SIP messages associated with a
>    dialog, if the SIP entity has negotiated sending of keep-alives
>    associated with a dialog.
> 
>Why is that in this draft?? (I don't understand how it relates)

This was addressed in -06 by removing the text, and adding text saying that connection reuse must be provided using another mechanism.

---------

> 5) Security considerations.  There are some security 
> considerations which are not in 5626.  Just off the top of my 
> head they are:
> 
> a) Any downstream SIP entity, beyond the adjacent downstream 
> peer node, can modify the Via header identifying the local 
> node and thus cause the local node to send keepalives to its 
> adjacent peer (at high rates) if the peer does not support 
> this keepalive mechanism.
> 
> b) A man-in-the-middle between two SIP nodes can modify the 
> Via header at will, and thus can prevent keepalive from being 
> used, or can decrease the timer value to a very low value to 
> consume resources, or can cause the downstream to expect 
> keepalives while the upstream does not know to send them and 
> thus cause a failure.

This was addressed in -06, by adding the following text to the security considerations.

   "Unless SIP messages are integrity protected, a man-in-the-middle can
   modify Via header fields used by two entities to negotiate sending of
   keep-alives, e.g. by removing the indications used to indicate
   willingness to send and receive keep-alives, or by decreasing the
   timer value to a verly low value, which might trigger additional
   resource consumption due to the frequently sent keep-alives."

That covers mostly b). Nothing was added regarding a),because entities can always check that, 
before forwarding responses, the the Via header field hasn't been tempered with.

---------

>Editorial:
>- s/CRLF/double-CRLF/g

Fixed in -06.

---------

>- Section 4.4 first para, change:
>    The SIP entity must support those aspects of STUN that are required in order 
>    to apply the STUN keep-alive mechanism defined in [RFC5626], and it must support
>    the CRLF keep-alive mechanism defined in [RFC5626].
> To:
>    The SIP entity must support those aspects of STUN that are required in order 
>    to apply the STUN keep-alive mechanism defined in [RFC5626] for connection-less
>    transports such as UDP, and it must support the CRLF keep-alive 
>    mechanism defined in [RFC5626] for connection-oriented transports 
>    such as TCP and TLS over TCP.

This was addressed in -06 by inserting the text provided by John Elwell:

     "[RFC5626] defines when to use STUN, respectively double-CRLF, for keep-alives."

---------

Regards,

Christer

---

From: Adam Roach <[email-masked]>
Date: Wed, 13 Oct 2010 09:28:42 -0500

[as a participant]

>> adjacent peer (at high rates) if the peer does not support
...
> Nothing was added regarding a),because entities can always check that,

The purpose of the "Security Considerations" section is to warn 
implementors about things they need to take into consideration to avoid 
security problems. Shouldn't the document at least suggest that 
implementations validate the Via header field to prevent this kind of 
attack?

/a

---

From: Christer Holmberg <[email-masked]>
Date: Wed, 13 Oct 2010 17:28:17 +0200

Hi, 

>>>a) Any downstream SIP entity, beyond the adjacent downstream peer 
>>>node, can modify the Via header identifying the local node 
>>>and thus cause the local node to send keepalives to its adjacent 
>>>peer (at high rates) if the peer does not support
>> ...
>>Nothing was added regarding a),because entities can always 
>>check that, before forwarding responses, the the Via header field 
>>hasn't been tempered with.
> 
>The purpose of the "Security Considerations" section is to 
>warn implementors about things they need to take into 
>consideration to avoid security problems. Shouldn't the 
>document at least suggest that implementations validate the 
>Via header field to prevent this kind of attack?

Fair enough.

I can add the following text:

   "Downstream SIP entities can modify Via header fields identifying other SIP entities,
   and cause keepalives to be sent (at hight rates) to entities that do not not support 
   the keepalive mechanism. SIP entities can prevent this, when a SIP response is received, 
   by validating that Via headers have not been modified in a way which would cause such 
   sending of keepalives."

Regards,

Christer

---

From: Adam Roach <[email-masked]>
Date: Wed, 13 Oct 2010 10:56:51 -0500

>     by validating that Via headers have not been modified in a way which would cause such

That's good, but I think we can improve it by more clearly calling out 
what the implementations need to look for:

"Downstream SIP entities can modify Via header fields identifying other 
SIP entities,
and cause keepalives to be sent (at high rates) to entities that do not 
not support
the keepalive mechanism. SIP entities can prevent this, when a SIP 
response is received,
by examining their own Via header field to determine that downstream 
entities have
not added a 'keep' parameter or set an existing 'keep' parameter to a 
value not supported
by the implementation."

/a

---

From: Christer Holmberg <[email-masked]>
Date: Wed, 13 Oct 2010 18:06:57 +0200

Your text proposal looks good. I can add it.

Regards,

Christer 

> -----Original Message-----
> From: Adam Roach [mailto:[email-masked]] 
> Sent: 13. lokakuuta 2010 18:57
> To: Christer Holmberg
> Cc: [email-masked]; Hadriel Kaplan; [email-masked]; 
> Gonzalo Camarillo
> Subject: Re: Comments on draft-ietf-sipcore-keep-05
> 
> 
> >>>> node, can modify the Via header identifying the local 
> node and thus 
> >>>> cause the local node to send keepalives to its adjacent peer (at 
> >>>> high rates) if the peer does not support
> >>> ...
> >>> Nothing was added regarding a),because entities can always check 
> >>> that, before forwarding responses, the the Via header 
> field hasn't 
> >>> been tempered with.
> >> implementors about things they need to take into consideration to 
> >> avoid security problems. Shouldn't the document at least 
> suggest that 
> That's good, but I think we can improve it by more clearly 
> calling out what the implementations need to look for:
> 
> "Downstream SIP entities can modify Via header fields 
> identifying other SIP entities, and cause keepalives to be 
> sent (at high rates) to entities that do not not support the 
> keepalive mechanism. SIP entities can prevent this, when a 
> SIP response is received, by examining their own Via header 
> field to determine that downstream entities have not added a 
> 'keep' parameter or set an existing 'keep' parameter to a 
> value not supported by the implementation."
> 
> /a
> 
>

---

From: Christer Holmberg <[email-masked]>
Date: Wed, 13 Oct 2010 19:00:57 +0200

Hi,

I've submitted a new version of the keep-alive draft.

The changes are:

- I added the text suggested by Adam to the security considerations section.
- Small minor change in the 4th paragraph of section 4.1, as suggested by Paul earlier (after -06 had been submitted).

Regards,

Christer

> From: [email-masked] 
> [mailto:[email-masked]] On Behalf Of Christer Holmberg
> Sent: 13. lokakuuta 2010 19:07
> To: Adam Roach
> Cc: [email-masked]; Gonzalo Camarillo; Hadriel Kaplan
> Subject: Re: [sipcore] Comments on draft-ietf-sipcore-keep-05
> 
> 
> > Cc: [email-masked]; Hadriel Kaplan; [email-masked]; Gonzalo 
> > Camarillo
> > identifying other SIP entities,
> calling out 
> > "Downstream SIP entities can modify Via header fields identifying 
> > other SIP entities, and cause keepalives to be sent (at 
> high rates) to 
> > entities that do not not support the keepalive mechanism. 
> SIP entities 
> > can prevent this, when a SIP response is received, by 
> examining their 
> > own Via header field to determine that downstream entities have not 
> > added a 'keep' parameter or set an existing 'keep' parameter to a 
> _______________________________________________
> sipcore mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/sipcore
>