This is the first experimental feature to be deleted in the list published at PR #12400.
There is more code here than I anticipated. It is about 40,000 lines total, of which about three quarters are in dedicated files which I am deleting, and about one quarter is in shared files. That means about 10k lines in shared files, which is the stuff we tend to notice day to day (that plus the test failures on heretofore not-yet-disabled test cases, which I am now deleting).
I ran 3 million simulations, mostly against 692df86 or very similar code (differing by one TraceEvent). This was prior to syncing with upstream/main, which had no conflicts and from which I don't expect problems. The number of failures in these runs was about 8. We looked at them and believe there is a high likelihood that these are existing issues not related to the changes in this PR. More details on these failures can be found in docs linked from here: https://quip-apple.com/MN7gAyXLjgyn
* change Long Term status for unowned features for "scheduled for deletion" where applicable
* Relax wording about scheduled for deletion features
* Delete blob granule feature. WIP. Does not compile.
* more incremental hacking to remove / comment out blob granule related code
* more hacking to remove blob granule related code, e.g. blob manager and blob migrator roles
* delete more blob granule stuff
* more hacking
* more hacking
* more hacking
* More changes to remove blob granule related code. IT COMPILES NOW
* dont try to run AuthzSecurity tests as we have deleted that workload as part of this effort
* delete more stuff that matches, abbreviates, or smells like blob granule related
* EncryptKeyProxy: dont do blobMetadata stuff, because that is not used and support is being removed
* delete more references to blob granule stuff
* SimulationConfig::setEncryptionAtRestMode: always use DISABLED; also disable EncryptKeyProxyTest.toml
* format code
* manual update to bindings/java/src/tests.cmake to remove a deleted file
* fix compile errors. I guess by default I dont build Java bindings
* remove unneeded blob granule functions rather than #if..#endif them out
* remove more code in #if..#endif
* remove more code in #if 0..#endif
* revert changes to fdb_c.h in preparation for marking removed API calls as removed
* rework C API declarations to in preparation for marking blob granule APIs as removed
* deprecate removed glob granule related API functions as of version 740 (and add a comment to request a justification of this convention)
* make progress on broken ctests. E.g. 1) python does not need to do blob granule stuff. 2) authz tests seemingly not needed
* remove blob granule stuff from Java and Python APIs and fix test runner stuff so that ctests pass
* reformat comments to fix compile error. FIXME: why is this error not happening on the default compile commands we use
* hacks all the way down to try to fix the Mac build
* add pointed comment about the perceived pointlessness of the API deprecation scheme embodied in this source file
* really serious about the C++ style comments, arent we
* remove commented-out code from prior iterative efforts
* put back undeleted code in original order
* delete commented-out code
* update feature-status.md to say blob granule is mostly deleted
* upgrade `mostly deleted` to `has been deleted`
* EaR: Implement Key Check Value semantics
Description
Key Check Value (KCV) is a checksum of cryptographic encryption key
used to validate encryption keys's integrity. FDB Encryption at-rest
relies on external KMS to supply encryption keys.
Patch proposes following major changes:
1. Implement Sha256 based KCV implementation to protect against
'baseCipher' corruption in two possible scenarios:
a) potential corruption external to FDB
b) potential corruption within FDB processes.
2. Scheme persists computed KCV token in block encryption header,
which then gets validated as part of header validation during
decryption.
3. FDB Encryption key derivation uses HMAC_SHA256 digest generation
scheme, which allows max 64 bytes of 'cipher buffer', patch add
required check to ensure 'baseCipher' length are within bounds.
OpenSSL HMAC underlying call ignores extra length if supplied, however,
it weakens the security guarantees, hence, disallowed.
Testing
devRunCorrectness - multiple 500K runs
Valgrind & Asan - BlobCipherUnit, RESTKMSUnit, BlobGranuleCorrectness*,
EncryptionOps, EncryptKeyProxyTest
Description
Patch proposes two changes:
1. Avoid appending tls as part of URI for secure connections
2. RefreshEKs recurring task can be skipped if there are no keys to be refreshed
Testing
EncryptionOps.toml
EncryptKeyProxyTest.toml
devRunCorrectness
devRunCorrectnessFiltered 'Encrypt*'
Description
Two major changes proposed are:
I)
Used following setup for testing:
1. Run `fdbserver` locally.
2. Run a mock python based HTTP server (encryption endpoints not implemented)
Expectation was RESTClient code should go in loop trying to establish connection
to the desired encryption endpoint. However, observation was the code loops for
one cycle and followup cycle SEGV while printing a log using RESTUrl object which
is obtained as a 'pointer' from the caller. Update the code to use RESTUrl object
instead of the pointer.
II) In above setup, KMSConnector would throw 'encrypt_key_fetch_failed' error
which wasn't handled by EKProxy, hence, causing the service to terminate. Add
code to re-throw the error to the caller.
Testing
* [EAR]: Remove usage of EncryptDomainName for Encryption at-rest operations
Description
diff-1: Address review comments
EncryptDomainName is an auxillary information, given EAR encryption domain
matches with Tenants, EncryptDomainName maps to TenantName in the current
code. However, this mapping adds EAR depedency has multiple drawbacks:
1. In some scenarios obtaning consistent mapping of TenantId <-> TenantName
is difficult to maintain. For instance: StorageServer (SS) TLog mutation
pop loop, it is possible that same commit batch contains: TenantMap update
mutation as well as a Tenant user mutation. SS would parse TenantMap update
mutation (FDB System Keyspace encryption domain), process the mutation, but,
doesn't apply it to the process local TenantMap. SS then attempts to process,
Tenant user mutation and fails to decrypt the mutation given TenantMetadaMap
isn't updated yet.
2. FDB codebase uses EncryptDomainId matching TenantId, TenantName is used as
an auxillary information source and feels better to be handled by an
external KMS.
Major changes include:
1. EAR to remove TenantName dependency across all participating processes
such as: CommitProxy, Redwood, BlobGranule and Backup agent.
2. Update EKP and KmsConnector APIs to avoid relying on "domainName"
information being passed around to external KMS EAR endpoints.
Testing
devRunCorrectness - 100K
EncryptKeyProxyTest - 100K
EncryptionOps Test - 100K
* Enable EncryptKeyProxy test and add code probes
Description
diff-1: Address review comments
Major changes include:
1. Enable EncryptKeyProxyTest, the test was modified to leverage
GetEncrrptCipherKey actor, as existing code wouldn't work if EKP
process gets restarted while test is running.
2. Add code probes to EKP
3. Minor refactoring.
Testing
EncryptKeyProxyTest - 100K
Adding the following metrics:
* BlobCipherKeyCache hit/miss
* EKP: KMS requests latencies
* For each component that using encryption, they now need to pass a UsageType enum to the encryption helper methods (GetEncryptCipherKeys/GetLatestEncryptCipherKey/encrypt/decrypt) and those methods will help to log get cipher key latency samples and encryption/decryption cpu times accordingly.
* Move arena members to the end of serializer calls
See
https://github.com/apple/foundationdb/tree/main/flow#flatbuffersobjectserializer
for why this is necessary.
* Fix a heap-use-after-free
Previously memory owned by
EncryptKeyProxyData::baseCipherDomainIdKeyIdCache was borrowed by a call
to EncryptKeyProxyData::insertIntoBaseDomainIdCache where it was
invalidated and then used. Now
EncryptKeyProxyData::insertIntoBaseDomainIdCache takes shared ownership
by taking a Standalone.
And also rename some types to end in Ref to follow the flow conventions
described here: https://github.com/apple/foundationdb/tree/main/flow#arenas
* Implement TenantCacheEntry in-memory cache
Description
diff-4: TraceEvent usage improvements
diff-3: Address review comments
diff-2: Add APIs to read counter values, test improvements
diff-1: Address review comments
Major changes includes:
1. Implements an actor that enables an in-memory caching of
TenantCacheEntry object, allowing the caller to embed custom
information along with TenantCacheEntry.
2. The cache follows read-through cache semantics where the entry
gets loaded from underlying database on a miss.
3. The cache implements a "periodic poller" to refresh known Tenants
by consulting the database. Once a database keyrange-watch feature is
available, cache shall be updated.
Bonus:
Implement a 'recurringAsync' addition to genericActors allowing caller
to schedule a periodic task registering an "actor functor"; the routine
'waits' for the actor unlike existing 'recurring' implementation.
Testing
TenantEntryCache workload
devCorrectnessRun - 100K
Description
FDB Native encryption requires integration with external
KeyMangement Services to fetch required encryption keys.
For simulation runs, there exists SimKmsConnector implementation
that fakes interaction with external KMS.
Major changes suggested in the patch:
1. Enable setting KMS_CONNECTOR_TYPE via command line arguments.
2. If "FDBPerfKmsConnector" is set as KMS_CONNECTOR_TYPE, then
allow using SimKmsConnector implementation.
Note: SimKmsConnector can handle process reboots.
Testing
devRunCorrectness - 100K
This PR add support for TLog encryption through commit proxy. The encryption is done on per-mutation basis. As CP writes mutations to TLog, it inserts encryption header alongside encrypted mutations. Storage server (and other consumers of TLog such as storage cache and backup worker) decrypts the mutations as they peek TLog.
Description
Major changes include:
1. GetEncryptByKeyIds cache elements can expire.
2. Update iterator after erasing an element during refresh encryption keys
operation.
Testing
EncryptKeyProxyTest
* KmsConnector implementation to support KMS driven CipherKey TTL
Description
KMS CipherKeys can be of two types:
1. Revocable CipherKeys: having a finite lifetime, after which the CipherKey
shouldn't be used by the FDB.
2. Non-revocable CipherKeys: ciphers are not revocable, however, FDB would
still want to refresh ciphers to support KMS cipher rotation feature.
Patch proposes following change to incorporate support for above defined cipher-key
types:
1. Extend KmsConnector response to include optional 'refreshAfter' & 'expireAfter'
time intervals. EncryptKeyProxy (EKP) cache would define corresponding absolute refresh &
expiry timestamp for a given cipherKey. On an event of transient KMS connectivity outage,
a caller of EKP API for a non-revocable key should continue using cached cipherKey until
it expires.
2. Simplify KmsConnector API arena handling by using VectorRef to represent component
structs and manage associated memory allocation/lifetime.
Testing
1. EncryptKeyProxyTest
2. RESTKmsConnectorTest
3. SimKmsConnectorTest
* KmsConnector implementation to support KMS driven CipherKey TTL
Description
diff-1: Set expireTS for baseCipherId indexed cache
KMS CipherKeys can be of two types:
1. Revocable CipherKeys: having a finite lifetime, after which the CipherKey
shouldn't be used by the FDB.
2. Non-revocable CipherKeys: ciphers are not revocable, however, FDB would
still want to refresh ciphers to support KMS cipher rotation feature.
Patch proposes following change to incorporate support for above defined cipher-key
types:
1. Extend KmsConnector response to include optional 'refreshAfter' & 'expireAfter'
time intervals. EncryptKeyProxy (EKP) cache would define corresponding absolute refresh &
expiry timestamp for a given cipherKey. On an event of transient KMS connectivity outage,
a caller of EKP API for a non-revocable key should continue using cached cipherKey until
it expires.
2. Simplify KmsConnector API arena handling by using VectorRef to represent component
structs and manage associated memory allocation/lifetime.
Testing
1. EncryptKeyProxyTest
2. RESTKmsConnectorTest
3. SimKmsConnectorTest
* KmsConnector implementation to support KMS driven CipherKey TTL
Description
diff-2: Fix Valgrind issues discovered runnign tests
diff-1: Set expireTS for baseCipherId indexed cache
KMS CipherKeys can be of two types:
1. Revocable CipherKeys: having a finite lifetime, after which the CipherKey
shouldn't be used by the FDB.
2. Non-revocable CipherKeys: ciphers are not revocable, however, FDB would
still want to refresh ciphers to support KMS cipher rotation feature.
Patch proposes following change to incorporate support for above defined cipher-key
types:
1. Extend KmsConnector response to include optional 'refreshAfter' & 'expireAfter'
time intervals. EncryptKeyProxy (EKP) cache would define corresponding absolute refresh &
expiry timestamp for a given cipherKey. On an event of transient KMS connectivity outage,
a caller of EKP API for a non-revocable key should continue using cached cipherKey until
it expires.
2. Simplify KmsConnector API arena handling by using VectorRef to represent component
structs and manage associated memory allocation/lifetime.
Testing
1. EncryptKeyProxyTest
2. RESTKmsConnectorTest
3. SimKmsConnectorTest
* KmsConnector implementation to support KMS driven CipherKey TTL
Description
diff-3: Address review comment
diff-2: Fix Valgrind issues discovered runnign tests
diff-1: Set expireTS for baseCipherId indexed cache
KMS CipherKeys can be of two types:
1. Revocable CipherKeys: having a finite lifetime, after which the CipherKey
shouldn't be used by the FDB.
2. Non-revocable CipherKeys: ciphers are not revocable, however, FDB would
still want to refresh ciphers to support KMS cipher rotation feature.
Patch proposes following change to incorporate support for above defined cipher-key
types:
1. Extend KmsConnector response to include optional 'refreshAfter' & 'expireAfter'
time intervals. EncryptKeyProxy (EKP) cache would define corresponding absolute refresh &
expiry timestamp for a given cipherKey. On an event of transient KMS connectivity outage,
a caller of EKP API for a non-revocable key should continue using cached cipherKey until
it expires.
2. Simplify KmsConnector API arena handling by using VectorRef to represent component
structs and manage associated memory allocation/lifetime.
Testing
1. EncryptKeyProxyTest
2. RESTKmsConnectorTest
3. SimKmsConnectorTest
Description
Major changes proposed include:
1. Update EncryptKeyServer APIs to be tenant aware.
2. Update KmsConnector APIs to be tenant aware
Client of above APIs such as: CP, SS and BlobWorker need to supply
encryption domain info that includes: tenantId and tenantName
Testing
1. Update EncryptKeyProxyTest
2. Update RESTKmsConnectorTest
3. Update SimKmsConnectorTest
Description
Major changes proposed in the patch includes:
1. Update EncryptKeyProxy EncyrptBaseCipherKeyId cache to be indexed
using {encryptDomainId, baseCipherId} instead of only 'baseCipherId'
2. Enhance RESTKmsConnector 'error' tag to encapsulte: errorMessage
and errorCode information
Testing
1. Updated EncyrptKeyProxy test
2. Updated RESTKmsConnector unit test
* Enable debugId tracing for encryption requests
Description
diff-1: Minor fixes, address review comment
Proposed changes include:
1. Update EncryptKeyProxy API to embded Optional<UID> for debugging
request execution.
2. Encryption participant FDB processes can set 'debugId' enabling
tracing requests within FDB cluster processes and beyond.
3. The 'debugId' if available is embedded as part of 'request_json_payload'
by RESTKmsConnector, enabling tracing request between FDB <--> KMS.
4. Fix EncryptKeyProxyTest which got broken due to recent changes.
Testing
Updated following test:
1. EncryptKeyProxy simulation test.
2. RESTKmsConnector simulation test.
Description
Testing
* REST KmsConnector implementation
Description
diff-1: Address review comments.
Add utility interface to Platform namespace to
create and operate on tmpfile
diff-2: Address review comments
Link Boost::filesystem to CMake build process
Major changes includes:
1. Implement REST based KmsConnector implementation.
2. Salient features of the connector:
2.1. Two required configuration are:
a. Discovery KMS URLs - enable KMS discovery on bootstrap
b. Endpoint path configuration to construct URI to fetch/refresh
encryption keys
c. Configuration to provide "validationTokens" to connect with
external KMS. Patch implements file-based token validation scheme.
2.2. On startup, RESTKmsConnector discovers KMS Urls and caches
them in-memory. Extracts "validationTokens" based on input config.
2.3. Expose endpoints to allow fetch/refresh of encryption keys.
2.4. Defines JSON format to interact with external KMS - request &
response payload format.
3. Extend Platform namespace with an interface to create and operate on
tmp files.
4. Update Platform 'readFileBytes' and 'writeFileBytes' to leverage
fstream supported implementation.
NOTE: KMS URLs fetched after initial discovery will be persisted using
DynamicKnobs. It is TODO at the moment and shall be completed
once DynamicKnobs is feature complete
Testing
Unit test to validation following:
1. Parsing on "validation tokens" logic.
2. Construction and parsing of REST JSON request and response strings.
* 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
* Update EKPServer LookupKeysByKeyIds API
Description
Major changes include:
1. Update EKPServer LookupKeysByKeyIds API to include
'encryptDomainId' as input along with 'baseCipherId'
2. Unify EKPServer response definition to return
std::vector<EKPBaseCipherDetails>
3. Update SimKmsProxy interfaces as per above mentioned
changes.
Testing
1. Update EncryptKeyProxyTest simulation test.
2. Update SimEncyrptKeyProxy unit test
* EncryptKeyProxy server APIs for simulation runs.
Description
diff-2: FlowSingleton util class
Bug fixes
diff-1: Expected errors returned to the caller
Major changes proposed are:
1. EncryptKeyProxy server APIs:
1.1. Lookup Cipher details via BaseCipherId
1.2. Lookup latest Cipher details via encryption domainId.
2. EncyrptKeyProxy implements caches indexed by: baseCipherId &
encyrptDomainId
3. Periodic task to refresh domainId indexed cache to support
'limiting cipher lifetime' abilities if supported by
external KMS solutions.
Testing
EncyrptKeyProxyTest workload to validate the newly added code.
Description
Major changes proposed are:
1. Rename ServerKnob->ENABLE_ENCRYPT_KEY_PROXY to
ServerKnob->ENABLE_ENCRYPTION. Approach simplifies enabling
controlling encyrption code change using a single knob (desirable)
2. Implement EncyrptKeyVaultProxy simulated interface to assist
validating encyrption workflows in simulation runs. The interface
is leveraged to satisfy "encryption keys" lookup which otherwise
gets satisfied by integrating organization preferred Encryption
Key Management solution.
Testing
Unit test to validate the newly added code
Major changes includes:
1. Add a new FDB role responsible- EncyrptKeyProxy. The role is
responsible to expose APIs to fetch encyrption keys interacting
with external Encryption KeyManager interface.
2. The process is a FDB singleton process following similar recruitment
rules as other singleton processes in the system.
3. Code to recruit the worker process; given the encryption keys are
needed during recovery (decode TLog records), for now the process
is co-located in same datacenter as ClusterController.
4. Skeleton process actor code; more functionality will be added in
subsequent PRs.
NOTE: The code is protected under a SERVER_KNOB with the default
value as 'false' for now.
Jingyu Zhou