Repository navigation
feat(encryption): add EncryptionManager and StandardEncryptionManager - #1071
shangxinli wants to merge 4 commits into
Conversation
- EncryptionManager (core): per-table interface to generate key metadata, open AES GCM Stream files for reading and writing, and encrypt and decrypt manifest list key metadata. - EncryptionManagerRegistry (core): creates the manager of an encrypted table from the trusted table key ID, the table's encryption-keys and the catalog's KMS client. The bundle registers the implementation, so the core library has no crypto dependency. - StandardEncryptionManager (bundle): Java's envelope encryption. Key encryption keys are wrapped by the table master key in the KMS, timestamped, used for 730 days and then rotated; manifest list key metadata is encrypted by a KEK with the KEK timestamp as AAD. Unwrapped KEKs are cached for one hour. Thread-safe. - encryption::RegisterAll() / IsEncryptionAvailable() in the bundle; a no-op when built with ICEBERG_ENCRYPTION=OFF. Adds the Java-written encrypted table fixture under test/resources/encryption/table. Tests decrypt the manifest list key metadata of every Java snapshot and check that its file length is the stored size of the manifest list.
Resolves modernize-use-designated-initializers clang-tidy warnings.
| deletes/position_delete_index.cc | ||
| deletes/position_delete_range_consumer.cc | ||
| deletes/roaring_position_bitmap.cc | ||
| encryption/encryption_manager.cc |
There was a problem hiding this comment.
The new public headers are missing from the explicit install lists: encryption_manager.h, encryption_register.h and standard_encryption_manager.h. Please add them to the core, bundle and encryption groups respectively and regenerate the public-header coverage. Installed consumers currently cannot include these APIs; the registration header should remain available when the bundle is built with encryption disabled.
There was a problem hiding this comment.
Added the three missing headers to the install lists — encryption_manager.h as core, encryption_register.h tagged ICEBERG_PUBLIC_HEADERS_BUNDLE so it stays available with encryption disabled, and standard_encryption_manager.h in the existing encryption group — and regenerated cpp20_compatibility_internal.h.
| continue; | ||
| } | ||
| auto timestamp = ParseTimestamp(key); | ||
| if (timestamp.has_value() && now - *timestamp < kKekLifespan.count()) { |
There was a problem hiding this comment.
ParseTimestamp accepts INT64_MIN, so KEY_TIMESTAMP=-9223372036854775808 causes signed overflow in now - *timestamp. Please validate the range or use an overflow-safe comparison, and add a boundary test.
There was a problem hiding this comment.
Good catch — ParseTimestamp now rejects negative values so a corrupted INT64_MIN timestamp can't reach the now - timestamp subtraction, and added a boundary test covering it.
…verflow Register encryption_manager.h, encryption_register.h, and standard_encryption_manager.h in the public header install lists so installed consumers can include the new encryption APIs. encryption_register.h is grouped as a bundle header (not encryption-only) so it stays available when the bundle is built with encryption disabled. Also reject negative KEY_TIMESTAMP values in ParseTimestamp to avoid signed overflow in CurrentKekId's now - timestamp comparison when the stored timestamp is corrupted to INT64_MIN.
| @@ -0,0 +1 @@ | |||
| {"format-version":3,"table-uuid":"2091a278-2727-4bc0-8427-ffb04a2b7e6d","location":"/tmp/iceberg-cpp-encryption-fixture/encrypted_table","last-sequence-number":0,"last-updated-ms":1791460592557,"last-column-id":2,"current-schema-id":0,"schemas":[{"type":"struct","schema-id":0,"fields":[{"id":1,"name":"id","required":true,"type":"long"},{"id":2,"name":"data","required":false,"type":"string"}]}],"default-spec-id":0,"partition-specs":[{"spec-id":0,"fields":[]}],"last-partition-id":999,"default-sort-order-id":0,"sort-orders":[{"order-id":0,"fields":[]}],"properties":{"encryption.key-id":"keyA","write.parquet.compression-codec":"zstd"},"current-snapshot-id":null,"next-row-id":0,"refs":{},"snapshots":[],"statistics":[],"partition-statistics":[],"snapshot-log":[],"metadata-log":[]} | |||
There was a problem hiding this comment.
Is it possible to remove some unused files here? Will they be used in the future? I think eventually these files should be generated on-demand like what the Java implementation does.
BTW, we need to add them to dev/release/rat_exclude_files.txt to not fail the future release.
There was a problem hiding this comment.
removed the unused v1-v4 metadata, version-hint.text, the per-snapshot m0.avro manifests, and the sample data files, keeping only v5.metadata.json and the manifest-list avros it actually reads in the test. test/resources/** and *.json are already in rat_exclude_files.txt so no change needed there.
Keep only v5.metadata.json and the 4 manifest-list avro files its snapshots reference at runtime; v1-v4.metadata.json, version-hint.text, the per-snapshot m0.avro manifest files, and the sample data files under data/ are not read by any test.
Part of the envelope encryption stack for #988.
Adds
EncryptionManager/EncryptionManagerRegistry(the pluggable manager interfaceand factory) and
StandardEncryptionManager, the envelope encryption manager compatiblewith Java's
StandardEncryptionManager:every 730 days (NIST SP 800-57), manifest-list key metadata AES-GCM encrypted by a KEK
with the KEK timestamp as AAD.
Gated behind
ICEBERG_ENCRYPTION, registered in CMakeLists.txt, with a newcrypto_testtarget covering manifest-list key decryption against a Java-produced fixture, key
registration/decryption round trips, KEK rotation, and unwrapped-key cache expiry.