Subject: Re: [sip-overload] AD review of draft-ietf-soc-load-control-event-package-08

From: Richard Barnes <[email-masked]>
Date: Tue, 29 Oct 2013 15:08:37 -0400

Dear authors,

Thanks for updating this document.  It looks much better.  Sorry for the
delay in my reply.

The one thing I don't think is quite adequately addressed by the
considerations around the use of the "redirect" action.

The attack I'm worried about is something like the following:
-- A bunch of proxies in a Trust Domain are using UDP, and configured to
get their policies from a central server.
-- Attacker spoofs the server's address to send a bunch of NOTIFY bodies
telling proxies to send all calls to [email-masked]
-- Proxies redirect all calls to victim, who is now DoSed off of the
Internet

In order to address this threat, you need to either (1) remove the
"redirect" action, or (2) constrain redirects to a safe space.  I assume
that the WG doesn't want to do (1) :)

Proposed edits:
-- Add to the definition of a Trust Domain an agreement on the types of
calls that can be affected by load control.  For example, <one> and <many>
element might be limited to specific domains; <*-tel> elements might be
limited to be within certain prefixes.
-- Add to the definition of a Trust Domain an agreement on the destinations
to which calls may be redirected.  For example, the URI might have to match
a given set of domains.
-- Add text to the Security Considerations describing the above attack, and
REQUIREing that implementations enforce the limits described in the
definition of their Trust Domain.

Thanks,
--Richard

On Wed, Apr 17, 2013 at 1:53 PM, Richard Barnes <[email-masked]> wrote:

> I have reviewed this document, and have a few questions before IETF LC:
>
> Major:
>
> In a few places, for example Section 5.3., the document suggests that this
> event package could be used to communicate load filtering policies between
> domains.  This seems like a bad idea for a few reasons.  First, policies
> can be based on "P-Asserted-Identity", which is itself limited to use
> within a trust domain / Spec(T).  It doesn't make sense to have policies
> based on these identifiers outside of the domain in which they are used.
>  Second, inter-domain policies can create subtle and dangerous security
> risks.  For example, according to the current specification, Domain A could
> tell Domain B to drop all calls between Domain B and Domain C.  It's not
> clear how you would prevent these sorts of attacks, especially where "tel:"
> URIs are involved.  I think both of these issues go away if this event
> package is limited in a similar way to P-Asserted-Identity, i.e., limited
> to use within a trust domain.
>
> Section 6.3., "SUBSCRIBE Bodies" doesn't actually say anything about what
> goes in the body of a SUBSCRIBE message.  On the one hand, it implies that
> there could be a body (since the last sentence considers a request without
> a body as a special case), but it doesn't say what goes in the body if
> there is one.  Please either (1) define what may go in the body, (2)
> explicitly say that this document does not specify what goes in SUBSCRIBE
> bodies, or (3) require that the body be empty.   (Also, the first paragraph
> of this section seems out of place here, since it also has nothing to do
> with the body.)
>
> In Section 6.5., the last sentence is wrong.  The presence of an Accept
> header indicates that the response body should be one of the indicated
> types.  The indicated behavior would only be acceptable if the Accept
> header included "multipart/mixed".  Suggest deleting the last sentence.
>  (Also,  it would be clearer to change "the request body will contain" to
> "the request body MUST contain".)
>
> In Section 7.3.1, you need to specify how a "tel:" URI is matched against
> a domain value starting with "+".  Your examples seem to indicate simple
> string prefix matching, but I doubt that's what you actually want, since
> non-digit characters can break things.  For example, "+1212" should match "
> +1-212-555-1212".  Please specify a matching algorithm here.
>
> In Section 7.3.2, please change "Non-initial requests ... are not
> subjected" to "Non-initial requests ... MUST NOT be subjected".
>
> Section 7.4. doesn't adequately define the actions to be taken.  Each of
> the action elements (<rate>, <window>, <percent>) need to define a concrete
> action that the proxy should take.
>
> RFC 4745 allows rules to combine when multiple rules match a given call.
>  This document needs to define combination rules for the actions defined
> here.
>
> What's the reason for having both the "drop" action and the "reject"
> action?  It seems like the "drop" action is almost always harmful.  With
> unreliable transport, it causes retransmits, and even with reliable
> transport, it causes the client to wait unnecessarily until the connection
> times out.  In any case, the "simple drop" action is underspecified.  For
> example, does the server simply ignore the SIP message, or does it close
> the transport connection?
>
>
> Minor:
>
> In Section 7.3, "we re-define" -- the document doesn't re-define any of
> the elements in RFC 4745 (that's good; redefinition is bad).  Instead, you
> should say you define new identity elements.
>
> In Section 7.3.1, please break up paragraph starting "To include the two
> forms..." for greater readability.  Suggested break points: Before "Note
> that the tradeoff...", and before "It should be noted..."
>
> In Section 7.3.3, it would be helpful to break up the paragraph starting
> "The following are two example...".  Break before "Usecase I" and "Usecase
> II".  Also, s/Usecase/Use case/g
>
>
> Thanks,
> --Richard
>

---

From: Charles Shen <[email-masked]>
Date: Wed, 30 Oct 2013 14:11:56 -0400

Dear Richard,

Thank you very much for your comments! I've incorporated your suggestions
into a revised version, as attached for your reference. Changes are in
Section 5.4 (definition of Trust Domain) and Section 11 (Security
Considerations). If there are any further concerns please let me know, I
will upload the most updated revision when the IETF submission site
re-opens on Nov. 4.

Thanks!

Charles

On Tue, Oct 29, 2013 at 3:08 PM, Richard Barnes <[email-masked]> wrote:

> -- Add to the definition of a Trust Domain an agreement on the
> destinations to which calls may be redirected.  For example, the URI might
> have to match a given set of domains.
> -- Add text to the Security Considerations describing the above attack,
> and REQUIREing that implementations enforce the limits described in the
>> In a few places, for example Section 5.3., the document suggests that
>> this event package could be used to communicate load filtering policies
>> between domains.  This seems like a bad idea for a few reasons.  First,
>> policies can be based on "P-Asserted-Identity", which is itself limited to
>> use within a trust domain / Spec(T).  It doesn't make sense to have
>> policies based on these identifiers outside of the domain in which they are
>> used.  Second, inter-domain policies can create subtle and dangerous
>> security risks.  For example, according to the current specification,
>> Domain A could tell Domain B to drop all calls between Domain B and Domain
>> C.  It's not clear how you would prevent these sorts of attacks, especially
>> where "tel:" URIs are involved.  I think both of these issues go away if
>> this event package is limited in a similar way to P-Asserted-Identity,
>> i.e., limited to use within a trust domain.
>>
> _______________________________________________
> sip-overload mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/sip-overload
>
>

---

From: Richard Barnes <[email-masked]>
Date: Wed, 30 Oct 2013 16:52:19 -0400

I wouldn't have used my text quite so directly (maybe edited out some
"bunch of" instances), but this looks OK to me.  Thanks for the quick
update.  Please feel free to post.

Is there anyone in the WG that has an objection to these changes?

On Wed, Oct 30, 2013 at 2:11 PM, Charles Shen <[email-masked]> wrote:

>>> Section 6.3., "SUBSCRIBE Bodies" doesn't actually say anything about
>>> what goes in the body of a SUBSCRIBE message.  On the one hand, it implies
>>> that there could be a body (since the last sentence considers a request
>>> without a body as a special case), but it doesn't say what goes in the body
>>> if there is one.  Please either (1) define what may go in the body, (2)
>>> In Section 7.3.1, you need to specify how a "tel:" URI is matched
>>> against a domain value starting with "+".  Your examples seem to indicate
>>> simple string prefix matching, but I doubt that's what you actually want,
>>> since non-digit characters can break things.  For example, "+1212" should
>>> match "+1-212-555-1212".  Please specify a matching algorithm here.
>>>

---

From: Shida Schubert <[email-masked]>
Date: Wed, 30 Oct 2013 14:45:42 -0700

Hi Richard;

 I am not an author but as one who had some concern with 
your comments on the security section, I think the following 
suggestions look good. 

 Thanks
  Shida

On Oct 29, 2013, at 12:08 PM, Richard Barnes <[email-masked]> wrote:

> Dear authors,
> 
> Thanks for updating this document.  It looks much better.  Sorry for the delay in my reply.
> 
> The one thing I don't think is quite adequately addressed by the considerations around the use of the "redirect" action.  
> 
> -- A bunch of proxies in a Trust Domain are using UDP, and configured to get their policies from a central server.  
> -- Attacker spoofs the server's address to send a bunch of NOTIFY bodies telling proxies to send all calls to [email-masked]
> -- Proxies redirect all calls to victim, who is now DoSed off of the Internet
> 
> In order to address this threat, you need to either (1) remove the "redirect" action, or (2) constrain redirects to a safe space.  I assume that the WG doesn't want to do (1) :)  
> 
> Proposed edits:
> -- Add to the definition of a Trust Domain an agreement on the types of calls that can be affected by load control.  For example, <one> and <many> element might be limited to specific domains; <*-tel> elements might be limited to be within certain prefixes.
> -- Add to the definition of a Trust Domain an agreement on the destinations to which calls may be redirected.  For example, the URI might have to match a given set of domains.
> -- Add text to the Security Considerations describing the above attack, and REQUIREing that implementations enforce the limits described in the definition of their Trust Domain.
> 
> Thanks,
> --Richard
> 
> 
> 
> 
> 
> 
> In a few places, for example Section 5.3., the document suggests that this event package could be used to communicate load filtering policies between domains.  This seems like a bad idea for a few reasons.  First, policies can be based on "P-Asserted-Identity", which is itself limited to use within a trust domain / Spec(T).  It doesn't make sense to have policies based on these identifiers outside of the domain in which they are used.  Second, inter-domain policies can create subtle and dangerous security risks.  For example, according to the current specification, Domain A could tell Domain B to drop all calls between Domain B and Domain C.  It's not clear how you would prevent these sorts of attacks, especially where "tel:" URIs are involved.  I think both of these issues go away if this event package is limited in a similar way to P-Asserted-Identity, i.e., limited to use within a trust domain.
> 
> Section 6.3., "SUBSCRIBE Bodies" doesn't actually say anything about what goes in the body of a SUBSCRIBE message.  On the one hand, it implies that there could be a body (since the last sentence considers a request without a body as a special case), but it doesn't say what goes in the body if there is one.  Please either (1) define what may go in the body, (2) explicitly say that this document does not specify what goes in SUBSCRIBE bodies, or (3) require that the body be empty.   (Also, the first paragraph of this section seems out of place here, since it also has nothing to do with the body.)
> 
> In Section 6.5., the last sentence is wrong.  The presence of an Accept header indicates that the response body should be one of the indicated types.  The indicated behavior would only be acceptable if the Accept header included "multipart/mixed".  Suggest deleting the last sentence.  (Also,  it would be clearer to change "the request body will contain" to "the request body MUST contain".)
> 
> In Section 7.3.1, you need to specify how a "tel:" URI is matched against a domain value starting with "+".  Your examples seem to indicate simple string prefix matching, but I doubt that's what you actually want, since non-digit characters can break things.  For example, "+1212" should match "+1-212-555-1212".  Please specify a matching algorithm here.
> 
> In Section 7.3.2, please change "Non-initial requests ... are not subjected" to "Non-initial requests ... MUST NOT be subjected".
> 
> Section 7.4. doesn't adequately define the actions to be taken.  Each of the action elements (<rate>, <window>, <percent>) need to define a concrete action that the proxy should take. 
> 
> RFC 4745 allows rules to combine when multiple rules match a given call.  This document needs to define combination rules for the actions defined here.  
> 
> What's the reason for having both the "drop" action and the "reject" action?  It seems like the "drop" action is almost always harmful.  With unreliable transport, it causes retransmits, and even with reliable transport, it causes the client to wait unnecessarily until the connection times out.  In any case, the "simple drop" action is underspecified.  For example, does the server simply ignore the SIP message, or does it close the transport connection?
> 
> 
> Minor:
> 
> In Section 7.3, "we re-define" -- the document doesn't re-define any of the elements in RFC 4745 (that's good; redefinition is bad).  Instead, you should say you define new identity elements.
> 
> In Section 7.3.1, please break up paragraph starting "To include the two forms..." for greater readability.  Suggested break points: Before "Note that the tradeoff...", and before "It should be noted..."
> 
> In Section 7.3.3, it would be helpful to break up the paragraph starting "The following are two example...".  Break before "Usecase I" and "Usecase II".  Also, s/Usecase/Use case/g
> 
> 
> Thanks,
> --Richard
>

---

From: Shida Schubert <[email-masked]>
Date: Wed, 30 Oct 2013 14:19:02 -0700

Hi Richard;

 The proposed text looks good to me.. 

  Shida

---

From: Charles Shen <[email-masked]>
Date: Thu, 31 Oct 2013 11:00:07 -0400

Hi Richard, I will let the "bunch of"s go before upload :)

On Wed, Oct 30, 2013 at 4:52 PM, Richard Barnes <[email-masked]> wrote:

>>>> Section 7.4. doesn't adequately define the actions to be taken.  Each
>>>> of the action elements (<rate>, <window>, <percent>) need to define a
>>>> concrete action that the proxy should take.
>>>>
>>>> RFC 4745 allows rules to combine when multiple rules match a given
>>>> call.  This document needs to define combination rules for the actions
>>>> defined here.
>>>>
>>>> In Section 7.3.1, please break up paragraph starting "To include the
>>>> two forms..." for greater readability.  Suggested break points: Before
>>>> "Note that the tradeoff...", and before "It should be noted..."
>>>>
>>>> In Section 7.3.3, it would be helpful to break up the paragraph
>>>> starting "The following are two example...".  Break before "Usecase I" and
>>>> "Usecase II".  Also, s/Usecase/Use case/g
>>>>
>>>>
>>>> Thanks,
>>>> --Richard
>>>>
>>>
>>>