OpenStack Compute (nova)

[OSSA 2013-023] Potential unsafe XML usage (CVE-2013-4179, CVE-2013-4202)

Bug #1190229 reported by Thierry Carrez on 2013-06-12

266

This bug affects 1 person

	Status	Importance	Assigned to	Milestone
Cinder	Fix Released	High	Thierry Carrez	Cinder 2013.2 "havana"
Grizzly	Fix Released	High	Thierry Carrez	Cinder 2013.1.3
OpenStack Compute (nova)	Fix Released	High	Michael Still	OpenStack Compute (nova) 2013.2 "havana"
Grizzly	Fix Released	High	Thierry Carrez	OpenStack Compute (nova) 2013.1.3
OpenStack Security Advisory	Fix Released	Medium	Thierry Carrez
neutron	Invalid	Undecided	Unassigned

Bug Description

Grant Murphy (<email address hidden>) conducted an audit of OpenStack and reported the following potential XML related vulnerabilities. These may well not be exploitable, we need to doublecheck them.

===================================================
Issue #1 (CWE-776) - Widespread and inconsistent usage of XML libraries that are vulnerable to various XML resource exhaustion attack vectors. This was mostly addressed in bug 1100282 however I can still find instances of minidom.parseString used directly in contributors code:

nova/nova/virt/libvirt/driver.py
1640: dom = minidom.parseString(xml)
1657: dom = minidom.parseString(xml)

nova/nova/virt/xenapi/vm_utils.py
1501: rrd = minidom.parseString(xml)
1542: doc = minidom.parseString(xml)

nova/nova/api/openstack/compute/contrib/security_groups.py
542: root = minidom.parseString(req.body)

nova/nova/api/openstack/compute/contrib/security_group_default_rules.py
76: dom = minidom.parseString(string)

cinder/cinder/api/contrib/backups.py
85: dom = minidom.parseString(string)
104: dom = minidom.parseString(string)

cinder/cinder/api/contrib/volume_transfer.py
65: dom = minidom.parseString(string)
83: dom = minidom.parseString(string)

Furthermore there are inconsistencies in the fix between components. For example in Quantum they use a ProtectedXMLParser in quantum/quantum/wsgi.py for XMLDeserialization. The implementation is as follows:

class ProtectedXMLParser(etree.XMLParser):
    def __init__(self, *args, **kwargs):
        etree.XMLParser.__init__(self, *args, **kwargs)
        self._parser.StartDoctypeDeclHandler = self.start_doctype_decl

def start_doctype_decl(self, name, sysid, pubid, internal):
raise ValueError(_("Inline DTD forbidden"))

def doctype(self, name, pubid, system):
raise ValueError(_("Inline DTD forbidden"))

Although etree.XMLParser does not automatically expand external entities by default it is vulnerable to billion laughs and quadratic blowup according to the diffusedxml documentation. (I'm not sure this is still accurate though).

For consistency alone I would recommend shifting these libraries across the board:
- https://pypi.python.org/pypi/defusedxml/
- https://pypi.python.org/pypi/defusedexpat/

========================================================
Issue #2 (CWE-112)- Missing schema validation. There are several instances where XML data is processed from an external source that is not validated against an XML schema. This is a best practice that should be considered.

========================================================
Issue #3 (Probably not a bug)- HTTP parameters used directly in XML output. The output appears to use sax.xhtml_escape so in theory is safe. Could use a whitelist reject unexpected input.

The case I'm specifically talking about is the BucketHandler class in nova/nova/objectstore/s3server.py. This does:

prefix = self.request.params.get("prefix", "")
marker = self.request.params.get("marker", "")

And uses these values directly in render_xml (results sent to end user).
This one is probably harmless but thought I'd mention it anyway.

Related branches

lp://staging/ubuntu/raring-security/cinder

CVE References

Thierry Carrez (ttx) on 2013-06-12

Changed in ossa:
status:	New → Incomplete

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-06-17:

Analysis in Nova:

nova/nova/virt/libvirt/driver.py
What's getting parsed is XML from libvirt, not user-provided XML requests. So that's safe.

nova/nova/virt/xenapi/vm_utils.py
What's getting parsed is XML from XenServer, not user-provided XML requests, so that's safe.

nova/nova/api/openstack/compute/contrib/security_groups.py
While most calls use xmlutil.safe_minidom_parse_string (which is safe), there is one call in _extend_servers that still uses minidom.parseString(req.body) and therefore looks vulnerable.

nova/nova/api/openstack/compute/contrib/security_group_default_rules.py
Uses pure minidom in its XML deserializer, so probably vulnerable.

Changed in nova:
status:	New → Confirmed

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-06-17:

Quantum analysis:
My understanding is that million laughs/quadratic blowup needs DTD evaluation, and the ProtectedXMLParser protects against reading an included DTD, so that sounds safe.

I agree that more consistency across the board would be welcome though.

Changed in quantum:
status:	New → Incomplete

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-06-17:

Looks vulnerable to me (contributed backups API extension)

Changed in cinder:
status:	New → Confirmed

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-06-20:

Adding nova-core and cinder-core for help patching

Changed in ossa:
status:	Incomplete → Confirmed

Thierry Carrez (ttx) on 2013-06-21

Changed in ossa:
importance:	Undecided → Medium

Revision history for this message

Daniel Berrange (berrange) wrote on 2013-06-21:

Any patches should also include test case which validates that no code in GIT uses the vulnerable APIs, so we don't regress in this yet again.

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-06-21:

We should push a minimal patch to fix the vulnerabilities first, that we can backport to stable branches.
Then for Havana and beyond we should push defusedxml parsers across the board.

Thierry Carrez (ttx) on 2013-07-03

Changed in neutron:
status:	Incomplete → Invalid

Revision history for this message

Michael Still (mikal) wrote on 2013-07-04:

nova.patch Edit (9.0 KiB, text/plain)

A patch for nova is attached.

Changed in nova:
importance:	Undecided → High
assignee:	nobody → Michael Still (mikalstill)

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-07-09:

Could we have a patch for Cinder ?

Changed in nova:
status:	Confirmed → In Progress

John Griffith (john-griffith) on 2013-07-12

Changed in cinder:
assignee:	nobody → John Griffith (john-griffith)
importance:	Undecided → High

Revision history for this message

John Griffith (john-griffith) wrote on 2013-07-12:

1190229-cinder.patch Edit (2.5 KiB, text/plain)

cinder patch here

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-07-15:

#10

Looks good, could we have a cinder-core pre-approval here ? Patch is trivial so one should be enough

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-07-15:

#11

We need nova-core pre-approval on Mikal's patch too.

Revision history for this message

Pádraig Brady (p-draigbrady) wrote on 2013-07-15:

#12

+2 On nova patch

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-07-15:

#13

Proposed impact description:

Title: Denial of Service using XML entities in some Nova and Cinder extensions
Reporter: Grant Murphy (Red Hat)
Products: Nova, Cinder
Affects: Grizzly

Description:
Grant Murphy from Red Hat reported that vulnerabilities in XML request parsers were not fully patched in OSSA 2013-004. By leveraging XML entity expansion in specific extensions, an unauthenticated attacker may still consume excessive resources on the Nova or Cinder API servers, resulting in a denial of service and potentially a crash. Only Nova setups making use of the security group extension in Grizzly are affected. Only Cinder setups making use of the backups or volume transfer API extension in Grizzly are affected.

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-07-15:

#14

The affected code was present in Grizzly but not in Folsom.

Changed in ossa:
assignee:	nobody → Thierry Carrez (ttx)
status:	Confirmed → Triaged

Revision history for this message

Jeremy Stanley (fungi) wrote on 2013-07-15:

#15

Thierry's impact description in comment #14 looks sane to me.

Revision history for this message

Jeremy Stanley (fungi) wrote on 2013-07-15:

#16

Sorry, Thierry's impact description in comment #13 looks sane to me.

Revision history for this message

Mike Perez (thingee) wrote on 2013-07-15:

#17

+2 on the cinder patch.

Thierry Carrez (ttx) on 2013-07-17

Changed in ossa:
status:	Triaged → In Progress

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-07-19:

#18

CVE requested, no answer from Kurt yet

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-07-29:

#19

Sent CVE request again

Thierry Carrez (ttx) on 2013-07-30

summary:

- Potential unsafe XML usage
+ Potential unsafe XML usage (CVE-2013-4179)

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-08-01: Re: Potential unsafe XML usage (CVE-2013-4179)

#20

nova-havana.patch Edit (9.1 KiB, text/plain)

Refreshed nova/havana patch

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-08-01:

#21

nova-grizzly.patch Edit (6.4 KiB, text/plain)

nova/grizzly patch backport

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-08-01:

#22

cinder-grizzly.patch Edit (1.3 KiB, text/plain)

cinder/grizzly patch backport

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-08-01:

#23

cinder/master/gate-devstack-vm-full: PASS
cinder/grizzly/gate-devstack-vm-cinder: PASS

more tests tomorrow.

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-08-02:

#24

Nova tests fail with:

File "/opt/stack/new/nova/nova/api/openstack/compute/contrib/security_groups.py", line 33, in <module>
from nova.openstack.common import xmlutils
ImportError: cannot import name xmlutils

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-08-02:

#25

nova-grizzly.patch Edit (9.5 KiB, text/plain)

missing a file in the grizzly backport. New version attached

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-08-02:

#26

nova/master/gate-devstack-vm-full: PASS
nova/grizzly/gate-devstack-vm-full: PASS

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-08-02:

#27

nova-grizzly.patch Edit (9.5 KiB, text/plain)

Version that doesn't fail pep8

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-08-02:

#28

nova-havana.patch Edit (9.1 KiB, text/plain)

Version that doesn't fail pep8

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-08-02:

#29

cinder/master/py27: PASS
cinder/master/pep8: PASS
nova/grizzly/py27: PASS
nova/grizzly/pep8: PASS
nova/master/py27: PASS
nova/master/pep8: PASS

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-08-02:

#30

Sent to downstream stakeholders
Proposed public disclosure date/time: Thursday, August 8, 1500UTC.

Changed in ossa:
status:	In Progress → Fix Committed

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-08-04:

#31

From Kurt:

Please use CVE-2013-4179 for Denial of Service using XML entities in
Nova extensions

Please use CVE-2013-4202 for Denial of Service using XML entities in
Cinder extensions

summary:

- Potential unsafe XML usage (CVE-2013-4179)
+ Potential unsafe XML usage (CVE-2013-4179, CVE-2013-4202)

Thierry Carrez (ttx) on 2013-08-08

information type:

Private Security → Public Security

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2013-08-08: Fix proposed to cinder (master)

#32

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

Changed in cinder:
assignee:	John Griffith (john-griffith) → Thierry Carrez (ttx)
status:	Confirmed → In Progress

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2013-08-08: Fix proposed to cinder (stable/grizzly)

#33

Fix proposed to branch: stable/grizzly
Review: https://review.openstack.org/40883

Thierry Carrez (ttx) on 2013-08-08

summary:

- Potential unsafe XML usage (CVE-2013-4179, CVE-2013-4202)
+ [OSSA 2013-023] Potential unsafe XML usage (CVE-2013-4179,
+ CVE-2013-4202)

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2013-08-08: Fix merged to cinder (master)

#34

Reviewed: https://review.openstack.org/40881
Committed: http://github.com/openstack/cinder/commit/4ad95dba4fccbbc0df923dea0dc9e5c3ac9f4cc2
Submitter: Jenkins
Branch: master

commit 4ad95dba4fccbbc0df923dea0dc9e5c3ac9f4cc2
Author: Thierry Carrez <email address hidden>
Date: Thu Aug 8 12:13:52 2013 +0200

Use utils.safe_minidom_parse_string in extensions

Use utils.safe_minidom_parse_string in extensions that were still
using potentially-unsafe minidom.

Fixes bug 1190229

Change-Id: I43afb2e188bbea99ea30fe6cb2eb1aeedc4ddfd4

Changed in cinder:
status:	In Progress → Fix Committed

Revision history for this message

Thierry Carrez (ttx) wrote on 2013-08-08:

#35

OSSA 2013-023

Changed in nova:
status:	In Progress → Fix Committed

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2013-08-08: Fix merged to cinder (stable/grizzly)

#36

Reviewed: https://review.openstack.org/40883
Committed: http://github.com/openstack/cinder/commit/2023eecc4b1a35daf42a64fa01967ed12c7d017b
Submitter: Jenkins
Branch: stable/grizzly

commit 2023eecc4b1a35daf42a64fa01967ed12c7d017b
Author: John Griffith <email address hidden>
Date: Thu Aug 8 12:10:30 2013 +0200

Use utils.safe_minidom_parse_string in extensions

Use utils.safe_minidom_parse_string in extensions that were still
directly using potentially-unsafe minidom.

Grizzly backport of John Griffith's master patch.

Fixes bug 1190229

Change-Id: I43afb2e188bbea99ea30fe6cb2eb1aeedc4ddfd4

Thierry Carrez (ttx) on 2013-08-08

Changed in ossa:
status:	Fix Committed → Fix Released

Thierry Carrez (ttx) on 2013-09-05

Changed in cinder:
milestone:	none → havana-3
status:	Fix Committed → Fix Released

Thierry Carrez (ttx) on 2013-09-05

Changed in nova:
milestone:	none → havana-3
status:	Fix Committed → Fix Released

Revision history for this message

Yang Yu (yuyangbj) wrote on 2013-09-06: I will be on vacation from 9/5 to 9/15, urgent call: 13811509950

#37

I will be out of the office starting 2013-09-05 and will not return until
2013-09-15.

I will be on my marriage leave from 9/5 to 9/15, for any urgent issue
please call me before 9/7.

For daily work, please ask my scrum master Zhu Zhu for help.
For glance issue, please ask glance SME Feilong Wang for help.
For defect report, there will be no report next week.

Thierry Carrez (ttx) on 2013-10-17