Wildcards in Permissions Policy Origins #482

arichiv · 2022-08-17T13:47:29Z

This change will add support for wildcard in permissions policy structured like SCHEME://.HOST:PORT (e.g., https://.foo.com/) where a valid Origin could be constructed from SCHEME://HOST:PORT (e.g., https://foo.com/). This requires that HOST is at least eTLD+1 (a registrable domain). This means that https://.bar.foo.com/ works but https://.com/ won’t (if you want to allow all domains to use the feature, you should just delegate to ). Wildcards in the scheme and port section will be unsupported and https://.foo.com/ does not delegate to https://foo.com/.

Before, a permissions policy might need to look like:
permissions-policy: ch-ua-platform-version=(self "https://foo.com" "https://cdn1.foo.com" "https://cdn2.foo.com")

With this feature, it could look like:
permissions-policy: ch-ua-platform-version=(self "https://foo.com" "https://*.foo.com")

This flexibility assists developers that need to delegate permissions to multiple subdomains (e.g., CDNs).

This is an in-progress change, the following components are needed:

Update ABNF
Define matching
Add example

Design Doc: https://docs.google.com/document/d/1_Y5qRs0CO2_R7o8M_YaVAWQvyjXniG70RG2z1a2uwN8/edit

closes #479

Preview | Diff

This change will add support for wildcard in permissions policy structured like SCHEME://*.HOST:PORT (e.g., https://*.foo.com/) where a valid Origin could be constructed from SCHEME://HOST:PORT (e.g., https://foo.com/). This requires that HOST is at least eTLD+1 (a registrable domain). This means that https://*.bar.foo.com/ works but https://*.com/ won’t (if you want to allow all domains to use the feature, you should just delegate to *). Wildcards in the scheme and port section will be unsupported and https://*.foo.com/ does not delegate to https://foo.com/. Before, a permissions policy might need to look like: permissions-policy: ch-ua-platform-version=(self "https://foo.com" "https://cdn1.foo.com" "https://cdn2.foo.com") With this feature, it could look like: permissions-policy: ch-ua-platform-version=(self "https://foo.com" "https://*.foo.com") This flexibility assists developers that need to delegate permissions to multiple subdomains (e.g., CDNs). This is an in-progress change, the following components are needed: - [x] Update ABNF - [ ] Define matching - [ ] Add example closes #479

arichiv · 2022-08-17T13:48:45Z

@yoavweiss have a sec for review? I want to get the ABNF correct before moving on to deeper integration

The other option is to do something more explicit like https://www.w3.org/TR/CSP2/#host_part, but it seemed desirable to not reinvent the wheel since we aren't supporting wildcards everywhere.

arichiv · 2022-08-17T17:27:37Z

Forgot @yoavweiss is out, @clelland have time to take a look?

index.bs

arichiv · 2022-08-29T20:28:55Z

Updated with the matching logic.

index.bs

yoavweiss

Non-authoritative LGTM

clelland · 2022-09-02T20:22:24Z

I think there's still a piece missing here -- we're going to need at least 4 parts to incorporate this:

The syntax of an origin with a wildcard (which I think is good)
An internal representation that can be part of an allowlist
The parsing logic to get from a header string to that internal representation
The matching logic to compare that internal representation to an origin in algorithms

So far, this has the syntax, and correctly defines serialized-origin-with-wildcard-subdomain as a string, but those strings aren't allowed in allowlists (per https://w3c.github.io/webappsec-permissions-policy/#allowlists), and I think that the matching logic is trying to operate directly on those strings, which is awkward -- and to do it right, we'd need to convert back and forth between strings and origins.

A way to resolve this would be to change the definition of what an allowlist can contain -- define an "origin with possible wildcard" to be an origin, and a boolean that says whether it also matches subdomains. Then the matching logic can check that boolean to know whether to do the deeper match.

Regardless of how we handle it, we'll need to change 9.2. Construct policy from dictionary and origin and 9.3. Parse policy directive to parse the serialized-origin-with-wildcard-subdomain strings explicitly. Currently the call to the URL parser will fail because of the wildcards.

arichiv · 2022-09-15T15:52:40Z

@clelland good point! I think I integrated at the necessary spots now.

index.bs

…atches This CL focuses just on the wildcard subdomain matching function itself as it's a dangerous point of failure and needs deep review. Design doc: w3c/webappsec-permissions-policy#482 This CL is part of a series: (1) Add function to detect subdomain matches (2) Use OriginWithPossibleWildcards in policy (3) Parse wildcard subdomain matches in policy (4) Add WPTs Bug: 1345994 Change-Id: I42b4513038e91db0883d3e9afa77c7ebed57e3fb Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3910955 Auto-Submit: Ari Chivukula <[email protected]> Reviewed-by: Dominick Ng <[email protected]> Reviewed-by: Yoav Weiss <[email protected]> Reviewed-by: Ian Clelland <[email protected]> Commit-Queue: Ari Chivukula <[email protected]> Cr-Commit-Position: refs/heads/main@{#1052043}

… policy This CL just propagates OriginWithPossibleWildcards up and down the stack. In all cases, the wildcard bool will be set to false as only the next CL will start parsing cases where wildcards are on. Design doc: w3c/webappsec-permissions-policy#482 This CL is part of a series: (1) Add function to detect subdomain matches (2) Use OriginWithPossibleWildcards in policy (3) Parse wildcard subdomain matches in policy (4) Add WPTs Bug: 1345994 Change-Id: Id38e069b434193ad3b2e77ec94ccc692a7db31fe Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3913410 Reviewed-by: Christoph Schwering <[email protected]> Auto-Submit: Ari Chivukula <[email protected]> Reviewed-by: Bo Liu <[email protected]> Reviewed-by: Dominick Ng <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Reviewed-by: Yao Xiao <[email protected]> Commit-Queue: Ari Chivukula <[email protected]> Reviewed-by: danakj <[email protected]> Cr-Commit-Position: refs/heads/main@{#1052570}

annevk

I thought we reached agreement at TPAC that this should share logic with CSP?

index.bs

annevk · 2022-10-05T13:11:29Z

index.bs

+          with <var>origin</var> and the boolean <code>True</code>, then:</li>
+          <ol>
+            <li>If <var>origin</var> is not a <a>tuple origin</a>, then return false.<li>
+            <li>If <var>origin</var>'s <a>host</a> has a null <a>registrable domain</a>, return false.<li>


This has to refer to the host of an origin using the for attribute.

Could you explain how you resolved this, as far as I can tell you reordered the words, but that doesn't help the fact that host points to the wrong thing.

I think I had misunderstood, is it correct now?

annevk · 2022-10-05T13:12:00Z

index.bs

+            <li>Set <var>originWithoutWildcard</var> to be equal to the first value of the tuple <var>item</var>.</li>
+            <li>Set <var>originCandidate</var> to be equal to <var>origin</var> with the sequence
+            of <a>code points</a> in the <a>host</a> from the start up to and including the first "." removed.</li>
+            <li>While <var>originCandidate</var> has a <a>host</a> with a non-nill <a>registrable domain</a>:</li>


Same problem with the host field as above.

Also, it's non-null.

index.bs

arichiv · 2022-10-05T14:01:42Z

I thought we reached agreement at TPAC that this should share logic with CSP?

I thought the agreement was that any further expansion of the wildcard syntax would require sharing logic with CSP, but that for this restrictive wildcard support a local modification was acceptable.

annevk · 2022-10-05T14:30:32Z

I think that any kind of wildcard parsing needs to have shared logic. We don't want multiple independent code paths for this that need to be audited separately and have the opportunity for divergence if future maintainers are not careful.

We can see the entire thing work end to end now! This includes a fix for the JS permissions inspection API now that it's tested e2e. Design doc: w3c/webappsec-permissions-policy#482 This CL is part of a series: (1) Add function to detect subdomain matches (2) Use OriginWithPossibleWildcards in policy (3) Propagate node source to allow list parser (4) Parse wildcard subdomain matches (5) Add WPTs (6) Enable by default Bug: 1345994 Change-Id: I8826db4e74eb8a40e09e176459fe5f84343d456e

We can see the entire thing work end to end now! This includes a fix for the JS permissions inspection API now that it's tested e2e. Design doc: w3c/webappsec-permissions-policy#482 This CL is part of a series: (1) Add function to detect subdomain matches (2) Use OriginWithPossibleWildcards in policy (3) Propagate node source to allow list parser (4) Parse wildcard subdomain matches (5) Support wildcards in getAllowlistForFeature (6) Add WPTs (7) Enable by default Bug: 1345994 Change-Id: I8826db4e74eb8a40e09e176459fe5f84343d456e

We can see the entire thing work end to end now! Design doc: w3c/webappsec-permissions-policy#482 This CL is part of a series: (1) Add function to detect subdomain matches (2) Use OriginWithPossibleWildcards in policy (3) Propagate node source to allow list parser (4) Parse wildcard subdomain matches (5) Support wildcards in getAllowlistForFeature (6) Add WPTs (7) Enable by default Bug: 1345994 Change-Id: I8826db4e74eb8a40e09e176459fe5f84343d456e

…t parser The next CL will actually use the propagated type, but for now the important thing is to preserve the type so that we can differentiate attribute policies from header policies. Why not fold this CL into the next one which actually uses the type? This code is especially sensitive and I want to divide off portions where possible since the next CL has some very risky code. Why not add a test? This code is all internal, so a DCHECK and existing tests seems sufficient. Design doc: w3c/webappsec-permissions-policy#482 This CL is part of a series: (1) Add function to detect subdomain matches (2) Use OriginWithPossibleWildcards in policy (3) Propagate node source to allow list parser (4) Parse wildcard subdomain matches (5) Support wildcards in getAllowlistForFeature (6) Add WPTs (7) Enable by default Bug: 1345994 Change-Id: I9f5e47f3ca04b63dce0c14c03745a187a4f3171a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3936205 Commit-Queue: Ari Chivukula <[email protected]> Auto-Submit: Ari Chivukula <[email protected]> Reviewed-by: Ian Clelland <[email protected]> Reviewed-by: Daniel Murphy <[email protected]> Cr-Commit-Position: refs/heads/main@{#1056236}

This CL finally implements the critical `origin with subdomain wildcard parser` portion of the code! It's not a lot of code, but needs some deep testing to be sure we aren't opening ourselves up to trouble. WPTs are split into the next CL so that we don't distract from targeted review here. Design doc: w3c/webappsec-permissions-policy#482 This CL is part of a series: (1) Add function to detect subdomain matches (2) Use OriginWithPossibleWildcards in policy (3) Propagate node source to allow list parser (4) Parse wildcard subdomain matches (5) Support wildcards in getAllowlistForFeature (6) Add WPTs (7) Enable by default Bug: 1345994 Change-Id: Ic8dc79be3262cdd872987c818ce01e959cdce83a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3934705 Commit-Queue: Ari Chivukula <[email protected]> Reviewed-by: Ian Clelland <[email protected]> Auto-Submit: Ari Chivukula <[email protected]> Cr-Commit-Position: refs/heads/main@{#1056237}

…orFeature We need to be sure to fix the getAllowlistForFeature function so that wildcard subdomain permissions aren't 'invisible' to to frontend users. Design doc: w3c/webappsec-permissions-policy#482 This CL is part of a series: (1) Add function to detect subdomain matches (2) Use OriginWithPossibleWildcards in policy (3) Propagate node source to allow list parser (4) Parse wildcard subdomain matches (5) Support wildcards in getAllowlistForFeature (6) Add WPTs (7) Enable by default Bug: 1345994 Change-Id: Ib41c07c9027fa1aef4b7672155e3232b5372392c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3938265 Reviewed-by: Ian Clelland <[email protected]> Auto-Submit: Ari Chivukula <[email protected]> Commit-Queue: Ari Chivukula <[email protected]> Cr-Commit-Position: refs/heads/main@{#1056241}

We can see the entire thing work end to end now! Design doc: w3c/webappsec-permissions-policy#482 This CL is part of a series: (1) Add function to detect subdomain matches (2) Use OriginWithPossibleWildcards in policy (3) Propagate node source to allow list parser (4) Parse wildcard subdomain matches (5) Support wildcards in getAllowlistForFeature (6) Add WPTs (7) Enable by default Bug: 1345994 Change-Id: I8826db4e74eb8a40e09e176459fe5f84343d456e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3938164 Auto-Submit: Ari Chivukula <[email protected]> Reviewed-by: Koji Ishii <[email protected]> Commit-Queue: Ari Chivukula <[email protected]> Cr-Commit-Position: refs/heads/main@{#1056243}

This inverts the VTS so we retain testing of the disabled code path. Design doc: w3c/webappsec-permissions-policy#482 This CL is part of a series: (1) Add function to detect subdomain matches (2) Use OriginWithPossibleWildcards in policy (3) Propagate node source to allow list parser (4) Parse wildcard subdomain matches (5) Support wildcards in getAllowlistForFeature (6) Add WPTs (7) Enable by default Bug: 1345994 Change-Id: I269c863da497b1fb352801e848e63c4733fc46f4 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3938165 Reviewed-by: Koji Ishii <[email protected]> Auto-Submit: Ari Chivukula <[email protected]> Reviewed-by: Ian Clelland <[email protected]> Cr-Commit-Position: refs/heads/main@{#1056250}

We can see the entire thing work end to end now! Design doc: w3c/webappsec-permissions-policy#482 This CL is part of a series: (1) Add function to detect subdomain matches (2) Use OriginWithPossibleWildcards in policy (3) Propagate node source to allow list parser (4) Parse wildcard subdomain matches (5) Support wildcards in getAllowlistForFeature (6) Add WPTs (7) Enable by default Bug: 1345994 Change-Id: I8826db4e74eb8a40e09e176459fe5f84343d456e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3938164 Auto-Submit: Ari Chivukula <[email protected]> Reviewed-by: Koji Ishii <[email protected]> Commit-Queue: Ari Chivukula <[email protected]> Cr-Commit-Position: refs/heads/main@{#1056243}

…tions Resolve TODOs in web_applications and support subdomain wildcard. By using the new Serialize/Parse functions we get support for free. This also adds some more testing to hedge against breakage. Design doc: w3c/webappsec-permissions-policy#482 This CL is part of a series: (1) Add Serialize/Parse functions (2) Use Serialize/Parse in web_applications Bug: 1345994 Change-Id: Id60e022e9f520eda17d20345fd803e1a253c69c0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3933647 Reviewed-by: Dominick Ng <[email protected]> Commit-Queue: Dominick Ng <[email protected]> Auto-Submit: Ari Chivukula <[email protected]> Cr-Commit-Position: refs/heads/main@{#1056824}

…PTs, a=testonly Automatic update from web-platform-tests [Permissions Policy Wildcards] (6) Add WPTs We can see the entire thing work end to end now! Design doc: w3c/webappsec-permissions-policy#482 This CL is part of a series: (1) Add function to detect subdomain matches (2) Use OriginWithPossibleWildcards in policy (3) Propagate node source to allow list parser (4) Parse wildcard subdomain matches (5) Support wildcards in getAllowlistForFeature (6) Add WPTs (7) Enable by default Bug: 1345994 Change-Id: I8826db4e74eb8a40e09e176459fe5f84343d456e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3938164 Auto-Submit: Ari Chivukula <[email protected]> Reviewed-by: Koji Ishii <[email protected]> Commit-Queue: Ari Chivukula <[email protected]> Cr-Commit-Position: refs/heads/main@{#1056243} -- wpt-commits: 65aefb278c8c05feb767bebd3840b1b2dd2d7c61 wpt-pr: 36314

arichiv · 2023-06-01T13:53:21Z

Restarting this since it's been a while. Will live on in:
#516

…PTs, a=testonly Automatic update from web-platform-tests [Permissions Policy Wildcards] (6) Add WPTs We can see the entire thing work end to end now! Design doc: w3c/webappsec-permissions-policy#482 This CL is part of a series: (1) Add function to detect subdomain matches (2) Use OriginWithPossibleWildcards in policy (3) Propagate node source to allow list parser (4) Parse wildcard subdomain matches (5) Support wildcards in getAllowlistForFeature (6) Add WPTs (7) Enable by default Bug: 1345994 Change-Id: I8826db4e74eb8a40e09e176459fe5f84343d456e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3938164 Auto-Submit: Ari Chivukula <[email protected]> Reviewed-by: Koji Ishii <[email protected]> Commit-Queue: Ari Chivukula <[email protected]> Cr-Commit-Position: refs/heads/main@{#1056243} -- wpt-commits: 65aefb278c8c05feb767bebd3840b1b2dd2d7c61 wpt-pr: 36314

clelland reviewed Aug 26, 2022

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

arichiv added 3 commits August 29, 2022 16:03

Update index.bs

95dd38a

Update index.bs

3d32a4b

Update index.bs

a0c3f34

yoavweiss reviewed Aug 30, 2022

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

arichiv added 3 commits August 30, 2022 12:38

Update index.bs

2e1edfa

Update index.bs

4f863dc

Update index.bs

8d8904e

yoavweiss reviewed Aug 31, 2022

View reviewed changes

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

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

arichiv added 2 commits August 31, 2022 12:59

Update index.bs

5b37685

Update index.bs

8421b5a

arichiv requested review from clelland and yoavweiss and removed request for clelland and yoavweiss September 1, 2022 15:54

yoavweiss reviewed Sep 2, 2022

View reviewed changes

arichiv added 3 commits September 15, 2022 11:32

Update index.bs

5b4453a

Update index.bs

947f92c

Update index.bs

fe2d861

clelland requested changes Sep 26, 2022

View reviewed changes

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

Update index.bs

c20a0d7

arichiv requested a review from clelland October 4, 2022 15:12

annevk reviewed Oct 5, 2022

View reviewed changes

arichiv and others added 4 commits October 5, 2022 09:51

Update index.bs

3974fac

Update index.bs

af420af

Update index.bs

f40a6e8

Merge branch 'w3c:main' into patch-1

2a48716

Update index.bs

7f4572c

chromium-wpt-export-bot mentioned this pull request Oct 6, 2022

[Permissions Policy Wildcards] (6) Add WPTs web-platform-tests/wpt#36314

Merged

arichiv closed this Jun 1, 2023

Wildcards in Permissions Policy Origins #482

Wildcards in Permissions Policy Origins #482

Uh oh!

Conversation

arichiv commented Aug 17, 2022 • edited by pr-preview bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arichiv commented Aug 17, 2022

Uh oh!

arichiv commented Aug 17, 2022

Uh oh!

Uh oh!

Uh oh!

arichiv commented Aug 29, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yoavweiss left a comment

Choose a reason for hiding this comment

Uh oh!

clelland commented Sep 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arichiv commented Sep 15, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

annevk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

annevk Oct 5, 2022

Choose a reason for hiding this comment

Uh oh!

annevk Oct 5, 2022

Choose a reason for hiding this comment

Uh oh!

arichiv Oct 5, 2022

Choose a reason for hiding this comment

Uh oh!

annevk Oct 5, 2022

Choose a reason for hiding this comment

Uh oh!

annevk Oct 5, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arichiv commented Oct 5, 2022

Uh oh!

annevk commented Oct 5, 2022

Uh oh!

arichiv commented Jun 1, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

arichiv commented Aug 17, 2022 •

edited by pr-preview bot

Loading

clelland commented Sep 2, 2022 •

edited

Loading