Migrate to HttpsFirstMode v2 #17856

arthuredelstein · 2023-03-31T18:46:06Z

Resolves brave/brave-browser#28935
Resolves brave/brave-browser#28809

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:

chromium_src/chrome/common/chrome_features.cc

chromium_src/chrome/browser/ssl/https_upgrades_navigation_throttle.cc

chromium_src/chrome/browser/ssl/https_upgrades_interceptor.cc

test/filters/browser_tests-macos.filter

chromium_src/chrome/browser/ssl/https_upgrades_interceptor.cc

arthuredelstein · 2023-04-17T22:21:10Z

Thanks for the reviews!

Migrate HTTPS by Default feature to use HttpsFirstMode v2

kjozwiak · 2023-04-27T05:22:13Z

`Test Case #1` - brave/brave-browser#28809

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

Brave | 1.52.76 Chromium: 113.0.5672.53 (Official Build) nightly (64-bit)
-- | --
Revision | 12f5dac35d12e8f4e72d7dd11df557ef93bc046f-refs/branch-heads/5672@{#703}
OS | Windows 11 Version 22H2 (Build 22621.1555)

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

changed UA to Android (4.0.2) as per the screenshot/examples below
loaded https://anicobin.ldblog.jp/archives/59981055.html and ensured that clicking on the comment image loads the page

`Example`	`Example`	`Example`

Verification PASSED on Pixel 6 running Android 14 using the following build(s):

Brave | 1.53.1 Chromium: 113.0.5672.53 (Official Build) canary (32-bit)
--- | ---
Revision | 12f5dac35d12e8f4e72d7dd11df557ef93bc046f-refs/branch-heads/5672@{#703}
OS | Android 13; Build/UPB1.230309.014; 33; UpsideDownCake

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

loaded https://anicobin.ldblog.jp/archives/59981055.html and ensured that clicking on the comment image loads the page

`Example`	`Example`	`Example`

`Test Case #2` - brave/brave-browser#28935

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

Brave | 1.52.76 Chromium: 113.0.5672.53 (Official Build) nightly (64-bit)
-- | --
Revision | 12f5dac35d12e8f4e72d7dd11df557ef93bc046f-refs/branch-heads/5672@{#703}
OS | Windows 11 Version 22H2 (Build 22621.1555)

Shields Panel (`Upgrade connections to HTTPS`)

Using the STR/Cases mentioned via brave/brave-browser#27141 (comment), went through the following:

ensured that Upgrade connections to HTTPS is selected as the default
- ensured that http://insecure.arthuredelstein.net loads without any issues
- ensured that http://http.badssl.com loads without any issues
- ensured that http://upgradable.arthuredelstein.net -> https://upgradable.arthuredelstein.net (should be upgraded)

`Example`	`Example`	`Example`

Shields Panel (`Only connect with HTTPS`)

visit http://insecure.arthuredelstein.net, http://http.badssl.com and http://upgradable.arthuredelstein.net and ensure that Only connect with HTTPS is selected via the shields panel
- ensure that http://insecure.arthuredelstein.net displays The connection to insecure.arthuredelstein.net is not secure
  - ensure that Continue to site loads http://insecure.arthuredelstein.net without any issues
  - ensured that http://insecure.arthuredelstein.net loads without any issues after several restarts once Continue is selected
  - ensured that you Turn on warnings works as expected via the Not Secure drop down
- ensure that http://http.badssl.com displays The connection to http.badssl.com is not secure
  - ensure that Continue to site loads http://http.badssl.com without any issues
  - ensured that http://http.badssl.com loads without any issues after several restarts once Continue is selected
  - ensured that you Turn on warnings works as expected via the Not Secure drop down
- ensured that http://upgradable.arthuredelstein.net -> https://upgradable.arthuredelstein.net (should be upgraded)

`Example`	`Example`	`Example`	`Example`	`Example`	`Example`	`Example`

Shields Panel (`Don't upgrade HTTP connections`)

visit http://insecure.arthuredelstein.net, http://http.badssl.com and http://upgradable.arthuredelstein.net and ensure that Don't upgrade HTTP connections is selected via the shields panel
- ensure that http://insecure.arthuredelstein.net loads without any issues
- ensure that http://http.badssl.com loads without any issues
- ensured that http://upgradable.arthuredelstein.net doesn't upgrade to HTTPS (make sure website loads without issues)
  - basically ensuring that https://upgradable.arthuredelstein.net doesn't load

`Example`	`Example`	`Example`

`Tor Windows`

As per brave/brave-browser#27141 (comment), brave://settings are isolated on Tor windows and Strict should always be used.

change Upgrade connections to HTTPS via brave://settings from Standard -> Disabled
open a Tor window and wait till it connects to the Tor network and you receive the Tor connected successfully message
ensure that http://insecure.arthuredelstein.net displays The connection to insecure.arthuredelstein.net is not secure
- ensured that Upgrade connections to HTTPS settings are not being displayed via the shields panel
- ensure that Continue to site loads http://insecure.arthuredelstein.net without any issues
- ensured that http://insecure.arthuredelstein.net loads without any issues after several restarts once Continue is selected
- ensured that you Turn on warnings works as expected via the Not Secure drop down
ensure that http://http.badssl.com displays The connection to http.badssl.com is not secure
- ensured that Upgrade connections to HTTPS settings are not being displayed via the shields panel
- ensure that Continue to site loads http://http.badssl.com without any issues
- ensured that http://http.badssl.com loads without any issues after several restarts once Continue is selected
- ensured that you Turn on warnings works as expected via the Not Secure drop down
ensured that http://upgradable.arthuredelstein.net -> https://upgradable.arthuredelstein.net (should be upgraded)
- ensured that Upgrade connections to HTTPS settings are not being displayed via the shields panel

`Example`	`Example`	`Example`

Verification PASSED on Pixel 6 running Android 14 using the following build(s):

Brave | 1.53.1 Chromium: 113.0.5672.53 (Official Build) canary (32-bit)
--- | ---
Revision | 12f5dac35d12e8f4e72d7dd11df557ef93bc046f-refs/branch-heads/5672@{#703}
OS	Android 13; Build/UPB1.230309.014; 33; UpsideDownCake

Shields Panel (`Upgrade to HTTPS whenever possible (default)`)

ensured that Upgrade to HTTPS whenever possible (default) is selected as the default (if the user hasn't changed anything)
- ensured that http://insecure.arthuredelstein.net loads without any issues (shouldn't be upgrading)
- ensured that http://http.badssl.com loads without any issues (shouldn't be upgrading)
- ensured that http://upgradable.arthuredelstein.net -> https://upgradable.arthuredelstein.net (should be upgraded)

`Example`	`Example`	`Example`	`Example`

Shields Panel (`Require all connections to use HTTPS (strict)`)

visit http://insecure.arthuredelstein.net, http://http.badssl.com and http://upgradable.arthuredelstein.net and ensure that Require all connections to use HTTPS (strict) is selected via the shields panel
- ensure that http://insecure.arthuredelstein.net displays The connection to insecure.arthuredelstein.net is not secure
  - ensure that Continue to site loads http://insecure.arthuredelstein.net without any issues
  - ensured that http://insecure.arthuredelstein.net loads without any issues after several restarts once Continue is selected
  - ensured that you Turn on warnings works as expected via the Not Secure drop down
- ensure that http://http.badssl.com displays The connection to http.badssl.com is not secure
  - ensure that Continue to site loads http://http.badssl.com without any issues
  - ensured that http://http.badssl.com loads without any issues after several restarts once Continue is selected
  - ensured that you Turn on warnings works as expected via the Not Secure drop down
- ensured that http://upgradable.arthuredelstein.net -> https://upgradable.arthuredelstein.net (should be upgraded)

`Example`	`Example`	`Example`	`Example`	`Example`

`Example`	`Example`	`Example`	`Example`	`Example`

Shields Panel (`Don't upgrade connections to HTTPS (disabled)`)

visit http://insecure.arthuredelstein.net, http://http.badssl.com and http://upgradable.arthuredelstein.net and ensure that Don't upgrade connections to HTTPS (disabled) is selected via the shields panel
- ensure that http://insecure.arthuredelstein.net loads without any issues
- ensure that http://http.badssl.com loads without any issues
- ensured that http://upgradable.arthuredelstein.net doesn't upgrade to HTTPS (make sure website loads without issues)
  - basically ensuring that https://upgradable.arthuredelstein.net doesn't load

`Example`	`Example`	`Example`	`Example`

Prevent permissive HTTPS Upgrade settings from leaking from `Normal` to `Private` windows

Basically used the STR/Cases outlined via #17421 (comment) and went through the following:

`Test Case #1` - `Upgrade to HTTPS whenever possible (default)`

visited http://upgradable.arthuredelstein.net in a Normal window and ensured that Upgrade to HTTPS whenever possible (default)
- ensured that http://upgradable.arthuredelstein.net -> https://upgradable.arthuredelstein.net
opened a Private window and visited http://upgradable.arthuredelstein.net and ensured Upgrade to HTTPS whenever possible (default)
- ensured that http://upgradable.arthuredelstein.net -> https://upgradable.arthuredelstein.net

`Test Case #2` - `Require all connections to use HTTPS (strict)`

visited http://upgradable.arthuredelstein.net and switched HTTPS upgrades to Require all connections to use HTTPS (strict)
- ensured that http://upgradable.arthuredelstein.net -> https://upgradable.arthuredelstein.net
opened a Private window and visited http://upgradable.arthuredelstein.net and ensured Require all connections to use HTTPS (strict)
- ensured that http://upgradable.arthuredelstein.net -> https://upgradable.arthuredelstein.net

`Test Case #3` - `Don't upgrade connections to HTTPS (disabled)`

visited http://upgradable.arthuredelstein.net switched HTTPS upgrades to Don't upgrade connections to HTTPS (disabled)
reloaded http://upgradable.arthuredelstein.net and ensured that the website was not being upgraded to HTTPS
opened a Private window and visited http://upgradable.arthuredelstein.net and ensured Upgrade to HTTPS whenever possible (default)
- ensured that http://upgradable.arthuredelstein.net -> https://upgradable.arthuredelstein.net

Ensure that Don't upgrade connections to HTTPS (disabled) is NOT being used

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

opened a Private window and visited http://upgradable.arthuredelstein.net and ensured Upgrade to HTTPS whenever possible (default)
- ensured that http://upgradable.arthuredelstein.net -> https://upgradable.arthuredelstein.net
change the HTTPS upgrade setting to Don't upgrade connections to HTTPS (disabled) 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.

Migrate HTTPS by Default feature to use HttpsFirstMode v2

…lback (#18141) (#18179) * Migrate to HttpsFirstMode v2 (#17856) Migrate HTTPS by Default feature to use HttpsFirstMode v2 * Force HTTPS Upgrader to fall back to HTTP if we have an HTTP error code (#18141)

commit c98ff5e Author: brave-builds <[email protected]> Date: Fri Apr 28 08:29:10 2023 +0000 1.51.107 commit ce813ef Author: Max <[email protected]> Date: Fri Apr 28 04:25:41 2023 -0400 Upgrade from Chromium 113.0.5672.53 to Chromium 113.0.5672.63 (1.51.x). (#18269) * Upgrade from Chromium 113.0.5672.53 to Chromium 113.0.5672.63 * Upgrade patches from Chromium 113.0.5672.53 to Chromium 113.0.5672.63 * Update pins list timestamp * Temporarily disables WindowClosingConfirmBrowserTest.TestWithDownload on MacOS. The test fails because in Chromium Private/Incognito profiles now ignore the "Ask for download location" setting and always prompt for the location. We will fix that in brave/brave-browser#29823, but until then let's disable this test on MacOS. --------- Co-authored-by: brave-builds <[email protected]> commit b18aeae Author: brave-builds <[email protected]> Date: Fri Apr 28 10:15:10 2023 +0200 Disable site affiliation fetcher (uplift to 1.51.x) (#18286) Uplift of #18153 (squashed) to release commit 6b1db58 Author: brave-builds <[email protected]> Date: Fri Apr 28 09:47:20 2023 +0200 Revise the behavior of "Extensions (Brave Wallet fallback)" setting (uplift to 1.51.x) (#18228) Uplift of #18172 (squashed) to beta commit f7f8042 Author: brave-builds <[email protected]> Date: Fri Apr 28 09:42:41 2023 +0200 Send kUAModel and kUAPlatformVersion CHs when requested. (uplift to 1.51.x) (#18263) Uplift of #18154 (squashed) to beta commit 545fa57 Author: Max <[email protected]> Date: Fri Apr 28 03:07:51 2023 -0400 Don't call GetSessionRoute() when media router is disabled (#18245) (1.51.x). (#18264) Don't call GetSessionRoute() when media router is disabled (#18245) Otherwise, it would end in reaching NOTREACHED() Co-authored-by: Sangwoo Ko <[email protected]> commit 0fbe592 Author: Arthur Edelstein <[email protected]> Date: Thu Apr 27 17:44:23 2023 -0700 Uplift HTTPS First Mode v2 Migration (#17856) and HTTP error code fallback (#18141) (#18179) * Migrate to HttpsFirstMode v2 (#17856) Migrate HTTPS by Default feature to use HttpsFirstMode v2 * Force HTTPS Upgrader to fall back to HTTP if we have an HTTP error code (#18141) commit e369d71 Author: Aleksey Khoroshilov <[email protected]> Date: Fri Apr 28 03:34:51 2023 +0700 Disable flaky upstream tests. (Uplift to 1.51.x) (#18274) Update nft-details.tsx

Hide network icon commit c98ff5e Author: brave-builds <[email protected]> Date: Fri Apr 28 08:29:10 2023 +0000 1.51.107 commit ce813ef Author: Max <[email protected]> Date: Fri Apr 28 04:25:41 2023 -0400 Upgrade from Chromium 113.0.5672.53 to Chromium 113.0.5672.63 (1.51.x). (#18269) * Upgrade from Chromium 113.0.5672.53 to Chromium 113.0.5672.63 * Upgrade patches from Chromium 113.0.5672.53 to Chromium 113.0.5672.63 * Update pins list timestamp * Temporarily disables WindowClosingConfirmBrowserTest.TestWithDownload on MacOS. The test fails because in Chromium Private/Incognito profiles now ignore the "Ask for download location" setting and always prompt for the location. We will fix that in brave/brave-browser#29823, but until then let's disable this test on MacOS. --------- Co-authored-by: brave-builds <[email protected]> commit b18aeae Author: brave-builds <[email protected]> Date: Fri Apr 28 10:15:10 2023 +0200 Disable site affiliation fetcher (uplift to 1.51.x) (#18286) Uplift of #18153 (squashed) to release commit 6b1db58 Author: brave-builds <[email protected]> Date: Fri Apr 28 09:47:20 2023 +0200 Revise the behavior of "Extensions (Brave Wallet fallback)" setting (uplift to 1.51.x) (#18228) Uplift of #18172 (squashed) to beta commit f7f8042 Author: brave-builds <[email protected]> Date: Fri Apr 28 09:42:41 2023 +0200 Send kUAModel and kUAPlatformVersion CHs when requested. (uplift to 1.51.x) (#18263) Uplift of #18154 (squashed) to beta commit 545fa57 Author: Max <[email protected]> Date: Fri Apr 28 03:07:51 2023 -0400 Don't call GetSessionRoute() when media router is disabled (#18245) (1.51.x). (#18264) Don't call GetSessionRoute() when media router is disabled (#18245) Otherwise, it would end in reaching NOTREACHED() Co-authored-by: Sangwoo Ko <[email protected]> commit 0fbe592 Author: Arthur Edelstein <[email protected]> Date: Thu Apr 27 17:44:23 2023 -0700 Uplift HTTPS First Mode v2 Migration (#17856) and HTTP error code fallback (#18141) (#18179) * Migrate to HttpsFirstMode v2 (#17856) Migrate HTTPS by Default feature to use HttpsFirstMode v2 * Force HTTPS Upgrader to fall back to HTTP if we have an HTTP error code (#18141) commit e369d71 Author: Aleksey Khoroshilov <[email protected]> Date: Fri Apr 28 03:34:51 2023 +0700 Disable flaky upstream tests. (Uplift to 1.51.x) (#18274) Update nft-details.tsx

bridiver · 2024-05-08T20:43:24Z

chromium_src/chrome/browser/ssl/https_upgrades_navigation_throttle.cc

+    content::NavigationHandle* handle,
+    std::unique_ptr<SecurityBlockingPageFactory> blocking_page_factory,
+    PrefService* prefs) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);


what are we doing here? Copying code from upstream is the worst of all possible options and we should basically never be doing this.

Ticket for this is here: brave/brave-browser#38177

arthuredelstein requested a review from a team as a code owner March 31, 2023 18:46

github-actions bot added the CI/run-network-audit Run network-audit label Mar 31, 2023

github-actions bot assigned arthuredelstein Mar 31, 2023

goodov self-requested a review April 10, 2023 14:52

goodov reviewed Apr 11, 2023

View reviewed changes

chromium_src/chrome/common/chrome_features.cc Show resolved Hide resolved

chromium_src/chrome/browser/ssl/https_upgrades_navigation_throttle.cc Outdated Show resolved Hide resolved

chromium_src/chrome/browser/ssl/https_upgrades_interceptor.cc Show resolved Hide resolved

goodov reviewed Apr 17, 2023

View reviewed changes

test/filters/browser_tests-macos.filter Outdated Show resolved Hide resolved

goodov approved these changes Apr 17, 2023

View reviewed changes

chromium_src/chrome/browser/ssl/https_upgrades_interceptor.cc Outdated Show resolved Hide resolved

arthuredelstein force-pushed the issues/28935 branch from e5886e2 to 23562d2 Compare April 17, 2023 18:59

Migrate HTTPS by Default feature to use HttpsFirstMode v2

1b31386

arthuredelstein force-pushed the issues/28935 branch from 23562d2 to 1b31386 Compare April 17, 2023 19:56

arthuredelstein merged commit 854b779 into master Apr 17, 2023

arthuredelstein deleted the issues/28935 branch April 17, 2023 22:21

github-actions bot added this to the 1.52.x - Nightly milestone Apr 17, 2023

This was referenced Apr 21, 2023

HTTPS Upgrades should prevent referrer stripping on same site brave/brave-browser#28809

Closed

Uplift #17856 and #18141 fixes to our HTTPS Upgrades feature in 1.51.x brave/brave-browser#29914

Closed

arthuredelstein added a commit that referenced this pull request Apr 21, 2023

Migrate to HttpsFirstMode v2 (#17856)

c6425b6

Migrate HTTPS by Default feature to use HttpsFirstMode v2

kjozwiak mentioned this pull request Apr 25, 2023

Uplift HTTPS First Mode v2 Migration (#17856) and HTTP error code fallback (#18141) #18179

Merged

25 tasks

kjozwiak mentioned this pull request Apr 27, 2023

Update HTTPS by Default to work with HttpsFirstModeV2 brave/brave-browser#28935

Closed

arthuredelstein added a commit that referenced this pull request Apr 27, 2023

Migrate to HttpsFirstMode v2 (#17856)

da8f56c

Migrate HTTPS by Default feature to use HttpsFirstMode v2

bridiver reviewed May 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to HttpsFirstMode v2 #17856

Migrate to HttpsFirstMode v2 #17856

arthuredelstein commented Mar 31, 2023 •

edited by kjozwiak

Loading

arthuredelstein commented Apr 17, 2023

kjozwiak commented Apr 27, 2023 •

edited

Loading

bridiver May 8, 2024

arthuredelstein Aug 27, 2024

Migrate to HttpsFirstMode v2 #17856

Migrate to HttpsFirstMode v2 #17856

Conversation

arthuredelstein commented Mar 31, 2023 • edited by kjozwiak Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

arthuredelstein commented Apr 17, 2023

kjozwiak commented Apr 27, 2023 • edited Loading

Test Case #1 - brave/brave-browser#28809

Test Case #2 - brave/brave-browser#28935

Shields Panel (Upgrade connections to HTTPS)

Shields Panel (Only connect with HTTPS)

Shields Panel (Don't upgrade HTTP connections)

Tor Windows

Shields Panel (Upgrade to HTTPS whenever possible (default))

Shields Panel (Require all connections to use HTTPS (strict))

Shields Panel (Don't upgrade connections to HTTPS (disabled))

Prevent permissive HTTPS Upgrade settings from leaking from Normal to Private windows

Test Case #1 - Upgrade to HTTPS whenever possible (default)

Test Case #2 - Require all connections to use HTTPS (strict)

Test Case #3 - Don't upgrade connections to HTTPS (disabled)

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

bridiver May 8, 2024

Choose a reason for hiding this comment

arthuredelstein Aug 27, 2024

Choose a reason for hiding this comment

arthuredelstein commented Mar 31, 2023 •

edited by kjozwiak

Loading

kjozwiak commented Apr 27, 2023 •

edited

Loading

`Test Case #1` - brave/brave-browser#28809

`Test Case #2` - brave/brave-browser#28935

Shields Panel (`Upgrade connections to HTTPS`)

Shields Panel (`Only connect with HTTPS`)

Shields Panel (`Don't upgrade HTTP connections`)

`Tor Windows`

Shields Panel (`Upgrade to HTTPS whenever possible (default)`)

Shields Panel (`Require all connections to use HTTPS (strict)`)

Shields Panel (`Don't upgrade connections to HTTPS (disabled)`)

Prevent permissive HTTPS Upgrade settings from leaking from `Normal` to `Private` windows

`Test Case #1` - `Upgrade to HTTPS whenever possible (default)`

`Test Case #2` - `Require all connections to use HTTPS (strict)`

`Test Case #3` - `Don't upgrade connections to HTTPS (disabled)`

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