[OSSA 2013-008] DOS by allocating all fixed ips

IMO the easy fix for folsom/essex is probably just to disable/remove the add-fixed-ip code. For grizzly we should be enforcing this with a quota.

The offending command is in the multinic extension.

another potential easy fix for essex/folsom would be to disable multinic via policy.

Vish:

For Folsom/Essex: Are we sure people aren't using that code? I like the idea of updating the default policy to disallow it slightly better (although this could be a breaking change too).

For Grizzly using a quota sounds reasonable. Something like 'quota_fixed_ips'?

Dan: I'm not sure that no-one is using it. RAX uses it but enforces quotas in melange. Turning it off by default in policy is fine with me.

Didn't you do quotas for something else recently? I haven't taken a look at the quota code recently, so if you want to take a swing at it that would be awesome.

Hmm, that one is a bit tricky. Disabling the whole feature in a stable update sounds like a bad idea, this can hardly be considered a fix for the vulnerability, at best a workaround.

One alternative would be to issue a "security note" instead, advising people to disable it...
Adding markmc for the stable team for their opinion.

The rationale for disabling the feature is that only RAX is using it and they enforce quotas separately?

I don't like assuming RAX is the only one using it. We have enough users now that it's impossible to predict what people may be doing. If we publish this issue, recommend disabling the extension and there are people who are using it ... then we've nicely screwed them.

How about we cook up the quotas patch and see how invasive it is? It might be backportable with manageable regression risk

I took a quick look at folsom, and one small wart I've found is that I'd like to make the quota per project and the fixips model object doesn't store project id. Do people think this quota should be per project or per instance? If its per instance isn't it still pretty easy to DoS people? You just have to start a bunch of instances as well.

Ahhh, I can do a joined load. Here's a patch for folsom. Its untested (not even unit tests) because I have to leave for the airport in a minute. I'll try to take another look at this on the flight.

The folsom changes don't see too invasive to me, noting that I've never done a quota change before and might have that bit horribly wrong.

And now that I have stopped travelling, here's a version which passes unit tests. Do people think this is too invasive for folsom?

some errors in the above patch:

in db/api.py the method is called floating_ip_count_by_project, when it should be fixed_ip...

the exception should be a new exception FixedIp... not FloatingIp.

I don't think the patch is too invasive. We don't need a db migration right?

Agreed that this does not look too invasive for folsom.

I can't see why we'd need a migration. Sorry for the typos. I blame jet lag.

This version fixes Vishy's comments.

Agree it's not too invasive for Folsom

Looks good except you need a QUOTAS.rollback() if there's an exception between the reserve() and commit()

Sorry I didn't get a chance to progress this today... I will work on this more tomorrow.

So, this took a long time to test and get right, but this new version of the folsom patch has been tested with devstack and appears to work. It includes Mark's request for a rollback of quota allocations on exceptions, and required re-writing the database code to count fixed_ips. I believe this patch is good to go for Folsom, but would appreciate comments from nova-cores.

Looks pretty good to me. I guess we're not so worried about rollback if a failure happens in deallocate_fixed_ip().

Yeah, I did think about that, but we don't care about rollback for floating ips, so I figure we similarly don't care for fixed ips. I've just found a PEP8 failure in this code, so I'll send yet another patch which fixes that in a minute.

Ok, here are patches for essex and folsom, both of which have been tested with devstack and pass PEP8. The Folsom one also passes unit tests, but I can't get the essex unit tests to run for me, so that one hasn't been done.

@ttx -- are you sure we don't need a patch for grizzly? That's how I am reading your "no longer affects nova/grizzly" above. Am I confused?

We need a patch for Grizzly, it's just that grizzly does not need a specific series task. it's tracked in the main bugtask. We only use series tasks when we need a stable/* backport. The main bugtask tracks "master" (master happening to be grizzly right now). So it's not "no longer affects Grizzly", it's "removing extraneous bugtask".

Ok, here's a patch for grizzly. Again, passes unit tests and devstack twiddling.

It would be cool if other nova-core's could check each of these three patches and see what they think.

Adding nova-core for review on this bug.

(reviewing the grizzly patch): All the quota tests seem to set fixed_ips quota just to the default of 10, which isn't a very good update test. That should be updated. Other than that this approach seems sane.

Sean -- I'm happy to tweak the unit tests. Are there any other
comments from others?

Michael

@Mikal: wanna have a try at the impact description ? See https://wiki.openstack.org/wiki/VulnerabilityManagement for templates.

@ttx -- I pinged you with a draft of one the other day, but perhaps you didn't see it. How about something like this?

-----

SUBJECT: Vulnerability in OpenStack Nova

This is an advance warning of a vulnerability discovered in OpenStack,
to give you, as downstream stakeholders, a chance to coordinate the
release of fixes and reduce the vulnerability window. Please treat the
following information as confidential until the proposed public
disclosure date.

Title: DoS by allocating all Fixed IPs
Impact: Medium
Reporter: Vish Ishaya (Nebula)
Products: Nova
Affects: All versions

Description:
Vish Ishaya reported a vulnerability in Nova where there is no quota
for Fixed IPs. Previously the instance quota acted as a proxy for
a Fixed IP quota, but if your configuration allows an instance to
consume more than one Fixed IP via an extension such as multinic
then this is no longer true. Running out of Fixed IPs would result in
not being able to spawn new instances.

Proposed patches:
See attached patches. Unless a flaw is discovered in them, these
patches will be merged to Nova master (Grizzly), stable/folsom,
and stable/essex branches on the public disclosure date.

CVE:
No CVE has been assigned to this issue yet.

Proposed public disclosure date/time:
Tuesday, March 12, 1500UTC
Please do not make the issue public (or release public patches) before
this coordinated embargo date.

Regards,

-----

If we're running with that disclosure date we should probably send this out ASAP. Perhaps you could do it overnight?

+1 on the description. Looks like we're missing some approvals on that patch, though. If we gather them today then we can send this baby out with a disclosure date of Thursday, March 14.

Thanks ttx. At this point I think we should handle Sean's request for more unit tests for trunk in a separate patch. I'm happy to do that work, I just don't want to block this fix. Given I am trapped in a training course yet again today, can you please run with getting this disclosure email sent when we're ready?

Grizzly patch looks good to me.

Notification sent to downstream stakeholders.

Proposed public disclosure date/time:
Thursday, March 14, 1500UTC

Adding Stable and Essex maintainers

FYI, applying the essex patch to 2012.1.3+stable-20120827 results in several unit test errors (see attached).

Attempting to fetch that attachment gets me a "processing error" message from LP.

I was able to fetch it by clicking on it, then using 'Save page as' in my browser. If using wget or similar, that won't work because the bug is private.

Jamie -- I got it working by waiting a bit and trying again.

Jamie, sorry about that. Does this new patch work for you?

The updated patch works with the version of essex in Ubuntu 12.04 LTS, yes. Thanks!

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

Fix proposed to branch: stable/folsom
Review: https://review.openstack.org/24452

Fix proposed to branch: stable/essex
Review: https://review.openstack.org/24453

Reviewed: https://review.openstack.org/24452
Committed: http://github.com/openstack/nova/commit/9561484166f245d0e4602a36351d6cac72dd9426
Submitter: Jenkins
Branch: stable/folsom

commit 9561484166f245d0e4602a36351d6cac72dd9426
Author: Michael Still <email address hidden>
Date: Wed Mar 13 04:44:14 2013 +1100

    Add quotas for fixed ips.

    DocImpact: there is now a default quota of 10 fixed ips per tenant.
    This will need to be adjusted by deployers if that number does not
    meet their needs.

    Resolves bug 1125468 for folsom.

    Change-Id: I970d540cfa6a61b7e903703f845a6453ff55f225

OSSA-2013-008

Reviewed: https://review.openstack.org/24453
Committed: http://github.com/openstack/nova/commit/efaacdaee116388234558e2682b647d41fe5b149
Submitter: Jenkins
Branch: stable/essex

commit efaacdaee116388234558e2682b647d41fe5b149
Author: Michael Still <email address hidden>
Date: Fri Mar 15 03:45:41 2013 +1100

    Add quotas for fixed ips.

    DocImpact: there is now a default quota of 10 fixed ips per tenant.
    This will need to be adjusted by deployers if that number does not
    meet their needs.

    Resolves bug 1125468 for essex.

    Change-Id: I2a5afaa47afb182f4917cd43e1ebd0d6cd1330e3

Reviewed: https://review.openstack.org/24451
Committed: http://github.com/openstack/nova/commit/99429214d4ddb5bdc7de185693b8a53ad50df3c6
Submitter: Jenkins
Branch: master

commit 99429214d4ddb5bdc7de185693b8a53ad50df3c6
Author: Michael Still <email address hidden>
Date: Tue Mar 5 05:53:43 2013 +1100

    Add quotas for fixed ips.

    DocImpact: there is now a default quota of 10 fixed ips per tenant.
    This will need to be adjusted by deployers if that number does not
    meet their needs.

    Resolves bug 1125468.

    Change-Id: Iffa19583340f80cb2a13ba5fce31f7ff724a52d6