loop client should explicitly validate that endpoints are using HTTPS

RESOLVED FIXED

Status

Firefox OS
Gaia::Loop
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: freddyb, Assigned: Carmen Jimenez Cabezas)

Tracking

({sec-high})

unspecified
x86_64
Linux
sec-high

Firefox Tracking Flags

(firefox-esr31 unaffected)

Details

Attachments

(1 attachment)

The loop client should ensure that endpoints are HTTPS (WSS) and refuse to talk in clear text.

e.g., http://loop.dev.mozaws.net and ws://loop.dev.mozaws.net/websocket should use https and wss instead
Keywords: sec-high
Group: core-security
Is this issue Gaia only or will it affect Desktop Firefox?
This was filed while looking at the Gaia source code. I did not test the Desktop code.
Loading our Domains into the HTTPS Strict Transport Security (HSTS) preload list would solve both, though.
Assignee: ferjmoreno → nobody
(Assignee)

Updated

3 years ago
Assignee: nobody → carmen.jimenezcabezas
(Assignee)

Comment 3

3 years ago
Are the domains going to be included on the HSTS preload list finally? And if so do you want me to also add explicit protocol validation to the Firefox OS loop client?
Flags: needinfo?(fbraun)
I would argue for protocol validation on the firefox OS loop client, because pinning isn't consistent across all b2g versions.

The loop endpoint is under services.mozilla.com and thus covered by bug 1030135.
(The desktop version talks to call.mozilla.com, for which I filed bug 1074091).
Flags: needinfo?(fbraun)
Summary: loop client should only allow talk to secure endpoints → loop client should explicitly validate that endpoints are using HTTPS
(Assignee)

Comment 5

3 years ago
Created attachment 8497762 [details] [review]
V1 Proposed patch
Thanks for the quick turnaround.
f+ from my side, but take a look at the comments on Github.
(Assignee)

Comment 7

3 years ago
Frederik, I tried using the URL parser instead of using regular expressions but ran into a problem. When the URL is of the type:

url = "protocol://a.host.domain"

then 

new URL(url).href 

is 

"protocol://a.host.domain/" (note the extra / at the end). And with that /, the BrowserId assertions/logging on the Loop servers fails (it gives a 302). And since the / isn't added always, I cannot just remove it blindly (I would have to check if the original URL had a / before parsing it)... at the end it's just easier and faster using the regexp :).
(Assignee)

Updated

3 years ago
Attachment #8497762 - Flags: review?(ferjmoreno)
Ah, that's unfortunate. Let's continue with the regexp then.
Comment on attachment 8497762 [details] [review]
V1 Proposed patch

Thanks Carmen! I left one comment on the PR.
Attachment #8497762 - Flags: review?(ferjmoreno)
(Assignee)

Updated

3 years ago
Attachment #8497762 - Flags: review?(ferjmoreno)
Attachment #8497762 - Flags: review?(ferjmoreno) → review+
(Assignee)

Comment 10

3 years ago
https://github.com/mozilla-b2g/firefoxos-loop-client/commit/0bfa6ca9cca27d052a7b06b805a6674ded0bd882
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Group: b2g-core-security
status-firefox-esr31: --- → unaffected

Updated

3 years ago
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.