Change in vdsm[ovirt-3.5]: jsonrpc: filter params while logging secure method invocation
nsoffer at redhat.com
nsoffer at redhat.com
Sat May 16 20:31:55 UTC 2015
Nir Soffer has posted comments on this change.
Change subject: jsonrpc: filter params while logging secure method invocation
......................................................................
Patch Set 2:
(3 comments)
https://gerrit.ovirt.org/#/c/40961/2/lib/yajsonrpc/__init__.py
File lib/yajsonrpc/__init__.py:
Line 494
Line 495
Line 496
Line 497
Line 498
Same:
if req.method in self.SECURE_RETURN:
protected_res = protect_passwords(picklecopy(res))
else:
protected_res = res
And log protected_res instead of res
Line 454: 'StoragePool.connectStorageServer',
Line 455: 'StoragePool.disconnectStorageServer',
Line 456: 'StoragePool.validateStorageServerConnection',
Line 457: 'VM.desktopLogin',
Line 458: 'VM.setTicket')
Host_fenceNode is missing - logging it only in TRACE level is not good enough.
Same for virt methods containing passwords - please look at the master patch to find the methods that need protection.
Line 459:
Line 460: SECURE_RETURN = ('Host.getDeviceList')
Line 461:
Line 462: def __init__(self, bridge, threadFactory=None):
Line 474: 'Host_getStats', 'StorageDomain_getStats',
Line 475: 'VM_getStats', 'Host_fenceNode'):
Line 476: logLevel = logging.TRACE
Line 477: self._log_message(logLevel, 'Calling', self.SECURE_ARGS, req.method,
Line 478: req.params)
This works, but is more complicated than needed, and is harder to follow. I think the only change needed is:
if req.method in self.SECURE_ARGS:
protected_params = protect_passwords(picklecopy(req.params))
else:
protected_params = req.params
And log protected_params instead of req.params. No need to add _log_message to log two different messages that need different protection.
If you like to keep the code cleaner, I think this can help:
def protected_params(self, method, args):
if method in self.SECURE_ARGS:
return protect_passwords(picklecopy(args))
return args
def _protected_result(self, method, result):
if method in self.SECURE_RETURN:
return protect_passwords(picklecopy(result))
return result
Then you just do:
protected_params = self._protected_args(req.method, req.params)
Or:
protected_res = self._protected_result(req.method, res)
We don't want to mix logging formatting and password protection logic.
Line 479:
Line 480: try:
Line 481: method = getattr(self._bridge, mangledMethod)
Line 482: except AttributeError:
--
To view, visit https://gerrit.ovirt.org/40961
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I34a36a361d2be28343352ae83ceac94b86162a12
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list