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 <vdel...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: Shahar Havivi <shav...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <wallaroo1...@gmail.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
