Merge "Refactor bufferhub token to be more secure" (72993231) · Commits · e / os / android_frameworks_native

services/bufferhub/Android.bp

+2 −0

Original line number	Diff line number	Diff line
		@@ -34,6 +34,7 @@ cc_library_shared {
		],
		shared_libs: [
		"android.frameworks.bufferhub@1.0",
		"libcrypto",
		"libcutils",
		"libhidlbase",
		"libhidltransport",
		@@ -60,6 +61,7 @@ cc_binary {
		shared_libs: [
		"android.frameworks.bufferhub@1.0",
		"libbufferhubservice",
		"libcrypto",
		"libcutils",
		"libhidltransport",
		"libhwbinder",

services/bufferhub/BufferHubService.cpp

+64 −18

Original line number	Diff line number	Diff line
		@@ -14,13 +14,16 @@
		* limitations under the License.
		*/

		#include <array>
		#include <iomanip>
		#include <random>
		#include <sstream>

		#include <android/hardware_buffer.h>
		#include <bufferhub/BufferHubService.h>
		#include <cutils/native_handle.h>
		#include <log/log.h>
		#include <openssl/hmac.h>
		#include <system/graphics-base.h>

		using ::android::BufferHubDefs::MetadataHeader;
		@@ -32,6 +35,13 @@ namespace bufferhub {
		namespace V1_0 {
		namespace implementation {

		BufferHubService::BufferHubService() {
		std::mt19937_64 randomEngine;
		randomEngine.seed(time(nullptr));

		mKey = randomEngine();
		}

		Return<void> BufferHubService::allocateBuffer(const HardwareBufferDescription& description,
		const uint32_t userMetadataSize,
		allocateBuffer_cb _hidl_cb) {
		@@ -66,27 +76,49 @@ Return<void> BufferHubService::allocateBuffer(const HardwareBufferDescription& d

		Return<void> BufferHubService::importBuffer(const hidl_handle& tokenHandle,
		importBuffer_cb _hidl_cb) {
		if (!tokenHandle.getNativeHandle() \|\| tokenHandle->numFds != 0 \|\| tokenHandle->numInts != 1) {
		if (!tokenHandle.getNativeHandle() \|\| tokenHandle->numFds != 0 \|\| tokenHandle->numInts <= 1) {
		// nullptr handle or wrong format
		_hidl_cb(/status=/BufferHubStatus::INVALID_TOKEN, /bufferClient=/nullptr,
		/bufferTraits=/{});
		return Void();
		}

		uint32_t token = tokenHandle->data[0];
		int tokenId = tokenHandle->data[0];

		wp<BufferClient> originClientWp;
		{
		std::lock_guard<std::mutex> lock(mTokenMapMutex);
		auto iter = mTokenMap.find(token);
		std::lock_guard<std::mutex> lock(mTokenMutex);
		auto iter = mTokenMap.find(tokenId);
		if (iter == mTokenMap.end()) {
		// Invalid token
		// Token Id not exist
		ALOGD("%s: token #%d not found.", __FUNCTION__, tokenId);
		_hidl_cb(/status=/BufferHubStatus::INVALID_TOKEN, /bufferClient=/nullptr,
		/bufferTraits=/{});
		return Void();
		}

		const std::vector<uint8_t>& tokenHMAC = iter->second.first;

		int numIntsForHMAC = (int)ceil(tokenHMAC.size() * sizeof(uint8_t) / (double)sizeof(int));
		if (tokenHandle->numInts - 1 != numIntsForHMAC) {
		// HMAC size not match
		ALOGD("%s: token #%d HMAC size not match. Expected: %d Actual: %d", __FUNCTION__,
		tokenId, numIntsForHMAC, tokenHandle->numInts - 1);
		_hidl_cb(/status=/BufferHubStatus::INVALID_TOKEN, /bufferClient=/nullptr,
		/bufferTraits=/{});
		return Void();
		}

		originClientWp = iter->second;
		size_t hmacSize = tokenHMAC.size() * sizeof(uint8_t);
		if (memcmp(tokenHMAC.data(), &tokenHandle->data[1], hmacSize) != 0) {
		// HMAC not match
		ALOGD("%s: token #%d HMAC not match.", __FUNCTION__, tokenId);
		_hidl_cb(/status=/BufferHubStatus::INVALID_TOKEN, /bufferClient=/nullptr,
		/bufferTraits=/{});
		return Void();
		}

		originClientWp = iter->second.second;
		mTokenMap.erase(iter);
		}

		@@ -225,9 +257,9 @@ Return<void> BufferHubService::debug(const hidl_handle& fd, const hidl_vec<hidl_
		// Map from bufferId to tokenCount
		std::map<int, uint32_t> tokenCount;
		{
		std::lock_guard<std::mutex> lock(mTokenMapMutex);
		std::lock_guard<std::mutex> lock(mTokenMutex);
		for (auto iter = mTokenMap.begin(); iter != mTokenMap.end(); ++iter) {
		sp<BufferClient> client = iter->second.promote();
		sp<BufferClient> client = iter->second.second.promote();
		if (client != nullptr) {
		const std::shared_ptr<BufferNode> node = client->getBufferNode();
		auto mapIter = tokenCount.find(node->id());
		@@ -262,21 +294,35 @@ Return<void> BufferHubService::debug(const hidl_handle& fd, const hidl_vec<hidl_
		}

		hidl_handle BufferHubService::registerToken(const wp<BufferClient>& client) {
		uint32_t token;
		std::lock_guard<std::mutex> lock(mTokenMapMutex);
		// Find next available token id
		std::lock_guard<std::mutex> lock(mTokenMutex);
		do {
		token = mTokenEngine();
		} while (mTokenMap.find(token) != mTokenMap.end());
		++mLastTokenId;
		} while (mTokenMap.find(mLastTokenId) != mTokenMap.end());

		std::array<uint8_t, EVP_MAX_MD_SIZE> hmac;
		uint32_t hmacSize = 0U;

		// native_handle_t use int[], so here need one slots to fit in uint32_t
		native_handle_t* handle = native_handle_create(/numFds=/0, /numInts=/1);
		handle->data[0] = token;
		HMAC(/evp_md=/EVP_sha256(), /key=/&mKey, /key_len=/kKeyLen,
		/data=/(uint8_t)&mLastTokenId, /data_len=*/mTokenIdSize,
		/out=/hmac.data(), /out_len=/&hmacSize);

		int numIntsForHMAC = (int)ceil(hmacSize / (double)sizeof(int));
		native_handle_t* handle = native_handle_create(/numFds=/0, /numInts=/1 + numIntsForHMAC);
		handle->data[0] = mLastTokenId;
		// Set all the the bits of last int to 0 since it might not be fully overwritten
		handle->data[numIntsForHMAC] = 0;
		memcpy(&handle->data[1], hmac.data(), hmacSize);

		// returnToken owns the native_handle_t* thus doing lifecycle management
		hidl_handle returnToken;
		returnToken.setTo(handle, /shoudOwn=/true);

		mTokenMap.emplace(token, client);
		std::vector<uint8_t> hmacVec;
		hmacVec.resize(hmacSize);
		memcpy(hmacVec.data(), hmac.data(), hmacSize);
		mTokenMap.emplace(mLastTokenId, std::pair(hmacVec, client));

		return returnToken;
		}

		@@ -291,10 +337,10 @@ void BufferHubService::onClientClosed(const BufferClient* client) {
		}

		void BufferHubService::removeTokenByClient(const BufferClient* client) {
		std::lock_guard<std::mutex> lock(mTokenMapMutex);
		std::lock_guard<std::mutex> lock(mTokenMutex);
		auto iter = mTokenMap.begin();
		while (iter != mTokenMap.end()) {
		if (iter->second == client) {
		if (iter->second.second == client) {
		auto oldIter = iter;
		++iter;
		mTokenMap.erase(oldIter);

services/bufferhub/include/bufferhub/BufferHubService.h

+18 −6

Original line number	Diff line number	Diff line
		@@ -17,8 +17,10 @@
		#ifndef ANDROID_FRAMEWORKS_BUFFERHUB_V1_0_BUFFER_HUB_SERVICE_H
		#define ANDROID_FRAMEWORKS_BUFFERHUB_V1_0_BUFFER_HUB_SERVICE_H

		#include <map>
		#include <mutex>
		#include <random>
		#include <set>
		#include <vector>

		#include <android/frameworks/bufferhub/1.0/IBufferHub.h>
		#include <bufferhub/BufferClient.h>
		@@ -39,6 +41,8 @@ using hardware::graphics::common::V1_2::HardwareBufferDescription;

		class BufferHubService : public IBufferHub {
		public:
		BufferHubService();

		Return<void> allocateBuffer(const HardwareBufferDescription& description,
		const uint32_t userMetadataSize,
		allocateBuffer_cb _hidl_cb) override;
		@@ -60,11 +64,19 @@ private:
		std::mutex mClientSetMutex;
		std::set<wp<BufferClient>> mClientSet GUARDED_BY(mClientSetMutex);

		// TODO(b/118180214): use a more secure implementation
		std::mt19937 mTokenEngine;
		// The mapping from token to the client creates it.
		std::mutex mTokenMapMutex;
		std::map<uint32_t, const wp<BufferClient>> mTokenMap GUARDED_BY(mTokenMapMutex);
		// Token generation related
		// A random number used as private key for HMAC
		uint64_t mKey;
		static constexpr size_t kKeyLen = sizeof(uint64_t);

		std::mutex mTokenMutex;
		// The first TokenId will be 1. TokenId could be negative.
		int mLastTokenId GUARDED_BY(mTokenMutex) = 0;
		static constexpr size_t mTokenIdSize = sizeof(int);
		// A map from token id to the token-buffer_client pair. Using token id as the key to reduce
		// looking up time
		std::map<int, std::pair<std::vector<uint8_t>, const wp<BufferClient>>> mTokenMap
		GUARDED_BY(mTokenMutex);
		};

		} // namespace implementation