Change in vdsm[master]: logging: Add log handler that enforces perms

Dima Kuznetsov has uploaded a new change for review.

Change subject: logging: Add log handler that enforces perms
......................................................................

logging: Add log handler that enforces perms

This handler, extending WatchedFileHandler, given the desired uid + gid of the
log file, checks that the running process satisfies the uid + gid
requirement.

Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Signed-off-by: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
---
M vdsm/logUtils.py
1 file changed, 16 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/28/26728/1

diff --git a/vdsm/logUtils.py b/vdsm/logUtils.py
index 6949d44..dd16249 100644
--- a/vdsm/logUtils.py
+++ b/vdsm/logUtils.py
@@ -19,6 +19,8 @@
 #
 
 import logging
+import logging.handlers
+import os
 import sys
 from functools import wraps
 from inspect import ismethod
@@ -163,3 +165,17 @@
             raise
         except:
             self.handleError(record)
+
+
+class EnforcingWatchedFileHandler(logging.handlers.WatchedFileHandler):
+    def __init__(self, uid, gid, *args, **kwargs):
+        self._uid = uid
+        self._gid = gid
+        logging.handlers.WatchedFileHandler.__init__(self, *args, **kwargs)
+
+    def _open(self):
+        if (os.geteuid() != self._uid) or (os.getegid() != self._gid):
+            raise RuntimeError(
+                "Attempt to open log with incorrect credentials"
+            )
+        return logging.handlers.WatchedFileHandler._open(self)


-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;

Yaniv Bronhaim has posted comments on this change.

Change subject: logging: Add log handler that enforces perms
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

i'd like to here your thoughts about enforce this handler to avoid more permissions
issues with log files

http://gerrit.ovirt.org/#/c/26728/1//COMMIT_MSG
Commit Message:

Line 7: logging: Add log handler that enforces perms
Line 8: 
Line 9: This handler, extending WatchedFileHandler, given the desired uid + gid of the
Line 10: log file, checks that the running process satisfies the uid + gid
Line 11: requirement.
otherwise raise RuntimeException
Line 12: 
Line 13: Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6


-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: logging: Add log handler that enforces perms
......................................................................


Patch Set 1:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8015/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8128/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7225/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: log: Change vdsm log file to enforce user/group

Saggi Mizrahi has posted comments on this change.

Change subject: log: Change vdsm log file to enforce user/group
......................................................................


Patch Set 6: Code-Review+2

-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: log: Change vdsm log file to enforce user/group

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: log: Change vdsm log file to enforce user/group
......................................................................


Patch Set 8:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5703/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged_test_debug/68/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3861/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged/1726/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: log: Change vdsm log file to enforce user/group

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: log: Change vdsm log file to enforce user/group
......................................................................


log: Change vdsm log file to enforce user/group

This commit adds a new log handler that extends WatchedFileHandler,
This handler checks current user and group and checks with the expected
values, to make sure logs are created with correct permissions. In case of bad
access RuntimeError is thrown.

This patch also updates logger.conf to use this handler.

Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Signed-off-by: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Reviewed-on: http://gerrit.ovirt.org/26728
Reviewed-by: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
---
M vdsm/logUtils.py
M vdsm/logger.conf.in
M vdsm/svdsm.logger.conf.in
M vdsm/vdsm
4 files changed, 36 insertions(+), 5 deletions(-)

Approvals:
  Saggi Mizrahi: Looks good to me, approved
  Dima Kuznetsov: Verified



-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: log: Change vdsm log file to enforce user/group

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: log: Change vdsm log file to enforce user/group
......................................................................


Patch Set 7:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/10925/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11867/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/11710/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: log: Change vdsm log file to enforce user/group

Dima Kuznetsov has posted comments on this change.

Change subject: log: Change vdsm log file to enforce user/group
......................................................................


Patch Set 7: Verified+1

Rebased, verified by:

Building, installing, and starting vdsm without errors.
Made sure vdsm.log and supervdsm.log are active and being logged to.

Tell me if any other verification should be done.

-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: log: Change vdsm log file to enforce user/group

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: log: Change vdsm log file to enforce user/group
......................................................................


Patch Set 6:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/10498/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/11283/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11440/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: log: Change vdsm log file to enforce user/group

Dima Kuznetsov has posted comments on this change.

Change subject: log: Change vdsm log file to enforce user/group
......................................................................


Patch Set 5:

this patch needs http://gerrit.ovirt.org/#/c/27193/ to pass

-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: log: Change vdsm log file to enforce user/group

Yaniv Bronhaim has posted comments on this change.

Change subject: log: Change vdsm log file to enforce user/group
......................................................................


Patch Set 5: Code-Review+1

have you tested it ?

-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: log: Change vdsm log file to enforce user/group

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: log: Change vdsm log file to enforce user/group
......................................................................


Patch Set 5:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9428/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10212/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10368/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5294/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3452/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: logging: Add log handler that enforces perms
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

http://gerrit.ovirt.org/#/c/26728/1/vdsm/logUtils.py
File vdsm/logUtils.py:

Line 166:         except:
Line 167:             self.handleError(record)
Line 168: 
Line 169: 
Line 170: class EnforcingWatchedFileHandler(logging.handlers.WatchedFileHandler):
I do not mind using this extra safety measure, but the name of class must be changed
(it's not clear what is enforced here), and please include a user for this class (in
this or in a follow up patch).
Line 171:     def __init__(self, uid, gid, *args, **kwargs):
Line 172:         self._uid = uid
Line 173:         self._gid = gid
Line 174:         logging.handlers.WatchedFileHandler.__init__(self, *args, **kwargs)


-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: log: Change vdsm log file to enforce user/group

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: log: Change vdsm log file to enforce user/group
......................................................................


Patch Set 4:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9425/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10209/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10365/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5291/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3449/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: log: Change vdsm log file to enforce user/group

Yaniv Bronhaim has posted comments on this change.

Change subject: log: Change vdsm log file to enforce user/group
......................................................................


Patch Set 4:

I wonder:

1. if we need to touch the metadata.log at all which i don't find where we use - dima,
do you see the usage? does really only vdsm user touch this log ?

2. if supervdsm and update logs also should have such check that only root user will be
able to open them

but for now I prefer not to add more noises . this is good enough to avoid the permission
changes we had in the recent past

-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: log: Change vdsm log file to enforce user/group

Dima Kuznetsov has posted comments on this change.

Change subject: log: Change vdsm log file to enforce user/group
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.ovirt.org/#/c/26728/3/vdsm/logger.conf.in
File vdsm/logger.conf.in:

Line 51: 
Line 52: [handler_metadata]
Line 53: class=logUtils.UserGroupEnforcingHandler
Line 54: args=('@VDSMUSER@', '@VDSMGROUP@',
'@VDSMLOGDIR(a)/vdsm.log',)
Line 55: level=WARNING
>  you changed the name of the log 
you're right :(
Line 56: formatter=long
Line 57: 
Line 58: [handler_console]
Line 59: class: StreamHandler


-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: log: Change vdsm log file to enforce user/group

Yaniv Bronhaim has posted comments on this change.

Change subject: log: Change vdsm log file to enforce user/group
......................................................................


Patch Set 3: Code-Review-1

(1 comment)

http://gerrit.ovirt.org/#/c/26728/3/vdsm/logger.conf.in
File vdsm/logger.conf.in:

Line 51: 
Line 52: [handler_metadata]
Line 53: class=logUtils.UserGroupEnforcingHandler
Line 54: args=('@VDSMUSER@', '@VDSMGROUP@',
'@VDSMLOGDIR(a)/vdsm.log',)
Line 55: level=WARNING
you changed the name of the log

copy paste is evil from time to time ...
Line 56: formatter=long
Line 57: 
Line 58: [handler_console]
Line 59: class: StreamHandler


-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: log: Change vdsm log file to enforce user/group

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: log: Change vdsm log file to enforce user/group
......................................................................


Patch Set 3:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9413/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10197/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10353/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5279/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3437/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: log: Change vdsm log file to enforce user/group

Dima Kuznetsov has posted comments on this change.

Change subject: log: Change vdsm log file to enforce user/group
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/26728/1/vdsm/logUtils.py
File vdsm/logUtils.py:

Line 166:         except:
Line 167:             self.handleError(record)
Line 168: 
Line 169: 
Line 170: class EnforcingWatchedFileHandler(logging.handlers.WatchedFileHandler):
>  I do not mind using this extra safety measure, but the name of class
> must b 
Done, hope the new name is better.

Also, could not find a way to use uid/gid with fileConfig (as they are not known at code
generation time), so this class now accepts user name and group name.
Line 171:     def __init__(self, uid, gid, *args, **kwargs):
Line 172:         self._uid = uid
Line 173:         self._gid = gid
Line 174:         logging.handlers.WatchedFileHandler.__init__(self, *args, **kwargs)


-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: log: Change vdsm log file to enforce user/group

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: log: Change vdsm log file to enforce user/group
......................................................................


Patch Set 2:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9407/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10191/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10347/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5273/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3431/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Itamar Heim has posted comments on this change.

Change subject: logging: Add log handler that enforces perms
......................................................................


Patch Set 1:

ping

-- 
To view, visit http://gerrit.ovirt.org/26728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a4d7212cb311b22e4fb60ffdc45163a496a74d6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No