From 70c432dfc6b0269d2db119c5b1aca0d4245c0106 Mon Sep 17 00:00:00 2001 From: Malini Bhandaru Date: Thu, 25 Jul 2013 19:36:54 -0700 Subject: [PATCH] Enforce authenticated connections to libvirt Authenticated connections for both reads and writes increases VM security. The virConnectOpenAuth API is flexible, it effectively obsoletes virConnectOpen and virConnectOpenReadOnly. ReadOnly access, with its reduced access to the API, is obtained by passing in the vir connect flag VIR_CONNECT_RO (=1) An added advantage of using openAuth is the fewer API calls that need to be maintained in the nova libvirt driver. Removed virConnectOpenReadOnly methods and tests. Change-Id: I6cb3825d1225807955177d7500899babbb569e20 --- nova/tests/virt/libvirt/fakelibvirt.py | 12 ++--- nova/tests/virt/libvirt/test_fakelibvirt.py | 52 +++++++-------------- nova/virt/libvirt/driver.py | 9 ++-- 3 files changed, 24 insertions(+), 49 deletions(-) diff --git a/nova/tests/virt/libvirt/fakelibvirt.py b/nova/tests/virt/libvirt/fakelibvirt.py index 27c2a36867..322aed2263 100644 --- a/nova/tests/virt/libvirt/fakelibvirt.py +++ b/nova/tests/virt/libvirt/fakelibvirt.py @@ -125,6 +125,9 @@ VIR_ERR_NO_NWFILTER = 620 VIR_ERR_SYSTEM_ERROR = 900 VIR_ERR_INTERNAL_ERROR = 950 +# Readonly +VIR_CONNECT_RO = 1 + def _parse_disk_info(element): disk_info = {} @@ -888,14 +891,7 @@ class Connection(object): return [] -def openReadOnly(uri): - return Connection(uri, readonly=True) - - def openAuth(uri, auth, flags): - if flags != 0: - raise Exception(_("Please extend mock libvirt module to support " - "flags")) if type(auth) != list: raise Exception(_("Expected a list for 'auth' parameter")) @@ -908,7 +904,7 @@ def openAuth(uri, auth, flags): raise Exception( _("Expected a function in 'auth[1]' parameter")) - return Connection(uri, readonly=False) + return Connection(uri, (flags == VIR_CONNECT_RO)) def virEventRunDefaultImpl(): diff --git a/nova/tests/virt/libvirt/test_fakelibvirt.py b/nova/tests/virt/libvirt/test_fakelibvirt.py index 8fa2ffc63f..6ffd03e3e0 100644 --- a/nova/tests/virt/libvirt/test_fakelibvirt.py +++ b/nova/tests/virt/libvirt/test_fakelibvirt.py @@ -66,56 +66,36 @@ class FakeLibvirtTests(test.TestCase): super(FakeLibvirtTests, self).setUp() libvirt._reset() - def get_openReadOnly_curry_func(self): - return lambda uri: libvirt.openReadOnly(uri) - - def get_openAuth_curry_func(self): + def get_openAuth_curry_func(self, readOnly=False): def fake_cb(credlist): return 0 - return lambda uri: libvirt.openAuth(uri, - [[libvirt.VIR_CRED_AUTHNAME, - libvirt.VIR_CRED_NOECHOPROMPT], - fake_cb, - None], 0) - def _test_connect_method_accepts_None_uri_by_default(self, conn_method): - conn = conn_method(None) - self.assertNotEqual(conn, None, "Connecting to fake libvirt failed") - - def test_openReadOnly_accepts_None_uri_by_default(self): - conn_method = self.get_openReadOnly_curry_func() - self._test_connect_method_accepts_None_uri_by_default(conn_method) + creds = [[libvirt.VIR_CRED_AUTHNAME, + libvirt.VIR_CRED_NOECHOPROMPT], + fake_cb, + None] + flags = 0 + if readOnly: + flags = libvirt.VIR_CONNECT_RO + return lambda uri: libvirt.openAuth(uri, creds, flags) def test_openAuth_accepts_None_uri_by_default(self): conn_method = self.get_openAuth_curry_func() - self._test_connect_method_accepts_None_uri_by_default(conn_method) - - def _test_connect_method_can_refuse_None_uri(self, conn_method): - libvirt.allow_default_uri_connection = False - self.assertRaises(ValueError, conn_method, None) - - def test_openReadOnly_can_refuse_None_uri(self): - conn_method = self.get_openReadOnly_curry_func() - self._test_connect_method_can_refuse_None_uri(conn_method) + conn = conn_method(None) + self.assertNotEqual(conn, None, "Connecting to fake libvirt failed") def test_openAuth_can_refuse_None_uri(self): conn_method = self.get_openAuth_curry_func() - self._test_connect_method_can_refuse_None_uri(conn_method) - - def _test_connect_method_refuses_invalid_URI(self, conn_method): - self.assertRaises(libvirt.libvirtError, conn_method, 'blah') - - def test_openReadOnly_refuses_invalid_URI(self): - conn_method = self.get_openReadOnly_curry_func() - self._test_connect_method_refuses_invalid_URI(conn_method) + libvirt.allow_default_uri_connection = False + self.assertRaises(ValueError, conn_method, None) def test_openAuth_refuses_invalid_URI(self): conn_method = self.get_openAuth_curry_func() - self._test_connect_method_refuses_invalid_URI(conn_method) + self.assertRaises(libvirt.libvirtError, conn_method, 'blah') def test_getInfo(self): - conn = libvirt.openReadOnly(None) - res = conn.getInfo() + conn_method = self.get_openAuth_curry_func(readOnly=True) + res = conn_method(None).getInfo() self.assertIn(res[0], ('i686', 'x86_64')) self.assertTrue(1024 <= res[1] <= 16384, "Memory unusually high or low.") diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 95fbf3ae07..634eea188c 100755 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -663,10 +663,10 @@ class LibvirtDriver(driver.ComputeDriver): None] try: + flags = 0 if read_only: - return libvirt.openReadOnly(uri) - else: - return libvirt.openAuth(uri, auth, 0) + flags = libvirt.VIR_CONNECT_RO + return libvirt.openAuth(uri, auth, flags) except libvirt.libvirtError as ex: LOG.exception(_("Connection to libvirt failed: %s"), ex) payload = dict(ip=LibvirtDriver.get_host_ip_addr(), @@ -3260,8 +3260,7 @@ class LibvirtDriver(driver.ComputeDriver): def _compare_cpu(self, cpu_info): """Checks the host cpu is compatible to a cpu given by xml. - - "xml" must be a part of libvirt.openReadonly().getCapabilities(). + "xml" must be a part of libvirt.openAuth(...).getCapabilities(). return values follows by virCPUCompareResult. if 0 > return value, do live migration. 'http://libvirt.org/html/libvirt-libvirt.html#virCPUCompareResult'