Commit graph

11 commits

Author SHA1 Message Date
Shawn Willden
17b8709c67 Fix handling of user password changes.
A bug introduced in a patch intended to upgrade keystore master keys
to use AES-256 and SHA-256 instead of AES-128 and SHA1 causes the
newly-updated master key to fail to be retrievable ever again.  Making
this worse, after five successive failures, keystore decided that all
the data is bad and wipes the user's keystore.  This problem happens
on every password change if the master key is 128 bits.  Luckily,
since the introduction of synthetic passwords to support escrow
tokens, the password presented to keystore is the synthetic password,
which never changes.  So this problem only crops up in devices that
did not have synthetic passwords (launched with Android N or earlier),
were not upgraded to O DR1 (when synthetic passwords were enabled by
default), were never factory reset or had their password removed and
re-added during all of that time and were then upgraded to P or Q,
when the master key upgrade code was present.

This CL fixes the upgrade process so that updated master keys can be
used.  It doesn't change the key size, the keys stay 128 bits, but now
they're readable and usable.  Factory resetting allows an entirely
new master key to be generated, which will be AES-256.

Note that the keystore master key is not really essential to the
security of Keystore keys.  They're also encrypted by the secure
world (TEE or SE), which is their primary protection.  The master key
just provides a cryptographic dependency on the user's password, so
that in the event of a secure world break the attacker still has to
brute force the user's password to recover the key material, or use of
the protected keys.

Bug: 129970023
Test: Manual
Change-Id: I8ce2bb2359cf822039c137bb6bb1fc225da47c29
2019-10-01 17:43:43 -06: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
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
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
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
Yi Kong
d29167516f Modernize codebase by replacing NULL with nullptr
Fixes -Wzero-as-null-pointer-constant warning.

Test: m
Bug: 68236239
Change-Id: I41cd58617d6df6de7942a541fb6dc9519c70bef0
Merged-In: I41cd58617d6df6de7942a541fb6dc9519c70bef0
2018-07-30 08:41:16 +00: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
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