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)

defect
Not set
normal

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)

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.
(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.
blocking-b2g: --- → 2.5+
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)
(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)
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.
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.
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.
(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.
So, do we have an agreement ? Can I re-do from scratch my patch in bug 1201556 ?
Flags: needinfo?(bhearsum)
(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)
We're working on this now. :) Will have something soon for you, Ben.
User Story: (updated)
Flags: needinfo?(jgong)
(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 ?
(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
(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.
Depends on: 1207313
User Story: (updated)
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".
Attached file b2g-whitelist.json (deleted) —
Example whitelist format (uploaded in release table for the special whitelist blob)
Assignee: nobody → rwood
Status: NEW → ASSIGNED
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)
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+
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)
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+
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
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)
Yes, I think we can do it.
Flags: needinfo?(lissyx+mozillians)
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.
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)
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+
Add the new 'whitelist' column to the admin view
Attachment #8666125 - Flags: review?(bhearsum)
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
Attachment #8666125 - Flags: review?(bhearsum)
Attachment #8666125 - Flags: review+
Attachment #8666125 - Flags: checked-in+
Attachment #8666032 - Flags: checked-in+
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.
Whiteboard: [tc-build-support]
Whiteboard: [tc-build-support] → [b2g-build-support]
Alexandre did some additional testing in dev and everything went well! I'll be pushing this to prod later today.
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
Blocks: 1201556
No longer depends on: 1201556
Depends on: 1211973
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
Product: Release Engineering → Release Engineering Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: