Dima Kuznetsov has uploaded a new change for review.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
caps: Add selinux enforcement reporting.
Added selinuxEnforceModed field to getVdsCaps() what indicates whether selinux is enforced on host or not
Change-Id: I98e0fcb71e831a76c4584bca46dc58fc4298180f Signed-off-by: Dima Kuznetsov dkuznets@redhat.com --- M vdsm/caps.py M vdsm_api/vdsmapi-schema.json 2 files changed, 25 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/51/26951/1
diff --git a/vdsm/caps.py b/vdsm/caps.py index 3636228..597c3ed 100644 --- a/vdsm/caps.py +++ b/vdsm/caps.py @@ -32,6 +32,7 @@
import libvirt import rpm +import selinux
from vdsm.config import config from vdsm import libvirtconnection @@ -390,6 +391,20 @@ return platform.machine()
+def getSELinuxEnforceMode(): + selinux_mnts = ['/sys/fs/selinux', '/selinux'] + for mnt in selinux_mnts: + enforce_path = os.path.join(mnt, 'enforce') + if not os.path.exists(enforce_path): + continue + + with open(enforce_path) as fileStream: + return int(fileStream.read().strip()) + + # Assume disabled if cannot find + return -1 + + def get(): targetArch = getTargetArch()
@@ -459,6 +474,10 @@ caps['numaNodeDistance'] = _getNumaNodeDistance() caps['autoNumaBalancing'] = _getAutoNumaBalancingInfo()
+ if selinux.is_selinux_enabled() == 0: + caps['selinuxEnforceMode'] = str(-1) + else: + caps['selinuxEnforceMode'] = getSELinuxEnforceMode() return caps
diff --git a/vdsm_api/vdsmapi-schema.json b/vdsm_api/vdsmapi-schema.json index 6571cb7..5ef5cbe 100644 --- a/vdsm_api/vdsmapi-schema.json +++ b/vdsm_api/vdsmapi-schema.json @@ -1112,7 +1112,10 @@ # # @autoNumaBalancing: The status of auto numa balancing function # -# Since: 4.10.0 +# @selinuxEnforceMode: The mode of enforcement of SELinux policies on the +# host. +# +# Since: 4.15.0 # # Notes: Since ovirt-engine cannot parse software versions in 'x.y.z' format, # the current API truncates @software_version to 'x.y'. @@ -1134,7 +1137,8 @@ 'guestOverhead': 'uint', 'netConfigDirty': 'bool', 'rngSources': ['VmRngDeviceSource'], 'numaNodes': 'NumaNodeMap', 'numaNodeDistance': 'NumaNodeDistanceMap', - 'autoNumaBalancing': 'AutoNumaBalancingStatus'}} + 'autoNumaBalancing': 'AutoNumaBalancingStatus', + 'selinuxEnforceMode': 'int'}}
## # @Host.getCapabilities:
oVirt Jenkins CI Server has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8213/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8326/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7423/ : SUCCESS
Yaniv Bronhaim has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 1:
(3 comments)
http://gerrit.ovirt.org/#/c/26951/1//COMMIT_MSG Commit Message:
Line 7: caps: Add selinux enforcement reporting. Line 8: Line 9: Added selinuxEnforceModed field to getVdsCaps() what indicates whether Line 10: selinux is enforced on host or not Line 11: add direct link to engine's part Line 12: Change-Id: I98e0fcb71e831a76c4584bca46dc58fc4298180f
http://gerrit.ovirt.org/#/c/26951/1/vdsm/caps.py File vdsm/caps.py:
Line 405: - I would add here the comment from /etc/selinux/config:
# 1= enforcing - SELinux security policy is enforced. # 0 = permissive - SELinux prints warnings instead of enforcing. # -1= disabled - No SELinux policy is loaded.
or use enums
Line 474: caps['numaNodeDistance'] = _getNumaNodeDistance() Line 475: caps['autoNumaBalancing'] = _getAutoNumaBalancingInfo() Line 476: Line 477: if selinux.is_selinux_enabled() == 0: Line 478: caps['selinuxEnforceMode'] = str(-1) do we really need selinux package here? isn't getSELinuxEnforceMode enough for this purpose ? Line 479: else: Line 480: caps['selinuxEnforceMode'] = getSELinuxEnforceMode() Line 481: return caps Line 482:
Douglas Schilling Landgraf has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 1:
(3 comments)
please verify
http://gerrit.ovirt.org/#/c/26951/1//COMMIT_MSG Commit Message:
Line 7: caps: Add selinux enforcement reporting. Line 8: Line 9: Added selinuxEnforceModed field to getVdsCaps() what indicates whether Line 10: selinux is enforced on host or not Line 11:
add direct link to engine's part
+1 to yaniv's comment and Bug-Url if it has. Line 12: Change-Id: I98e0fcb71e831a76c4584bca46dc58fc4298180f
http://gerrit.ovirt.org/#/c/26951/1/vdsm/caps.py File vdsm/caps.py:
Line 401: with open(enforce_path) as fileStream: Line 402: return int(fileStream.read().strip()) Line 403: Line 404: # Assume disabled if cannot find Line 405: return -1
I would add here the comment from /etc/selinux/config:
Agree with yaniv, please use docstring format. Line 406: Line 407: Line 408: def get(): Line 409: targetArch = getTargetArch()
Line 474: caps['numaNodeDistance'] = _getNumaNodeDistance() Line 475: caps['autoNumaBalancing'] = _getAutoNumaBalancingInfo() Line 476: Line 477: if selinux.is_selinux_enabled() == 0: Line 478: caps['selinuxEnforceMode'] = str(-1)
do we really need selinux package here? isn't getSELinuxEnforceMode enough
+1 for Yaniv comment. Line 479: else: Line 480: caps['selinuxEnforceMode'] = getSELinuxEnforceMode() Line 481: return caps Line 482:
Douglas Schilling Landgraf has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 1: Code-Review-1
Douglas Schilling Landgraf has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 1:
(1 comment)
http://gerrit.ovirt.org/#/c/26951/1/vdsm/caps.py File vdsm/caps.py:
Line 390: else: Line 391: return platform.machine() Line 392: Line 393: Line 394: def getSELinuxEnforceMode(): What about have this function available in /usr/<lib>/python-2.7/site-packages/vdsm/utils.py ? IMO, can fit there, not limiting the availability in caps.py. Before doing that, please consult others. Line 395: selinux_mnts = ['/sys/fs/selinux', '/selinux'] Line 396: for mnt in selinux_mnts: Line 397: enforce_path = os.path.join(mnt, 'enforce') Line 398: if not os.path.exists(enforce_path):
Dima Kuznetsov has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 1:
(3 comments)
http://gerrit.ovirt.org/#/c/26951/1/vdsm/caps.py File vdsm/caps.py:
Line 390: else: Line 391: return platform.machine() Line 392: Line 393: Line 394: def getSELinuxEnforceMode():
What about have this function available in /usr/<lib>/python-2.7/site-packa
ok Line 395: selinux_mnts = ['/sys/fs/selinux', '/selinux'] Line 396: for mnt in selinux_mnts: Line 397: enforce_path = os.path.join(mnt, 'enforce') Line 398: if not os.path.exists(enforce_path):
Line 401: with open(enforce_path) as fileStream: Line 402: return int(fileStream.read().strip()) Line 403: Line 404: # Assume disabled if cannot find Line 405: return -1
Agree with yaniv, please use docstring format.
ok Line 406: Line 407: Line 408: def get(): Line 409: targetArch = getTargetArch()
Line 474: caps['numaNodeDistance'] = _getNumaNodeDistance() Line 475: caps['autoNumaBalancing'] = _getAutoNumaBalancingInfo() Line 476: Line 477: if selinux.is_selinux_enabled() == 0: Line 478: caps['selinuxEnforceMode'] = str(-1)
+1 for Yaniv comment.
Yes will change Line 479: else: Line 480: caps['selinuxEnforceMode'] = getSELinuxEnforceMode() Line 481: return caps Line 482:
oVirt Jenkins CI Server has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 2:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8261/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8375/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7471/ : SUCCESS
oVirt Jenkins CI Server has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8261/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7471/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8383/ : SUCCESS
Douglas Schilling Landgraf has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 2:
(1 comment)
http://gerrit.ovirt.org/#/c/26951/2//COMMIT_MSG Commit Message:
Line 6: Line 7: caps: Add selinux enforcement reporting. Line 8: Line 9: Added selinuxEnforceModed field to getVdsCaps() what indicates whether Line 10: selinux is enforced on host or not missing the link to engine code or/and bug-url if it exists. Line 11: Line 12: Change-Id: I98e0fcb71e831a76c4584bca46dc58fc4298180f
oVirt Jenkins CI Server has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 3:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8364/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7574/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8484/ : SUCCESS
oVirt Jenkins CI Server has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 4:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8365/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7575/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8485/ : SUCCESS
Dan Kenigsberg has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 4: Code-Review-1
(3 comments)
http://gerrit.ovirt.org/#/c/26951/4/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 1270: def prependDefer(self, func, *args, **kwargs): Line 1271: self._finally.insert(0, (func, args, kwargs)) Line 1272: Line 1273: Line 1274: def getSELinuxEnforceMode(): vdsm.utils is intended for general-purpose utility funcitons, that are expected to be called from various places.
Unless you have such plans for this function, please make it a private funciton in caps.py. Line 1275: """ Line 1276: Returns the SELinux mode as reported by kernel. Line 1277: Line 1278: 1 = enforcing - SELinux security policy is enforced.
http://gerrit.ovirt.org/#/c/26951/4/vdsm/caps.py File vdsm/caps.py:
Line 391: Line 392: Line 393: def _getSELinux(): Line 394: selinux = dict() Line 395: selinux['mode'] = str(utils.getSELinuxEnforceMode()) why not report it as an int? Line 396: Line 397: return selinux Line 398: Line 399:
Line 393: def _getSELinux(): Line 394: selinux = dict() Line 395: selinux['mode'] = str(utils.getSELinuxEnforceMode()) Line 396: Line 397: return selinux Why are you using another level of a dictionary? Do you have plans to repot values other than "mode"? Is "mode" a standard name for the enforcement mode of selinux? Line 398: Line 399: Line 400: def get(): Line 401: targetArch = getTargetArch()
Dima Kuznetsov has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 4:
(3 comments)
http://gerrit.ovirt.org/#/c/26951/4/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 1270: def prependDefer(self, func, *args, **kwargs): Line 1271: self._finally.insert(0, (func, args, kwargs)) Line 1272: Line 1273: Line 1274: def getSELinuxEnforceMode():
vdsm.utils is intended for general-purpose utility funcitons, that are expe
Ok, will move back to caps. Line 1275: """ Line 1276: Returns the SELinux mode as reported by kernel. Line 1277: Line 1278: 1 = enforcing - SELinux security policy is enforced.
http://gerrit.ovirt.org/#/c/26951/4/vdsm/caps.py File vdsm/caps.py:
Line 391: Line 392: Line 393: def _getSELinux(): Line 394: selinux = dict() Line 395: selinux['mode'] = str(utils.getSELinuxEnforceMode())
why not report it as an int?
All the int values that are part of the response are converted to string:
340 caps['cpuThreads'] = str(cpuTopology.threads()) 341 caps['cpuSockets'] = str(cpuTopology.sockets()) Line 396: Line 397: return selinux Line 398: Line 399:
Line 393: def _getSELinux(): Line 394: selinux = dict() Line 395: selinux['mode'] = str(utils.getSELinuxEnforceMode()) Line 396: Line 397: return selinux
Why are you using another level of a dictionary? Do you have plans to repot
I think additional level is a good idea because it'd allows us to add more SELinux info later. And mode is pretty standard name for enforcement mode, sestatus reports is just as 'mode':
$ sestatus SELinux status: enabled SELinuxfs mount: /sys/fs/selinux SELinux root directory: /etc/selinux Loaded policy name: targeted Current mode: enforcing Mode from config file: enforcing Policy MLS status: enabled Policy deny_unknown status: allowed Max kernel policy version: 28
About future plans, some of these have no use being reported like the mount-point, but maybe some day engine would like to know if there is a difference between configured and actual mode, or the current policy running. Line 398: Line 399: Line 400: def get(): Line 401: targetArch = getTargetArch()
Dan Kenigsberg has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 4:
(2 comments)
http://gerrit.ovirt.org/#/c/26951/4/vdsm/caps.py File vdsm/caps.py:
Line 391: Line 392: Line 393: def _getSELinux(): Line 394: selinux = dict() Line 395: selinux['mode'] = str(utils.getSELinuxEnforceMode())
All the int values that are part of the response are converted to string:
right. let us conform.
However, have you considered reporting the strings enforcing/permissive/disabled ? I believe it would be easier to understand. Line 396: Line 397: return selinux Line 398: Line 399:
Line 393: def _getSELinux(): Line 394: selinux = dict() Line 395: selinux['mode'] = str(utils.getSELinuxEnforceMode()) Line 396: Line 397: return selinux
I think additional level is a good idea because it'd allows us to add more
good reason indeed. Line 398: Line 399: Line 400: def get(): Line 401: targetArch = getTargetArch()
Dima Kuznetsov has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 4:
(1 comment)
http://gerrit.ovirt.org/#/c/26951/4/vdsm/caps.py File vdsm/caps.py:
Line 391: Line 392: Line 393: def _getSELinux(): Line 394: selinux = dict() Line 395: selinux['mode'] = str(utils.getSELinuxEnforceMode())
right. let us conform.
I don't really have a strong opinion here.
The numeric values are documented by SELinux, and are good enough for them (as available at /sys/fs/selinux/enforce), also.
On the other hand, on restapi I do report the more human friendly format of enforcing/permissive/disabled strings. Line 396: Line 397: return selinux Line 398: Line 399:
Dima Kuznetsov has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 5:
rebased and moved _getSELinuxEnforceMode from utils.py to caps.py.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 5:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8597/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7807/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8725/ : FAILURE
Dan Kenigsberg has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 5: Code-Review+2
Dima Kuznetsov has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 5: Verified+1
Verified with vdsClient getVdsCaps, reports correctly.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
caps: Add selinux enforcement reporting.
Added selinux field to getVdsCaps() what currently contains the mode of enforcement of the host
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=894084 Change-Id: I98e0fcb71e831a76c4584bca46dc58fc4298180f Signed-off-by: Dima Kuznetsov dkuznets@redhat.com Reviewed-on: http://gerrit.ovirt.org/26951 Reviewed-by: Dan Kenigsberg danken@redhat.com --- M vdsm/caps.py M vdsm_api/vdsmapi-schema.json 2 files changed, 49 insertions(+), 2 deletions(-)
Approvals: Dima Kuznetsov: Verified Dan Kenigsberg: Looks good to me, approved
oVirt Jenkins CI Server has posted comments on this change.
Change subject: caps: Add selinux enforcement reporting. ......................................................................
Patch Set 6:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged/1259/ : SUCCESS
vdsm-patches@lists.fedorahosted.org