mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-09 19:56:48 +00:00
feat(security): harden storage uploads and vrtManager snapshot XML handling
This commit is contained in:
parent
b3e6fb2b7a
commit
5e240236c3
5 changed files with 389 additions and 104 deletions
|
|
@ -1265,22 +1265,26 @@ class wvmInstance(wvmConnect):
|
|||
|
||||
def create_snapshot(self, name, desc=None):
|
||||
state = "shutoff" if self.get_status() == 5 else "running"
|
||||
name_esc = util.xml_escape(name)
|
||||
desc_esc = util.xml_escape(desc) if desc else ""
|
||||
xml = """<domainsnapshot>
|
||||
<name>%s</name>
|
||||
<description>%s</description>
|
||||
<state>%s</state>
|
||||
<creationTime>%d</creationTime>""" % (
|
||||
name,
|
||||
desc,
|
||||
name_esc,
|
||||
desc_esc,
|
||||
state,
|
||||
time.time(),
|
||||
)
|
||||
self.change_snapshot_xml()
|
||||
xml += self._XMLDesc(VIR_DOMAIN_XML_SECURE)
|
||||
xml += """<active>0</active>
|
||||
</domainsnapshot>"""
|
||||
self._snapshotCreateXML(xml, 0)
|
||||
self.recover_snapshot_xml()
|
||||
try:
|
||||
xml += self._XMLDesc(VIR_DOMAIN_XML_SECURE)
|
||||
xml += """<active>0</active>
|
||||
</domainsnapshot>"""
|
||||
self._snapshotCreateXML(xml, 0)
|
||||
finally:
|
||||
self.recover_snapshot_xml()
|
||||
|
||||
def change_snapshot_xml(self):
|
||||
xml_temp = self._XMLDesc(VIR_DOMAIN_XML_SECURE).replace(
|
||||
|
|
@ -1299,6 +1303,8 @@ class wvmInstance(wvmConnect):
|
|||
def create_external_snapshot(self, name, date=None, desc=None):
|
||||
creation_time = time.time()
|
||||
state = "shutoff" if self.get_status() == 5 else "running"
|
||||
name_esc = util.xml_escape(name)
|
||||
desc_esc = util.xml_escape(desc) if desc else ""
|
||||
#<seclabel type='none' model='dac' relabel='no'/>
|
||||
xml = """<domainsnapshot>
|
||||
<name>%s</name>
|
||||
|
|
@ -1306,18 +1312,21 @@ class wvmInstance(wvmConnect):
|
|||
<state>%s</state>
|
||||
<creationTime>%d</creationTime>
|
||||
""" % (
|
||||
name,
|
||||
desc,
|
||||
name_esc,
|
||||
desc_esc,
|
||||
state,
|
||||
creation_time,
|
||||
)
|
||||
|
||||
self.change_snapshot_xml()
|
||||
xml += self._XMLDesc(VIR_DOMAIN_XML_SECURE)
|
||||
xml += """<active>0</active>
|
||||
</domainsnapshot>"""
|
||||
try:
|
||||
xml += self._XMLDesc(VIR_DOMAIN_XML_SECURE)
|
||||
xml += """<active>0</active>
|
||||
</domainsnapshot>"""
|
||||
|
||||
self._snapshotCreateXML(xml, VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY)
|
||||
self._snapshotCreateXML(xml, VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY)
|
||||
finally:
|
||||
self.recover_snapshot_xml()
|
||||
self.refresh_instance_pools()
|
||||
|
||||
def get_external_snapshots(self):
|
||||
|
|
@ -1394,9 +1403,11 @@ class wvmInstance(wvmConnect):
|
|||
|
||||
def snapshot_revert(self, snapshot):
|
||||
self.change_snapshot_xml()
|
||||
snap = self.instance.snapshotLookupByName(snapshot, 0)
|
||||
self.instance.revertToSnapshot(snap, 0)
|
||||
self.recover_snapshot_xml()
|
||||
try:
|
||||
snap = self.instance.snapshotLookupByName(snapshot, 0)
|
||||
self.instance.revertToSnapshot(snap, 0)
|
||||
finally:
|
||||
self.recover_snapshot_xml()
|
||||
|
||||
def get_managed_save_image(self):
|
||||
return self.instance.hasManagedSaveImage(0)
|
||||
|
|
@ -1784,12 +1795,17 @@ class wvmInstance(wvmConnect):
|
|||
return qos_values
|
||||
|
||||
def set_qos(self, mac, direction, average, peak, burst):
|
||||
if direction not in ("inbound", "outbound"):
|
||||
raise ValueError("Direction must be inbound or outbound")
|
||||
|
||||
average = int(average)
|
||||
peak = int(peak)
|
||||
burst = int(burst)
|
||||
|
||||
if direction == "inbound":
|
||||
xml = f"<inbound average='{average}' peak='{peak}' burst='{burst}'/>"
|
||||
elif direction == "outbound":
|
||||
xml = f"<outbound average='{average}' peak='{peak}' burst='{burst}'/>"
|
||||
else:
|
||||
raise Exception("Direction must be inbound or outbound")
|
||||
xml = f"<outbound average='{average}' peak='{peak}' burst='{burst}'/>"
|
||||
|
||||
tree = etree.fromstring(self._XMLDesc(0))
|
||||
|
||||
|
|
|
|||
|
|
@ -69,42 +69,55 @@ class wvmNetworks(wvmConnect):
|
|||
openvswitch,
|
||||
fixed=False,
|
||||
):
|
||||
esc_name = util.xml_escape(str(name))
|
||||
esc_forward = util.xml_escape(str(forward)) if forward else ""
|
||||
esc_bridge = util.xml_escape(str(bridge)) if bridge else ""
|
||||
esc_gateway = util.xml_escape(str(gateway)) if gateway else ""
|
||||
esc_mask = util.xml_escape(str(mask)) if mask else ""
|
||||
esc_gateway6 = util.xml_escape(str(gateway6)) if gateway6 else ""
|
||||
esc_prefix6 = util.xml_escape(str(prefix6)) if prefix6 else ""
|
||||
|
||||
xml = f"""
|
||||
<network>
|
||||
<name>{name}</name>"""
|
||||
<name>{esc_name}</name>"""
|
||||
if forward in ["nat", "route", "bridge"]:
|
||||
xml += f"""<forward mode='{forward}'/>"""
|
||||
xml += f"""<forward mode='{esc_forward}'/>"""
|
||||
if forward == "macvtap":
|
||||
xml += f"""<forward mode='bridge'>
|
||||
<interface dev='{bridge}'/>
|
||||
<interface dev='{esc_bridge}'/>
|
||||
</forward>"""
|
||||
else:
|
||||
xml += """<bridge """
|
||||
if forward in ["nat", "route", "none"]:
|
||||
xml += """stp='on' delay='0'"""
|
||||
if forward == "bridge":
|
||||
xml += f"""name='{bridge}'"""
|
||||
xml += f"""name='{esc_bridge}'"""
|
||||
xml += """/>"""
|
||||
if openvswitch is True:
|
||||
xml += """<virtualport type='openvswitch'/>"""
|
||||
if forward not in ["bridge", "macvtap"]:
|
||||
if ipv4:
|
||||
xml += f"""<ip address='{gateway}' netmask='{mask}'>"""
|
||||
xml += f"""<ip address='{esc_gateway}' netmask='{esc_mask}'>"""
|
||||
if dhcp4:
|
||||
dhcp4_0 = util.xml_escape(str(dhcp4[0]))
|
||||
dhcp4_1 = util.xml_escape(str(dhcp4[1]))
|
||||
xml += f"""<dhcp>
|
||||
<range start='{dhcp4[0]}' end='{dhcp4[1]}' />"""
|
||||
<range start='{dhcp4_0}' end='{dhcp4_1}' />"""
|
||||
if fixed:
|
||||
fist_oct = int(dhcp4[0].strip().split(".")[3])
|
||||
last_oct = int(dhcp4[1].strip().split(".")[3])
|
||||
gw_prefix = util.xml_escape(str(gateway)[:-2])
|
||||
for ip in range(fist_oct, last_oct + 1):
|
||||
xml += f"""<host mac='{util.randomMAC()}' ip='{gateway[:-2]}.{ip}' />"""
|
||||
xml += f"""<host mac='{util.randomMAC()}' ip='{gw_prefix}.{ip}' />"""
|
||||
xml += """</dhcp>"""
|
||||
xml += """</ip>"""
|
||||
if ipv6:
|
||||
xml += f"""<ip family='ipv6' address='{gateway6}' prefix='{prefix6}'>"""
|
||||
xml += f"""<ip family='ipv6' address='{esc_gateway6}' prefix='{esc_prefix6}'>"""
|
||||
if dhcp6:
|
||||
dhcp6_0 = util.xml_escape(str(dhcp6[0]))
|
||||
dhcp6_1 = util.xml_escape(str(dhcp6[1]))
|
||||
xml += f"""<dhcp>
|
||||
<range start='{dhcp6[0]}' end='{dhcp6[1]}' />"""
|
||||
<range start='{dhcp6_0}' end='{dhcp6_1}' />"""
|
||||
xml += """</dhcp>"""
|
||||
xml += """</ip>"""
|
||||
xml += """</network>"""
|
||||
|
|
@ -259,10 +272,15 @@ class wvmNetwork(wvmConnect):
|
|||
parent_index = self.parent_count - 1
|
||||
for h in hosts:
|
||||
if h.get("ip") == ip:
|
||||
h_name = util.xml_escape(h.get("name") or "")
|
||||
name_attr = f' name="{h_name}"' if h.get("name") else ""
|
||||
ip_esc = util.xml_escape(ip)
|
||||
if family == "ipv4":
|
||||
new_xml = f'<host mac="{h.get("mac")}" name="{h.get("name")}" ip="{ip}"/>'
|
||||
h_mac = util.xml_escape(h.get("mac") or "")
|
||||
new_xml = f'<host mac="{h_mac}"{name_attr} ip="{ip_esc}"/>'
|
||||
if family == "ipv6":
|
||||
new_xml = f'<host id="{h.get("id")}" name="{h.get("name")}" ip="{ip}"/>'
|
||||
h_id = util.xml_escape(h.get("id") or "")
|
||||
new_xml = f'<host id="{h_id}"{name_attr} ip="{ip_esc}"/>'
|
||||
|
||||
self.update(
|
||||
VIR_NETWORK_UPDATE_COMMAND_DELETE,
|
||||
|
|
@ -275,13 +293,17 @@ class wvmNetwork(wvmConnect):
|
|||
|
||||
def modify_fixed_address(self, name, address, mac_duid, family="ipv4"):
|
||||
tree = etree.fromstring(self._XMLDesc(0))
|
||||
name_esc = util.xml_escape(name) if name else ""
|
||||
name_attr = f' name="{name_esc}"' if name else ""
|
||||
mac_esc = util.xml_escape(mac_duid)
|
||||
ip_addr = ipaddress.ip_address(address)
|
||||
if family == "ipv4":
|
||||
new_xml = '<host mac="{}" {} ip="{}"/>'.format(mac_duid, 'name="' + name + '"' if name else "", ipaddress.ip_address(address))
|
||||
new_xml = f'<host mac="{mac_esc}"{name_attr} ip="{ip_addr}"/>'
|
||||
hosts = tree.xpath("./ip[not(@family='ipv6')]/dhcp/host")
|
||||
compare_var = "mac"
|
||||
parent_index = self.parent_count - 2
|
||||
if family == "ipv6":
|
||||
new_xml = '<host id="{}" {} ip="{}"/>'.format(mac_duid, 'name="' + name + '"' if name else "", ipaddress.ip_address(address))
|
||||
new_xml = f'<host id="{mac_esc}"{name_attr} ip="{ip_addr}"/>'
|
||||
hosts = tree.xpath("./ip[@family='ipv6']/dhcp/host")
|
||||
compare_var = "id"
|
||||
parent_index = self.parent_count - 1
|
||||
|
|
@ -331,8 +353,12 @@ class wvmNetwork(wvmConnect):
|
|||
return qos_values
|
||||
|
||||
def set_qos(self, direction, average, peak, burst):
|
||||
if direction not in ("inbound","outbound"):
|
||||
raise Exception("Direction must be inbound or outbound")
|
||||
if direction not in ("inbound", "outbound"):
|
||||
raise ValueError("Direction must be inbound or outbound")
|
||||
|
||||
average = int(average)
|
||||
peak = int(peak)
|
||||
burst = int(burst)
|
||||
|
||||
xml = f"<{direction} average='{average}' peak='{peak}' burst='{burst}'/>"
|
||||
tree = etree.fromstring(self._XMLDesc(0))
|
||||
|
|
|
|||
92
vrtManager/test_xml_safety.py
Normal file
92
vrtManager/test_xml_safety.py
Normal file
|
|
@ -0,0 +1,92 @@
|
|||
import unittest
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from django.conf import settings
|
||||
|
||||
if not settings.configured:
|
||||
settings.configure(MAC_OUI="52:54:10")
|
||||
|
||||
from vrtManager.instance import wvmInstance
|
||||
from vrtManager.network import wvmNetworks, wvmNetwork
|
||||
|
||||
|
||||
class TestXmlSafety(unittest.TestCase):
|
||||
def test_create_network_escaping(self):
|
||||
wn = wvmNetworks.__new__(wvmNetworks)
|
||||
wn.wvm = MagicMock()
|
||||
mock_net = MagicMock()
|
||||
wn.get_network = MagicMock(return_value=mock_net)
|
||||
|
||||
malicious_name = "net<script>alert(1)</script>&name"
|
||||
malicious_bridge = "br0' onfocus='alert(2)"
|
||||
|
||||
wn.create_network(
|
||||
name=malicious_name,
|
||||
forward="bridge",
|
||||
ipv4=False,
|
||||
gateway=None,
|
||||
mask=None,
|
||||
dhcp4=None,
|
||||
ipv6=False,
|
||||
gateway6=None,
|
||||
prefix6=None,
|
||||
dhcp6=None,
|
||||
bridge=malicious_bridge,
|
||||
openvswitch=False,
|
||||
)
|
||||
|
||||
wn.wvm.networkDefineXML.assert_called_once()
|
||||
defined_xml = wn.wvm.networkDefineXML.call_args[0][0]
|
||||
self.assertNotIn("<script>", defined_xml)
|
||||
self.assertIn("<script>alert(1)</script>&name", defined_xml)
|
||||
self.assertIn("name='br0' onfocus='alert(2)'", defined_xml)
|
||||
|
||||
def test_network_set_qos_validation(self):
|
||||
wn = wvmNetwork.__new__(wvmNetwork)
|
||||
wn._XMLDesc = MagicMock(return_value="<network><name>testnet</name></network>")
|
||||
wn.wvm = MagicMock()
|
||||
|
||||
# Invalid direction
|
||||
with self.assertRaises(ValueError):
|
||||
wn.set_qos("lateral", 100, 200, 300)
|
||||
|
||||
# Invalid numeric strings
|
||||
with self.assertRaises(ValueError):
|
||||
wn.set_qos("inbound", "not_a_number", 200, 300)
|
||||
|
||||
def test_snapshot_escaping_and_recovery_on_failure(self):
|
||||
inst = wvmInstance.__new__(wvmInstance)
|
||||
inst.get_status = MagicMock(return_value=5) # shutoff
|
||||
inst.change_snapshot_xml = MagicMock()
|
||||
inst.recover_snapshot_xml = MagicMock()
|
||||
inst._XMLDesc = MagicMock(return_value="<domain></domain>")
|
||||
inst._snapshotCreateXML = MagicMock(side_effect=RuntimeError("libvirt exploded"))
|
||||
|
||||
bad_name = "snap<inject>""
|
||||
bad_desc = "desc<tag>&"
|
||||
|
||||
with self.assertRaises(RuntimeError):
|
||||
inst.create_snapshot(bad_name, bad_desc)
|
||||
|
||||
inst.change_snapshot_xml.assert_called_once()
|
||||
# Verify recover_snapshot_xml was called in finally despite error
|
||||
inst.recover_snapshot_xml.assert_called_once()
|
||||
|
||||
def test_external_snapshot_recovery(self):
|
||||
inst = wvmInstance.__new__(wvmInstance)
|
||||
inst.get_status = MagicMock(return_value=1)
|
||||
inst.change_snapshot_xml = MagicMock()
|
||||
inst.recover_snapshot_xml = MagicMock()
|
||||
inst._XMLDesc = MagicMock(return_value="<domain></domain>")
|
||||
inst._snapshotCreateXML = MagicMock(return_value=None)
|
||||
inst.refresh_instance_pools = MagicMock()
|
||||
|
||||
inst.create_external_snapshot("ext_snap_1", desc="normal snapshot")
|
||||
|
||||
inst.change_snapshot_xml.assert_called_once()
|
||||
inst.recover_snapshot_xml.assert_called_once()
|
||||
inst.refresh_instance_pools.assert_called_once()
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Loading…
Add table
Add a link
Reference in a new issue