Commit graph

1493 commits

Author SHA1 Message Date
Janis Danisevskis
6a0d998380 Fix grants get lost on key upgrade
The upgrade routine used to call KeyStore->del which purges the given
key blob from the keystore including all existing grants.
With this patch, upgrade only calls Keymaster::delete on the keyblobs
without purging it from the keystore. Also it only calls
Keymaster::delete once the upgrade key was successfully written to disk.

This patch also calls fsync on the directory containing keyblobs to
narrow the window in which keyblob may be lost due to power loss.

Bug: 110450771
Test: Upgrade path tested by manually creating a key, bumping the
      patchlevel, using the key subsequently and inspecting the logs.
Change-Id: I89241b5d4033b270733ff61838ab9244fce28c60
2019-05-03 16:19:06 -07:00
Janis Danisevskis
37896101ea Clearly indicate vendor errors from keymaster in logcat
This patch addes verbose logging whenever an error code in the vendor
error code range is returned by keymaster.

Bug: 123562864
Test: atest android.keystore.cts
Merged-In: Ifceece542d6f3536ad87d053145c7aa8dd6d6603
Change-Id: Ifceece542d6f3536ad87d053145c7aa8dd6d6603
2019-05-03 16:18:45 -07:00
Treehugger Robot
939ef15ed5 Merge "Make keystore more noisy on diagnosing corrupted key blobs." 2019-04-12 02:34:38 +00:00
Janis Danisevskis
56ec439a28 Make keystore more noisy on diagnosing corrupted key blobs.
Bug: 130163310
Test: Added logging, no functional changes.
Change-Id: I69a70eff2db4069ff479b2a397cb7b083f97d922
2019-04-11 13:28:15 -07:00
Treehugger Robot
72a0ffe9c1 Merge "Adding thread safety to proto uploader" 2019-03-26 05:07:50 +00:00
Max Bires
772a8de356 Adding thread safety to proto uploader
Due to lack of thread safety it seems that protobuf values were
occasionally getting deleted during parsing, causing ubsan to register
an error down the line in the protobuf library.

Bug: 128991260
Bug: 128810613
Test: atest cts/tests/tests/keystore/src/android/keystore/cts
Change-Id: Iee7ec6195e1e0aa4b28a7484737f984ed389a75e
2019-03-25 17:00:29 -07:00
Janis Danisevskis
dece12434d Merge "Fix keystore wifi concurrency issue." 2019-03-25 22:11:14 +00:00
Janis Danisevskis
b50236a08b Fix keystore wifi concurrency issue.
Keystore was conceptually single threaded. Even with the introduction of
Keymaster workers it was always assumed that the service dispatcher
thread was single threaded. The wifi keystore service, however, calls
into the keystore service concurrently.

This patch adds a lock around all keystore service entry points to make
sure all dispatcher executions are serialised despite being called from
both the binder and hwbinder service thread.

Bug: 128810613
Bug: 129145334
Bug: 128774635
Test: Regressions tested with Keystore CTS test suite.
Change-Id: I8c5602d2c2cb1dd9423df713037e99b247cee71f
2019-03-25 13:02:40 -07:00
Treehugger Robot
2cd64544e0 Merge "keystore_backend_binder: fix misinterpretation of getKeyCharacteristics return value." 2019-03-13 23:49:29 +00:00
Janis Danisevskis
61aea51375 keystore_backend_binder: fix misinterpretation of getKeyCharacteristics return value.
The keystore backend used by racoon interprets the return value of
getKeyCharacteristics such that it thinks that it failed when it didn't.

Test: Initiate VPN connection with racoon.
Bug: 120024003
Change-Id: Ibe936a2d2d81181c10f0dd1075cc5ab3646f736e
2019-03-13 14:34:06 -07:00
Treehugger Robot
eb7b084d60 Merge "DO NOT MERGE - Skip PPRL.190305.001 into master" 2019-03-11 22:36:06 +00:00
The Android Open Source Project
9d672e1ef9 DO NOT MERGE - Skip PPRL.190305.001 into master
Bug: 127812889
Change-Id: I6de3d7b2ebf4592b86b7088e545b86c551505fd5
2019-03-11 14:29:45 -07:00
Max Bires
3e9cd87e48 Merge "Adding KEY_PERMANENTLY_INVALIDATED to ResponseCode" 2019-03-08 17:53:05 +00:00
Branden Archer
b46532269f Merge "Give some permissions to bluetooth" 2019-02-22 20:29:59 +00:00
Branden Archer
a930c149d1 Give some permissions to bluetooth
The bluetooth user will need to encrypt and decrypt data to ensure
that the integrity of passwords can be verified. These provide
the needed permissions to create/remove keys and sign/verify using them.

Bug: 117993149
Test: Bluetooth operations work from the UI and unit tests pass
Change-Id: I9092c2c282f26b40cd15da84125e6e11354ec48b
2019-02-22 09:19:29 -08:00
Max Bires
6669421021 Adding KEY_PERMANENTLY_INVALIDATED to ResponseCode
This response code needs to be added in the condition where a super
encrypted key blob fails to be read in after a user changes their pin.
Currently, the error code being sent back is VALUE_CORRUPTED, which is
incorrect.

Bug: 118883532
Test: atest cts/hostsidetests/appsecurity/src/android/appsecurity/cts/AuthBoundKeyTest.java
Change-Id: I188948e6e2e66903ee259108db9b8d26d11ca92c
Merged-In: I188948e6e2e66903ee259108db9b8d26d11ca92c
2019-02-10 23:36:23 +00:00
android-build-team Robot
5510b799f9 Snap for 5180536 from 72a9c568f1 to pi-platform-release
Change-Id: I7acd29755d476f8b23e438b9f7c8b72fa31fd469
2019-02-09 02:37:21 +00:00
Branden Archer
3ed33f4013 Merge "Grant VTS tests all permissions in keystore on userdebug/eng" 2019-01-31 16:06:37 +00:00
Branden Archer
84e7231d73 Grant VTS tests all permissions in keystore on userdebug/eng
A VTS test for the Wifi Keystore HAL is being created. The test
is run as root and attempts various operations that directly
use the Keystore service. By default that test will not be
able to perform necessary actions to exercise the HAL code,
such as creating keys for tests.

This change will enable the root user to perform all key
operations, but only on userdebug and eng builds. In addition,
the root user will be able to perform actions on behalf of the
wifi user; this is necessary as some operations in the Wifi
Keystore HAL assume the wifi user.

Bug: 120182820
Test: atest system/hardware/interfaces/wifi/keystore/1.0/vts/
      functional/VtsHalWifiKeystoreV1_0TargetTest.cpp

Change-Id: Ic6eb5748e0e19b64a44c4bdf88a7074f7367db3d
2019-01-29 11:16:53 -08:00
Treehugger Robot
60f9053686 Merge "Reducing amount of files created in dropbox for keystore" 2019-01-22 16:32:28 +00:00
Treehugger Robot
e91257f445 Merge "Fix class/struct mismatch." 2019-01-16 17:44:20 +00:00
Dan Albert
b1e79364b3 Fix class/struct mismatch.
libc++ has switched this from a class to a struct to match libstdc++.
The standard does not require either specifically, but Clang warns
about the mismatch: https://bugs.llvm.org/show_bug.cgi?id=39871.

Test: m
Bug: None
Exempt-From-Owner-Approval: Janitorial
Change-Id: Iffbd944f60eb363ef7cd8f9f2c2eea77ff967310
2019-01-15 21:41:30 +00:00
Max Bires
091ed1b975 Reducing amount of files created in dropbox for keystore
This change will reduce the number of files written to dropbox for the
purposes of keystore logging. Previously, if a ton of operations were
being done, a file would be created for each operation which led to
spamming of the dropbox directory.

Now, all equivalent operations are counted and only one copy is sent
along with the corresponding count over the time period of an hour. This
limits the number of files keystore can write to dropbox to 24 a day,
and will reduce the possible size of files since redundant operations
aren't being written.

Bug: 117823210
Bug: 110988360
Test: atest cts/tests/tests/keystore/src/android/keystore/cts
Change-Id: I79367aa7a8eb3679aace2058e128d06e513e25ea
2019-01-15 18:47:42 +00:00
Chih-hung Hsieh
3a33d315bf Merge "Fix/suppress system/security google-explicit-constructor warnings" 2019-01-11 18:03:38 +00:00
Treehugger Robot
91df8c0c63 Merge changes from topic "keystore-niap"
* changes:
  Increase the master key size to 256 bits
  Use vector to pass around keys
  Use SHA256 for 32 byte keys
  Replace Entropy with RAND_bytes
2019-01-11 16:59:09 +00:00
Branden Archer
d0315735fd Increase the master key size to 256 bits
NIAP certification finds that the 128 bit key size is insufficient
and requires a 256 bit key size. This change increases the
size of new master keys to 256 bits. Any existing master keys are
not changed and continue to be supported.

A new BlobType, TYPE_MASTER_KEY_AES256, is used to signal when a
key is the new larger size.

Bug: 121272336
Test: (1) Ran Keystore CTS tests against Walleye.
      (2) Created keys in build without change, moved to build
          with change and verified old key could be loaded and
	  used. Also, a new key could be created with the
	  increased size and could be reloaded after a reboot.
Change-Id: If00331c303e6cc7bc95a2ab624d0e19bec4e587e
2019-01-10 17:01:14 -08:00
Branden Archer
f5953d7ace Use vector to pass around keys
In the future the key size for new master keys will increase.
To maintain backwards compatibility the size of the key
can no longer be assumed. To help communicate the actual
size of the key, it will be passed around in a vector.

Bug: 121272336
Test: Ran Keystore CTS tests against Walleye
Change-Id: I4c05acb15b77959f2bf89abbdc325904fffb497a
2019-01-10 17:01:10 -08:00
Branden Archer
cefbcbcb15 Use SHA256 for 32 byte keys
For NIAP certification keys need to be generated using SHA256 or
higher. Presently SHA1 is used. To satisfy this requirement,
SHA256 will be used for new keys. As the master key has recently
increased in size, the key size is used to determine if SHA1 is used
(for older keys) or SHA256.

Bug: 121272336
Test: Ran Keystore CTS tests against Walleye

Change-Id: I6099156173e04b22c6edafd9fb0e072f7201c5ee
2019-01-10 09:03:17 -08:00
Chih-Hung Hsieh
4fa39ef416 Fix/suppress system/security google-explicit-constructor warnings
* Add explicit to conversion constructors/operators
* Use NOLINT or NOLINTNEXTLINE to suppress warnings on intended converters

Bug: 28341362
Test: make with WITH_TIDY=1 DEFAULT_GLOBAL_TIDY_CHECKS=-*,google-explicit-constructor
Change-Id: I4ed5aea36fcdcd8dbda9a4be9607c5af606f2a08
2019-01-04 13:34:55 -08:00
Branden Archer
44d1afa2c0 Replace Entropy with RAND_bytes
/dev/urandom is not an approved random number generator
for NIAP certification. Changing to use BoringSSL's
RAND_bytes(), which is approved.

Bug: 121272336
Test: Ran Keystore CTS tests against Walleye
Change-Id: I579d140ef56c90b477b0d8989e3b02375681aee8
2018-12-28 10:19:15 -08:00
Rob Barnes
4cf3bf237a Merge "Changed uid output parameter from an int array to a list of strings." 2018-12-20 23:33:25 +00:00
Rob Barnes
5d59e6351c Changed uid output parameter from an int array to a list of strings.
Why?: 1) Returning an int array is unsafe because it must be allocated in Java and C++ must not change the size. 2) List<Integer> is not supported by AIDL, but List<String> is. I decided it was simpler to pass back integers encoded as strings than to create yet another parcelable.

Bug: b/119616956
Test: ./list_auth_bound_keys_test.sh
Test: Temporarily modified settings app to call listUidsOfAuthBoundKeys
Change-Id: Ibf86864a5df1608a39f438745dde6f2f8c296b66
2018-12-20 19:00:05 +00:00
Chih-hung Hsieh
6df27abdd0 Merge "Fix performance-for-range-copy warnings" 2018-12-13 06:30:14 +00:00
Chih-Hung Hsieh
b3bfdb050e Fix performance-for-range-copy warnings
Bug: 30413223
Test: make with WITH_TIDY=1 DEFAULT_GLOBAL_TIDY_CHECKS=-*,performance*
Change-Id: Ibd59f19978cd2d6048fa608723f7d68098a18216
2018-12-12 14:43:45 -08:00
Treehugger Robot
a4b74660a2 Merge "C++17 is now the default." 2018-12-12 03:44:57 +00:00
Treehugger Robot
fe2a27d9a9 Merge "DO NOT MERGE" 2018-12-07 01:38:19 +00:00
The Android Open Source Project
e8d5c5ca08 DO NOT MERGE
Merge pie-platform-release (PPRL.181105.017, history only) into master

Bug: 118454372
Change-Id: If897b9d79f937e7cae6e0f131e66e68f1bf4b4f1
2018-12-06 14:11:47 -08:00
Elliott Hughes
f1d336b64e C++17 is now the default.
Test: builds
Change-Id: Ib96ce1db95dfef6d2b5a269ee47dbd0a89650bc4
2018-12-05 19:48:42 -08:00
Treehugger Robot
538a4e789e Merge "C++17 is now the default." 2018-12-05 20:41:50 +00:00
Treehugger Robot
e8c1317b77 Merge changes from topic "keystore-rc"
* changes:
  Replace cast operator with getValue() for key store return codes
  Check key store result with isOk() instead of casted value
  Use stream operator to report result code
2018-12-05 18:19:29 +00:00
Treehugger Robot
41bf9b04e4 Merge "KeyStore: Fix key name decoding" 2018-12-05 17:32:34 +00:00
Elliott Hughes
9db4be105a C++17 is now the default.
Test: builds
Change-Id: Ibd6569c25f4ddfed3fd0cec771fba72cd5b9bd14
2018-12-04 13:06:50 -08:00
Eran Messeri
2ba77c303c KeyStore: Fix key name decoding
The key name is encoded into the filename containing the (encrypted) key
material.

Since the key name might contain characters that are not valid in a
filename, the name is encoded using a multi-character custom encoding
scheme.

However, the decoding function did not decode the key name correctly -
in particular, spaces were decoded to 'P', causing CtsVerifier tests
that install a key with a space in the name to fail (due to internal
inconsistency between the key names in KeyChain's DB and key names
obtained from Keystore).

Fix by correctly compensating for the "carrier" character.

Test: atest keystore_unit_tests
Bug: 116716944
Change-Id: I0326a9d9e6912b04bb13b3b350ead8ddcfcc12f8
2018-12-04 12:35:27 +00:00
Bill Yi
e09a42c206 Merge pi-qpr1-release PQ1A.181105.017.A1 to pi-platform-release
Change-Id: I4e5befe2b190512c55e11d429aa57fa9132a5080
2018-11-28 18:35:05 -08:00
Treehugger Robot
e229d5efac Merge "Fix key upgrade on begin" 2018-11-28 06:02:23 +00:00
Janis Danisevskis
64eb3ebb33 Fix key upgrade on begin
Since getKeyCharacteristins uses a cache file it is it is no longer
guarantied to call upgrade key any more. So we have to put the upgrade
key logic back into begin. Also we need to get extract the key blob data
from the key blob object every time the key blob could have changed.

Test: enroll a password, bump the patch level, rebuild and flash.
      Then attempt to unlock the device with the password.
Bug: 120063166

Change-Id: If91c30d3f0599452b43923255bb88fee490beb21
2018-11-27 20:50:57 -08:00
Treehugger Robot
8ba1cb3268 Merge "Fix KeyStoreClientImpl::doesKeyExist to return correct result" 2018-11-22 00:33:50 +00:00
Branden Archer
1a87fdc2fa Fix KeyStoreClientImpl::doesKeyExist to return correct result
The KeyStoreService returns NO_ERROR if the key was found,
and another response code otherwise. All of these are
mapped to non-zero values. As a result, if a key's
existence was queried it would always respond "true",
regardless if it exists or not or if there was a permissions
error.

Test: Key existence can be successfully checked with the
      keystore_cli_v2 tool.

Change-Id: Iffc2e155a61354f1fbffbece093b19e5cbc537fd
2018-11-21 14:58:38 -08:00
Branden Archer
7008074335 Replace cast operator with getValue() for key store return codes
The value of the error code is not intended to be used
in checks, and instead isOk() should be used. A few places
were found which used the error codes directly via the
cast operator. To make it less likely this will happen
in the future unintentionally, the cast operator is being
removed. Some code still wants to access the error code
directly, such as when logging, so getValue() is added
for these cases.

Bug: 119771891
Test: Built for walleye successfully, basic operations with
      keystore_cli_v2 tool work correctly.
Change-Id: I46e82d66dc4932472d8a5b2749ece08e398e7c88
2018-11-21 13:46:43 -08:00
Branden Archer
d166a88fb9 Check key store result with isOk() instead of casted value
The underlying value of the return code is not meaningful
in a check, instead isOk() should be used.

Bug: 119771891
Test: Built for walleye successfully, basic operations with
      keystore_cli_v2 tool work correctly, new cts test
      no longer finds a crash in keystore.
Change-Id: Id20612824677619cbbd0b2ba4a11b15fe5258ecb
2018-11-21 13:45:58 -08:00