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

Make Chromium storage partitioning compatible with Brave ephemeral storage implementation #26165

Closed
goodov opened this issue Oct 21, 2022 · 10 comments · Fixed by brave/brave-core#21014

Comments

@goodov
Copy link
Member

goodov commented Oct 21, 2022

Chromium has been working on its own storage partitioning for a while, they've come up with StorageKey that is spread across many storage APIs. The structure is similar to NetworkIsolationKey/NetworkAnonymizationKey, but is used mostly for eTLD+1 partitioning of cross sites (relative to the top origin).

We should adopt StorageKey as much as possible, befriend it with our ephemeral storage implementation and enable upstream partitioning where possible. Some of our partitioning tasks (for ex. #24712) can use StorageKey for simpler implementation.

Here's Chromium invite to test their partitioning in the wild: https://developer.chrome.com/en/blog/storage-partitioning-dev-trial/

@goodov goodov self-assigned this Oct 21, 2022
@rebron rebron added the priority/P4 Planned work. We expect to get to it "soon". label Oct 21, 2022
@brave-builds brave-builds added this to the 1.64.x - Nightly milestone Jan 29, 2024
@goodov goodov added the QA/Yes label Feb 2, 2024
@goodov
Copy link
Member Author

goodov commented Feb 2, 2024

QA:
Run https://dev-pages.brave.software/storage/ephemeral-storage.html

@kjozwiak
Copy link
Member

QA: Run https://dev-pages.brave.software/storage/ephemeral-storage.html

Thanks @goodov 👍 Work to support Chromium Storage on the QA test pages was done via brave-experiments/qa-test-pages#16. Just adding some context.

@LaurenWags
Copy link
Member

@goodov should this be checked on Android as well - or is there a separate issue for implementation on Android?

@goodov
Copy link
Member Author

goodov commented Feb 27, 2024

This issue also covers Android and it should be tested as well.

@LaurenWags LaurenWags added the OS/Android Fixes related to Android browser functionality label Feb 27, 2024
@LaurenWags
Copy link
Member

LaurenWags commented Mar 5, 2024

Verified with

Brave | 1.64.95 Chromium: 122.0.6261.94 (Official Build) beta (x86_64)
-- | --
Revision | 6f4193fbcee7c358ae3684f777faa9f12e52206e
OS | macOS Version 13.6.4 (Build 22G513)

Per #26165 (comment), running through the tests under https://dev-pages.brave.software/storage/ephemeral-storage.html (cookie changes were made via the shields panel, not globally):

Also ran through the test under https://dev-pages.brave.software/storage/ephemeral-storage.html where cookie changes were made globally via brave://settings/shields:

  • Cross-site cookies blocked - PASSED
  • Cookies Blocked - PASSED
  • All cookies allowed - PASSED

Note, per #26165 (comment), no need to run these tests with Ephemeral Storage disabled.

@LaurenWags LaurenWags added QA Pass-macOS QA/In-Progress Indicates that QA is currently in progress for that particular issue and removed QA Pass-macOS labels Mar 5, 2024
@LaurenWags
Copy link
Member

LaurenWags commented Mar 5, 2024

@goodov two questions for you:

  1. I'm seeing a problem with Step 6 from the test page with All cookies allowed and Ephemeral Storage enabled (by default). The tables don't match. Could you take a look? Unsure if this is a problem with the test page or the browser.
Actual Expected
Screenshot 2024-03-05 at 4 04 52 PM Screenshot 2024-03-05 at 4 05 04 PM
  1. Do we need to run the tests on this page with Ephemeral Storage disabled? Test page says to but I'm not sure if there's a need since Ephemeral Storage is enabled by default afaik (looks like the drop down to indicate Ephemeral Storage enabled/disabled was removed from the page). I did give it a try by disabling brave://flags/#brave-ephemeral-storage, relaunching, and then attempting to run the first test. However, it did not go as expected, so I wanted to check before going any farther:
Screenshot 2024-03-05 at 4 17 58 PM

Thanks!

@goodov
Copy link
Member Author

goodov commented Mar 6, 2024

  1. I'm seeing a problem with Step 6 from the test page with All cookies allowed and Ephemeral Storage enabled (by default). The tables don't match. Could you take a look? Unsure if this is a problem with the test page or the browser.

That doesn't look right. Did you enable cookies globally on brave://settings/shields or via the Brave Shields popup? Please create a separate issue for this, I'll look into it.

  1. Do we need to run the tests on this page with Ephemeral Storage disabled?

No, please stop doing that. I'll update the test page to remove this text. We should not test the disabled stage at this point as it's an always-on feature.

@hffvld
Copy link
Contributor

hffvld commented Mar 7, 2024

Verified on Galaxy Tab S8 and Pixel 7 using version(s):

Device/OS: 
- Galaxy Tab S8 / gts8wifixx-user 14 UP1A.231005.007 release-keys
- Pixel 7 / panther_beta-user 14 AP21.240119.009 release-keys
Brave build: 1.64.95 
Chromium: 122.0.6261.94 (Official Build) beta (64-bit) 

STEPS:

  1. Follow the steps from Make Chromium storage partitioning compatible with Brave ephemeral storage implementation #26165 (comment)
  2. Verify

ACTUAL RESULTS:

  • Verified that Cross-site cookies blocked testing PASS
  • Verified that Cookies Blocked testing PASS
  • Verified that All cookies allowed testing PASS

Galaxy Tab S8 / Tablet
Block cross-site cookies / Cross-site cookies blocked
1 2
1 2
1 2
1 2
1 2
1 2
1 2
1 2
Block all cookies / Cookies blocked
1 2
1 2
1 2
1 2
1 2
1 2
1 2
1 2
Allow all cookies / All cookies allowed
1 2
1 2
1 2
1 2
1 2
1 2
1 2
1 2
Pixel 7 / Phone
Block cross-site cookies / Cross-site cookies blocked
1 2
1 2
1 2
1 2
1 2
1 2
1 2
1 2
Block all cookies / Cookies blocked
1 2
1 2
1 2
1 2
1 2
1 2
1 2
1 2
Allow all cookies / All cookies allowed
1 2
1 2
1 2
1 2
1 2
1 2
1 2
1 2

@stephendonner
Copy link

stephendonner commented Mar 13, 2024

Verification PASSED using

Brave | 1.64.103 Chromium: 122.0.6261.128 (Official Build) beta (64-bit)
-- | --
Revision | 41534736ca142c5829699b512024398eaa2bbfc0
OS | Windows 10 Version 22H2 (Build 19045.4170)

Cross-site cookies blocked - PASSED

Steps:

  1. installed 1.64.103
  2. launched Brave
  3. loaded https://dev-pages.brave.software/storage/ephemeral-storage.html
  4. clicked on Start test
  5. followed the rest of the directions

Step 1 - PASS

image

Step 2 - PASS

image

Step 3 - PASS

image

Step 4 - PASS

image

Step 5 - PASS

image

Step 6 - PASS

image

Block all cookies - PASSED

Step 1 - PASSED

image

Step 2 - PASSED

image

Step 3 - PASSED

image

Step 4 - PASSED

image

Step 5 - PASSED

image

Step 6 - PASSED

image

Allow all cookies - PASSED

Step 1 - PASS

image

Step 2 - PASS

image

Step 3 - PASS

image

Step 4 - PASS

image

Step 5 - PASS

image

Step 6 - FAIL

results expected
image image

Encountered:

@stephendonner stephendonner added QA/In-Progress Indicates that QA is currently in progress for that particular issue QA Pass-Win64 and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Mar 13, 2024
@stephendonner
Copy link

stephendonner commented Mar 14, 2024

Verification IN-PROGRESS using

Brave	1.64.104 Chromium: 123.0.6312.46 (Official Build) (64-bit) 
Revision	0ca3d372cf8aab96fafdd75f5c5c8d2012bc0cc6
OS	Linux

Steps:

  1. installed 1.64.104
  2. launched Brave
  3. loaded https://dev-pages.brave.software/storage/ephemeral-storage.html
  4. followed directions

Cross-site cookies blocked - PASSED

step 1 step 2 step 3 step 4 step 5 step 6
Screen Shot 2024-03-14 at 7 50 36 PM Screen Shot 2024-03-14 at 7 50 57 PM Screen Shot 2024-03-14 at 7 51 21 PM Screen Shot 2024-03-14 at 7 51 55 PM Screen Shot 2024-03-14 at 7 52 25 PM Screen Shot 2024-03-14 at 7 54 38 PM

Block all cookies - PASSED

step 1 step 2 step 3 step 4 step 5 step 6
Screen Shot 2024-03-14 at 7 29 44 PM Screen Shot 2024-03-14 at 7 32 30 PM Screen Shot 2024-03-14 at 7 34 01 PM Screen Shot 2024-03-14 at 7 35 44 PM Screen Shot 2024-03-14 at 7 36 36 PM Screen Shot 2024-03-14 at 7 38 07 PM

Allow all cookies - PASSED

step 1 step 2 step 3 step 4 step 5 step 6
Screen Shot 2024-03-14 at 8 11 15 PM Screen Shot 2024-03-14 at 8 12 01 PM Screen Shot 2024-03-14 at 8 12 31 PM Screen Shot 2024-03-14 at 8 12 52 PM Screen Shot 2024-03-14 at 8 13 11 PM Screen Shot 2024-03-14 at 8 14 39 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment