ash: refactor crypto in TokenEncryptor
This change migrates TokenEncryptor to the new crypto/aes_ctr and crypto/kdf interfaces, which allows deleting a bunch of failure paths that are now impossible. This change also documents how TokenEncryptor's encryption works and the weaknesses of the scheme. Bug: 372283556 Change-Id: I683df58a45139c173b43515853313f705687a214 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6056044 Reviewed-by: Sergey Poromov <poromov@chromium.org> Reviewed-by: David Benjamin <davidben@chromium.org> Commit-Queue: Elly FJ <ellyjones@chromium.org> Cr-Commit-Position: refs/heads/main@{#1394877}
This commit is contained in:
Elly
@ -11,34 +11,37 @@
|
||||
#include <vector>
|
||||
|
||||
#include "base/check_is_test.h"
|
||||
#include "base/containers/span.h"
|
||||
#include "base/logging.h"
|
||||
#include "base/strings/string_number_conversions.h"
|
||||
#include "base/strings/string_util.h"
|
||||
#include "base/system/sys_info.h"
|
||||
#include "chromeos/ash/components/cryptohome/system_salt_getter.h"
|
||||
#include "crypto/encryptor.h"
|
||||
#include "crypto/nss_util.h"
|
||||
#include "crypto/aes_ctr.h"
|
||||
#include "crypto/kdf.h"
|
||||
#include "crypto/random.h"
|
||||
#include "crypto/sha2.h"
|
||||
#include "crypto/symmetric_key.h"
|
||||
#include "crypto/subtle_passkey.h"
|
||||
|
||||
namespace ash {
|
||||
|
||||
namespace {
|
||||
const size_t kNonceSize = 16;
|
||||
|
||||
constexpr crypto::kdf::Pbkdf2HmacSha1Params kPbkdf2Params = {
|
||||
.iterations = 1000,
|
||||
};
|
||||
|
||||
} // namespace
|
||||
|
||||
CryptohomeTokenEncryptor::CryptohomeTokenEncryptor(
|
||||
const std::string& system_salt)
|
||||
: system_salt_(system_salt) {
|
||||
DCHECK(!system_salt.empty());
|
||||
// TODO(davidroche): should this use the system salt for both the password
|
||||
// and the salt value, or should this use a separate salt value?
|
||||
system_salt_key_ = PassphraseToKey(system_salt_, system_salt_);
|
||||
const std::string& system_salt) {
|
||||
CHECK(!system_salt.empty());
|
||||
|
||||
auto salt = base::as_byte_span(system_salt);
|
||||
crypto::kdf::DeriveKeyPbkdf2HmacSha1(kPbkdf2Params, salt, salt, key_,
|
||||
crypto::SubtlePassKey{});
|
||||
base::span(nonce_).copy_from(salt.first<kNonceSize>());
|
||||
}
|
||||
|
||||
CryptohomeTokenEncryptor::~CryptohomeTokenEncryptor() {
|
||||
}
|
||||
CryptohomeTokenEncryptor::~CryptohomeTokenEncryptor() {}
|
||||
|
||||
std::string CryptohomeTokenEncryptor::EncryptWithSystemSalt(
|
||||
std::string_view token) {
|
||||
@ -46,32 +49,16 @@ std::string CryptohomeTokenEncryptor::EncryptWithSystemSalt(
|
||||
if (!base::SysInfo::IsRunningOnChromeOS())
|
||||
return std::string(token);
|
||||
|
||||
if (!system_salt_key_) {
|
||||
LOG(WARNING) << "System salt key is not available for encrypt.";
|
||||
return std::string();
|
||||
}
|
||||
|
||||
// Encrypt the token using the system salt as the key and a nonce as the
|
||||
// counter.
|
||||
crypto::Encryptor encryptor;
|
||||
if (!encryptor.Init(system_salt_key_.get(), crypto::Encryptor::CTR,
|
||||
std::string())) {
|
||||
LOG(WARNING) << "Failed to initialize Encryptor.";
|
||||
return std::string();
|
||||
}
|
||||
std::array<uint8_t, kNonceSize> nonce;
|
||||
crypto::RandBytes(nonce);
|
||||
CHECK(encryptor.SetCounter(nonce));
|
||||
std::string encoded_token;
|
||||
if (!encryptor.Encrypt(token, &encoded_token)) {
|
||||
LOG(WARNING) << "Failed to encrypt token.";
|
||||
return std::string();
|
||||
}
|
||||
|
||||
auto ciphertext =
|
||||
crypto::aes_ctr::Encrypt(key_, nonce, base::as_byte_span(token));
|
||||
|
||||
// Return a concatenation of the nonce (counter) and the encrypted data, both
|
||||
// hex-encoded.
|
||||
return base::ToLowerASCII(base::HexEncode(nonce) +
|
||||
base::HexEncode(encoded_token));
|
||||
base::HexEncode(ciphertext));
|
||||
}
|
||||
|
||||
std::string CryptohomeTokenEncryptor::DecryptWithSystemSalt(
|
||||
@ -81,11 +68,6 @@ std::string CryptohomeTokenEncryptor::DecryptWithSystemSalt(
|
||||
return std::string(encrypted_token_hex);
|
||||
}
|
||||
|
||||
if (!system_salt_key_) {
|
||||
LOG(WARNING) << "System salt key is not available for decrypt.";
|
||||
return std::string();
|
||||
}
|
||||
|
||||
// Convert the encrypted token from hex to binary and then split out the
|
||||
// counter at the start from the rest of the payload.
|
||||
std::string encrypted_token;
|
||||
@ -97,24 +79,12 @@ std::string CryptohomeTokenEncryptor::DecryptWithSystemSalt(
|
||||
LOG(WARNING) << "Corrupt encrypted token found, too short.";
|
||||
return std::string();
|
||||
}
|
||||
std::string_view encrypted_token_view = encrypted_token;
|
||||
std::string_view counter = encrypted_token_view.substr(0, kNonceSize);
|
||||
std::string_view payload = encrypted_token_view.substr(kNonceSize);
|
||||
|
||||
// Use the salt+nonce to decrypt the
|
||||
crypto::Encryptor encryptor;
|
||||
if (!encryptor.Init(system_salt_key_.get(), crypto::Encryptor::CTR,
|
||||
std::string())) {
|
||||
LOG(WARNING) << "Failed to initialize Encryptor.";
|
||||
return std::string();
|
||||
}
|
||||
std::string token;
|
||||
CHECK(encryptor.SetCounter(counter));
|
||||
if (!encryptor.Decrypt(payload, &token)) {
|
||||
LOG(WARNING) << "Failed to decrypt token.";
|
||||
return std::string();
|
||||
}
|
||||
return token;
|
||||
auto nonce = base::as_byte_span(encrypted_token).first<kNonceSize>();
|
||||
auto payload = base::as_byte_span(encrypted_token).subspan<kNonceSize>();
|
||||
|
||||
return std::string(
|
||||
base::as_string_view(crypto::aes_ctr::Decrypt(key_, nonce, payload)));
|
||||
}
|
||||
|
||||
std::string CryptohomeTokenEncryptor::WeakEncryptWithSystemSalt(
|
||||
@ -127,29 +97,8 @@ std::string CryptohomeTokenEncryptor::WeakEncryptWithSystemSalt(
|
||||
return token;
|
||||
}
|
||||
|
||||
if (!system_salt_key_) {
|
||||
LOG(WARNING) << "System salt key is not available for encrypt.";
|
||||
return std::string();
|
||||
}
|
||||
|
||||
// Encrypt the token using the system salt as both the key and the counter.
|
||||
// Note that using the salt for both of these things is problematic, which is
|
||||
// why this encryption is "weak".
|
||||
crypto::Encryptor encryptor;
|
||||
if (!encryptor.Init(system_salt_key_.get(), crypto::Encryptor::CTR,
|
||||
std::string())) {
|
||||
LOG(WARNING) << "Failed to initialize Encryptor.";
|
||||
return std::string();
|
||||
}
|
||||
std::string nonce = system_salt_.substr(0, kNonceSize);
|
||||
std::string encoded_token;
|
||||
CHECK(encryptor.SetCounter(nonce));
|
||||
if (!encryptor.Encrypt(token, &encoded_token)) {
|
||||
LOG(WARNING) << "Failed to encrypt token.";
|
||||
return std::string();
|
||||
}
|
||||
|
||||
return base::ToLowerASCII(base::HexEncode(encoded_token));
|
||||
return base::ToLowerASCII(base::HexEncode(
|
||||
crypto::aes_ctr::Encrypt(key_, nonce_, base::as_byte_span(token))));
|
||||
}
|
||||
|
||||
std::string CryptohomeTokenEncryptor::WeakDecryptWithSystemSalt(
|
||||
@ -159,39 +108,13 @@ std::string CryptohomeTokenEncryptor::WeakDecryptWithSystemSalt(
|
||||
return encrypted_token_hex;
|
||||
}
|
||||
|
||||
if (!system_salt_key_) {
|
||||
LOG(WARNING) << "System salt key is not available for decrypt.";
|
||||
return std::string();
|
||||
}
|
||||
|
||||
std::string encrypted_token;
|
||||
if (!base::HexStringToString(encrypted_token_hex, &encrypted_token)) {
|
||||
std::vector<uint8_t> encrypted_token;
|
||||
if (!base::HexStringToBytes(encrypted_token_hex, &encrypted_token)) {
|
||||
LOG(WARNING) << "Corrupt encrypted token found.";
|
||||
return std::string();
|
||||
}
|
||||
|
||||
crypto::Encryptor encryptor;
|
||||
if (!encryptor.Init(system_salt_key_.get(), crypto::Encryptor::CTR,
|
||||
std::string())) {
|
||||
LOG(WARNING) << "Failed to initialize Encryptor.";
|
||||
return std::string();
|
||||
}
|
||||
|
||||
std::string nonce = system_salt_.substr(0, kNonceSize);
|
||||
std::string token;
|
||||
CHECK(encryptor.SetCounter(nonce));
|
||||
if (!encryptor.Decrypt(encrypted_token, &token)) {
|
||||
LOG(WARNING) << "Failed to decrypt token.";
|
||||
return std::string();
|
||||
}
|
||||
return token;
|
||||
}
|
||||
|
||||
std::unique_ptr<crypto::SymmetricKey> CryptohomeTokenEncryptor::PassphraseToKey(
|
||||
const std::string& passphrase,
|
||||
const std::string& salt) {
|
||||
return crypto::SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
|
||||
crypto::SymmetricKey::AES, passphrase, salt, 1000, 256);
|
||||
return std::string(base::as_string_view(
|
||||
crypto::aes_ctr::Decrypt(key_, nonce_, encrypted_token)));
|
||||
}
|
||||
|
||||
} // namespace ash
|
||||
|
||||
@ -5,18 +5,32 @@
|
||||
#ifndef CHROME_BROWSER_ASH_SETTINGS_TOKEN_ENCRYPTOR_H_
|
||||
#define CHROME_BROWSER_ASH_SETTINGS_TOKEN_ENCRYPTOR_H_
|
||||
|
||||
#include <array>
|
||||
#include <memory>
|
||||
#include <string>
|
||||
#include <string_view>
|
||||
|
||||
namespace crypto {
|
||||
class SymmetricKey;
|
||||
}
|
||||
|
||||
namespace ash {
|
||||
|
||||
// Interface class for classes that encrypt and decrypt tokens using the
|
||||
// system salt.
|
||||
//
|
||||
// This class supports two methods of encryption: the old "weak" method and the
|
||||
// new method. Unfortunately neither method provides actual confidentiality
|
||||
// protection and both have completely equivalent strength. Both methods rely on
|
||||
// the system salt, which is stored in the clear on disk. The old method is:
|
||||
//
|
||||
// AES-256-CTR(key = salt, counter = salt[0 .. 15], token)
|
||||
//
|
||||
// and the new method is:
|
||||
//
|
||||
// counter = random_bytes(16)
|
||||
// counter || AES-256-CTR(key = salt, counter = counter, token)
|
||||
//
|
||||
// so in both cases there are no actual secrets involved as either key or
|
||||
// counter. The only practical distinction is that the new method generates
|
||||
// larger encoded values and that two separate encryptions of the same token
|
||||
// will yield different encoded values.
|
||||
class TokenEncryptor {
|
||||
public:
|
||||
virtual ~TokenEncryptor() = default;
|
||||
@ -62,18 +76,11 @@ class CryptohomeTokenEncryptor : public TokenEncryptor {
|
||||
const std::string& encrypted_token_hex) override;
|
||||
|
||||
private:
|
||||
// Converts |passphrase| to a SymmetricKey using the given |salt|.
|
||||
std::unique_ptr<crypto::SymmetricKey> PassphraseToKey(
|
||||
const std::string& passphrase,
|
||||
const std::string& salt);
|
||||
static constexpr size_t kDerivedKeySize = 32;
|
||||
static constexpr size_t kNonceSize = 16;
|
||||
|
||||
// The cached system salt passed to the constructor, originally coming
|
||||
// from cryptohome daemon.
|
||||
std::string system_salt_;
|
||||
|
||||
// A key based on the system salt. Useful for encrypting device-level
|
||||
// data for which we have no additional credentials.
|
||||
std::unique_ptr<crypto::SymmetricKey> system_salt_key_;
|
||||
std::array<uint8_t, kDerivedKeySize> key_;
|
||||
std::array<uint8_t, kNonceSize> nonce_;
|
||||
};
|
||||
|
||||
} // namespace ash
|
||||
|
||||
@ -7,6 +7,10 @@
|
||||
|
||||
#include "crypto/crypto_export.h"
|
||||
|
||||
namespace ash {
|
||||
class CryptohomeTokenEncryptor;
|
||||
}
|
||||
|
||||
namespace syncer {
|
||||
class Nigori;
|
||||
}
|
||||
@ -42,6 +46,10 @@ class CRYPTO_EXPORT SubtlePassKey final {
|
||||
// SymmetricKey.
|
||||
friend class SymmetricKey;
|
||||
|
||||
// This class uses custom PBKDF2 parameters, and has to keep doing so for
|
||||
// compatibility with persisted data on disk.
|
||||
friend class ash::CryptohomeTokenEncryptor;
|
||||
|
||||
// This class uses custom PBKDF2 parameters - the Nigori spec requires this.
|
||||
friend class syncer::Nigori;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user