Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5a109815 authored by Martin Brabham's avatar Martin Brabham Committed by android-build-team Robot
Browse files

Revert "[DO NOT MERGE] Handle edge cases where input or hash/data could be null."

This reverts commit 35664063.

Change-Id: I67f0195660598f1939654a79ceb406e3ecfa0bcc
(cherry picked from commit 1400157615a4873176590800aa86d59390f52cff)
parent 997f25c0
Loading
Loading
Loading
Loading
+1 −7
Original line number Diff line number Diff line
@@ -127,10 +127,7 @@ cc_test {
    name: "net_test_btif",
    defaults: ["fluoride_defaults"],
    include_dirs: btifCommonIncludes,
    srcs: [
        "test/btif_storage_test.cc",
        "test/btif_keystore_test.cc"
    ],
    srcs: ["test/btif_storage_test.cc"],
    header_libs: ["libbluetooth_headers"],
    shared_libs: [
        "libaudioclient",
@@ -170,9 +167,6 @@ cc_test {
        "libbluetooth-for-tests",
    ],
    cflags: ["-DBUILDCFG"],
    sanitize: {
        integer_overflow: true,
    },
}

// btif profile queue unit tests for target
+6 −39
Original line number Diff line number Diff line
@@ -18,7 +18,6 @@

#include <base/logging.h>
#include <keystore/keystore_client_impl.h>
#include <mutex>

#include "osi/include/alarm.h"
#include "osi/include/allocator.h"
@@ -28,45 +27,13 @@
#include "osi/include/osi.h"
#include "osi/include/properties.h"

namespace bluetooth {
/**
 * Client wrapper to access AndroidKeystore.
 *
 * <p>Use to encrypt/decrypt data and store to disk.
 */
using namespace keystore;

class BtifKeystore {
 public:
  /**
   * @param keystore_client injected pre-created client object for keystore
   */
  BtifKeystore(keystore::KeystoreClient* keystore_client);

  /**
   * Stores encrypted data to disk.
   *
   * <p>Returns true on success.
   *
   * @param data to be encrypted
   * @param output_filename location to write the file
   * @param flags
   */
  bool Encrypt(const std::string& data, const std::string& output_filename,
  BtifKeystore();
  ~BtifKeystore();
  int Encrypt(const std::string& hash, const std::string& output_filename,
              int32_t flags);

  /**
   * Returns a decrypted string representation of the encrypted data or empty
   * string on error.
   *
   * @param input_filename location of file to read and decrypt
   */
  std::string Decrypt(const std::string& input_filename);

 private:
  std::unique_ptr<keystore::KeystoreClient> keystore_client_;
  std::mutex api_mutex_;
  keystore::KeyStoreNativeReturnCode GenerateKey(const std::string& name,
                                                 int32_t flags,
                                                 bool auth_bound);
};

}  // namespace bluetooth
+26 −15
Original line number Diff line number Diff line
@@ -56,10 +56,6 @@
#define TIME_STRING_LENGTH sizeof("YYYY-MM-DD HH:MM:SS")
static const char* TIME_STRING_FORMAT = "%Y-%m-%d %H:%M:%S";

constexpr int kBufferSize = 400 * 10;  // initial file is ~400B

using bluetooth::BtifKeystore;

// TODO(armansito): Find a better way than searching by a hardcoded path.
#if defined(OS_GENERIC)
static const char* CONFIG_FILE_PATH = "bt_config.conf";
@@ -87,6 +83,8 @@ static std::unique_ptr<config_t> btif_config_open(const char* filename, const ch
static std::string hash_file(const char* filename);
static std::string read_checksum_file(const char* filename);
static void write_checksum_file(const char* filename, const std::string& hash);
static bool verify_hash(const std::string& current_hash,
                        const std::string& stored_hash);

static enum ConfigSource {
  NOT_LOADED,
@@ -132,8 +130,7 @@ bool btif_get_address_type(const RawAddress& bda, int* p_addr_type) {
static std::mutex config_lock;  // protects operations on |config|.
static std::unique_ptr<config_t> config;
static alarm_t* config_timer;

static BtifKeystore btif_keystore(new keystore::KeystoreClientImpl);
static BtifKeystore btifKeystore;

// Module lifecycle functions

@@ -229,7 +226,7 @@ static std::unique_ptr<config_t> btif_config_open(const char* filename, const ch
    }
  }
  // Compare hashes
  if (current_hash != stored_hash) {
  if (!verify_hash(current_hash, stored_hash)) {
    return nullptr;
  }
  // END KEY ATTESTATION
@@ -585,13 +582,18 @@ static std::string hash_file(const char* filename) {
               << "': " << strerror(errno);
    return "";
  }
  uint8_t hash[SHA256_DIGEST_LENGTH];
  unsigned char hash[SHA256_DIGEST_LENGTH];
  SHA256_CTX sha256;
  SHA256_Init(&sha256);
  std::array<unsigned char, kBufferSize> buffer;
  int bytes_read = 0;
  while ((bytes_read = fread(buffer.data(), 1, buffer.size(), fp))) {
    SHA256_Update(&sha256, buffer.data(), bytes_read);
  const int bufSize = 400 * 10;  // initial file is ~400B
  std::byte* buffer = (std::byte*) osi_calloc(bufSize);
  int bytesRead = 0;
  if (!buffer) {
    fclose(fp);
    return "";
  }
  while ((bytesRead = fread(buffer, 1, bufSize, fp))) {
    SHA256_Update(&sha256, buffer, bytesRead);
  }
  SHA256_Final(hash, &sha256);
  std::stringstream ss;
@@ -599,6 +601,7 @@ static std::string hash_file(const char* filename) {
    ss << std::hex << std::setw(2) << std::setfill('0') << (int)hash[i];
  }
  fclose(fp);
  osi_free(buffer);
  return ss.str();
}

@@ -607,11 +610,19 @@ static std::string read_checksum_file(const char* checksum_filename) {
  if (access(checksum_filename, R_OK) != 0) {
    return "";
  }
  return btif_keystore.Decrypt(checksum_filename);
  std::string output = btifKeystore.Decrypt(checksum_filename);
  return output;
}

static void write_checksum_file(const char* checksum_filename,
                                const std::string& hash) {
  bool result = btif_keystore.Encrypt(hash, checksum_filename, 0);
  CHECK(result) << "Failed writing checksum";
  int result = btifKeystore.Encrypt(hash, checksum_filename, 0);
  if (result != 0) {
    LOG(ERROR) << "Failed writing checksum!";
  }
}

static bool verify_hash(const std::string& current_hash,
                        const std::string& stored_hash) {
  return current_hash.compare(stored_hash) == 0;
}
+76 −68
Original line number Diff line number Diff line
@@ -16,7 +16,10 @@
 *
 ******************************************************************************/

#define LOG_TAG "bt_btif_keystore"

#include "btif_keystore.h"
#include "osi/include/properties.h"

#include <base/files/file_util.h>
#include <base/logging.h>
@@ -27,91 +30,82 @@
#include <sys/stat.h>

using namespace keystore;
using namespace bluetooth;

constexpr char kKeyStore[] = "AndroidKeystore";
static std::unique_ptr<keystore::KeystoreClient> CreateKeystoreInstance(void);
static void WriteFile(const std::string& filename, const std::string& content);
static std::string ReadFile(const std::string& filename);
static int GenerateKey(const std::string& name, int32_t flags, bool auth_bound);
static bool DoesKeyExist(const std::string& name);

static std::string ReadFile(const std::string& filename) {
  CHECK(!filename.empty()) << __func__ << ": filename should not be empty";
const std::string FILE_SUFFIX = ".encrypted-checksum";
const std::string CIPHER_ALGORITHM = "AES/GCM/NoPadding";
const std::string DIGEST_ALGORITHM = "SHA-256";
const std::string KEY_STORE = "AndroidKeystore";

  std::string content;
  base::FilePath path(filename);
  if (!base::PathExists(path)) {
    // Config file checksum file doesn't exist on first run after OTA.
    LOG(ERROR) << "file '" << filename.c_str() << "'doesn't exists yet";
  }
  if (!base::ReadFileToString(path, &content)) {
    LOG(ERROR) << "ReadFile failed: " << filename.c_str();
  }
  return content;
}
std::unique_ptr<KeystoreClient> keystoreClient;

static void WriteFile(const std::string& filename, const std::string& content) {
  CHECK(!filename.empty()) << __func__ << ": filename should not be empty";
  CHECK(!content.empty()) << __func__ << ": content should not be empty";
BtifKeystore::BtifKeystore() { keystoreClient = CreateKeystoreInstance(); }

  base::FilePath path(filename);
  int size = content.size();
  if (base::WriteFile(path, content.data(), size) != size) {
    LOG(FATAL) << "WriteFile failed.\n" << filename.c_str();
  }
BtifKeystore::~BtifKeystore() {
  // Using a smart pointer, does it delete itself?
  // delete keystoreClient;
}

namespace bluetooth {

BtifKeystore::BtifKeystore(keystore::KeystoreClient* keystore_client)
    : keystore_client_(keystore_client) {}

bool BtifKeystore::Encrypt(const std::string& data,
int BtifKeystore::Encrypt(const std::string& hash,
                          const std::string& output_filename, int32_t flags) {
  std::lock_guard<std::mutex> lock(api_mutex_);
  if (data.empty()) {
    LOG(ERROR) << __func__ << ": empty data";
    return false;
  }
  if (output_filename.empty()) {
    LOG(ERROR) << __func__ << ": empty output filename";
    return false;
  }
  std::string output;
  if (!keystore_client_->doesKeyExist(kKeyStore)) {
    auto gen_result = GenerateKey(kKeyStore, 0, false);
    if (!gen_result.isOk()) {
      LOG(FATAL) << "EncryptWithAuthentication Failed: generateKey response="
                 << gen_result;
      return false;
  if (!DoesKeyExist(KEY_STORE)) {
    GenerateKey(KEY_STORE, 0, false);
  }
  }
  if (!keystore_client_->encryptWithAuthentication(kKeyStore, data, flags,
  char is_unittest[PROPERTY_VALUE_MAX] = {0};
  osi_property_get("debug.bluetooth.unittest", is_unittest, "false");
  if (strcmp(is_unittest, "false") == 0) {
    if (!keystoreClient->encryptWithAuthentication(KEY_STORE, hash, flags,
                                                   &output)) {
    LOG(FATAL) << "EncryptWithAuthentication failed.";
    return false;
      LOG(ERROR) << "EncryptWithAuthentication failed.\n";
      return 1;
    }
  }
  WriteFile(output_filename, output);
  return true;
  return 0;
}

std::string BtifKeystore::Decrypt(const std::string& input_filename) {
  std::lock_guard<std::mutex> lock(api_mutex_);
  std::string input = ReadFile(input_filename);
  std::string output;
  if (input_filename.empty()) {
    LOG(ERROR) << __func__ << ": empty input filename";
    return output;

  char is_unittest[PROPERTY_VALUE_MAX] = {0};
  osi_property_get("debug.bluetooth.unittest", is_unittest, "false");
  if (strcmp(is_unittest, "false") == 0) {
    if (!keystoreClient->decryptWithAuthentication(KEY_STORE, input, &output)) {
      LOG(ERROR) << "DecryptWithAuthentication failed.\n";
    }
  }
  std::string input = ReadFile(input_filename);
  if (input.empty()) {
    LOG(ERROR) << __func__ << ": empty input data";
  return output;
}
  if (!keystore_client_->decryptWithAuthentication(kKeyStore, input, &output)) {
    LOG(FATAL) << "DecryptWithAuthentication failed.\n";

static std::string ReadFile(const std::string& filename) {
  std::string content;
  struct stat buffer;
  if (stat(filename.c_str(), &buffer) == 0) {
    base::FilePath path(filename);
    if (!base::ReadFileToString(path, &content)) {
      LOG(ERROR) << "ReadFile failed.\n" << filename.c_str();
    }
  }
  return content;
}

static void WriteFile(const std::string& filename, const std::string& content) {
  base::FilePath path(filename);
  int size = content.size();
  if (base::WriteFile(path, content.data(), size) != size) {
    LOG(ERROR) << "WriteFile failed.\n" << filename.c_str();
  }
  return output;
}

// Note: auth_bound keys created with this tool will not be usable.
KeyStoreNativeReturnCode BtifKeystore::GenerateKey(const std::string& name,
                                                   int32_t flags,
static int GenerateKey(const std::string& name, int32_t flags,
                       bool auth_bound) {
  AuthorizationSetBuilder params;
  params.RsaSigningKey(2048, 65537)
@@ -130,9 +124,23 @@ KeyStoreNativeReturnCode BtifKeystore::GenerateKey(const std::string& name,
  }
  AuthorizationSet hardware_enforced_characteristics;
  AuthorizationSet software_enforced_characteristics;
  return keystore_client_->generateKey(name, params, flags,

  char is_unittest[PROPERTY_VALUE_MAX] = {0};
  osi_property_get("debug.bluetooth.unittest", is_unittest, "false");
  if (strcmp(is_unittest, "false") != 0) {
      return -1;
  }
  auto result = keystoreClient->generateKey(name, params, flags,
                                            &hardware_enforced_characteristics,
                                            &software_enforced_characteristics);
  return result.getErrorCode();
}

}  // namespace bluetooth
static bool DoesKeyExist(const std::string& name) {
  return keystoreClient->doesKeyExist(name) ? true : false;
}

static std::unique_ptr<KeystoreClient> CreateKeystoreInstance(void) {
  return std::unique_ptr<KeystoreClient>(
      static_cast<KeystoreClient*>(new KeystoreClientImpl));
}
+0 −98
Original line number Diff line number Diff line
/******************************************************************************
 *
 *  Copyright 2019 Google, Inc.
 *
 *  Licensed under the Apache License, Version 2.0 (the "License");
 *  you may not use this file except in compliance with the License.
 *  You may obtain a copy of the License at:
 *
 *  http://www.apache.org/licenses/LICENSE-2.0
 *
 *  Unless required by applicable law or agreed to in writing, software
 *  distributed under the License is distributed on an "AS IS" BASIS,
 *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 *  See the License for the specific language governing permissions and
 *  limitations under the License.
 *
 ******************************************************************************/

#include <base/files/file_util.h>
#include <base/logging.h>
#include <binder/ProcessState.h>
#include <gtest/gtest.h>
#include <fstream>

#include "btif/include/btif_keystore.h"

using namespace bluetooth;

constexpr char kFilename[] = "/data/misc/bluedroid/testfile.txt";

class BtifKeystoreTest : public ::testing::Test {
 protected:
  std::unique_ptr<BtifKeystore> btif_keystore_;
  base::FilePath file_path_;
  BtifKeystoreTest() : file_path_(kFilename) {}
  void SetUp() override {
    android::ProcessState::self()->startThreadPool();
    btif_keystore_ =
        std::make_unique<BtifKeystore>(static_cast<keystore::KeystoreClient*>(
            new keystore::KeystoreClientImpl));
    base::DeleteFile(file_path_, true);
  };
  void TearDown() override { btif_keystore_ = nullptr; };
};

// Encrypt
TEST_F(BtifKeystoreTest, test_encrypt_decrypt) {
  std::string hash = "test";

  EXPECT_TRUE(btif_keystore_->Encrypt(hash, kFilename, 0));
  std::string decrypted_hash = btif_keystore_->Decrypt(kFilename);

  EXPECT_TRUE(base::PathExists(file_path_));
  EXPECT_EQ(hash, decrypted_hash);
}

TEST_F(BtifKeystoreTest, test_encrypt_empty_hash) {
  std::string hash = "";

  EXPECT_FALSE(btif_keystore_->Encrypt(hash, kFilename, 0));

  EXPECT_FALSE(base::PathExists(file_path_));
}

TEST_F(BtifKeystoreTest, test_encrypt_empty_filename) {
  std::string hash = "test";

  EXPECT_FALSE(btif_keystore_->Encrypt(hash, "", 0));

  EXPECT_FALSE(base::PathExists(file_path_));
}

// Decrypt
TEST_F(BtifKeystoreTest, test_decrypt_empty_hash) {
  // Only way to get the hash to decrypt is to read it from the file
  // So make empty file manually
  std::ofstream outfile(kFilename);
  outfile.close();

  std::string decrypted_hash = btif_keystore_->Decrypt(kFilename);

  EXPECT_TRUE(decrypted_hash.empty());
}

TEST_F(BtifKeystoreTest, test_decrypt_file_not_exist) {
  // Ensure file doesn't exist, then decrypt
  EXPECT_FALSE(base::PathExists(file_path_));

  std::string decrypted_hash = btif_keystore_->Decrypt(kFilename);

  EXPECT_TRUE(decrypted_hash.empty());
}

TEST_F(BtifKeystoreTest, test_decrypt_empty_filename) {
  std::string decrypted_hash = btif_keystore_->Decrypt("");

  EXPECT_TRUE(decrypted_hash.empty());
}
Loading