Opened 15 months ago

Closed 8 months ago

Last modified 7 months ago

#23439 closed defect (fixed)

Exempt .onion domains from mixed content warnings

Reported by: gk Owned by: gk
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: TorBrowserTeam201803R, GeorgKoppen201803, tbb-backported
Cc: tbb-team, arthuredelstein Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Part one of getting .onions exempted for the HTTPS requirement for secure contexts was done in #21321. Now we want to extend that to mixed content settings as well. See: https://bugzilla.mozilla.org/show_bug.cgi?id=1382359 for the Mozilla bug.

Child Tickets

Change History (24)

comment:1 Changed 15 months ago by gk

Owner: set to gk
Status: newassigned

comment:2 Changed 15 months ago by gk

Component: - Select a componentApplications/Tor Browser

comment:3 Changed 14 months ago by gk

Keywords: TorBrowserTeam201710 added; TorBrowserTeam201709 removed

Items for October 2017

comment:4 Changed 14 months ago by gk

Keywords: GeorgKoppen201710 added; GeorgKoppen201709 removed

comment:5 Changed 13 months ago by gk

Keywords: GeorgKoppen201711 added; GeorgKoppen201710 removed

Moving my tickets to November.

comment:6 Changed 13 months ago by gk

Keywords: TorBrowserTeam201711 added; TorBrowserTeam201710 removed

Moving tickets over to November.

comment:7 Changed 12 months ago by gk

Moving tickets to December 2017

comment:8 Changed 12 months ago by gk

Keywords: TorBrowserTeam201712 added; TorBrowserTeam201711 removed

Moving tickets to December 2017, for realz.

comment:9 Changed 12 months ago by gk

Keywords: GeorgKoppen201712 added; GeorgKoppen201711 removed

Moving my tickets to December.

comment:10 Changed 11 months ago by gk

#24819 is a duplicate.

comment:11 Changed 11 months ago by gk

Keywords: GeorgKoppen201801 added; GeorgKoppen201712 removed

Moving my tickets to 2018

comment:12 Changed 11 months ago by gk

Keywords: TorBrowserTeam201801 added; TorBrowserTeam201712 removed

Moving tickets to 2018.

comment:13 Changed 10 months ago by gk

Keywords: GeorgKoppen201802 added; GeorgKoppen201801 removed

Moving my tickets to Feb.

comment:14 Changed 10 months ago by gk

Keywords: TorBrowserTeam201802 added; TorBrowserTeam201801 removed

Moving tickets to Feb

comment:15 Changed 9 months ago by gk

Cc: arthuredelstein added
Keywords: TorBrowserTeam201802R added; TorBrowserTeam201802 removed
Status: assignedneeds_review

bug_23439_v2 (https://gitweb.torproject.org/user/gk/tor-browser.git/log/?h=bug_23439_v2) is up for review. It contains the code (f10bad83876aee09c716086db659ab2bbe9652af) and a test for it (a7d9d4cf255368eef139b1dbde7d16933bfcaa30). We might need to add other tests for the uplift, I'll ask the Mozilla folks about it. For us I think it's enough to have this one test for mixed content blocking exemption.

comment:16 Changed 9 months ago by gk

Keywords: TorBrowserTeam201803R added; TorBrowserTeam201802R removed

Moving our reviews to March 2018

comment:17 Changed 9 months ago by gk

Keywords: GeorgKoppen201803 added; GeorgKoppen201802 removed

Moving my tickets to March.

comment:18 Changed 9 months ago by arthuredelstein

As a version of these patches have landed in Mozilla, shall we backport them to include in the next Tor Browser alpha?

comment:19 in reply to:  18 Changed 9 months ago by gk

Replying to arthuredelstein:

As a version of these patches have landed in Mozilla, shall we backport them to include in the next Tor Browser alpha?

All that is needed is to review the patch in this bug (and adapt it if needed).

comment:20 Changed 9 months ago by arthuredelstein

I looked over both patches in https://gitweb.torproject.org/user/gk/tor-browser.git/log/?h=bug_23439_v2 and they look good to me. gk mentioned it might be good to add the pref caching as Christoph suggested in https://bugzilla.mozilla.org/show_bug.cgi?id=1382359

comment:21 in reply to:  18 ; Changed 8 months ago by mcs

Replying to arthuredelstein:

As a version of these patches have landed in Mozilla, shall we backport them to include in the next Tor Browser alpha?

I thought of doing this a couple of weeks ago, but I got stuck because the automated test fails for a strange reason, at least in my non-rbm macOS build. What happens is that an SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED error occurs while trying to load https://example.com/browser/browser/base/content/test/siteIdentity/test_no_mcb_for_onions.html

Manually testing does show that the patch is correct. The main advantage over the patches from comment:15 is that pref caching is included. You can find the two commits here:
https://gitweb.torproject.org/user/brade/tor-browser.git/log/?h=bug23439-01

comment:22 in reply to:  21 Changed 8 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Replying to mcs:

Replying to arthuredelstein:

As a version of these patches have landed in Mozilla, shall we backport them to include in the next Tor Browser alpha?

I thought of doing this a couple of weeks ago, but I got stuck because the automated test fails for a strange reason, at least in my non-rbm macOS build. What happens is that an SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED error occurs while trying to load https://example.com/browser/browser/base/content/test/siteIdentity/test_no_mcb_for_onions.html

Manually testing does show that the patch is correct. The main advantage over the patches from comment:15 is that pref caching is included. You can find the two commits here:
https://gitweb.torproject.org/user/brade/tor-browser.git/log/?h=bug23439-01

Hm. So, I think the test in your branch is at the wrong place. In ESR 52 the MCB tests are under browser/base/content/test/general. They got later moved to /browser/base/content/test/siteIdentity. We should keep that place while we are on ESR 52 I think. I actually wonder how running that test got that far for you given that assertMixedContentBlockingState() is defined in browser/base/content/test/general/head.js. I remember that one being broken made me actually realize that the location of the tests moved between ESR52 and m-c. Anyway, I've fixed that in 1316acb053d6191176e9ae4e4f502415b068525e.

The code backport looks good although I think I am not so happy about essentially reverting e3f5021a4103f1cdc4e902c6ecded73bdcf5327b without actually indicating that. But in order to avoid another roundtrip I'll take it as-is. (commit 680dece41e71d30afd4616aa19001c60e55dc852). Both commits landed on tor-browser-52.7.2esr-8.0-1.

comment:23 Changed 8 months ago by gk

Keywords: tbb-backport added

comment:24 Changed 7 months ago by gk

Keywords: tbb-backported added; tbb-backport removed

Backported to tor-browser-52.8.0esr-7.5-1 (commit c4d20a867e257b15895cc2123e2c88b80df70b41 and e52a50f2099501efd4e2892a702a1a3730439426). Should be available in 7.5.4.

Note: See TracTickets for help on using tickets.