Juan Hernandez has posted comments on this change.
Change subject: core, webadmin, restapi: Detach password from VdcUser
......................................................................
Patch Set 3:
(3 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmLogonCommand.java
Line 40: final VM vm = getVm();
Line 41:
Line 42: // Send the log on command to the virtual machine:
Line 43: final VdcUser currentUser = getCurrentUser();
Line 44: final String password =
SessionDataContainer.getInstance().getPassword();
I agree that it is much better to use a char array than a string for this
purpose, specially if that array is cleaned somewhere, something like this:
char[] password = SessionDataContainer.getInstance().getPassword();
// Use the password.
Arrays.fill(password, '\0');
Without this the password is potentially leaked in the JVM memory.
However, I prefer to do that in different patches, I am already touching many
things with this collection of patches.
Line 45: final String domainController = currentUser != null ?
currentUser.getDomainControler() : "";
Line 46: final boolean sentToVM = Backend
Line 47: .getInstance()
Line 48: .getResourceManager()
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/security/auth/LoginValidator.java
Line 94: VdcQueryReturnValue queryReturnValue =
backend.RunPublicQuery(VdcQueryType.ValidateSession, params);
Line 95: if (queryReturnValue != null) {
Line 96: VdcUser vdcUser = (VdcUser)
queryReturnValue.getReturnValue();
Line 97: if (vdcUser != null) {
Line 98: principal = new Principal(vdcUser.getUserName(), null,
vdcUser.getDomainControler());
The method LoginValidator#validate(Principal principalString sessionId) is only
called from the Challenger#executeBasicAuthentication(...) method, and in that
context the Principal that is passed is built from the HTTP headers of the
request, i.e., the password used for this is not the password that user
provided in the past, but the password to be checked that the browser is
sending. So for this particular usage the password isn't needed.
Line 99: sessionHelper.setSessionId(sessionId);
Line 100: current.set(vdcUser);
Line 101: }
Line 102: }
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/Frontend.java
Line 779: public void onSuccess(final VdcReturnValueBase result) {
Line 780: logger.finer("Succesful returned result from
Login."); //$NON-NLS-1$
Line 781: setLoggedInUser(null);
Line 782: callback.getDel().onSuccess(callback.getModel(),
result);
Line 783: setLoginPassword(null);
Good question. I don't remember, to be honest. I will check again, it isn't
probably needed.
Line 784: if (getLoginHandler() != null &&
result.getSucceeded()) {
Line 785: getLoginHandler().onLoginSuccess(userName,
password, domain);
Line 786: }
Line 787: }
--
To view, visit http://gerrit.ovirt.org/17096
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0ab9321816ee28355a4118910086891f5a552014
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
