Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #37: Add cross-origin leaks, hijacking browsing history #38

Merged
merged 1 commit into from Sep 4, 2017

Conversation

Projects
None yet
4 participants
@anssiko
Copy link
Member

commented Aug 29, 2017

PTAL @lknik @alexshalamov @pozdnyakov

I took @lknik's initial proposal at #37 and tried to make it as concise as possible while retain enough information to help implementers make an informed decision.

I've understood from @alexshalamov and Google Security folks the currently recommended mitigations we have in place in the spec are able to mitigate these attacks, so seeking feedback whether we want to expand the recommended mitigations as suggested.


Preview | Diff

@alexshalamov

This comment has been minimized.

Copy link

commented Aug 29, 2017

@anssiko

I've understood from @alexshalamov and Google Security folks the currently recommended mitigations we have in place in the spec are able to mitigate these attacks, so seeking feedback whether we want to expand the recommended mitigations as suggested.

These two strategies mitigate cross-origin information leaks. Moreover, both are implemented in Chrome.
https://w3c.github.io/sensors/#top-level-browsing-context
https://w3c.github.io/sensors/#losing-focus

In my opinion, no need expand mitigation strategies at this point.

@anssiko

This comment has been minimized.

Copy link
Member Author

commented Aug 29, 2017

@alexshalamov, thanks for the additional details regarding mitigations that are in place in both the spec and its Chrome implementation.

@lknik, I will keep this PR open until we've received your explicit review. Thanks for your contributions.

@lknik

This comment has been minimized.

Copy link

commented Aug 29, 2017

Hello. Looks sound (perhaps it would benefit from some proof reading though).

@alexshalamov can you elaborate how top-level-browsing-context and losing-focus mitigate the risks discussed here?

Note: I would still allow permissions. In the original issue I actually addressed that some mitigations (frequency/precision) reduce risk, but may not solve all "instances". In that case, we should consider at least documenting that.

@anssiko

This comment has been minimized.

Copy link
Member Author

commented Aug 30, 2017

@lknik, please note that permissions are already part of the generic mitigation strategies, and as such, apply to Ambient Light Sensor as well, see: https://w3c.github.io/sensors/#permissions

Now that this remaining concern seems to be addressed, would you be fine with us landing this PR that fixes issue #37?

@anssiko

This comment has been minimized.

Copy link
Member Author

commented Aug 31, 2017

Gently nudging @lknik.

@lknik

This comment has been minimized.

Copy link

commented Sep 3, 2017

Oh, sure @anssiko (though maybe still not clear how top-level-browsing-context and losing-focus is fixing all of these cross-origin leaks; that said - they close many - indeed)

@anssiko

This comment has been minimized.

Copy link
Member Author

commented Sep 4, 2017

@lknik, thanks for the confirmation, we'll merge this PR now.

Your further contributions are always welcome, that said, I'm happy with the current state of the security and privacy considerations and that the spec has been scrutinized so carefully by domain experts, so I see no immediate need to embellish this section further.

@alexshalamov can explain how the specified mitigations work together to address the known threats, and perhaps shed some light on how they are implemented in Chrome (an implementation detail and as such out of scope for spec discussions, but perhaps still of interest to you).

@anssiko anssiko merged commit 0bd8f40 into gh-pages Sep 4, 2017

1 check passed

ipr PR deemed acceptable.
Details

@anssiko anssiko deleted the privacy-risks branch Sep 4, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.