Closed Bug 491810 Opened 10 years ago Closed 10 years ago
Geolocation "cookie" isn't cleared at exit for network
.cookie .lifetime Policy == 2
Geolocation's access_token are supposed to behave like a cookie. However, with network.cookie.lifetimePolicy set to 2 (clear all cookies at the end of the session), this nonetheless token persists.
The token isn't clear that way by design. To clear the token, you can do one if the following: 1) wait two weeks. It will expire. 2) clear cookies 3) enter and exit private browsing mode.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → INVALID
Would you mind pointing me to the design spec? From http://dougt.wordpress.com/2009/04/30/geolocation-in-firefox-35-and-fennec/ > The access token basically acts like a two week cookie, and if you clear > cookies in the browser, this value is deleted and a new one is used. Note that I effectively _do_ clear cookies in the browser at every shutdown (resp. I tell it to ignore their two-week-in-the-future-expiry) which isn't honored by geolocation's access token. BTW: What's that token needed for at all? If a new token gives me the same result as an old one, my guess would be that it's rather for Google's statistical tracking - which I pretty explicitly opted out of through the "Keep cookies ... until Firefox is closed" option (in the options' Privacy section).
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Doug, I think we need to consider the user setting "clear cookies at shutdown" through the cookie options (which is the setting Simon's talking about) to be equivalent to removing all cookies. Not sure I'd block on it, as there's still a way to get rid of it and the token only exists if a user chooses to use the geolocation information, and then that token is only again used at the next geolocation lookup, but we should expedite a fix.
Hi Simon, Sorry for the confusion. When explaining to a layperson what this token is, it is sometime easier to say that it is a cookie because people more-or-less already understand what those do. However, it is clear that people who are more informed, like yourself, see that this is a weak metaphor. The way this is built, we do not use any cookies (rfc2109). The connection to the network geolocation provider (eg google) strips most http headers. The access token is part of a json string that is sent over ssl to google. Since there are no cookies, this value really isn't subject to the network.cookie.lifetimePolicy preference. We could build in some support for this, but I don't think I would like to do that given that we aren't really using cookies. Instead, there is a different way to clear cookies which many people already use. In the Firefox UI there is a "Clear Recent History..." feature that allows you to delete your history/cache/cookies immediately or at shutdown. Looking at this feature, we thought it was an overkill to add a new item just for geolocation. When looking at where exactly to put this, we settled on using the "cookie" type: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/sanitize.js#155 I could consider adding network.cookie.lifetimePolicy == 2 because as you suggest you are using it with the intention of clearing this sort of token. Out of curiosity, are you using the "Clear Recent History..." stuff at shutdown at all? Thanks!
(In reply to comment #4) > The way this is built, we do not use any cookies (rfc2109). On a side-note: Why not? Sure they can't be transmitted in http headers, but they could still be stored through the cookie manager (with e.g. an otherwise invalid "<geolocation>" host), couldn't they? Then you wouldn't need to spread your "do as if"s everywhere... > The access token is part of a json string that is sent over ssl to google. To repeat my question: Why do we do that at all? > Out of curiosity, are you using the "Clear Recent History..." stuff at > shutdown at all? No (and I'm aware that sites could track me e.g. through History or the Cache, however the sites I usually frequent don't - so convenience trumps privacy).
Assignee: nobody → doug.turner
Attachment #376265 - Flags: review?(vladimir)
Comment on attachment 376265 [details] [diff] [review] patch v.1 This look ok, provided you add a TODO to the comment saying that to be accurate, we should be clearing the token on browser shutdown, not every time the provider is shut down. Or, at the very least, a comment describing what a "session" is for geolocation.
Attachment #376265 - Flags: review?(vladimir) → review+
Hi Simon, You are probably right in that it could have possibly been stored as a cookie even though it isn't a HTTP cookie. I suppose that it was much easier to just use preferences similar to what we do with safe browsing than to attempt to use the cookie manager. On the DoS stuff. IP doesn't work very well for DoS style attacks when there are many people sitting behind a proxy.
http://hg.mozilla.org/mozilla-central/rev/02cfcc7837a5 Marking fixed.
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
(In reply to comment #11) > I suppose that it was much easier to just use preferences similar to what > we do with safe browsing than to attempt to use the cookie manager. Considering all the additional code you've had to include to clear the access token, wouldn't it make sense to reconsider that easiness? ;-) > IP doesn't work very well for DoS style attacks I didn't say that IP addresses worked well - just that they're still more reliable than an access token (thus making the access token pointless from the server's point of view - except for statistical purposes). Anyway, thanks for the speedy fix for this issue!
Hi Simon, no problem on the speedy fix -- I totally appreciate your feedback and I think because if it, we have a better Firefox! So, regarding why we didn't use the cookie manager -- We basically didn't want this access token to be sent ask a cookie when ever you visited the site in the browser. For example, if you used Geolocation and we set this access token as a cookie, then when you visited the location provider's home page in the browser, we would then send the access token back to them. This break the sandboxing we were hoping for as it uses the access token is for much more than its current limited purpose. I hope this helps.
(In reply to comment #14) > We basically didn't want this access token to be sent ask a cookie when ever > you visited the site in the browser. That's why you'd have set the cookie's host e.g. to "<geolocation>" (which isn't a valid host name, so the cookie'd never actually be added to any http request).
simon, if you like, file a seperate bug on this issue. I think we both can agree that this bug can be solved without converting to using the cookie manager, that if we did convert to the cookie manager we would have to clean up alot of stuff, and we would also have to look for edge cases where the host might not be checked for being valid (e.g <geolocation> somehow match an actual host). Thanks for the suggestion btw.
(In reply to comment #16) > this bug can be solved without converting to using the cookie manager, This bug's already FIXED. I've filed bug 492043 for the more general issue.
Comment on attachment 376265 [details] [diff] [review] patch v.1 a191=beltzner
Attachment #376265 - Flags: approval1.9.1? → approval1.9.1+
Rhetorical question: Has this code path ever been tested? Error: prefService.getBranch is not a function Source: file:///C:/Programme/Mozilla%20Firefox/components/NetworkGeolocationProvider.js Line: 176
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 377411 [details] [diff] [review] follow-up fix how embarrassing.
Attachment #377411 - Flags: review?(doug.turner) → review+
This also needs check-in on the 1.9.1 branch (I guess approval carries over, considering that this is pretty much the same patch as has already gone in).
Perhaps a unit test or at least a litmus test would be appropriate, let me know if you would like me to hack at it.
@natch, that would be awesome. also, we should add litmus tests for "clear recent history...", enter/exit private browsing mode, changing permissions via page info. If you like, I can file separate bugs? @zeniko, thanks again. i will push to 1.9.1 tomorrow after I can verify the fix on the trunk.
Actually, since this deals with session cookies, this can't be tested automatically now since mochitests don't support restarting the browser atm. Should go in litmus though. I'll file another bug (or you can point me at the other bugs) to deal with testing pb and clear recent history as those can be tested automatically (if they aren't already).
Flags: in-testsuite? → in-testsuite-
the test case follow bug is 493122. simon, thanks for cleaning my mess up! http://hg.mozilla.org/releases/mozilla-1.9.1/rev/50be518fe1c0
I have one Litmus test case already for the PB scenario: https://litmus.mozilla.org/show_test.cgi?id=9321 I will check to see if I have the other scenarios covered. (In reply to comment #26) > @natch, that would be awesome. also, we should add litmus tests for "clear > recent history...", enter/exit private browsing mode, changing permissions via > page info. If you like, I can file separate bugs? > > @zeniko, thanks again. i will push to 1.9.1 tomorrow after I can verify the > fix on the trunk.
I created a Litmus test case for the Page Info scenario - https://litmus.mozilla.org/show_test.cgi?id=9585. There is also a generic test case which covers CRH - https://litmus.mozilla.org/show_test.cgi?id=9351. So I think we have the main areas covered here from a Litmus standpoint. Would be willing to hear other suggestions.
Flags: in-litmus? → in-litmus+
You need to log in before you can comment on or make changes to this bug.