If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Set MobileID service https dev URL and force https

RESOLVED FIXED in Firefox 32, Firefox OS v2.0

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: On parental leave, Assigned: On parental leave)

Tracking

(Blocks: 1 bug, {sec-high})

unspecified
mozilla33
sec-high
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(feature-b2g:2.0, firefox31 unaffected, firefox32 fixed, firefox33 fixed, firefox-esr24 unaffected, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

(Whiteboard: ft:loop)

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
Bug 988469 will land allowing http, we need to change that as soon as the server is https. Maybe we can keep it like that in eng builds.
(Assignee)

Updated

3 years ago
Assignee: nobody → ferjmoreno
What's blocking the server to move to HTTPS?
I want to make sure this goes in before we release.
(Assignee)

Comment 2

3 years ago
I believe bug 1020899 is the one blocking here
Depends on: 1020899
Whiteboard: ft:loop
Nominating to 2.0, as the server is expected to used https when loop is released (2.0 FxOS version)
blocking-b2g: --- → 2.0?
blocking on 2.0.
blocking-b2g: 2.0? → 2.0+
Clearing blocking flag - this is technically feature work, not a bug. We need to set the feature-b2g flag here.
blocking-b2g: 2.0+ → ---
Flags: needinfo?(oteo)
(In reply to Jason Smith [:jsmith] from comment #5)
> Clearing blocking flag - this is technically feature work, not a bug. We
> need to set the feature-b2g flag here.

Jason, one question, if I set feature-b2g:2.0, when Fernando has the patch, does he ask for approval? I want to understand if there is any problem or deadline with these feature-b2g:2.0 as FL finished 2 weeks ago
Thanks
feature-b2g: --- → 2.0
Flags: needinfo?(oteo) → needinfo?(jsmith)
(In reply to Maria Angeles Oteo (:oteo) from comment #6)
> (In reply to Jason Smith [:jsmith] from comment #5)
> > Clearing blocking flag - this is technically feature work, not a bug. We
> > need to set the feature-b2g flag here.
> 
> Jason, one question, if I set feature-b2g:2.0, when Fernando has the patch,
> does he ask for approval? I want to understand if there is any problem or
> deadline with these feature-b2g:2.0 as FL finished 2 weeks ago
> Thanks

Yeah, he'll need to ask for approval.
Flags: needinfo?(jsmith)
(In reply to Jason Smith [:jsmith] from comment #5)
> Clearing blocking flag - this is technically feature work, not a bug. We
> need to set the feature-b2g flag here.

Can we please make this block again?
The implication of this not landing with the webapi are truly dire:
All the security properties rely on a secure connection channel. We use it to send phone numbre and carrier information. (I am not a lawyer, but in some legislation this is PII)
Flags: needinfo?(jsmith)
Jason - can you explain the difference between feature work and regular blocking ? I have not seen this before. Sending sensitive data unencrypted is likely to be a high security if that puts this in context.
blocking-b2g: --- → 2.0?
Keywords: sec-high
(In reply to Paul Theriault [:pauljt] from comment #9)
> Jason - can you explain the difference between feature work and regular
> blocking ? I have not seen this before. Sending sensitive data unencrypted
> is likely to be a high security if that puts this in context.

Both flags indicate that this is required for 2.0. However, a blocking b2g flag allows the bug to automatically land on 2.0. A feature b2g flag requires approval for 2.0. Think of it as a gatekeeping approach to ensure that we don't crash land on 2.0.

Clearing the blocking flag since there's already a feature flag here.
blocking-b2g: 2.0? → ---
Flags: needinfo?(jsmith)

Comment 11

3 years ago
reset to blocking, per comment 8. 
Fernando- when do you expect this bug to land?
blocking-b2g: --- → 2.0?
Flags: needinfo?(ferplus)
(In reply to sescalante from comment #11)
> reset to blocking, per comment 8. 
> Fernando- when do you expect this bug to land?

Please see the above comment that has clarified that this is feature work, not a bug. The feature-b2g flag is already set correctly here.
blocking-b2g: 2.0? → ---
(Assignee)

Comment 13

3 years ago
(In reply to sescalante from comment #11)
> reset to blocking, per comment 8. 
> Fernando- when do you expect this bug to land?

As soon as the server is https, which I believe it will happen in bug 1020899
Flags: needinfo?(ferplus)
Well the stage environment deployment defined in Bug 1020899 is not related to what we did for the dev environment.

We did deploy the dev environment behind SSL here: https://msisdn-dev.stage.mozaws.net

You can start pointing to https://msisdn-dev.stage.mozaws.net as soon as you are ready, ask me to switch the configuration from noSSL to useSSL to enable hawk.
(Assignee)

Comment 15

3 years ago
We also need to set the final production endpoint, so we can do it all in one patch as soon as the urls mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1020899#c29 are working. This needs to land in 2.0 as we need to use the production server, so blocking 2.0?
Summary: Set "services.mobileid.forcehttps" to true once the server is https → Set MobileID service production URL and force https
Even if our production server is not deployed, We can have the client side point to https://msisdn.services.mozilla.com and route the calls to the current stage or dev servers for testings.

We can change the CNAME back to the real prod as soon as it is fully tested and ready

I will add a bug for this domain creation and routing
(Assignee)

Updated

3 years ago
blocking-b2g: --- → 2.0?
feature-b2g: 2.0 → ---
(In reply to Fernando Jiménez Moreno [:ferjm] from comment #15)
> We also need to set the final production endpoint, so we can do it all in
> one patch as soon as the urls mentioned in
> https://bugzilla.mozilla.org/show_bug.cgi?id=1020899#c29 are working. This
> needs to land in 2.0 as we need to use the production server, so blocking
> 2.0?

This is not a bug - it's required feature work to ship the feature in 2.0. The flags should not have been flipped here. I'm going to ask Bhavana to fix the feature-b2g flag here, since I don't the ability to set it.
blocking-b2g: 2.0? → ---
Flags: needinfo?(bbajaj)
(Assignee)

Comment 18

3 years ago
(In reply to Jason Smith [:jsmith] from comment #17)
> (In reply to Fernando Jiménez Moreno [:ferjm] from comment #15)
> > We also need to set the final production endpoint, so we can do it all in
> > one patch as soon as the urls mentioned in
> > https://bugzilla.mozilla.org/show_bug.cgi?id=1020899#c29 are working. This
> > needs to land in 2.0 as we need to use the production server, so blocking
> > 2.0?
> 
> This is not a bug - it's required feature work to ship the feature in 2.0.
> The flags should not have been flipped here. I'm going to ask Bhavana to fix
> the feature-b2g flag here, since I don't the ability to set it.

Sorry Jason, but IMHO this just makes us lose time by adding the extra step of requesting and waiting for the approval when we already know that this needs to land in 2.0 one way or the other. But whatever...

(In reply to Tarek Ziadé (:tarek) from comment #16)
> Even if our production server is not deployed, We can have the client side
> point to https://msisdn.services.mozilla.com and route the calls to the
> current stage or dev servers for testings.
> 
> We can change the CNAME back to the real prod as soon as it is fully tested
> and ready
> 
> I will add a bug for this domain creation and routing

Thanks Tarek. After talking with Rémy it seems that it is preferable to do this in two steps. First move to https with the dev server and after that add the final production url when it is ready. I'll file the bug for this last part and nominate it for 2.0 (feature 2.0).
Summary: Set MobileID service production URL and force https → Set MobileID service https dev URL and force https
(In reply to Fernando Jiménez Moreno [:ferjm] from comment #18)
> (In reply to Jason Smith [:jsmith] from comment #17)
> > (In reply to Fernando Jiménez Moreno [:ferjm] from comment #15)
> > > We also need to set the final production endpoint, so we can do it all in
> > > one patch as soon as the urls mentioned in
> > > https://bugzilla.mozilla.org/show_bug.cgi?id=1020899#c29 are working. This
> > > needs to land in 2.0 as we need to use the production server, so blocking
> > > 2.0?
> > 
> > This is not a bug - it's required feature work to ship the feature in 2.0.
> > The flags should not have been flipped here. I'm going to ask Bhavana to fix
> > the feature-b2g flag here, since I don't the ability to set it.
> 
> Sorry Jason, but IMHO this just makes us lose time by adding the extra step
> of requesting and waiting for the approval when we already know that this
> needs to land in 2.0 one way or the other. But whatever...

The whole point of the feature-b2g vs. blocking-b2g usage post FL is to actually ensure that we're placing approvals on late breaking feature requests so that we don't crash land on 2.0 while stability testing is ongoing. That's correct that it will land for 2.0 no matter what, it's just it needs to land cleanly on 2.1 first before getting uplifted. That's why a feature-b2g flag applies here, not a blocking flag.
feature-b2g: --- → 2.0
Flags: needinfo?(bbajaj)
(Assignee)

Comment 20

3 years ago
Created attachment 8449483 [details] [diff] [review]
v1
Attachment #8449483 - Flags: review?(jed+bmo)
(In reply to Fernando Jiménez Moreno [:ferjm] from comment #18)
> Thanks Tarek. After talking with Rémy it seems that it is preferable to do
> this in two steps. First move to https with the dev server and after that
> add the final production url when it is ready. I'll file the bug for this
> last part and nominate it for 2.0 (feature 2.0).

That's fine as long as we are all in sync with the client pref final freeze.
Comment on attachment 8449483 [details] [diff] [review]
v1

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

Looks good to me, Fernando.  I only have a suggestion about the default forceHttps behavior, but if anyone else agrees with me, that can be the topic of a follow-up.

::: b2g/app/b2g.js
@@ +991,5 @@
>  pref("identity.fxaccounts.enabled", true);
>  #endif
>  
> +// Mobile Identity API prefs.
> +pref("services.mobileid.server.uri", "https://msisdn-dev.stage.mozaws.net");

Per Comment 18, I see that you'll be able to make a final url change for production for 2.0, so that the stage url doesn't get shipped.

@@ +992,5 @@
>  #endif
>  
> +// Mobile Identity API prefs.
> +pref("services.mobileid.server.uri", "https://msisdn-dev.stage.mozaws.net");
> +pref("services.mobileid.forcehttps", true);

I feel like it would be better to have MobileIdentityClient.jsm default to forceHttps=true, in order to protect against misspelling or omitting the pref, and let people set this pref to false for local testing or whatnot.
Attachment #8449483 - Flags: review?(jed+bmo) → review+
(Assignee)

Comment 23

3 years ago
Thanks Jed!

https://hg.mozilla.org/integration/b2g-inbound/rev/1f24cbde93de
(Assignee)

Comment 24

3 years ago
Comment on attachment 8449483 [details] [diff] [review]
v1

Approval Request Comment
[Feature/regressing bug #]: Mobile ID
[User impact if declined]: Mobile ID won't work as it is using a deprecated endpoint.
[Describe test coverage new/current, TBPL]: Sadly no tests in TBPL yet. Unit tests will be added at bug 1022181 next week. We've tested it manually.
[Risks and why]: No risk. It is just pointing the requests to the right server and making sure that the server is https. 
[String/UUID change made/needed]: None
Attachment #8449483 - Flags: approval-mozilla-aurora?
FYI from Bhavana:  This will need tests (Bug 1022181) for this to be uplifted.
Depends on: 1022181
(In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #25)
> FYI from Bhavana:  This will need tests (Bug 1022181) for this to be
> uplifted.

Fernando - In this case your patch flips the prefs to require SSL and use an SSL endpoint. If the tests that you'll be writing in bug 1022181 are more general for the feature and not specific to this change, I don't think there is a reason to wait after we see a green build on master. Do you intend to write tests specific to this pref change?
Group: core-security
Flags: needinfo?(ferjmoreno)
(Assignee)

Comment 27

3 years ago
(In reply to Lawrence Mandel [:lmandel] from comment #26)
> (In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #25)
> > FYI from Bhavana:  This will need tests (Bug 1022181) for this to be
> > uplifted.
> 
> Fernando - In this case your patch flips the prefs to require SSL and use an
> SSL endpoint. If the tests that you'll be writing in bug 1022181 are more
> general for the feature and not specific to this change, I don't think there
> is a reason to wait after we see a green build on master. Do you intend to
> write tests specific to this pref change?

The tests at bug 1022181 that I was referring in comment 24 are to test the feature in a more generic way, not specific to this change. I don't think we should block this uplift on those tests. Sorry for not being more specific about this.
Flags: needinfo?(ferjmoreno)
(In reply to Fernando Jiménez Moreno [:ferjm] from comment #27)
> (In reply to Lawrence Mandel [:lmandel] from comment #26)
> > (In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #25)
> > > FYI from Bhavana:  This will need tests (Bug 1022181) for this to be
> > > uplifted.
> > 
> > Fernando - In this case your patch flips the prefs to require SSL and use an
> > SSL endpoint. If the tests that you'll be writing in bug 1022181 are more
> > general for the feature and not specific to this change, I don't think there
> > is a reason to wait after we see a green build on master. Do you intend to
> > write tests specific to this pref change?
> 
> The tests at bug 1022181 that I was referring in comment 24 are to test the
> feature in a more generic way, not specific to this change. I don't think we
> should block this uplift on those tests. Sorry for not being more specific
> about this.

Thanks for the clarification here, I'll approve the patch once the central landing happens within the next few hours.
https://hg.mozilla.org/mozilla-central/rev/1f24cbde93de
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-b2g-v2.1: --- → fixed
status-firefox33: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Comment on attachment 8449483 [details] [diff] [review]
v1

Aurora+
Attachment #8449483 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/ca9c1bf798be
status-b2g-v2.0: --- → fixed
status-firefox31: --- → unaffected
status-firefox32: --- → fixed
status-firefox-esr24: --- → unaffected
Group: core-security
You need to log in before you can comment on or make changes to this bug.