Closed Bug 1109475 Opened 5 years ago Closed 5 years ago

Firefox should use HTTPS instead of HTTP for Safe Browsing URLs

Categories

(Toolkit :: Safe Browsing, defect)

x86
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla41
Tracking Status
firefox41 --- verified

People

(Reporter: cpeterson, Assigned: Dolske)

References

Details

Attachments

(1 file, 2 obsolete files)

The current Safe Browsing URL prefs don't use HTTPS:

pref("browser.safebrowsing.reportGenericURL", "http://%LOCALE%.phish-generic.mozilla.com/?hl=%LOCALE%");
pref("browser.safebrowsing.reportErrorURL", "http://%LOCALE%.phish-error.mozilla.com/?hl=%LOCALE%");
pref("browser.safebrowsing.reportPhishURL", "http://%LOCALE%.phish-report.mozilla.com/?hl=%LOCALE%");
pref("browser.safebrowsing.reportMalwareURL", "http://%LOCALE%.malware-report.mozilla.com/?hl=%LOCALE%");
pref("browser.safebrowsing.reportMalwareErrorURL", "http://%LOCALE%.malware-error.mozilla.com/?hl=%LOCALE%");

https://mxr.mozilla.org/mozilla-central/source/browser/app/profile/firefox.js#1011

https://mxr.mozilla.org/mozilla-central/source/mobile/android/app/mobile.js#596
Blocks: 771788
Looks like we redirect to Google, and that's already supporting SSL, so this should just be a simple fix.

Although it also looks like a bunch of these prefs are not actually used anywhere (even indirectly, via getReportURL), so I'm got to take a stab at cleaning that up too.
Attached patch Patch v.1 (obsolete) — Splinter Review
Haven't actually tested this yet, but should work! *crosses fingers*

Also, I kinda wanted to give browser.safebrowsing.malware.reportURL a similar cleanup, but that's a yak too far. It's used slightly differently in the code, and I'm already straying from the core purpose of this bug.
Assignee: nobody → dolske
Attachment #8571705 - Flags: review?(gpascutto)
Attachment #8571705 - Flags: review?(gpascutto) → review+
Need bug 1138797 fixed before this can land, NI myself so it stays on my radar.
Flags: needinfo?(dolske)
Attached patch Patch v.1 (updated for bitrot) (obsolete) — Splinter Review
Updated to apply cleanly to current mozilla-central.
Attachment #8571705 - Attachment is obsolete: true
Attached patch Patch v.2Splinter Review
Oops. When I was finalizing the original patch I renamed the prefs, but didn't change the code that used those prefs. >_<

Verified that this all works now, and the server changes in bug 1138797 are live.

I created a small testplan in that bug (attachment 8622778 [details]), that might be useful for future checking.
Attachment #8622775 - Attachment is obsolete: true
Flags: needinfo?(dolske)
https://hg.mozilla.org/mozilla-central/rev/1a21a5d0e9da
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/96ae8d914fab9baad903cac07bf9f37da98fc0bc
Bug 1109475 - Firefox should use HTTPS instead of HTTP for Safe Browsing URLs. r=gcp
Verified fixed on latest Aurora 41.0a2 (buildID: 20150730004009).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.