Improvements to the key storage module

The key storage module didn't comply with Android coding standards
and had room for improvemnet in a few other ways, so have cleaned up.

Change-Id: I260ccff316423169cf887e538113b5ea400892f2
diff --git a/Android.mk b/Android.mk
index 979ab5e..5785ad9 100644
--- a/Android.mk
+++ b/Android.mk
@@ -55,7 +55,6 @@
 	libhardware \
 	libsoftkeymaster \
 	libbase \
-	libkeymaster1 \
 	libkeymaster_messages \
 
 common_static_libraries := \
diff --git a/Ext4Crypt.cpp b/Ext4Crypt.cpp
index 475c196..7dbdc2a 100644
--- a/Ext4Crypt.cpp
+++ b/Ext4Crypt.cpp
@@ -550,7 +550,7 @@
 }
 
 static std::string get_key_path(userid_t user_id) {
-    return StringPrintf("%s/%d", user_key_dir.c_str(), user_id);
+    return StringPrintf("%s/user_%d/current", user_key_dir.c_str(), user_id);
 }
 
 static bool e4crypt_is_key_ephemeral(const std::string &key_path) {
@@ -565,7 +565,7 @@
         key = ephemeral_key_it->second;
         return true;
     }
-    if (!android::vold::RetrieveKey(key_path, key)) return false;
+    if (!android::vold::retrieveKey(key_path, key)) return false;
     if (key.size() != key_length/8) {
         LOG(ERROR) << "Wrong size key " << key.size() << " in " << key_path;
         return false;
@@ -573,11 +573,15 @@
     return true;
 }
 
-static bool create_user_key(userid_t user_id, bool create_ephemeral) {
-    if (fs_prepare_dir(user_key_dir.c_str(), 0700, AID_ROOT, AID_ROOT)) {
-        PLOG(ERROR) << "Failed to prepare " << user_key_dir;
+static bool prepare_dir(const std::string &dir, mode_t mode, uid_t uid, gid_t gid) {
+    if (fs_prepare_dir(dir.c_str(), mode, uid, gid) != 0) {
+        PLOG(ERROR) << "Failed to prepare " << dir;
         return false;
     }
+    return true;
+}
+
+static bool create_user_key(userid_t user_id, bool create_ephemeral) {
     const auto key_path = get_key_path(user_id);
     std::string key;
     if (android::vold::ReadRandomBytes(key_length / 8, key) != 0) {
@@ -588,8 +592,11 @@
     if (create_ephemeral) {
         // If the key should be created as ephemeral, store it in memory only.
         s_ephemeral_user_keys[key_path] = key;
-    } else if (!android::vold::StoreKey(key_path, key)) {
-        return false;
+    } else {
+        if (!prepare_dir(user_key_dir, 0700, AID_ROOT, AID_ROOT)) return false;
+        if (!prepare_dir(user_key_dir + "/user_" + std::to_string(user_id),
+            0700, AID_ROOT, AID_ROOT)) return false;
+        if (!android::vold::storeKey(key_path, key)) return false;
     }
     LOG(DEBUG) << "Created key " << key_path;
     return true;
@@ -648,7 +655,7 @@
     if (e4crypt_is_key_ephemeral(key_path)) {
         s_ephemeral_user_keys.erase(key_path);
     } else {
-        if (!android::vold::DestroyKey(key_path)) {
+        if (!android::vold::destroyKey(key_path)) {
             return -1;
         }
     }
@@ -755,27 +762,15 @@
     } else {
         LOG(DEBUG) << "e4crypt_prepare_user_storage, null volume " << user_id;
     }
-    std::string system_ce_path(android::vold::BuildDataSystemCePath(user_id));
-    std::string media_ce_path(android::vold::BuildDataMediaPath(volume_uuid, user_id));
-    std::string user_ce_path(android::vold::BuildDataUserPath(volume_uuid, user_id));
-    std::string user_de_path(android::vold::BuildDataUserDePath(volume_uuid, user_id));
+    auto system_ce_path = android::vold::BuildDataSystemCePath(user_id);
+    auto media_ce_path = android::vold::BuildDataMediaPath(volume_uuid, user_id);
+    auto user_ce_path = android::vold::BuildDataUserPath(volume_uuid, user_id);
+    auto user_de_path = android::vold::BuildDataUserDePath(volume_uuid, user_id);
 
-    if (fs_prepare_dir(system_ce_path.c_str(), 0700, AID_SYSTEM, AID_SYSTEM)) {
-        PLOG(ERROR) << "Failed to prepare " << system_ce_path;
-        return -1;
-    }
-    if (fs_prepare_dir(media_ce_path.c_str(), 0770, AID_MEDIA_RW, AID_MEDIA_RW)) {
-        PLOG(ERROR) << "Failed to prepare " << media_ce_path;
-        return -1;
-    }
-    if (fs_prepare_dir(user_ce_path.c_str(), 0771, AID_SYSTEM, AID_SYSTEM)) {
-        PLOG(ERROR) << "Failed to prepare " << user_ce_path;
-        return -1;
-    }
-    if (fs_prepare_dir(user_de_path.c_str(), 0771, AID_SYSTEM, AID_SYSTEM)) {
-        PLOG(ERROR) << "Failed to prepare " << user_de_path;
-        return -1;
-    }
+    if (!prepare_dir(system_ce_path, 0700, AID_SYSTEM, AID_SYSTEM)) return -1;
+    if (!prepare_dir(media_ce_path, 0770, AID_MEDIA_RW, AID_MEDIA_RW)) return -1;
+    if (!prepare_dir(user_ce_path, 0771, AID_SYSTEM, AID_SYSTEM)) return -1;
+    if (!prepare_dir(user_de_path, 0771, AID_SYSTEM, AID_SYSTEM)) return -1;
 
     if (e4crypt_crypto_complete(DATA_MNT_POINT) == 0) {
         if (e4crypt_set_user_policy(user_id, serial, system_ce_path)
diff --git a/KeyStorage.cpp b/KeyStorage.cpp
index d435539..070b79d 100644
--- a/KeyStorage.cpp
+++ b/KeyStorage.cpp
@@ -49,7 +49,7 @@
 static const char* kFn_encrypted_key = "encrypted_key";
 static const char* kFn_secdiscardable = "secdiscardable";
 
-static bool CheckSize(const std::string& kind, size_t actual, size_t expected) {
+static bool checkSize(const std::string& kind, size_t actual, size_t expected) {
     if (actual != expected) {
         LOG(ERROR) << "Wrong number of bytes in " << kind << ", expected " << expected
             << " got " << actual;
@@ -58,172 +58,158 @@
     return true;
 }
 
-static std::string HashSecdiscardable(const std::string &secdiscardable) {
+static std::string hashSecdiscardable(const std::string &secdiscardable) {
     SHA512_CTX c;
 
     SHA512_Init(&c);
     // Personalise the hashing by introducing a fixed prefix.
     // Hashing applications should use personalization except when there is a
     // specific reason not to; see section 4.11 of https://www.schneier.com/skein1.3.pdf
-    std::string secdiscardable_hashing_prefix = "Android secdiscardable SHA512";
-    secdiscardable_hashing_prefix.resize(SHA512_CBLOCK);
-    SHA512_Update(&c, secdiscardable_hashing_prefix.data(), secdiscardable_hashing_prefix.size());
+    std::string secdiscardableHashingPrefix = "Android secdiscardable SHA512";
+    secdiscardableHashingPrefix.resize(SHA512_CBLOCK);
+    SHA512_Update(&c, secdiscardableHashingPrefix.data(), secdiscardableHashingPrefix.size());
     SHA512_Update(&c, secdiscardable.data(), secdiscardable.size());
     std::string res(SHA512_DIGEST_LENGTH, '\0');
     SHA512_Final(reinterpret_cast<uint8_t *>(&res[0]), &c);
     return res;
 }
 
-static bool GenerateKeymasterKey(Keymaster &keymaster,
-        const keymaster::AuthorizationSet &extra_params,
+static bool generateKeymasterKey(Keymaster &keymaster,
+        const keymaster::AuthorizationSet &extraParams,
         std::string &key) {
-    keymaster::AuthorizationSetBuilder param_builder;
-    param_builder
+    auto params = keymaster::AuthorizationSetBuilder()
         .AesEncryptionKey(AES_KEY_BYTES * 8)
         .Authorization(keymaster::TAG_BLOCK_MODE, KM_MODE_GCM)
         .Authorization(keymaster::TAG_MIN_MAC_LENGTH, GCM_MAC_BYTES * 8)
         .Authorization(keymaster::TAG_PADDING, KM_PAD_NONE)
-        .Authorization(keymaster::TAG_NO_AUTH_REQUIRED); // FIXME integrate with gatekeeper
-    auto params = param_builder.build();
-    params.push_back(extra_params);
-    return keymaster.GenerateKey(params, key);
+        .Authorization(keymaster::TAG_NO_AUTH_REQUIRED) // FIXME integrate with gatekeeper
+        .build();
+    params.push_back(extraParams);
+    return keymaster.generateKey(params, key);
 }
 
-static bool EncryptWithKeymasterKey(
+static bool encryptWithKeymasterKey(
         Keymaster &keymaster,
         const std::string &key,
-        const keymaster::AuthorizationSet &extra_params,
+        const keymaster::AuthorizationSet &extraParams,
         const std::string &message,
         std::string &ciphertext) {
     // FIXME fix repetition
-    keymaster::AuthorizationSetBuilder param_builder;
-    param_builder
+    auto params = keymaster::AuthorizationSetBuilder()
         .Authorization(keymaster::TAG_BLOCK_MODE, KM_MODE_GCM)
         .Authorization(keymaster::TAG_MAC_LENGTH, GCM_MAC_BYTES * 8)
-        .Authorization(keymaster::TAG_PADDING, KM_PAD_NONE);
-    auto params = param_builder.build();
-    params.push_back(extra_params);
-    keymaster::AuthorizationSet out_params;
-    auto op_handle = keymaster.Begin(KM_PURPOSE_ENCRYPT, key, params, out_params);
-    if (!op_handle) return false;
-    keymaster_blob_t nonce_blob;
-    if (!out_params.GetTagValue(keymaster::TAG_NONCE, &nonce_blob)) {
+        .Authorization(keymaster::TAG_PADDING, KM_PAD_NONE)
+        .build();
+    params.push_back(extraParams);
+    keymaster::AuthorizationSet outParams;
+    auto opHandle = keymaster.begin(KM_PURPOSE_ENCRYPT, key, params, outParams);
+    if (!opHandle) return false;
+    keymaster_blob_t nonceBlob;
+    if (!outParams.GetTagValue(keymaster::TAG_NONCE, &nonceBlob)) {
         LOG(ERROR) << "GCM encryption but no nonce generated";
         return false;
     }
-    // nonce_blob here is just a pointer into existing data, must not be freed
-    std::string nonce(reinterpret_cast<const char *>(nonce_blob.data), nonce_blob.data_length);
-    if (!CheckSize("nonce", nonce.size(), GCM_NONCE_BYTES)) return false;
+    // nonceBlob here is just a pointer into existing data, must not be freed
+    std::string nonce(reinterpret_cast<const char *>(nonceBlob.data), nonceBlob.data_length);
+    if (!checkSize("nonce", nonce.size(), GCM_NONCE_BYTES)) return false;
     std::string body;
-    if (!op_handle.UpdateCompletely(message, body)) return false;
+    if (!opHandle.updateCompletely(message, body)) return false;
 
     std::string mac;
-    if (!op_handle.FinishWithOutput(mac)) return false;
-    if (!CheckSize("mac", mac.size(), GCM_MAC_BYTES)) return false;
+    if (!opHandle.finishWithOutput(mac)) return false;
+    if (!checkSize("mac", mac.size(), GCM_MAC_BYTES)) return false;
     ciphertext = nonce + body + mac;
     return true;
 }
 
-static bool DecryptWithKeymasterKey(
+static bool decryptWithKeymasterKey(
         Keymaster &keymaster, const std::string &key,
-        const keymaster::AuthorizationSet &extra_params,
+        const keymaster::AuthorizationSet &extraParams,
         const std::string &ciphertext,
         std::string &message) {
     auto nonce = ciphertext.substr(0, GCM_NONCE_BYTES);
-    auto body_mac = ciphertext.substr(GCM_NONCE_BYTES);
+    auto bodyAndMac = ciphertext.substr(GCM_NONCE_BYTES);
     // FIXME fix repetition
-    keymaster::AuthorizationSetBuilder param_builder;
-    param_builder
+    auto params = addStringParam(keymaster::AuthorizationSetBuilder(), keymaster::TAG_NONCE, nonce)
         .Authorization(keymaster::TAG_BLOCK_MODE, KM_MODE_GCM)
         .Authorization(keymaster::TAG_MAC_LENGTH, GCM_MAC_BYTES * 8)
-        .Authorization(keymaster::TAG_PADDING, KM_PAD_NONE);
-    AddStringParam(param_builder, keymaster::TAG_NONCE, nonce);
-    auto params = param_builder.build();
-    params.push_back(extra_params);
+        .Authorization(keymaster::TAG_PADDING, KM_PAD_NONE)
+        .build();
+    params.push_back(extraParams);
 
-    auto op_handle = keymaster.Begin(KM_PURPOSE_DECRYPT, key, params);
-    if (!op_handle) return false;
-    if (!op_handle.UpdateCompletely(body_mac, message)) return false;
-    if (!op_handle.Finish()) return false;
+    auto opHandle = keymaster.begin(KM_PURPOSE_DECRYPT, key, params);
+    if (!opHandle) return false;
+    if (!opHandle.updateCompletely(bodyAndMac, message)) return false;
+    if (!opHandle.finish()) return false;
     return true;
 }
 
-bool StoreKey(const std::string &dir, const std::string &key) {
+static bool readFileToString(const std::string &filename, std::string &result) {
+    if (!android::base::ReadFileToString(filename, &result)) {
+         PLOG(ERROR) << "Failed to read from " << filename;
+         return false;
+    }
+    return true;
+}
+
+static bool writeStringToFile(const std::string &payload, const std::string &filename) {
+    if (!android::base::WriteStringToFile(payload, filename)) {
+         PLOG(ERROR) << "Failed to write to " << filename;
+         return false;
+    }
+    return true;
+}
+
+bool storeKey(const std::string &dir, const std::string &key) {
     if (TEMP_FAILURE_RETRY(mkdir(dir.c_str(), 0700)) == -1) {
         PLOG(ERROR) << "key mkdir " << dir;
         return false;
     }
     std::string secdiscardable;
-    if (ReadRandomBytes(SECDISCARDABLE_BYTES, secdiscardable) != 0) {
+    if (ReadRandomBytes(SECDISCARDABLE_BYTES, secdiscardable) != OK) {
         // TODO status_t plays badly with PLOG, fix it.
         LOG(ERROR) << "Random read failed";
         return false;
     }
-    // FIXME create a wrapper around reads and writes which handles error logging
-    if (!android::base::WriteStringToFile(secdiscardable, dir + "/" + kFn_secdiscardable)) {
-         PLOG(ERROR) << "Unable to write secdiscardable to " << dir;
-         return false;
-    }
-    keymaster::AuthorizationSetBuilder param_builder;
-    AddStringParam(param_builder, keymaster::TAG_APPLICATION_ID,
-        HashSecdiscardable(secdiscardable));
-    auto extra_params = param_builder.build();
+    if (!writeStringToFile(secdiscardable, dir + "/" + kFn_secdiscardable)) return false;
+    auto extraParams = addStringParam(keymaster::AuthorizationSetBuilder(),
+            keymaster::TAG_APPLICATION_ID, hashSecdiscardable(secdiscardable)).build();
     Keymaster keymaster;
     if (!keymaster) return false;
-    std::string km_key;
-    if (!GenerateKeymasterKey(keymaster, extra_params, km_key)) return false;
-    std::string encrypted_key;
-    if (!EncryptWithKeymasterKey(
-        keymaster, km_key, extra_params, key, encrypted_key)) return false;
-    if (!android::base::WriteStringToFile(km_key, dir + "/" + kFn_keymaster_key_blob)) {
-        PLOG(ERROR) << "Unable to write keymaster_key_blob to " << dir;
-        return false;
-    }
-    if (!android::base::WriteStringToFile(encrypted_key, dir + "/" + kFn_encrypted_key)) {
-        PLOG(ERROR) << "Unable to write encrypted_key to " << dir;
-        return false;
-    }
+    std::string kmKey;
+    if (!generateKeymasterKey(keymaster, extraParams, kmKey)) return false;
+    std::string encryptedKey;
+    if (!encryptWithKeymasterKey(
+        keymaster, kmKey, extraParams, key, encryptedKey)) return false;
+    if (!writeStringToFile(kmKey, dir + "/" + kFn_keymaster_key_blob)) return false;
+    if (!writeStringToFile(encryptedKey, dir + "/" + kFn_encrypted_key)) return false;
     return true;
 }
 
-bool RetrieveKey(const std::string &dir, std::string &key) {
+bool retrieveKey(const std::string &dir, std::string &key) {
     std::string secdiscardable;
-    if (!android::base::ReadFileToString(dir + "/" + kFn_secdiscardable, &secdiscardable)) {
-         PLOG(ERROR) << "Unable to read secdiscardable from " << dir;
-         return false;
-    }
-    keymaster::AuthorizationSetBuilder param_builder;
-    AddStringParam(param_builder, keymaster::TAG_APPLICATION_ID,
-        HashSecdiscardable(secdiscardable));
-    auto extra_params = param_builder.build();
-    std::string km_key;
-    if (!android::base::ReadFileToString(dir + "/" + kFn_keymaster_key_blob, &km_key)) {
-         PLOG(ERROR) << "Unable to read keymaster_key_blob from " << dir;
-         return false;
-    }
-    std::string encrypted_message;
-    if (!android::base::ReadFileToString(dir + "/" + kFn_encrypted_key, &encrypted_message)) {
-         PLOG(ERROR) << "Unable to read encrypted_key to " << dir;
-         return false;
-    }
+    if (!readFileToString(dir + "/" + kFn_secdiscardable, secdiscardable)) return false;
+    auto extraParams = addStringParam(keymaster::AuthorizationSetBuilder(),
+            keymaster::TAG_APPLICATION_ID, hashSecdiscardable(secdiscardable)).build();
+    std::string kmKey;
+    if (!readFileToString(dir + "/" + kFn_keymaster_key_blob, kmKey)) return false;
+    std::string encryptedMessage;
+    if (!readFileToString(dir + "/" + kFn_encrypted_key, encryptedMessage)) return false;
     Keymaster keymaster;
     if (!keymaster) return false;
-    return DecryptWithKeymasterKey(keymaster, km_key, extra_params, encrypted_message, key);
+    return decryptWithKeymasterKey(keymaster, kmKey, extraParams, encryptedMessage, key);
 }
 
-static bool DeleteKey(const std::string &dir) {
-    std::string km_key;
-    if (!android::base::ReadFileToString(dir + "/" + kFn_keymaster_key_blob, &km_key)) {
-         PLOG(ERROR) << "Unable to read keymaster_key_blob from " << dir;
-         return false;
-    }
+static bool deleteKey(const std::string &dir) {
+    std::string kmKey;
+    if (!readFileToString(dir + "/" + kFn_keymaster_key_blob, kmKey)) return false;
     Keymaster keymaster;
     if (!keymaster) return false;
-    if (!keymaster.DeleteKey(km_key)) return false;
+    if (!keymaster.deleteKey(kmKey)) return false;
     return true;
 }
 
-static bool SecdiscardSecdiscardable(const std::string &dir) {
+static bool secdiscardSecdiscardable(const std::string &dir) {
     if (ForkExecvp(std::vector<std::string> {
             kSecdiscardPath, "--", dir + "/" + kFn_secdiscardable}) != 0) {
         LOG(ERROR) << "secdiscard failed";
@@ -232,7 +218,7 @@
     return true;
 }
 
-static bool RecursiveDeleteKey(const std::string &dir) {
+static bool recursiveDeleteKey(const std::string &dir) {
     if (ForkExecvp(std::vector<std::string> {
             kRmPath, "-rf", dir}) != 0) {
         LOG(ERROR) << "recursive delete failed";
@@ -241,12 +227,12 @@
     return true;
 }
 
-bool DestroyKey(const std::string &dir) {
+bool destroyKey(const std::string &dir) {
     bool success = true;
     // Try each thing, even if previous things failed.
-    success &= DeleteKey(dir);
-    success &= SecdiscardSecdiscardable(dir);
-    success &= RecursiveDeleteKey(dir);
+    success &= deleteKey(dir);
+    success &= secdiscardSecdiscardable(dir);
+    success &= recursiveDeleteKey(dir);
     return success;
 }
 
diff --git a/KeyStorage.h b/KeyStorage.h
index f3b7261..a35349c 100644
--- a/KeyStorage.h
+++ b/KeyStorage.h
@@ -26,13 +26,13 @@
 // in such a way that it can only be retrieved via Keymaster and
 // can be securely deleted.
 // It's safe to move/rename the directory after creation.
-bool StoreKey(const std::string &target_dir, const std::string &key);
+bool storeKey(const std::string &dir, const std::string &key);
 
 // Retrieve the key from the named directory.
-bool RetrieveKey(const std::string &dir, std::string &key);
+bool retrieveKey(const std::string &dir, std::string &key);
 
 // Securely destroy the key stored in the named directory and delete the directory.
-bool DestroyKey(const std::string &dir);
+bool destroyKey(const std::string &dir);
 
 }  // namespace vold
 }  // namespace android
diff --git a/Keymaster.cpp b/Keymaster.cpp
index 4888fe3..0fde8fa 100644
--- a/Keymaster.cpp
+++ b/Keymaster.cpp
@@ -21,39 +21,39 @@
 namespace android {
 namespace vold {
 
-bool KeymasterOperation::UpdateCompletely(
+bool KeymasterOperation::updateCompletely(
         const std::string &input,
         std::string &output) {
     output.clear();
     auto it = input.begin();
     while (it != input.end()) {
-        size_t to_read = static_cast<size_t>(input.end() - it);
-        keymaster_blob_t input_blob {reinterpret_cast<const uint8_t *>(&*it),  to_read};
-        keymaster_blob_t output_blob;
-        size_t input_consumed;
-        auto error = device->update(device, op_handle,
-            nullptr, &input_blob, &input_consumed, nullptr, &output_blob);
+        size_t toRead = static_cast<size_t>(input.end() - it);
+        keymaster_blob_t inputBlob {reinterpret_cast<const uint8_t *>(&*it),  toRead};
+        keymaster_blob_t outputBlob;
+        size_t inputConsumed;
+        auto error = mDevice->update(mDevice, mOpHandle,
+            nullptr, &inputBlob, &inputConsumed, nullptr, &outputBlob);
         if (error != KM_ERROR_OK) {
             LOG(ERROR) << "update failed, code " << error;
-            device = nullptr;
+            mDevice = nullptr;
             return false;
         }
-        output.append(reinterpret_cast<const char *>(output_blob.data), output_blob.data_length);
-        free(const_cast<uint8_t *>(output_blob.data));
-        if (input_consumed > to_read) {
+        output.append(reinterpret_cast<const char *>(outputBlob.data), outputBlob.data_length);
+        free(const_cast<uint8_t *>(outputBlob.data));
+        if (inputConsumed > toRead) {
             LOG(ERROR) << "update reported too much input consumed";
-            device = nullptr;
+            mDevice = nullptr;
             return false;
         }
-        it += input_consumed;
+        it += inputConsumed;
     }
     return true;
 }
 
-bool KeymasterOperation::Finish() {
-    auto error = device->finish(device, op_handle,
+bool KeymasterOperation::finish() {
+    auto error = mDevice->finish(mDevice, mOpHandle,
         nullptr, nullptr, nullptr, nullptr);
-    device = nullptr;
+    mDevice = nullptr;
     if (error != KM_ERROR_OK) {
         LOG(ERROR) << "finish failed, code " << error;
         return false;
@@ -61,22 +61,22 @@
     return true;
 }
 
-bool KeymasterOperation::FinishWithOutput(std::string &output) {
-    keymaster_blob_t output_blob;
-    auto error = device->finish(device, op_handle,
-        nullptr, nullptr, nullptr, &output_blob);
-    device = nullptr;
+bool KeymasterOperation::finishWithOutput(std::string &output) {
+    keymaster_blob_t outputBlob;
+    auto error = mDevice->finish(mDevice, mOpHandle,
+        nullptr, nullptr, nullptr, &outputBlob);
+    mDevice = nullptr;
     if (error != KM_ERROR_OK) {
         LOG(ERROR) << "finish failed, code " << error;
         return false;
     }
-    output.assign(reinterpret_cast<const char *>(output_blob.data), output_blob.data_length);
-    free(const_cast<uint8_t *>(output_blob.data));
+    output.assign(reinterpret_cast<const char *>(outputBlob.data), outputBlob.data_length);
+    free(const_cast<uint8_t *>(outputBlob.data));
     return true;
 }
 
 Keymaster::Keymaster() {
-    device = nullptr;
+    mDevice = nullptr;
     const hw_module_t *module;
     int ret = hw_get_module_by_class(KEYSTORE_HARDWARE_MODULE_ID, NULL, &module);
     if (ret != 0) {
@@ -88,31 +88,31 @@
         LOG(ERROR) << "module_api_version is " << module->module_api_version;
         return;
     }
-    ret = keymaster1_open(module, &device);
+    ret = keymaster1_open(module, &mDevice);
     if (ret != 0) {
         LOG(ERROR) << "keymaster1_open returned " << ret;
-        device = nullptr;
+        mDevice = nullptr;
         return;
     }
 }
 
-bool Keymaster::GenerateKey(
-        const keymaster::AuthorizationSet &in_params,
+bool Keymaster::generateKey(
+        const keymaster::AuthorizationSet &inParams,
         std::string &key) {
-    keymaster_key_blob_t key_blob;
-    auto error = device->generate_key(device, &in_params, &key_blob, nullptr);
+    keymaster_key_blob_t keyBlob;
+    auto error = mDevice->generate_key(mDevice, &inParams, &keyBlob, nullptr);
     if (error != KM_ERROR_OK) {
         LOG(ERROR) << "generate_key failed, code " << error;
         return false;
     }
-    key.assign(reinterpret_cast<const char *>(key_blob.key_material), key_blob.key_material_size);
+    key.assign(reinterpret_cast<const char *>(keyBlob.key_material), keyBlob.key_material_size);
     return true;
 }
 
-bool Keymaster::DeleteKey(const std::string &key) {
-    if (device->delete_key == nullptr) return true;
-    keymaster_key_blob_t key_blob { reinterpret_cast<const uint8_t *>(key.data()), key.size() };
-    auto error = device->delete_key(device, &key_blob);
+bool Keymaster::deleteKey(const std::string &key) {
+    if (mDevice->delete_key == nullptr) return true;
+    keymaster_key_blob_t keyBlob { reinterpret_cast<const uint8_t *>(key.data()), key.size() };
+    auto error = mDevice->delete_key(mDevice, &keyBlob);
     if (error != KM_ERROR_OK) {
         LOG(ERROR) << "delete_key failed, code " << error;
         return false;
@@ -120,39 +120,39 @@
     return true;
 }
 
-KeymasterOperation Keymaster::Begin(
+KeymasterOperation Keymaster::begin(
         keymaster_purpose_t purpose,
         const std::string &key,
-        const keymaster::AuthorizationSet &in_params,
-        keymaster::AuthorizationSet &out_params) {
-    keymaster_key_blob_t key_blob { reinterpret_cast<const uint8_t *>(key.data()), key.size() };
-    keymaster_operation_handle_t op_handle;
-    keymaster_key_param_set_t out_params_set;
-    auto error = device->begin(device, purpose,
-        &key_blob, &in_params, &out_params_set, &op_handle);
+        const keymaster::AuthorizationSet &inParams,
+        keymaster::AuthorizationSet &outParams) {
+    keymaster_key_blob_t keyBlob { reinterpret_cast<const uint8_t *>(key.data()), key.size() };
+    keymaster_operation_handle_t mOpHandle;
+    keymaster_key_param_set_t outParams_set;
+    auto error = mDevice->begin(mDevice, purpose,
+        &keyBlob, &inParams, &outParams_set, &mOpHandle);
     if (error != KM_ERROR_OK) {
         LOG(ERROR) << "begin failed, code " << error;
-        return KeymasterOperation(nullptr, op_handle);
+        return KeymasterOperation(nullptr, mOpHandle);
     }
-    out_params.Clear();
-    out_params.push_back(out_params_set);
-    keymaster_free_param_set(&out_params_set);
-    return KeymasterOperation(device, op_handle);
+    outParams.Clear();
+    outParams.push_back(outParams_set);
+    keymaster_free_param_set(&outParams_set);
+    return KeymasterOperation(mDevice, mOpHandle);
 }
 
-KeymasterOperation Keymaster::Begin(
+KeymasterOperation Keymaster::begin(
         keymaster_purpose_t purpose,
         const std::string &key,
-        const keymaster::AuthorizationSet &in_params) {
-    keymaster_key_blob_t key_blob { reinterpret_cast<const uint8_t *>(key.data()), key.size() };
-    keymaster_operation_handle_t op_handle;
-    auto error = device->begin(device, purpose, 
-        &key_blob, &in_params, nullptr, &op_handle);
+        const keymaster::AuthorizationSet &inParams) {
+    keymaster_key_blob_t keyBlob { reinterpret_cast<const uint8_t *>(key.data()), key.size() };
+    keymaster_operation_handle_t mOpHandle;
+    auto error = mDevice->begin(mDevice, purpose,
+        &keyBlob, &inParams, nullptr, &mOpHandle);
     if (error != KM_ERROR_OK) {
         LOG(ERROR) << "begin failed, code " << error;
-        return KeymasterOperation(nullptr, op_handle);
+        return KeymasterOperation(nullptr, mOpHandle);
     }
-    return KeymasterOperation(device, op_handle);
+    return KeymasterOperation(mDevice, mOpHandle);
 }
 
 }  // namespace vold
diff --git a/Keymaster.h b/Keymaster.h
index cc3104d..003baa6 100644
--- a/Keymaster.h
+++ b/Keymaster.h
@@ -40,36 +40,28 @@
 // to LOG(ERROR).
 class KeymasterOperation {
 public:
-    ~KeymasterOperation() { if (device) device->abort(device, op_handle); }
+    ~KeymasterOperation() { if (mDevice) mDevice->abort(mDevice, mOpHandle); }
     // Is this instance valid? This is false if creation fails, and becomes
     // false on finish or if an update fails.
-    explicit operator bool() {return device != nullptr;}
-    // Call "update" repeatedly until all of the input is consumed, and 
+    explicit operator bool() {return mDevice != nullptr;}
+    // Call "update" repeatedly until all of the input is consumed, and
     // concatenate the output. Return true on success.
-    bool UpdateCompletely(const std::string &input, std::string &output);
-    // Finish; pass nullptr for the "output" param. 
-    bool Finish();
+    bool updateCompletely(const std::string &input, std::string &output);
+    // Finish; pass nullptr for the "output" param.
+    bool finish();
     // Finish and write the output to this string.
-    bool FinishWithOutput(std::string &output);
+    bool finishWithOutput(std::string &output);
     // Move constructor
     KeymasterOperation(KeymasterOperation&& rhs) {
-        op_handle = rhs.op_handle;
-        device = rhs.device;
-        rhs.device = nullptr;
+        mOpHandle = rhs.mOpHandle;
+        mDevice = rhs.mDevice;
+        rhs.mDevice = nullptr;
     }
-    // Move assignation.
-    KeymasterOperation& operator=(KeymasterOperation&& rhs) {
-        op_handle = rhs.op_handle;
-        device = rhs.device;
-        rhs.device = nullptr;
-        return *this;
-    }
-
 private:
     KeymasterOperation(keymaster1_device_t *d, keymaster_operation_handle_t h):
-        device {d}, op_handle {h} {}
-    keymaster1_device_t *device;
-    keymaster_operation_handle_t op_handle;
+        mDevice {d}, mOpHandle {h} {}
+    keymaster1_device_t *mDevice;
+    keymaster_operation_handle_t mOpHandle;
     DISALLOW_COPY_AND_ASSIGN(KeymasterOperation);
     friend class Keymaster;
 };
@@ -79,31 +71,31 @@
 class Keymaster {
 public:
     Keymaster();
-    ~Keymaster() { if (device) keymaster1_close(device); }
+    ~Keymaster() { if (mDevice) keymaster1_close(mDevice); }
     // false if we failed to open the keymaster device.
-    explicit operator bool() {return device != nullptr;}
+    explicit operator bool() {return mDevice != nullptr;}
     // Generate a key in the keymaster from the given params.
-    bool GenerateKey(const AuthorizationSet &in_params, std::string &key);
+    bool generateKey(const AuthorizationSet &inParams, std::string &key);
     // If the keymaster supports it, permanently delete a key.
-    bool DeleteKey(const std::string &key);
+    bool deleteKey(const std::string &key);
     // Begin a new cryptographic operation, collecting output parameters.
-    KeymasterOperation Begin(
+    KeymasterOperation begin(
             keymaster_purpose_t purpose,
             const std::string &key,
-            const AuthorizationSet &in_params,
-            AuthorizationSet &out_params);
+            const AuthorizationSet &inParams,
+            AuthorizationSet &outParams);
     // Begin a new cryptographic operation; don't collect output parameters.
-    KeymasterOperation Begin(
+    KeymasterOperation begin(
             keymaster_purpose_t purpose,
             const std::string &key,
-            const AuthorizationSet &in_params);
+            const AuthorizationSet &inParams);
 private:
-    keymaster1_device_t *device;
+    keymaster1_device_t *mDevice;
     DISALLOW_COPY_AND_ASSIGN(Keymaster);
 };
 
 template <keymaster_tag_t Tag>
-inline AuthorizationSetBuilder& AddStringParam(AuthorizationSetBuilder &params,
+inline AuthorizationSetBuilder& addStringParam(AuthorizationSetBuilder &&params,
         TypedTag<KM_BYTES, Tag> tag, const std::string& val) {
     return params.Authorization(tag, val.data(), val.size());
 }