Small BlobCipher and SimKmsConnector fixes and changes (#6936)

* SimKmsConnector fix domain id being unsigned
* SimKmsConnector fix returning cipher id 0 as latest key, which is invalid
* SimKmsConnector fix keys initialized as c-style strings with incorrect length and uninitialized bytes
* SimKmsConnector fix returning different keys for the same id after restart
* BlobCipher change APIs to return null reference when key not found
* BlobCipher insertCipherKey to return the inserted key
This commit is contained in:
Yi Wu 2022-05-04 14:09:31 -07:00 committed by GitHub
parent c1c316591c
commit 66f1c5c85a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 103 additions and 84 deletions

View File

@ -232,7 +232,6 @@ ACTOR Future<Void> getLatestCipherKeys(Reference<EncryptKeyProxyData> ekpProxyDa
EKPGetLatestBaseCipherKeysRequest req) {
// Scan the cached cipher-keys and filter our baseCipherIds locally cached
// for the rest, reachout to KMS to fetch the required details
state std::vector<EKPBaseCipherDetails> cachedCipherDetails;
state EKPGetLatestBaseCipherKeysRequest latestKeysReq = req;
state EKPGetLatestBaseCipherKeysReply latestCipherReply;

View File

@ -29,14 +29,13 @@
#include "fdbclient/FDBTypes.h"
#include "fdbrpc/fdbrpc.h"
#include "flow/EncryptUtils.h"
#include "flow/FileIdentifier.h"
#include "flow/Trace.h"
#include "flow/flow.h"
#include "flow/network.h"
#include "flow/actorcompiler.h" // This must be the last #include.
using SimEncryptKeyId = uint64_t;
using SimEncryptDomainId = uint64_t;
using SimEncryptKey = std::string;
struct SimKmsProxyInterface {
@ -73,12 +72,15 @@ struct SimKmsProxyInterface {
struct SimEncryptKeyDetails {
constexpr static FileIdentifier file_identifier = 1227025;
SimEncryptDomainId encryptDomainId;
SimEncryptKeyId encryptKeyId;
EncryptCipherDomainId encryptDomainId;
EncryptCipherBaseKeyId encryptKeyId;
StringRef encryptKey;
SimEncryptKeyDetails() {}
explicit SimEncryptKeyDetails(SimEncryptDomainId domainId, SimEncryptKeyId keyId, StringRef key, Arena& arena)
explicit SimEncryptKeyDetails(EncryptCipherDomainId domainId,
EncryptCipherBaseKeyId keyId,
StringRef key,
Arena& arena)
: encryptDomainId(domainId), encryptKeyId(keyId), encryptKey(StringRef(arena, key)) {}
template <class Ar>
@ -102,11 +104,12 @@ struct SimGetEncryptKeysByKeyIdsReply {
struct SimGetEncryptKeysByKeyIdsRequest {
constexpr static FileIdentifier file_identifier = 6913396;
std::vector<std::pair<SimEncryptKeyId, SimEncryptDomainId>> encryptKeyIds;
std::vector<std::pair<EncryptCipherBaseKeyId, EncryptCipherDomainId>> encryptKeyIds;
ReplyPromise<SimGetEncryptKeysByKeyIdsReply> reply;
SimGetEncryptKeysByKeyIdsRequest() {}
explicit SimGetEncryptKeysByKeyIdsRequest(const std::vector<std::pair<SimEncryptKeyId, SimEncryptDomainId>>& keyIds)
explicit SimGetEncryptKeysByKeyIdsRequest(
const std::vector<std::pair<EncryptCipherBaseKeyId, EncryptCipherDomainId>>& keyIds)
: encryptKeyIds(keyIds) {}
template <class Ar>
@ -130,11 +133,12 @@ struct SimGetEncryptKeyByDomainIdReply {
struct SimGetEncryptKeysByDomainIdsRequest {
constexpr static FileIdentifier file_identifier = 9918682;
std::vector<SimEncryptDomainId> encryptDomainIds;
std::vector<EncryptCipherDomainId> encryptDomainIds;
ReplyPromise<SimGetEncryptKeyByDomainIdReply> reply;
SimGetEncryptKeysByDomainIdsRequest() {}
explicit SimGetEncryptKeysByDomainIdsRequest(const std::vector<SimEncryptDomainId>& ids) : encryptDomainIds(ids) {}
explicit SimGetEncryptKeysByDomainIdsRequest(const std::vector<EncryptCipherDomainId>& ids)
: encryptDomainIds(ids) {}
template <class Ar>
void serialize(Ar& ar) {

View File

@ -23,6 +23,7 @@
#include "fdbrpc/sim_validation.h"
#include "fdbserver/Knobs.h"
#include "flow/ActorCollection.h"
#include "flow/BlobCipher.h"
#include "flow/EncryptUtils.h"
#include "flow/Error.h"
#include "flow/FastRef.h"
@ -42,7 +43,7 @@ struct SimEncryptKeyCtx {
EncryptCipherBaseKeyId id;
SimEncryptKey key;
explicit SimEncryptKeyCtx(EncryptCipherBaseKeyId kId, const char* data) : id(kId), key(data) {}
explicit SimEncryptKeyCtx(EncryptCipherBaseKeyId kId, const char* data) : id(kId), key(data, AES_256_KEY_LENGTH) {}
};
struct SimKmsConnectorContext {
@ -50,13 +51,16 @@ struct SimKmsConnectorContext {
std::unordered_map<EncryptCipherBaseKeyId, std::unique_ptr<SimEncryptKeyCtx>> simEncryptKeyStore;
explicit SimKmsConnectorContext(uint32_t keyCount) : maxEncryptionKeys(keyCount) {
uint8_t buffer[AES_256_KEY_LENGTH];
const unsigned char SHA_KEY[] = "0c39e7906db6d51ac0573d328ce1b6be";
// Construct encryption keyStore.
for (int i = 0; i < maxEncryptionKeys; i++) {
generateRandomData(&buffer[0], AES_256_KEY_LENGTH);
SimEncryptKeyCtx ctx(i, reinterpret_cast<const char*>(buffer));
simEncryptKeyStore[i] = std::make_unique<SimEncryptKeyCtx>(i, reinterpret_cast<const char*>(buffer));
// Note the keys generated must be the same after restart.
for (int i = 1; i <= maxEncryptionKeys; i++) {
Arena arena;
StringRef digest = computeAuthToken(
reinterpret_cast<const unsigned char*>(&i), sizeof(i), SHA_KEY, AES_256_KEY_LENGTH, arena);
simEncryptKeyStore[i] =
std::make_unique<SimEncryptKeyCtx>(i, reinterpret_cast<const char*>(digest.begin()));
}
}
};
@ -103,7 +107,7 @@ ACTOR Future<Void> simKmsConnectorCore_impl(KmsConnectorInterface interf) {
// mean multiple domains gets mapped to the same encryption key which is fine, the EncryptKeyStore
// guarantees that keyId -> plaintext encryptKey mapping is idempotent.
for (EncryptCipherDomainId domainId : req.encryptDomainIds) {
EncryptCipherBaseKeyId keyId = domainId % SERVER_KNOBS->SIM_KMS_MAX_KEYS;
EncryptCipherBaseKeyId keyId = 1 + abs(domainId) % SERVER_KNOBS->SIM_KMS_MAX_KEYS;
const auto& itr = ctx->simEncryptKeyStore.find(keyId);
if (itr != ctx->simEncryptKeyStore.end()) {
keysByDomainIdRep.cipherKeyDetails.emplace_back(

View File

@ -114,11 +114,10 @@ void BlobCipherKey::reset() {
// BlobKeyIdCache class methods
BlobCipherKeyIdCache::BlobCipherKeyIdCache()
: domainId(ENCRYPT_INVALID_DOMAIN_ID), latestBaseCipherKeyId(ENCRYPT_INVALID_CIPHER_KEY_ID),
latestRandomSalt(ENCRYPT_INVALID_RANDOM_SALT) {}
: domainId(ENCRYPT_INVALID_DOMAIN_ID), latestBaseCipherKeyId(), latestRandomSalt() {}
BlobCipherKeyIdCache::BlobCipherKeyIdCache(EncryptCipherDomainId dId)
: domainId(dId), latestBaseCipherKeyId(ENCRYPT_INVALID_CIPHER_KEY_ID), latestRandomSalt(ENCRYPT_INVALID_RANDOM_SALT) {
: domainId(dId), latestBaseCipherKeyId(), latestRandomSalt() {
TraceEvent("Init_BlobCipherKeyIdCache").detail("DomainId", domainId);
}
@ -131,57 +130,45 @@ BlobCipherKeyIdCacheKey BlobCipherKeyIdCache::getCacheKey(const EncryptCipherBas
}
Reference<BlobCipherKey> BlobCipherKeyIdCache::getLatestCipherKey() {
if (keyIdCache.empty()) {
// Cache is empty, nothing more to do.
throw encrypt_key_not_found();
if (!latestBaseCipherKeyId.present()) {
return Reference<BlobCipherKey>();
}
ASSERT_NE(latestBaseCipherKeyId.get(), ENCRYPT_INVALID_CIPHER_KEY_ID);
ASSERT(latestRandomSalt.present());
ASSERT_NE(latestRandomSalt.get(), ENCRYPT_INVALID_RANDOM_SALT);
// Ensure latestCipher details sanity
ASSERT_GT(latestBaseCipherKeyId, ENCRYPT_INVALID_CIPHER_KEY_ID);
ASSERT_GT(latestRandomSalt, ENCRYPT_INVALID_RANDOM_SALT);
return getCipherByBaseCipherId(latestBaseCipherKeyId, latestRandomSalt);
return getCipherByBaseCipherId(latestBaseCipherKeyId.get(), latestRandomSalt.get());
}
Reference<BlobCipherKey> BlobCipherKeyIdCache::getCipherByBaseCipherId(const EncryptCipherBaseKeyId& baseCipherKeyId,
const EncryptCipherRandomSalt& salt) {
BlobCipherKeyIdCacheMapCItr itr = keyIdCache.find(getCacheKey(baseCipherKeyId, salt));
if (itr == keyIdCache.end()) {
TraceEvent("CipherByBaseCipherId_KeyMissing")
.detail("DomainId", domainId)
.detail("BaseCipherId", baseCipherKeyId)
.detail("Salt", salt);
throw encrypt_key_not_found();
return Reference<BlobCipherKey>();
}
return itr->second;
}
void BlobCipherKeyIdCache::insertBaseCipherKey(const EncryptCipherBaseKeyId& baseCipherId,
const uint8_t* baseCipher,
int baseCipherLen) {
Reference<BlobCipherKey> BlobCipherKeyIdCache::insertBaseCipherKey(const EncryptCipherBaseKeyId& baseCipherId,
const uint8_t* baseCipher,
int baseCipherLen) {
ASSERT_GT(baseCipherId, ENCRYPT_INVALID_CIPHER_KEY_ID);
// BaseCipherKeys are immutable, given the routine invocation updates 'latestCipher',
// ensure no key-tampering is done
try {
Reference<BlobCipherKey> cipherKey = getLatestCipherKey();
if (cipherKey.isValid() && cipherKey->getBaseCipherId() == baseCipherId) {
if (memcmp(cipherKey->rawBaseCipher(), baseCipher, baseCipherLen) == 0) {
TraceEvent("InsertBaseCipherKey_AlreadyPresent")
.detail("BaseCipherKeyId", baseCipherId)
.detail("DomainId", domainId);
// Key is already present; nothing more to do.
return;
} else {
TraceEvent("InsertBaseCipherKey_UpdateCipher")
.detail("BaseCipherKeyId", baseCipherId)
.detail("DomainId", domainId);
throw encrypt_update_cipher();
}
}
} catch (Error& e) {
if (e.code() != error_code_encrypt_key_not_found) {
throw e;
Reference<BlobCipherKey> latestCipherKey = getLatestCipherKey();
if (latestCipherKey.isValid() && latestCipherKey->getBaseCipherId() == baseCipherId) {
if (memcmp(latestCipherKey->rawBaseCipher(), baseCipher, baseCipherLen) == 0) {
TraceEvent("InsertBaseCipherKey_AlreadyPresent")
.detail("BaseCipherKeyId", baseCipherId)
.detail("DomainId", domainId);
// Key is already present; nothing more to do.
return latestCipherKey;
} else {
TraceEvent("InsertBaseCipherKey_UpdateCipher")
.detail("BaseCipherKeyId", baseCipherId)
.detail("DomainId", domainId);
throw encrypt_update_cipher();
}
}
@ -193,14 +180,16 @@ void BlobCipherKeyIdCache::insertBaseCipherKey(const EncryptCipherBaseKeyId& bas
// Update the latest BaseCipherKeyId for the given encryption domain
latestBaseCipherKeyId = baseCipherId;
latestRandomSalt = cipherKey->getSalt();
return cipherKey;
}
void BlobCipherKeyIdCache::insertBaseCipherKey(const EncryptCipherBaseKeyId& baseCipherId,
const uint8_t* baseCipher,
int baseCipherLen,
const EncryptCipherRandomSalt& salt) {
ASSERT_GT(baseCipherId, ENCRYPT_INVALID_CIPHER_KEY_ID);
ASSERT_GT(salt, ENCRYPT_INVALID_RANDOM_SALT);
ASSERT_NE(baseCipherId, ENCRYPT_INVALID_CIPHER_KEY_ID);
ASSERT_NE(salt, ENCRYPT_INVALID_RANDOM_SALT);
BlobCipherKeyIdCacheKey cacheKey = getCacheKey(baseCipherId, salt);
@ -244,10 +233,10 @@ std::vector<Reference<BlobCipherKey>> BlobCipherKeyIdCache::getAllCipherKeys() {
// BlobCipherKeyCache class methods
void BlobCipherKeyCache::insertCipherKey(const EncryptCipherDomainId& domainId,
const EncryptCipherBaseKeyId& baseCipherId,
const uint8_t* baseCipher,
int baseCipherLen) {
Reference<BlobCipherKey> BlobCipherKeyCache::insertCipherKey(const EncryptCipherDomainId& domainId,
const EncryptCipherBaseKeyId& baseCipherId,
const uint8_t* baseCipher,
int baseCipherLen) {
if (domainId == ENCRYPT_INVALID_DOMAIN_ID || baseCipherId == ENCRYPT_INVALID_CIPHER_KEY_ID) {
throw encrypt_invalid_id();
}
@ -257,12 +246,14 @@ void BlobCipherKeyCache::insertCipherKey(const EncryptCipherDomainId& domainId,
if (domainItr == domainCacheMap.end()) {
// Add mapping to track new encryption domain
Reference<BlobCipherKeyIdCache> keyIdCache = makeReference<BlobCipherKeyIdCache>(domainId);
keyIdCache->insertBaseCipherKey(baseCipherId, baseCipher, baseCipherLen);
Reference<BlobCipherKey> cipherKey =
keyIdCache->insertBaseCipherKey(baseCipherId, baseCipher, baseCipherLen);
domainCacheMap.emplace(domainId, keyIdCache);
return cipherKey;
} else {
// Track new baseCipher keys
Reference<BlobCipherKeyIdCache> keyIdCache = domainItr->second;
keyIdCache->insertBaseCipherKey(baseCipherId, baseCipher, baseCipherLen);
return keyIdCache->insertBaseCipherKey(baseCipherId, baseCipher, baseCipherLen);
}
TraceEvent("InsertCipherKey").detail("DomainId", domainId).detail("BaseCipherKeyId", baseCipherId);
@ -309,19 +300,23 @@ void BlobCipherKeyCache::insertCipherKey(const EncryptCipherDomainId& domainId,
}
Reference<BlobCipherKey> BlobCipherKeyCache::getLatestCipherKey(const EncryptCipherDomainId& domainId) {
if (domainId == ENCRYPT_INVALID_DOMAIN_ID) {
TraceEvent("GetLatestCipherKey_InvalidID").detail("DomainId", domainId);
throw encrypt_invalid_id();
}
auto domainItr = domainCacheMap.find(domainId);
if (domainItr == domainCacheMap.end()) {
TraceEvent("GetLatestCipherKey_DomainNotFound").detail("DomainId", domainId);
throw encrypt_key_not_found();
return Reference<BlobCipherKey>();
}
Reference<BlobCipherKeyIdCache> keyIdCache = domainItr->second;
Reference<BlobCipherKey> cipherKey = keyIdCache->getLatestCipherKey();
if ((now() - cipherKey->getCreationTime()) > FLOW_KNOBS->ENCRYPT_CIPHER_KEY_CACHE_TTL) {
if (cipherKey.isValid() && (now() - cipherKey->getCreationTime()) > FLOW_KNOBS->ENCRYPT_CIPHER_KEY_CACHE_TTL) {
TraceEvent("GetLatestCipherKey_ExpiredTTL")
.detail("DomainId", domainId)
.detail("BaseCipherId", cipherKey->getBaseCipherId());
throw encrypt_key_ttl_expired();
return Reference<BlobCipherKey>();
}
return cipherKey;
@ -332,8 +327,7 @@ Reference<BlobCipherKey> BlobCipherKeyCache::getCipherKey(const EncryptCipherDom
const EncryptCipherRandomSalt& salt) {
auto domainItr = domainCacheMap.find(domainId);
if (domainItr == domainCacheMap.end()) {
TraceEvent("GetCipherKey_MissingDomainId").detail("DomainId", domainId);
throw encrypt_key_not_found();
return Reference<BlobCipherKey>();
}
Reference<BlobCipherKeyIdCache> keyIdCache = domainItr->second;
@ -343,7 +337,7 @@ Reference<BlobCipherKey> BlobCipherKeyCache::getCipherKey(const EncryptCipherDom
void BlobCipherKeyCache::resetEncryptDomainId(const EncryptCipherDomainId domainId) {
auto domainItr = domainCacheMap.find(domainId);
if (domainItr == domainCacheMap.end()) {
throw encrypt_key_not_found();
return;
}
Reference<BlobCipherKeyIdCache> keyIdCache = domainItr->second;
@ -777,9 +771,22 @@ TEST_CASE("flow/BlobCipher") {
}
ASSERT_EQ(domainKeyMap.size(), maxDomainId);
Reference<BlobCipherKeyCache> cipherKeyCache = BlobCipherKeyCache::getInstance();
// validate getLatestCipherKey return empty when there's no cipher key
TraceEvent("BlobCipherTest_LatestKeyNotExists").log();
Reference<BlobCipherKey> latestKeyNonexists =
cipherKeyCache->getLatestCipherKey(deterministicRandom()->randomInt(minDomainId, maxDomainId));
ASSERT(!latestKeyNonexists.isValid());
try {
cipherKeyCache->getLatestCipherKey(ENCRYPT_INVALID_DOMAIN_ID);
ASSERT(false); // shouldn't get here
} catch (Error& e) {
ASSERT_EQ(e.code(), error_code_encrypt_invalid_id);
}
// insert BlobCipher keys into BlobCipherKeyCache map and validate
TraceEvent("BlobCipherTest_InsertKeys").log();
Reference<BlobCipherKeyCache> cipherKeyCache = BlobCipherKeyCache::getInstance();
for (auto& domainItr : domainKeyMap) {
for (auto& baseKeyItr : domainItr.second) {
Reference<BaseCipher> baseCipher = baseKeyItr.second;

View File

@ -244,12 +244,12 @@ public:
const EncryptCipherRandomSalt& salt);
// API returns the last inserted cipherKey.
// If none exists, 'encrypt_key_not_found' is thrown.
// If none exists, null reference is returned.
Reference<BlobCipherKey> getLatestCipherKey();
// API returns cipherKey corresponding to input 'baseCipherKeyId'.
// If none exists, 'encrypt_key_not_found' is thrown.
// If none exists, null reference is returned.
Reference<BlobCipherKey> getCipherByBaseCipherId(const EncryptCipherBaseKeyId& baseCipherKeyId,
const EncryptCipherRandomSalt& salt);
@ -257,17 +257,19 @@ public:
// API enables inserting base encryption cipher details to the BlobCipherKeyIdCache.
// Given cipherKeys are immutable, attempting to re-insert same 'identical' cipherKey
// is treated as a NOP (success), however, an attempt to update cipherKey would throw
// 'encrypt_update_cipher' exception.
// 'encrypt_update_cipher' exception. Returns the inserted cipher key if success.
//
// API NOTE: Recommended usecase is to update encryption cipher-key is updated the external
// keyManagementSolution to limit an encryption key lifetime
void insertBaseCipherKey(const EncryptCipherBaseKeyId& baseCipherId, const uint8_t* baseCipher, int baseCipherLen);
Reference<BlobCipherKey> insertBaseCipherKey(const EncryptCipherBaseKeyId& baseCipherId,
const uint8_t* baseCipher,
int baseCipherLen);
// API enables inserting base encryption cipher details to the BlobCipherKeyIdCache
// Given cipherKeys are immutable, attempting to re-insert same 'identical' cipherKey
// is treated as a NOP (success), however, an attempt to update cipherKey would throw
// 'encrypt_update_cipher' exception.
// 'encrypt_update_cipher' exception. Returns the inserted cipher key if sucess.
//
// API NOTE: Recommended usecase is to update encryption cipher-key regeneration while performing
// decryption. The encryptionheader would contain relevant details including: 'encryptDomainId',
@ -288,8 +290,8 @@ public:
private:
EncryptCipherDomainId domainId;
BlobCipherKeyIdCacheMap keyIdCache;
EncryptCipherBaseKeyId latestBaseCipherKeyId;
EncryptCipherRandomSalt latestRandomSalt;
Optional<EncryptCipherBaseKeyId> latestBaseCipherKeyId;
Optional<EncryptCipherRandomSalt> latestRandomSalt;
};
using BlobCipherDomainCacheMap = std::unordered_map<EncryptCipherDomainId, Reference<BlobCipherKeyIdCache>>;
@ -305,19 +307,21 @@ public:
// The cipherKeys are indexed using 'baseCipherId', given cipherKeys are immutable,
// attempting to re-insert same 'identical' cipherKey is treated as a NOP (success),
// however, an attempt to update cipherKey would throw 'encrypt_update_cipher' exception.
// Returns the inserted cipher key if success.
//
// API NOTE: Recommended usecase is to update encryption cipher-key is updated the external
// API NOTE: Recommended use case is to update encryption cipher-key is updated the external
// keyManagementSolution to limit an encryption key lifetime
void insertCipherKey(const EncryptCipherDomainId& domainId,
const EncryptCipherBaseKeyId& baseCipherId,
const uint8_t* baseCipher,
int baseCipherLen);
Reference<BlobCipherKey> insertCipherKey(const EncryptCipherDomainId& domainId,
const EncryptCipherBaseKeyId& baseCipherId,
const uint8_t* baseCipher,
int baseCipherLen);
// Enable clients to insert base encryption cipher details to the BlobCipherKeyCache.
// The cipherKeys are indexed using 'baseCipherId', given cipherKeys are immutable,
// attempting to re-insert same 'identical' cipherKey is treated as a NOP (success),
// however, an attempt to update cipherKey would throw 'encrypt_update_cipher' exception.
// Returns the inserted cipher key if success.
//
// API NOTE: Recommended usecase is to update encryption cipher-key regeneration while performing
// decryption. The encryptionheader would contain relevant details including: 'encryptDomainId',
@ -331,12 +335,13 @@ public:
const EncryptCipherRandomSalt& salt);
// API returns the last insert cipherKey for a given encryption domain Id.
// If none exists, it would throw 'encrypt_key_not_found' exception.
// If domain Id is invalid, it would throw 'encrypt_invalid_id' exception,
// otherwise, and if none exists, it would return null reference.
Reference<BlobCipherKey> getLatestCipherKey(const EncryptCipherDomainId& domainId);
// API returns cipherKey corresponding to {encryptionDomainId, baseCipherId} tuple.
// If none exists, it would throw 'encrypt_key_not_found' exception.
// If none exists, it would return null reference.
Reference<BlobCipherKey> getCipherKey(const EncryptCipherDomainId& domainId,
const EncryptCipherBaseKeyId& baseCipherId,