Added mixed content check for *.onion origins.

[boocmp]

Resolves https://github.com/brave/brave-browser/issues/25939

Submitter Checklist:

• [x] I confirm that no security/privacy review is needed, or that I have requested one
• [x] There is a ticket for my issue
• [x] 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
• [x] Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
• [x] Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
• [x] 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:

• [ ] The associated issue milestone is set to the smallest version that the changes has landed on
• [ ] All relevant documentation has been updated, for instance:
  • [ ] https://github.com/brave/brave-browser/wiki/Deviations-from-Chromium-(features-we-disable-or-remove)
  • [ ] https://github.com/brave/brave-browser/wiki/Proxy-redirected-URLs
  • [ ] https://github.com/brave/brave-browser/wiki/Fingerprinting-Protections
  • [ ] https://github.com/brave/brave-browser/wiki/Brave%E2%80%99s-Use-of-Referral-Codes
  • [ ] https://github.com/brave/brave-browser/wiki/Custom-Headers
  • [ ] https://github.com/brave/brave-browser/wiki/Web-Compatibility-Exceptions-in-Brave
  • [ ] https://github.com/brave/brave-browser/wiki/QA-Guide
  • [ ] https://github.com/brave/brave-browser/wiki/P3A

Test Plan:

[diracdeltas]

suggested test plan for qa:

1. go to http://xao2lxsmia2edq2n5zxg6uahx6xox2t7bfjw6b5vdzsxi7ezmqob6qid.onion/ in tor window
2. open devtools, enter fetch("https://www.example.com"). In the Network tab, you can see the request to https://www.example.com is not blocked.
3. repeat step 2 but change https to http; it should be blocked
4. go to https://brave4u7jddbv7cyviptqjc7jusxh72uik7zt6adtckl5f4nwy2v72qd.onion/index.html. repeat steps 2-3
5. visit https://mixed.badssl.com/ and verify that it shows warning messages in the console about an insecure resource being requested (but was upgraded)
6. go to https://mixed-favicon.badssl.com/ and verify it shows error message in the console about favicon being blocked
7. verify https://mixed-form.badssl.com/ also shows a console warning
8. verify https://mixed-script.badssl.com/ shows up with a grey background and has an error message in the console
9. verify https://very.badssl.com/ shows both warnings and errors in the console

[kdenhartog]

Looks like this one would close https://github.com/brave/brave-browser/issues/25939 and https://github.com/brave/brave-browser/issues/1135 from what I'm seeing

[diracdeltas]

i don't think it would close https://github.com/brave/brave-browser/issues/1135 since there's more to secure origins than just mixed content (geolocation allowed, etc.)

[kdenhartog]

Thank you for the correction, I wasn't aware of that aspect of secure origins so seems like I've got some new "known unknowns" to read up on.

[fmarier]

I created two test pages to confirm that the Tor Browser behaves as we expected (it does):

• https://fmarier.org/brave-browser-25939/mixed.html

• http://ixrdj3iwwhkuau5tby5jh3a536a2rdhpbdbu6ldhng43r47kim7a3lid.onion/brave-browser-25939/mixed.html

To run this test, I had to customize the following in Tor Browser:

• dom.security.https_only_mode=false
• security.mixed_content.block_display_content=true

[diracdeltas]

@fmarier awesome! did you test the mixed content autoupgrade case? (i.e., secure page embeds an HTTP image that supports HTTPS. in chrome it should autoupgrade the request to HTTPS. you can try with this URL: http://mixed.badssl.com/image.jpg.)

i'm guessing tor browser doesn't do the autoupgrade, but might be nice to have in brave.

[arthuredelstein]

I created two test pages to confirm that the Tor Browser behaves as we expected (it does):

@fmarier's test pages will also be perfect for qa for this PR. I'd suggest adding mixed-content favicons and iframes also to make sure they are handled correctly. (Happy to work on that if it's useful.)

[fmarier]

@arthuredelstein If you've got more ideas of test cases to add, I've copied the file here: https://gist.github.com/fmarier/2b29761888f6582b9ecc026d6cb2a1f2

[fmarier]

i'm guessing tor browser doesn't do the autoupgrade, but might be nice to have in brave.

The Tor Browser is actually running with HTTPS-only mode now (like Firefox private browsing I believe). I had to explicitly disable that feature in order to see the behavior of the mixed content blocker :)

[arthuredelstein]

i'm guessing tor browser doesn't do the autoupgrade, but might be nice to have in brave.

The Tor Browser is actually running with HTTPS-only mode now (like Firefox private browsing I believe). I had to explicitly disable that feature in order to see the behavior of the mixed content blocker :)

HTTPS-Only Mode (HOM) in Tor Windows only enforces HTTPS on top-level (non-onion) domains, so it's a good idea to make sure that, when HOM is enabled, the mixed-content blocker blocks the loading of insecure (HTTP, non-onion images) on http://[address].onion pages.

(FWIW, Firefox private browsing actually runs HTTPS-by-Default, which is somewhat weaker than HTTPS-Only Mode.)

[diracdeltas]

this is pretty much good to go for me except i think this test page should not have the <meta> policy: https://github.com/brave/brave-core/pull/15436/files#r994963774

any other concerns @fmarier @arthuredelstein ?

[arthuredelstein]

Looks good to me. Thanks @boocmp !

[boocmp]

this is pretty much good to go for me except i think this test page should not have the <meta> policy: https://github.com/brave/brave-core/pull/15436/files#r994963774

I don't know how to configure embedded http test server to support "upgrade-insecure-requests", The client request contains "upgrade-insecure-requests: 1" header, but without meta tag it doesn't work. It works only on embedded https server.

[diracdeltas]

this is pretty much good to go for me except i think this test page should not have the <meta> policy: https://github.com/brave/brave-core/pull/15436/files#r994963774

I don't know how to configure embedded http test server to support "upgrade-insecure-requests", The client request contains "upgrade-insecure-requests: 1" header, but without meta tag it doesn't work. It works only on embedded https server.

oh i see, the problem is that the test server is HTTP.

assuming there is no way to run an HTTPS test server, this is fine as-is assuming @fmarier has a test case for QA with autoupgrading mixed content.

[diracdeltas]

should be ok to merge?