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

Issue 729905 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Jun 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 703492
issue 730966



Sign in to add a comment

cryptohome : replace the use of AsyncMountPublic by MountEx

Project Member Reported by kinaba@chromium.org, Jun 6 2017

Issue description

This is a preparation step for supporting encryption migration for ARC Kiosk ( crbug.com/703492 ).

Cryptohomed's MountEx() method already implements some necessary features for encryption migration,
while Kiosk mode mounting uses yet another method called AsyncMountPublic().


If I understand correctly, the AsyncMount family is a legacy way to mount cryptohome and is to be deprecated:

https://chromium.googlesource.com/chromiumos/platform2/+/1b1e65cfe23ea71e0f24cdafead7dabeea5252f9/cryptohome/service.cc#2004
// This function implements the _old_ style Mounts.  It should be removed
// once MountEx is used everywhere.

Let's makes this happen and start using MountEx (rather than to backport the code for encryption migration from MountEx to the legacy AsyncMount.)

 
Sounds great!
There more than 3000 lines in cryptohome/service.cc and many of them are duplicates.
Removing one member of the Mount family should make the code easier to maintain.
Progress update:

WIP CL (the remaining task is to add tests) is here.
https://chromium-review.googlesource.com/#/q/topic:MountExPublic
Cc: fukino@chromium.org
And for Chrome side:
https://chromium-review.googlesource.com/#/c/527754/

+fukino@: #2 and the above CL together should be enough for switching the Kiosk mounting to
return OLD_ENCRYPTION errors when it detected ecryptfs.
Blocking: 730966
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 22 2017

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

commit e79b0c771217d6b84b85c692ae5740df8e8a2b36
Author: Kazuhiro Inaba <kinaba@chromium.org>
Date: Thu Jun 22 10:34:49 2017

cryptohome: Protobuf change to support public mount in MountEx.

BUG= chromium:729905 
TEST=None

Change-Id: I7ef3efdfbdfd07e1055b1a026034aa16ae47e662
Reviewed-on: https://chromium-review.googlesource.com/526839
Commit-Ready: Kazuhiro Inaba <kinaba@chromium.org>
Tested-by: Kazuhiro Inaba <kinaba@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>

[modify] https://crrev.com/e79b0c771217d6b84b85c692ae5740df8e8a2b36/dbus/cryptohome/rpc.proto

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 23 2017

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

commit 2e0c3f8760c89523c7a513258cd519155c21a523
Author: Kazuhiro Inaba <kinaba@chromium.org>
Date: Fri Jun 23 05:42:39 2017

cros_system_api: DEPS rolls for supporting public mount in MountEx.

It will be used from:
https://chromium-review.googlesource.com/c/527016/

BUG= 729905 
TEST=it builds.

Change-Id: I7344ed4f86705a7f46b7f6b5791f340fabb94157
Reviewed-on: https://chromium-review.googlesource.com/544453
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Commit-Queue: Kazuhiro Inaba <kinaba@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481813}
[modify] https://crrev.com/2e0c3f8760c89523c7a513258cd519155c21a523/DEPS

Comment 7 by kinaba@chromium.org, Jun 23 2017

2 out of 4 changes has landed. 2 more is needed.
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 24 2017

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

commit d03de8e4b1ab3eab2bfbb924342b02f176b50b08
Author: Kazuhiro Inaba <kinaba@chromium.org>
Date: Sat Jun 24 05:56:47 2017

cryptohome: Support public mount in MountEx.

This is meant to replace and deprecate the legacy AsyncMountPublic
method. The expectation is that MountEx with public_mount=true is
behaving as equal as AsyncMountPublic.

BUG= chromium:729905 
TEST=cros_workon_make --board=reef cryptohome --test
TEST=Manually confirmed ARC Kiosk sign-in, with exiting ARC Kiosk
 user cryptohome and without it (fresh cryptohome creation), with
 and without --direncryption enabled.
CQ-DEPEND=CL:526839

Change-Id: I5842dc738fcc6fabaa81c412d4ea7f390ff2801a
Reviewed-on: https://chromium-review.googlesource.com/527016
Commit-Ready: Kazuhiro Inaba <kinaba@chromium.org>
Tested-by: Kazuhiro Inaba <kinaba@chromium.org>
Reviewed-by: Kazuhiro Inaba <kinaba@chromium.org>

[modify] https://crrev.com/d03de8e4b1ab3eab2bfbb924342b02f176b50b08/cryptohome/service.cc
[modify] https://crrev.com/d03de8e4b1ab3eab2bfbb924342b02f176b50b08/cryptohome/service_unittest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 28 2017

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

commit 11b33cca76d28e36634564175d023af0474d8ae0
Author: Kazuhiro Inaba <kinaba@chromium.org>
Date: Wed Jun 28 00:53:34 2017

cryptohome: Switch the use of AsyncMountPublic() to MountEx().

The former has been used for Kiosk sing-in, but it is legacy and
doesn't support recent enhancement in MountEx() to support the
filesystem encryption migration.

As a preparation to support migration for ARK Kiosk user,
this CL first converts the uses of AsyncMountPublic to MountEx.

This change depends on the following ChromiumOS-side changes:
https://chromium-review.googlesource.com/#/q/topic:MountExPublic

BUG= 729905 
TEST=Manually confirmed ARC Kiosk sign-in, with exiting ARC Kiosk
 user cryptohome and without it (fresh cryptohome creation), with
 and without --direncryption enabled.

Change-Id: I9e02482eef3299ed497ac5e408cc0d4b3a08a951
Reviewed-on: https://chromium-review.googlesource.com/527754
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Darren Krahn <dkrahn@chromium.org>
Commit-Queue: Kazuhiro Inaba <kinaba@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482828}
[modify] https://crrev.com/11b33cca76d28e36634564175d023af0474d8ae0/chromeos/cryptohome/cryptohome_parameters.h
[modify] https://crrev.com/11b33cca76d28e36634564175d023af0474d8ae0/chromeos/cryptohome/homedir_methods.cc
[modify] https://crrev.com/11b33cca76d28e36634564175d023af0474d8ae0/chromeos/login/auth/cryptohome_authenticator.cc

Status: Fixed (was: Started)

Comment 11 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment