[WIP] Filter VMs by users and groups
This is a working version of the patchset. It will use the users and groups metadata on each VM to decide if the current user is able to perform an operation on the VM. If the user (1) has sudo access or (2) is listed in the VM 'users' metadata or (3) is part of at least one of the groups listed in the VM 'groups' metadata, they will be able to see and perform any operation on that VM. The VMs listed by /vms are also filtered by the current user's permissions. This patch contains the actual feature implementation. The tests and appropriate updates to the mockmodel are not ready yet, as they happen to require more changes in the current code than expected. Crístian Viana (1): Filter VMs by users and groups src/kimchi/control/base.py | 4 +++ src/kimchi/model/vms.py | 63 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 65 insertions(+), 2 deletions(-) -- 1.9.3
Currently, every user with sudo permissions can perform any operation on any virtual machine. In order to add more security, Kimchi will only allow users listed in the VM metadata - along with those with sudo permissions - to be able to perform actions on it. A VM may contain a list of system users and groups associated with it. If a user is not listed to access a VM, they will not be able to see it or to perform any operation on it. Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 4 +++ src/kimchi/model/vms.py | 63 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index f8a5210..6022472 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -118,6 +118,10 @@ class Resource(object): @cherrypy.expose def index(self): + authorized = getattr(self.model, model_fn(self, 'is_authorized'), None) + if authorized is not None and not authorized(*self.model_args): + raise cherrypy.HTTPError(403) + method = validate_method(('GET', 'DELETE', 'PUT')) try: return {'GET': self.get, diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 17bda04..00dcd0f 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -19,16 +19,19 @@ from lxml.builder import E import lxml.etree as ET +import grp import os import time import uuid from xml.etree import ElementTree +import cherrypy import libvirt from cherrypy.process.plugins import BackgroundTask from kimchi import vnc from kimchi import xmlutils +from kimchi.auth import USER_NAME, USER_SUDO from kimchi.config import READONLY_POOL_TYPE from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import NotFoundError, OperationFailed @@ -232,8 +235,8 @@ class VMsModel(object): @staticmethod def get_vms(conn): - conn = conn.get() - names = [dom.name().decode('utf-8') for dom in conn.listAllDomains(0)] + libvirtconn = conn.get() + names = [dom.name().decode('utf-8') for dom in libvirtconn.listAllDomains(0) if VMModel._vm_is_authorized(dom.name(), conn)] return sorted(names, key=unicode.lower) @@ -526,6 +529,62 @@ class VMModel(object): kimchi_log.error('Error trying to delete vm screenshot from ' 'database due error: %s', e.message) + @staticmethod + def _vm_get_access_users(name, conn): + dom = conn.get().lookupByName(name) + metadata = get_metadata_node(dom, 'access') + if metadata == '': + return [] + + users_xml = ET.fromstring(metadata).find('user') + return [] if users_xml is None else [ users_xml.text ] + + @staticmethod + def _vm_get_access_groups(name, conn): + dom = conn.get().lookupByName(name) + metadata = get_metadata_node(dom, 'access') + if metadata == '': + return [] + + groups_xml = ET.fromstring(metadata).find('group') + return [] if groups_xml is None else [ groups_xml.text ] + + @staticmethod + def _vm_is_authorized(name, conn): + try: + user_name = cherrypy.session.get(USER_NAME, None) + user_sudo = cherrypy.session.get(USER_SUDO, False) + except AttributeError: + return False + + if user_sudo: + return True + + dom = conn.get().lookupByName(name) + metadata = get_metadata_node(dom, 'access') + + if user_name is None or metadata == '': + return True + + users = VMModel._vm_get_access_users(name, conn) + groups = VMModel._vm_get_access_groups(name, conn) + + users_in_groups = set() + for vm_g in groups: + try: + for u in grp.getgrnam(vm_g).gr_mem: + users_in_groups.add(u) + except KeyError: + kimchi_log.warning("group '%s' is listed as authorized by VM '%s' but it doesn't exist" + % (vm_g, name)) + + if user_name in users or user_name in users_in_groups: + return True + + return False + + def is_authorized(self, name): + return VMModel._vm_is_authorized(name, self.conn) class VMScreenshotModel(object): def __init__(self, **kargs): -- 1.9.3
On 07/11/2014 04:08 PM, Crístian Viana wrote:
Currently, every user with sudo permissions can perform any operation on any virtual machine.
In order to add more security, Kimchi will only allow users listed in the VM metadata - along with those with sudo permissions - to be able to perform actions on it. A VM may contain a list of system users and groups associated with it. If a user is not listed to access a VM, they will not be able to see it or to perform any operation on it.
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 4 +++ src/kimchi/model/vms.py | 63 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index f8a5210..6022472 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -118,6 +118,10 @@ class Resource(object):
@cherrypy.expose def index(self): + authorized = getattr(self.model, model_fn(self, 'is_authorized'), None) + if authorized is not None and not authorized(*self.model_args): + raise cherrypy.HTTPError(403) + method = validate_method(('GET', 'DELETE', 'PUT')) try: return {'GET': self.get, diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 17bda04..00dcd0f 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -19,16 +19,19 @@
from lxml.builder import E import lxml.etree as ET +import grp import os import time import uuid from xml.etree import ElementTree
+import cherrypy import libvirt from cherrypy.process.plugins import BackgroundTask
from kimchi import vnc from kimchi import xmlutils +from kimchi.auth import USER_NAME, USER_SUDO from kimchi.config import READONLY_POOL_TYPE from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import NotFoundError, OperationFailed @@ -232,8 +235,8 @@ class VMsModel(object):
@staticmethod def get_vms(conn): - conn = conn.get() - names = [dom.name().decode('utf-8') for dom in conn.listAllDomains(0)] + libvirtconn = conn.get() + names = [dom.name().decode('utf-8') for dom in libvirtconn.listAllDomains(0) if VMModel._vm_is_authorized(dom.name(), conn)]
80 characters
return sorted(names, key=unicode.lower)
@@ -526,6 +529,62 @@ class VMModel(object): kimchi_log.error('Error trying to delete vm screenshot from ' 'database due error: %s', e.message)
+ @staticmethod + def _vm_get_access_users(name, conn): + dom = conn.get().lookupByName(name) + metadata = get_metadata_node(dom, 'access') + if metadata == '': + return [] + + users_xml = ET.fromstring(metadata).find('user') + return [] if users_xml is None else [ users_xml.text ] + + @staticmethod + def _vm_get_access_groups(name, conn): + dom = conn.get().lookupByName(name) + metadata = get_metadata_node(dom, 'access') + if metadata == '': + return [] + + groups_xml = ET.fromstring(metadata).find('group') + return [] if groups_xml is None else [ groups_xml.text ] +
Those 2 functions are equals! I recommend to create just one that works in both case. Example: @staticmethod def _vm_get_access_elements(name, conn, element): dom = conn.get().lookupByName(name) metadata = get_metadata_node(dom, 'access') if metadata == ''; return [] xml = ET.fromstring(metadata)find(element) return [] if xml is None else [xml.text] And then use: _vm_get_access_elements(<my-vm>, <conn>, "user") _vm_get_access_elements(<my-vm>, <conn>, "group")
+ @staticmethod + def _vm_is_authorized(name, conn): + try: + user_name = cherrypy.session.get(USER_NAME, None) + user_sudo = cherrypy.session.get(USER_SUDO, False)
According to last discussion, we decided to don't use USER_SUDO and instead of of that, use USER_ROLES A user will be a role per tab, so it'd be better to check USER_ROLES['guests'] == 'admin' as in future we will support user role changes, ie, a non-root user can be a guest admin
+ except AttributeError: + return False + + if user_sudo: + return True + + dom = conn.get().lookupByName(name) + metadata = get_metadata_node(dom, 'access') + + if user_name is None or metadata == '': + return True + + users = VMModel._vm_get_access_users(name, conn) + groups = VMModel._vm_get_access_groups(name, conn) + + users_in_groups = set() + for vm_g in groups: + try: + for u in grp.getgrnam(vm_g).gr_mem: + users_in_groups.add(u) + except KeyError: + kimchi_log.warning("group '%s' is listed as authorized by VM '%s' but it doesn't exist" + % (vm_g, name)) + + if user_name in users or user_name in users_in_groups: + return True + + return False + + def is_authorized(self, name): + return VMModel._vm_is_authorized(name, self.conn)
class VMScreenshotModel(object): def __init__(self, **kargs):
Currently, every user with sudo permissions can perform any operation on any virtual machine.
In order to add more security, Kimchi will only allow users listed in the VM metadata - along with those with sudo permissions - to be able to perform actions on it. A VM may contain a list of system users and groups associated with it. If a user is not listed to access a VM, they will not be able to see it or to perform any operation on it.
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 4 +++ src/kimchi/model/vms.py | 63 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index f8a5210..6022472 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -118,6 +118,10 @@ class Resource(object):
@cherrypy.expose def index(self): + authorized = getattr(self.model, model_fn(self, 'is_authorized'), None) + if authorized is not None and not authorized(*self.model_args): + raise cherrypy.HTTPError(403) + method = validate_method(('GET', 'DELETE', 'PUT')) try: return {'GET': self.get, diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 17bda04..00dcd0f 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -19,16 +19,19 @@
from lxml.builder import E import lxml.etree as ET +import grp import os import time import uuid from xml.etree import ElementTree
+import cherrypy import libvirt from cherrypy.process.plugins import BackgroundTask
from kimchi import vnc from kimchi import xmlutils +from kimchi.auth import USER_NAME, USER_SUDO from kimchi.config import READONLY_POOL_TYPE from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import NotFoundError, OperationFailed @@ -232,8 +235,8 @@ class VMsModel(object):
@staticmethod def get_vms(conn): - conn = conn.get() - names = [dom.name().decode('utf-8') for dom in conn.listAllDomains(0)] + libvirtconn = conn.get() + names = [dom.name().decode('utf-8') for dom in libvirtconn.listAllDomains(0) if VMModel._vm_is_authorized(dom.name(), conn)] Filtering resources here can work, while I suggest using filter resources in controller to be more generic, cover all resources will filter logic. In model all resources report a 'users' information in its data()--means
On 2014年07月12日 03:08, Crístian Viana wrote: the valid user to view this resource: vm--{'users':["kimchi_user_1", "root"]} In controller, base.py: def filter_data(): user_name = cherrypy.session.get(USER_NAME, None) data = [] for res in resources: if all(key in res.data and res.data[key] == val and user_name in data["user"] for key, val in fields_filter.iteritems()): data.append(res.data) return data
return sorted(names, key=unicode.lower)
@@ -526,6 +529,62 @@ class VMModel(object): kimchi_log.error('Error trying to delete vm screenshot from ' 'database due error: %s', e.message)
+ @staticmethod + def _vm_get_access_users(name, conn): + dom = conn.get().lookupByName(name) + metadata = get_metadata_node(dom, 'access') + if metadata == '': + return [] + + users_xml = ET.fromstring(metadata).find('user') + return [] if users_xml is None else [ users_xml.text ] + + @staticmethod + def _vm_get_access_groups(name, conn): + dom = conn.get().lookupByName(name) + metadata = get_metadata_node(dom, 'access') + if metadata == '': + return [] + + groups_xml = ET.fromstring(metadata).find('group') + return [] if groups_xml is None else [ groups_xml.text ] + + @staticmethod + def _vm_is_authorized(name, conn): + try: + user_name = cherrypy.session.get(USER_NAME, None) + user_sudo = cherrypy.session.get(USER_SUDO, False) + except AttributeError: + return False + + if user_sudo: + return True + + dom = conn.get().lookupByName(name) + metadata = get_metadata_node(dom, 'access') + + if user_name is None or metadata == '': + return True + + users = VMModel._vm_get_access_users(name, conn) + groups = VMModel._vm_get_access_groups(name, conn) + + users_in_groups = set() + for vm_g in groups: + try: + for u in grp.getgrnam(vm_g).gr_mem: + users_in_groups.add(u) + except KeyError: + kimchi_log.warning("group '%s' is listed as authorized by VM '%s' but it doesn't exist" + % (vm_g, name)) + + if user_name in users or user_name in users_in_groups: + return True + + return False + + def is_authorized(self, name): + return VMModel._vm_is_authorized(name, self.conn)
class VMScreenshotModel(object): def __init__(self, **kargs):
On 07/16/2014 05:32 AM, Royce Lv wrote:
On 2014年07月12日 03:08, Crístian Viana wrote:
Currently, every user with sudo permissions can perform any operation on any virtual machine.
In order to add more security, Kimchi will only allow users listed in the VM metadata - along with those with sudo permissions - to be able to perform actions on it. A VM may contain a list of system users and groups associated with it. If a user is not listed to access a VM, they will not be able to see it or to perform any operation on it.
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 4 +++ src/kimchi/model/vms.py | 63 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index f8a5210..6022472 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -118,6 +118,10 @@ class Resource(object): @cherrypy.expose def index(self): + authorized = getattr(self.model, model_fn(self, 'is_authorized'), None) + if authorized is not None and not authorized(*self.model_args): + raise cherrypy.HTTPError(403) + method = validate_method(('GET', 'DELETE', 'PUT')) try: return {'GET': self.get, diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 17bda04..00dcd0f 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -19,16 +19,19 @@ from lxml.builder import E import lxml.etree as ET +import grp import os import time import uuid from xml.etree import ElementTree +import cherrypy import libvirt from cherrypy.process.plugins import BackgroundTask from kimchi import vnc from kimchi import xmlutils +from kimchi.auth import USER_NAME, USER_SUDO from kimchi.config import READONLY_POOL_TYPE from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import NotFoundError, OperationFailed @@ -232,8 +235,8 @@ class VMsModel(object): @staticmethod def get_vms(conn): - conn = conn.get() - names = [dom.name().decode('utf-8') for dom in conn.listAllDomains(0)] + libvirtconn = conn.get() + names = [dom.name().decode('utf-8') for dom in libvirtconn.listAllDomains(0) if VMModel._vm_is_authorized(dom.name(), conn)] Filtering resources here can work, while I suggest using filter resources in controller to be more generic, cover all resources will filter logic. In model all resources report a 'users' information in its data()--means the valid user to view this resource: vm--{'users':["kimchi_user_1", "root"]}
In controller, base.py: def filter_data(): user_name = cherrypy.session.get(USER_NAME, None)
data = [] for res in resources: if all(key in res.data and res.data[key] == val and user_name in data["user"] for key, val in fields_filter.iteritems()): data.append(res.data) return data
Good suggestion, Royce! It makes the logic simpler and easier to use
return sorted(names, key=unicode.lower) @@ -526,6 +529,62 @@ class VMModel(object): kimchi_log.error('Error trying to delete vm screenshot from ' 'database due error: %s', e.message) + @staticmethod + def _vm_get_access_users(name, conn): + dom = conn.get().lookupByName(name) + metadata = get_metadata_node(dom, 'access') + if metadata == '': + return [] + + users_xml = ET.fromstring(metadata).find('user') + return [] if users_xml is None else [ users_xml.text ] + + @staticmethod + def _vm_get_access_groups(name, conn): + dom = conn.get().lookupByName(name) + metadata = get_metadata_node(dom, 'access') + if metadata == '': + return [] + + groups_xml = ET.fromstring(metadata).find('group') + return [] if groups_xml is None else [ groups_xml.text ] + + @staticmethod + def _vm_is_authorized(name, conn): + try: + user_name = cherrypy.session.get(USER_NAME, None) + user_sudo = cherrypy.session.get(USER_SUDO, False) + except AttributeError: + return False + + if user_sudo: + return True + + dom = conn.get().lookupByName(name) + metadata = get_metadata_node(dom, 'access') + + if user_name is None or metadata == '': + return True + + users = VMModel._vm_get_access_users(name, conn) + groups = VMModel._vm_get_access_groups(name, conn) + + users_in_groups = set() + for vm_g in groups: + try: + for u in grp.getgrnam(vm_g).gr_mem: + users_in_groups.add(u) + except KeyError: + kimchi_log.warning("group '%s' is listed as authorized by VM '%s' but it doesn't exist" + % (vm_g, name)) + + if user_name in users or user_name in users_in_groups: + return True + + return False + + def is_authorized(self, name): + return VMModel._vm_is_authorized(name, self.conn) class VMScreenshotModel(object): def __init__(self, **kargs):
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
Thanks for the feedback! I'll send another version with the proposed changes.
As I commented in Aline's authorization patch, I think we get to the point to split Linux user/group with kimchi user/group, 1. we want different admin just responsible for their own parts: network admin manage network, storage admin manage storage, but superuser/un-previledged user does not have such fine grained view. 2. we want multi-level of access of one tab: take guest management as an example, we want create/destroy--start/stop--access vnc, at least 3 levels of access. superuser way cannot reflect multi-level control. On 2014年07月12日 03:08, Crístian Viana wrote:
This is a working version of the patchset. It will use the users and groups metadata on each VM to decide if the current user is able to perform an operation on the VM. If the user (1) has sudo access or (2) is listed in the VM 'users' metadata or (3) is part of at least one of the groups listed in the VM 'groups' metadata, they will be able to see and perform any operation on that VM.
The VMs listed by /vms are also filtered by the current user's permissions.
This patch contains the actual feature implementation. The tests and appropriate updates to the mockmodel are not ready yet, as they happen to require more changes in the current code than expected.
Crístian Viana (1): Filter VMs by users and groups
src/kimchi/control/base.py | 4 +++ src/kimchi/model/vms.py | 63 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 65 insertions(+), 2 deletions(-)
participants (3)
-
Aline Manera -
Crístian Viana -
Royce Lv