New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 847226 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Security



Sign in to add a comment

Current update_engine code breaks rollback protection for enterprise devices

Project Member Reported by mnissler@chromium.org, May 28 2018

Issue description

Unless I'm missing something, the behavior implemented at https://cs.corp.google.com/chromeos_public/src/aosp/system/update_engine/omaha_request_action.cc?rcl=80f4d4cefb5270960d17cd347d71594e97c7a954&l=1858 (introduced via https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/940567) effectively breaks rollback protection for *all* enterprise customers and requires a code change to restore.

We can't ship a stable release with this code - in case we need to bump kernel key versions during the M68 life cycle, we'd require a *code change* to dis-engage the short-circuited kernel rollback check. Furthermore, who guarantees that the code eventually gets adjusted to properly handle rollback windows as instructed by the server? At the very least, this code needs to be adjusted to take into account the maximum rollback window size (but that isn't easily done given the design doesn't allow a client to figure out kernel key version / OS version mappings).

If this can't be implemented properly for M68, then the change should be reverted to restore the old behavior until the server wiring is done.
 
Project Member

Comment 1 by sheriffbot@chromium.org, May 29 2018

Labels: -Pri-3 Pri-1
Project Member

Comment 2 by sheriffbot@chromium.org, May 29 2018

Status: Assigned (was: Untriaged)
Project Member

Comment 3 by sheriffbot@chromium.org, May 30 2018

Labels: -Security_Impact-Head Security_Impact-Beta
Cc: derat@chromium.org
Project Member

Comment 5 by bugdroid1@chromium.org, May 31 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/external/libbrillo/+/d7220e5c8555ebae190362b554973e7458eff13c

commit d7220e5c8555ebae190362b554973e7458eff13c
Author: Marton Hunyady <hunyadym@chromium.org>
Date: Thu May 31 19:26:16 2018

libbrillo: Set RollbackAllowedMilestones enterprise default to 0

Changing the default value of RollbackAllowedMilestones policy
from 4 to 0 for enterprises (already 0 for consumer devices).

Until the kernel and firmware key versions can be set properly
from Omaha, we don't freeze key rolls by default, only if it's
explicitly specified using enterprise policy.

BUG= chromium:847226 
TEST='cros_run_unit_tests --board=caroline --packages libbrillo'

Change-Id: I4451a4e31f475a8b7557fea4f44dc35afe95ef0b
Reviewed-on: https://chromium-review.googlesource.com/1078811
Commit-Ready: Marton Hunyady <hunyadym@chromium.org>
Tested-by: Marton Hunyady <hunyadym@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/d7220e5c8555ebae190362b554973e7458eff13c/policy/tests/device_policy_impl_unittest.cc
[modify] https://crrev.com/d7220e5c8555ebae190362b554973e7458eff13c/policy/tests/libpolicy_unittest.cc
[modify] https://crrev.com/d7220e5c8555ebae190362b554973e7458eff13c/policy/device_policy_impl.cc

Labels: Merge-Request-68
Project Member

Comment 7 by sheriffbot@chromium.org, Jun 1 2018

Labels: -Merge-Request-68 Hotlist-Merge-Approved Merge-Approved-68
Your change meets the bar and is auto-approved for M68. Please go ahead and merge the CL to branch 3440 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 2 2018

Labels: merge-merged-release-R68-10718.B
The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/external/libbrillo/+/b0d8595b22253f329e56b130d193f5f6a7d7852b

commit b0d8595b22253f329e56b130d193f5f6a7d7852b
Author: Marton Hunyady <hunyadym@chromium.org>
Date: Sat Jun 02 01:17:29 2018

libbrillo: Set RollbackAllowedMilestones enterprise default to 0

Changing the default value of RollbackAllowedMilestones policy
from 4 to 0 for enterprises (already 0 for consumer devices).

Until the kernel and firmware key versions can be set properly
from Omaha, we don't freeze key rolls by default, only if it's
explicitly specified using enterprise policy.

BUG= chromium:847226 
TEST='cros_run_unit_tests --board=caroline --packages libbrillo'

Change-Id: I4451a4e31f475a8b7557fea4f44dc35afe95ef0b
Reviewed-on: https://chromium-review.googlesource.com/1078811
Commit-Ready: Marton Hunyady <hunyadym@chromium.org>
Tested-by: Marton Hunyady <hunyadym@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
(cherry picked from commit d7220e5c8555ebae190362b554973e7458eff13c)

[modify] https://crrev.com/b0d8595b22253f329e56b130d193f5f6a7d7852b/policy/tests/device_policy_impl_unittest.cc
[modify] https://crrev.com/b0d8595b22253f329e56b130d193f5f6a7d7852b/policy/tests/libpolicy_unittest.cc
[modify] https://crrev.com/b0d8595b22253f329e56b130d193f5f6a7d7852b/policy/device_policy_impl.cc

Project Member

Comment 9 by sheriffbot@chromium.org, Jun 5 2018

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Approved-68
Status: Fixed (was: Assigned)
We kept the old behavior for now, taking on the (small) risk that we need to roll key versions. In 69 we will wire up the final version. If on the small chance we need to rev the versions before then we will determine if we try and bundle a fix with the key rev or accept the possible shrinking of the rollback window.
Project Member

Comment 13 by sheriffbot@chromium.org, Jun 7 2018

Labels: Restrict-View-SecurityNotify
Labels: -ReleaseBlock-Stable
Project Member

Comment 15 by sheriffbot@chromium.org, Sep 13

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Verified (was: Fixed)

Sign in to add a comment