smaheshwar-pltr commented on code in PR #13225:
URL: https://github.com/apache/iceberg/pull/13225#discussion_r2479156094
##########
core/src/main/java/org/apache/iceberg/rest/RESTTableOperations.java:
##########
@@ -166,7 +208,67 @@ public void commit(TableMetadata base, TableMetadata
metadata) {
@Override
public FileIO io() {
- return io;
+ if (encryptionKeyId == null) {
+ return io;
+ }
+
+ if (encryptingFileIO == null) {
+ encryptingFileIO = EncryptingFileIO.combine(io, encryption());
+ }
+
+ return encryptingFileIO;
+ }
+
+ @Override
+ public EncryptionManager encryption() {
+ if (encryptionManager != null) {
+ return encryptionManager;
+ }
+
+ if (encryptionKeyId != null) {
+ if (kmsClient == null) {
+ throw new RuntimeException(
+ "Cant create encryption manager, because key management client is
not set");
+ }
+
+ Map<String, String> tableProperties = Maps.newHashMap();
+ tableProperties.put(TableProperties.ENCRYPTION_TABLE_KEY,
encryptionKeyId);
+ tableProperties.put(
+ TableProperties.ENCRYPTION_DEK_LENGTH,
String.valueOf(encryptionDekLength));
+ encryptionManager =
+ EncryptionUtil.createEncryptionManager(
+ encryptedKeysFromMetadata, tableProperties, kmsClient);
+ } else {
+ return PlaintextEncryptionManager.instance();
+ }
+
+ return encryptionManager;
+ }
+
+ private void encryptionPropsFromMetadata(TableMetadata metadata) {
Review Comment:
Thanks for the context - yeah, I think the problem is placing onus on the
server implementations, that's why I suggested the config approach, but it
sounds that more is needed.
> One thing we can do is to document this in the REST spec, so the
implementors are aware of the threats
Makes sense, it's maybe good also to mention / understand what's typically
required for REST maintainers - am I right that a REST service can read and
write the metadata JSON files using
[AES-GCM](https://iceberg.apache.org/gcm-stream-spec/) to protect against these
threats? @ggershinsky, I understand that the Java implementation of this should
work for JSON files?
I realise that our discussion in
https://github.com/apache/iceberg/pull/13066#discussion_r2385730326 goes
against this, but REST services encrypting and tamper-proofing metadata JSONs
does seem like a good solution here unless I've misunderstood.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
