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

Clarification needed regarding permissions policy integration #44

Open
bershanskiy opened this issue Oct 26, 2023 · 0 comments · May be fixed by #45
Open

Clarification needed regarding permissions policy integration #44

bershanskiy opened this issue Oct 26, 2023 · 0 comments · May be fixed by #45

Comments

@bershanskiy
Copy link

bershanskiy commented Oct 26, 2023

The Question

As of now, the getLayoutMap() algorithm states:

If this's relevant global object's associated Document is not allowed to use the policy-controlled permission named "keyboard-map", reject the promise.

But the Privacy Mitigations section states:

[T]he API is only available from secure contexts and can only be called from the currently active top-level browsing context or granted access via policy-controlled feature.

These two statements contradict each other in the case when top-level browsing context does not have the permission.

As of October 2023, Chrome/Chromium implementation follows the Privacy Mitigations version and ignores the permission policy state in top-level frame and allows access even if keyboard-map is explicitly forbidden. Chromium issue here. As of October 2023, there is no WPT specifically for this case.

Background

Originally, getLayoutMap() was allowed only from top-level context and did not have any other restrictions. Then in #38 there was some discussion about access from sub-frames, which resulted in keyboard-map Permissions Policy.

Chronology of most important events:

  • commit 78cfc48 added keyboard-map permission identifier with default allow-list 'self'
  • commit 9237b26 updated the getLayoutMap() algorithm to use the permission replacing the mention of top-level browsing context with the new permission. Please note that since the default allow list was 'self', the top-level document was still allowed access by default.
  • in this commit Chromium implemented keyboard-map permission but did not remove the mention of top-level browsing context, effectively putting it in conflict with the specification. Note: the new WPT did not get a dedicated test specifically for the case of top-level context having explicit Permissions Policy keyboard-map=() and trying to access the API.
  • commit bf67454 updated the "Privacy Mitigations" section adding wording about keyboard-map permission but not removing the mention of top-level browsing context. This was in direct contradiction with the previous commit 9237b26.
  • (inconsequential) Chromium commit moved the check from Blink renderer process to the core browser process.
  • (inconsequential) Chromium issue was filed specifically about this ambiguity.

Possible solutions

Remove access from top-level context when it is explicitly forbidden by Permissions Policy

  • This will likely reflect the intent of the Permissions Policy system better.
  • This will not change behavior for most websites since they do not specify keyboard-map permission.
  • This will require a one-line change to Chromium KeyboardLockServiceImpl::GetKeyboardLayoutMap() method.
  • This will require change to "Privacy Mitigations" section. PR Update Privacy Mitigations to align with getLayoutMap() algorithm #45 implements this change.
  • This will require a WPT test.

Allow access from top-level context even in case of explicit restriction

  • This will reflect the current Chromium behavior better.
  • This will not change behavior for any websites at all.
  • This will require no changes on Chromium side.
  • This will require change to the getLayoutMap() algorithm.
  • This will require a WPT test.

I can crate a Chromium CL for either solution once there is a consensus which option to take. I would favor the first solution (consistency with Permissions Policy).

Related discussion

This came up in uBlock Origin discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant