Merge "ECIES: fix memory leaks and add malloc checks in HKDF. Use fixed-timing memcmp in HmacOperation."
diff --git a/Makefile b/Makefile
index b5795b1..119fb48 100644
--- a/Makefile
+++ b/Makefile
@@ -125,19 +125,19 @@
massif: $(BINARIES:=.massif)
hmac_test: hmac_test.o \
- hmac.o \
authorization_set.o \
google_keymaster_test_utils.o \
+ hmac.o \
logger.o \
serializable.o \
$(GTEST)/src/gtest-all.o
hkdf_test: hkdf_test.o \
+ authorization_set.o \
+ google_keymaster_test_utils.o \
hkdf.o \
hmac.o \
- authorization_set.o \
logger.o \
- google_keymaster_test_utils.o \
serializable.o \
$(GTEST)/src/gtest-all.o
diff --git a/hkdf.cpp b/hkdf.cpp
index 3f0de46..5c69446 100644
--- a/hkdf.cpp
+++ b/hkdf.cpp
@@ -18,38 +18,34 @@
#include "hkdf.h"
#include <assert.h>
-#include <keymaster/logger.h>
namespace keymaster {
const size_t kSHA256HashLength = 32;
Rfc5869HmacSha256Kdf::Rfc5869HmacSha256Kdf(Buffer& secret, Buffer& salt, Buffer& info,
- size_t key_bytes_to_generate) {
+ size_t key_bytes_to_generate, keymaster_error_t* error) {
Rfc5869HmacSha256Kdf(secret.peek_read(), secret.available_read(), salt.peek_read(),
salt.available_read(), info.peek_read(), info.available_read(),
- key_bytes_to_generate);
+ key_bytes_to_generate, error);
}
Rfc5869HmacSha256Kdf::Rfc5869HmacSha256Kdf(const uint8_t* secret, size_t secret_len,
const uint8_t* salt, size_t salt_len,
const uint8_t* info, size_t info_len,
- size_t key_bytes_to_generate) {
- // https://tools.ietf.org/html/rfc5869#section-2.2
- Buffer actual_salt;
- if (salt) {
- actual_salt.Reinitialize(salt, salt_len);
- } else {
- char zeros[kSHA256HashLength];
- // If salt is not given, HashLength zeros are used.
- memset(zeros, 0, sizeof(zeros));
- actual_salt.Reinitialize(zeros, sizeof(zeros));
- }
-
+ size_t key_bytes_to_generate, keymaster_error_t* error) {
// Step 1. Extract: PRK = HMAC-SHA256(actual_salt, secret)
// https://tools.ietf.org/html/rfc5869#section-2.2
HmacSha256 prk_hmac;
- bool result = prk_hmac.Init(actual_salt);
+ bool result;
+ if (salt) {
+ result = prk_hmac.Init(salt, salt_len);
+ } else {
+ uint8_t zeros[kSHA256HashLength];
+ // If salt is not given, HashLength zeros are used.
+ memset(zeros, 0, sizeof(zeros));
+ result = prk_hmac.Init(zeros, sizeof(zeros));
+ }
assert(result);
// |prk| is a pseudorandom key (of kSHA256HashLength octets).
@@ -62,30 +58,42 @@
// https://tools.ietf.org/html/rfc5869#section-2.3
const size_t n = (key_bytes_to_generate + kSHA256HashLength - 1) / kSHA256HashLength;
assert(n < 256u);
+ output_.reset(new uint8_t[n * kSHA256HashLength]);
+ if (!output_.get()) {
+ *error = KM_ERROR_MEMORY_ALLOCATION_FAILED;
+ return;
+ }
- output_.Reinitialize(n * kSHA256HashLength);
uint8_t buf[kSHA256HashLength + info_len + 1];
uint8_t digest[kSHA256HashLength];
- Buffer previous;
-
HmacSha256 hmac;
result = hmac.Init(prk, sizeof(prk));
assert(result);
for (size_t i = 1; i <= n; i++) {
- memcpy(buf, previous.peek_read(), previous.available_read());
- size_t j = previous.available_read();
+ size_t j = 0;
+ if (i != 1) {
+ memcpy(buf, digest, sizeof(digest));
+ j = sizeof(digest);
+ }
memcpy(buf + j, info, info_len);
j += info_len;
buf[j++] = static_cast<uint8_t>(i);
result = hmac.Sign(buf, j, digest, sizeof(digest));
assert(result);
- output_.write(digest, sizeof(digest));
- previous.Reinitialize(reinterpret_cast<uint8_t*>(digest), sizeof(digest));
+ memcpy(output_.get() + (i - 1) * sizeof(digest), digest, sizeof(digest));
}
- if (key_bytes_to_generate)
- secret_key_.Reinitialize(output_.peek_read(), key_bytes_to_generate);
+ if (key_bytes_to_generate) {
+ secret_key_len_ = key_bytes_to_generate;
+ secret_key_.reset(new uint8_t[key_bytes_to_generate]);
+ if (!secret_key_.get()) {
+ *error = KM_ERROR_MEMORY_ALLOCATION_FAILED;
+ return;
+ }
+ memcpy(secret_key_.get(), output_.get(), key_bytes_to_generate);
+ }
+ *error = KM_ERROR_OK;
}
} // namespace keymaster
diff --git a/hkdf.h b/hkdf.h
index b33b920..3ecdbe0 100644
--- a/hkdf.h
+++ b/hkdf.h
@@ -17,8 +17,11 @@
#ifndef SYSTEM_KEYMASTER_HKDF_H_
#define SYSTEM_KEYMASTER_HKDF_H_
+#include <hardware/keymaster_defs.h>
#include <keymaster/serializable.h>
+#include <UniquePtr.h>
+
namespace keymaster {
// Rfc5869HmacSha256Kdf implements the key derivation function specified in RFC 5869 (using
@@ -35,19 +38,21 @@
// optional context and application specific information (can be a zero-length
// string).
// |key_bytes_to_generate|: the number of bytes of key material to generate.
- Rfc5869HmacSha256Kdf(Buffer& secret, Buffer& salt, Buffer& info, size_t key_bytes_to_generate);
+ Rfc5869HmacSha256Kdf(Buffer& secret, Buffer& salt, Buffer& info, size_t key_bytes_to_generate,
+ keymaster_error_t* error);
Rfc5869HmacSha256Kdf(const uint8_t* secret, size_t secret_len, const uint8_t* salt,
size_t salt_len, const uint8_t* info, size_t info_len,
- size_t key_bytes_to_generate);
+ size_t key_bytes_to_generate, keymaster_error_t* error);
- void secret_key(Buffer* buf) const {
- buf->Reinitialize(secret_key_.peek_read(), secret_key_.available_read());
- }
+ bool secret_key(Buffer* buf) const {
+ return buf->Reinitialize(secret_key_.get(), secret_key_len_);
+ };
private:
- Buffer output_;
- Buffer secret_key_;
+ UniquePtr<uint8_t[]> output_;
+ UniquePtr<uint8_t[]> secret_key_;
+ size_t secret_key_len_;
};
} // namespace keymaster
diff --git a/hkdf_test.cpp b/hkdf_test.cpp
index 789e08e..dd126aa 100644
--- a/hkdf_test.cpp
+++ b/hkdf_test.cpp
@@ -77,16 +77,17 @@
const string salt = hex2str(test.salt_hex);
const string info = hex2str(test.info_hex);
const string expected = hex2str(test.output_hex);
-
+ keymaster_error_t error;
// We set the key_length to the length of the expected output and then take
// the result.
Rfc5869HmacSha256Kdf hkdf(reinterpret_cast<const uint8_t*>(key.data()), key.size(),
reinterpret_cast<const uint8_t*>(salt.data()), salt.size(),
reinterpret_cast<const uint8_t*>(info.data()), info.size(),
- expected.size());
-
+ expected.size(), &error);
+ ASSERT_EQ(error, KM_ERROR_OK);
Buffer secret_key;
- hkdf.secret_key(&secret_key);
+ bool result = hkdf.secret_key(&secret_key);
+ ASSERT_TRUE(result);
ASSERT_EQ(expected.size(), secret_key.available_read());
EXPECT_EQ(0, memcmp(expected.data(), secret_key.peek_read(), expected.size()));
}
diff --git a/hmac.cpp b/hmac.cpp
index 0591236..78e254d 100644
--- a/hmac.cpp
+++ b/hmac.cpp
@@ -27,24 +27,29 @@
return SHA256_DIGEST_LENGTH;
}
+bool HmacSha256::Init(const Buffer& key) {
+ return Init(key.peek_read(), key.available_read());
+}
+
bool HmacSha256::Init(const uint8_t* key, size_t key_len) {
if (!key)
return false;
- key_.Reinitialize(key, key_len);
+ key_len_ = key_len;
+ key_.reset(new uint8_t[key_len]);
+ if (!key_.get()) {
+ return false;
+ }
+ memcpy(key_.get(), key, key_len);
return true;
}
-bool HmacSha256::Init(const Buffer& key) {
- return Init(key.peek_read(), key.available_read());
-}
-
bool HmacSha256::Sign(const Buffer& data, uint8_t* out_digest, size_t digest_len) const {
return Sign(data.peek_read(), data.available_read(), out_digest, digest_len);
}
bool HmacSha256::Sign(const uint8_t* data, size_t data_len, uint8_t* out_digest,
- size_t digest_len) const {
+ size_t digest_len) const {
assert(digest_len);
uint8_t tmp[SHA256_DIGEST_LENGTH];
@@ -52,8 +57,7 @@
if (digest_len >= SHA256_DIGEST_LENGTH)
digest = out_digest;
- if (nullptr == ::HMAC(EVP_sha256(), key_.peek_read(), key_.available_read(),
- data, data_len, digest, nullptr)) {
+ if (nullptr == ::HMAC(EVP_sha256(), key_.get(), key_len_, data, data_len, digest, nullptr)) {
return false;
}
if (digest_len < SHA256_DIGEST_LENGTH)
@@ -68,7 +72,7 @@
}
bool HmacSha256::Verify(const uint8_t* data, size_t data_len, const uint8_t* digest,
- size_t digest_len) const {
+ size_t digest_len) const {
if (digest_len != SHA256_DIGEST_LENGTH)
return false;
diff --git a/hmac.h b/hmac.h
index 09ae5e8..ebd5b70 100644
--- a/hmac.h
+++ b/hmac.h
@@ -21,11 +21,10 @@
namespace keymaster {
-
// Only HMAC-SHA256 is supported.
class HmacSha256 {
public:
- HmacSha256() {};
+ HmacSha256(){};
// DigestLength returns the length, in bytes, of the resulting digest.
size_t DigestLength() const;
@@ -52,7 +51,8 @@
size_t digest_len) const;
private:
- Buffer key_;
+ UniquePtr<uint8_t[]> key_;
+ size_t key_len_;
};
} // namespace keymaster
diff --git a/hmac_operation.cpp b/hmac_operation.cpp
index b773920..e9d1a89 100644
--- a/hmac_operation.cpp
+++ b/hmac_operation.cpp
@@ -164,7 +164,7 @@
case KM_PURPOSE_VERIFY:
if (signature.available_read() != tag_length_)
return KM_ERROR_INVALID_INPUT_LENGTH;
- if (memcmp(signature.peek_read(), digest, tag_length_) != 0)
+ if (CRYPTO_memcmp(signature.peek_read(), digest, tag_length_) != 0)
return KM_ERROR_VERIFICATION_FAILED;
return KM_ERROR_OK;
default: