[OSSA 2014-025] There is no quota for allowed address pair (CVE-2014-3555)

Bug #1336207 reported by Liping Mao
258
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Security Advisory
Fix Released
High
Tristan Cacqueray
neutron
Fix Released
Undecided
Liping Mao
Havana
Fix Released
Undecided
Liping Mao
Icehouse
Fix Released
Undecided
Liping Mao

Bug Description

Hi all,

There is no quota for allowed address pair, user can create unlimited allowed address pair, in the backend, there will be at least 1 iptables rule for one allowed address pair. I tested if we use the attachment script to add about 10,000 allowed address pair. It will cost 30 sec to reflesh iptables rules in kernel... I think that bad man can use this api to attack compute nodes. This will make the compute nodes crash or very slow only if we add enough allowed address pair rules...

Thanks.
Liping Mao

Revision history for this message
Liping Mao (limao) wrote :
information type: Private Security → Private
Revision history for this message
Jeremy Stanley (fungi) wrote :

This sounds similar to nova bug 1184041 (OSSA 2013-020, CVE-2013-4185). In which OpenStack release(s) did you observe this behavior?

Changed in ossa:
status: New → Incomplete
Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

The OSSA tasks is set to incomplete pending for additional security detail.

Revision history for this message
Liping Mao (limao) wrote :

Hi Jeremy Stanley ,

I think this is not the same problem with bug 1184041.

This problem happens only if neutron with allowed address pair extension.
So it can happen in Havana / Icehouse / Juno trunk.

The reproduce steps :
1. boot a vm.
2. update the vm port with the attachment script.

Then you can see about 10,000 iptables rules on compute node. and the update of iptable rule will be very slow. And I think the performance of iptables update rule will be very bad if there are too many rules.

And the root cause is that we do not have quota for allowed address pair. And a user can update unlimited allowed address pair for one port.

Regards,
Liping Mao

Liping Mao (limao)
Changed in neutron:
assignee: nobody → Liping Mao (limao)
Revision history for this message
Liping Mao (limao) wrote :

Hi,

I write a patch to add quota for allowed address pair, this can avoid the user to set up unlimited rules.
Thanks.

Regards,
Liping Mao

Revision history for this message
Jeremy Stanley (fungi) wrote :

Right, I didn't mean to imply that this is the same bug as described in 1184041, merely that the vulnerability is similar enough for us to reuse some of the same impact analysis wording when publishing a security advisory.

information type: Private → Private Security
Revision history for this message
Liping Mao (limao) wrote :

@Tristan Cacqueray @Jeremy

Is there any other detail info need? If need , please let me know. Thanks.

Regards,
Liping Mao

Revision history for this message
Jeremy Stanley (fungi) wrote :

At this point we're waiting on Neutron security reviewers confirm the bug and assess your proposed fix, let us know any mitigating factors, which releases are impacted, et cetera.

Thierry Carrez (ttx)
Changed in ossa:
status: Incomplete → Confirmed
importance: Undecided → High
Revision history for this message
Salvatore Orlando (salvatore-orlando) wrote :

There is clearly a possibility of causing a denial of services using this kind of 'attack'.
In theory on would make allowed address pairs a resource with a quota, but that's not easy as they're an attribute rather than a resource.

So I would rather take the approach in the suggested patch of putting a configurable hard limit on the number of address pairs allowed on a port.
It should also possible to have both a maximum number of pairs per port and a global per-tenant maximum number of additional address pairs.

Revision history for this message
Liping Mao (limao) wrote :

Hi Salvatore,

Thanks for your confirm and comments.

One question about the "global per-tenant maximum number of additional address pairs".

In my opinion, if we have the maximum number of the pairs per port , and we also already have the quota for ports. Then the max number of the tenant is Pair_Max_Per_Port * Port_Num. Seems like we do not need the maximum number for the tenant.

What I do in the patch is similar to "extra route" extension.
https://github.com/openstack/neutron/blob/master/neutron/db/extraroute_db.py#L100-103

It also did not limit the maximum number of route per tenant.

What do you think about this? Thanks .

Regards,
Liping Mao

Changed in ossa:
assignee: nobody → Tristan Cacqueray (tristan-cacqueray)
Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

Here is impact description draft #1:

Title: Denial of Service in Neutron allowed address pair
Reporter: Liping Mao (Cisco)
Products: Neutron
Versions: up to 2013.2.3, and 2014.1 versions up to 2014.1.1

Description:
Liping Mao from Cisco reported a denial of service vulnerability in Neutron's handling of address pair. By creating a large number of allowed address pair, an authenticated user may overwhelm neutron firewall rules and render compute nodes unusable. All Neutron setups are affected.

Revision history for this message
Liping Mao (limao) wrote :

hi Tristan,

"handling of address pair."
I think "handling of allowed address pair." may be better.

Regards,
Liping Mao

Revision history for this message
Thierry Carrez (ttx) wrote :

Maybe "By creating a large number of allowed address pair*s*"

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

Thanks! Here is impact description draft #2:

Title: Denial of Service in Neutron allowed address pair
Reporter: Liping Mao (Cisco)
Products: Neutron
Versions: up to 2013.2.3, and 2014.1 versions up to 2014.1.1

Description:
Liping Mao from Cisco reported a denial of service vulnerability in Neutron's handling of allowed address pair. By creating a large number of allowed address pairs, an authenticated user may overwhelm neutron firewall rules and render compute nodes unusable. All Neutron setups are affected.

Revision history for this message
Thierry Carrez (ttx) wrote :

+1 on version #2

Revision history for this message
Jeremy Stanley (fungi) wrote :

Tristan's impact description draft #2 looks good to me.

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

@Liping Mao, Trying to apply your patch to stable/havana says:

patching file neutron/extensions/allowedaddresspairs.py
Hunk #1 succeeded at 20 (offset 4 lines).
Hunk #2 succeeded at 49 with fuzz 1 (offset 8 lines).
patching file neutron/tests/unit/test_extension_allowedaddresspairs.py
Hunk #1 succeeded at 23 with fuzz 2 (offset 1 line).
Hunk #2 FAILED at 161.
Hunk #3 succeeded at 184 (offset -4 lines).
1 out of 3 hunks FAILED -- saving rejects to file neutron/tests/unit/test_extension_allowedaddresspairs.py.rej

And stable/icehouse:
patching file neutron/extensions/allowedaddresspairs.py
Hunk #2 succeeded at 45 with fuzz 1 (offset 4 lines).
patching file neutron/tests/unit/test_extension_allowedaddresspairs.py
Hunk #1 succeeded at 22 with fuzz 2.
Hunk #2 FAILED at 161.
Hunk #3 succeeded at 176 (offset -12 lines).
1 out of 3 hunks FAILED -- saving rejects to file neutron/tests/unit/test_extension_allowedaddresspairs.py.rej

Could you please backport your patch to make it applies cleanly to those two stable branch ?

@VMT, Considering the fix seems to apply, should I request for CVE with the imapct description draft #2 now ?

Revision history for this message
Liping Mao (limao) wrote :

Patch for stable havana

Revision history for this message
Liping Mao (limao) wrote :

Patch for stable icehouse

summary: - There is no quota for allowed address pair
+ There is no quota for allowed address pair (CVE-2014-3555)
Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote : Re: There is no quota for allowed address pair (CVE-2014-3555)

Thanks Liping Mao! ./run_tests.sh succeeded on both backports!

Though, there are those flake8 errors:

./neutron/tests/unit/test_extension_allowedaddresspairs.py:176:5: E303 too many blank lines (2)
    def test_more_than_max_allowed_address_pair(self):
    ^
./neutron/tests/unit/test_extension_allowedaddresspairs.py:215:5: E303 too many blank lines (2)
    def test_update_add_address_pairs(self):
    ^
./neutron/extensions/allowedaddresspairs.py:33:1: E302 expected 2 blank lines, found 1
class AllowedAddressPairsMissingIP(nexception.InvalidInput):
^

@neutron-coresec: Can you please review those three patches ?

Revision history for this message
Liping Mao (limao) wrote :

Update patch for havana

Revision history for this message
Liping Mao (limao) wrote :

Update patch for icehouse

Revision history for this message
Aaron Rosen (arosen) wrote :

Thanks, this patch looks good to me. +2

Revision history for this message
Liping Mao (limao) wrote :

@Tristan

Thanks for your remind, I update the havana and icehouse patch, it should be OK now.

@Aaron

Thanks for your review.

Thierry Carrez (ttx)
Changed in ossa:
status: Confirmed → In Progress
Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

The disclosure date have been set to
2014-07-17, 1500UTC

@Liping Mao Would you be available at that time to submit patchs to gerrit ? Else, can you please reformat your patch and add a commit message using "git am" in order for us to push them while keeping authorship.

Thierry Carrez (ttx)
Changed in ossa:
status: In Progress → Fix Committed
Changed in neutron:
status: New → In Progress
Revision history for this message
Liping Mao (limao) wrote :

patch for trunk with commit log

Revision history for this message
Liping Mao (limao) wrote :

patch for havana with commit log

Revision history for this message
Liping Mao (limao) wrote :

patch for icehouse with commit log

Revision history for this message
Liping Mao (limao) wrote :

@Tristan Cacqueray

Thanks for your comments. And I use "git format-patch" to re-create the patch file, and this time it has commit log. Please feel free to tell me, if there is anything need to be modified.

BTW,
Do you mean that I need to commit these three patchs on 2014-07-17, 1500UTC ?
Sure , I am available to do that at that time. Although this is mid-night in my local time : -)

Regards,
Liping Mao

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

@limao Oh well, don't worry, I can git-review those patch for you if you want, or maybe someone else from @neutron-coresec might be around to help in case of problems ?

Revision history for this message
Liping Mao (limao) wrote :

@Tristan Cacqueray

I think I can do it by myself :) Thanks for your help.

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

@Liping Mao, bug is now public, if you can please push patches now that would be just perfect!

summary: - There is no quota for allowed address pair (CVE-2014-3555)
+ [OSSA 2014-025] There is no quota for allowed address pair
+ (CVE-2014-3555)
information type: Private Security → Public Security
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to neutron (stable/havana)

Fix proposed to branch: stable/havana
Review: https://review.openstack.org/107731

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to neutron (stable/icehouse)

Fix proposed to branch: stable/icehouse
Review: https://review.openstack.org/107733

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to neutron (master)

Fix proposed to branch: master
Review: https://review.openstack.org/107734

Revision history for this message
Liping Mao (limao) wrote :

@ Tristan Cacqueray

Done, and waiting for the jenkins now...

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

Reviewed: https://review.openstack.org/107734
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=dc6b07d4a37ef0db9906187611fec8e8753803cd
Submitter: Jenkins
Branch: master

commit dc6b07d4a37ef0db9906187611fec8e8753803cd
Author: Liping Mao <email address hidden>
Date: Tue Jul 15 14:22:46 2014 +0800

    no quota for allowed address pair

    There is no quota for allowed address pair. User can create unlimited
    allowed address pairs. I add quota for allowed address pairs.

    Change-Id: I2efb0c0f527f1fb22c4d4b07f6d280863f565648
    Closes-Bug: #1336207

Changed in neutron:
status: In Progress → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (stable/havana)

Reviewed: https://review.openstack.org/107731
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=526412ee6123534bdf41039a8c4d5fada87dcf3a
Submitter: Jenkins
Branch: stable/havana

commit 526412ee6123534bdf41039a8c4d5fada87dcf3a
Author: Liping Mao <email address hidden>
Date: Tue Jul 15 14:20:15 2014 +0800

    no quota for allowed address pair

    There is no quota for allowed address pair. User can create unlimited
    allowed address pairs. I add quota for allowed address pairs.

    Change-Id: Ie672373f770a4f9fe70d87a779cabc731413711a
    Closes-Bug: #1336207
    (cherry picked from commit I2efb0c0f527f1fb22c4d4b07f6d280863f565648)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (stable/icehouse)

Reviewed: https://review.openstack.org/107733
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=2c4828e28a5ff6e537be9db4da0e98eca33135d5
Submitter: Jenkins
Branch: stable/icehouse

commit 2c4828e28a5ff6e537be9db4da0e98eca33135d5
Author: Liping Mao <email address hidden>
Date: Tue Jul 15 14:22:05 2014 +0800

    no quota for allowed address pair

    There is no quota for allowed address pair. User can create unlimited
    allowed address pairs. I add quota for allowed address pairs.

    Change-Id: I7fec291a4838fba96b6d400ad56dbcdba9584d0f
    Closes-Bug: #1336207
    (cherry picked from commit I2efb0c0f527f1fb22c4d4b07f6d280863f565648)

Changed in ossa:
status: Fix Committed → Fix Released
Changed in neutron:
milestone: none → juno-2
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in neutron:
milestone: juno-2 → 2014.2
To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.