Closed Bug 841221 Opened 9 years ago Closed 8 years ago

Use HTTPS instead of HTTP for manual update link to firefox.com in Help|About dialog

Categories

(Firefox Graveyard :: Help Documentation, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 24

People

(Reporter: briansmith, Assigned: raymondlee)

References

Details

Attachments

(1 file, 4 obsolete files)

+++ This bug was initially created as a clone of Bug #840687 +++
+++ This bug was initially created as a clone of Bug #771788 +++

app.update.url.manual;http://www.firefox.com

If somebody were trying to trick a user into installing a malicious update then I guess one way to do it would be to block automatic updates and then intercept this request if/when the user clicks the link in Help|About. So, I see some real security value in making this an HTTPS link, in the unlikely case the user can figure out how to do that.

firefox.com doesn't currently have a valid certificate; it uses the mozilla.org certificate.
Firefox.com is not a website. It is 301 redirect to www.mozilla.org/firefox. We only use Firefox.com as vanity URL to get people to the mozilla.org Firefox download pages. www.mozilla.org/firefox redirects to either /firefox/new/ or /firefox/fx/ depending on what browser you are running.
Sounds like we should change the link to point to http://www.mozilla.org/firefox then.
Though I guess the idea was that this URL was displayed visible in a prompt to the user, so a nicer/simpler URL was a better choice. https://www.firefox.com does not work.
There is a specific Firefox update URL that has version sniffing on it at:

https://www.mozilla.org/firefox/update/

but https://www.mozilla.org/firefox/ would be fine too. Just make sure there is a trailing slash on it avoid another redirect.
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #5)
> OK, let's change to https://www.mozilla.org/firefox/

Can you explain how people would be directed to this URL? I want to understand what the users see and when it happens because I am trying to explain a traffic pattern I am seeing on mozilla.org.
app.update.url.manual is the URL linked to in the About dialog when either the update download fails (for whatever reason), or when updates are disabled. The relevant context is:

"Updates available at _http://www.firefox.com_" (update system is disabled)
"Update failed. _Download the latest version_" (update download failed)
Attached patch v1 (obsolete) — Splinter Review
Redirect map:
https://www.mozilla.org/firefox/ -> https://www.mozilla.org/en-US/firefox/fx/
Assignee: nobody → raymond
Status: NEW → ASSIGNED
Attachment #720581 - Flags: review?(gavin.sharp)
(In reply to Raymond Lee [:raymondlee] from comment #8)
> v1

Can we get HTTPS for app.releaseNotesURL, app.vendorURL, and app.update.url.details too?
Attached patch v2 (obsolete) — Splinter Review
(In reply to Eldmannen from comment #9)
> (In reply to Raymond Lee [:raymondlee] from comment #8)
> > v1
> 
> Can we get HTTPS for app.releaseNotesURL, app.vendorURL, and
> app.update.url.details too?

Done.

I have also removed /projects/ part for app.releaseNotesURL because the url doesn't exists.
Attachment #720581 - Attachment is obsolete: true
Attachment #720581 - Flags: review?(gavin.sharp)
Attachment #720705 - Flags: review?(gavin.sharp)
(In reply to Raymond Lee [:raymondlee] from comment #10)
> Created attachment 720705 [details] [diff] [review]
> v2
> 
> (In reply to Eldmannen from comment #9)
> > (In reply to Raymond Lee [:raymondlee] from comment #8)
> > > v1
> > 
> > Can we get HTTPS for app.releaseNotesURL, app.vendorURL, and
> > app.update.url.details too?
> 
> Done.
> 
> I have also removed /projects/ part for app.releaseNotesURL because the url
> doesn't exists.

I am still seeing a lot of https://www.mozilla.org/projects/%APP%* in that file.
Attached patch v3 (obsolete) — Splinter Review
(In reply to Chris More [:cmore] from comment #11)
> I am still seeing a lot of https://www.mozilla.org/projects/%APP%* in that
> file.

I have removed them now.  

Do we still want https://www.mozilla.org/products/%APP%/ or should change it to https://www.mozilla.org/%APP%/ because both are redirected to https://www.mozilla.org/en-US/firefox/fx/?
Attachment #721048 - Flags: review?(gavin.sharp)
Attachment #720705 - Attachment is obsolete: true
Attachment #720705 - Flags: review?(gavin.sharp)
Comment on attachment 721048 [details] [diff] [review]
v3

>diff --git a/browser/branding/aurora/pref/firefox-branding.js b/browser/branding/aurora/pref/firefox-branding.js

>+pref("app.update.url.manual", "https://www.mozilla.org/firefox/channel/");
>+pref("app.update.url.details", "https://www.mozilla.org/%APP%/");

I think these should probably just both point to https://www.mozilla.org/firefox/aurora/

>-pref("app.releaseNotesURL", "http://www.mozilla.org/projects/%APP%/%VERSION%/releasenotes/");
>-pref("app.vendorURL", "http://www.mozilla.org/projects/%APP%/");

These prefs are actually only used in about:, which is seldom-seen. I think we should just remove them (or make them optional). Can you remove these changes from the patch and file a new bug on doing that?

>diff --git a/browser/branding/nightly/pref/firefox-branding.js b/browser/branding/nightly/pref/firefox-branding.js

>+pref("app.update.url.manual", "https://nightly.mozilla.org/");
>+pref("app.update.url.details", "https://www.mozilla.org/%APP%/");

Similarly, let's make these both point to https://nightly.mozilla.org.

>diff --git a/browser/branding/official/pref/firefox-branding.js b/browser/branding/official/pref/firefox-branding.js

>+pref("app.update.url.details", "https://www.mozilla.org/%LOCALE%/%APP%/releases/");

I think we should use https://www.mozilla.org/en-US/firefox/%VERSION%/releasenotes/ for this.

>diff --git a/browser/branding/unofficial/pref/firefox-branding.js b/browser/branding/unofficial/pref/firefox-branding.js

>+pref("app.update.url.manual", "https://www.mozilla.org/products/%APP%/");
>+pref("app.update.url.details", "https://www.mozilla.org/%APP%/");

The goal for this package is to eventually use it as a replacement for "nightly". So for now, just use the same values as in the nightly file.

Overall, I think we should eliminate the use of %APP% in our URLs, since there's no valid use case where someone would want to change the value of %APP% but keep our mozilla.org URLs. So let's replace it with a hardcoded "firefox" whenever you're modifying URLs that include it.

I'll quickly review a patch implementing these changes.
Attachment #721048 - Flags: review?(gavin.sharp) → review-
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #13)
> These prefs are actually only used in about:, which is seldom-seen. I think
> we should just remove them (or make them optional). Can you remove these
> changes from the patch and file a new bug on doing that?

I filed the bug: bug 876037.
Attached patch v4 (obsolete) — Splinter Review
Attachment #721048 - Attachment is obsolete: true
Attachment #754378 - Flags: review?(gavin.sharp)
Comment on attachment 754378 [details] [diff] [review]
v4

>diff --git a/browser/branding/official/pref/firefox-branding.js b/browser/branding/official/pref/firefox-branding.js

>-pref("app.update.url.details", "http://www.mozilla.com/%LOCALE%/firefox/releases/");
>+pref("app.update.url.details", "https://www.mozilla.org/en-US/firefox/%VERSION%/releasenotes/");

Sorry, I pasted the wrong link - there shouldn't be a hard-coded "en-US/" here.

r=me with that removed.
Attachment #754378 - Flags: review?(gavin.sharp) → review+
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #16)
> >+pref("app.update.url.details", "https://www.mozilla.org/en-US/firefox/%VERSION%/releasenotes/");
> 
> Sorry, I pasted the wrong link - there shouldn't be a hard-coded "en-US/"
> here.

Hmm, on second thought, I think I made the wrong decision here. This is supposed to link to details about the available update, so linking to %VERSION%'s release notes won't give the right content. Let's hold off for a second while I try to find out if there's a better link for this.
Comment on attachment 754378 [details] [diff] [review]
v4

Also perhaps worth changing: using /%LOCALE%/ instead of the non-locale-specific links. We should probably be consistent with bug 840692 here.
Attachment #754378 - Flags: review+
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #17)
> > >+pref("app.update.url.details", "https://www.mozilla.org/en-US/firefox/%VERSION%/releasenotes/");

OK, let's use:

https://www.mozilla.org/%LOCALE%/firefox/notes

for this pref in the "official" package.
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #19)
> (In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment
> #17)
> > > >+pref("app.update.url.details", "https://www.mozilla.org/en-US/firefox/%VERSION%/releasenotes/");
> 
> OK, let's use:
> 
> https://www.mozilla.org/%LOCALE%/firefox/notes
> 
> for this pref in the "official" package.

Updated
Attachment #754378 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/36c05db04cc4
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
Verified fixed on Windows 7 64bit, Ubuntu 13.10 and Mac OSX 10.8.5 using latest Nightly 34.0a1.
Status: RESOLVED → VERIFIED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.