Commit graph

403 commits

Author SHA1 Message Date
Rubin Xu
0010dae9ff resolve merge conflicts of 2b93ec4 to oc-dr1-dev
Test: I solemnly swear I tested this conflict resolution.
Change-Id: Ie605cbb7f90eca6d17c2c5f6a50ec1ee21edf633
Merged-In: I6709b7562d47ad6156bee88a9e2d961f8a4a797d
2017-11-03 23:23:43 +00:00
Rubin Xu
2b93ec41b1 [DO NOT MERGE] Fix keychain key upgrade issue
Fix issues in keystore where it didn't handle key upgrade for granted keys
or keys with effective uid (keys under WiFi uid)  correctly.

Test: manual
Bug: 66094261
Change-Id: I6709b7562d47ad6156bee88a9e2d961f8a4a797d
Merged-In: I6709b7562d47ad6156bee88a9e2d961f8a4a797d
2017-11-01 23:23:09 +00:00
Janis Danisevskis
aca4c4cf48 Refurbish granting mechanism
Keystore stores key blobs in with filenames that include the symbolic
name and the uid of the owner. This behaviour should have been
completely opaque to the user keystore. However, the granting mechanism,
by which an app can allow another app to use one of its keys, leaked the
internal structure in that the grantee had to specify the key name with
the granter's uid prefix in order to use the granted key. This in turn
collided with prefix handling in other parts of the framework.

This patch refurbishes the granting mechanism such that keystore can
choose a name for the grant. It uses the original symbolic key name as
prefix and appends _KEYSTOREGRANT_<grant_no> where the grant_no is
chosen as first free slot starting from 0. Each uid has its own grant_no
space.

This changes the grant call such that it now returns a string, which is
the alias name of the newly created grant. The string is empty if the
grant operation failed.

As before apps can still mask granted keys by importing a key with the
exact same name including the added suffix. But everybody deserves the
right to shoot themselves in the foot if they really want to.

Bug: 37264540
Bug: 62237038
Test: run cts-dev --module CtsDevicePolicyManagerTestCases --test
          com.android.cts.devicepolicy.DeviceOwnerTest#testKeyManagement
	  because it grants a key
Merged-In: I723c44c7ae6782c8de42063744717d088cd49ba1
Change-Id: I723c44c7ae6782c8de42063744717d088cd49ba1
2017-07-22 09:59:06 -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
Janis Danisevskis
5d50d7e605 Rename libkeymaster to libkeymaster_staging
Fix a build breakage by renaming libkeymaster to
libkeymaster_staging. fugu's vendor tree already had
a libkeymaster.so which masked system/keymaster/libkeymaster.

Bug: 37997750
Change-Id: I589e9c87c2120034d5709a0d7a141d724065d683
2017-05-04 14:16:04 -07:00
Janis Danisevskis
5cc0d5ee60 Split libkeymaster1 into libkeymaster and libkeymaster_portable
Also removed unused include

Bug: 37467707
Test: trivial
Change-Id: Ie029462cb79d7aec28a37aac22f04ce73ebac8da
2017-05-03 15:53:26 +00:00
Nick Kralevich
7069027bc7 Merge "Revert "Split libkeymaster1 into libkeymaster and libkeymaster_portable"" 2017-05-03 00:41:32 +00:00
Janis Danisevskis
be0ec4f4be Revert "Split libkeymaster1 into libkeymaster and libkeymaster_portable"
This reverts commit ea3b820b93.

Reason for revert: build breakage

Change-Id: Ib101a5e896ffa816a7b9fb46774113846fa82590
2017-05-03 00:23:24 +00:00
Janis Danisevskis
e0267942c6 Merge "Split libkeymaster1 into libkeymaster and libkeymaster_portable" 2017-05-02 21:51:20 +00:00
Janis Danisevskis
356b9223a3 Merge "Remove use of UniquePtr from keystore" 2017-05-02 17:49:37 +00:00
Steven Moreland
52868f226e Explicitly declare dependencies on libbase macros.
These were being included transitively through libhidl.

Test: links
Bug: 37791060
Change-Id: I6cbe3a64b1ae078b4625c5e4b905be50e04f7a55
2017-05-01 12:35:41 -07:00
Janis Danisevskis
ea3b820b93 Split libkeymaster1 into libkeymaster and libkeymaster_portable
Also removed unused include

Test: trivial
Change-Id: I37122aead5b60be8dd0697afa22489532d314b9c
2017-05-01 12:34:46 -07:00
Janis Danisevskis
ccfff10f66 Remove use of UniquePtr from keystore
Remove UniquePtr from keystore in favour of std::unique_ptr

Change-Id: I8e02adab4326028e26dbf59ac836679abe2a40de
2017-05-01 12:34:46 -07:00
TreeHugger Robot
48dc4ad3b9 Merge "Do not clear critical keys in clear_uid()" into oc-dev 2017-04-27 15:09:47 +00:00
Rubin Xu
85c85e9840 Do not clear critical keys in clear_uid()
If clear_uid() is called on system uid, skip clearing keys with
FLAG_CRITICAL_TO_DEVICE_ENCRYPTION flag since device authenticaion
would be broken without them.

Bug: 34600579
Test: Add device lock under synthtic password, goto Settings/security/encryption,
      tap clear credentials and verify device lock is still intact.

Change-Id: I6c009163831b0901b0973d13906f56139028052c
2017-04-26 20:07:30 +01:00
Bartosz Fabianowski
5aa93e08a8 Add device ID attestation method to keymaster
Device ID attestation consists of three steps:
* Generate a temporary key
* Attest the key and desired device IDs
* Delete the temporary key

Rather than being spread over three keymaster APIs, these operations
should happen automatically in a single keymaster method.

Bug: 34734938
Test: GTS com.google.android.gts.security.DeviceIdAttestationHostTest

Change-Id: Icbbc2dfc84f8b4f39d0e7ea880844d4f38b63f66
2017-04-26 10:51:24 +02:00
Bartosz Fabianowski
e0256a8dbb Fix temporary key deletion after device ID attestation
After device ID attestation has been performed, the key used for it
should be deleted. Calling directly into the keymaster HAL for this
is wrong as it removes the key from keymaster but still leaves the
actual key matter around and the alias visible to KeyStore.

The key should be deleted using the KeyStore's delete method.

Bug: 37522655
Test: GTS DeviceIdAttestationHostTest

Change-Id: If3da9913fb54b077d3471f82269341966b1687e4
2017-04-26 10:50:54 +02:00
TreeHugger Robot
e3b7240bf5 Merge "Revert "Fix temporary key deletion after device ID attestation"" into oc-dev 2017-04-25 19:44:09 +00:00
Dan Stoza
3afff91fb7 Revert "Fix temporary key deletion after device ID attestation"
This reverts commit a65ab425aa.

Reason for revert: Breaks Ryu build

Change-Id: I57c760b561cbbfcb1f0191e8309fc678461dba36
2017-04-25 17:23:16 +00:00
TreeHugger Robot
211dcefb77 Merge "Fix temporary key deletion after device ID attestation" into oc-dev 2017-04-25 16:02:07 +00: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
Bartosz Fabianowski
a65ab425aa Fix temporary key deletion after device ID attestation
After device ID attestation has been performed, the key used for it
should be deleted. Calling directly into the keymaster HAL for this
is wrong as it removes the key from keymaster but still leaves the
actual key matter around and the alias visible to KeyStore.

The key should be deleted using the KeyStore's delete method.

Bug: 37522655
Test: GTS DeviceIdAttestationHostTest

Change-Id: I10b27216642893d2a1cf8407f65eb4207bcde1f5
2017-04-20 04:41:26 +02: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
TreeHugger Robot
ead11aa0e1 Merge "Fix unique ID attestation." into oc-dev 2017-04-13 21:25:57 +00:00
Shawn Willden
e2a7b52849 Fix unique ID attestation.
Test: CTS test will be added.
Bug: 34671471
Change-Id: I2f36b85ba7a46e7aabe83b8e0c58a8092ee1f643
2017-04-11 11:48:50 -06:00
Jiyong Park
ecb258e775 fix: wifi doesn't work on the generic system image
libkeystore-wifi-hidl and libkeystore-engine-wifi-hidl are required by
/vendor/bin/hw/wpa_supplicant. They are installed to /system partition.
This does not cause any problem as long as both /system and /vendor
partitions are built for the same target product, as we do for most of
our products.

However, it becomes a problem when we build only the /system partition
for the generic AOSP system.img. In that case, the libs are not
installed to the partition since we don't build vendor image for the
target and thus wpa_supplicant (and its dependencies as well) aren't
on the list of dmoules to be built/installed.

Moving them to vendor partition by adding LOCAL_VENDOR_MODULE := true
solves the problem.

Bug: 37126829
Test: basic functionalities of wifi work on marlin/sailfish with
system.img built from aosp_arm64_ab.

Change-Id: I783756a5848786b15c1f227f06a1ee2e291d3ce9
2017-04-11 09:12:00 +09:00
Steven Moreland
4cb6f38017 Fix transitive include.
Was relying on include from MQDescriptor.h

Test: pass
Change-Id: Ic3f24fea3875ed1f598b18e4a1fa05c226a86037
2017-04-06 12:41:59 -07:00
Roshan Pius
6d38eda694 keystore: Allow wifi keystore HAL to lookup wifi blobs
Since wifi keystore HAL service runs inside keystore daemon, allow it to
proxy wifi queries.

Bug: 34603782
Test: Able to connect to wifi passpoint networks.
Change-Id: I005c3850a8dd1b14e3f935d6f68def880061c140
2017-04-03 13:49:57 -07:00
Roshan Pius
e653c93db1 keystore: Run Wifi keystore HAL in keystore daemon
The wifi keystore hal will run in the context of the main keystore
daemon.

Also,
Use the new IKeystore::tryGetService() for retrieveing the HAL service.

Bug: 34603782
Test: Able to connect to wifi passpoint networks.

Change-Id: I1436ea83166e5ad17372d98b0fd699c0dd732a11
2017-03-30 13:04:46 -07:00
Paul Stewart
bf7fc8df76 Add a HIDL-based keystore_get variant
Create a "keystore_get" library that uses the HIDL path insted
of using binder directly.

Bug: 34603782
Test: Able to connect to wifi passpoint networks.
Change-Id: I0f545ea104e3f27bebd262bc5a2e79f6b517c972
2017-03-29 11:29:43 -07:00
Shawn Willden
d3ed3a2079 Revert "Delegate auth token parsing to HAL."
This reverts commit 76f21b2676.

Reason for revert: b/36637075

Bug: 36637075
Change-Id: Ica737cf96d14086aae7918f8bf2f86a36555d03b
2017-03-28 00:44:33 +00:00
Shawn Willden
76f21b2676 Delegate auth token parsing to HAL.
Auth tokens have an unfortunate dual character. To most of the system
they are opaque blobs that are intended only to be obtained from one
HAL (e.g. gatekeeper or fingerprint) and passed to another
HAL (keymaster), but keystore actually needs to extract some bits of
information from them in order to determine which of the available blobs
should be provided for a given keymaster key operation.

This CL adds a method that resolves this dual nature by moving the
responsibility of parsing blobs to the HAL so that no component of the
framework has to make any assumptions about their content and all can
treat them as fully opaque. This still means that the various HAL
implementers have to agree on content, but they also have to agree on an
HMAC key which much be securely distributed to all at every boot, so
asking them to agree on an auth token format is perfectly
acceptable. But now the Android system doesn't have to care about the
format.

Bug: 32962548
Test: CTS tests pass, plus manual testing.
Change-Id: I2ab4b4fbea1425fc08aa754fc10f8e386899af25
2017-03-24 22:23:56 -06:00
Shawn Willden
b8550a0929 Add digest support and implementation name to getHardwareFeatures
Test: Manual
Change-Id: Iee20528e8d4f3931164aa988e11bfe71be4f56dc
2017-03-24 20:02:41 -06:00
TreeHugger Robot
30038a601c Merge "Fixes KeyStore::isHardwareBacked" 2017-03-23 16:23:06 +00:00
TreeHugger Robot
45d2b3cc93 Merge "Add manufacturer and model to device ID attestation" 2017-03-23 00:01:39 +00:00
Steven Moreland
5fa8af3803 keystore: remove unused using statement
Test: pass
Bug: 36099713
Change-Id: I8f7e1559ce2dafbffee3d7c36083bf9a6307917f
2017-03-20 07:34:09 -07:00
Bartosz Fabianowski
634a1aac20 Add manufacturer and model to device ID attestation
Discussions have shown that in addition to brand, device and product,
we should also allow devices to attest their manufacturer and model.

Bug: 36433192
Test: GTS com.google.android.gts.security.DeviceIdAttestationHostTest

Change-Id: I28ee51d9f95c3e4efb8932f3c9b899082eb62e55
2017-03-20 14:02:36 +01:00
Rubin Xu
7675c9f2c7 Add logging to keystore
Add extra logging to keystore in places where a key entry gets
deleted. This is to assist investigating a mysterious key missing
bug in keystore.

Bug: 35929605
Test: None
Change-Id: I423b401b0c411e20a8f0f631ffdcea74c4173961
2017-03-16 12:05:14 +00:00
Janis Danisevskis
e2b6caff1a Fixes KeyStore::isHardwareBacked
KeyStore::isHardwareBacked was broken by the hidlization of the
Keymaster HAL. This patch implements the functionality by quarrying
the Keymaster HAL for hardware features.

Bug: 35866007
Change-Id: I237e29a8b1c1b93a88b9fa8a969c3c832af384ff
2017-03-02 16:37:10 -08:00
Chris Phoenix
9b3791caa8 keymaster HAL uses "default" service name
The getService() and registerAsService() methods of interface objects
now have default parameters of "default" for the service name. HALs
will not have to use any service name unless they want to register
more than one service.

Test: marlin boots

Bug: 33844934
Change-Id: I8ba24cd078dc5d36ecf02ddc0febc745b98d5a95
2017-02-24 14:31:57 -08:00
Janis Danisevskis
e8ba1802a6 Phase out keymaster fallback support
Keystore uses two different keymaster devices.
One device is provided by the OEM providing
hardware/trust zone backed functionality. The other
is a pure software implementation of keymaster.
The latter was used when a "hardware" implementation
failed generating or importing keys with certain
parameters.

This tolerance of misbehaving "hardware" implementations
had the effect that this behavior has done unnoticed for
too long. Therefore, we are phasing out the fallback
device.

This patch ensures that on devices with hardware
implementations supporting keymaster 2.0 and higher
there will be no fallback device papering over failures
in the underlying keymaster implementation.

Test: given a faulty KM2.0 implementation, import and generation
      of keys with otherwise supported parameters returns an error

Change-Id: I8c2118e72558c326031368df13e836c3ef6b1da1
2017-01-30 11:49:14 +00:00
Janis Danisevskis
69c434aee7 Fix keystore::del to use correct keymaster device.
Keystore uses two different keymaster devices.
One device is provided by the OEM providing
hardware/trust zone backed functionality. The other
is a pure software implementation of keymaster.
The latter was used when a "hardware" implementation
failed generating or importing keys with certain
parameters.

During the port to HIDL based HALs this fallback software
device was removed and later reinstated. The delete
function of keystore, however was left unaware of the
existence of the fallback device and passed do-be-deleted
"fallback"-keys to the hardware device which is unaware
of the format and fails.

This patch makes Keystore::del aware of the fallback
device.

Test: import and delete a key that is unsupported by the
      underlying "hardware" implementation of keymaster

Change-Id: I011c19c515b4b41fedec0c21f89efb58279f297c
2017-01-30 10:27:10 +00:00
Janis Danisevskis
b0245eef46 Fix passing wrong set of parameters in keystore
In keystore update, the wrong set of parameters was passed
to keymaster, thus ommiting the auth token. This broke
authenticated key operations.

Test: FingerprintDialog.apk
Bug: 34692128
Change-Id: I1db8df962ccc5c3df948a9ac3e3b8ada1f48e84e
2017-01-25 15:43:01 +00:00
Bartosz Fabianowski
a9452d92ca Add device id attestation
This adds device id attestation to KeyStoreService. The service
validates that the user holds the required permissions before
allowing attestation to proceed.

Bug: 34597337
Test: CTS CtsKeystoreTestCases and GTS DeviceIdAttestationHostTest

Change-Id: I6ff6146fad4656b8e1367650de922124b3d7f7b2
2017-01-25 03:55:15 +01:00
Janis Danisevskis
25daf8b2d6 Fix auth token conversion dropping a byte
Test: TestDPC
Bug: 34656122
Change-Id: I000b2115e7c7764de18e1f3b3e3d5c16ca4b9443
2017-01-24 19:01:51 +00: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
Janis Danisevskis
668cd308b7 Merge "fix is_secret_key_operation check" am: 7f35beb6cb am: 216b33f09c am: 4623971442
am: a3097ff932

Change-Id: If93e31e54333abbe1d8f8844f87c177ce2845473
2017-01-04 16:52:33 +00:00