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

Issue 835759 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Create DBus call to compute and return the EID

Project Member Reported by igorcov@chromium.org, Apr 23 2018

Issue description

Currently we have DBus call to get the EID that can be obtained by command "cryptohome --action=get_enrollment_id". This returns the EID on the device, but the value is cached first time it is obtained and returned from cache after that.

We want to have the possibility to re-compute the value when we need, if e.g. the device secret has been changed. The plan is to have the command
"cryptohome --action=recompute_enrollment_id" and something similar in attestation client daemon.

I think it makes sense for re-computation to also fill/change the contents of cache.
 
Actually I think it will be better to add a flag to existent get_enrollment_id command instead of creating a new one. So the plan is to have "cryptohome --action=get_enrollment_id --recompute"
I don't think recomputation should change the value of the cache. The cache
should match the value gotten from an AIK cert, i.e. the current value.
Computing a value just means "ignore the cache and do the computation."
Blocking: 840496
Project Member

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

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

commit eeeaa17ea42d69e186766d58f49c96add1ccb01f
Author: Igor <igorcov@chromium.org>
Date: Wed May 16 19:41:26 2018

cryptohome: Support for get_enrollment_id action with ignore_cache option

The get_enrollment_id action from cryptohome caches the result first time
the EID is computed and uses the value from cache to return the data. This
change will allow the caller to specify to ignore the cache. This means the
EID should be recomputed while the new value should not be stored in cache.

BUG= chromium:835759 
TEST=unit test and manual test with cryptohome --action=get_enrollment_id --ignore_cache
CQ-DEPEND=CL:1024031

Change-Id: I2659f4b775b2de98d7592e793c186063ce78823c
Reviewed-on: https://chromium-review.googlesource.com/1024030
Commit-Ready: Igor <igorcov@chromium.org>
Tested-by: Igor <igorcov@chromium.org>
Reviewed-by: Igor <igorcov@chromium.org>
Reviewed-by: Yves Arrouye <drcrash@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/eeeaa17ea42d69e186766d58f49c96add1ccb01f/cryptohome/service_distributed.h
[modify] https://crrev.com/eeeaa17ea42d69e186766d58f49c96add1ccb01f/cryptohome/attestation.h
[modify] https://crrev.com/eeeaa17ea42d69e186766d58f49c96add1ccb01f/cryptohome/cryptohome.cc
[modify] https://crrev.com/eeeaa17ea42d69e186766d58f49c96add1ccb01f/cryptohome/service_monolithic.cc
[modify] https://crrev.com/eeeaa17ea42d69e186766d58f49c96add1ccb01f/cryptohome/service_monolithic.h
[modify] https://crrev.com/eeeaa17ea42d69e186766d58f49c96add1ccb01f/cryptohome/service.h
[modify] https://crrev.com/eeeaa17ea42d69e186766d58f49c96add1ccb01f/cryptohome/interface.cc
[modify] https://crrev.com/eeeaa17ea42d69e186766d58f49c96add1ccb01f/cryptohome/interface.h
[modify] https://crrev.com/eeeaa17ea42d69e186766d58f49c96add1ccb01f/cryptohome/service_distributed.cc
[modify] https://crrev.com/eeeaa17ea42d69e186766d58f49c96add1ccb01f/cryptohome/attestation_unittest.cc
[modify] https://crrev.com/eeeaa17ea42d69e186766d58f49c96add1ccb01f/cryptohome/dbus_bindings/org.chromium.CryptohomeInterface.xml

Project Member

Comment 6 by bugdroid1@chromium.org, May 16 2018

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

commit b2c4ec9d209b3ca94a9566a2dcdc30d1c2e63146
Author: Igor <igorcov@chromium.org>
Date: Wed May 16 19:41:26 2018

attestation: Support for get_enrollment_id action with ignore_cache option

The get_enrollment_id action from cryptohome caches the result first time
the EID is computed and uses the value from cache to return the data. This
change will allow the caller to specify that the cache should be ignored.
That would mean, the EID is recomputed while the new value is not stored in
cache.

BUG= chromium:835759 
TEST=unit test and manual test with cryptohome --get_enrollment_id --recompute

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

[modify] https://crrev.com/b2c4ec9d209b3ca94a9566a2dcdc30d1c2e63146/attestation/common/interface.proto
[modify] https://crrev.com/b2c4ec9d209b3ca94a9566a2dcdc30d1c2e63146/attestation/server/attestation_service.cc
[modify] https://crrev.com/b2c4ec9d209b3ca94a9566a2dcdc30d1c2e63146/attestation/common/print_interface_proto.cc
[modify] https://crrev.com/b2c4ec9d209b3ca94a9566a2dcdc30d1c2e63146/attestation/common/print_interface_proto.h
[modify] https://crrev.com/b2c4ec9d209b3ca94a9566a2dcdc30d1c2e63146/attestation/client/main.cc

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, May 26 2018

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

commit 9dc14d08658c0be36ddec29838d141393466fbf9
Author: Yves Arrouye <drcrash@google.com>
Date: Sat May 26 03:32:12 2018

cryptohome: allow Chromium to make DBUS calls to get an EID

BUG= chromium:835759 
TEST=manual on device, permission error disappears

Change-Id: I301a3bc23b9167b4152528e3587045537b1ad31d
Reviewed-on: https://chromium-review.googlesource.com/1072519
Commit-Ready: Yves Arrouye <drcrash@chromium.org>
Tested-by: Yves Arrouye <drcrash@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/9dc14d08658c0be36ddec29838d141393466fbf9/cryptohome/etc/Cryptohome.conf

Project Member

Comment 9 by bugdroid1@chromium.org, May 29 2018

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

commit bee75f64c9c84dce068bb90183dd2bdb90875b53
Author: Igor <igorcov@chromium.org>
Date: Tue May 29 16:44:50 2018

cryptohome: Remove permission for D-Bus calls to get EID from arc-oemcrypto

That permission is for chronos user. It is added by CL:1072519.

BUG= chromium:835759 
TEST=None

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

[modify] https://crrev.com/bee75f64c9c84dce068bb90183dd2bdb90875b53/cryptohome/etc/Cryptohome.conf

Cc: bthomson@chromium.org
Labels: Merge-Request-68
I would like #8 to be merged into M-68, as without it Chromium doesn't have permissions to call the method over DBUS (needed for chromium:840496).
Project Member

Comment 11 by sheriffbot@chromium.org, May 31 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
Blocking: -840496
Project Member

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

Labels: merge-merged-release-R68-10718.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/0763d4e324d2ba40721d1490c24d8bce6419aed2

commit 0763d4e324d2ba40721d1490c24d8bce6419aed2
Author: Yves Arrouye <drcrash@google.com>
Date: Thu May 31 18:41:52 2018

cryptohome: allow Chromium to make DBUS calls to get an EID

BUG= chromium:835759 
TEST=manual on device, permission error disappears

Change-Id: I301a3bc23b9167b4152528e3587045537b1ad31d
Reviewed-on: https://chromium-review.googlesource.com/1072519
Commit-Ready: Yves Arrouye <drcrash@chromium.org>
Tested-by: Yves Arrouye <drcrash@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>
(cherry picked from commit 9dc14d08658c0be36ddec29838d141393466fbf9)
Reviewed-on: https://chromium-review.googlesource.com/1081028
Commit-Queue: Yves Arrouye <drcrash@chromium.org>
Trybot-Ready: Yves Arrouye <drcrash@chromium.org>

[modify] https://crrev.com/0763d4e324d2ba40721d1490c24d8bce6419aed2/cryptohome/etc/Cryptohome.conf

Project Member

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

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

commit 8aa0dbaa55f3976abab0b3f09186b1ee3684e05f
Author: Igor <igorcov@chromium.org>
Date: Thu May 31 20:39:20 2018

cryptohome: Remove permission for D-Bus calls to get EID from arc-oemcrypto

That permission is for chronos user. It is added by CL:1072519.

BUG= chromium:835759 
TEST=None

Change-Id: I51b940e8fc9ec825da05b58e0a636bc85727a972
Reviewed-on: https://chromium-review.googlesource.com/1073281
Commit-Ready: Igor <igorcov@chromium.org>
Tested-by: Igor <igorcov@chromium.org>
Reviewed-by: Igor <igorcov@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>
(cherry picked from commit bee75f64c9c84dce068bb90183dd2bdb90875b53)
Reviewed-on: https://chromium-review.googlesource.com/1081031
Commit-Queue: Yves Arrouye <drcrash@chromium.org>
Tested-by: Yves Arrouye <drcrash@chromium.org>
Trybot-Ready: Yves Arrouye <drcrash@chromium.org>

[modify] https://crrev.com/8aa0dbaa55f3976abab0b3f09186b1ee3684e05f/cryptohome/etc/Cryptohome.conf

Project Member

Comment 15 by sheriffbot@chromium.org, Jun 4 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
Project Member

Comment 16 by sheriffbot@chromium.org, Jun 8 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

Sign in to add a comment