diff --git a/api/src/main/java/org/apache/iceberg/ManifestListFile.java b/api/src/main/java/org/apache/iceberg/ManifestListFile.java index e727a35a4e09..00871f08e8b3 100644 --- a/api/src/main/java/org/apache/iceberg/ManifestListFile.java +++ b/api/src/main/java/org/apache/iceberg/ManifestListFile.java @@ -21,6 +21,12 @@ import java.nio.ByteBuffer; import org.apache.iceberg.encryption.EncryptionManager; +/** + * @deprecated since 1.12.0. Will be removed in 2.0.0; Use {@link + * org.apache.iceberg.io.FileIO#newInputFile(String, String)} and {@link + * EncryptionManager#decryptKeyMetadata(String)} providing location and keyId. + */ +@Deprecated public interface ManifestListFile { /** Location of manifest list file. */ diff --git a/api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java b/api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java index 6a14db3dd439..dcf4ca9039f3 100644 --- a/api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java +++ b/api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java @@ -130,6 +130,11 @@ public InputFile newInputFile(ManifestFile manifest) { } } + /** + * @deprecated since 1.12.0. Will be removed in 2.0.0; use {@link #newInputFile(String, String)} + * instead. + */ + @Deprecated @Override public InputFile newInputFile(ManifestListFile manifestList) { if (manifestList.encryptionKeyID() != null) { @@ -140,6 +145,16 @@ public InputFile newInputFile(ManifestListFile manifestList) { } } + @Override + public InputFile newInputFile(String location, String keyId) { + if (keyId != null) { + ByteBuffer keyMetadata = em.decryptKeyMetadata(keyId); + return newDecryptingInputFile(location, keyMetadata); + } else { + return newInputFile(location); + } + } + public InputFile newDecryptingInputFile(String path, ByteBuffer buffer) { return em.decrypt(wrap(io.newInputFile(path), buffer)); } diff --git a/api/src/main/java/org/apache/iceberg/encryption/EncryptionManager.java b/api/src/main/java/org/apache/iceberg/encryption/EncryptionManager.java index 22d2858599a8..3e09ac14915d 100644 --- a/api/src/main/java/org/apache/iceberg/encryption/EncryptionManager.java +++ b/api/src/main/java/org/apache/iceberg/encryption/EncryptionManager.java @@ -19,6 +19,7 @@ package org.apache.iceberg.encryption; import java.io.Serializable; +import java.nio.ByteBuffer; import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; import org.apache.iceberg.relocated.com.google.common.collect.Iterables; @@ -51,6 +52,17 @@ default Iterable decrypt(Iterable encrypted) { return Iterables.transform(encrypted, this::decrypt); } + /** + * Decrypt an encrypted key metadata referred by a key id. + * + * @param keyId the encryption key ID + * @return the decrypted key metadata buffer + */ + default ByteBuffer decryptKeyMetadata(String keyId) { + throw new UnsupportedOperationException( + this.getClass().getName() + " does not support key metadata decryption"); + } + /** * Given a handle on an {@link OutputFile} that writes raw bytes to the underlying file system, * return a bundle of an {@link EncryptedOutputFile#encryptingOutputFile()} that writes encrypted diff --git a/api/src/main/java/org/apache/iceberg/io/FileIO.java b/api/src/main/java/org/apache/iceberg/io/FileIO.java index 72d2d9b0fca2..e11802c4398c 100644 --- a/api/src/main/java/org/apache/iceberg/io/FileIO.java +++ b/api/src/main/java/org/apache/iceberg/io/FileIO.java @@ -71,6 +71,11 @@ default InputFile newInputFile(ManifestFile manifest) { return newInputFile(manifest.path(), manifest.length()); } + /** + * @deprecated since 1.12.0. Will be removed in 2.0.0; use {@link #newInputFile(String, String)} + * instead. + */ + @Deprecated default InputFile newInputFile(ManifestListFile manifestList) { Preconditions.checkArgument( manifestList.encryptionKeyID() == null, @@ -80,6 +85,13 @@ default InputFile newInputFile(ManifestListFile manifestList) { return newInputFile(manifestList.location()); } + default InputFile newInputFile(String location, String keyId) { + Preconditions.checkArgument( + keyId == null, "Cannot decrypt file: %s (use EncryptingFileIO)", location); + // cannot pass length because it is not tracked outside of key metadata + return newInputFile(location); + } + /** Get a {@link OutputFile} instance to write bytes to the file at the given path. */ OutputFile newOutputFile(String path); diff --git a/core/src/main/java/org/apache/iceberg/AllManifestsTable.java b/core/src/main/java/org/apache/iceberg/AllManifestsTable.java index 8ceffc29c9e4..0dc53d17124b 100644 --- a/core/src/main/java/org/apache/iceberg/AllManifestsTable.java +++ b/core/src/main/java/org/apache/iceberg/AllManifestsTable.java @@ -142,7 +142,8 @@ protected CloseableIterable doPlanFiles() { io, schema(), specs, - new BaseManifestListFile(snap.manifestListLocation(), snap.keyId()), + snap.manifestListLocation(), + snap.keyId(), residual, snap.snapshotId()); } else { @@ -165,7 +166,8 @@ static class ManifestListReadTask implements DataTask { private final FileIO io; private final Schema schema; private final Map specs; - private final ManifestListFile manifestList; + private final String location; + private final String encryptionKeyId; private final Expression residual; private final long referenceSnapshotId; private DataFile lazyDataFile = null; @@ -175,14 +177,16 @@ static class ManifestListReadTask implements DataTask { FileIO io, Schema schema, Map specs, - ManifestListFile manifestList, + String location, + String encryptionKeyId, Expression residual, long referenceSnapshotId) { this.dataTableSchema = dataTableSchema; this.io = io; this.schema = schema; this.specs = specs; - this.manifestList = manifestList; + this.location = location; + this.encryptionKeyId = encryptionKeyId; this.residual = residual; this.referenceSnapshotId = referenceSnapshotId; } @@ -195,7 +199,7 @@ public List deletes() { @Override public CloseableIterable rows() { try (CloseableIterable manifests = - InternalData.read(FileFormat.AVRO, io.newInputFile(manifestList)) + InternalData.read(FileFormat.AVRO, io.newInputFile(location, encryptionKeyId)) .setRootType(GenericManifestFile.class) .setCustomType( ManifestFile.PARTITION_SUMMARIES_ELEMENT_ID, GenericPartitionFieldSummary.class) @@ -213,8 +217,7 @@ public CloseableIterable rows() { return CloseableIterable.transform(rowIterable, projection::wrap); } catch (IOException e) { - throw new RuntimeIOException( - e, "Cannot read manifest list file: %s", manifestList.location()); + throw new RuntimeIOException(e, "Cannot read manifest list file: %s", location); } } @@ -223,7 +226,7 @@ public DataFile file() { if (lazyDataFile == null) { this.lazyDataFile = DataFiles.builder(PartitionSpec.unpartitioned()) - .withInputFile(io.newInputFile(manifestList)) + .withInputFile(io.newInputFile(location, encryptionKeyId)) .withRecordCount(1) .withFormat(FileFormat.AVRO) .build(); @@ -276,8 +279,12 @@ Map specsById() { return specs; } - ManifestListFile manifestList() { - return manifestList; + String location() { + return location; + } + + String encryptionKeyId() { + return encryptionKeyId; } long referenceSnapshotId() { diff --git a/core/src/main/java/org/apache/iceberg/AllManifestsTableTaskParser.java b/core/src/main/java/org/apache/iceberg/AllManifestsTableTaskParser.java index e6539f2d714f..70c3f9acad99 100644 --- a/core/src/main/java/org/apache/iceberg/AllManifestsTableTaskParser.java +++ b/core/src/main/java/org/apache/iceberg/AllManifestsTableTaskParser.java @@ -64,9 +64,9 @@ static void toJson(AllManifestsTable.ManifestListReadTask task, JsonGenerator ge generator.writeEndArray(); - generator.writeStringField(MANIFEST_LIST_LOCATION, task.manifestList().location()); - if (task.manifestList().encryptionKeyID() != null) { - generator.writeStringField(MANIFEST_LIST_KEY_ID, task.manifestList().encryptionKeyID()); + generator.writeStringField(MANIFEST_LIST_LOCATION, task.location()); + if (task.encryptionKeyId() != null) { + generator.writeStringField(MANIFEST_LIST_KEY_ID, task.encryptionKeyId()); } generator.writeFieldName(RESIDUAL); @@ -105,7 +105,8 @@ static AllManifestsTable.ManifestListReadTask fromJson(JsonNode jsonNode) { fileIO, schema, specsById, - new BaseManifestListFile(manifestListLocation, manifestListKeyId), + manifestListLocation, + manifestListKeyId, residualFilter, referenceSnapshotId); } diff --git a/core/src/main/java/org/apache/iceberg/BaseManifestListFile.java b/core/src/main/java/org/apache/iceberg/BaseManifestListFile.java deleted file mode 100644 index e0ecfd50c863..000000000000 --- a/core/src/main/java/org/apache/iceberg/BaseManifestListFile.java +++ /dev/null @@ -1,49 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.iceberg; - -import java.io.Serializable; -import java.nio.ByteBuffer; -import org.apache.iceberg.encryption.EncryptionManager; -import org.apache.iceberg.encryption.EncryptionUtil; - -class BaseManifestListFile implements ManifestListFile, Serializable { - private final String location; - private final String encryptionKeyID; - - BaseManifestListFile(String location, String encryptionKeyID) { - this.location = location; - this.encryptionKeyID = encryptionKeyID; - } - - @Override - public String location() { - return location; - } - - @Override - public String encryptionKeyID() { - return encryptionKeyID; - } - - @Override - public ByteBuffer decryptKeyMetadata(EncryptionManager em) { - return EncryptionUtil.decryptManifestListKeyMetadata(this, em); - } -} diff --git a/core/src/main/java/org/apache/iceberg/BaseSnapshot.java b/core/src/main/java/org/apache/iceberg/BaseSnapshot.java index 826b9624c0e6..62f69aab72f5 100644 --- a/core/src/main/java/org/apache/iceberg/BaseSnapshot.java +++ b/core/src/main/java/org/apache/iceberg/BaseSnapshot.java @@ -183,9 +183,7 @@ private void cacheManifests(FileIO fileIO) { if (allManifests == null) { // if manifests isn't set, then the snapshotFile is set and should be read to get the list this.allManifests = - ManifestLists.read( - ManifestLists.newInputFile( - fileIO, new BaseManifestListFile(manifestListLocation, keyId))); + ManifestLists.read(ManifestLists.newInputFile(fileIO, manifestListLocation, keyId)); } if (dataManifests == null || deleteManifests == null) { diff --git a/core/src/main/java/org/apache/iceberg/ManifestLists.java b/core/src/main/java/org/apache/iceberg/ManifestLists.java index dbe080584b13..48472e328bf7 100644 --- a/core/src/main/java/org/apache/iceberg/ManifestLists.java +++ b/core/src/main/java/org/apache/iceberg/ManifestLists.java @@ -32,8 +32,8 @@ class ManifestLists { private ManifestLists() {} - static InputFile newInputFile(FileIO io, ManifestListFile manifestList) { - InputFile input = io.newInputFile(manifestList); + static InputFile newInputFile(FileIO io, String location, String keyId) { + InputFile input = io.newInputFile(location, keyId); if (ManifestFiles.cachingEnabled(io)) { return ManifestFiles.contentCache(io).tryCache(input); } diff --git a/core/src/main/java/org/apache/iceberg/encryption/EncryptionUtil.java b/core/src/main/java/org/apache/iceberg/encryption/EncryptionUtil.java index 382d244883d6..a565430c7a14 100644 --- a/core/src/main/java/org/apache/iceberg/encryption/EncryptionUtil.java +++ b/core/src/main/java/org/apache/iceberg/encryption/EncryptionUtil.java @@ -140,19 +140,32 @@ public static ByteBuffer setFileLength(ByteBuffer keyMetadata, long fileLength) * @param manifestList a ManifestListFile * @param em the table's EncryptionManager * @return a decrypted key metadata buffer + * @deprecated since 1.12.0. Will be removed in 2.0.0; use {@link + * EncryptionManager#decryptKeyMetadata(String)} instead. */ + @Deprecated public static ByteBuffer decryptManifestListKeyMetadata( ManifestListFile manifestList, EncryptionManager em) { + return decryptKeyMetadata(manifestList.encryptionKeyID(), em); + } + + /** + * Decrypt the key metadata referred by key ID using an encryption manager. + * + * @param encryptionKeyId the key ID of the encrypted key metadata + * @param em the table's EncryptionManager + * @return a decrypted key metadata buffer + */ + static ByteBuffer decryptKeyMetadata(String encryptionKeyId, EncryptionManager em) { Preconditions.checkState( em instanceof StandardEncryptionManager, - "Snapshot key metadata encryption requires a StandardEncryptionManager"); + "Key metadata decryption requires a StandardEncryptionManager"); StandardEncryptionManager sem = (StandardEncryptionManager) em; - String manifestListKeyId = manifestList.encryptionKeyID(); Map encryptionKeys = sem.encryptionKeys(); - EncryptedKey manifestListKey = encryptionKeys.get(manifestListKeyId); - ByteBuffer encryptedKeyMetadata = manifestListKey.encryptedKeyMetadata(); - String keyEncryptionKeyID = manifestListKey.encryptedById(); - ByteBuffer keyEncryptionKey = sem.encryptedByKey(manifestListKeyId); + EncryptedKey encryptionKey = encryptionKeys.get(encryptionKeyId); + ByteBuffer encryptedKeyMetadata = encryptionKey.encryptedKeyMetadata(); + String keyEncryptionKeyID = encryptionKey.encryptedById(); + ByteBuffer keyEncryptionKey = sem.encryptedByKey(encryptionKeyId); String keyEncryptionKeyTimestamp = encryptionKeys .get(keyEncryptionKeyID) @@ -182,22 +195,22 @@ public static Map encryptionKeys(EncryptionManager em) { } /** - * Encrypts the key metadata for a manifest list. + * Encrypts an encryption key metadata. * * @param key key encryption key bytes * @param keyTimestamp timestamp of the key encryption key - * @param mlkMetadata manifest list key metadata + * @param keyMetadata key metadata * @return encrypted key metadata */ - static ByteBuffer encryptManifestListKeyMetadata( - ByteBuffer key, String keyTimestamp, EncryptionKeyMetadata mlkMetadata) { + static ByteBuffer encryptKeyMetadata( + ByteBuffer key, String keyTimestamp, EncryptionKeyMetadata keyMetadata) { Ciphers.AesGcmEncryptor encryptor = new Ciphers.AesGcmEncryptor(ByteBuffers.toByteArray(key)); - byte[] mlkMetadataBytes = ByteBuffers.toByteArray(mlkMetadata.buffer()); + byte[] keyMetadataBytes = ByteBuffers.toByteArray(keyMetadata.buffer()); // Use key encryption key timestamp as AES GCM signature (AAD) of encryption - in order to // prevent timestamp tampering attacks byte[] encryptedKeyMetadata = - encryptor.encrypt(mlkMetadataBytes, keyTimestamp.getBytes(StandardCharsets.UTF_8)); + encryptor.encrypt(keyMetadataBytes, keyTimestamp.getBytes(StandardCharsets.UTF_8)); return ByteBuffer.wrap(encryptedKeyMetadata); } diff --git a/core/src/main/java/org/apache/iceberg/encryption/StandardEncryptionManager.java b/core/src/main/java/org/apache/iceberg/encryption/StandardEncryptionManager.java index ec1719a0b45b..c4b2a8d5ce9b 100644 --- a/core/src/main/java/org/apache/iceberg/encryption/StandardEncryptionManager.java +++ b/core/src/main/java/org/apache/iceberg/encryption/StandardEncryptionManager.java @@ -106,6 +106,11 @@ public Iterable decrypt(Iterable encrypted) { return Iterables.transform(encrypted, this::decrypt); } + @Override + public ByteBuffer decryptKeyMetadata(String keyId) { + return EncryptionUtil.decryptKeyMetadata(keyId, this); + } + private LoadingCache unwrappedKeyCache() { if (this.unwrappedKeyCache == null) { this.unwrappedKeyCache = @@ -168,18 +173,16 @@ private long currentTimeMillis() { return System.currentTimeMillis() + testTimeShift; } - ByteBuffer encryptedByKey(String manifestListKeyID) { - EncryptedKey encryptedKeyMetadata = encryptionKeys.get(manifestListKeyID); + ByteBuffer encryptedByKey(String keyId) { + EncryptedKey encryptedKeyMetadata = encryptionKeys.get(keyId); Preconditions.checkState( - encryptedKeyMetadata != null, - "Cannot find manifest list key metadata with id %s", - manifestListKeyID); + encryptedKeyMetadata != null, "Cannot find manifest list key metadata with id %s", keyId); Preconditions.checkArgument( !encryptedKeyMetadata.encryptedById().equals(tableKeyId), "%s is a key encryption key, not manifest list key metadata", - manifestListKeyID); + keyId); return unwrappedKeyCache().get(encryptedKeyMetadata.encryptedById()); } @@ -207,7 +210,7 @@ public FileEncryptionKeys registerKeyMetadata(NativeEncryptionKeyMetadata keyMet EncryptedKey keyEncryptionKey = encryptionKeys.get(keyEncryptionKeyID); String keyEncryptionKeyTimestamp = keyEncryptionKey.properties().get(KEY_TIMESTAMP); ByteBuffer encryptedKeyMetadata = - EncryptionUtil.encryptManifestListKeyMetadata( + EncryptionUtil.encryptKeyMetadata( unwrappedKeyCache().get(keyEncryptionKeyID), keyEncryptionKeyTimestamp, keyMetadata); BaseEncryptedKey key = new BaseEncryptedKey(fileKeyID, encryptedKeyMetadata, keyEncryptionKeyID, null); diff --git a/core/src/test/java/org/apache/iceberg/TestAllManifestsTableTaskParser.java b/core/src/test/java/org/apache/iceberg/TestAllManifestsTableTaskParser.java index 05656926a881..7ea9b3ac4987 100644 --- a/core/src/test/java/org/apache/iceberg/TestAllManifestsTableTaskParser.java +++ b/core/src/test/java/org/apache/iceberg/TestAllManifestsTableTaskParser.java @@ -91,7 +91,8 @@ private AllManifestsTable.ManifestListReadTask createTask() { fileIO, AllManifestsTable.MANIFEST_FILE_SCHEMA, specsById, - new BaseManifestListFile("/path/manifest-list-file.avro", "a"), + "/path/manifest-list-file.avro", + "a", Expressions.equal("id", 1), 1L); } @@ -147,9 +148,8 @@ private void assertTaskEquals( .isEqualTo(expected.schema().asStruct()); assertThat(actual.specsById()).isEqualTo(expected.specsById()); - assertThat(actual.manifestList().location()).isEqualTo(expected.manifestList().location()); - assertThat(actual.manifestList().encryptionKeyID()) - .isEqualTo(expected.manifestList().encryptionKeyID()); + assertThat(actual.location()).isEqualTo(expected.location()); + assertThat(actual.encryptionKeyId()).isEqualTo(expected.encryptionKeyId()); assertThat(actual.residual().toString()).isEqualTo(expected.residual().toString()); assertThat(actual.referenceSnapshotId()).isEqualTo(expected.referenceSnapshotId()); } diff --git a/core/src/test/java/org/apache/iceberg/TestManifestListEncryption.java b/core/src/test/java/org/apache/iceberg/TestManifestListEncryption.java index 94fe4f615e34..7eaa359fa036 100644 --- a/core/src/test/java/org/apache/iceberg/TestManifestListEncryption.java +++ b/core/src/test/java/org/apache/iceberg/TestManifestListEncryption.java @@ -332,9 +332,7 @@ private List readManifestListWithCommittedKeys( EncryptingFileIO.combine( new TestTables.LocalFileIO(), EncryptionTestHelpers.createEncryptionManager(metadata.encryptionKeys()))) { - return ManifestLists.read( - io.newInputFile( - new BaseManifestListFile(snapshot.manifestListLocation(), snapshot.keyId()))); + return ManifestLists.read(io.newInputFile(snapshot.manifestListLocation(), snapshot.keyId())); } } @@ -374,9 +372,7 @@ private ManifestFile writeAndReadEncryptedManifestList(EncryptionManager em) thr List.of(encryptionKeys.keyEncryptionKey(), encryptionKeys.fileKey())))) { List manifests = ManifestLists.read( - readingIO.newInputFile( - new BaseManifestListFile( - outputFile.location(), encryptionKeys.fileKey().keyId()))); + readingIO.newInputFile(outputFile.location(), encryptionKeys.fileKey().keyId())); assertThat(manifests).hasSize(1); return manifests.get(0); } diff --git a/core/src/test/java/org/apache/iceberg/hadoop/TestCatalogUtilDropTable.java b/core/src/test/java/org/apache/iceberg/hadoop/TestCatalogUtilDropTable.java index 7c1e284b27e9..cc1a8ef72e73 100644 --- a/core/src/test/java/org/apache/iceberg/hadoop/TestCatalogUtilDropTable.java +++ b/core/src/test/java/org/apache/iceberg/hadoop/TestCatalogUtilDropTable.java @@ -35,7 +35,6 @@ import org.apache.iceberg.GenericStatisticsFile; import org.apache.iceberg.ImmutableGenericPartitionStatisticsFile; import org.apache.iceberg.ManifestFile; -import org.apache.iceberg.ManifestListFile; import org.apache.iceberg.PartitionStatisticsFile; import org.apache.iceberg.Snapshot; import org.apache.iceberg.SnapshotChanges; @@ -199,9 +198,11 @@ private static FileIO createMockFileIO(FileIO wrapped) { .thenAnswer( invocation -> wrapped.newInputFile(invocation.getArgument(0), invocation.getArgument(1))); - Mockito.when(mockIO.newInputFile(Mockito.any(ManifestListFile.class))) + Mockito.when(mockIO.newInputFile(Mockito.anyString(), Mockito.nullable(String.class))) .thenAnswer( - invocation -> wrapped.newInputFile((ManifestListFile) invocation.getArgument(0))); + invocation -> + wrapped.newInputFile( + (String) invocation.getArgument(0), (String) invocation.getArgument(1))); Mockito.when(mockIO.newInputFile(Mockito.any(ManifestFile.class))) .thenAnswer(invocation -> wrapped.newInputFile((ManifestFile) invocation.getArgument(0))); Mockito.when(mockIO.newInputFile(Mockito.any(DataFile.class)))