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

Prohibit Create Credential from cross-origin iframes #1394

Merged

Conversation

jcjones
Copy link
Contributor

@jcjones jcjones commented Mar 24, 2020

This reverts part of PR #1276, again prohibiting the use of the Create method when sameOriginWithAncestors is false. The Note is simplified, since the integration between Credential Management and Feature Policy is now complete.


Preview | Diff

@jcjones jcjones self-assigned this Mar 24, 2020
@jcjones jcjones linked an issue Mar 24, 2020 that may be closed by this pull request
@jcjones
Copy link
Contributor Author

jcjones commented Mar 24, 2020

This will also need to rebase on #1393.

Copy link
Contributor

@agl agl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to split the feature-policy, right?

@jcjones
Copy link
Contributor Author

jcjones commented Mar 25, 2020

We also need to split the feature-policy, right?

Do we?

@kenrb
Copy link

kenrb commented Mar 25, 2020

Or rename the feature policy, at least? I thought the idea was that if we decide later to allow MakeCredential from cross-origin iframes, we would need a new feature policy, so as not to change the behavior of an existing one for sites that might be using it. So the feature policy being added for this should correspond specifically to GetAssertion, if I understand correctly.

@ve7jtb
Copy link
Contributor

ve7jtb commented Mar 25, 2020

We agreed at the F2F to split the policy so we could add make credential later if we decide to without breaking the current get behavior.

@nadalin nadalin requested a review from akshayku March 25, 2020 19:18
@nadalin nadalin added this to the L2-WD-03 milestone Mar 25, 2020
@agl
Copy link
Contributor

agl commented Mar 25, 2020

Discussed on the call: we're going to rename with, for example, -create and -get.

This reverts part of PR w3c#1276, again prohibiting the use of the Create method
when `sameOriginWithAncestors` is `false`. The `Note` is simplified, since
the integration between Credential Management and Feature Policy is now
complete.
@jcjones jcjones force-pushed the 1336-prohibit_create_from_xorigin_iframes branch from c7a8576 to 454fc75 Compare March 25, 2020 22:53
@jcjones
Copy link
Contributor Author

jcjones commented Mar 25, 2020

Thanks for the reviews; see what you think of this revision.

@equalsJeffH
Copy link
Contributor

on 2020-04-01 call: @equalsJeffH & @akshayku to review

Copy link
Contributor

@equalsJeffH equalsJeffH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @jcjones -- overall LGTM, however feedback at this time from chrome implementor is that he does not advise defining a publickey-credentials-create policy at this time because at least some code for it would end up in browsers and just sit there (see changelist). So I'm intending to update the list of defined feature policies and define only publickey-credentials-get. Suggestions below as a result...

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
Co-Authored-By: =JeffH <jdhodges@google.com>
@jcjones jcjones requested a review from equalsJeffH April 8, 2020 19:02
@equalsJeffH
Copy link
Contributor

on 2020-04-08 call: @equalsJeffH to re-review and merge if good

Copy link
Contributor

@equalsJeffH equalsJeffH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thx @jcjones !

@equalsJeffH equalsJeffH merged commit 6626671 into w3c:master Apr 9, 2020
WebAuthnBot pushed a commit that referenced this pull request Apr 9, 2020
equalsJeffH added a commit to equalsJeffH/webappsec-feature-policy that referenced this pull request Apr 9, 2020
Due to WebAuthn issue #1336, the WebAuthn's "publickey-credentials" feature policy is renamed to "publickey-credentials-get", allowing only authentication (aka "getting an assertion") in cross-origin iframes. Credential creation is not allowed in cross-origin iframes, although it remains allowed by default (no feature-policy required) in same-origin-with-ancestors iframes.

See also w3c/webauthn#1394, and https://w3c.github.io/webauthn/#sctn-feature-policy
@equalsJeffH
Copy link
Contributor

feature policy name change PR submitted: w3c/webappsec-permissions-policy#370 "Update publickey-credentials to be "get-only" policy"

@jcjones jcjones deleted the 1336-prohibit_create_from_xorigin_iframes branch April 22, 2020 19:14
clelland pushed a commit to w3c/webappsec-permissions-policy that referenced this pull request Apr 30, 2020
Due to WebAuthn issue #1336, the WebAuthn's "publickey-credentials" feature policy is renamed to "publickey-credentials-get", allowing only authentication (aka "getting an assertion") in cross-origin iframes. Credential creation is not allowed in cross-origin iframes, although it remains allowed by default (no feature-policy required) in same-origin-with-ancestors iframes.

See also w3c/webauthn#1394, and https://w3c.github.io/webauthn/#sctn-feature-policy
@equalsJeffH
Copy link
Contributor

blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this pull request May 1, 2020
The WebAuthn spec has removed the ability to allow cross-origin iframes
to perform Web Authentication MakeCredential requests. This CL restricts
it to just GetAssertion, and reflects the renamed Feature Policy.

Feature Policy spec change:
w3c/webappsec-permissions-policy#370

WebAuthn spec change:
w3c/webauthn#1394

Bug: 993007
Change-Id: I9b5ccf05b5e39a5e5920b475111cdf797dcdb5a5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2120268
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Commit-Queue: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#764743}
Elchi3 pushed a commit to mdn/browser-compat-data that referenced this pull request May 6, 2020
Feature Policy directive controlling WebAuth was renamed from
publickey-credentials to publickey-credentials-get.
Sources:
  Feature Policy change:
    w3c/webappsec-permissions-policy#370
  WebAuthn specification change:
    w3c/webauthn#1394
  Chrome already updated the name:
    https://crbug.com/993007#c9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prohibit Create Credential from cross-origin iframes
7 participants