From 63c18d3ba9179ee0e678564e12aa845d9a6c3ec8 Mon Sep 17 00:00:00 2001 From: Paul Crowley Date: Wed, 10 Feb 2016 14:02:47 +0000 Subject: [PATCH] Add scrypt-based password stretching. Bug: 27056334 Change-Id: Ifa7f776c21c439f89dad7836175fbd045e1c603e --- Android.mk | 1 + Ext4Crypt.cpp | 4 -- KeyStorage.cpp | 99 +++++++++++++++++++++++++++++++++++++++----- ScryptParameters.cpp | 50 ++++++++++++++++++++++ ScryptParameters.h | 32 ++++++++++++++ cryptfs.c | 48 ++++----------------- cryptfs.h | 3 -- 7 files changed, 180 insertions(+), 57 deletions(-) create mode 100644 ScryptParameters.cpp create mode 100644 ScryptParameters.h diff --git a/Android.mk b/Android.mk index 99a1739..049cd80 100644 --- a/Android.mk +++ b/Android.mk @@ -29,6 +29,7 @@ common_src_files := \ TrimTask.cpp \ Keymaster.cpp \ KeyStorage.cpp \ + ScryptParameters.cpp \ secontext.cpp \ common_c_includes := \ diff --git a/Ext4Crypt.cpp b/Ext4Crypt.cpp index b82e75d..1921546 100644 --- a/Ext4Crypt.cpp +++ b/Ext4Crypt.cpp @@ -42,13 +42,9 @@ #include "cryptfs.h" #include "ext4_crypt.h" -#define LOG_TAG "Ext4Crypt" - #define EMULATED_USES_SELINUX 0 #include -#include -#include #include #include diff --git a/KeyStorage.cpp b/KeyStorage.cpp index def1a32..2338f23 100644 --- a/KeyStorage.cpp +++ b/KeyStorage.cpp @@ -17,6 +17,7 @@ #include "KeyStorage.h" #include "Keymaster.h" +#include "ScryptParameters.h" #include "Utils.h" #include @@ -32,8 +33,16 @@ #include #include +#include + #include +extern "C" { + +#include "crypto_scrypt.h" + +} + namespace android { namespace vold { @@ -42,14 +51,19 @@ const KeyAuthentication kEmptyAuthentication { "", "" }; static constexpr size_t AES_KEY_BYTES = 32; static constexpr size_t GCM_NONCE_BYTES = 12; static constexpr size_t GCM_MAC_BYTES = 16; -// FIXME: better name than "secdiscardable" sought! +static constexpr size_t SALT_BYTES = 1<<4; static constexpr size_t SECDISCARDABLE_BYTES = 1<<14; +static constexpr size_t STRETCHED_BYTES = 1<<6; static const char* kCurrentVersion = "1"; static const char* kRmPath = "/system/bin/rm"; static const char* kSecdiscardPath = "/system/bin/secdiscard"; +static const char* kStretch_none = "none"; +static const char* kStretch_nopassword = "nopassword"; +static const std::string kStretchPrefix_scrypt = "scrypt "; static const char* kFn_encrypted_key = "encrypted_key"; static const char* kFn_keymaster_key_blob = "keymaster_key_blob"; +static const char* kFn_salt = "salt"; static const char* kFn_secdiscardable = "secdiscardable"; static const char* kFn_stretching = "stretching"; static const char* kFn_version = "version"; @@ -165,15 +179,64 @@ static bool writeStringToFile(const std::string &payload, const std::string &fil return true; } -static keymaster::AuthorizationSet buildParams( - const KeyAuthentication &auth, const std::string &secdiscardable) { +static std::string getStretching() { + char paramstr[PROPERTY_VALUE_MAX]; + + property_get(SCRYPT_PROP, paramstr, SCRYPT_DEFAULTS); + return std::string() + kStretchPrefix_scrypt + paramstr; +} + +static bool stretchingNeedsSalt(const std::string &stretching) { + return stretching != kStretch_nopassword && stretching != kStretch_none; +} + +static bool stretchSecret(const std::string &stretching, const std::string &secret, + const std::string &salt, std::string &stretched) { + if (stretching == kStretch_nopassword) { + if (!secret.empty()) { + LOG(ERROR) << "Password present but stretching is nopasswd"; + // Continue anyway + } + stretched.clear(); + } else if (stretching == kStretch_none) { + stretched = secret; + } else if (std::equal(kStretchPrefix_scrypt.begin(), + kStretchPrefix_scrypt.end(), stretching.begin())) { + int Nf, rf, pf; + if (!parse_scrypt_parameters( + stretching.substr(kStretchPrefix_scrypt.size()).c_str(), &Nf, &rf, &pf)) { + LOG(ERROR) << "Unable to parse scrypt params in stretching: " << stretching; + return false; + } + stretched.assign(STRETCHED_BYTES, '\0'); + if (crypto_scrypt( + reinterpret_cast(secret.data()), secret.size(), + reinterpret_cast(salt.data()), salt.size(), + 1 << Nf, 1 << rf, 1 << pf, + reinterpret_cast(&stretched[0]), stretched.size()) != 0) { + LOG(ERROR) << "scrypt failed with params: " << stretching; + return false; + } + } else { + LOG(ERROR) << "Unknown stretching type: " << stretching; + return false; + } + return true; +} + +static bool buildParams(const KeyAuthentication &auth, const std::string &stretching, + const std::string &salt, const std::string &secdiscardable, + keymaster::AuthorizationSet &result) { + std::string stretched; + if (!stretchSecret(stretching, auth.secret, salt, stretched)) return false; + auto appId = hashSecdiscardable(secdiscardable) + stretched; keymaster::AuthorizationSetBuilder paramBuilder; - auto appId = hashSecdiscardable(secdiscardable) + auth.secret; addStringParam(paramBuilder, keymaster::TAG_APPLICATION_ID, appId); if (!auth.token.empty()) { addStringParam(paramBuilder, keymaster::TAG_AUTH_TOKEN, auth.token); } - return paramBuilder.build(); + result = paramBuilder.build(); + return true; } bool storeKey(const std::string &dir, const KeyAuthentication &auth, const std::string &key) { @@ -189,17 +252,24 @@ bool storeKey(const std::string &dir, const KeyAuthentication &auth, const std:: return false; } if (!writeStringToFile(secdiscardable, dir + "/" + kFn_secdiscardable)) return false; - // Future proofing for when we add key stretching per b/27056334 - auto stretching = auth.secret.empty() ? "nopassword" : "none"; + std::string stretching = auth.secret.empty() ? kStretch_nopassword : getStretching(); if (!writeStringToFile(stretching, dir + "/" + kFn_stretching)) return false; - auto extraParams = buildParams(auth, secdiscardable); + std::string salt; + if (stretchingNeedsSalt(stretching)) { + if (ReadRandomBytes(SALT_BYTES, salt) != OK) { + LOG(ERROR) << "Random read failed"; + return false; + } + if (!writeStringToFile(salt, dir + "/" + kFn_salt)) return false; + } + keymaster::AuthorizationSet extraParams; + if (!buildParams(auth, stretching, salt, secdiscardable, extraParams)) return false; Keymaster keymaster; if (!keymaster) 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 (!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; @@ -214,7 +284,14 @@ bool retrieveKey(const std::string &dir, const KeyAuthentication &auth, std::str } std::string secdiscardable; if (!readFileToString(dir + "/" + kFn_secdiscardable, secdiscardable)) return false; - auto extraParams = buildParams(auth, secdiscardable); + std::string stretching; + if (!readFileToString(dir + "/" + kFn_stretching, stretching)) return false; + std::string salt; + if (stretchingNeedsSalt(stretching)) { + if (!readFileToString(dir + "/" + kFn_salt, salt)) return false; + } + keymaster::AuthorizationSet extraParams; + if (!buildParams(auth, stretching, salt, secdiscardable, extraParams)) return false; std::string kmKey; if (!readFileToString(dir + "/" + kFn_keymaster_key_blob, kmKey)) return false; std::string encryptedMessage; diff --git a/ScryptParameters.cpp b/ScryptParameters.cpp new file mode 100644 index 0000000..669809b --- /dev/null +++ b/ScryptParameters.cpp @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * 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 "ScryptParameters.h" + +#include +#include + +bool parse_scrypt_parameters(const char* paramstr, int *Nf, int *rf, int *pf) { + int params[3]; + char *token; + char *saveptr; + int i; + + /* + * The token we're looking for should be three integers separated by + * colons (e.g., "12:8:1"). Scan the property to make sure it matches. + */ + for (i = 0, token = strtok_r(const_cast(paramstr), ":", &saveptr); + token != nullptr && i < 3; + i++, token = strtok_r(nullptr, ":", &saveptr)) { + char *endptr; + params[i] = strtol(token, &endptr, 10); + + /* + * Check that there was a valid number and it's 8-bit. + */ + if ((*token == '\0') || (*endptr != '\0') || params[i] < 0 || params[i] > 255) { + return false; + } + } + if (token != nullptr) { + return false; + } + *Nf = params[0]; *rf = params[1]; *pf = params[2]; + return true; +} diff --git a/ScryptParameters.h b/ScryptParameters.h new file mode 100644 index 0000000..1b43ea5 --- /dev/null +++ b/ScryptParameters.h @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * 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. + */ + +#ifndef ANDROID_VOLD_SCRYPT_PARAMETERS_H +#define ANDROID_VOLD_SCRYPT_PARAMETERS_H + +#include +#include + +#define SCRYPT_PROP "ro.crypto.scrypt_params" +#define SCRYPT_DEFAULTS "15:3:1" + +__BEGIN_DECLS + +bool parse_scrypt_parameters(const char* paramstr, int *Nf, int *rf, int *pf); + +__END_DECLS + +#endif diff --git a/cryptfs.c b/cryptfs.c index 65acb60..b99dd56 100644 --- a/cryptfs.c +++ b/cryptfs.c @@ -52,6 +52,7 @@ #include "cutils/android_reboot.h" #include "hardware_legacy/power.h" #include +#include "ScryptParameters.h" #include "VolumeManager.h" #include "VoldUtil.h" #include "crypto_scrypt.h" @@ -475,48 +476,17 @@ static void ioctl_init(struct dm_ioctl *io, size_t dataSize, const char *name, u * given device. */ static void get_device_scrypt_params(struct crypt_mnt_ftr *ftr) { - const int default_params[] = SCRYPT_DEFAULTS; - int params[] = SCRYPT_DEFAULTS; char paramstr[PROPERTY_VALUE_MAX]; - char *token; - char *saveptr; - int i; - - property_get(SCRYPT_PROP, paramstr, ""); - if (paramstr[0] != '\0') { - /* - * The token we're looking for should be three integers separated by - * colons (e.g., "12:8:1"). Scan the property to make sure it matches. - */ - for (i = 0, token = strtok_r(paramstr, ":", &saveptr); - token != NULL && i < 3; - i++, token = strtok_r(NULL, ":", &saveptr)) { - char *endptr; - params[i] = strtol(token, &endptr, 10); - - /* - * Check that there was a valid number and it's 8-bit. If not, - * break out and the end check will take the default values. - */ - if ((*token == '\0') || (*endptr != '\0') || params[i] < 0 || params[i] > 255) { - break; - } - } + int Nf, rf, pf; - /* - * If there were not enough tokens or a token was malformed (not an - * integer), it will end up here and the default parameters can be - * taken. - */ - if ((i != 3) || (token != NULL)) { - SLOGW("bad scrypt parameters '%s' should be like '12:8:1'; using defaults", paramstr); - memcpy(params, default_params, sizeof(params)); - } + property_get(SCRYPT_PROP, paramstr, SCRYPT_DEFAULTS); + if (!parse_scrypt_parameters(paramstr, &Nf, &rf, &pf)) { + SLOGW("bad scrypt parameters '%s' should be like '12:8:1'; using defaults", paramstr); + parse_scrypt_parameters(SCRYPT_DEFAULTS, &Nf, &rf, &pf); } - - ftr->N_factor = params[0]; - ftr->r_factor = params[1]; - ftr->p_factor = params[2]; + ftr->N_factor = Nf; + ftr->r_factor = rf; + ftr->p_factor = pf; } static unsigned int get_fs_size(char *dev) diff --git a/cryptfs.h b/cryptfs.h index 033767f..fbcec4e 100644 --- a/cryptfs.h +++ b/cryptfs.h @@ -76,9 +76,6 @@ #define CRYPT_MNT_MAGIC 0xD0B5B1C4 #define PERSIST_DATA_MAGIC 0xE950CD44 -#define SCRYPT_PROP "ro.crypto.scrypt_params" -#define SCRYPT_DEFAULTS { 15, 3, 1 } - /* Key Derivation Function algorithms */ #define KDF_PBKDF2 1 #define KDF_SCRYPT 2