1141052 - SelfSupportBackend.jsm should only allow https URLs in browser.selfsupport.url

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[:Gavin Sharp [email: [e-mail]]]

It's generally not safe to allow using an http:// page for self-support, so we should add code to only allow loading https URIs.

This will also require changing the dummy URLs used in Talos (similar bug 1138823) and in addon-sdk/source/python-lib/cuddlefish/prefs.py and addon-sdk/source/test/preferences/no-connections.json.

Comment 1

[Alessio Placitelli [:Dexter]]

Thanks for reporting this. It's on my radar and I'll be working on it asap :)

Comment 2

[Alessio Placitelli [:Dexter]]

Attachment: bug141052.patch

This patch makes SelfSupport backend report an error if the self support URL is not using HTTPS. It also changes tests accordingly.

Comment 3

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8575221 [details] [diff] [review]
bug141052.patch

Review of attachment 8575221 [details] [diff] [review]:
-----------------------------------------------------------------

What about convenient local testing or developing without https?
For other components we allow overrides with a hidden pref (with no default value).

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Prefs can be abuse, easy enough to modify the relevant code locally for dev testing.

Comment 5

[Alessio Placitelli [:Dexter]]

Attachment: Changes to Talos

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8579312 [details] [diff] [review]
Changes to Talos

Review of attachment 8579312 [details] [diff] [review]:
-----------------------------------------------------------------

you are aware this will resolve to a 404, but http vs https will still be valid :)

Comment 7

[Alessio Placitelli [:Dexter]]

(In reply to Joel Maher (:jmaher) from comment #6)
> Comment on attachment 8579312 [details] [diff] [review]
> Changes to Talos
> 
> Review of attachment 8579312 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> you are aware this will resolve to a 404, but http vs https will still be
> valid :)

Yes, but thanks for pointing that out ;-)

Comment 8

[Alessio Placitelli [:Dexter]]

Attachment: Changes to addon-sdk prefs

This patch changes addon-sdk prefs.

Comment 9

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8579905 [details] [diff] [review]
Changes to addon-sdk prefs

Review of attachment 8579905 [details] [diff] [review]:
-----------------------------------------------------------------

::: addon-sdk/source/python-lib/cuddlefish/prefs.py
@@ +59,5 @@
>      'browser.safebrowsing.updateURL': 'http://localhost/safebrowsing-dummy/update',
>      'browser.safebrowsing.gethashURL': 'http://localhost/safebrowsing-dummy/gethash',
>      'browser.safebrowsing.reportURL': 'http://localhost/safebrowsing-dummy/report',
>      'browser.safebrowsing.malware.reportURL': 'http://localhost/safebrowsing-dummy/malwarereport',
> +    'browser.selfsupport.url': 'https://localhost/repair-dummy',

Better naming please, selfsupport-dummy?

::: addon-sdk/source/test/preferences/no-connections.json
@@ +16,5 @@
>    "browser.safebrowsing.updateURL": "http://localhost/safebrowsing-dummy/update",
>    "browser.safebrowsing.gethashURL": "http://localhost/safebrowsing-dummy/gethash",
>    "browser.safebrowsing.reportURL": "http://localhost/safebrowsing-dummy/report",
>    "browser.safebrowsing.malware.reportURL": "http://localhost/safebrowsing-dummy/malwarereport",
> +  "browser.selfsupport.url": "https://localhost/repair-dummy",

Better naming please, selfsupport-dummy?

Comment 10

[Alessio Placitelli [:Dexter]]

Attachment: Changes to addon-sdk prefs v2

Comment 11

[Alessio Placitelli [:Dexter]]

Attachment: bug141052.patch

Fixed the failing tests on try server and updated the commit message.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=757a43a50e6d

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

https://hg.mozilla.org/build/talos/rev/2fc73b49bbc0

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

talos is updated on inbound.  Please close this bug if there is nothing left to do.

Comment 14

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: changes to talos preference and font

and with the font taken care of we should be good!

Comment 15

[Alessio Placitelli [:Dexter]]

Comment on attachment 8591717 [details] [diff] [review]
changes to talos preference and font

Review of attachment 8591717 [details] [diff] [review]:
-----------------------------------------------------------------

::: talos/mtio-whitelist.json
@@ +38,5 @@
>   "{FONTS}\\RAAVI.TTF": {},
>   "{FONTS}\\SEGOEUI.TTF": {},
>   "{FONTS}\\SEGOEUIB.TTF": {},
>   "{FONTS}\\SEGOEUII.TTF": {},
> + "{fonts}\\segoeuil.ttf": {},

The other fonts are uppercase, while this is lowercase. Is it the same? Maybe we should keep the case to be consistent with the other entries. What do you think?

Comment 16

[Joel Maher ( :jmaher ) (UTC -8)]

I had to think about that for a moment, I will fix it to be uppercase and land it!

Comment 17

[Joel Maher ( :jmaher ) (UTC -8)]

https://hg.mozilla.org/build/talos/rev/c61a0da714bc

Comment 18

[Joel Maher ( :jmaher ) (UTC -8)]

talos is live on trunk, feel free to close this out if needed.

Comment 19

[Alessio Placitelli [:Dexter]]

Thanks :jmaher! I'm marking this as checkin-needed for the other patches (bug141052.patch and Changes to addon-sdk prefs v2).

Updated try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e5cf29f62351

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/6bd496d40fd1
https://hg.mozilla.org/integration/fx-team/rev/29d6efbac93b

Comment 21

[Gregory Szorc [:gps]]

https://hg.mozilla.org/integration/fx-team/rev/4ff9b0890ea7
https://hg.mozilla.org/integration/fx-team/rev/cc0dc8a81bad

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4ff9b0890ea7
https://hg.mozilla.org/mozilla-central/rev/cc0dc8a81bad

Comment 23

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/1ef9b052b5103cee7ca9e2a8459cff5f9479724d
Bug 1141052 - Change SelfSupport test preferences to use HTTPS instead of HTTP. r=gfritzsche