Commit graph

33 commits

Author SHA1 Message Date
Shawn Willden
a4c76c5541 Encrypt AES-256 keystore master keys.
am: 921d3a3678

Change-Id: I0cf7053bd1391eb02c1cac144ff1aa506e3dfc74
2019-10-04 18:32:18 -07:00
Shawn Willden
921d3a3678 Encrypt AES-256 keystore master keys.
ag/5984229 that added support for AES-256 master keys inadvertently
caused them not to be encyrpted by the user's password.  This is less
damaging to security than it might appear because these keys are also
encrypted by Keymaster, in the TEE or StrongBox.

Bug: 141955555
Test:  Manually verify password is encryption on a userdebug build.
Change-Id: Ic5e82546df67346e4c348273cf4fe2bac382c9dc
2019-10-02 15:17:51 +00:00
David Benjamin
e2d3e0b584 Merge "Replace custom BoringSSL scopers with bssl::UniquePtr."
am: 147f3df5c3

Change-Id: Ieb0bb08b000fe2eada74c8884fc4845beadc9d0f
2019-08-19 16:54:11 -07:00
David Benjamin
dc4d142303 Replace custom BoringSSL scopers with bssl::UniquePtr.
BoringSSL already provides C++ scopers.

Test: mma
Change-Id: I34d4ec36fc0b51750560be0886768a83fe69fbf5
2019-08-08 13:13:54 -04:00
Janis Danisevskis
06b96aa1ea Merge changes from topic "verbose-vendor-logging" am: b6069dcb3c
am: bc57549c90

Change-Id: Ifb53b5e715b1e66a0af4afee54be9133e4c3b198
2019-05-06 17:18:48 -07:00
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
55570066c7 Merge "Make keystore more noisy on diagnosing corrupted key blobs." am: 939ef15ed5
am: fb27742c63

Change-Id: Ie402dc09899934d85e5eece1759a11eae7301c1f
2019-04-11 19:45:46 -07: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
Janis Danisevskis
8196b8c937 Add logging around access calls.
We saw evidence, that the keystore exist method may be flaky. This
patch adds logging around the access calls used to check for key blobs.
Also adds a retry loop in case access return EAGAIN. (Although access
should not return EAGAIN).

Bug: 128318105
Test: Only logging added.
Change-Id: Ib07396f590c2f58e227f85869ada8c6d6dd46df4
2019-03-15 15:01:33 -07:00
Max Bires
3e9cd87e48 Merge "Adding KEY_PERMANENTLY_INVALIDATED to ResponseCode" 2019-03-08 17:53:05 +00: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
Max Bires
624cf843b5 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
2019-02-07 14:29:30 -08:00
Chih-hung Hsieh
3a33d315bf Merge "Fix/suppress system/security google-explicit-constructor warnings" 2019-01-11 18:03:38 +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
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
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
Janis Danisevskis
ff3d7f4b83 Multithreaded Keystore
This patch transitions keystore a threading model with one dispatcher
thread and one worker thread per keymaster instance, i.e. fallback, TEE,
Strongbox (if available). Singleton objects, such as the user state
database, the enforcement policy, and grant database have been moved to
KeyStore and were made concurrency safe.
Other noteworthy changes in this patch:

* Cached key characteristics. The key characteristics file used to hold
  a limited set of parameters used generate or import the key. This
  patch introduces a new blob type that holds full characteristics as
  returned by generate, import, or getKeyCharacteristics, with the
  original parameters mixed into the software enforced list. When
  keystore encounters a lagacy characteristics file it will grab the
  characteristics from keymaster, merge them with the cached parameters,
  and update the cache file to the new format. If keystore encounters
  the new cache no call to keymaster will be made for retrieving the
  key characteristics.
* Changed semantic of list. The list call takes a prefix used for
  filtering key entries. By the old semantic, list would return a list
  of aliases stripped of the given prefix. By the new semantic list
  always returns a filtered list of full alias string. Callers may
  strip prefixes if they are so inclined.
* Entertain per keymaster instance operation maps. With the introduction
  of Strongbox keystore had to deal with multiple keymaster instances.
  But until now it would entertain a single operations map. Keystore
  also enforces the invariant that no more than 15 operation slots are
  used so there is always a free slot available for vold. With a single
  operation map, this means no more than 15 slots can ever be used
  although with TEE and Strongbox there are a total of 32 slots. With
  strongbox implementation that have significantly fewer slots we see
  another effect of the single operation map. If a slot needs to be
  freed on Stronbox but the oldest operations are on TEE, the latter
  will be unnecessarily pruned before a Strongbox slot is freed up.
  With this patch each keymaster instance has its own operation map and
  pruning is performed on a per keymaster instance basis.
* Introduce KeyBlobEntries which are independent from files. To allow
  concurrent access to the key blob data base, entries can be
  individually locked so that operations on entries become atomic.
  LockedKeyBlobEntries are move only objects that track ownership of an
  Entry on the stack or in functor object representing keymaster worker
  requests. Entries must only be locked by the dispatcher Thread. Worker
  threads can only be granted access to a LockedKeyBlobEntry by the
  dispatcher thread. This allows the dispatcher thread to execute a
  barrier that waits until all locks held by workers have been
  relinquished to perform blob database maintenance operations, e.g.,
  clearing a uid of all entries.
* Verification tokens are now acquired asynchronously. When a begin
  operation requires a verification token a request is submitted to the
  other keymaster worker while the begin call returns. When the
  operation commences with update or finish, we block until the
  verification token becomes available.

As of this patch the keystore IPC interface is still synchronous. That
is, the dispatcher thread dispatches a request to a worker and then
waits until the worker has finished. In a followup patch the IPC
interface shall be made asynchronous so that multiple requests may be in
flight.

Test: Ran full CTS test suite
      atest android.keystore.cts
Bug: 111443219
Bug: 110495056
Change-Id: I305e28d784295a0095a34810d83202f7423498bd
2018-10-31 14:31:26 -07:00
Logan Chien
67519a5f72 Merge "Deprecate <cutils/log.h> and <utils/Log.h>" 2018-10-02 02:58:43 +00:00
Rob Barnes
b9c293b302 Use unique temporary filename for keystore blob.
If the writeBlob is invoked by concurrent threads, the temporary
file can be deleted before the write operation completes.

Bug: b/115688924
Test: atest keystore_unit_tests
Test: atest KeystoreTests
Test: atest cts/tests/tests/keystore/
Change-Id: Icdadacc9af4af281bff6fc17a6b43ba731377b53
2018-09-26 14:26:21 -06:00
Logan Chien
cdc813f782 Deprecate <cutils/log.h> and <utils/Log.h>
This commit replaces <cutils/log.h> and <utils/Log.h> with <log/log.h>.

Background:
<cutils/log.h> has been moved to <log/log.h> for a while.  Both
<cutils/log.h> and <utils/Log.h> simply includes <log/log.h> for
backward compatibility.  This commit is a part of the effort to remove
<cutils/log.h> and <utils/Log.h> from the source tree eventually.

Bug: 78370064
Test: lunch aosp_walleye-userdebug && cd system/security && mma
Change-Id: I798f06d78e2cc5cd197727c0bcdd05c87d769a90
2018-09-19 13:38:34 +08:00
Pavel Grafov
cef39477f9 NIAP: Log key integrity failure to audit log.
Logs key integrity violation in two cases:
1. software-detected corruption of key blob.
2. keymaster operation returning INVALID_KEY_BLOB

Changed AES_gcm_decrypt to return VALUE_CORRUPTED on decryption errors
to be consistent with digest check for older version blob.

Bug: 70886042
Test: manual, by patching some bytes in the blob.
Test: cts-tradefed run cts -m CtsKeystoreTestCases
Change-Id: Ic8f6b7a2a49aee01253b429644af409e568d7deb
2018-02-15 18:20:28 +00:00
Janis Danisevskis
c1460141c0 KeyStore: use security level to chose keymaster device
Keymaster4 introduces security levels. Android devices
may have multiple keymaster implementations, one for each
possible security level, where the presence of a strong
security level implies the presence of all lower levels.

This patch adds code that enumerates all keymaster device
implementations available from ServiceManager and populates
Keystore's keymaster device database with at most one keymaster
implementation per security level. It gives precedence to
newer versions if multiple implementations exist for the same security
level.

The security level is chosen by a set of flags passed to the keystore
operations generate, import, addRngEntropy.
For existing keys the right security level is chosen by the blob flags.

To that end a new flag KEYSTORE_FLAG_STRONGBOX was added, and the
security level is expressed through a combination of
KEYSTORE_FLAG_FALLBACK (F) and KEYSTORE_FLAG_STRONGBOX (S).
Encoding is as follows:

             F     S
Software     1     X (don't care)
TEE          0     0
Strongbox    0     1

Some operations in keystore cli2 where amended with the optional
--seclevel flags. Allowing the user to chose the security level for the
given operation. Possible options are "software", "strongbox", and "tee"
where tee is the default value.

Test: Existing KeyStore CTS tests run

Change-Id: I01ef238f5e7067e480cf9b171630237236046bb1
2017-12-27 16:38:09 -08:00
Janis Danisevskis
36316d673e Fix use of auth-bound keys after screen lock removal
When auth-bound keys are used after the screen lock has been removed it
is expected that getKeyCharacteristics still succeeds. However, when the
super encrypt feature was introduced the key blob is no longer
accessible, and thus, the retrieving the key characteristics fails.

This patch retrieves the key characteristics from the characteristics
cache file, which is not super encrypted. Using such a key still fails
but in ways expected by the framework.

Bug: 65200397
Test: CtsVerifier ScreenLockBoundKeysTest:
      1. Run test
      2. with CtsVerifier in the background remove the screen lock
         through the settings dialog
      3. Select VtsVerifier in 'recents'
      4. Run test again

Change-Id: Ifa88c58a41c376e4f800a76114d4cf9149506ac0
2017-09-01 14:58:39 -07:00
Shawn Willden
0ed642cb8b Make zeroing more portable.
Bug:35849499
Test: None required.
Change-Id: Iadee02a253b491f192c4a8b1cf3e57125ad866a6
2017-05-26 10:13:28 -06:00
Shawn Willden
e9830589dd Use AES-GCM to encrypt keystore blobs.
Keystore currently uses AES-CBC to encrypt keystore blobs, plus an MD5
digest for authentication.  This scheme is mildly broken (b/26804580),
but has not been replaced because keystore encryption was slated for
removal.  In order to support cryptographic binding of keys to user
authentication on devices with trusted secure computing modules,
keystore encryption has temporarily become relevant again, until a
better solution can be constructed.  Thus there's a motivation to
replace the broken scheme with a proper authenticated encryption mode.

Along the way, this CL also fixes a low-priority security vulnerability,
b/31824325.

Bug: 26804580
Bug: 31824325
Bug: 35849499
Test: Manually tested the new scheme and upgrading from the old scheme
Change-Id: I139f2a7b7a3c01eade4e2d2a674d49d027179d43
2017-05-23 20:16:04 -06:00
Rubin Xu
67899de5ad Introduce KEYSTORE_FLAG_CRITICAL_TO_DEVICE_ENCRYPTION
This flag is used by system server to mark keys used during the
synthetic password auth flow. keys marked with this flag will not
be super encrypted because super encryption requires knowledge of
the synthetic password, causing a chicken-and-egg problem.

Bug: 35849499
Bug: 34600579
Test: cts-tradefed run cts-dev -m CtsDevicePolicyManagerTestCases -t com.android.cts.devicepolicy.MixedProfileOwnerTest#testResetPasswordWithToken
Change-Id: Ibd900e3ede1f51c476d462085caaf216d911d693
2017-04-24 21:08:07 +01:00
Rubin Xu
484779559c Disable super encryption for now
It's clashing with synthetic password flow due to the fact that
synthetic password flow requires an auth-bound key in keystore,
but when the key is being requested, keystore is yet to receive
the synthetic password so it can't encrypt the auth-bound key

Bug: 37474130
Bug: 35849499
Test: cts-tradefed run cts-dev -m CtsDevicePolicyManagerTestCases -t com.android.cts.devicepolicy.MixedProfileOwnerTest#testResetPasswordWithToken
Change-Id: I58b05f1464d2c85a30ce17fbf5d7eca5da114173
2017-04-19 15:39:20 +01:00
Shawn Willden
45c112a566 Decrypt super-encrypted keys
This fixes a bug introduce in ag/2108644.  That CL added automatic
super-encryption of authentication-bound keys, but missed the necessary
change to decrypt them when needed.

Test: Manually tested
Change-Id: I84693b145a6d57d5957aa40d10236e1a0610d12c
2017-04-17 09:02:42 -06:00
Shawn Willden
d5a24e6745 Superencrypt authentication-bound keys.
This CL causes keystore to automatically encrypt all newly-created
keymaster key blobs which are authentication-bound.  This appears on its
face to be pointless, since the sensitive key material in the key blobs
is already encrypted by the Trusted Execution Environment.  It's not
pointless because this adds a cryptographic dependency on the user's
password, including any strengthening performed by
LockSettingService... which may include the use of a separate hardware
trusted module, separate from (and presumably more secure than) the TEE.

A better solution is planned for the next release, but that requires
changes to Gatekeeper and Keymaster. This superencryption will be
removed when that work is done.

Note that the encryption method used by keystore is weak. A separate CL will
replace the weak method with a proper authenticated encryption.

(cherry picked from commit 07aebe7305)

Test: Manual testing.
Bug: 35849499
Change-Id: I0c4910ea24b97bc8046f3d114bfb336670d03321
2017-04-13 17:45:49 -06:00
Janis Danisevskis
c7a9fa29c1 Port to binderized keymaster HAL
This patch ports keystore to the HIDL based binderized keymaster HAL.
Keystore has no more dependencies on legacy keymaster headers, and
therefore data structures, constant declarations, or enums. All
keymaster related data structures and enums used by keystore are the
once defined by the HIDL based keymaster HAL definition.  In the process
of porting, keystore underwent some changes:

* Keystore got a new implementation of AuthorizationSet that is fully
  based on the new HIDL data structures. Key parameters are now either
  organised as AuthorizationSets or hidl_vec<KeyParameter>.  (Formerly,
  this was a mixture of keymaster's AuthorizationSet,
  std::vec<keymaster_key_param_t>, and keymaster_key_param_set_t.)  The
  former is used for memory management and provides algorithms for
  assembling, joining, and subtracting sets of parameters.  The latter
  is used as wire format for the HAL IPC; it can wrap the memory owned
  by an AuthorizationSet for this purpose.  The AuthorizationSet is
  accompanied by a new implementation of type safe functions for
  creating and accessing tagged key parameters,
  Authorizations (keystore/keymaster_tags.h).
* A new type (KSSReturnCode) was introduced that wraps keystore service
  response codes. Keystore has two sets of error codes.  ErrorCode
  errors are less than 0 and use 0 as success value.  ResponseCode
  errors are greater than zero and use 1 as success value.  This patch
  changes ResponseCode to be an enum class so that is no longer
  assignable to int without a cast. The new return type can only be
  initialized by ResponseCode or ErrorCode and when accessed as int32_t,
  which happens on serialization when the response is send to a client,
  the success values are coalesced onto 1 as expected by the
  clients. KSSreturnCode is also comparable to ResponseCode and
  ErrorCode, and the predicate isOk() returns true if it was initialized
  with either ErrorCode::OK (0) or ReponseCode::NO_ERROR (1).
* A bug was fixed, that caused the keystore verify function to return
  success, regardless of the input, internal errors, or lack of
  permissions.
* The marshalling code in IKeystoreService.cpp was rewritten.  For data
  structures that are known to keymaster, the client facing side of
  keystore uses HIDL based data structures as (target) source
  for (un)marshaling to avoid further conversion.  hidl_vecs are used to
  wrap parcel memory without copying and taking ownership where
  possible.
* Explicit use of malloc is reduced (malloc was required by the C nature
  of the old HAL).  The new implementations avoid explicit use of
  malloc/new and waive the use of pointers for return values. Instead,
  functions return by value objects that take ownership of secondary
  memory allocations where required.

Test: runtest --path=cts/tests/tests/keystore/src/android/keystore/cts

Bug: 32020919
Change-Id: I59d3a0f4a6bdf6bb3bbf791ad8827c463effa286
2017-01-23 08:30:49 -07:00
Shawn Willden
c1d1feee51 Refactor keystore.
This CL isn't nearly as big as it looks.  It doesn't change keystore
functionality, it just moves all of the classes out of the former
keystore.cpp into their own .h and .cpp files.

Note that this is a cherry-pick from:

    https://android-review.googlesource.com/#/c/194971

Change-Id: Ide326c4f1d03984994d1bd9a76fa68d37da230dc
2016-01-26 22:48:06 -07:00