New issue
Advanced search Search tips

Issue 764337 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jan 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Implement device policy redundancy

Project Member Reported by igorcov@chromium.org, Sep 12 2017

Issue description

When device policy file gets corrupted, the device loses enrollment. We need a solution to have the policy redundant.

Design doc for that:
https://docs.google.com/document/d/1RdUKb-deQCBiyc6Dpdf4RDBEPV-feb5WHu7uzY1YozQ/edit
 
Labels: -Pri-3 Pri-1
Cc: tnagel@chromium.org atwilson@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e14b7c6a22cec82b1db66dd3b2f952ed348badb1

commit e14b7c6a22cec82b1db66dd3b2f952ed348badb1
Author: Igor <igorcov@chromium.org>
Date: Fri Oct 20 11:19:36 2017

UMA for number of invalid policy files.

When reading the device policy, the policy files are taken from newest
to oldest until a valid file is identified. This statistic tracks the
number of invalid files identified until a valid one was found.

Implementations that log the statistic:
https://chromium-review.googlesource.com/c/aosp/platform/external/libbrillo/+/674935
https://chromium-review.googlesource.com/c/chromiumos/platform2/+/681656

Bug:  764337 
Change-Id: I4a328d2d37eed62e5ddf9b8a7f0dccb80aeceec8
Reviewed-on: https://chromium-review.googlesource.com/681939
Commit-Queue: Igor <igorcov@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510399}
[modify] https://crrev.com/e14b7c6a22cec82b1db66dd3b2f952ed348badb1/tools/metrics/histograms/histograms.xml

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 14 2017

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

commit d213a401f639649b3e38f6def5782ae2f1322fce
Author: Igor <igorcov@chromium.org>
Date: Thu Dec 14 18:18:21 2017

libbrillo: policy: Increase resilience for policy read

The policy files will be stored with indexes as suffix to improve
resilience to disk corruptions. This change makes it possible to
read the policy data from new files and validate them.

Design doc:
https://docs.google.com/document/d/1RdUKb-deQCBiyc6Dpdf4RDBEPV-feb5WHu7uzY1YozQ/edit#heading=h.h0q5njx6l4xf

BUG= chromium:764337 
TEST=Manual
CQ-DEPEND=CL:681939, CL:819250, CL:819353, CL:819412

Change-Id: I10f31d307925c6ccef3cfd0887a71b7c774139e2
Reviewed-on: https://chromium-review.googlesource.com/674935
Commit-Ready: Igor <igorcov@chromium.org>
Tested-by: Igor <igorcov@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/d213a401f639649b3e38f6def5782ae2f1322fce/policy/libpolicy.cc
[modify] https://crrev.com/d213a401f639649b3e38f6def5782ae2f1322fce/policy/libpolicy.h
[add] https://crrev.com/d213a401f639649b3e38f6def5782ae2f1322fce/policy/policy_util.h
[add] https://crrev.com/d213a401f639649b3e38f6def5782ae2f1322fce/policy/policy_util.cc
[modify] https://crrev.com/d213a401f639649b3e38f6def5782ae2f1322fce/policy/device_policy_impl.h
[add] https://crrev.com/d213a401f639649b3e38f6def5782ae2f1322fce/policy/resilient_policy_util.cc
[modify] https://crrev.com/d213a401f639649b3e38f6def5782ae2f1322fce/libbrillo.gypi
[modify] https://crrev.com/d213a401f639649b3e38f6def5782ae2f1322fce/policy/tests/libpolicy_unittest.cc
[modify] https://crrev.com/d213a401f639649b3e38f6def5782ae2f1322fce/policy/device_policy.h
[modify] https://crrev.com/d213a401f639649b3e38f6def5782ae2f1322fce/libpolicy.gypi
[add] https://crrev.com/d213a401f639649b3e38f6def5782ae2f1322fce/policy/tests/resilient_policy_util_unittest.cc
[add] https://crrev.com/d213a401f639649b3e38f6def5782ae2f1322fce/policy/resilient_policy_util.h
[add] https://crrev.com/d213a401f639649b3e38f6def5782ae2f1322fce/policy/tests/policy_util_unittest.cc
[modify] https://crrev.com/d213a401f639649b3e38f6def5782ae2f1322fce/policy/device_policy_impl.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/73484d63dcc9d6cde61953ac0fb82a6dbb2d01f6

commit 73484d63dcc9d6cde61953ac0fb82a6dbb2d01f6
Author: Igor <igorcov@chromium.org>
Date: Thu Dec 14 18:18:21 2017

metrics: Update unit_test to use unique_ptr for PolicyProvider

The class policy::PolicyProvider from libbrillo has been updated
to accept unique_ptr instead of raw pointer for constructor.

That contructor is used in metrics_library_test. This CL updates
those tests to use the new constructor.

The changes in PolicyProvider contructor come from CL
https://chromium-review.googlesource.com/c/aosp/platform/external/libbrillo/+/674935

BUG= chromium:764337 
TEST=Unit tests pass.
CQ-DEPEND=CL:674935, CL:819353, CL:819412

Change-Id: Ia91fc0ba4373fe006fe25e204553aa80c9c1c530
Reviewed-on: https://chromium-review.googlesource.com/819250
Commit-Ready: Igor <igorcov@chromium.org>
Tested-by: Igor <igorcov@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/73484d63dcc9d6cde61953ac0fb82a6dbb2d01f6/metrics/metrics_library_test.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/578c221e9d0490f581ba6cf8ca9b5274dd20eb6c

commit 578c221e9d0490f581ba6cf8ca9b5274dd20eb6c
Author: Igor <igorcov@chromium.org>
Date: Thu Dec 14 18:18:21 2017

cryptohome: Update unit_test to use unique_ptr for PolicyProvider

The class policy::PolicyProvider from libbrillo has been updated
to accept unique_ptr instead of raw pointer for constructor.

This CL updates the tests to use the new constructor.

The changes in PolicyProvider contructor come from CL
https://chromium-review.googlesource.com/c/aosp/platform/external/libbrillo/+/674935

BUG= chromium:764337 
TEST=Unit tests pass.
CQ-DEPEND=CL:674935, CL:819250, CL:819412

Change-Id: I4ba9e202de9596aa696f78e5773ace4d15ed0de1
Reviewed-on: https://chromium-review.googlesource.com/819353
Commit-Ready: Igor <igorcov@chromium.org>
Tested-by: Igor <igorcov@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/578c221e9d0490f581ba6cf8ca9b5274dd20eb6c/cryptohome/homedirs_unittest.cc
[modify] https://crrev.com/578c221e9d0490f581ba6cf8ca9b5274dd20eb6c/cryptohome/service_unittest.cc
[modify] https://crrev.com/578c221e9d0490f581ba6cf8ca9b5274dd20eb6c/cryptohome/make_tests.cc
[modify] https://crrev.com/578c221e9d0490f581ba6cf8ca9b5274dd20eb6c/cryptohome/mount_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Dec 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/update_engine/+/9fd76b6b1dc4a80f8464427e981220c29abc1d66

commit 9fd76b6b1dc4a80f8464427e981220c29abc1d66
Author: Igor <igorcov@chromium.org>
Date: Thu Dec 14 18:18:21 2017

update_engine: Update unit tests to use unique_ptr for PolicyProvider

The class policy::PolicyProvider from libbrillo has been updated
to accept unique_ptr instead of raw pointer for constructor.

This CL updates the tests to use the new constructor.

The changes in PolicyProvider contructor come from CL
https://chromium-review.googlesource.com/c/aosp/platform/external/libbrillo/+/674935

BUG= chromium:764337 
TEST=cros_workon_make --board=${BOARD} chromeos-base/update_engine --test
CQ-DEPEND=CL:674935, CL:819250, CL:819353

Change-Id: I5537cfe9a88053193fbe8b0b6d844c59df148fc9
Reviewed-on: https://chromium-review.googlesource.com/819412
Commit-Ready: Igor <igorcov@chromium.org>
Tested-by: Igor <igorcov@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Sen Jiang <senj@chromium.org>

[modify] https://crrev.com/9fd76b6b1dc4a80f8464427e981220c29abc1d66/update_attempter_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/87b097eef01c27c9faaf2eae322d31bd550e93bf

commit 87b097eef01c27c9faaf2eae322d31bd550e93bf
Author: Igor <igorcov@chromium.org>
Date: Fri Jan 12 06:22:54 2018

login: Implementation for load/persist resilient policy

The device policy files will have indexes as extensions and every
boot will store the policy in a file with a new index. The load
policy reads the files from newest to oldest until a valid file is
found.

It is expected that only the latest file will be read at load, but
to protect the policy against possible disk corruptions the
redundancy is implemented to keep the policy from the latest three
boots. Design doc:
https://docs.google.com/document/d/1RdUKb-deQCBiyc6Dpdf4RDBEPV-feb5WHu7uzY1YozQ/edit#heading=h.h0q5njx6l4xf

BUG= chromium:764337 
TEST=Manual
CQ-DEPEND=CL:674935

Change-Id: I02a32189f03dfc1a65dfdbf75dd2f23f32ae3384
Reviewed-on: https://chromium-review.googlesource.com/681656
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Igor <igorcov@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>

[modify] https://crrev.com/87b097eef01c27c9faaf2eae322d31bd550e93bf/login_manager/mock_constructors.cc
[modify] https://crrev.com/87b097eef01c27c9faaf2eae322d31bd550e93bf/login_manager/login_metrics.cc
[modify] https://crrev.com/87b097eef01c27c9faaf2eae322d31bd550e93bf/login_manager/login_metrics.h
[modify] https://crrev.com/87b097eef01c27c9faaf2eae322d31bd550e93bf/login_manager/policy_service_unittest.cc
[add] https://crrev.com/87b097eef01c27c9faaf2eae322d31bd550e93bf/login_manager/resilient_policy_store.h
[modify] https://crrev.com/87b097eef01c27c9faaf2eae322d31bd550e93bf/login_manager/user_policy_service.cc
[modify] https://crrev.com/87b097eef01c27c9faaf2eae322d31bd550e93bf/login_manager/policy_service.h
[modify] https://crrev.com/87b097eef01c27c9faaf2eae322d31bd550e93bf/login_manager/device_policy_service.cc
[modify] https://crrev.com/87b097eef01c27c9faaf2eae322d31bd550e93bf/login_manager/device_policy_service.h
[modify] https://crrev.com/87b097eef01c27c9faaf2eae322d31bd550e93bf/login_manager/policy_service.cc
[modify] https://crrev.com/87b097eef01c27c9faaf2eae322d31bd550e93bf/login_manager/login_manager.gyp
[modify] https://crrev.com/87b097eef01c27c9faaf2eae322d31bd550e93bf/login_manager/device_local_account_manager.cc
[add] https://crrev.com/87b097eef01c27c9faaf2eae322d31bd550e93bf/login_manager/resilient_policy_store.cc
[modify] https://crrev.com/87b097eef01c27c9faaf2eae322d31bd550e93bf/login_manager/device_policy_service_unittest.cc
[modify] https://crrev.com/87b097eef01c27c9faaf2eae322d31bd550e93bf/login_manager/policy_store.h
[add] https://crrev.com/87b097eef01c27c9faaf2eae322d31bd550e93bf/login_manager/resilient_policy_store_unittest.cc
[modify] https://crrev.com/87b097eef01c27c9faaf2eae322d31bd550e93bf/login_manager/policy_store.cc

Status: Fixed (was: Started)
Status: Verified (was: Fixed)
Verified fixed in M67.0.3383.0 10539.0.0 dev paine.

Tested by:
1. Accumulated several policy.* in /var/lib/whitelist.
2. Corrupted the latest policy files leaving at least one good one.
3. Reboot and verified the sign-in screen comes up being still enrolled.
4. Signed in and verified the policies were in place and policies update worked.

Sign in to add a comment