Omer Frenkel has posted comments on this change. Change subject: core, engine: servlet to support the console proxy ......................................................................
Patch Set 27: (4 comments) https://gerrit.ovirt.org/#/c/35887/27/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAllVmsForAnotherUserQuery.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAllVmsForAnotherUserQuery.java: Line 19: protected void executeQueryCommand() { Line 20: List<VM> vmsList = getDbFacade().getVmDao().getAllForUser(getParameters().getId()); Line 21: Line 22: getQueryReturnValue().setReturnValue(vmsList); Line 23: } can you explain where and how this is used, the implication of this is that one user can get information on vms of another user without any permission check.. this is bypassing the whole permission system https://gerrit.ovirt.org/#/c/35887/27/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java: Line 8: GetVmByVmId(VdcQueryAuthType.User), Line 9: GetVmByVmNameForDataCenter(VdcQueryAuthType.User), Line 10: GetAllVms(VdcQueryAuthType.User), Line 11: GetAllVmsForUser(VdcQueryAuthType.User), Line 12: GetAllVmsForAnotherUser(VdcQueryAuthType.User), this doesnt look right.. why is this a user query? why allowing any user to get all vms of other user without any permissions considerations? Line 13: GetUnregisteredVms, Line 14: GetUnregisteredVmTemplates, Line 15: GetVmsRunningOnOrMigratingToVds, Line 16: GetVmsByStorageDomain, Line 177: GetDbUserByUserNameAndDomain(VdcQueryAuthType.User), Line 178: GetUserBySessionId(VdcQueryAuthType.User), Line 179: GetEngineSessionIdToken(VdcQueryAuthType.User), Line 180: GetUserProfile(VdcQueryAuthType.User), Line 181: GetAllUserProfiles(VdcQueryAuthType.User), also questionable Line 182: Line 183: // Directory queries: Line 184: GetDirectoryUserById(VdcQueryAuthType.User), Line 185: GetDirectoryGroupById(VdcQueryAuthType.User), https://gerrit.ovirt.org/#/c/35887/27/backend/manager/modules/services/src/main/java/org/ovirt/engine/core/services/VMConsoleProxyServlet.java File backend/manager/modules/services/src/main/java/org/ovirt/engine/core/services/VMConsoleProxyServlet.java: Line 130: StringBuilder buffer = new StringBuilder(); Line 131: Line 132: int r; Line 133: while ((r = body.read()) != -1) { Line 134: buffer.append((char) r); wouldnt it be easier to read lines and get it as strings already? String line; while ((line = body.readLine()) != null) { buffer.append(line) } Line 135: } Line 136: Line 137: body.close(); Line 138: -- To view, visit https://gerrit.ovirt.org/35887 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I53c721da21cefcf4069d14c7016b6f7d97f9eac9 Gerrit-PatchSet: 27 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vitor de Lima <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Ravi Nori <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: Shahar Havivi <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
