Commit graph

1124 commits

Author SHA1 Message Date
Karuna Wadhera
77eb25fc04 Merge "Query for /default and /strongbox IRPCs directly" into main 2024-06-10 17:46:06 +00:00
Karuna Wadhera
9ae66c0bef Query for /default and /strongbox IRPCs directly
This removes the need to query for and call find on all IRPCs.

Bug: 312427637
Test: atest -p --include-subdirs system/security/keystore2
Change-Id: I8893627e983eac577b9920497f926f12c0e98271
2024-06-10 15:10:40 +00:00
David Drysdale
5238d77a83 Simplify map_or_log_err
All (non-test) invocations use the same second argument `Ok` (which
actually acts as a 1-arg closure `|v| Ok(v)`), so no need to have it as
a parameter.  The common second argument just maps `Ok(v)` to `Ok(v)`,
which means that `map_err()` can be used instead of `map_or_else()`,
and the same type parameter is used for both input and output.

Test: legacykeystore_test
Test: keystore2_test
Flag: None, pure refactor
Change-Id: I46b6e327d0b546df6be6664781a52bb888c04eca
2024-06-07 16:34:53 +00:00
David Drysdale
7b9ca23b18 Add debugging info for transactions
Pass around information about which code is performing an exclusive
database transaction, and run an additional watchdog inside the
transaction closure.

Bug: 319563050
Test: CtsKeystoreTestCases
Change-Id: Ib54f1f4c0c37f9d7392d21d9ccb880d066029945
2024-05-24 15:42:24 +01:00
David Drysdale
541846b93c Add/use watchdog with standard timeout
Almost all uses of the watchdog use the same 500ms timeout, so add a new
method that assumes that.

Test: CtsKeystoreTestCases
Change-Id: Idf7852400a58ba954e4a71e5e2282734a0960072
2024-05-23 13:23:22 +01:00
David Drysdale
8c4c4f3420 Remove unused code
Test: TreeHugger
Change-Id: If7a4606f5a6a09c574574ed8ed04788435259fa8
2024-05-23 12:58:15 +01:00
Max Bires
ef518cbe49 Merge "Revert "Deprecating the aidl for Android Protected Confirmation"" into main 2024-05-23 03:32:38 +00:00
Max Bires
9535b1b443 Revert "Deprecating the aidl for Android Protected Confirmation"
Revert submission 2864688-apc-deprecate

Reason for revert: fix inadvertent partial deprecation

Reverted changes: /q/submissionid:2864688-apc-deprecate

Change-Id: Id97e7ec533b630a22ea91db82ab75f14b0d32edc
2024-05-22 05:13:36 +00:00
David Drysdale
115c4722f8 Give up on busy DB after a while
Calls to `with_transaction(Immediate, ...)` act as an exclusive lock on
the Keystore database, because the sleep-loop does not release the
transaction.  That gives the potential for deadlock if any of the code
in the invoked callback takes some other lock without consideration
for lock inversions.

There isn't (yet) a smoking gun that definitively identifies a lock
inversion, but this CL adds timeout behaviour just in case.

Include a unit test that deadlocks without the code change, because
of an explicit lock inversion between the immediate-mode database and a
`KeyIdGuard` object (which acts like a `MutexGuard`).

Bug: 319563050
Bug: 315165314
Flag: android.security.keystore2.database_loop_timeout
Test: keystore2_test#database::tests::test_key_id_guard_immediate
Change-Id: I34fa044ce8e3185a89084b84c6f9ac880944982c
2024-05-20 19:07:03 +01:00
Treehugger Robot
134da755b0 Merge "Clean up OWNERS" into main 2024-05-14 02:13:28 +00:00
Max Bires
a78e7c5727 Clean up OWNERS
Remove OWNERS entry for person no longer working on the project.

Test: N/A
Change-Id: Icbfba05512e1e1c85593ed58ef88ff15b6673ecc
2024-05-13 19:08:39 -07:00
David Drysdale
40e41f1088 Adjust keystore2_client_tests
Adjust the keystore2 client tests to cope with a wider variety of
underlying KeyMint / Keymaster devices.

A couple of these changes involve test modifications to match the
behaviour of the KeyMint VTS tests:

- `keystore2_gen_key_device_unique_attest_with_default_sec_level_unimplemented`:
  Allow an extra error code, to match
  `DeviceUniqueAttestationTest.EcdsaNonStrongBoxUnimplemented`.
- `keystore2_import_ec_key_success`: Skip the check that EC keys can be
  imported without an explicitly specified `EC_CURVE` on pre-VSR-V
  devices, to match the equivalent logic in the VTS tests
  (`ImportKeyTest.EcdsaSuccessCurveNotSpecified`).

The other two changes are:

- `keystore2_gen_key_auth_boot_loader_only_op_fail`: Drop this test, as
  it's the first/only place that exercises the optional
  `BOOTLOADER_ONLY` tag. (The KeyMint VTS tests would be the best place
  to exercise this for the first time.)
- `keystore2_ec_25519_generate_key_fail`: For now, skip the check that
  an Ed25519 key should reject use of any digest value other than `NONE`
  (on account of Ed25519 having its own internal digest). That behaviour
  isn't quite right, but which is not currently tested by the KeyMint
  VTS tests and so we can't require existing devices to be modified to
  pass the check.

Bug: 336695416
Test: keystore2_client_tests
Change-Id: I06e90c859f33d8b4125541a67709ec67e8898c60
2024-05-01 18:16:46 +01:00
David Drysdale
98e175b6f8 Merge "Drop unused PerBootDbKeepAlive type" into main 2024-04-15 05:35:59 +00:00
David Drysdale
e2b37b6c62 Drop unused PerBootDbKeepAlive type
Looks like the DB was moved out of SQLite in aosp/1699645 but the
connection-holding type was missed.

Bug: 333894699
Test: TreeHugger
Change-Id: I4b87690343bc890cb1aa1c6e1595ac4b42c11670
2024-04-12 14:38:28 +00:00
Frederick Mayle
e5b6854fb8 Merge "Upgrade nix to 0.28.0" into main 2024-04-11 15:44:35 +00:00
David Drysdale
b0914ea70a Cope with rkp-only devices in legacy keyblob test
On a rkp-only device it is not possible to generate an attestation
on a bare key generation (attestations can only be generated if an
ATTEST_KEY is provided).

Bug: 329409739
Test: keystore2_legacy_blobs_test
Change-Id: Icdc4037466ab32366c34feeb42b58822ea110ebf
2024-04-10 12:58:23 +01:00
Frederick Mayle
42632079aa Upgrade nix to 0.28.0
Bug: 333427576
Test: TreeHugger
Change-Id: I549be37c37e99b4a73da2a12758675ad3191716b
2024-04-09 16:05:43 -07:00
Charisee
4339115a79 Update needed for Rust v1.77.1
error: initializer for `thread_local` value can be made `const`
    --> system/security/keystore2/src/database.rs:5022:47
     |
5022 |         static RANDOM_COUNTER: RefCell<i64> = RefCell::new(0);
     |                                               ^^^^^^^^^^^^^^^ help: replace with: `const { RefCell::new(0) }`
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#thread_local_initializer_can_be_made_const
     = note: `-D clippy::thread-local-initializer-can-be-made-const` implied by `-D warnings`
     = help: to override `-D warnings` add `#[allow(clippy::thread_local_initializer_can_be_made_const)]`

error: aborting due to 1 previous error


Bug: http://b/330185853
Test: ./test_compiler.py --prebuilt-path dist/rust-dev.tar.xz  --target aosp_cf_x86_64_phone --image
Change-Id: Ic583a76f7ea7fc27ce6c214b3247748d7dbaa1b4
2024-04-02 16:18:23 +00:00
Charisee
95ea3ceef5 Update needed for Rust v1.77.0
error: field `0` is never read
   --> system/security/keystore2/src/database.rs:848:31
    |
848 | pub struct PerBootDbKeepAlive(Connection);
    |            ------------------ ^^^^^^^^^^
    |            |
    |            field in this struct
    |
    = note: `-D dead-code` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(dead_code)]`
help: consider changing the field to be of unit type to suppress this warning wh
ile preserving the field numbering, or remove the field
    |
848 | pub struct PerBootDbKeepAlive(());
    |                               ~~

error: aborting due to 1 previous error


Bug: 330185853
 ./test_compiler.py --prebuilt-path dist/rust-dev.tar.xz  --target aosp_cf_x86_64_phone --image

Test: m rust
Change-Id: I4cb81c955372c4f8f50b940b389f2d7cfc5d3ce9
2024-03-27 23:46:39 +00:00
Eric Biggers
0361bbf664 Remove obsolete TODO from IKeystoreAuthorization.aidl
IKeystoreAuthorization already has @SensitiveData.

Bug: 176110256
Test: N/A
Change-Id: Ia59bb2d9e2154c51b5ab7568a321359ff096c59f
2024-03-22 21:03:11 +00:00
Eric Biggers
b5613dae22 Remove broken and unused support for expiring keys when off-body
Remove IKeystoreMaintenance#onDeviceOffBody(), as it's no longer called.

In addition, remove the code that tried to enforce the AllowWhileOnBody
key parameter.  This code was broken during the rewrite of Keystore in
Android 12, and as a result, AllowWhileOnBody has no user-visible
effect.  AllowWhileOnBody is *supposed* to cause the key's
authentication timeout, if it has one, to automatically expire when the
device is removed from the user's body.  (A better name for it might
have been something like UserAuthenticationExpiresWhenRemovedFromBody.)
Android 11 Keystore implemented this behavior; see
https://android.googlesource.com/platform/system/security/+/refs/heads/android11-release/keystore/auth_token_table.cpp#165

Android 12 Keystore changed AllowWhileOnBody to have no effect.
Apparently due to a misunderstanding, the (incorrect) behavior that was
attempted to be implemented was "The key may be used after
authentication timeout if device is still on-body".  But what was
actually implemented was that the Keystore daemon stopped enforcing
authentication timeouts for AllowWhileOnBody keys entirely, except after
a wearable device was removed from the body in which case the timeout is
enforced for any earlier authentications.  Yet, this has no user-visible
effect because KeyMint still enforces the authentication timeout as
usual.  So, AllowWhileOnBody has really been a no-op since Android 12.

We can always bring this code back, fixed and with tests, if this
feature comes back.  But for now there is no reason to keep it around.

Bug: 289849354
Test: atest -p --include-subdirs system/security/keystore2
Test: atest CtsKeystoreTestCases
Change-Id: I4a7b3a90b56dacbb5316e30a30bf3fabc0debe48
2024-03-14 17:43:49 +00:00
Markus Vill
fdf431762f Migrate structured logging for audit logging to the Rust macro.
This uses the new macro for structured logging that simplifies the usage
of structured logging.

Bug: 290589708

Test: Run keystore client and checked the log
Change-Id: I4d941d8b03c09d0541cf1159c38f4eba60e07292
2024-03-07 16:56:22 +00:00
Treehugger Robot
90eadc655f Merge "Added not_multi_abi configuration for keystore2_client_tests module." into main 2024-03-06 20:37:33 +00:00
Shaquille Johnson
b484dc1ce5 Merge "Update authorization log to be more clear" into main 2024-03-06 14:42:56 +00:00
Shaquille Johnson
dae62efa2c Merge "Update globals to log security levels on fail" into main 2024-03-06 14:40:26 +00:00
Eran Messeri
15a04c4d1f Merge "Updated the logic to determine the VSR API level for device ID attestation tests." into main 2024-03-06 11:54:17 +00:00
Rajesh Nyamagoud
a8cf68ef41 Added not_multi_abi configuration for keystore2_client_tests module.
Changes made to avoid running keystore2_client_tests of armeabi-v7a
builds on arm64-v8a platforms.

Bug: 322112515
Test: run vts -m keystore2_client_tests
Change-Id: Ic7205ecc80146cdd36b1a618c9c5cde114b98e71
2024-03-05 18:37:12 +00:00
Shaquille Johnson
a4d10dbee0 Update authorization log to be more clear
Based on examinations in go/keystore-error-logs-overhaul
we want to update the logs to allow keystore errors
to be properly routed.

Test: atest keystore2_test
Change-Id: I704ca5bdeaf32acdd6a619ca778b04b3df72bcfd
2024-03-05 12:39:58 +00:00
Rajesh Nyamagoud
3f6c8a250d Updated the logic to determine the VSR API level for device ID
attestation tests.

The following order of precedence is used to determine the VSR API level:
1. If the `ro.vendor.api_level` property is present, then use it as the
   VSR API level.
2. Otherwise, determine the VSR API level with the following logic:
  - Get the vendor API level using the `ro.board.api_level` property if
    present; otherwise, use the `ro.board.first_api_level` property.
  - Get the product API level using the `ro.product.first_api_level`
    property if present; otherwise, use the `ro.build.version.sdk`
    property.
  - If it is unable to determine the vendor API level, then use the
    product API level as the VSR API level.
  - If both the vendor API level and product API level are available,
    then use the minimum of `vendor_api_level` and `product_api_level`
    as the VSR API level.
  - Otherwise, the vendor API level will be used as the VSR API level.

Bug: 326675646
Test: atest keystore2_client_tests
Change-Id: I3aa48d05f367fafab5151fa7eb6dd447840dae0d
2024-02-29 19:02:11 +00:00
Shaquille Johnson
a83982159f Merge "Change the log level from error to warn" into main 2024-02-29 14:26:44 +00:00
Shaquille Johnson
69c92a092e Update globals to log security levels on fail
Per go/keystore-error-logs-overhaul we are updating
the logs in keystore to be more clear. This adds
logging around the security level when the Hardware
type error is sent to the caller.

Test: atest keystore2_test
Change-Id: I7d41f02832a02976b3e1b6535ba0df5ed3863e53
2024-02-28 22:14:05 +00:00
James Farrell
efe1a2fb73 Fix style warnings for rustc 1.76.0
Test: Built with test_compiler.py
Bug: 327204642
Change-Id: I95f8965cb1db564fb3c86b4529aa707d1b75fd78
2024-02-28 21:53:07 +00:00
Shaquille Johnson
89106b8e6e Change the log level from error to warn
This log is said to be ignored so it should not
be at the level of an error as that could confuse
people.

Test: N/A
Change-Id: I561fc8c16337de9d40714d87d3525f432f5afad7
2024-02-28 20:43:17 +00:00
Treehugger Robot
3dfac14787 Merge "Fixes for the issues found while running Keystore2 client tests on a device with keymaster implementation." into main 2024-02-20 13:16:46 +00:00
Rajesh Nyamagoud
7620921a7f Fixes for the issues found while running Keystore2 client tests on a
device with keymaster implementation.

- Ignore INVALID tag in generated key characteristics if keymaster
  implementation is present.
- RSA_OAEP_MGF_DIGEST, ATTEST_KEY, USAGE_COUNT_LIMIT are not expected in
  generated key characteristics if keymaster implementation is present.
- Corrected device attest ids names.
- Skip device id attestation on device with GSI image and device
  first_api_level is less than 34.
- When the DEVICE_UNIQUE_ATTESTATION tag is used in key generation,
  root certificate signature verification is ignored during cert-chain
  verification.

Bug: 322118247
Test: atest keystore2_client_tests
Change-Id: I42d339a7797114d9139c64bc4d397889b965cb48
2024-02-19 20:24:47 +00:00
Shaquille Johnson
ea9fe60bf5 Merge "Update typo and add blob id to log" into main 2024-02-17 19:35:20 +00:00
Shaquille Johnson
f23fc9489b Update typo and add blob id to log
The logs were missing a word and also this will
add the blob id into the logs when a query fails
to delete the blob.

Test: atest keystore2_test
Change-Id: Ieff6e9266837744d16633c93df0b6da27871eca7
2024-02-13 17:01:29 +00:00
Stephen Hines
4d23482d7e Prefer .first() to .get(0)
```
error: accessing first element with `key_descriptors.get(0)`
   -->
system/security/keystore2/tests/keystore2_client_list_entries_tests.rs:143:27
    |
143 |                 let key = key_descriptors.get(0).unwrap();
    |                           ^^^^^^^^^^^^^^^^^^^^^^ help: try:
`key_descriptors.first()`
    |
    = help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#get_first
    = note: `-D clippy::get-first` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::get_first)]`
```

Bug: http://b/321303117
Test: toolchain/android_rust/test_compiler.py --prebuilt-path dist/rust-dev.tar.xz --target aosp_cf_x86_64_phone --all-rust
Change-Id: Ic278ca9ec8fc48e779741f43b1fd53702b54396f
2024-02-09 15:39:25 -08:00
Eric Biggers
3d4f5457af Merge "keystore2: rename MonotonicRawTime to BootTime" into main 2024-02-07 20:22:55 +00:00
Jeff Vander Stoep
153d1aaff4 Replace use of deprecated logging functions
This is needed to upgrade the android_logger crate from 0.12.0
to 0.13.3.

with_max_level provides the same functionality as with_min_level.
The renaming is admittedly confusing, but the new name is accurate
and it makes sense that they deprecated and then removed the
previously poorly named with_min_level.

See crate documentation [1] and code [2].

[1]: https://docs.rs/android_logger/0.12.0/android_logger/struct.Config.html#method.with_min_level
[2]: https://docs.rs/android_logger/0.12.0/src/android_logger/lib.rs.html#227

Bug: 322718401
Test: build and run CF with the change.
Test: m aosp_cf_x86_64_phone
Change-Id: Ibd13989ffe52a93191dd2d5a1b8f5a651eecd91d
2024-02-07 14:33:36 +01:00
Eran Messeri
cfe79f1828 Correcting permission check for App UIDs listing
Correct the permission check for the Keystore maintenance method
that returns the list of app UIDs which have keys that are
bound to a specific SID.

The previous check relied on SELinux policies. But the Settings
app that calls this method has a permission - MANAGE_USERS -
that is more appropriate to check.

Bug: 302109605
Test: Manual.
Change-Id: Ia26256cf995d16d03d0bb92d8b237f7bbea30d07
2024-02-06 14:58:09 +00:00
Oriol Prieto Gasco
85d84ff9ed Set the container field of aconfig flags
Test: m
Bug: 312769710
Change-Id: I366717c7139886e30360914256ad7710da5095e9
2024-02-03 02:39:40 +00:00
Treehugger Robot
2730678378 Merge "Replace use of deprecated logging functions" into main 2024-02-01 11:11:14 +00:00
Eric Biggers
19b3b0d894 keystore2: rename MonotonicRawTime to BootTime
Due to https://r.android.com/2822970 ("Use CLOCK_BOOTTIME for keystore2
auth token received time"), MonotonicRawTime now uses CLOCK_BOOTTIME
instead of CLOCK_MONOTONIC_RAW.  Therefore, rename it to BootTime.

Bug: 309686873
Test: atest -p --include-subdirs system/security/keystore2
Change-Id: If1fbbac2eccb03dc7360ae742d79e58f871fb80d
2024-01-31 22:54:54 +00:00
Jeff Vander Stoep
940820cfa1 Replace use of deprecated logging functions
This is needed to upgrade the android_logger crate from 0.12.0
to 0.13.3.

with_max_level provides the same functionality as with_min_level.
The renaming is admittedly confusing, but the new name is accurate
and it makes sense that they deprecated and then removed the
previously poorly named with_min_level.

See crate documentation [1] and code [2].

[1]: https://docs.rs/android_logger/0.12.0/android_logger/struct.Config.html#method.with_min_level
[2]: https://docs.rs/android_logger/0.12.0/src/android_logger/lib.rs.html#227

Bug: 322718401
Test: build and run CF with the change.
Test: m aosp_cf_x86_64_phone
Change-Id: I8d9d7c42100ede48496f9846068ed312fb8a15cb
2024-01-31 10:55:55 +01:00
Shaquille Johnson
07fec0ff0c Deprecating the aidl for Android Protected Confirmation
Android Protected Confirmation is deprecated due to the high
support/maintenance cost for Android device makers and low adoption rate
among app developers. APC requires Android device makers to have a
substantial amount of device-specific UI code running in the trusted
execution environment. That has proven to be expensive to maintain and
non-scalable, as there cannot be a single implementations device makers
can share or use as a reference. Additionally, app developers have not
adopted this feature, as the Android platform offers other mechanisms
for authentication a user's intent. These mechanisms, such as
authentication-bound Keystore keys, are less secure than Trusted UI, but
are more wide-spread. While we explore alternatives to APC that are
viable to the device makers ecosystem, we sunset the APC API.

Bug: 313856313
Test: atest keystore2_test && atest CtsKeystoreTestCases
Change-Id: If065697ed13e3de706b8dde5cc5e2b6018592018
2024-01-25 16:02:32 +00:00
Eran Messeri
4dc27b52eb List apps affected by secure user ID
Add a method to the Keystore maintenance interface to list the UIDs of
apps that are affected by a given secure user ID.

With this method, it would be possible to tell if removing a given
user's LSKF or enrolling new biometrics will invalidate Keystore keys,
thus affecting some apps.

Bug: 302109605
Test: atest keystore2_test
Change-Id: If5888506e0c72a56eca3339778889c7d8038acc5
2024-01-24 14:48:54 +00:00
Eric Biggers
3b862a87dd Merge "Fix UnlockedDeviceRequired with weak unlock methods" into main 2024-01-18 22:22:12 +00:00
Luca Stefani
481b5d663b Format Android.bp files with bpfmt
Change-Id: I083e96e3dd94a48ebad473bcfbbb7fcbb89ce466
2024-01-18 08:34:35 +01:00
Eric Biggers
6946daa1ab Fix UnlockedDeviceRequired with weak unlock methods
Starting in Android 12, unlocking the device with a class 1
("convenience") biometric, class 2 ("weak") biometric, or a trust agent
unexpectedly doesn't allow the use of UnlockedDeviceRequired keys.  The
cause of this bug is that the cryptographic protection that Keystore now
applies to UnlockedDeviceRequired keys incorrectly assumes that the
device can only be unlocked using LSKF or via a biometric that
participates in Keystore (has a SID and uses HardwareAuthTokens).
Actually, Keyguard also allows the device to be unlocked using weaker
biometrics that do not particiate in Keystore, if they are enrolled.
Similarly, there are also cases where a trust agent can actively unlock
the device, e.g. unlocking a phone using a paired watch.

In combination with the system_server changes in
I34dc49f1338e94755e96c1cf84de0638dc70d311, this CL fixes the bug by
making Keystore retain the UnlockedDeviceRequired super keys in memory
if a weak unlock method is enabled at device lock time.  This does mean
that UnlockedDeviceRequired is enforced only logically when a weak
unlock method is enabled, but this is the best we can do in this case.

This CL also adds methods by which Keystore can be notified of the
expiration of unlock methods, causing the security level of
UnlockedDeviceRequired keys to be upgraded.  A future CL for
system_server is planned to use these.

Test: see I34dc49f1338e94755e96c1cf84de0638dc70d311
Bug: 296464083
Change-Id: I1b0d9ec4f9e31dc91642e865045766bd17e34cad
2024-01-17 22:51:37 +00:00