Nir Soffer has posted comments on this change.

Change subject: jsonrpc: filter params while logging secure method invocation
......................................................................


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/40961/1/lib/yajsonrpc/__init__.py
File lib/yajsonrpc/__init__.py:

Line 453:     SECURE_METHODS = ('VM.desktopLogin', 
'StoragePool.disconnectStorageServer'
Line 454:                       'StoragePool.connectStorageServer',
Line 455:                       'ISCSIConnection.discoverSendTargets',
Line 456:                       'VM.setTicket',
Line 457:                       'StoragePool.validateStorageServerConnection')
> These should be called SECURE_ARGS, and used to filter out arguments from "
Also, please format this list with one item per line and sorted.

Compare this list with the change here:
https://github.com/oVirt/vdsm/commit/862d6440176c2d2ef3a84a865fb0461ed835bd8c#diff-9d82b8e11f569c981a6d93a9f6158b20R27

Each name = protect_passwords(name) should have an item here, except the v2v 
verbs which are not available on 3.5.

Consider also backporting the password module and use it to filter the input 
arguments instead of not logging them - for example:

    params = protect_passwords(utils.picklecopy(req.params))
    self.log.log(logLevel, "Calling '%s' in bridge with %s",
                 req.method, params)
Line 458: 
Line 459:     def __init__(self, bridge, threadFactory=None):
Line 460:         self._bridge = bridge
Line 461:         self._workQueue = Queue()


-- 
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: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to