2:08 p.m.
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
2:08 p.m.
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(a)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
7:33 p.m.
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(a)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):
3:32 a.m.
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(a)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
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):
10:17 a.m.
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(a)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(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel
2:55 a.m.
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(-)
3570
days inactive
3576
days old
6 comments
3 participants
participants (3)
-
Aline Manera -
Crístian Viana -
Royce Lv