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

Prevent permissive HTTPS Upgrade settings from leaking from normal to incognito windows #17421

Merged
merged 1 commit into from
Mar 2, 2023

Conversation

arthuredelstein
Copy link
Collaborator

@arthuredelstein arthuredelstein commented Mar 1, 2023

Resolves brave/brave-browser#28792

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally:
    • npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
    • npm run lint, npm run presubmit wiki, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

@arthuredelstein arthuredelstein requested a review from a team as a code owner March 1, 2023 01:41
@arthuredelstein arthuredelstein removed their assignment Mar 1, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2023

⚠️ PR head is an unsigned commit
commit: 8b9e15979cddc5acfd0749b77e5a8d2f2a68de49
reason: unsigned
Please follow the handbook to configure commit signing
cc: @arthuredelstein

@arthuredelstein arthuredelstein enabled auto-merge (squash) March 2, 2023 05:49
@arthuredelstein arthuredelstein self-assigned this Mar 2, 2023
@arthuredelstein arthuredelstein merged commit 33c3af7 into master Mar 2, 2023
@arthuredelstein arthuredelstein deleted the issues/28792 branch March 2, 2023 07:45
@github-actions github-actions bot added this to the 1.51.x - Nightly milestone Mar 2, 2023
arthuredelstein added a commit that referenced this pull request Mar 28, 2023
… incognito windows (#17421)

Prevent permissive HTTPS Upgrade settings from leaking from normal to private windows
@kjozwiak
Copy link
Member

kjozwiak commented Mar 30, 2023

Verification PASSED on Win 11 x64 using the following build(s):

Brave | 1.52.2 Chromium: 112.0.5615.39 (Official Build) nightly (64-bit)
-- | --
Revision | a0e7b9718a92bcd1cf33b7c95316caff3fc20714-refs/branch-heads/5615@{#753}
OS | Windows 11 Version 22H2 (Build 22621.1413)

Using the STR/Cases outlined via brave/brave-browser#28792 (comment), ensured the following:

Test Case #1 - Upgrade connections to HTTPS

  • launched/restarted 1.52.2 Chromium: 112.0.5615.39 so BraveHttpsByDefaultRolloutStudy:Enabled
  • visited http://upgradable.arthuredelstein.net in a Normal window and ensured that Upgrade connections to HTTPS
    • ensured that http://upgradable.arthuredelstein.net -> https://upgradable.arthuredelstein.net
  • opened a Private window and visited http://upgradable.arthuredelstein.net and ensured Upgrade connections to HTTPS
    • ensured that http://upgradable.arthuredelstein.net -> https://upgradable.arthuredelstein.net

Test Case #2 - Only connect using HTTPS

  • launched/restarted 1.52.2 Chromium: 112.0.5615.39 so BraveHttpsByDefaultRolloutStudy:Enabled
  • visited http://upgradable.arthuredelstein.net switched HTTPS upgrades to Only connect using HTTPS
    • ensured that http://upgradable.arthuredelstein.net -> https://upgradable.arthuredelstein.net
  • opened a Private window and visited http://upgradable.arthuredelstein.net and ensured Only connect using HTTPS
    • ensured that http://upgradable.arthuredelstein.net -> https://upgradable.arthuredelstein.net

Test Case #3 - Don't upgrade HTTPS connections

Ensure that Don't upgrade HTTPS connections is NOT being used

Test Case #4 - Don't upgrade HTTPS connections (Private Window Only)

  • launched/restarted 1.52.2 Chromium: 112.0.5615.39 so BraveHttpsByDefaultRolloutStudy:Enabled
  • opened a Private window and visited http://upgradable.arthuredelstein.net and ensured Upgrade connections to HTTPS
    • ensured that http://upgradable.arthuredelstein.net -> https://upgradable.arthuredelstein.net
  • change the HTTPS upgrade setting to Don't upgrade HTTPS connections and load http://upgradable.arthuredelstein.net

Ensure that http://upgradable.arthuredelstein.net is not upgrade. With this case, we're basically ensuring that you can still use Don't upgrade HTTPS connections if changed within the Private window.

kjozwiak pushed a commit that referenced this pull request Mar 30, 2023
…rmal to incognito windows (#17801)

Prevent permissive HTTPS Upgrade settings from leaking from normal to incognito windows (#17421)

Prevent permissive HTTPS Upgrade settings from leaking from normal to private windows
@kjozwiak kjozwiak mentioned this pull request Apr 27, 2023
25 tasks
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 this pull request may close these issues.

HTTPS shield setting from Normal windows are leaking into Private windows
3 participants