From fe584fb317bc7e22d88c97629679a875ee5d9433 Mon Sep 17 00:00:00 2001 From: Janis Danisevskis Date: Mon, 23 Apr 2018 08:56:16 -0700 Subject: [PATCH] Fix wrongly initialized test key. According to spec the test key has 32 bytes set to TextKeyBits::BYTE. The VTS test and default implmementation only set the first 16 bytes and left the remaining 16 bytes zero. This bug fixes both, the VTS test and the default implementation. Also some long overdue changes to how the auth token key is handled by the generic operation. Bug: 78456249 Test: VtsHalConfirmationUIV1_0TargetTest Change-Id: I509f2c2a99704ee00625e6f6169479771a3bc17a --- .../1.0/default/PlatformSpecifics.cpp | 8 +++---- .../1.0/default/PlatformSpecifics.h | 5 ++-- .../VtsHalConfirmationUIV1_0TargetTest.cpp | 23 ++++++++----------- .../1.0/generic/GenericOperation.h | 17 ++++++-------- .../support/confirmationui_utils.h | 15 ++++++++++-- 5 files changed, 36 insertions(+), 32 deletions(-) diff --git a/confirmationui/1.0/default/PlatformSpecifics.cpp b/confirmationui/1.0/default/PlatformSpecifics.cpp index dd039e22ba..03d6165471 100644 --- a/confirmationui/1.0/default/PlatformSpecifics.cpp +++ b/confirmationui/1.0/default/PlatformSpecifics.cpp @@ -36,11 +36,11 @@ MonotonicClockTimeStamper::TimeStamp MonotonicClockTimeStamper::now() { } } -support::NullOr> HMacImplementation::hmac256( - const uint8_t key[32], std::initializer_list buffers) { +support::NullOr HMacImplementation::hmac256( + const support::auth_token_key_t& key, std::initializer_list buffers) { HMAC_CTX hmacCtx; HMAC_CTX_init(&hmacCtx); - if (!HMAC_Init_ex(&hmacCtx, key, 32, EVP_sha256(), nullptr)) { + if (!HMAC_Init_ex(&hmacCtx, key.data(), key.size(), EVP_sha256(), nullptr)) { return {}; } for (auto& buffer : buffers) { @@ -48,7 +48,7 @@ support::NullOr> HMacImplementation::hmac256( return {}; } } - support::array result; + support::hmac_t result; if (!HMAC_Final(&hmacCtx, result.data(), nullptr)) { return {}; } diff --git a/confirmationui/1.0/default/PlatformSpecifics.h b/confirmationui/1.0/default/PlatformSpecifics.h index 488da6d1b7..29f299c7d6 100644 --- a/confirmationui/1.0/default/PlatformSpecifics.h +++ b/confirmationui/1.0/default/PlatformSpecifics.h @@ -48,8 +48,9 @@ struct MonotonicClockTimeStamper { class HMacImplementation { public: - static support::NullOr> hmac256( - const uint8_t key[32], std::initializer_list buffers); + static support::NullOr hmac256( + const support::auth_token_key_t& key, + std::initializer_list buffers); }; class MyOperation : public generic::Operation, diff --git a/confirmationui/1.0/vts/functional/VtsHalConfirmationUIV1_0TargetTest.cpp b/confirmationui/1.0/vts/functional/VtsHalConfirmationUIV1_0TargetTest.cpp index 463bb40a15..278d1f4440 100644 --- a/confirmationui/1.0/vts/functional/VtsHalConfirmationUIV1_0TargetTest.cpp +++ b/confirmationui/1.0/vts/functional/VtsHalConfirmationUIV1_0TargetTest.cpp @@ -46,13 +46,16 @@ namespace V1_0 { namespace test { namespace { +const support::auth_token_key_t testKey(static_cast(TestKeyBits::BYTE)); + class HMacImplementation { public: - static support::NullOr> hmac256( - const uint8_t key[32], std::initializer_list buffers) { + static support::NullOr hmac256( + const support::auth_token_key_t& key, + std::initializer_list buffers) { HMAC_CTX hmacCtx; HMAC_CTX_init(&hmacCtx); - if (!HMAC_Init_ex(&hmacCtx, key, 32, EVP_sha256(), nullptr)) { + if (!HMAC_Init_ex(&hmacCtx, key.data(), key.size(), EVP_sha256(), nullptr)) { return {}; } for (auto& buffer : buffers) { @@ -60,7 +63,7 @@ class HMacImplementation { return {}; } } - support::array result; + support::hmac_t result; if (!HMAC_Final(&hmacCtx, result.data(), nullptr)) { return {}; } @@ -70,23 +73,15 @@ class HMacImplementation { using HMacer = support::HMac; -constexpr uint8_t testKeyByte = static_cast(TestKeyBits::BYTE); - template hidl_vec testHMAC(const Data&... data) { - constexpr uint8_t testKey[32] = {testKeyByte, testKeyByte, testKeyByte, testKeyByte, - testKeyByte, testKeyByte, testKeyByte, testKeyByte, - testKeyByte, testKeyByte, testKeyByte, testKeyByte, - testKeyByte, testKeyByte, testKeyByte, testKeyByte}; - constexpr uint8_t hmac_size_bytes = sizeof testKey; - auto hmac = HMacer::hmac256(testKey, data...); if (!hmac.isOk()) { EXPECT_TRUE(false) << "Failed to compute test hmac. This is a self-test error."; return {}; } - hidl_vec result(hmac_size_bytes); - copy(hmac.value().data(), hmac.value().data() + hmac_size_bytes, result.data()); + hidl_vec result(hmac.value().size()); + copy(hmac.value().data(), hmac.value().data() + hmac.value().size(), result.data()); return result; } diff --git a/confirmationui/support/include/android/hardware/confirmationui/1.0/generic/GenericOperation.h b/confirmationui/support/include/android/hardware/confirmationui/1.0/generic/GenericOperation.h index b480942998..b1c322ce53 100644 --- a/confirmationui/support/include/android/hardware/confirmationui/1.0/generic/GenericOperation.h +++ b/confirmationui/support/include/android/hardware/confirmationui/1.0/generic/GenericOperation.h @@ -99,7 +99,8 @@ class Operation { void setPending() { error_ = ResponseCode::OK; } - void setHmacKey(const uint8_t (&key)[32]) { hmacKey_ = {key}; } + void setHmacKey(const auth_token_key_t& key) { hmacKey_ = key; } + NullOr hmacKey() const { return hmacKey_; } void abort() { if (isPending()) { @@ -112,7 +113,7 @@ class Operation { if (isPending()) error_ = ResponseCode::Canceled; } - void finalize(const uint8_t key[32]) { + void finalize(const auth_token_key_t& key) { if (error_ == ResponseCode::Ignored) return; resultCB_->result(error_, getMessage(), userConfirm(key)); error_ = ResponseCode::Ignored; @@ -127,11 +128,7 @@ class Operation { } ResponseCode deliverSecureInputEvent(const HardwareAuthToken& secureInputToken) { - constexpr uint8_t testKeyByte = static_cast(TestKeyBits::BYTE); - constexpr uint8_t testKey[32] = {testKeyByte, testKeyByte, testKeyByte, testKeyByte, - testKeyByte, testKeyByte, testKeyByte, testKeyByte, - testKeyByte, testKeyByte, testKeyByte, testKeyByte, - testKeyByte, testKeyByte, testKeyByte, testKeyByte}; + const auth_token_key_t testKey(static_cast(TestKeyBits::BYTE)); auto hmac = HMacer::hmac256(testKey, "\0", bytes_cast(secureInputToken.challenge), bytes_cast(secureInputToken.userId), @@ -171,7 +168,7 @@ class Operation { result.setToExternal(formattedMessageBuffer_, formattedMessageLength_); return result; } - hidl_vec userConfirm(const uint8_t key[32]) { + hidl_vec userConfirm(const auth_token_key_t& key) { if (error_ != ResponseCode::OK) return {}; confirmationTokenScratchpad_ = HMacer::hmac256(key, "confirmation token", getMessage()); if (!confirmationTokenScratchpad_.isOk()) { @@ -188,10 +185,10 @@ class Operation { uint8_t formattedMessageBuffer_[uint32_t(MessageSize::MAX)]; char promptStringBuffer_[uint32_t(MessageSize::MAX)]; size_t formattedMessageLength_ = 0; - NullOr> confirmationTokenScratchpad_; + NullOr confirmationTokenScratchpad_; Callback resultCB_; typename TimeStamper::TimeStamp startTime_; - NullOr> hmacKey_; + NullOr hmacKey_; }; } // namespace diff --git a/confirmationui/support/include/android/hardware/confirmationui/support/confirmationui_utils.h b/confirmationui/support/include/android/hardware/confirmationui/support/confirmationui_utils.h index d551433622..3f7810a7e8 100644 --- a/confirmationui/support/include/android/hardware/confirmationui/support/confirmationui_utils.h +++ b/confirmationui/support/include/android/hardware/confirmationui/support/confirmationui_utils.h @@ -81,17 +81,23 @@ class array { public: array() : data_{} {} array(const T (&data)[elements]) { std::copy(data, data + elements, data_); } + explicit array(const T& v) { fill(v); } T* data() { return data_; } const T* data() const { return data_; } constexpr size_t size() const { return elements; } - operator const array_type&() const { return data_; } T* begin() { return data_; } T* end() { return data_ + elements; } const T* begin() const { return data_; } const T* end() const { return data_ + elements; } + void fill(const T& v) { + for (size_t i = 0; i < elements; ++i) { + data_[i] = v; + } + } + private: array_type data_; }; @@ -157,6 +163,11 @@ class ByteBufferProxy { size_t size_; }; +constexpr uint8_t auth_token_key_size = 32; +constexpr uint8_t hmac_size_bytes = support::auth_token_key_size; +using auth_token_key_t = array; +using hmac_t = auth_token_key_t; + /** * Implementer are expected to provide an implementation with the following prototype: * static NullOr> hmac256(const uint8_t key[32], @@ -166,7 +177,7 @@ template class HMac { public: template - static NullOr> hmac256(const uint8_t key[32], const Data&... data) { + static NullOr hmac256(const auth_token_key_t& key, const Data&... data) { return Impl::hmac256(key, {data...}); } };