New issue
Advanced search Search tips

Issue 809616 link

Starred by 3 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Task



Sign in to add a comment

Consolidate usage of cryptohome APIs to new-style Ex methods

Project Member Reported by ejcaruso@chromium.org, Feb 6 2018

Issue description

This will get rid of D-Bus API surface area and lots of redundant code, making it easier for us to move to newer D-Bus bindings.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 7 2018

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

commit 3a52d3c3da39ceb487474e2590ac15b69d1e511d
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Wed Feb 07 02:18:41 2018

cryptohome: remove AddKey and AsyncAddKey

The only user of AsyncAddKey is SupervisedUserAuthenticator in
Chrome, which is dead code and is being removed in CL:898252.
There are also no users of either AddKey or AsyncAddKey via CLI
or D-Bus in Chrome OS.

This also allows us to get rid of MountTaskAddPasskey.

BUG=chromium:809616
TEST=unit tests

Change-Id: Ibe6f9d1e7861c271daa6cbff81603bc333c6007c
Reviewed-on: https://chromium-review.googlesource.com/899507
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/3a52d3c3da39ceb487474e2590ac15b69d1e511d/cryptohome/etc/Cryptohome.conf
[modify] https://crrev.com/3a52d3c3da39ceb487474e2590ac15b69d1e511d/cryptohome/cryptohome.cc
[modify] https://crrev.com/3a52d3c3da39ceb487474e2590ac15b69d1e511d/cryptohome/service.h
[modify] https://crrev.com/3a52d3c3da39ceb487474e2590ac15b69d1e511d/cryptohome/mount_task.h
[modify] https://crrev.com/3a52d3c3da39ceb487474e2590ac15b69d1e511d/cryptohome/mount_task_unittest.cc
[modify] https://crrev.com/3a52d3c3da39ceb487474e2590ac15b69d1e511d/cryptohome/interface.cc
[modify] https://crrev.com/3a52d3c3da39ceb487474e2590ac15b69d1e511d/cryptohome/interface.h
[modify] https://crrev.com/3a52d3c3da39ceb487474e2590ac15b69d1e511d/cryptohome/service.cc
[modify] https://crrev.com/3a52d3c3da39ceb487474e2590ac15b69d1e511d/cryptohome/mount_task.cc
[modify] https://crrev.com/3a52d3c3da39ceb487474e2590ac15b69d1e511d/cryptohome/dbus_bindings/org.chromium.CryptohomeInterface.xml

Cc: hashimoto@chromium.org xiy...@chromium.org hidehiko@chromium.org satorux@chromium.org
 Issue 809878  has been merged into this issue.
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 7 2018

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

commit eee5c1d3508efbac2cc138273f75eacb06c5b2ac
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Wed Feb 07 17:24:44 2018

Remove AsyncAddKey from CryptohomeClient

SupervisedUserAuthenticator was the last user of this API, but it
was dead code and has now been removed. New users should be calling
AddKeyEx instead, so we can get rid of this method to prevent new
calls to AsyncAddKey from being added.

BUG=chromium:809616

Change-Id: I5a3deec23aabc31965762732c0c571106e43d310
Reviewed-on: https://chromium-review.googlesource.com/905094
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Commit-Queue: Eric Caruso <ejcaruso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535037}
[modify] https://crrev.com/eee5c1d3508efbac2cc138273f75eacb06c5b2ac/chromeos/cryptohome/async_method_caller.cc
[modify] https://crrev.com/eee5c1d3508efbac2cc138273f75eacb06c5b2ac/chromeos/cryptohome/async_method_caller.h
[modify] https://crrev.com/eee5c1d3508efbac2cc138273f75eacb06c5b2ac/chromeos/cryptohome/mock_async_method_caller.cc
[modify] https://crrev.com/eee5c1d3508efbac2cc138273f75eacb06c5b2ac/chromeos/cryptohome/mock_async_method_caller.h
[modify] https://crrev.com/eee5c1d3508efbac2cc138273f75eacb06c5b2ac/chromeos/dbus/cryptohome_client.cc
[modify] https://crrev.com/eee5c1d3508efbac2cc138273f75eacb06c5b2ac/chromeos/dbus/cryptohome_client.h
[modify] https://crrev.com/eee5c1d3508efbac2cc138273f75eacb06c5b2ac/chromeos/dbus/fake_cryptohome_client.cc
[modify] https://crrev.com/eee5c1d3508efbac2cc138273f75eacb06c5b2ac/chromeos/dbus/fake_cryptohome_client.h

Project Member

Comment 4 by bugdroid1@chromium.org, Feb 13 2018

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

commit 4f551d36c112a904a45b1973548f9b4d87b65c4c
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Tue Feb 13 17:28:43 2018

CryptohomeAuthenticator: replace AsyncCheckKey with CheckKeyEx

This is the last use of AsyncCheckKey in the code base and can
be pretty trivially ported to CheckKeyEx.

BUG=chromium:809616
TEST=install tbarzic's kiosk test app and boot with consumer kiosk
  mode enabled, then disconnect from the network and attempt to
  set up network config from the kiosk app splash screen, ensure that
  a CheckKeyEx call is generated with dbus-monitor and that log in/
  network configuration works

Change-Id: Iced96727d4ae665399844b84c6ba4ca13e371976
Reviewed-on: https://chromium-review.googlesource.com/910408
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Commit-Queue: Eric Caruso <ejcaruso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536398}
[modify] https://crrev.com/4f551d36c112a904a45b1973548f9b4d87b65c4c/chrome/browser/chromeos/login/auth/cryptohome_authenticator_unittest.cc
[modify] https://crrev.com/4f551d36c112a904a45b1973548f9b4d87b65c4c/chromeos/login/auth/cryptohome_authenticator.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Feb 13 2018

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

commit 39ed2a2f58bb7672a2437e4a26a07195e04f9508
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Tue Feb 13 22:10:13 2018

Remove AsyncCheckKey from CryptohomeClient

This is now unused after porting CryptohomeAuthenticator's use
to CheckKeyEx.

BUG=chromium:809616
TEST=compile

Change-Id: I05a47472d8cf43d948025538e11b10a5327320ab
Reviewed-on: https://chromium-review.googlesource.com/910390
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Commit-Queue: Eric Caruso <ejcaruso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536503}
[modify] https://crrev.com/39ed2a2f58bb7672a2437e4a26a07195e04f9508/chromeos/cryptohome/async_method_caller.cc
[modify] https://crrev.com/39ed2a2f58bb7672a2437e4a26a07195e04f9508/chromeos/cryptohome/async_method_caller.h
[modify] https://crrev.com/39ed2a2f58bb7672a2437e4a26a07195e04f9508/chromeos/cryptohome/mock_async_method_caller.cc
[modify] https://crrev.com/39ed2a2f58bb7672a2437e4a26a07195e04f9508/chromeos/cryptohome/mock_async_method_caller.h
[modify] https://crrev.com/39ed2a2f58bb7672a2437e4a26a07195e04f9508/chromeos/dbus/cryptohome_client.cc
[modify] https://crrev.com/39ed2a2f58bb7672a2437e4a26a07195e04f9508/chromeos/dbus/cryptohome_client.h
[modify] https://crrev.com/39ed2a2f58bb7672a2437e4a26a07195e04f9508/chromeos/dbus/fake_cryptohome_client.cc
[modify] https://crrev.com/39ed2a2f58bb7672a2437e4a26a07195e04f9508/chromeos/dbus/fake_cryptohome_client.h

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/system_api/+/668650a1227dce05c3aaeb49b7d22d0bdae2d1dd

commit 668650a1227dce05c3aaeb49b7d22d0bdae2d1dd
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Wed Feb 14 05:16:35 2018

cryptohome: remove constant for AsyncAddKey

This method is deprecated and unused, so we're deleting the
method name to prevent new calls to it.

CQ-DEPEND=CL:899507
BUG=chromium:809616
TEST=unit tests

Change-Id: I5d6238808d1a976cfc718180afb030617092f1c9
Reviewed-on: https://chromium-review.googlesource.com/904974
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/668650a1227dce05c3aaeb49b7d22d0bdae2d1dd/dbus/cryptohome/dbus-constants.h

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/de07cf83b5a14d38f95a9d23dc541f5fbbd116ce

commit de07cf83b5a14d38f95a9d23dc541f5fbbd116ce
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Sat Feb 17 02:48:05 2018

cryptohome: replace AsyncCheckKey call with CheckKeyEx

The test_auth action in the cryptohome CLI uses AsyncCheckKey,
but we want to move to CheckKeyEx. This is pretty trivially
doable.

BUG=chromium:809616
TEST=platform_CryptohomeTestAuth

Change-Id: Ide42f1e711026ef4bad8c42d09570ccb6ef4071a
Reviewed-on: https://chromium-review.googlesource.com/915051
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/de07cf83b5a14d38f95a9d23dc541f5fbbd116ce/client/cros/cryptohome.py

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 22 2018

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

commit 73321f7ecefe2b21457f361aa853e0c7c635947f
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Thu Feb 22 06:26:51 2018

cryptohome: remove CheckKey and AsyncCheckKey

These have no users, either from Chrome, elsewhere in platform2,
or using --action=test_auth on the command line. Remove these
methods so future users will stick to CheckKeyEx. This also makes
MountTaskTestCredentials dead so we can remove that as well.

BUG=chromium:809616
TEST=unit tests

Change-Id: I0ce4bd517ad9a636e8b18342f82e5fd1923f9964
Reviewed-on: https://chromium-review.googlesource.com/926963
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/73321f7ecefe2b21457f361aa853e0c7c635947f/cryptohome/service_unittest.cc
[modify] https://crrev.com/73321f7ecefe2b21457f361aa853e0c7c635947f/cryptohome/etc/Cryptohome.conf
[modify] https://crrev.com/73321f7ecefe2b21457f361aa853e0c7c635947f/cryptohome/cryptohome.cc
[modify] https://crrev.com/73321f7ecefe2b21457f361aa853e0c7c635947f/cryptohome/service.h
[modify] https://crrev.com/73321f7ecefe2b21457f361aa853e0c7c635947f/cryptohome/mount_task.h
[modify] https://crrev.com/73321f7ecefe2b21457f361aa853e0c7c635947f/cryptohome/mount_task_unittest.cc
[modify] https://crrev.com/73321f7ecefe2b21457f361aa853e0c7c635947f/cryptohome/interface.cc
[modify] https://crrev.com/73321f7ecefe2b21457f361aa853e0c7c635947f/cryptohome/interface.h
[modify] https://crrev.com/73321f7ecefe2b21457f361aa853e0c7c635947f/cryptohome/service.cc
[modify] https://crrev.com/73321f7ecefe2b21457f361aa853e0c7c635947f/cryptohome/mount_task.cc
[modify] https://crrev.com/73321f7ecefe2b21457f361aa853e0c7c635947f/cryptohome/dbus_bindings/org.chromium.CryptohomeInterface.xml

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/system_api/+/d0e1f7622c923d6d8431d06031df2267714047a4

commit d0e1f7622c923d6d8431d06031df2267714047a4
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Fri Feb 23 07:54:29 2018

cryptohome: remove constant for CheckKey/AsyncCheckKey

These methods are deprecated and no longer used, so remove the
method name constants to prevent new users from showing up. They
should be using CheckKeyEx instead.

BUG=chromium:809616
TEST=build_packages

Change-Id: I072c4f14178c9ba367592c6ea3b58a29bd8f8923
Reviewed-on: https://chromium-review.googlesource.com/926711
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/d0e1f7622c923d6d8431d06031df2267714047a4/dbus/cryptohome/dbus-constants.h

Project Member

Comment 10 by bugdroid1@chromium.org, Apr 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/28b6552c5bc31a72beb0dfc4c80bb034d35f53ff

commit 28b6552c5bc31a72beb0dfc4c80bb034d35f53ff
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Thu Apr 12 01:08:18 2018

autotest: add autotest-deps-dbus package

This allows us to compile protos that are sent over D-Bus so
we can use them in autotests.

CQ-DEPEND=CL:1000693
BUG=chromium:809616
TEST=use deps to call cryptohome MountEx method from autotest

Change-Id: I283c4731386eea991ac554f750be50733a9d1a0c
Reviewed-on: https://chromium-review.googlesource.com/1000493
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>

[modify] https://crrev.com/28b6552c5bc31a72beb0dfc4c80bb034d35f53ff/chromeos-base/autotest-tests-ownershipapi/autotest-tests-ownershipapi-9999.ebuild
[add] https://crrev.com/28b6552c5bc31a72beb0dfc4c80bb034d35f53ff/chromeos-base/autotest-deps-dbus/autotest-deps-dbus-9999.ebuild

Project Member

Comment 11 by bugdroid1@chromium.org, Apr 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/8dc40982a09862fef5d80b2ead76669a3c08827e

commit 8dc40982a09862fef5d80b2ead76669a3c08827e
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Thu Apr 12 01:08:19 2018

compile cryptohome RPC protos for autotests

This allows us to use MountEx from the cryptohome proxy, and
converts all users of the cryptohome proxy that call mount to
use it.

CQ-DEPEND=CL:1000493
BUG=chromium:809616
TEST=login_OwnershipRetaken login_RemoteOwnership
  login_UserPolicyKeys platform_CryptohomeMigrateChapsTokenClient
  login_RetrieveActiveSessions login_MultiUserPolicy
  login_MultipleSessions

Change-Id: Iccc415dc6233ddce0944fd6e8772c5c5bbf617c5
Reviewed-on: https://chromium-review.googlesource.com/1000693
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/8dc40982a09862fef5d80b2ead76669a3c08827e/client/site_tests/platform_CryptohomeMigrateChapsTokenClient/platform_CryptohomeMigrateChapsTokenClient.py
[modify] https://crrev.com/8dc40982a09862fef5d80b2ead76669a3c08827e/client/site_tests/login_OwnershipRetaken/login_OwnershipRetaken.py
[modify] https://crrev.com/8dc40982a09862fef5d80b2ead76669a3c08827e/client/site_tests/login_UserPolicyKeys/login_UserPolicyKeys.py
[modify] https://crrev.com/8dc40982a09862fef5d80b2ead76669a3c08827e/client/site_tests/login_MultipleSessions/login_MultipleSessions.py
[modify] https://crrev.com/8dc40982a09862fef5d80b2ead76669a3c08827e/client/site_tests/login_MultiUserPolicy/login_MultiUserPolicy.py
[modify] https://crrev.com/8dc40982a09862fef5d80b2ead76669a3c08827e/client/site_tests/login_RetrieveActiveSessions/login_RetrieveActiveSessions.py
[modify] https://crrev.com/8dc40982a09862fef5d80b2ead76669a3c08827e/client/cros/cryptohome.py
[modify] https://crrev.com/8dc40982a09862fef5d80b2ead76669a3c08827e/client/site_tests/login_RemoteOwnership/login_RemoteOwnership.py
[add] https://crrev.com/8dc40982a09862fef5d80b2ead76669a3c08827e/client/deps/dbus_protos/dbus_protos.py

Components: OS>Systems>Security
Is this still active?
Cc: zuan@chromium.org
John, looks like this isn't being actively worked on at the moment, but perhaps useful context for you.
Some of this is still going on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1395357 should probably reference this bug too.
Project Member

Comment 16 by bugdroid, Today (6 hours ago)

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

commit f211920105f7db3de8d5d407ef6fd0dc77638ef2
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Wed Jan 23 08:20:32 2019

cryptohome: add UnmountEx method

This is a D-Bus method that allows us to unmount asynchronously
and uses protos for easier expansion of the API in the future if
we decide to support unmounting per user.

BUG=chromium:809616
TEST=using cryptohome command line

Change-Id: I1cc2bc0364e944b85b57541faa9bafdeba989b17
Reviewed-on: https://chromium-review.googlesource.com/1401342
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/f211920105f7db3de8d5d407ef6fd0dc77638ef2/cryptohome/cryptohome.cc
[modify] https://crrev.com/f211920105f7db3de8d5d407ef6fd0dc77638ef2/cryptohome/service.h
[modify] https://crrev.com/f211920105f7db3de8d5d407ef6fd0dc77638ef2/cryptohome/interface.cc
[modify] https://crrev.com/f211920105f7db3de8d5d407ef6fd0dc77638ef2/cryptohome/interface.h
[modify] https://crrev.com/f211920105f7db3de8d5d407ef6fd0dc77638ef2/system_api/dbus/cryptohome/dbus-constants.h
[modify] https://crrev.com/f211920105f7db3de8d5d407ef6fd0dc77638ef2/cryptohome/service.cc
[modify] https://crrev.com/f211920105f7db3de8d5d407ef6fd0dc77638ef2/cryptohome/dbus_bindings/org.chromium.CryptohomeInterface.xml

Project Member

Comment 17 by bugdroid, Today (6 hours ago)

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

commit f211920105f7db3de8d5d407ef6fd0dc77638ef2
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Wed Jan 23 08:20:32 2019

cryptohome: add UnmountEx method

This is a D-Bus method that allows us to unmount asynchronously
and uses protos for easier expansion of the API in the future if
we decide to support unmounting per user.

BUG=chromium:809616
TEST=using cryptohome command line

Change-Id: I1cc2bc0364e944b85b57541faa9bafdeba989b17
Reviewed-on: https://chromium-review.googlesource.com/1401342
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/f211920105f7db3de8d5d407ef6fd0dc77638ef2/cryptohome/cryptohome.cc
[modify] https://crrev.com/f211920105f7db3de8d5d407ef6fd0dc77638ef2/cryptohome/service.h
[modify] https://crrev.com/f211920105f7db3de8d5d407ef6fd0dc77638ef2/cryptohome/interface.cc
[modify] https://crrev.com/f211920105f7db3de8d5d407ef6fd0dc77638ef2/cryptohome/interface.h
[modify] https://crrev.com/f211920105f7db3de8d5d407ef6fd0dc77638ef2/system_api/dbus/cryptohome/dbus-constants.h
[modify] https://crrev.com/f211920105f7db3de8d5d407ef6fd0dc77638ef2/cryptohome/service.cc
[modify] https://crrev.com/f211920105f7db3de8d5d407ef6fd0dc77638ef2/cryptohome/dbus_bindings/org.chromium.CryptohomeInterface.xml

Sign in to add a comment