New issue
Advanced search Search tips

Issue 706307 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Apr 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 688900
issue 708015



Sign in to add a comment

eCryptfs -> ext4 : Add cryptohome method for checking the necessity of migration

Project Member Reported by kinaba@chromium.org, Mar 29 2017

Issue description

Request from fukino@.

Currently cryptohomed is returning the status as an error return code of MountEx.

However, according to fukino@, it turned out to be useful for implementing the migration wizard UI if there were a separate method for checking it, especially for testing the PREVIOUS_MIGRATION_INCOMPLETE status.
 

Comment 1 by kinaba@chromium.org, Mar 29 2017

Labels: -Pri-2 Pri-3
Did one more offline discussion and this might be P3 (we can live without it.)
Blocking: 708015
Brain-dumping to increase the chance of work offloading:

This can be implemented by adding a Dbus method to cryptohomed, that just performs a check similar to this:
https://chromium-review.googlesource.com/#/c/458317/5/cryptohome/mount.cc

MOUNT_ERROR_PREVIOUS_MIGRATION_INCOMPLETE or MOUNT_ERROR_OLD_ENCRYPTION case needs migration
Otherwise the account does not need migration.


However, if we want to use this method for checking the status before user enters credentials,
like the use case in Bug 708015, we have to move to a more rough check without using credentials.

Since DoesEcryptfsCryptohomeExist() is that checking the existence of a directory..., simple

"return DoesEcryptfsCryptohomeExist() ? NEED_MIGRATION : NO_NEED_FOR_MIGRATION"

may be a good alternative for this case.
Or if Bug 708015 is the only concern,
just checking the existence of '/home/.shadow/$hash/vault' purely on Chrome can also correctly identify the necessity of migration.
(Though it sounds somewhat like breaking an abstraction barrier a bit and should be limited to a very short-term solution.)
We have to do the detection via cryptohomed. Chrome does not have access to "/home/.shadow" so #4 would not work.
Cc: kinaba@chromium.org
Owner: xiy...@chromium.org
I would take this with issue 708015
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 12 2017

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

commit 877c3882607d6941c2bf64b4a8bec97ca98eb891
Author: Xiyuan Xia <xiyuan@google.com>
Date: Wed Apr 12 23:07:50 2017

cryptohome: Add a NeedsDircryptoMigration method

- Add a NeedsDircryptoMigration to D-Bus interface and hook it
  with Service::NeedsDircryptoMigration;
- Add actual check in HomeDirs::NeedsDircryptoMigration by using
  the existence of eCryptfs vault dir as a signal;

BUG= chromium:706307 
TEST=Manual. Use cryptohome --action=needs_dircrypto_migration
    --user=<user_id> to test on device.

Change-Id: I47324732b1ebb5efe551a05b7e5f6ae010179099
Reviewed-on: https://chromium-review.googlesource.com/473986
Commit-Ready: Xiyuan Xia <xiyuan@chromium.org>
Tested-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>

[modify] https://crrev.com/877c3882607d6941c2bf64b4a8bec97ca98eb891/cryptohome/service.cc
[modify] https://crrev.com/877c3882607d6941c2bf64b4a8bec97ca98eb891/cryptohome/homedirs.h
[modify] https://crrev.com/877c3882607d6941c2bf64b4a8bec97ca98eb891/cryptohome/etc/Cryptohome.conf
[modify] https://crrev.com/877c3882607d6941c2bf64b4a8bec97ca98eb891/cryptohome/cryptohome.cc
[modify] https://crrev.com/877c3882607d6941c2bf64b4a8bec97ca98eb891/cryptohome/service.h
[modify] https://crrev.com/877c3882607d6941c2bf64b4a8bec97ca98eb891/cryptohome/interface.cc
[modify] https://crrev.com/877c3882607d6941c2bf64b4a8bec97ca98eb891/cryptohome/interface.h
[modify] https://crrev.com/877c3882607d6941c2bf64b4a8bec97ca98eb891/cryptohome/cryptohome.xml
[modify] https://crrev.com/877c3882607d6941c2bf64b4a8bec97ca98eb891/cryptohome/homedirs.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 12 2017

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

commit 19115ef1bc72e0068951d236cdc0c549346adfda
Author: Xiyuan Xia <xiyuan@google.com>
Date: Wed Apr 12 23:07:43 2017

system-api: Add cryptohome NeedsDircryptoMigration name

BUG= chromium:706307 
TEST=None.

Change-Id: I75f45bab9de5b56d4a0991c3f8c48fcc5ee9f6fc
Reviewed-on: https://chromium-review.googlesource.com/473528
Commit-Ready: Xiyuan Xia <xiyuan@chromium.org>
Tested-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Kazuhiro Inaba <kinaba@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>

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

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 12 2017

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

commit 19115ef1bc72e0068951d236cdc0c549346adfda
Author: Xiyuan Xia <xiyuan@google.com>
Date: Wed Apr 12 23:07:43 2017

system-api: Add cryptohome NeedsDircryptoMigration name

BUG= chromium:706307 
TEST=None.

Change-Id: I75f45bab9de5b56d4a0991c3f8c48fcc5ee9f6fc
Reviewed-on: https://chromium-review.googlesource.com/473528
Commit-Ready: Xiyuan Xia <xiyuan@chromium.org>
Tested-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Kazuhiro Inaba <kinaba@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>

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

Project Member

Comment 10 by bugdroid1@chromium.org, Apr 13 2017

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

commit 03323c74be50811df1ac0056087f19c7cf69a832
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Thu Apr 13 01:57:17 2017

Roll cros_system_api b78cb488..19115ef1

https://chromium.googlesource.com/chromiumos/platform/system_api.git/+log/b78cb488..19115ef1

$ git log b78cb488..19115ef1 --date=short --no-merges --format='%ad %ae %s'
2017-04-10 xiyuan@google.com system-api: Add cryptohome NeedsDircryptoMigration name
2017-03-27 ejcaruso@chromium.org system_api: remove unused debugd D-Bus method name

BUG= 706307 
TEST=Manual.

Change-Id: I51afd2afc614a0bc5d754a48b3f1e6c6352fccf1
Reviewed-on: https://chromium-review.googlesource.com/476094
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#464241}
[modify] https://crrev.com/03323c74be50811df1ac0056087f19c7cf69a832/DEPS

Status: Fixed (was: Assigned)
For testers like command line:

# cryptohome --action=needs_dircrypto_migration --user=<test_user_id, e.g. test@gmail.com>

It should output "Yes" or "No" to reflect whether the user needs to do dircrypto migration.

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

Status: Archived (was: Fixed)

Sign in to add a comment