Commit graph

44 commits

Author SHA1 Message Date
Janis Danisevskis
f6f522c525 Merge "Fix strict weak ordering requirement of less than operation" am: e82263dd74 am: 36b364abfb
am: 22368369f7

Change-Id: I2301e7fec1c5c28516dafff483a8a0f2a2e00b0a
2019-03-26 09:34:44 -07:00
Janis Danisevskis
c7a8b863cd Keymaster support: Verbose vendor errors
Added function for verbosely logging Keymaster vendor errors.

Bug: 123562864
Test: atest android.keystore.cts
Change-Id: Ida093941d3b76b3d2e953439229081345909c16b
2019-03-20 16:13:53 +00:00
Janis Danisevskis
93c7276e3a Fix strict weak ordering requirement of less than operation
operator< on hidl_vec<uint8_t> violates strict weak ordering in the case
that one oparand is shorter that the other and the shorter is a prefix
of the longer.

if x and y are incomparable, i.e., neither x < y nor y < x and
   y and z are incomparable, i.e., neither y < z nor z < y, then
   x and z must be incomparable.
As for the current implementation the first two statements are true but
the third is not given the following example input:
x:="aa", y:="a", z:="ab".

This patch fixes the issue by defining a < b if a is a prefix of b.

As this relation is used in a std::sort algorithm which demands strict
weak ordering this bug leads to undefined behavior.

Change-Id: I4961bb35e2fd4f5fcf561ec0c7c536f81830aab8
2019-03-19 09:54:04 -07:00
Yi Kong
45cb85f8c0 Suppress null-dereference warning
It is unclear whether author intentionally meant to cause segfault here.
While waiting for the author to explain/fix the code, suppress the
warning to unblock enabling the warning globally.

Test: m checkbuild
Bug: 121390225
Change-Id: Iad03842833cfdc243404a32f6b31d161387c3890
2018-12-21 14:52:47 -08:00
Elliott Hughes
d9de6aa270 C++17 compatibility: add a non-const char* overload.
C++17 adds a non-const std::basic_string::data, so non-const std::strings in the
test are `char*` and the const std::strings are `const char*`. See
https://en.cppreference.com/w/cpp/string/basic_string/data for details.

Without adding the non-const overload, the varargs overload is preferred, leading
to static_assert failures:

  In file included from hardware/interfaces/keymaster/3.0/vts/functional/keymaster_hidl_hal_test.cpp:33:
  In file included from hardware/interfaces/keymaster/3.0/vts/functional/authorization_set.h:20:
  hardware/interfaces/keymaster/3.0/vts/functional/keymaster_tags.h:257:5: error: static_assert failed "Authorization other then TagType::BOOL take exactly one parameter."
  static_assert(tag_type == TagType::BOOL || (sizeof...(args) == 1),
  ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  hardware/interfaces/keymaster/3.0/vts/functional/authorization_set.h:213:19: note: in instantiation of function template specialization 'android::hardware::keymaster::V3_0::Authorization<android::hardware::keymaster::V3_0::TagType::BYTES, android::hardware::keymaster::V3_0::Tag::ASSOCIATED_DATA, char *, unsigned long>' requested here
  push_back(Authorization(tag, std::forward<Value>(val)...));
  ^
  hardware/interfaces/keymaster/3.0/vts/functional/authorization_set.h:245:9: note: in instantiation of function template specialization 'android::hardware::keymaster::V3_0::AuthorizationSet::push_back<android::hardware::keymaster::V3_0::TypedTag<android::hardware::keymaster::V3_0::TagType::BYTES, android::hardware::keymaster::V3_0::Tag::ASSOCIATED_DATA>, char *, unsigned long>' requested here
  push_back(ttag, std::forward<ValueType>(value)...);
  ^
  hardware/interfaces/keymaster/3.0/vts/functional/keymaster_hidl_hal_test.cpp:3426:35: note: in instantiation of function template specialization 'android::hardware::keymaster::V3_0::AuthorizationSetBuilder::Authorization<android::hardware::keymaster::V3_0::TypedTag<android::hardware::keymaster::V3_0::TagType::BYTES, android::hardware::keymaster::V3_0::Tag::ASSOCIATED_DATA>, char *, unsigned long>' requested here
  AuthorizationSetBuilder().Authorization(TAG_ASSOCIATED_DATA, aad.data(), aad.size());
  ^

Bug: http://b/111067277
Test: builds
Change-Id: I3d70fb5a41db16cc9dff50364cd793e0c3510ed0
2018-11-27 16:40:33 +00:00
Janis Danisevskis
91a01c5cfc Merge "Removed unsafe use of hidl_vec<>.setToExternal" 2018-11-15 01:19:10 +00:00
Janis Danisevskis
8f45a1c5c3 keymaster: fix authorization set serialization
Invalid and unknown tags were treated as zero size but they where still
counted as entry. This lead to invalid tags being persisted. When
Serialized blobs were used to cache key characteristics, these invalid
tags were send to clients of keystore. However, the serialization cannot
cope with invalid tags.

Bug: 119414176
Test: Successfully used the Skype app which triggered the problem
Change-Id: Ia46ac4a16395db3d10f93d3722eda69d523db478
2018-11-13 13:21:30 -08:00
Janis Danisevskis
28a6b79f4b authorization_set.cpp: relax serialization of unknown tags
Bug: 119414176
Change-Id: I16722f2a2b1a00a352322c603d2bf18a996d6ee7
2018-11-12 12:06:32 -08:00
Janis Danisevskis
9c41221206 Removed unsafe use of hidl_vec<>.setToExternal
hidl_vec objects that do not own their associated buffer are highly
unsafe in multithreaded environments where move semantic is used to
transfer ownership between threads. With keystore transitioning to a
multi threaded execution model we can no longer use this optimization
safely.

Bug: 111443219
Test: Ran full keystore cts test suite.
Change-Id: I9a366fc7df5dfee508dc092855545963ef6d9665
2018-11-09 10:49:55 -08:00
Janis Danisevskis
2ecd6597f3 Various fixes for async keystore.
* Added missing Tag::HARDWARE_TYPE and Tag::TRUSTED_CONFIRMATION_REQUIRED
* Made AuthorizationSet::hidl_data() safer to use.
  hidl_data() initializes a hidl_vec with the internal data of
  std::vector using setToExternal and returns it by value. This means
  the returned temporay does not own the buffer which has the life cycle
  of the AuthorizationSet. This is fine if passed as parameter to a
  function where it is bound to a cont reference. But if the temporary
  gets assigned to something with longer life cycle move semantics kicks
  in and the buffer is now tracked by something with a longer life
  cycle. This patch marks the returned temporary const, so that it can
  no longer be moved. It can still be bound to a const reference, but
  when assigned to a variable it must get copied.
* Add Filter function to AuthorizationSet.

Bug: 111443219
Test: KeyStore CTS tests
Change-Id: I4744b7c87d01fbd905c3afb8ebeefba93605994b
2018-11-07 11:32:03 -08:00
Chih-Hung Hsieh
8cca54bec0 Add noexcept to move constructors and assignment operators.
am: 19a5da4c13

Change-Id: Ib5b417deddc9af28a2de012e379f55d869053ec4
2018-10-01 16:13:05 -07:00
Chih-Hung Hsieh
19a5da4c13 Add noexcept to move constructors and assignment operators.
Bug: 116614593
Test: build with WITH_TIDY=1
Change-Id: Ib50ced82d650737cf99a9757133119945a3409f3
2018-10-01 20:30:38 +00:00
Yi Kong
fa8dfc724e Merge "Don't use initializer_list as return type"
am: 9c6b9bf7c3

Change-Id: Ided6c9a8952938912bf036b4c83544a568088e42
2018-09-24 14:32:46 -07:00
Yi Kong
7392175ccd Don't use initializer_list as return type
The underlying array may be cleaned up once its lifetime has ended,
the initializer_list would become ill-formed. Return as std::vector
instead.

This fixes "-Wreturn-stack-address" (clang) / "-Winit-list-lifetime"
(gcc) warning.

Test: mma
Bug: 111998531
Change-Id: Ie5bb6bc3d0d7689744fd573c5683b22e6fb6b178
2018-09-21 15:36:57 -07:00
Shawn Willden
7b00c75643 Fix attestation test.
Bug: 77588764
Bug: 80246122
Test: VtsHalKeymasterV4_0TargetTest
Change-Id: Ibe264d08ae7b3333a6949761a92759f5305b3fcb
2018-08-17 06:51:06 -06:00
Shawn Willden
d898d0a422 Fix attestation test.
Bug: 77588764
Test: VtsHalKeymasterV4_0TargetTest
Change-Id: Ibe264d08ae7b3333a6949761a92759f5305b3fcb
2018-06-25 06:36:02 -06:00
Janis Danisevskis
a1c4e0ec5d Relax HMAC computation check
This KM4 key agreement check is causing some pain on early units
that aren't completely provisioned in both locked and non-Green
(unlocked) states.

This doesn't impact KM3 devices (Pixel 2016/2017 etc.)

Bug: 110301629
Change-Id: I5a737ac8a335863b1099c29cf3c0496adeb41e15
2018-06-20 05:25:22 +00:00
Shawn Willden
6dad2b3a4b Activate HMAC sharing check.
This had to be disabled because Qualcomm's keymaster4 returned a bad
value.

Bug: 77588764
Bug: 79698245
Test: VtsHalKeymasterV4_0TargetTest
Change-Id: Ieb150d7f17c36f01acf2eeb665792594251b51ae
2018-05-23 05:44:42 -06:00
Shawn Willden
f0f05d4052 Add utility method to perform HMAC agreement
To make it easier for clients (vold & keystore) to perform key
agreement, this CL adds a service method that does it.  To make key
agreement consistent, this method sorts the HMAC sharing parameters
lexicographically.  The requirement for sorting is documented in the
HAL.

Test: Boot device
Bug: 79307225
Bug: 78766190
Change-Id: Idb224f27f8e4426281d9a0105605ba22bf7c7e95
2018-05-10 18:28:51 -06:00
nagendra modadugu
9c36c91945 keymaster: provide instance name to getService()
Bug: 38430282
Test: VtsHalKeymasterV3_0TargetTest pass with exception
  of (AesEcbWithUserId, RsaAttestation, EcAttestation)
  which are expected failures.

Change-Id: I48e7195f512190deb608f1a69783c92254eef1aa
2018-03-30 18:31:35 -07:00
Brian Young
3f48322658 Add "Unlocked device required" key API
Add a keymaster parameter for keys that should be inaccessible when
the device screen is locked. "Locked" here is a state where the device
can be used or accessed without any further trust factor such as a
PIN, password, fingerprint, or trusted face or voice.

This parameter is added to the Java keystore interface for key
creation and import, as well as enums specified by and for the native
keystore process.

Test: CTS tests in I8a5affd1eaed176756175158e3057e44934fffed

Bug: 67752510

Merged-In: Id19d19b19532ac8d4c52aad46a954faa4515289d
Change-Id: Id19d19b19532ac8d4c52aad46a954faa4515289d
(cherry picked from commit 1840be6d35)
2018-03-28 08:38:56 -07:00
Brian Young
f67e953919 Revert "Restore "Add "Unlocked device required" parameter to keys""
This reverts commit 97e02689d9.

Reason for revert: Regression in creating auth-bound keys

Bug: 73773914

Bug: 67752510

Change-Id: I8ccba28580099c4c533f53b0be92f1d607ce63c6
2018-02-23 01:31:40 +00:00
Brian C. Young
97e02689d9 Restore "Add "Unlocked device required" parameter to keys"
Add a keymaster parameter for keys that should be inaccessible when
the device screen is locked. "Locked" here is a state where the device
can be used or accessed without any further trust factor such as a
PIN, password, fingerprint, or trusted face or voice.

This parameter is added to the Java keystore interface for key
creation and import, as well as enums specified by and for the native
keystore process.

This reverts commit 95b60a0f41.

Test: CTS tests in I8a5affd1eaed176756175158e3057e44934fffed

Bug: 67752510

Change-Id: I2893c23ab173ff5c39085d56b555e54770900cbc
2018-02-15 11:19:40 -08:00
Jorim Jaggi
62df80180b Merge "Revert "Add "Unlocked device required" parameter to keys"" 2018-01-30 15:36:28 +00:00
Brian Young
95b60a0f41 Revert "Add "Unlocked device required" parameter to keys"
This reverts commit 5fe872413b.

Reason for revert: Build breakages on elfin, gce_x86_phone.

Bug: 72679761
Bug: 67752510
Change-Id: I2857b2a9b6ff26735bd4989a36c5e5deb4953904
2018-01-30 15:31:19 +00:00
TreeHugger Robot
ea52a4d3b8 Merge "Add "Unlocked device required" parameter to keys" 2018-01-29 23:16:02 +00:00
Shawn Willden
98b998b59a Support library enhancements, to ease transition of vold to KM4
Keymaster clients need to see all the available devices and figure out
which they want to use.  This method finds them all and returns them
in a vector sorted from most secure to least, according to a heuristic
defined in Keymaster::VersionResult::operator<

This CL also makes a few other minor improvements to the support
library, providing more information in VersionResult and adding some
more convenience methods in AuthorizationSetBuilder.

Test: Build & boot
Change-Id: I876238ee9ff72573c30d60e1cec665dd610bcde6
2018-01-25 22:38:56 -07:00
Brian C. Young
5fe872413b Add "Unlocked device required" parameter to keys
Add a keymaster parameter for keys that should be inaccessible when
the device screen is locked. "Locked" here is a state where the device
can be used or accessed without any further trust factor such as a
PIN, password, fingerprint, or trusted face or voice.

This parameter is added to the Java keystore interface for key
creation and import, as well as enums specified by and for the native
keystore process.

Test: go/asym-write-test-plan

Bug: 67752510

Change-Id: I466dfad3e2e515c43e68f08e0ec6163e0e86b933
2018-01-25 10:18:21 -08:00
TreeHugger Robot
406406fb90 Merge changes from topic "tui_keystore"
* changes:
  Add Trusted Confirmation support to Keymaster HAL.
  Sort tags in keymaster_tags.h alphabetically
2018-01-25 17:31:38 +00:00
Shawn Willden
129629bde4 Add Trusted Confirmation support to Keymaster HAL.
Bug: 63928580
Test: VtsHalKeymasterV4_0TargetTest

Change-Id: I402be6f182f7f375493334d5e000fec23f3551f6
2018-01-24 10:19:10 -08:00
Janis Danisevskis
83509cd758 Sort tags in keymaster_tags.h alphabetically
Test: No functional changes
Change-Id: I49c5632b5dae1f24634e99eb71a9471e91275fbd
2018-01-24 10:19:10 -08:00
Shawn Willden
b9be9ded26 Add support for BOOT and VENDOR patch levels to keymaster.
Bug: 68250869
Test: Manual.  VTS testing is not possible.
Change-Id: Ifa2025ce31592dbeb274ee3a2c300a7de416ae1f
2018-01-23 10:21:06 -07:00
TreeHugger Robot
af4d761cf5 Merge "Add additional parameters to importWrappedKey" 2018-01-22 20:18:17 +00:00
TreeHugger Robot
e541981ac2 Merge "Add VerificationToken tests." 2018-01-20 03:25:18 +00:00
Shawn Willden
8d28efa9b8 Add additional parameters to importWrappedKey
Bug: 31675676
Test: VtsHalKeymasterV4_0TargetTest
Change-Id: I31166d0c562d92bbdcf3357782ac2a076a1bc2d9
2018-01-19 20:09:05 -07:00
Shawn Willden
4fbc1d574b Add VerificationToken tests.
Bug: 70409878
Test: VtsHalKeymasterV4_0TargetTest
Change-Id: I5458729ef8c3494f45fe8274b391133b997d43f2
2018-01-20 02:54:23 +00:00
Shawn Willden
256929827a Move KeyParameter operator== to support lib.
Test: VtsHalKeymasterV4_0TargetTest
Change-Id: I4b71a9fbd986c1bd1001e3ab49de5d360b303b27
2018-01-19 09:44:11 -07:00
Shawn Willden
0555ddd6ba Fix build breakage
Previous CLs to move keymaster wrappers broke the build (but somehow
not in my tree, nor in TreeHugger's build).

Test: Build
Change-Id: I0494e1e38ee7e8806f3758d533b6b1e3a6c576d1
2018-01-18 19:12:53 -07:00
Shawn Willden
7d339812c9 Move Keymaster wrapper into support library.
This wrapper was used to manage KM3/KM4 compatibility in keystore.
It's also needed in vold, so this CL moves it here, to make it usable
for vold.

Test: keystore CTS tests
Change-Id: I8079b8577f7d4a8fd67f47fbe1f48861e4a0734b
2018-01-18 15:39:50 -07:00
Shawn Willden
8823a4415c Add support for 3DES algorithm to Keymaster.
Test: VtsHalKeymasterV4_0TargetTest
Bug: 31675676
Change-Id: I68a67b78979002a38e92454f79715ed516026889
2018-01-17 14:15:38 -07:00
Shawn Willden
a6eb3faeb5 Remove references to Keymaster::3.0 from Keymaster::4.0
Test: VtsHalKeymasterV4_0TargetTest
Change-Id: Idf627a3d7a51d2a464bd1723a32e88f43969bf45
2018-01-04 15:05:36 -07:00
Shawn Willden
9e0c1fe534 Add support for StrongBox implementations to Keymaster HAL
Also adds secure key import.

Bug: 63931634
Test: not yet
Change-Id: I54f38a8787e2fcb51e01f378228e4a0c576fdfbe
2017-12-12 07:14:45 -07:00
Shawn Willden
32aa7ecb55 Rename IKeymaster back to IKeymaserDevice
Test: VtsHalKeymasterV4_0TargetTest
Change-Id: Ic5e7f936ef75aa64834677f6589822fbedce39cd
2017-11-30 19:37:07 -07:00
Shawn Willden
1e50c676f6 Add Keymaster V4.0
This CL merely duplicates all of the Keymaster V3.0 functionality and
VTS tests, and provides a pure software implementation of the 4.0 HAL,
which passes the VTS tests.  Future CLs will remove some cruft and
unused features, then add new features and accompanying tests.

Note that the reason that this is V4.0 rather than V3.1 is because V4.0
will not be fully backward compatible with V3.0.  Specifically, V4.0
will allow for "StrongBox" implementations, which will only provide a
subset of Keymaster functionality.  StrongBox versions of Keymaster will
be implemented in discrete, special-purpose hardware which will
generally be much less powerful (slower, less RAM, etc.) than is needed
to support a full Keymaster implementation.

So, while the V4.0 interface will be a strict superset of the V3.0
interface, which could normally be best implemented as an extension, it
will allow StrongBox implementations which are unable to pass the V3.0
test suite, which means that it will not be true that a V4.0
impementation IS-A V3.0 implementation, as would be expected of a V3.1
implementation.  The V4.0 test suite will distinguish between StrongBox
and non-StrongBox implementations and enforce appropriately-reduced
requirements on the former.

In addition to the duplication, 4.0 also cleans up some cruft from 3.0:

  - Removes tags and types which were in previous versions but never
    used;
  - Removes support for wrapping pre-Treble keymaster HALs with KM4,
    since they'll only be wrapped by the default KM3 implementation;
  - Renames the ROLLBACK_RESISTANT tag to ROLLBACK_RESISTANCE and
    defines new semantics for it;
  - Changes auth token handling to use the HardwareAuthToken struct
    passed in as an explicit argument to the relevant methods,
    rather than an opaque byte vector provided as a KeyParameter;
  - Updates the VTS tests to use a gtest "environment" for better
    integration with VTS test infrastructure;
  - Adds a test for upgradeKey.
  - Makes comment formatting more consistent, including using the
    correct two-space typographical convention to separate sentences.

Bug: 63931634
Test: VtsHalKeymasterV4_0TargetTest
Change-Id: I3f01a4991beaa5c4332f72c91e8878a3bf0dec67
2017-11-28 13:41:52 -07:00