Commit graph

4493 commits

Author SHA1 Message Date
Paul Crowley
a236ff87b9 Merge "Remove wait_for_keymaster and all references" 2021-06-17 18:18:28 +00:00
Treehugger Robot
17e1a0919d Merge "No using inside header files" 2021-06-16 09:56:52 +00:00
Paul Crowley
604abdd0cb Remove wait_for_keymaster and all references
No longer needed now init listens for property changes on a
separate thread.

Bug: 186580823
Test: Cuttlefish boots successfully
Change-Id: I7dd1f85a73df6c2160ef8778703709e90309b9b4
2021-06-15 15:54:51 -07:00
Paul Crowley
f4430387d2 No using inside header files
I thought it was OK to use "using" in a header file so long as it was
inside a "namespace" block, but it just imports symbols from one
namespace into another, so things that shouldn't work do.

Test: Treehugger
Change-Id: I4d43d35339636af7e95761cada7120b4db638c01
2021-06-15 15:20:44 -07:00
Eric Biggers
ec78a94586 Merge "Replace most references to Keymaster with Keystore" 2021-06-15 21:35:16 +00:00
Eric Biggers
d86a8abec7 Replace most references to Keymaster with Keystore
Now that vold uses Keystore2 rather than the Keymaster HAL directly, and
also the new version of Keymaster is called "KeyMint" instead, replace
most of the references to Keymaster in vold with Keystore.

(I decided not to include the "2" in most places, as it seemed
unnecessarily precise in most places, and it would be something that
might need to keep being updated.  Only Keystore.{cpp,h} really need to
care about the version number.)

I didn't rename many things in cryptfs.cpp, as that file will be going
away soon anyway.  I also left "wait_for_keymaster" and "vdc keymaster
earlyBootEnded" as-is for now, as those are referenced outside vold.

Bug: 183669495
Change-Id: I92cd648fae09f8c9769f7cf34dbf6c6e956be4e8
2021-06-15 12:07:15 -07:00
Eric Biggers
e33bd41f49 Merge "cryptfs: log beginning/end of each unmount attempt" 2021-06-14 22:00:00 +00:00
Xin Li
12e48a85fb Merge "DO NOT MERGE - Merge ab/7272582" 2021-06-14 03:02:53 +00:00
Eric Biggers
47525e6f78 cryptfs: log beginning/end of each unmount attempt
Add more log messages to make it easier to understand failures in
wait_and_unmount().

Bug: 189250652
Change-Id: I621f54f30bb01cd52c4f9a74dba2d46b4d1a8a9d
2021-06-11 17:56:27 -07:00
Xin Li
1c79e144d5 DO NOT MERGE - Merge ab/7272582
Bug: 190855093
Change-Id: I6739d9fa0fc483ed6128811f0e03c8178fed821a
2021-06-11 17:34:10 -07:00
Treehugger Robot
9891ae7479 Merge changes from topic "rename-key-dir"
* changes:
  Don't export storeKey(), and update comments
  Always use RenameKeyDir() when moving/renaming key directories
  Make RenameKeyDir() use IsSameFile()
2021-06-09 00:21:43 +00:00
Satya Tangirala
351a4af716 Don't export storeKey(), and update comments
storeKey() is no longer used outside KeyStorage.cpp, so make it a static
function.  Also fix the documentation for storeKey() (e.g. it's no
longer safe to directly move/rename directories created by storeKey() --
one must use RenameKeyDir() instead).

No functional changes.

[ebiggers@ - cleaned up slightly from satyat@'s original change]

Bug: 190398249
Change-Id: I85918359e77bef414dfddfe5ded30fcde6514013
2021-06-08 15:57:31 -07:00
Satya Tangirala
0f890a93e1 Always use RenameKeyDir() when moving/renaming key directories
Make fixate_user_ce_key() use RenameKeyDir() to rename key directories
so that any deferred commits for these directories are also updated
appropriately.

This fixes a potential lost Keymaster key upgrade if a key were to be
re-wrapped while a user data checkpoint is pending.  This isn't a huge
issue as the key will just get upgraded again, but this should be fixed.

[ebiggers@ - cleaned up slightly from satyat@'s original change]

Bug: 190398249
Change-Id: Ic6c5b4468d07ab335368e3d373916145d096af01
2021-06-08 15:57:31 -07:00
Eric Biggers
107d21d484 Make RenameKeyDir() use IsSameFile()
Comparing paths is error-prone (e.g. "/foo/bar" vs "/foo//bar"), so
entries in key_dirs_to_commit are compared using inode and device
number.  However RenameKeyDir() breaks this rule and compares raw paths.

Avoid this quirk by finding the entry in the list to replace before
doing the rename.

This doesn't fix any known problem, as vold is fairly consistent with
its paths in practice; this is just a robustness improvement.

Bug: 190398249
Change-Id: I3ce2c0119cb2012ac9d12849570e56600bc23867
2021-06-08 15:57:31 -07:00
Treehugger Robot
827dfe6e75 Merge "cryptfs: try harder to unmount subdirectory mounts" 2021-06-08 19:39:12 +00:00
Eric Biggers
8953430064 cryptfs: try harder to unmount subdirectory mounts
ensure_subdirectory_unmounted() was ignoring the return value from
umount(), so it wasn't possible to tell whether it succeeded or failed.
Make it log an error message on failure.

Also, there might be cases where ensure_subdirectory_unmounted() fails
initially but would succeed later, e.g. due to files in a subdirectory
mount being open and requiring processes to be killed.  To make this
more robust, keep calling ensure_subdirectory_unmounted() before each
attempt of umount("/data").

I'm not sure whether this will actually fix bug 189250652, as it hasn't
been root-caused yet, but this might help.

Bug: 189250652
Change-Id: I979b12d3c6a88fe3335ff548b1f8a5db43683c4f
2021-06-07 12:45:54 -07:00
Eric Biggers
67db7b9786 Merge "Remove /data/misc/vold/user_keys/ce/${user_id} when no longer needed" 2021-06-01 17:07:31 +00:00
Eric Biggers
d863b2cd4a Remove /data/misc/vold/user_keys/ce/${user_id} when no longer needed
When a user is removed, vold is deleting the subdirectories of
/data/misc/vold/user_keys/ce/${user_id} but not that directory itself.
This is unexpected, as none of the user's directories should be left
around.  Delete it too.

Bug: 188702840
Test: pm create-user foo
      pm remove-user 10
      stat /data/misc/vold/user_keys/ce/10 # no longer exists
Change-Id: Id4033a668fa6de1debb9ba6fdd1351c940bd35fc
2021-05-27 17:34:19 -07:00
Satya Tangirala
54ebfb5806 Merge "Fix bug with deferred commits for key upgrades in temporary directories" 2021-05-13 21:59:41 +00:00
Satya Tangirala
9475b11a1e Fix bug with deferred commits for key upgrades in temporary directories
storeKeyAtomically() stores keys in a temp directory before renaming
that directory to the real target directory. However when the key is
stored in the temporary directory, the Keymaster storage key might get
upgraded, and it's possible that the temp directory is scheduled for a
deferred commit. storeKeyAtomically() renames that temp directory, but
doesn't update the list of directories marked for deferred commit.

This patch fixes this by removing the temp directory from the list and
adding the real target directory to that list instead.

This bug was found when trying to switch from using the guest keymint to
using the host remote keymint implementation on cuttlefish
(aosp/1701925).  The device triggers this bug (and boots to recovery)
when aosp/1701925 is cherry-picked.

Co-Developed-By: Eric Biggers <ebiggers@google.com>
Test: Cuttlefish boots with and without aosp/1701925
Change-Id: I3b6fd6ad32ed415da94423cca6f5a121c16472f2
2021-05-13 11:21:23 -07:00
Satya Tangirala
98692ab9bb Merge changes from topic "vold-keystore2-fixes"
* changes:
  Remove unused constants and cleanup KeyStorage.cpp
  Remove unused parameter "salt" from stretchSecret()
  Use AServiceManager_waitForService() to connect to keystore2
2021-05-12 22:32:32 +00:00
Satya Tangirala
6b98fb6122 Remove unused constants and cleanup KeyStorage.cpp
Now that the salt and hardware auth token related code has been removed,
we can remove the associated (and now unused) constants.

Also cleanup some comments and remove includes related to hardware auth
token support.

Bug: 181910578
Test: Cuttlefish boots.
Change-Id: I3733d5c6bbf6989adc165c554ee53faa2484f4b6
2021-05-12 13:05:35 -07:00
Satya Tangirala
478cea9783 Remove unused parameter "salt" from stretchSecret()
stretchSecret() no longer uses the "salt" parameter, so remove it and
simplify callers

Bug: 181910578
Test: Cuttlefish boots.
Change-Id: Ic2d0742b22b98a66da37f435e274c9d385b8e188
2021-05-12 13:05:35 -07:00
Satya Tangirala
6ef4e37351 Use AServiceManager_waitForService() to connect to keystore2
Vold currently uses AServiceManager_getService() to connect to
keystore2, which has an internal timeout of 5s. Since a lot of vold
keystore2 connection failures are fatal, we instead use
AServiceManager_waitForService(), which will wait efficiently for
keystore2 to start, instead of timing out after 5s.

Bug: 185934601
Test: Cuttlefish boots.
Change-Id: Ib4e977a997e020082382e0686f448d1aa72834ec
2021-05-11 19:30:30 -07:00
Treehugger Robot
93dd933d85 Merge "Show names of processes killed by KillProcessesWithOpenFiles()" 2021-05-11 20:24:49 +00:00
Eric Biggers
297b23837e Merge "cryptfs: kill processes more quickly in wait_and_unmount()" 2021-05-11 20:00:14 +00:00
Eric Biggers
b4faeb8d44 cryptfs: kill processes more quickly in wait_and_unmount()
In wait_and_unmount(), kill the processes with open files after umount()
has been failing for 2 seconds rather than 17 seconds.  This avoids a
long boot delay on devices that use FDE.

Detailed explanation:

On FDE devices, vold needs to unmount the tmpfs /data in order to mount
the real, decrypted /data.  On first boot, it also needs to unmount the
unencrypted /data in order to encrypt it in-place.

/data can't be unmounted if files are open inside it.  In theory, init
is responsible for killing all processes with open files in /data, via
the property trigger "vold.decrypt=trigger_shutdown_framework".

However, years ago, commit 6e8440fd50 ("cryptfs: kill processes with
open files on tmpfs /data") added a fallback where vold kills the
processes itself.  Since then, in practice people have increasingly been
relying on this fallback, as services keep being added that use /data
but don't get stopped by trigger_shutdown_framework.

This is slowing down boot, as vold sleeps for 17 seconds before it
actually kills the processes.

The problematic services include services that are now started
explicitly in the post-fs-data trigger rather than implicitly as part of
a class (e.g., tombstoned), as well as services that now need to be
started as part of one of the early-boot classes like core or early_hal
but can still open files in /data later (e.g. keystore2 and credstore).

Another complication is that on default-encrypted devices (devices with
no PIN/pattern/password), trigger_shutdown_framework isn't run at all,
but rather it's expected that the relevant services simply weren't
started yet.  This means that we can't fix the problem just by fixing
trigger_shutdown_framework to kill all the needed processes.

Therefore, given that the vold fallback is being relied on in practice,
and FDE won't be supported much longer anyway (so simple fixes are very
much preferable here), let's just change wait_and_unmount() in vold to
use more appropriate timeouts.  Instead of waiting for 17 seconds before
killing processes, just wait for 2 seconds.  Keep the total timeout of
20 seconds, but spend most of it retrying killing the processes, and
only if the unmount is still failing.

This avoids the long boot delays in practice.

Bug: 187231646
Bug: 186165644
Test: Tested FDE on Cuttlefish, and checked logcat to verify that the
      boot delay is gone.
Change-Id: Id06a9615a87988c8336396c49ee914b35f8d585b
2021-05-10 20:44:07 -07:00
Eric Biggers
c78ae60087 Show names of processes killed by KillProcessesWithOpenFiles()
Otherwise only the pids are shown, and it's hard to tell which
processes actually got killed.

Bug: 187231646
Change-Id: Icccf60d0ad4439d702f36ace31abe092df1c69c2
2021-05-10 17:34:11 +00:00
Xin Li
ef439c5367 Merge "DO NOT MERGE - Mark RQ2A.210105.001 as merged." 2021-05-08 01:28:13 +00:00
Xin Li
140116266e DO NOT MERGE - Mark RQ2A.210105.001 as merged.
Bug: 180401296
Merged-In: Ic37985f98e6cbfe4fa38b981d3332c4dfc40c5b8
Change-Id: Ic82b58f8975ae7b5410d87536342f83e827a7893
2021-05-07 14:32:31 -07:00
rickywai
ae11ab712f Merge "Always unmount data and obb directory that mounted" 2021-05-06 08:09:05 +00:00
Ricky Wai
5f2a9fee66 Always unmount data and obb directory that mounted
Otherwise, when system removes user's volume, it will hang
as there are mounts (obb and data mounts) still remain mounted in system.

Bug: 187122943
Test: atest UserLifecycleTests#managedProfileUnlock_stopped, it's not blocked anymore

Change-Id: Ic37985f98e6cbfe4fa38b981d3332c4dfc40c5b8
2021-05-05 14:44:16 +00:00
Alan Stokes
b2678b6654 Merge "Only kill apps with storage app data isolation enabled" 2021-05-05 14:16:00 +00:00
Eric Biggers
7505efbd5d Merge "Log error message if setting project quota ID fails" 2021-05-04 16:06:14 +00:00
Eric Biggers
39aa9584b1 Log error message if setting project quota ID fails
Otherwise, the only sign of what went wrong may be system_server
logging a "ServiceSpecificException".

Bug: 187079978
Change-Id: I59b2ba2b0e679dfd1ec1fd8fff6790256fbfdf29
2021-05-03 12:39:36 -07:00
Ricky Wai
23356377ae Only kill apps with storage app data isolation enabled
Originally it kills all the apps with obb and data mounted.
Due to recent changes, all apps will have obb and data dirs mounted
in default root namespace. Hence all apps will be killed by
by KillProcessesWithMounts().

To fix this, we also check if the dir is mounted as tmpfs,
as the default namespace one is bind mounted to lowerfs,
which app data isolation is mounted as tmpfs, so we only
kill the process that have obb dir mounted as tmpfs.

Bug: 148049767
Test: Able to boot without warnings
Change-Id: I5f862ad6f64f5df739b68ea7c9815352bae3be5c
Merged-In: I45d9a63ed47cbc27aebb63357a43f51ad62275db
2021-04-30 13:58:07 +00:00
Treehugger Robot
d2bb367549 Merge "Fix cryptfs RSA signing with keystore2" 2021-04-26 18:51:13 +00:00
Hasini Gunasinghe
68bdb45cf8 Merge "Make vold use the updated keystore 2 API for storage keys." 2021-04-23 22:39:04 +00:00
Eric Biggers
940c0e5f6e Fix cryptfs RSA signing with keystore2
Fix KeymasterOperation::updateCompletely() to not treat an empty output
as an error, since for RSA signing (used by cryptfs / FDE) it is
expected that the output from update() be empty.  The output is instead
produced at the end by finish().

This is one of a set of changes that is needed to get FDE working again
so that devices that launched with FDE can be upgraded to Android 12.

Bug: 186165644
Change-Id: Icf120f8b9526d051d0ebe16bc8ad1edf712241e1
2021-04-23 10:44:41 -07:00
Jaegeuk Kim
177b9db866 Merge "mkfs_f2fs: give the log in kernel" 2021-04-20 22:08:26 +00:00
Janis Danisevskis
3915b08f80 Make vold use the updated keystore 2 API for storage keys.
This CL updates vold to use the updated storage key API that provides an
optional upgraded key blob. In this patch the upgraded key blob is not
yet stored by vold.

Bug: 185811713
Test: N/A
Change-Id: I39eeb20df0eb2b023479f3adebab264d29d00048
2021-04-20 12:53:12 -07:00
Jaegeuk Kim
2c1380f1ab mkfs_f2fs: give the log in kernel
It's very useful to see the mkfs log in console to debug any issues.

Bug: 172378121
Signed-off-by: Jaegeuk Kim <jaegeuk@google.com>
Change-Id: Icdac3609860cf0bba3fa758cead885bd4960f2c0
2021-04-15 20:24:04 -07:00
Treehugger Robot
5e5819a761 Merge "vold: add getUnlockedUsers() method to Binder interface" 2021-04-15 02:33:22 +00:00
Eric Biggers
18ba15223c vold: add getUnlockedUsers() method to Binder interface
This is needed so that system_server can remind itself about which users
have their storage unlocked, if system_server is restarted due to a
userspace reboot (soft restart).

Bug: 146206679
Test: see I482ed8017f7bbc8f7d4fd5a2c0f58629317ce4ed
Change-Id: I02f0494d827094bd41bcfe5f63c24e204b728595
(cherry picked from commit 1799debfd6)
2021-04-13 10:53:00 -07:00
Satya Tangirala
10912a295f Merge changes from topic "vold-use-keystore2" am: 08873d0d7d am: 54460f0635
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1649730

Change-Id: I8a70c04881275aa5e3bf4cf629316870798df27a
2021-04-08 01:36:42 +00:00
Satya Tangirala
b79360f80c Make vold use keystore2 instead of keymaster am: e8de4ffd73 am: 7a8ac746a2
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1640885

Change-Id: I7a45fdb9ed25c5543d0a9dda80106241f90e53db
2021-04-08 01:36:41 +00:00
Satya Tangirala
57e480b3d5 Remove HardwareAuthToken support from vold::Keymaster am: e13617100d am: 695fadddf3
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1640884

Change-Id: I84747f3ea29f6b78f8f1a9bb11959a46ec8c3189
2021-04-08 01:36:40 +00:00
Satya Tangirala
54460f0635 Merge changes from topic "vold-use-keystore2" am: 08873d0d7d
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1649730

Change-Id: Ie7db671fc7e90fa86cf84773786ea6afaab37a7f
2021-04-08 00:52:41 +00:00
Satya Tangirala
7a8ac746a2 Make vold use keystore2 instead of keymaster am: e8de4ffd73
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1640885

Change-Id: I0a9b288902f5bb0f27d524dcf509ce461e4495fe
2021-04-08 00:52:40 +00:00
Satya Tangirala
695fadddf3 Remove HardwareAuthToken support from vold::Keymaster am: e13617100d
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1640884

Change-Id: Id8528a10d976e94e8bdb4e308d91107b1afdced6
2021-04-08 00:52:35 +00:00