Closed
Bug 1201538
Opened 9 years ago
Closed 9 years ago
implement IMEI whitelisting for b2g dogfood updates
Categories
(Release Engineering Graveyard :: Applications: Balrog (backend), defect)
Release Engineering Graveyard
Applications: Balrog (backend)
Tracking
(blocking-b2g:2.5+)
RESOLVED
FIXED
blocking-b2g | 2.5+ |
People
(Reporter: bhearsum, Assigned: rwood)
References
Details
(Whiteboard: [b2g-build-support])
User Story
Below is the 3 use cases for implementing IMEI whitelisting for b2g dogfood updates Scenario #1 Criteria Update channel is "dogfood". Outcome User gets regular OTA updates (Gecko and Gaia only). Scenario #2 Criteria Update channel is "foxfood". Device is on IMEI whitelist. Outcome User gets both FOTA and possibly OTA updates as well. Scenario #3 Criteria Update channel is "foxfood" (new channel) Device is not on IMEI whitelist. Outcome User gets no updates
Attachments
(4 files)
(deleted),
application/json
|
Details | |
(deleted),
text/x-github-pull-request
|
bhearsum
:
review+
bhearsum
:
feedback+
bhearsum
:
checked-in+
|
Details |
(deleted),
text/x-github-pull-request
|
bhearsum
:
review+
bhearsum
:
checked-in+
|
Details |
(deleted),
text/x-github-pull-request
|
bhearsum
:
review+
bhearsum
:
checked-in+
|
Details |
We need to be able to update existing B2G Dogfood users with new bits that are not publicly distributable. The solution we've come up with is to send the device's IMEI with the update ping, and only serve back updates if it matches one in our whitelist. This bug is to track figuring out how to implement this in Balrog.
I've suggested that the client should send it in the %DISTRIBUTION% field, since that is currently unused. Ideally, we'd be able to serve different updates depending on whether or not a user is whitelisted, though I'm not quite sure I understand the use case for the latter. Ie: who are the users that aren't whitelisted, but are using the same device as dogfood users?
Jean tells me that the whitelist should be fairly static. If that remains true I think it's fine for RelEng to manage it, though if it's just as easy to make it self serve, that's even better.
Comment 1•9 years ago
|
||
(In reply to Ben Hearsum (:bhearsum) from comment #0)
[...]
>
> I've suggested that the client should send it in the %DISTRIBUTION% field,
> since that is currently unused. Ideally, we'd be able to serve different
> updates depending on whether or not a user is whitelisted, though I'm not
> quite sure I understand the use case for the latter. Ie: who are the users
> that aren't whitelisted, but are using the same device as dogfood users?
>
Those could be contributors doing builds and using it on their own while subscribed to our dogfood channel. This is actually real as far as I can tell.
Updated•9 years ago
|
blocking-b2g: --- → 2.5+
Comment 2•9 years ago
|
||
My patch in bug 1201556 attachment 8657774 [details] [diff] [review] includes hashing the IMEI with a SHA512. I want to make sure this will be handled either by Jean or RelEng when setting the list. If it's too complex to handle, I'll remove the hashing part.
Flags: needinfo?(jgong)
Flags: needinfo?(bhearsum)
Reporter | ||
Comment 3•9 years ago
|
||
(In reply to Alexandre LISSY :gerard-majax from comment #2)
> My patch in bug 1201556 attachment 8657774 [details] [diff] [review]
> includes hashing the IMEI with a SHA512. I want to make sure this will be
> handled either by Jean or RelEng when setting the list. If it's too complex
> to handle, I'll remove the hashing part.
Doesn't matter to us whether they're hashed or not. However, Nick and I discussed this in length yesterday, and decided it would be best to use a new field in the update URL rather than overload distribution. I'm writing up a more complete description of that plan right now, but the new update url will be something like:
https://aus4.mozilla.org/update/3/%PRODUCT%/%VERSION%/%BUILD_ID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/%IMEI%/update.xml
Flags: needinfo?(bhearsum)
Reporter | ||
Comment 4•9 years ago
|
||
Nick and I came up with a design that should allow IMEI whitelisting with very little overhead. We will use a new data structure for whitelisted IMEIs that rules can point at. This lets us permanently associate the B2G dogfood channel rule with a set of IMEIs without the need to duplicate it into each B2G dogfood Release. It can be set up in such a way that whomever manages the Dogfood program should be able to manage the list directly - with no need to go through RelEng. We think this is best for all involved, since it stops us from being gatekeepers on adding or removing IMEIs from the whitelist.
Gory details as follows:
* Accept IMEI in new field of update URL, something like: https://aus4.mozilla.org/update/5/%PRODUCT%/%VERSION%/%BUILD_ID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/%PLATFORM_VERSION%/%IMEI%/update.xml
** This needs a little bit more thought still, because %PLATFORM_VERSION% doesn't really make sense of on B2G, but we can't leave it out if we're creating a new update url format.
* Add a new type of blob that will store the IMEIs. This is a bit of hack, since Releases are typically metadata that gets returned in a response, but it's the best place we have without a major refactor.
* Add a new column to the Rules table. When NULL, no whitelist will be enforced. When not NULL, it must point to a whitelist blob (stored in the Releases table). The blob will be given the incoming IMEI, and be asked whether or not the incoming query should be served an update. If it should, the server will continue processing the update as usual. If it shouldn't, it will return an empty response.
As mentioned in e-mail, no one in RelEng has time to work on implementing this right now, but we (probably I) can mentor. Balrog is a well tested code base with Vagrant support and a good dev environment - so this shouldn't be too difficult for someone else to take on as long as they're comfortable with Python. I can also provide some pointers on the code that will need to be touched.
Reporter | ||
Comment 5•9 years ago
|
||
In the meeting today I was reminded that we still want to serve _different_ updates for people on the dogfood channel that aren't in the IMEI whitelist. The plan in the previous comment doesn't satisfy that requirement.
I said in the meeting that if it's a requirement we'd find a way to support it, but the more I think about, the more I think it's a bad idea. The architecture of the update server assumes that product (B2G), channel (dogfood), platform (aries), and locale (en-US) represent a _single_ stream of users. I don't think that's an assumption we should change for a short term solution.
Additional, we've never supported updates for users that do their own builds and I don't see what makes this any different. If we _really_ care about this, then we should have these users set their channel to something different (eg: homemade-dogfood), which would fit just fine into the Balrog architecture.
Comment 6•9 years ago
|
||
Some clarifying points below, Ben. Can you please let me know if this changes your perspective at all this subject?
> In the meeting today I was reminded that we still want to serve _different_
> updates for people on the dogfood channel that aren't in the IMEI whitelist.
> The plan in the previous comment doesn't satisfy that requirement.
While deferring to the even use case details that Jean is working on, I believe the desire is still to only support two different channels. dogfood+whitelist, and everyone else as a 'default' (including non-whitelisted dogfooders, and non-dogfooders who send no IMEI information at all).
> Additional, we've never supported updates for users that do their own builds
> and I don't see what makes this any different. If we _really_ care about
> this, then we should have these users set their channel to something
> different (eg: homemade-dogfood), which would fit just fine into the Balrog
> architecture.
Again, I defer to product team and Jean for relative importance, but the separation is not to accommodate custom builds but to maintain update support for non-dogfood builds that we provide. Not all of the devices/builds that we ship will be dogfood builds; some will likely be built off master, etc.
Reporter | ||
Comment 7•9 years ago
|
||
(In reply to Elvin Lee [:ellee] from comment #6)
> > In the meeting today I was reminded that we still want to serve _different_
> > updates for people on the dogfood channel that aren't in the IMEI whitelist.
> > The plan in the previous comment doesn't satisfy that requirement.
>
> While deferring to the even use case details that Jean is working on, I
> believe the desire is still to only support two different channels.
> dogfood+whitelist, and everyone else as a 'default' (including
> non-whitelisted dogfooders, and non-dogfooders who send no IMEI information
> at all).
I think the word "channel" here is overloaded, so let's try to unpack that first. When I say "channel", I'm talking about the string sent in the "channel" section of the update URL. For example, here is an update URL with "dogfood" as the channel:
https://aus4.mozilla.org/update/3/B2G/43.0a1/20150812231434/aries/en-US/dogfood/Boot2Gecko%202.5.0.0-prerelease%20(SDK%2019)/default/default/update.xml.
What you're talking about above sounds to me like distinct populations (sets of users who we may want to serve different updates to), who share the *same* channel. If I'm wrong about that, and the "default" and "non-dogfooder" populations set a different channel in the URL, that's great!
> > Additional, we've never supported updates for users that do their own builds
> > and I don't see what makes this any different. If we _really_ care about
> > this, then we should have these users set their channel to something
> > different (eg: homemade-dogfood), which would fit just fine into the Balrog
> > architecture.
>
> Again, I defer to product team and Jean for relative importance, but the
> separation is not to accommodate custom builds but to maintain update
> support for non-dogfood builds that we provide. Not all of the
> devices/builds that we ship will be dogfood builds; some will likely be
> built off master, etc.
OK, perhaps I misunderstood. If we're talking about builds that _we_ control, we can probably change their channel to be something unique for that population, if they aren't already. It would be really helpful to have example update URLs for all of the scenarios we care about.
Comment 8•9 years ago
|
||
So, do we have an agreement ? Can I re-do from scratch my patch in bug 1201556 ?
Flags: needinfo?(bhearsum)
Reporter | ||
Comment 9•9 years ago
|
||
(In reply to Alexandre LISSY :gerard-majax from comment #8)
> So, do we have an agreement ? Can I re-do from scratch my patch in bug
> 1201556 ?
I don't know; last I heard Jean was going to double check what the requirements were, I've been waiting on that.
Flags: needinfo?(bhearsum)
Comment 10•9 years ago
|
||
We're working on this now. :) Will have something soon for you, Ben.
Updated•9 years ago
|
User Story: (updated)
Flags: needinfo?(jgong)
Comment 11•9 years ago
|
||
(Commenting on User Story)
> Below is the 3 use cases for implementing IMEI whitelisting for b2g dogfood
> updates
>
> scenario #1
> Requirement:
> 1. Update channel = dogfood
> 2. not a device issued to Moz employee (IMEI not whitelisted)
> Result: no updates
Currently we are delivering OTA of Gecko/Gaia. Looks like a regression. Looks like in contradiction with our goal to make updates more easily accessibles.
>
> scenario #2
> Requirement:
> 1. Update channel = dogfood
> 2. Is a device issued to Moz employee (IMEI whitelisted)
> Result: provide FOTA update
Will we always be delivering a FOTA fullimg (gecko-update-fota-fullimg target) ?
Or should we also switch to FOTA payload for Gecko/Gaia only (gecko-update-fota target) ?
Or can we mix FOTA and OTA on server-side ?
>
> scenario #3
> Requirement:
> 1. Update channel = nightly
> 2. Either Moz employee or contributor
> Result: always provide update (FFOS team in charge of ensuring permitted
> update)
What update payload ? Simple Gecko/Gaia or fullimg ?
What about QA ?
Comment 12•9 years ago
|
||
(In reply to Alexandre LISSY :gerard-majax from comment #11)
[...]
> >
> > scenario #1
> > Requirement:
> > 1. Update channel = dogfood
> > 2. not a device issued to Moz employee (IMEI not whitelisted)
> > Result: no updates
>
> Currently we are delivering OTA of Gecko/Gaia. Looks like a regression.
> Looks like in contradiction with our goal to make updates more easily
> accessibles.
Confere user story in bug 1180678
Reporter | ||
Comment 13•9 years ago
|
||
(Commenting on User Story)
> Below is the 3 use cases for implementing IMEI whitelisting for b2g dogfood
> updates
>
> scenario #1
> Requirement:
> 1. Update channel = dogfood
> 2. not a device issued to Moz employee (IMEI not whitelisted)
> Result: no updates
>
> scenario #2
> Requirement:
> 1. Update channel = dogfood
> 2. Is a device issued to Moz employee (IMEI whitelisted)
> Result: provide FOTA update
Because scenario #1 has no updates, this works fine with our design from comment #4. Please note that the fact that these two scenarios share a channel is problematic if we ever want to serve updates to those scenario #1 users - so implementing the solution in comment #4 will _not_ allow us to serve different updates to these users. The update server was built with the assumption that product+channel is one "stream" of builds, that gets one type of update. Additional fields, such as the IMEI one that we're adding, let us do some filtering of the updates, but they're not designed for segregating users into different streams of updates. Apologies if I gave a different impression at an earlier time - I think I misunderstood the difference between these groups of users.
Regardless of what we do here, I think we should have users on scenario #1 flash a different build, with a different channel - that will allow us to treat them as a distinct population, and give them whatever updates they need in the future.
> scenario #3
> Requirement:
> 1. Update channel = nightly
> 2. Either Moz employee or contributor
> Result: always provide update (FFOS team in charge of ensuring permitted
> update)
OK. I think this doesn't affect this bug at all? Ie: no whitelisting for this channel, and no changes to existing builds/updates?
(In reply to Alexandre LISSY :gerard-majax from comment #11)
> (Commenting on User Story)
> > scenario #2
> > Requirement:
> > 1. Update channel = dogfood
> > 2. Is a device issued to Moz employee (IMEI whitelisted)
> > Result: provide FOTA update
>
> Will we always be delivering a FOTA fullimg (gecko-update-fota-fullimg
> target) ?
> Or should we also switch to FOTA payload for Gecko/Gaia only
> (gecko-update-fota target) ?
> Or can we mix FOTA and OTA on server-side ?
The server can serve either - the only difference to it is a little bit of the metadata in the response.
Updated•9 years ago
|
User Story: (updated)
Comment 14•9 years ago
|
||
After talking to Ben on Thursday, there were further feedback from Doug, Naoki and Alex on the need to create a NEW Product+channel "Foxfood".
Assignee | ||
Comment 15•9 years ago
|
||
Example whitelist format (uploaded in release table for the special whitelist blob)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → rwood
Status: NEW → ASSIGNED
Assignee | ||
Comment 16•9 years ago
|
||
Patch for server-side b2g update white-listing. Note: The balrog admin-ui will also have to be updated to accommodate the new 'whitelist' column in the rules table; that will be a separate PR.
Attachment #8664503 -
Flags: review?(bhearsum)
Reporter | ||
Comment 17•9 years ago
|
||
Comment on attachment 8664503 [details]
https://github.com/mozilla/balrog/pull/26
Detailed comments are in the PR. tl;dr - this is pretty much good to go, just needs a few nits and test fixes.
Attachment #8664503 -
Flags: review?(bhearsum) → feedback+
Assignee | ||
Comment 18•9 years ago
|
||
Comment on attachment 8664503 [details]
https://github.com/mozilla/balrog/pull/26
PR updated - nits addressed, fixed broken unit tests, and added new unit tests for the new whitelist blob and new request url version.
Attachment #8664503 -
Flags: review?(bhearsum)
Reporter | ||
Comment 19•9 years ago
|
||
Comment on attachment 8664503 [details]
https://github.com/mozilla/balrog/pull/26
Looks great to me, I'll merge it over :).
Attachment #8664503 -
Flags: review?(bhearsum)
Attachment #8664503 -
Flags: review+
Attachment #8664503 -
Flags: checked-in+
Comment 20•9 years ago
|
||
Commits pushed to master at https://github.com/mozilla/balrog
https://github.com/mozilla/balrog/commit/8c411b9464cae3f80b17024b36dcdf299171e102
Bug 1201538 - Implement IMEI whitelisting for b2g fota updates; r=bhearsum
https://github.com/mozilla/balrog/commit/750ddce45547bf05d4b002252f4635d3a7be08b2
Merge pull request #26 from rwood-moz/bug1201538
Bug 1201538 - Implement IMEI whitelisting for b2g fota updates; r=bhearsum
Reporter | ||
Comment 21•9 years ago
|
||
As Rob as mentioned, there's still a necessary patch to the UI before we can push to prod, but we should be able to do some manual set-up for testing in dev.
Alexandre, if I set-up an update path on https://aus4-dev.allizom.org w/ IMEI whitelisting enabled, are you able to test with a real device?
Flags: needinfo?(lissyx+mozillians)
Reporter | ||
Comment 23•9 years ago
|
||
OK, I've got a test rule set-up in dev, which can been working through these two URLs:
https://aus5-dev.allizom.org/update/5/B2G/42.0a1/20141202185629/aries/en-US/dogfood/default/default/default/INVALID/update.xml?force=1 (not in the whitelist, gets no update)
https://aus5-dev.allizom.org/update/5/B2G/42.0a1/20141202185629/aries/en-US/dogfood/default/default/default/FAKE_IMEI_FOR_TESTING/update.xml?force=1 (in the whitelist, gets an update)
I can add other values to the whitelist if necessary. When someone has time to test, it would be great to test with bug 1201556 on a device, but with the app.update.url pointing at aus5-dev.allizom.org instead of aus5.mozilla.org.
Assignee | ||
Comment 24•9 years ago
|
||
Patch for the front-end: Adding the new rules 'whitelist' column to the admin-ui. The field appears in the adding/updating rule dialog and works, and the whitelist value will appear in the main rules list after it is added. However if you navigate to a different part of the admin like releases, then go back to rules, the whitelist value is gone.
I've never used angular before so just kind of guessed with this, any help is appreciated thanks :)
Attachment #8666032 -
Flags: feedback?(bhearsum)
Reporter | ||
Comment 25•9 years ago
|
||
Comment on attachment 8666032 [details]
https://github.com/mozilla/balrog-ui/pull/10
This patch looks fine, I left some comments about the issue you're seeing in the PR.
Attachment #8666032 -
Flags: feedback?(bhearsum) → review+
Assignee | ||
Comment 26•9 years ago
|
||
Add the new 'whitelist' column to the admin view
Attachment #8666125 -
Flags: review?(bhearsum)
Comment 27•9 years ago
|
||
Commits pushed to master at https://github.com/mozilla/balrog
https://github.com/mozilla/balrog/commit/cede4f527909736958ff027d79914b71b7507d41
Bug 1201538 - Update admin view with new whitelist column; r=bhearsum
https://github.com/mozilla/balrog/commit/fa05ecb0bf43820e462ed2e323d3ae4cc248e5cf
Merge pull request #27 from rwood-moz/bug1201538-admin
Bug 1201538 - Update admin view with new whitelist column; r=bhearsum
Reporter | ||
Updated•9 years ago
|
Attachment #8666125 -
Flags: review?(bhearsum)
Attachment #8666125 -
Flags: review+
Attachment #8666125 -
Flags: checked-in+
Reporter | ||
Updated•9 years ago
|
Attachment #8666032 -
Flags: checked-in+
Reporter | ||
Comment 28•9 years ago
|
||
Everything here has been landed into the Balrog dev environment. Before pushing to prod I'd like to get a test done against it to make sure there isn't any issues that we missed in review or other testing.
Updated•9 years ago
|
Whiteboard: [tc-build-support]
Updated•9 years ago
|
Whiteboard: [tc-build-support] → [b2g-build-support]
Reporter | ||
Comment 29•9 years ago
|
||
Alexandre did some additional testing in dev and everything went well! I'll be pushing this to prod later today.
Reporter | ||
Comment 30•9 years ago
|
||
I pushed this to prod and verified with a temporary rule on the "bhearsumtest" channel. It works \o/. Big thanks again to Rob for implementing it!
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Updated•9 years ago
|
Comment 31•8 years ago
|
||
Commit pushed to master at https://github.com/mozilla/balrog
https://github.com/mozilla/balrog/commit/2e8e1d8054e08bd7d57adb35825abce9407eb4bb
[balrog-ui] Merge pull request #10 from rwood-moz/bug1201538
bug 1201538: Add rules whitelist column to admin-ui; r=bhearsum
Updated•5 years ago
|
Product: Release Engineering → Release Engineering Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•