Closed Bug 552399 Opened 11 years ago Closed 11 years ago
API allows users to call some functions as other users
The XML-RPC API (until Bugzilla 3.6 - bug 525734) requires a separate login step. Therefore, BzAPI up to the current version (0.4.1) caches XML-RPC logins, to try and avoid having to make two requests to Bugzilla for every one request to the API. However, BzAPI does not check that the user using the cached credentials is the same user who created the session. Which means (I suspect via code reading, although I haven't tested) that if user A uses the BzAPI and calls an XML-RPC-backed function (such as the one for listing users) then user B can come along 5 minutes later and call any other XML-RPC-backed function as user A by passing email@example.com and no password. Currently, the following functions are XML-RPC-backed, in whole or part: /user GET (user search) /user/<id> GET (individual user info) /bug POST (bug creation) /bug/<id>/history GET (and other methods which return history) /bug/<id>/comment POST (add comment) /bug/<id>/comment GET (list comments - for Bugzilla 3.4 upwards) What are the options? 0) Switch back to having two calls per XML-RPC-backed request (which might well end up as 3 calls for bug creation, as it sometimes does a modification under the covers as well) for versions of Bugzilla before 3.6. Performance would suck :-| 1) Support only Bugzilla 3.6 and above. Apply the patch from bug 525734 to b.m.o. (or backport to bugzilla.org's next 3.4 release?) in order to prevent the main API installation sucking. 2) Use IP address checks as a way to make this much harder to exploit. This would prevent information leakage (unless user and attacker were using the same IP) but people could still forge packets to add comments or bugs as another user. 3) Make users pass back some sort of other auth token. But this defeats the RESTfulness of the API entirely. Something else? Ideas? Gerv
If you send the user a cookie, you should be able to verify their identity using that cookie. You could even pass them back the Bugzilla auth token itself. All or most HTTP clients should automatically understand cookies already.
You are suggesting solution 3). http://en.wikipedia.org/wiki/REST gives one of the definitions of REST as: Stateless: The client-server communication is further constrained by no client context being stored on the server between requests. Each request from any client contains all of the information necessary to service the request, and any state is held in the client. Adding cookies makes the server stateful. To put it another way, every request in a RESTful API should be independent. We could argue that "you can treat it independently, but the result will be that the API has worse performance. If you pass back the cookie, you get better performance." But that's not great. "Use the API the proper way, and it sucks. Use it the wrong way, and it works much better." Is there any chance of backporting the mentioned fix to the 3.4 stable branch? Gerv
There's little to no hope of that WebService change making it back to 3.4--it's based on some significant architectural changes in 3.6. I'd suggest just passing the cookie and documenting that it's necessary for reasonable performance. Most clients will handle it automatically. You're actually creating a stateful system, to some degree, by caching the logins, already.
Doh! The fix is obvious. Store the cached login keyed not by username, but by username and password. That requires the password on every request, thereby stopping the attack, but still permits us to cache logins. It means we do store the passwords in the proxy in memory, but I think that's OK. (BTW, caching logins on the proxy doesn't make the system stateful as perceived by the client side, which is the key thing - ease of API use.) Gerv
This bug needs to be fixed ASAP, but really, you should remove login caching completely and just implement comment #3.
Severity: critical → blocker
Priority: -- → P1
I'm in the middle of doing a release of a fixed version. I was working on it today, and it just passed the tests when I had to go out. I'll finish it off tomorrow. Gerv
OK, 0.5 has shipped with a fix for the impersonation problem. http://groups.google.com/group/mozilla.tools/browse_thread/thread/7ab7a7aabd237330# I've put a note in the release notes and on the wiki about the importance of people upgrading from 0.4 (although checking the server logs, most people seem to use /latest). Gerv
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
OK, 0.4.1 and below are now disabled on the main BzAPI server, and I'm just about to release 0.6. Opening bug. Gerv
You need to log in before you can comment on or make changes to this bug.