Vojtech Szocs has posted comments on this change. Change subject: userportal,webadmin: token generation fix ......................................................................
Patch Set 1: I think there's more to this patch than it might seem at first glance. Client obtains XSRF token, then includes this token in all requests to server. Server uses JSESSIONID cookie to associate request with session. After associating request with session, before invoking actual request body, server compares JSESSIONID cookie with XSRF token. If not successful, request is rejected as XSRF (maliciously forged request) attempt. The "client obtains the XSRF token" part is currently implemented via GWT RPC call, reading JSESSIONID cookie on server. The reason for reading cookie on server is HttpOnly flag. This patch alters the current implementation to read server-side (Java Servlet) session ID via GWT RPC call, instead of reading JSESSIONID cookie. There is an important conceptual difference: * (current) client, maybe attacker, makes request with JSESSIONID cookie: - server reads the cookie sent by client, *does not* expose actual session ID * (changed) client, maybe attacker, makes request with JSESSIONID cookie: - server reads the session ID, it *does* expose actual session ID So with this patch, assuming someone can forge GWT RPC call (out of scope of XSRF protection), he or she can obtain actual session ID and impersonate the victim. This is because there is still JSESSIONID cookie sent alongside malicious request. I really appreciate the effort to make XSRF protection easier/simpler, but I have a feeling this patch beats the whole concept of XSRF protection by not reading what the client sent in JSESSIONID cookie. Alex (maybe Alon as well), what is your opinion on this? -- To view, visit http://gerrit.ovirt.org/30849 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3e9a234bada73873f398d4220808f573810440dc Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
