19 files changed, 233 insertions, 192 deletions

diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py
index c6473a648..6e3d7eabc 100644
--- a/nova/api/openstack/common.py
+++ b/nova/api/openstack/common.py
@@ -31,7 +31,6 @@ from nova.compute import vm_states
 from nova import exception
 from nova.openstack.common import log as logging
 from nova import quota
-from nova import utils
 
 osapi_opts = [
     cfg.IntOpt('osapi_max_limit',
@@ -356,7 +355,7 @@ def raise_http_conflict_for_instance_invalid_state(exc, action):
 
 class MetadataDeserializer(wsgi.MetadataXMLDeserializer):
     def deserialize(self, text):
-        dom = utils.safe_minidom_parse_string(text)
+        dom = xmlutil.safe_minidom_parse_string(text)
         metadata_node = self.find_first_child_named(dom, "metadata")
         metadata = self.extract_metadata(metadata_node)
         return {'body': {'metadata': metadata}}
@@ -364,7 +363,7 @@ class MetadataDeserializer(wsgi.MetadataXMLDeserializer):
 
 class MetaItemDeserializer(wsgi.MetadataXMLDeserializer):
     def deserialize(self, text):
-        dom = utils.safe_minidom_parse_string(text)
+        dom = xmlutil.safe_minidom_parse_string(text)
         metadata_item = self.extract_metadata(dom)
         return {'body': {'meta': metadata_item}}
 
@@ -382,7 +381,7 @@ class MetadataXMLDeserializer(wsgi.XMLDeserializer):
         return metadata
 
     def _extract_metadata_container(self, datastring):
-        dom = utils.safe_minidom_parse_string(datastring)
+        dom = xmlutil.safe_minidom_parse_string(datastring)
         metadata_node = self.find_first_child_named(dom, "metadata")
         metadata = self.extract_metadata(metadata_node)
         return {'body': {'metadata': metadata}}
@@ -394,7 +393,7 @@ class MetadataXMLDeserializer(wsgi.XMLDeserializer):
         return self._extract_metadata_container(datastring)
 
     def update(self, datastring):
-        dom = utils.safe_minidom_parse_string(datastring)
+        dom = xmlutil.safe_minidom_parse_string(datastring)
         metadata_item = self.extract_metadata(dom)
         return {'body': {'meta': metadata_item}}
 
diff --git a/nova/api/openstack/compute/contrib/cells.py b/nova/api/openstack/compute/contrib/cells.py
index efd2cd189..03597ff0e 100644
--- a/nova/api/openstack/compute/contrib/cells.py
+++ b/nova/api/openstack/compute/contrib/cells.py
@@ -19,7 +19,6 @@
 
 from oslo.config import cfg
 from webob import exc
-from xml.parsers import expat
 
 from nova.api.openstack import common
 from nova.api.openstack import extensions
@@ -31,7 +30,6 @@ from nova import db
 from nova import exception
 from nova.openstack.common import log as logging
 from nova.openstack.common import timeutils
-from nova import utils
 
 
 LOG = logging.getLogger(__name__)
@@ -98,11 +96,7 @@ class CellDeserializer(wsgi.XMLDeserializer):
 
     def default(self, string):
         """Deserialize an xml-formatted cell create request."""
-        try:
-            node = utils.safe_minidom_parse_string(string)
-        except expat.ExpatError:
-            msg = _("cannot understand XML")
-            raise exception.MalformedRequestBody(reason=msg)
+        node = xmlutil.safe_minidom_parse_string(string)
 
         return {'body': {'cell': self._extract_cell(node)}}
 
diff --git a/nova/api/openstack/compute/contrib/hosts.py b/nova/api/openstack/compute/contrib/hosts.py
index 3ecfb9965..a3b3538fd 100644
--- a/nova/api/openstack/compute/contrib/hosts.py
+++ b/nova/api/openstack/compute/contrib/hosts.py
@@ -16,7 +16,6 @@
 """The hosts admin extension."""
 
 import webob.exc
-from xml.parsers import expat
 
 from nova.api.openstack import extensions
 from nova.api.openstack import wsgi
@@ -24,7 +23,6 @@ from nova.api.openstack import xmlutil
 from nova import compute
 from nova import exception
 from nova.openstack.common import log as logging
-from nova import utils
 
 LOG = logging.getLogger(__name__)
 authorize = extensions.extension_authorizer('compute', 'hosts')
@@ -71,11 +69,7 @@ class HostShowTemplate(xmlutil.TemplateBuilder):
 
 class HostUpdateDeserializer(wsgi.XMLDeserializer):
     def default(self, string):
-        try:
-            node = utils.safe_minidom_parse_string(string)
-        except expat.ExpatError:
-            msg = _("cannot understand XML")
-            raise exception.MalformedRequestBody(reason=msg)
+        node = xmlutil.safe_minidom_parse_string(string)
 
         updates = {}
         updates_node = self.find_first_child_named(node, 'updates')
diff --git a/nova/api/openstack/compute/contrib/security_groups.py b/nova/api/openstack/compute/contrib/security_groups.py
index af97a2a6b..ce6f2687f 100644
--- a/nova/api/openstack/compute/contrib/security_groups.py
+++ b/nova/api/openstack/compute/contrib/security_groups.py
@@ -32,7 +32,6 @@ from nova import exception
 from nova.network.security_group import openstack_driver
 from nova.network.security_group import quantum_driver
 from nova.openstack.common import log as logging
-from nova import utils
 from nova.virt import netutils
 
 
@@ -113,7 +112,7 @@ class SecurityGroupXMLDeserializer(wsgi.MetadataXMLDeserializer):
     """
     def default(self, string):
         """Deserialize an xml-formatted security group create request."""
-        dom = utils.safe_minidom_parse_string(string)
+        dom = xmlutil.safe_minidom_parse_string(string)
         security_group = {}
         sg_node = self.find_first_child_named(dom,
                                                'security_group')
@@ -134,7 +133,7 @@ class SecurityGroupRulesXMLDeserializer(wsgi.MetadataXMLDeserializer):
 
     def default(self, string):
         """Deserialize an xml-formatted security group create request."""
-        dom = utils.safe_minidom_parse_string(string)
+        dom = xmlutil.safe_minidom_parse_string(string)
         security_group_rule = self._extract_security_group_rule(dom)
         return {'body': {'security_group_rule': security_group_rule}}
 
diff --git a/nova/api/openstack/compute/contrib/volumes.py b/nova/api/openstack/compute/contrib/volumes.py
index 760dc953a..93d76495f 100644
--- a/nova/api/openstack/compute/contrib/volumes.py
+++ b/nova/api/openstack/compute/contrib/volumes.py
@@ -154,7 +154,7 @@ class CreateDeserializer(CommonDeserializer):
 
     def default(self, string):
         """Deserialize an xml-formatted volume create request."""
-        dom = utils.safe_minidom_parse_string(string)
+        dom = xmlutil.safe_minidom_parse_string(string)
         vol = self._extract_volume(dom)
         return {'body': {'volume': vol}}
 
diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py
index 00aa35538..ce40e087b 100644
--- a/nova/api/openstack/compute/servers.py
+++ b/nova/api/openstack/compute/servers.py
@@ -317,7 +317,7 @@ class ActionDeserializer(CommonDeserializer):
     """
 
     def default(self, string):
-        dom = utils.safe_minidom_parse_string(string)
+        dom = xmlutil.safe_minidom_parse_string(string)
         action_node = dom.childNodes[0]
         action_name = action_node.tagName
 
@@ -424,7 +424,7 @@ class CreateDeserializer(CommonDeserializer):
 
     def default(self, string):
         """Deserialize an xml-formatted server create request."""
-        dom = utils.safe_minidom_parse_string(string)
+        dom = xmlutil.safe_minidom_parse_string(string)
         server = self._extract_server(dom)
         return {'body': {'server': server}}
 
diff --git a/nova/api/openstack/wsgi.py b/nova/api/openstack/wsgi.py
index 5b9900f72..88b203a33 100644
--- a/nova/api/openstack/wsgi.py
+++ b/nova/api/openstack/wsgi.py
@@ -19,15 +19,14 @@ import inspect
 import math
 import time
 from xml.dom import minidom
-from xml.parsers import expat
 
 from lxml import etree
 import webob
 
+from nova.api.openstack import xmlutil
 from nova import exception
 from nova.openstack.common import jsonutils
 from nova.openstack.common import log as logging
-from nova import utils
 from nova import wsgi
 
 
@@ -216,13 +215,8 @@ class XMLDeserializer(TextDeserializer):
 
     def _from_xml(self, datastring):
         plurals = set(self.metadata.get('plurals', {}))
-
-        try:
-            node = utils.safe_minidom_parse_string(datastring).childNodes[0]
-            return {node.nodeName: self._from_xml_node(node, plurals)}
-        except expat.ExpatError:
-            msg = _("cannot understand XML")
-            raise exception.MalformedRequestBody(reason=msg)
+        node = xmlutil.safe_minidom_parse_string(datastring).childNodes[0]
+        return {node.nodeName: self._from_xml_node(node, plurals)}
 
     def _from_xml_node(self, node, listnames):
         """Convert a minidom node to a simple Python type.
@@ -634,7 +628,7 @@ def action_peek_json(body):
 def action_peek_xml(body):
     """Determine action to invoke."""
 
-    dom = utils.safe_minidom_parse_string(body)
+    dom = xmlutil.safe_minidom_parse_string(body)
     action_node = dom.childNodes[0]
 
     return action_node.tagName
@@ -890,17 +884,8 @@ class Resource(wsgi.Application):
         #            function.  If we try to audit __call__(), we can
         #            run into troubles due to the @webob.dec.wsgify()
         #            decorator.
-        try:
-            return self._process_stack(request, action, action_args,
-                                   content_type, body, accept)
-        except expat.ExpatError:
-            msg = _("Invalid XML in request body")
-            return Fault(webob.exc.HTTPBadRequest(explanation=msg))
-        except LookupError as e:
-        #NOTE(Vijaya Erukala): XML input such as
-        #                      <?xml version="1.0" encoding="TF-8"?>
-        #                      raises LookupError: unknown encoding: TF-8
-            return Fault(webob.exc.HTTPBadRequest(explanation=unicode(e)))
+        return self._process_stack(request, action, action_args,
+                               content_type, body, accept)
 
     def _process_stack(self, request, action, action_args,
                        content_type, body, accept):
diff --git a/nova/api/openstack/xmlutil.py b/nova/api/openstack/xmlutil.py
index a2f5b7506..9bcce808c 100644
--- a/nova/api/openstack/xmlutil.py
+++ b/nova/api/openstack/xmlutil.py
@@ -18,7 +18,12 @@
 import os.path
 
 from lxml import etree
+from xml.dom import minidom
+from xml.parsers import expat
+from xml import sax
+from xml.sax import expatreader
 
+from nova import exception
 from nova import utils
 
 
@@ -905,3 +910,59 @@ def make_flat_dict(name, selector=None, subselector=None, ns=None):
 
     # Return the template
     return root
+
+
+class ProtectedExpatParser(expatreader.ExpatParser):
+    """An expat parser which disables DTD's and entities by default."""
+
+    def __init__(self, forbid_dtd=True, forbid_entities=True,
+                 *args, **kwargs):
+        # Python 2.x old style class
+        expatreader.ExpatParser.__init__(self, *args, **kwargs)
+        self.forbid_dtd = forbid_dtd
+        self.forbid_entities = forbid_entities
+
+    def start_doctype_decl(self, name, sysid, pubid, has_internal_subset):
+        raise ValueError("Inline DTD forbidden")
+
+    def entity_decl(self, entityName, is_parameter_entity, value, base,
+                    systemId, publicId, notationName):
+        raise ValueError("<!ENTITY> entity declaration forbidden")
+
+    def unparsed_entity_decl(self, name, base, sysid, pubid, notation_name):
+        # expat 1.2
+        raise ValueError("<!ENTITY> unparsed entity forbidden")
+
+    def external_entity_ref(self, context, base, systemId, publicId):
+        raise ValueError("<!ENTITY> external entity forbidden")
+
+    def notation_decl(self, name, base, sysid, pubid):
+        raise ValueError("<!ENTITY> notation forbidden")
+
+    def reset(self):
+        expatreader.ExpatParser.reset(self)
+        if self.forbid_dtd:
+            self._parser.StartDoctypeDeclHandler = self.start_doctype_decl
+            self._parser.EndDoctypeDeclHandler = None
+        if self.forbid_entities:
+            self._parser.EntityDeclHandler = self.entity_decl
+            self._parser.UnparsedEntityDeclHandler = self.unparsed_entity_decl
+            self._parser.ExternalEntityRefHandler = self.external_entity_ref
+            self._parser.NotationDeclHandler = self.notation_decl
+            try:
+                self._parser.SkippedEntityHandler = None
+            except AttributeError:
+                # some pyexpat versions do not support SkippedEntity
+                pass
+
+
+def safe_minidom_parse_string(xml_string):
+    """Parse an XML string using minidom safely."""
+    try:
+        return minidom.parseString(xml_string, parser=ProtectedExpatParser())
+    except (sax.SAXParseException, ValueError,
+            expat.ExpatError, LookupError) as e:
+        #NOTE(Vijaya Erukala): XML input such as
+        #                      <?xml version="1.0" encoding="TF-8"?>
+        #                      raises LookupError: unknown encoding: TF-8
+        raise exception.MalformedRequestBody(reason=str(e))
diff --git a/nova/tests/api/openstack/compute/contrib/test_cells.py b/nova/tests/api/openstack/compute/contrib/test_cells.py
index 89ce4cd5a..bf6bff27c 100644
--- a/nova/tests/api/openstack/compute/contrib/test_cells.py
+++ b/nova/tests/api/openstack/compute/contrib/test_cells.py
@@ -27,6 +27,7 @@ from nova import exception
 from nova.openstack.common import timeutils
 from nova import test
 from nova.tests.api.openstack import fakes
+from nova.tests import utils
 
 
 FAKE_CELLS = [
@@ -394,3 +395,10 @@ class TestCellsXMLDeserializer(test.TestCase):
         deserializer = cells_ext.CellDeserializer()
         result = deserializer.deserialize(intext)
         self.assertEqual(dict(body=expected), result)
+
+    def test_with_corrupt_xml(self):
+        deserializer = cells_ext.CellDeserializer()
+        self.assertRaises(
+                exception.MalformedRequestBody,
+                deserializer.deserialize,
+                utils.killer_xml_body())
diff --git a/nova/tests/api/openstack/compute/contrib/test_hosts.py b/nova/tests/api/openstack/compute/contrib/test_hosts.py
index 85f93a9d5..5678933dc 100644
--- a/nova/tests/api/openstack/compute/contrib/test_hosts.py
+++ b/nova/tests/api/openstack/compute/contrib/test_hosts.py
@@ -21,9 +21,11 @@ from nova.compute import power_state
 from nova.compute import vm_states
 from nova import context as context_maker
 from nova import db
+from nova import exception
 from nova.openstack.common import log as logging
 from nova import test
 from nova.tests import fake_hosts
+from nova.tests import utils
 
 LOG = logging.getLogger(__name__)
 
@@ -390,3 +392,9 @@ class HostSerializerTest(test.TestCase):
         result = self.deserializer.deserialize(intext)
 
         self.assertEqual(dict(body=exemplar), result)
+
+    def test_corrupt_xml(self):
+        self.assertRaises(
+                exception.MalformedRequestBody,
+                self.deserializer.deserialize,
+                utils.killer_xml_body())
diff --git a/nova/tests/api/openstack/compute/contrib/test_security_groups.py b/nova/tests/api/openstack/compute/contrib/test_security_groups.py
index 4919d461f..02aa96956 100644
--- a/nova/tests/api/openstack/compute/contrib/test_security_groups.py
+++ b/nova/tests/api/openstack/compute/contrib/test_security_groups.py
@@ -30,6 +30,7 @@ from nova.openstack.common import jsonutils
 from nova import quota
 from nova import test
 from nova.tests.api.openstack import fakes
+from nova.tests import utils
 
 CONF = cfg.CONF
 FAKE_UUID = 'a47ae74e-ab08-447f-8eee-ffd43fc46c16'
@@ -727,13 +728,6 @@ class TestSecurityGroupRules(test.TestCase):
         self.assertEquals(security_group_rule['to_port'], 81)
 
     def test_create_by_invalid_cidr_json(self):
-        rules = {
-                  "security_group_rule": {
-                        "ip_protocol": "tcp",
-                        "from_port": "22",
-                        "to_port": "22",
-                        "parent_group_id": self.sg2['id'],
-                        "cidr": "10.2.3.124/2433"}}
         rule = security_group_rule_template(
                 ip_protocol="tcp",
                 from_port=22,
@@ -1146,6 +1140,13 @@ class TestSecurityGroupRulesXMLDeserializer(test.TestCase):
         }
         self.assertEquals(request['body'], expected)
 
+    def test_corrupt_xml(self):
+        """Should throw a 400 error on corrupt xml."""
+        self.assertRaises(
+                exception.MalformedRequestBody,
+                self.deserializer.deserialize,
+                utils.killer_xml_body())
+
 
 class TestSecurityGroupXMLDeserializer(test.TestCase):
 
@@ -1192,6 +1193,13 @@ class TestSecurityGroupXMLDeserializer(test.TestCase):
         }
         self.assertEquals(request['body'], expected)
 
+    def test_corrupt_xml(self):
+        """Should throw a 400 error on corrupt xml."""
+        self.assertRaises(
+                exception.MalformedRequestBody,
+                self.deserializer.deserialize,
+                utils.killer_xml_body())
+
 
 class TestSecurityGroupXMLSerializer(test.TestCase):
     def setUp(self):
diff --git a/nova/tests/api/openstack/compute/test_server_actions.py b/nova/tests/api/openstack/compute/test_server_actions.py
index 62a688962..754e103d4 100644
--- a/nova/tests/api/openstack/compute/test_server_actions.py
+++ b/nova/tests/api/openstack/compute/test_server_actions.py
@@ -32,6 +32,7 @@ from nova import test
 from nova.tests.api.openstack import fakes
 from nova.tests.image import fake
 from nova.tests import matchers
+from nova.tests import utils
 
 CONF = cfg.CONF
 CONF.import_opt('password_length', 'nova.utils')
@@ -1146,3 +1147,10 @@ class TestServerActionXMLDeserializer(test.TestCase):
                           self.deserializer.deserialize,
                           serial_request,
                           'action')
+
+    def test_corrupt_xml(self):
+        """Should throw a 400 error on corrupt xml."""
+        self.assertRaises(
+                exception.MalformedRequestBody,
+                self.deserializer.deserialize,
+                utils.killer_xml_body())
diff --git a/nova/tests/api/openstack/compute/test_servers.py b/nova/tests/api/openstack/compute/test_servers.py
index b77814003..2dfefc541 100644
--- a/nova/tests/api/openstack/compute/test_servers.py
+++ b/nova/tests/api/openstack/compute/test_servers.py
@@ -51,6 +51,7 @@ from nova.tests.api.openstack import fakes
 from nova.tests import fake_network
 from nova.tests.image import fake
 from nova.tests import matchers
+from nova.tests import utils
 
 CONF = cfg.CONF
 CONF.import_opt('password_length', 'nova.utils')
@@ -3859,6 +3860,13 @@ class TestServerCreateRequestXMLDeserializer(test.TestCase):
         }
         self.assertEquals(request['body'], expected)
 
+    def test_corrupt_xml(self):
+        """Should throw a 400 error on corrupt xml."""
+        self.assertRaises(
+                exception.MalformedRequestBody,
+                self.deserializer.deserialize,
+                utils.killer_xml_body())
+
 
 class TestAddressesXMLSerialization(test.TestCase):
 
diff --git a/nova/tests/api/openstack/test_common.py b/nova/tests/api/openstack/test_common.py
index 227044572..a6344c09f 100644
--- a/nova/tests/api/openstack/test_common.py
+++ b/nova/tests/api/openstack/test_common.py
@@ -28,7 +28,7 @@ from nova.api.openstack import common
 from nova.api.openstack import xmlutil
 from nova import exception
 from nova import test
-from nova.tests import utils as test_utils
+from nova.tests import utils
 
 
 NS = "{http://docs.openstack.org/compute/api/v1.1}"
@@ -297,7 +297,7 @@ class MiscFunctionsTest(test.TestCase):
             self.fail("webob.exc.HTTPConflict was not raised")
 
     def test_check_img_metadata_properties_quota_valid_metadata(self):
-        ctxt = test_utils.get_test_admin_context()
+        ctxt = utils.get_test_admin_context()
         metadata1 = {"key": "value"}
         actual = common.check_img_metadata_properties_quota(ctxt, metadata1)
         self.assertEqual(actual, None)
@@ -311,7 +311,7 @@ class MiscFunctionsTest(test.TestCase):
         self.assertEqual(actual, None)
 
     def test_check_img_metadata_properties_quota_inv_metadata(self):
-        ctxt = test_utils.get_test_admin_context()
+        ctxt = utils.get_test_admin_context()
         metadata1 = {"a" * 260: "value"}
         self.assertRaises(webob.exc.HTTPBadRequest,
                 common.check_img_metadata_properties_quota, ctxt, metadata1)
@@ -512,3 +512,11 @@ class MetadataXMLSerializationTest(test.TestCase):
         """.replace("  ", "").replace("\n", ""))
 
         self.assertEqual(expected.toxml(), actual.toxml())
+
+    def test_metadata_deserializer(self):
+        """Should throw a 400 error on corrupt xml."""
+        deserializer = common.MetadataXMLDeserializer()
+        self.assertRaises(
+                exception.MalformedRequestBody,
+                deserializer.deserialize,
+                utils.killer_xml_body())
diff --git a/nova/tests/api/openstack/test_wsgi.py b/nova/tests/api/openstack/test_wsgi.py
index a18dc78d5..374aa1162 100644
--- a/nova/tests/api/openstack/test_wsgi.py
+++ b/nova/tests/api/openstack/test_wsgi.py
@@ -7,6 +7,7 @@ from nova.api.openstack import wsgi
 from nova import exception
 from nova import test
 from nova.tests.api.openstack import fakes
+from nova.tests import utils
 
 
 class RequestTest(test.TestCase):
@@ -272,6 +273,21 @@ class ResourceTest(test.TestCase):
                                                  '<fooAction>true</fooAction>')
         self.assertEqual(controller._action_foo, method)
 
+    def test_get_method_action_corrupt_xml(self):
+        class Controller(wsgi.Controller):
+            @wsgi.action('fooAction')
+            def _action_foo(self, req, id, body):
+                return body
+
+        controller = Controller()
+        resource = wsgi.Resource(controller)
+        self.assertRaises(
+                exception.MalformedRequestBody,
+                resource.get_method,
+                None, 'action',
+                'application/xml',
+                utils.killer_xml_body())
+
     def test_get_method_action_bad_body(self):
         class Controller(wsgi.Controller):
             @wsgi.action('fooAction')
diff --git a/nova/tests/api/openstack/test_xmlutil.py b/nova/tests/api/openstack/test_xmlutil.py
index bd7f24233..3ed6a86fc 100644
--- a/nova/tests/api/openstack/test_xmlutil.py
+++ b/nova/tests/api/openstack/test_xmlutil.py
@@ -16,9 +16,12 @@
 #    under the License.
 
 from lxml import etree
+from xml.dom import minidom
 
 from nova.api.openstack import xmlutil
+from nova import exception
 from nova import test
+from nova.tests import utils as tests_utils
 
 
 class SelectorTest(test.TestCase):
@@ -720,3 +723,64 @@ class MiscellaneousXMLUtilTests(test.TestCase):
         tmpl = xmlutil.MasterTemplate(root, 1)
         result = tmpl.serialize(dict(wrapper=dict(a='foo', b='bar')))
         self.assertEqual(result, expected_xml)
+
+    def test_safe_parse_xml(self):
+
+        normal_body = ("""
+                 <?xml version="1.0" ?><foo>
+                    <bar>
+                        <v1>hey</v1>
+                        <v2>there</v2>
+                    </bar>
+                </foo>""").strip()
+
+        dom = xmlutil.safe_minidom_parse_string(normal_body)
+        self.assertEqual(normal_body, str(dom.toxml()))
+
+        self.assertRaises(exception.MalformedRequestBody,
+                          xmlutil.safe_minidom_parse_string,
+                          tests_utils.killer_xml_body())
+
+
+class SafeParserTestCase(test.TestCase):
+    def test_external_dtd(self):
+        xml_string = ("""<?xml version="1.0" encoding="utf-8"?>
+                <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
+                 "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
+                <html>
+                   <head/>
+                     <body>html with dtd</body>
+                   </html>""")
+
+        parser = xmlutil.ProtectedExpatParser(forbid_dtd=False,
+                                            forbid_entities=True)
+        self.assertRaises(ValueError,
+                          minidom.parseString,
+                          xml_string, parser)
+
+    def test_external_file(self):
+        xml_string = """<!DOCTYPE external [
+                <!ENTITY ee SYSTEM "file:///PATH/TO/root.xml">
+                ]>
+                <root>&ee;</root>"""
+
+        parser = xmlutil.ProtectedExpatParser(forbid_dtd=False,
+                                            forbid_entities=True)
+        self.assertRaises(ValueError,
+                          minidom.parseString,
+                          xml_string, parser)
+
+    def test_notation(self):
+        xml_string = """<?xml version="1.0" standalone="no"?>
+                        <!-- comment data -->
+                        <!DOCTYPE x [
+                        <!NOTATION notation SYSTEM "notation.jpeg">
+                        ]>
+                        <root attr1="value1">
+                        </root>"""
+
+        parser = xmlutil.ProtectedExpatParser(forbid_dtd=False,
+                                            forbid_entities=True)
+        self.assertRaises(ValueError,
+                          minidom.parseString,
+                          xml_string, parser)
diff --git a/nova/tests/test_utils.py b/nova/tests/test_utils.py
index c601bb0af..af6a9b9aa 100644
--- a/nova/tests/test_utils.py
+++ b/nova/tests/test_utils.py
@@ -23,7 +23,6 @@ import os
 import os.path
 import StringIO
 import tempfile
-from xml.dom import minidom
 
 import mox
 import netaddr
@@ -450,39 +449,6 @@ class GenericUtilsTestCase(test.TestCase):
                 self.assertEqual(fake_execute.uid, 2)
             self.assertEqual(fake_execute.uid, os.getuid())
 
-    def test_safe_parse_xml(self):
-
-        normal_body = ("""
-                 <?xml version="1.0" ?><foo>
-                    <bar>
-                        <v1>hey</v1>
-                        <v2>there</v2>
-                    </bar>
-                </foo>""").strip()
-
-        def killer_body():
-            return (("""<!DOCTYPE x [
-                    <!ENTITY a "%(a)s">
-                    <!ENTITY b "%(b)s">
-                    <!ENTITY c "%(c)s">]>
-                <foo>
-                    <bar>
-                        <v1>%(d)s</v1>
-                    </bar>
-                </foo>""") % {
-                'a': 'A' * 10,
-                'b': '&a;' * 10,
-                'c': '&b;' * 10,
-                'd': '&c;' * 9999,
-            }).strip()
-
-        dom = utils.safe_minidom_parse_string(normal_body)
-        self.assertEqual(normal_body, str(dom.toxml()))
-
-        self.assertRaises(ValueError,
-                          utils.safe_minidom_parse_string,
-                          killer_body())
-
     def test_xhtml_escape(self):
         self.assertEqual('&quot;foo&quot;', utils.xhtml_escape('"foo"'))
         self.assertEqual('&apos;foo&apos;', utils.xhtml_escape("'foo'"))
@@ -992,47 +958,3 @@ class StringLengthTestCase(test.TestCase):
         self.assertRaises(exception.InvalidInput,
                           utils.check_string_length,
                           'a' * 256, 'name', max_length=255)
-
-
-class SafeParserTestCase(test.TestCase):
-    def test_external_dtd(self):
-        xml_string = ("""<?xml version="1.0" encoding="utf-8"?>
-                <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
-                 "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
-                <html>
-                   <head/>
-                     <body>html with dtd</body>
-                   </html>""")
-
-        parser = utils.ProtectedExpatParser(forbid_dtd=False,
-                                            forbid_entities=True)
-        self.assertRaises(ValueError,
-                          minidom.parseString,
-                          xml_string, parser)
-
-    def test_external_file(self):
-        xml_string = """<!DOCTYPE external [
-                <!ENTITY ee SYSTEM "file:///PATH/TO/root.xml">
-                ]>
-                <root>&ee;</root>"""
-
-        parser = utils.ProtectedExpatParser(forbid_dtd=False,
-                                            forbid_entities=True)
-        self.assertRaises(ValueError,
-                          minidom.parseString,
-                          xml_string, parser)
-
-    def test_notation(self):
-        xml_string = """<?xml version="1.0" standalone="no"?>
-                        <!-- comment data -->
-                        <!DOCTYPE x [
-                        <!NOTATION notation SYSTEM "notation.jpeg">
-                        ]>
-                        <root attr1="value1">
-                        </root>"""
-
-        parser = utils.ProtectedExpatParser(forbid_dtd=False,
-                                            forbid_entities=True)
-        self.assertRaises(ValueError,
-                          minidom.parseString,
-                          xml_string, parser)
diff --git a/nova/tests/utils.py b/nova/tests/utils.py
index 755d49be1..e9248c7b7 100644
--- a/nova/tests/utils.py
+++ b/nova/tests/utils.py
@@ -183,3 +183,20 @@ def cleanup_dns_managers():
     for manager in test_dns_managers:
         manager.delete_dns_file()
     test_dns_managers = []
+
+
+def killer_xml_body():
+    return (("""<!DOCTYPE x [
+            <!ENTITY a "%(a)s">
+            <!ENTITY b "%(b)s">
+            <!ENTITY c "%(c)s">]>
+        <foo>
+            <bar>
+                <v1>%(d)s</v1>
+            </bar>
+        </foo>""") % {
+        'a': 'A' * 10,
+        'b': '&a;' * 10,
+        'c': '&b;' * 10,
+        'd': '&c;' * 9999,
+    }).strip()
diff --git a/nova/utils.py b/nova/utils.py
index fe6c75df3..dbbbd1eb6 100644
--- a/nova/utils.py
+++ b/nova/utils.py
@@ -36,10 +36,6 @@ import struct
 import sys
 import tempfile
 import time
-from xml.dom import minidom
-from xml.parsers import expat
-from xml import sax
-from xml.sax import expatreader
 from xml.sax import saxutils
 
 from eventlet import event
@@ -657,60 +653,6 @@ class DynamicLoopingCall(LoopingCallBase):
         return self.done
 
 
-class ProtectedExpatParser(expatreader.ExpatParser):
-    """An expat parser which disables DTD's and entities by default."""
-
-    def __init__(self, forbid_dtd=True, forbid_entities=True,
-                 *args, **kwargs):
-        # Python 2.x old style class
-        expatreader.ExpatParser.__init__(self, *args, **kwargs)
-        self.forbid_dtd = forbid_dtd
-        self.forbid_entities = forbid_entities
-
-    def start_doctype_decl(self, name, sysid, pubid, has_internal_subset):
-        raise ValueError("Inline DTD forbidden")
-
-    def entity_decl(self, entityName, is_parameter_entity, value, base,
-                    systemId, publicId, notationName):
-        raise ValueError("<!ENTITY> entity declaration forbidden")
-
-    def unparsed_entity_decl(self, name, base, sysid, pubid, notation_name):
-        # expat 1.2
-        raise ValueError("<!ENTITY> unparsed entity forbidden")
-
-    def external_entity_ref(self, context, base, systemId, publicId):
-        raise ValueError("<!ENTITY> external entity forbidden")
-
-    def notation_decl(self, name, base, sysid, pubid):
-        raise ValueError("<!ENTITY> notation forbidden")
-
-    def reset(self):
-        expatreader.ExpatParser.reset(self)
-        if self.forbid_dtd:
-            self._parser.StartDoctypeDeclHandler = self.start_doctype_decl
-            self._parser.EndDoctypeDeclHandler = None
-        if self.forbid_entities:
-            self._parser.EntityDeclHandler = self.entity_decl
-            self._parser.UnparsedEntityDeclHandler = self.unparsed_entity_decl
-            self._parser.ExternalEntityRefHandler = self.external_entity_ref
-            self._parser.NotationDeclHandler = self.notation_decl
-            try:
-                self._parser.SkippedEntityHandler = None
-            except AttributeError:
-                # some pyexpat versions do not support SkippedEntity
-                pass
-
-
-def safe_minidom_parse_string(xml_string):
-    """Parse an XML string using minidom safely.
-
-    """
-    try:
-        return minidom.parseString(xml_string, parser=ProtectedExpatParser())
-    except sax.SAXParseException as se:
-        raise expat.ExpatError()
-
-
 def xhtml_escape(value):
     """Escapes a string so it is valid within XML or XHTML.