You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2021/08/30 13:37:02 UTC
[GitHub] [iceberg] ggershinsky opened a new pull request #3053: Data encryption MVP
ggershinsky opened a new pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053
End-to-end (sans DDL) implementation of the minimal data encryption [design](https://docs.google.com/document/d/19O_qiQumz_66CdWLpw38GFJEsUpnNxXckP9rnYIQnCo/edit?usp=sharing). This pull request collects the disjoint PRs 2639, 2638, 2640 and Jack's 2443; modifies them and adds the key generation plus other code that creates a functional implementation. It comes with an example that uses a table encryption key from a keyfile ("pkcs12" format - the closest thing to the "pem" format for symmetric keys).
@jackye1995 @rdblue @huaxingao @andersonm @shangxinli @kbendick @RussellSpitzer
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ggershinsky commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
ggershinsky commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r740804992
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java
##########
@@ -82,6 +95,104 @@ public TableMetadata current() {
return currentMetadata;
}
+ @Override
+ @SuppressWarnings("checkstyle:CyclomaticComplexity")
+ public EncryptionManager encryption() {
+ // TODO run by single thread? or synchronize?
+ if (null != encryptionManager) {
+ return encryptionManager;
+ }
+
+ TableMetadata tableMetadata = current();
+ Map<String, String> tableProperties = tableMetadata.properties();
+
+ String keyManagerType = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_MANAGER_TYPE, TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT);
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT)) {
+ encryptionManager = new PlaintextEncryptionManager();
+ return encryptionManager;
+ }
+
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_SINGLE_ENVELOPE) ||
+ keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE)) {
+ boolean doubleWrap = keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE);
+ if (doubleWrap) {
+ throw new RuntimeException("Double envelope encryption is not supported yet");
+ }
+
+ Schema tableSchema = tableMetadata.schema();
+
+ String tableKeyId = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_TABLE_KEY, null);
+ if (null == tableKeyId) {
+ throw new RuntimeException("Table encryption key is not specified");
+ }
+
+ boolean pushdown = PropertyUtil.propertyAsBoolean(tableProperties,
+ TableProperties.ENCRYPTION_PUSHDOWN_ENABLED, TableProperties.ENCRYPTION_PUSHDOWN_ENABLED_DEFAULT);
+ // TODO since TableProperties.DEFAULT_FILE_FORMAT are overwritten eg in Spark,
+ // TODO check for pushdown in each data format and throw unsupported exception in Avro
+
+ String dataEncryptionAlgorithm = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_DATA_ALGORITHM, TableProperties.ENCRYPTION_DATA_ALGORITHM_DEFAULT);
+
+ EnvelopeConfig.Builder dataFileConfBuilder = EnvelopeConfig.builderFor(tableSchema)
+ .singleWrap(tableKeyId)
+ .useAlgorithm(EncryptionAlgorithm.valueOf(dataEncryptionAlgorithm));
+
+ String columnKeysProperty = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_COLUMN_KEYS, null);
+
+ if (null != columnKeysProperty) {
+ if (!pushdown) {
+ throw new RuntimeException("Column-specific master keys are supported only in pushdown mode");
+ }
+
+ // TODO
+ throw new RuntimeException("Column-specific master keys are not supported yet");
+ }
+
+ EnvelopeConfig dataFileConfig = dataFileConfBuilder.build();
+
+ String kmsClientImpl = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_KMS_CLIENT_IMPL, null);
+
+ // Pass custom kms configuration from table and Hadoop properties
+ Map<String, String> kmsProperties = new HashMap<>();
+ for (Map.Entry<String, String> property : tableProperties.entrySet()) {
+ if (property.getKey().contains("kms.client")) { // TODO
+ kmsProperties.put(property.getKey(), property.getValue());
+ }
+ }
+
+ for (Map.Entry<String, String> property : conf) {
+ if (property.getKey().contains("kms.client")) { // TODO
+ kmsProperties.put(property.getKey(), property.getValue());
+ }
+ }
Review comment:
Actually, passing the table properties pointer to a custom code, loaded at runtime, can be problematic. Passing a separate object with a copy of relevant properties seems to be the safest option (but TBD).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] shangxinli commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
shangxinli commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r715830979
##########
File path: api/src/main/java/org/apache/iceberg/encryption/NativeFileDecryptParameters.java
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.encryption;
+
+import java.nio.ByteBuffer;
+import java.util.Map;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * Barebone decryption parameters, one object per content file.
+ * Carries the file and column encryption keys (and optional AAD prefix).
+ */
+public class NativeFileDecryptParameters {
+ private ByteBuffer fileAadPrefix;
+ private Map<String, ByteBuffer> columnKeys;
+ private ByteBuffer fileKey;
+
+ private NativeFileDecryptParameters(Map<String, ByteBuffer> columnKeys, ByteBuffer fileKey,
+ ByteBuffer fileAadPrefix) {
+ Preconditions.checkState((columnKeys != null && columnKeys.size() > 0) || fileKey != null,
+ "No file or column keys are supplied");
+ this.columnKeys = columnKeys;
+ this.fileKey = fileKey;
+ this.fileAadPrefix = fileAadPrefix;
+ }
+
+ /**
+ * Creates the builder.
+ * @param fileKey per-file encryption key. For example, used as "footer key" DEK in Parquet encryption.
+ */
+ public static Builder create(ByteBuffer fileKey) {
+ return new Builder(fileKey);
+ }
+
+ public static class Builder {
+ private ByteBuffer fileAadPrefix;
+ private Map<String, ByteBuffer> columnKeys;
+ private ByteBuffer fileKey;
+
+ private Builder(ByteBuffer fileKey) {
+ this.fileKey = fileKey;
+ }
+
+ /**
+ * Set column keys.
+ * Key retrieval can require authorization checks and involve interaction with a KMS. In this case, supply
+ * keys only for projected columns.
+ * @param columnKeyMap Map of column names to column keys. Column names must be the original names,
+ * used during content file creation. For example, Parquet will use them to find and
+ * decrypt the relevant columns.
+ */
+ public Builder columnKeys(Map<String, ByteBuffer> columnKeyMap) {
+ this.columnKeys = columnKeyMap;
+ return this;
+ }
+
+ /**
+ * Set optional AAD prefix.
+ * @param aadPrefix Will be used by Parquet for cryptographic verification of file identity
+ */
+ public Builder aadPrefix(ByteBuffer aadPrefix) {
Review comment:
Same as above
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] shangxinli commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
shangxinli commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r715947499
##########
File path: core/src/main/java/org/apache/iceberg/encryption/EncryptionAlgorithm.java
##########
@@ -0,0 +1,52 @@
+/*
+ * 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.encryption;
+
+/**
+ * Algorithm supported for file encryption.
+ */
+public enum EncryptionAlgorithm {
+ /**
+ * Counter mode (CTR) allows fast encryption with high throughput.
+ * It is an encryption only cipher and does not ensure content integrity.
+ * Inputs to CTR cipher are:
+ * 1. encryption key
+ * 2. a 16-byte initialization vector (12-byte nonce, 4-byte counter)
+ * 3. plaintext data
+ */
+ AES_CTR,
Review comment:
We don't have this in Parquet. Do you plan to add it?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ggershinsky commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
ggershinsky commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r722156455
##########
File path: core/src/main/java/org/apache/iceberg/TableProperties.java
##########
@@ -228,4 +229,35 @@ private TableProperties() {
public static final String MERGE_CARDINALITY_CHECK_ENABLED = "write.merge.cardinality-check.enabled";
public static final boolean MERGE_CARDINALITY_CHECK_ENABLED_DEFAULT = true;
+
+ /**
+ * Encryption manager type
+ */
+ public static final String ENCRYPTION_MANAGER_TYPE = "encryption.manager.type";
+ public static final String ENCRYPTION_MANAGER_TYPE_PLAINTEXT = "plaintext";
+ public static final String ENCRYPTION_MANAGER_TYPE_SINGLE_ENVELOPE = "envelope";
+ public static final String ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE = "double.envelope";
+
+ public static final String ENCRYPTION_TABLE_KEY = "encryption.table.key";
+ // HIVE-21848 format
Review comment:
will do.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ggershinsky commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
ggershinsky commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r722185317
##########
File path: api/src/main/java/org/apache/iceberg/encryption/NativeFileDecryptParameters.java
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.encryption;
+
+import java.nio.ByteBuffer;
+import java.util.Map;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * Barebone decryption parameters, one object per content file.
+ * Carries the file and column encryption keys (and optional AAD prefix).
+ */
+public class NativeFileDecryptParameters {
+ private ByteBuffer fileAadPrefix;
+ private Map<String, ByteBuffer> columnKeys;
+ private ByteBuffer fileKey;
+
+ private NativeFileDecryptParameters(Map<String, ByteBuffer> columnKeys, ByteBuffer fileKey,
+ ByteBuffer fileAadPrefix) {
+ Preconditions.checkState((columnKeys != null && columnKeys.size() > 0) || fileKey != null,
+ "No file or column keys are supplied");
+ this.columnKeys = columnKeys;
+ this.fileKey = fileKey;
+ this.fileAadPrefix = fileAadPrefix;
+ }
+
+ /**
+ * Creates the builder.
+ * @param fileKey per-file encryption key. For example, used as "footer key" DEK in Parquet encryption.
+ */
+ public static Builder create(ByteBuffer fileKey) {
+ return new Builder(fileKey);
+ }
+
+ public static class Builder {
+ private ByteBuffer fileAadPrefix;
+ private Map<String, ByteBuffer> columnKeys;
+ private ByteBuffer fileKey;
+
+ private Builder(ByteBuffer fileKey) {
+ this.fileKey = fileKey;
+ }
+
+ /**
+ * Set column keys.
+ * Key retrieval can require authorization checks and involve interaction with a KMS. In this case, supply
+ * keys only for projected columns.
+ * @param columnKeyMap Map of column names to column keys. Column names must be the original names,
+ * used during content file creation. For example, Parquet will use them to find and
+ * decrypt the relevant columns.
+ */
+ public Builder columnKeys(Map<String, ByteBuffer> columnKeyMap) {
+ this.columnKeys = columnKeyMap;
+ return this;
+ }
+
+ /**
+ * Set optional AAD prefix.
+ * @param aadPrefix Will be used by Parquet for cryptographic verification of file identity
+ */
+ public Builder aadPrefix(ByteBuffer aadPrefix) {
+ this.fileAadPrefix = aadPrefix;
+ return this;
+ }
+
+ public NativeFileDecryptParameters build() {
+ return new NativeFileDecryptParameters(columnKeys, fileKey, fileAadPrefix);
+ }
+ }
+
+ public ByteBuffer aadPrefix() {
Review comment:
This class is a bundle of parameters, so `get` is implicit.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] shangxinli commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
shangxinli commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r715830865
##########
File path: api/src/main/java/org/apache/iceberg/encryption/NativeFileDecryptParameters.java
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.encryption;
+
+import java.nio.ByteBuffer;
+import java.util.Map;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * Barebone decryption parameters, one object per content file.
+ * Carries the file and column encryption keys (and optional AAD prefix).
+ */
+public class NativeFileDecryptParameters {
+ private ByteBuffer fileAadPrefix;
+ private Map<String, ByteBuffer> columnKeys;
+ private ByteBuffer fileKey;
+
+ private NativeFileDecryptParameters(Map<String, ByteBuffer> columnKeys, ByteBuffer fileKey,
+ ByteBuffer fileAadPrefix) {
+ Preconditions.checkState((columnKeys != null && columnKeys.size() > 0) || fileKey != null,
+ "No file or column keys are supplied");
+ this.columnKeys = columnKeys;
+ this.fileKey = fileKey;
+ this.fileAadPrefix = fileAadPrefix;
+ }
+
+ /**
+ * Creates the builder.
+ * @param fileKey per-file encryption key. For example, used as "footer key" DEK in Parquet encryption.
+ */
+ public static Builder create(ByteBuffer fileKey) {
+ return new Builder(fileKey);
+ }
+
+ public static class Builder {
+ private ByteBuffer fileAadPrefix;
+ private Map<String, ByteBuffer> columnKeys;
+ private ByteBuffer fileKey;
+
+ private Builder(ByteBuffer fileKey) {
+ this.fileKey = fileKey;
+ }
+
+ /**
+ * Set column keys.
+ * Key retrieval can require authorization checks and involve interaction with a KMS. In this case, supply
+ * keys only for projected columns.
+ * @param columnKeyMap Map of column names to column keys. Column names must be the original names,
+ * used during content file creation. For example, Parquet will use them to find and
+ * decrypt the relevant columns.
+ */
+ public Builder columnKeys(Map<String, ByteBuffer> columnKeyMap) {
Review comment:
Should it be named as 'withColumnKeys'? It is longer but with verb & noun
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ggershinsky commented on pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
ggershinsky commented on pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#issuecomment-1051759560
This PR is out of date. As per the comment above, is had been split into a number of chunks. These have quite a few unitests.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] flyrain commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
flyrain commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r709417504
##########
File path: api/src/main/java/org/apache/iceberg/encryption/EncryptedOutputFile.java
##########
@@ -31,6 +31,14 @@
*/
public interface EncryptedOutputFile {
+ /**
+ * Use flat filestream encryption (default) or pushdown to native format encryption
+ */
+ default boolean useNativeEncryption() {
+ return false;
+ }
+
+
Review comment:
Nit: an extra empty line
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] shangxinli commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
shangxinli commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r715842616
##########
File path: core/src/main/java/org/apache/iceberg/encryption/Ciphers.java
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.encryption;
+
+import java.security.GeneralSecurityException;
+import java.security.SecureRandom;
+import javax.crypto.AEADBadTagException;
+import javax.crypto.Cipher;
+import javax.crypto.spec.GCMParameterSpec;
+import javax.crypto.spec.SecretKeySpec;
+
+public class Ciphers {
+
+ public static class AesGcmEncryptor {
+ public static final int NONCE_LENGTH = 12;
+ public static final int GCM_TAG_LENGTH = 16;
+ public static final int GCM_TAG_LENGTH_BITS = 8 * GCM_TAG_LENGTH;
+
+ private final SecretKeySpec aesKey;
+ private final Cipher cipher;
+ private final SecureRandom randomGenerator;
+
+ AesGcmEncryptor(byte[] keyBytes) {
+ int keyLength = keyBytes.length;
+ if (!(keyLength == 16 || keyLength == 24 || keyLength == 32)) {
+ throw new IllegalArgumentException("Wrong key length " + keyLength);
+ }
+ this.aesKey = new SecretKeySpec(keyBytes, "AES");
+
+ try {
+ this.cipher = Cipher.getInstance("AES/GCM/NoPadding");
+ } catch (GeneralSecurityException e) {
+ throw new RuntimeException("Failed to create GCM cipher", e);
+ }
+
+ this.randomGenerator = new SecureRandom();
+ }
+
+ public byte[] encrypt(byte[] plainText) {
Review comment:
It would be great to have another API to allow to input the length of the plainText. During the Parquet [PR](https://github.com/apache/parquet-mr/pull/928), I found sometimes we might need to reuse some array and that the data to be encrypted only take partially of the array. If we don't provide that API, we are forced to dump the data to a new array with exactly the length of the data to be encrypted.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ggershinsky commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
ggershinsky commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r722181582
##########
File path: api/src/main/java/org/apache/iceberg/encryption/NativeFileDecryptParameters.java
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.encryption;
+
+import java.nio.ByteBuffer;
+import java.util.Map;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * Barebone decryption parameters, one object per content file.
+ * Carries the file and column encryption keys (and optional AAD prefix).
+ */
+public class NativeFileDecryptParameters {
+ private ByteBuffer fileAadPrefix;
+ private Map<String, ByteBuffer> columnKeys;
+ private ByteBuffer fileKey;
+
+ private NativeFileDecryptParameters(Map<String, ByteBuffer> columnKeys, ByteBuffer fileKey,
+ ByteBuffer fileAadPrefix) {
+ Preconditions.checkState((columnKeys != null && columnKeys.size() > 0) || fileKey != null,
+ "No file or column keys are supplied");
+ this.columnKeys = columnKeys;
+ this.fileKey = fileKey;
+ this.fileAadPrefix = fileAadPrefix;
+ }
+
+ /**
+ * Creates the builder.
+ * @param fileKey per-file encryption key. For example, used as "footer key" DEK in Parquet encryption.
+ */
+ public static Builder create(ByteBuffer fileKey) {
+ return new Builder(fileKey);
+ }
+
+ public static class Builder {
+ private ByteBuffer fileAadPrefix;
+ private Map<String, ByteBuffer> columnKeys;
+ private ByteBuffer fileKey;
+
+ private Builder(ByteBuffer fileKey) {
+ this.fileKey = fileKey;
+ }
+
+ /**
+ * Set column keys.
+ * Key retrieval can require authorization checks and involve interaction with a KMS. In this case, supply
+ * keys only for projected columns.
+ * @param columnKeyMap Map of column names to column keys. Column names must be the original names,
+ * used during content file creation. For example, Parquet will use them to find and
+ * decrypt the relevant columns.
+ */
+ public Builder columnKeys(Map<String, ByteBuffer> columnKeyMap) {
Review comment:
sounds good to me.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] shangxinli commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
shangxinli commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r715833175
##########
File path: api/src/main/java/org/apache/iceberg/encryption/NativeFileDecryptParameters.java
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.encryption;
+
+import java.nio.ByteBuffer;
+import java.util.Map;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * Barebone decryption parameters, one object per content file.
+ * Carries the file and column encryption keys (and optional AAD prefix).
+ */
+public class NativeFileDecryptParameters {
+ private ByteBuffer fileAadPrefix;
+ private Map<String, ByteBuffer> columnKeys;
+ private ByteBuffer fileKey;
+
+ private NativeFileDecryptParameters(Map<String, ByteBuffer> columnKeys, ByteBuffer fileKey,
+ ByteBuffer fileAadPrefix) {
+ Preconditions.checkState((columnKeys != null && columnKeys.size() > 0) || fileKey != null,
+ "No file or column keys are supplied");
+ this.columnKeys = columnKeys;
+ this.fileKey = fileKey;
+ this.fileAadPrefix = fileAadPrefix;
+ }
+
+ /**
+ * Creates the builder.
+ * @param fileKey per-file encryption key. For example, used as "footer key" DEK in Parquet encryption.
+ */
+ public static Builder create(ByteBuffer fileKey) {
Review comment:
Not clear why we need fileKey.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] shangxinli commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
shangxinli commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r715838931
##########
File path: core/src/main/java/org/apache/iceberg/TableProperties.java
##########
@@ -228,4 +229,35 @@ private TableProperties() {
public static final String MERGE_CARDINALITY_CHECK_ENABLED = "write.merge.cardinality-check.enabled";
public static final boolean MERGE_CARDINALITY_CHECK_ENABLED_DEFAULT = true;
+
+ /**
+ * Encryption manager type
+ */
+ public static final String ENCRYPTION_MANAGER_TYPE = "encryption.manager.type";
+ public static final String ENCRYPTION_MANAGER_TYPE_PLAINTEXT = "plaintext";
+ public static final String ENCRYPTION_MANAGER_TYPE_SINGLE_ENVELOPE = "envelope";
Review comment:
Why not using "single.envelop"?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] shangxinli commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
shangxinli commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r715832858
##########
File path: api/src/main/java/org/apache/iceberg/encryption/NativeFileEncryptParameters.java
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.encryption;
Review comment:
Can the two classes NativeFileEncryptParameters/NativeFileDecryptParameters share one? There are duplicate.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ggershinsky commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
ggershinsky commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r722193639
##########
File path: core/src/main/java/org/apache/iceberg/encryption/EncryptionAlgorithm.java
##########
@@ -0,0 +1,52 @@
+/*
+ * 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.encryption;
+
+/**
+ * Algorithm supported for file encryption.
+ */
+public enum EncryptionAlgorithm {
+ /**
+ * Counter mode (CTR) allows fast encryption with high throughput.
+ * It is an encryption only cipher and does not ensure content integrity.
+ * Inputs to CTR cipher are:
+ * 1. encryption key
+ * 2. a 16-byte initialization vector (12-byte nonce, 4-byte counter)
+ * 3. plaintext data
+ */
+ AES_CTR,
Review comment:
Nope, there is no reason to add a pure CTR cipher to Parquet. This parameter will be used by other formats. We will throw an exception if this is set for Parquet files.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ggershinsky commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
ggershinsky commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r722179525
##########
File path: parquet/src/main/java/org/apache/iceberg/parquet/ParquetWriter.java
##########
@@ -96,14 +109,24 @@
this.model = (ParquetValueWriter<T>) createWriterFunc.apply(parquetSchema);
this.metricsConfig = metricsConfig;
this.columnIndexTruncateLength = conf.getInt(COLUMN_INDEX_TRUNCATE_LENGTH, DEFAULT_COLUMN_INDEX_TRUNCATE_LENGTH);
+ this.rowGroupOrdinal = 0;
try {
this.writer = new ParquetFileWriter(ParquetIO.file(output, conf), parquetSchema,
- writeMode, rowGroupSize, 0);
+ writeMode, rowGroupSize, 0, columnIndexTruncateLength,
+ // TODO this value is used in current Iceberg. Get from conf instead?
+ ParquetProperties.DEFAULT_STATISTICS_TRUNCATE_LENGTH,
+ // TODO this value is used in current Iceberg. Get from conf instead?
Review comment:
Sounds good, I'll check if they currently can be set by users.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ggershinsky commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
ggershinsky commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r722155939
##########
File path: api/src/main/java/org/apache/iceberg/encryption/EncryptedOutputFile.java
##########
@@ -31,6 +31,14 @@
*/
public interface EncryptedOutputFile {
+ /**
+ * Use flat filestream encryption (default) or pushdown to native format encryption
+ */
+ default boolean useNativeEncryption() {
Review comment:
this is a boolean flag for an action, so having a verb makes it clearer, I think.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] flyrain commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
flyrain commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r709723506
##########
File path: parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java
##########
@@ -694,9 +757,17 @@ public static ReadBuilder read(InputFile file) {
private boolean reuseContainers = false;
private int maxRecordsPerBatch = 10000;
private NameMapping nameMapping = null;
+ private FileDecryptionProperties fileDecryptionProperties = null;
private ReadBuilder(InputFile file) {
this.file = file;
+ if (file instanceof EnvelopeEncryptedInputFile) {
+ EnvelopeEncryptedInputFile envelopeEncryptedInputFile = (EnvelopeEncryptedInputFile) file;
+ if (envelopeEncryptedInputFile.useNativeDecryption()) {
+ fileDecryptionProperties =
+ createDecryptionProperties(envelopeEncryptedInputFile.nativeDecryptionParameters());
Review comment:
Nit: indentation is off.
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java
##########
@@ -82,6 +95,104 @@ public TableMetadata current() {
return currentMetadata;
}
+ @Override
+ @SuppressWarnings("checkstyle:CyclomaticComplexity")
+ public EncryptionManager encryption() {
Review comment:
Can we extract most of logic here into a method of a utility class? These logic could be reused by other operations.
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java
##########
@@ -82,6 +95,104 @@ public TableMetadata current() {
return currentMetadata;
}
+ @Override
+ @SuppressWarnings("checkstyle:CyclomaticComplexity")
+ public EncryptionManager encryption() {
+ // TODO run by single thread? or synchronize?
+ if (null != encryptionManager) {
+ return encryptionManager;
+ }
+
+ TableMetadata tableMetadata = current();
+ Map<String, String> tableProperties = tableMetadata.properties();
+
+ String keyManagerType = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_MANAGER_TYPE, TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT);
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT)) {
+ encryptionManager = new PlaintextEncryptionManager();
+ return encryptionManager;
+ }
+
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_SINGLE_ENVELOPE) ||
+ keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE)) {
+ boolean doubleWrap = keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE);
+ if (doubleWrap) {
+ throw new RuntimeException("Double envelope encryption is not supported yet");
+ }
+
+ Schema tableSchema = tableMetadata.schema();
+
+ String tableKeyId = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_TABLE_KEY, null);
+ if (null == tableKeyId) {
+ throw new RuntimeException("Table encryption key is not specified");
+ }
Review comment:
We usually do like this `Preconditions.checkArgument(tableKeyId != null, "Table encryption key is not specified")`
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java
##########
@@ -82,6 +95,104 @@ public TableMetadata current() {
return currentMetadata;
}
+ @Override
+ @SuppressWarnings("checkstyle:CyclomaticComplexity")
+ public EncryptionManager encryption() {
+ // TODO run by single thread? or synchronize?
+ if (null != encryptionManager) {
+ return encryptionManager;
+ }
+
+ TableMetadata tableMetadata = current();
+ Map<String, String> tableProperties = tableMetadata.properties();
+
+ String keyManagerType = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_MANAGER_TYPE, TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT);
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT)) {
+ encryptionManager = new PlaintextEncryptionManager();
+ return encryptionManager;
+ }
+
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_SINGLE_ENVELOPE) ||
+ keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE)) {
+ boolean doubleWrap = keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE);
+ if (doubleWrap) {
+ throw new RuntimeException("Double envelope encryption is not supported yet");
+ }
Review comment:
We may just remove these logic since double envelope isn't supported any way, and at the end of the method we throw exception for any unsupported encryption type.
##########
File path: core/src/main/java/org/apache/iceberg/encryption/NativeEncryptedOutputFile.java
##########
@@ -0,0 +1,56 @@
+/*
+ * 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.encryption;
+
+import org.apache.iceberg.io.OutputFile;
+
+class NativeEncryptedOutputFile implements EncryptedOutputFile {
+
+ private final OutputFile rawOutput;
+ private final NativeFileEncryptParameters nativeEncryptionParameteres;
Review comment:
typo: -> "nativeEncryptionParameters"
##########
File path: core/src/main/java/org/apache/iceberg/TableProperties.java
##########
@@ -228,4 +229,35 @@ private TableProperties() {
public static final String MERGE_CARDINALITY_CHECK_ENABLED = "write.merge.cardinality-check.enabled";
public static final boolean MERGE_CARDINALITY_CHECK_ENABLED_DEFAULT = true;
+
+ /**
+ * Encryption manager type
+ */
+ public static final String ENCRYPTION_MANAGER_TYPE = "encryption.manager.type";
+ public static final String ENCRYPTION_MANAGER_TYPE_PLAINTEXT = "plaintext";
+ public static final String ENCRYPTION_MANAGER_TYPE_SINGLE_ENVELOPE = "envelope";
+ public static final String ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE = "double.envelope";
+
+ public static final String ENCRYPTION_TABLE_KEY = "encryption.table.key";
+ // HIVE-21848 format
Review comment:
It'd be nice to give format examples.
##########
File path: parquet/src/main/java/org/apache/iceberg/parquet/ParquetWriter.java
##########
@@ -96,14 +109,24 @@
this.model = (ParquetValueWriter<T>) createWriterFunc.apply(parquetSchema);
this.metricsConfig = metricsConfig;
this.columnIndexTruncateLength = conf.getInt(COLUMN_INDEX_TRUNCATE_LENGTH, DEFAULT_COLUMN_INDEX_TRUNCATE_LENGTH);
+ this.rowGroupOrdinal = 0;
try {
this.writer = new ParquetFileWriter(ParquetIO.file(output, conf), parquetSchema,
- writeMode, rowGroupSize, 0);
+ writeMode, rowGroupSize, 0, columnIndexTruncateLength,
+ // TODO this value is used in current Iceberg. Get from conf instead?
+ ParquetProperties.DEFAULT_STATISTICS_TRUNCATE_LENGTH,
+ // TODO this value is used in current Iceberg. Get from conf instead?
Review comment:
Do we allow user to set these properties? If yes, we should get them from conf, if no, the current logic should be fine.
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java
##########
@@ -82,6 +95,104 @@ public TableMetadata current() {
return currentMetadata;
}
+ @Override
+ @SuppressWarnings("checkstyle:CyclomaticComplexity")
+ public EncryptionManager encryption() {
+ // TODO run by single thread? or synchronize?
+ if (null != encryptionManager) {
+ return encryptionManager;
+ }
+
+ TableMetadata tableMetadata = current();
+ Map<String, String> tableProperties = tableMetadata.properties();
+
+ String keyManagerType = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_MANAGER_TYPE, TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT);
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT)) {
+ encryptionManager = new PlaintextEncryptionManager();
+ return encryptionManager;
+ }
+
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_SINGLE_ENVELOPE) ||
+ keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE)) {
+ boolean doubleWrap = keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE);
+ if (doubleWrap) {
+ throw new RuntimeException("Double envelope encryption is not supported yet");
+ }
+
+ Schema tableSchema = tableMetadata.schema();
+
+ String tableKeyId = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_TABLE_KEY, null);
+ if (null == tableKeyId) {
+ throw new RuntimeException("Table encryption key is not specified");
+ }
+
+ boolean pushdown = PropertyUtil.propertyAsBoolean(tableProperties,
+ TableProperties.ENCRYPTION_PUSHDOWN_ENABLED, TableProperties.ENCRYPTION_PUSHDOWN_ENABLED_DEFAULT);
+ // TODO since TableProperties.DEFAULT_FILE_FORMAT are overwritten eg in Spark,
+ // TODO check for pushdown in each data format and throw unsupported exception in Avro
+
+ String dataEncryptionAlgorithm = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_DATA_ALGORITHM, TableProperties.ENCRYPTION_DATA_ALGORITHM_DEFAULT);
+
+ EnvelopeConfig.Builder dataFileConfBuilder = EnvelopeConfig.builderFor(tableSchema)
+ .singleWrap(tableKeyId)
+ .useAlgorithm(EncryptionAlgorithm.valueOf(dataEncryptionAlgorithm));
+
+ String columnKeysProperty = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_COLUMN_KEYS, null);
+
+ if (null != columnKeysProperty) {
+ if (!pushdown) {
+ throw new RuntimeException("Column-specific master keys are supported only in pushdown mode");
+ }
+
+ // TODO
+ throw new RuntimeException("Column-specific master keys are not supported yet");
Review comment:
Use `Preconditions.checkArgument()`?
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java
##########
@@ -82,6 +95,104 @@ public TableMetadata current() {
return currentMetadata;
}
+ @Override
+ @SuppressWarnings("checkstyle:CyclomaticComplexity")
+ public EncryptionManager encryption() {
+ // TODO run by single thread? or synchronize?
+ if (null != encryptionManager) {
+ return encryptionManager;
+ }
+
+ TableMetadata tableMetadata = current();
+ Map<String, String> tableProperties = tableMetadata.properties();
+
+ String keyManagerType = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_MANAGER_TYPE, TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT);
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT)) {
+ encryptionManager = new PlaintextEncryptionManager();
+ return encryptionManager;
+ }
+
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_SINGLE_ENVELOPE) ||
+ keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE)) {
+ boolean doubleWrap = keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE);
+ if (doubleWrap) {
+ throw new RuntimeException("Double envelope encryption is not supported yet");
+ }
+
+ Schema tableSchema = tableMetadata.schema();
+
+ String tableKeyId = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_TABLE_KEY, null);
+ if (null == tableKeyId) {
+ throw new RuntimeException("Table encryption key is not specified");
+ }
+
+ boolean pushdown = PropertyUtil.propertyAsBoolean(tableProperties,
+ TableProperties.ENCRYPTION_PUSHDOWN_ENABLED, TableProperties.ENCRYPTION_PUSHDOWN_ENABLED_DEFAULT);
+ // TODO since TableProperties.DEFAULT_FILE_FORMAT are overwritten eg in Spark,
+ // TODO check for pushdown in each data format and throw unsupported exception in Avro
+
+ String dataEncryptionAlgorithm = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_DATA_ALGORITHM, TableProperties.ENCRYPTION_DATA_ALGORITHM_DEFAULT);
+
+ EnvelopeConfig.Builder dataFileConfBuilder = EnvelopeConfig.builderFor(tableSchema)
+ .singleWrap(tableKeyId)
+ .useAlgorithm(EncryptionAlgorithm.valueOf(dataEncryptionAlgorithm));
+
+ String columnKeysProperty = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_COLUMN_KEYS, null);
+
+ if (null != columnKeysProperty) {
+ if (!pushdown) {
+ throw new RuntimeException("Column-specific master keys are supported only in pushdown mode");
+ }
+
+ // TODO
+ throw new RuntimeException("Column-specific master keys are not supported yet");
+ }
+
+ EnvelopeConfig dataFileConfig = dataFileConfBuilder.build();
+
+ String kmsClientImpl = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_KMS_CLIENT_IMPL, null);
+
+ // Pass custom kms configuration from table and Hadoop properties
+ Map<String, String> kmsProperties = new HashMap<>();
+ for (Map.Entry<String, String> property : tableProperties.entrySet()) {
+ if (property.getKey().contains("kms.client")) { // TODO
+ kmsProperties.put(property.getKey(), property.getValue());
+ }
+ }
+
+ for (Map.Entry<String, String> property : conf) {
+ if (property.getKey().contains("kms.client")) { // TODO
+ kmsProperties.put(property.getKey(), property.getValue());
+ }
+ }
Review comment:
Is it cleaner if we put these logic into `TableEnvelopeKeyManager.loadKmsClient` by passing the table properties and conf?
##########
File path: core/src/main/java/org/apache/iceberg/encryption/TableEnvelopeKeyManager.java
##########
@@ -0,0 +1,185 @@
+/*
+ * 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.encryption;
+
+import java.nio.ByteBuffer;
+import java.security.SecureRandom;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.common.DynConstructors;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.types.Types;
+
+public class TableEnvelopeKeyManager implements EnvelopeKeyManager {
+ private final KmsClient kmsClient;
+ private final String tableKekID;
+ private final boolean doubleWrap;
+
+ private final boolean uniformEncryption;
+ private Set<String> columnNamesUniformEncryption;
+ private final int dataKeyLength;
+
+ private transient volatile SecureRandom workerRNG = null;
+
+ public TableEnvelopeKeyManager(KmsClient kmsClient, EnvelopeConfig dataEnvelopeConfig, boolean pushdown,
+ Schema schema, int dataKeyLength) {
+ this.kmsClient = kmsClient;
+ this.dataKeyLength = dataKeyLength;
+
+ if (dataEnvelopeConfig.isDoubleEnvelope()) {
+ throw new IllegalArgumentException("Double envelope encryption is not supported yet");
+ } else { // single wrapping
+ doubleWrap = false;
+ tableKekID = dataEnvelopeConfig.kekId();
+ Preconditions.checkArgument(tableKekID != null,
+ "Table key encryption key ID is not configured");
Review comment:
Nit: the new line isn't needed.
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java
##########
@@ -82,6 +95,104 @@ public TableMetadata current() {
return currentMetadata;
}
+ @Override
+ @SuppressWarnings("checkstyle:CyclomaticComplexity")
+ public EncryptionManager encryption() {
+ // TODO run by single thread? or synchronize?
+ if (null != encryptionManager) {
+ return encryptionManager;
+ }
+
+ TableMetadata tableMetadata = current();
+ Map<String, String> tableProperties = tableMetadata.properties();
+
+ String keyManagerType = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_MANAGER_TYPE, TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT);
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT)) {
+ encryptionManager = new PlaintextEncryptionManager();
+ return encryptionManager;
+ }
+
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_SINGLE_ENVELOPE) ||
+ keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE)) {
+ boolean doubleWrap = keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE);
+ if (doubleWrap) {
+ throw new RuntimeException("Double envelope encryption is not supported yet");
+ }
+
+ Schema tableSchema = tableMetadata.schema();
+
+ String tableKeyId = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_TABLE_KEY, null);
Review comment:
Nit: no need a new line.
##########
File path: api/src/main/java/org/apache/iceberg/encryption/EncryptedOutputFile.java
##########
@@ -31,6 +31,14 @@
*/
public interface EncryptedOutputFile {
+ /**
+ * Use flat filestream encryption (default) or pushdown to native format encryption
+ */
+ default boolean useNativeEncryption() {
Review comment:
rename it to "nativeEncryption"?
##########
File path: core/src/main/java/org/apache/iceberg/TableProperties.java
##########
@@ -228,4 +229,35 @@ private TableProperties() {
public static final String MERGE_CARDINALITY_CHECK_ENABLED = "write.merge.cardinality-check.enabled";
public static final boolean MERGE_CARDINALITY_CHECK_ENABLED_DEFAULT = true;
+
+ /**
+ * Encryption manager type
+ */
+ public static final String ENCRYPTION_MANAGER_TYPE = "encryption.manager.type";
+ public static final String ENCRYPTION_MANAGER_TYPE_PLAINTEXT = "plaintext";
+ public static final String ENCRYPTION_MANAGER_TYPE_SINGLE_ENVELOPE = "envelope";
+ public static final String ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE = "double.envelope";
+
+ public static final String ENCRYPTION_TABLE_KEY = "encryption.table.key";
+ // HIVE-21848 format
+ public static final String ENCRYPTION_COLUMN_KEYS = "encryption.column.keys";
+
+ public static final String ENCRYPTION_DEK_LENGTH = "encryption.data.key.length";
+ public static final int ENCRYPTION_DEK_LENGTH_DEFAULT = 16;
+
+ public static final String ENCRYPTION_DATA_ALGORITHM = "encryption.data.algorithm";
+ public static final String ENCRYPTION_DATA_ALGORITHM_DEFAULT = EncryptionAlgorithm.AES_GCM.toString();
+
+ /**
+ * Allow file format native encryption instead of
+ * encrypting the entire file through Iceberg encryption stream.
Review comment:
Nit: no need to add a new line. The comment can be in one line.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] shangxinli commented on pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
shangxinli commented on pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#issuecomment-1051038258
The test coverage seems low.
What kind of KMS has been tested so far?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ggershinsky commented on pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
ggershinsky commented on pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#issuecomment-934356218
> I think this will be easier to review if we break it into some chunks. I was thinking after doing a first pass that maybe it makes sense to break this into
>
> 1. Encryptors - Non Iceberg Specific Encryption Config and Methods
> The code for encrypting and decrypting bytes
> Ciphers.java
> EncryptionAlgorithm.java
> NativeFileEncryptParameters
> ....
> 2. Parquet Encryption
> Files and code relating to writing encrypted Parquet Files
> 3. Iceberg Encryption Management
> a. Envelope Metadata and Manger
> b. OutputStreams and the ability to actually turn on encryption
>
> I think this would speed up review a lot. If you have other break points to divide up the review I think that would also help. It would also make it easier for us to determine where test coverage should exist.
>
> I'm really excited by this PR and it will be a great step forward for Iceberg!
Thanks Russell! This proposal sounds good to me.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ggershinsky commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
ggershinsky commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r722189785
##########
File path: core/src/main/java/org/apache/iceberg/encryption/Ciphers.java
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.encryption;
+
+import java.security.GeneralSecurityException;
+import java.security.SecureRandom;
+import javax.crypto.AEADBadTagException;
+import javax.crypto.Cipher;
+import javax.crypto.spec.GCMParameterSpec;
+import javax.crypto.spec.SecretKeySpec;
+
+public class Ciphers {
+
+ public static class AesGcmEncryptor {
+ public static final int NONCE_LENGTH = 12;
+ public static final int GCM_TAG_LENGTH = 16;
+ public static final int GCM_TAG_LENGTH_BITS = 8 * GCM_TAG_LENGTH;
+
+ private final SecretKeySpec aesKey;
+ private final Cipher cipher;
+ private final SecureRandom randomGenerator;
+
+ AesGcmEncryptor(byte[] keyBytes) {
+ int keyLength = keyBytes.length;
+ if (!(keyLength == 16 || keyLength == 24 || keyLength == 32)) {
+ throw new IllegalArgumentException("Wrong key length " + keyLength);
+ }
+ this.aesKey = new SecretKeySpec(keyBytes, "AES");
+
+ try {
+ this.cipher = Cipher.getInstance("AES/GCM/NoPadding");
+ } catch (GeneralSecurityException e) {
+ throw new RuntimeException("Failed to create GCM cipher", e);
+ }
+
+ this.randomGenerator = new SecureRandom();
+ }
+
+ public byte[] encrypt(byte[] plainText) {
Review comment:
agreed.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ggershinsky commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
ggershinsky commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r722189371
##########
File path: core/src/main/java/org/apache/iceberg/TableProperties.java
##########
@@ -228,4 +229,35 @@ private TableProperties() {
public static final String MERGE_CARDINALITY_CHECK_ENABLED = "write.merge.cardinality-check.enabled";
public static final boolean MERGE_CARDINALITY_CHECK_ENABLED_DEFAULT = true;
+
+ /**
+ * Encryption manager type
+ */
+ public static final String ENCRYPTION_MANAGER_TYPE = "encryption.manager.type";
+ public static final String ENCRYPTION_MANAGER_TYPE_PLAINTEXT = "plaintext";
+ public static final String ENCRYPTION_MANAGER_TYPE_SINGLE_ENVELOPE = "envelope";
Review comment:
The standard technique is called "envelope encryption", so "envelope" is more intuitive / better known.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ggershinsky commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
ggershinsky commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r722171507
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java
##########
@@ -82,6 +95,104 @@ public TableMetadata current() {
return currentMetadata;
}
+ @Override
+ @SuppressWarnings("checkstyle:CyclomaticComplexity")
+ public EncryptionManager encryption() {
+ // TODO run by single thread? or synchronize?
+ if (null != encryptionManager) {
+ return encryptionManager;
+ }
+
+ TableMetadata tableMetadata = current();
+ Map<String, String> tableProperties = tableMetadata.properties();
+
+ String keyManagerType = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_MANAGER_TYPE, TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT);
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT)) {
+ encryptionManager = new PlaintextEncryptionManager();
+ return encryptionManager;
+ }
+
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_SINGLE_ENVELOPE) ||
+ keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE)) {
+ boolean doubleWrap = keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE);
+ if (doubleWrap) {
+ throw new RuntimeException("Double envelope encryption is not supported yet");
+ }
Review comment:
agreed
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ggershinsky commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
ggershinsky commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r740804992
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java
##########
@@ -82,6 +95,104 @@ public TableMetadata current() {
return currentMetadata;
}
+ @Override
+ @SuppressWarnings("checkstyle:CyclomaticComplexity")
+ public EncryptionManager encryption() {
+ // TODO run by single thread? or synchronize?
+ if (null != encryptionManager) {
+ return encryptionManager;
+ }
+
+ TableMetadata tableMetadata = current();
+ Map<String, String> tableProperties = tableMetadata.properties();
+
+ String keyManagerType = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_MANAGER_TYPE, TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT);
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT)) {
+ encryptionManager = new PlaintextEncryptionManager();
+ return encryptionManager;
+ }
+
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_SINGLE_ENVELOPE) ||
+ keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE)) {
+ boolean doubleWrap = keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE);
+ if (doubleWrap) {
+ throw new RuntimeException("Double envelope encryption is not supported yet");
+ }
+
+ Schema tableSchema = tableMetadata.schema();
+
+ String tableKeyId = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_TABLE_KEY, null);
+ if (null == tableKeyId) {
+ throw new RuntimeException("Table encryption key is not specified");
+ }
+
+ boolean pushdown = PropertyUtil.propertyAsBoolean(tableProperties,
+ TableProperties.ENCRYPTION_PUSHDOWN_ENABLED, TableProperties.ENCRYPTION_PUSHDOWN_ENABLED_DEFAULT);
+ // TODO since TableProperties.DEFAULT_FILE_FORMAT are overwritten eg in Spark,
+ // TODO check for pushdown in each data format and throw unsupported exception in Avro
+
+ String dataEncryptionAlgorithm = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_DATA_ALGORITHM, TableProperties.ENCRYPTION_DATA_ALGORITHM_DEFAULT);
+
+ EnvelopeConfig.Builder dataFileConfBuilder = EnvelopeConfig.builderFor(tableSchema)
+ .singleWrap(tableKeyId)
+ .useAlgorithm(EncryptionAlgorithm.valueOf(dataEncryptionAlgorithm));
+
+ String columnKeysProperty = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_COLUMN_KEYS, null);
+
+ if (null != columnKeysProperty) {
+ if (!pushdown) {
+ throw new RuntimeException("Column-specific master keys are supported only in pushdown mode");
+ }
+
+ // TODO
+ throw new RuntimeException("Column-specific master keys are not supported yet");
+ }
+
+ EnvelopeConfig dataFileConfig = dataFileConfBuilder.build();
+
+ String kmsClientImpl = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_KMS_CLIENT_IMPL, null);
+
+ // Pass custom kms configuration from table and Hadoop properties
+ Map<String, String> kmsProperties = new HashMap<>();
+ for (Map.Entry<String, String> property : tableProperties.entrySet()) {
+ if (property.getKey().contains("kms.client")) { // TODO
+ kmsProperties.put(property.getKey(), property.getValue());
+ }
+ }
+
+ for (Map.Entry<String, String> property : conf) {
+ if (property.getKey().contains("kms.client")) { // TODO
+ kmsProperties.put(property.getKey(), property.getValue());
+ }
+ }
Review comment:
Actually, passing the table properties pointer to a custom code, loaded at runtime, can be problematic. Passing a separate object with a copy of relevant properties seems to be the safest option (but TBD).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] shangxinli commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
shangxinli commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r715831122
##########
File path: api/src/main/java/org/apache/iceberg/encryption/NativeFileDecryptParameters.java
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.encryption;
+
+import java.nio.ByteBuffer;
+import java.util.Map;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * Barebone decryption parameters, one object per content file.
+ * Carries the file and column encryption keys (and optional AAD prefix).
+ */
+public class NativeFileDecryptParameters {
+ private ByteBuffer fileAadPrefix;
+ private Map<String, ByteBuffer> columnKeys;
+ private ByteBuffer fileKey;
+
+ private NativeFileDecryptParameters(Map<String, ByteBuffer> columnKeys, ByteBuffer fileKey,
+ ByteBuffer fileAadPrefix) {
+ Preconditions.checkState((columnKeys != null && columnKeys.size() > 0) || fileKey != null,
+ "No file or column keys are supplied");
+ this.columnKeys = columnKeys;
+ this.fileKey = fileKey;
+ this.fileAadPrefix = fileAadPrefix;
+ }
+
+ /**
+ * Creates the builder.
+ * @param fileKey per-file encryption key. For example, used as "footer key" DEK in Parquet encryption.
+ */
+ public static Builder create(ByteBuffer fileKey) {
+ return new Builder(fileKey);
+ }
+
+ public static class Builder {
+ private ByteBuffer fileAadPrefix;
+ private Map<String, ByteBuffer> columnKeys;
+ private ByteBuffer fileKey;
+
+ private Builder(ByteBuffer fileKey) {
+ this.fileKey = fileKey;
+ }
+
+ /**
+ * Set column keys.
+ * Key retrieval can require authorization checks and involve interaction with a KMS. In this case, supply
+ * keys only for projected columns.
+ * @param columnKeyMap Map of column names to column keys. Column names must be the original names,
+ * used during content file creation. For example, Parquet will use them to find and
+ * decrypt the relevant columns.
+ */
+ public Builder columnKeys(Map<String, ByteBuffer> columnKeyMap) {
+ this.columnKeys = columnKeyMap;
+ return this;
+ }
+
+ /**
+ * Set optional AAD prefix.
+ * @param aadPrefix Will be used by Parquet for cryptographic verification of file identity
+ */
+ public Builder aadPrefix(ByteBuffer aadPrefix) {
+ this.fileAadPrefix = aadPrefix;
+ return this;
+ }
+
+ public NativeFileDecryptParameters build() {
+ return new NativeFileDecryptParameters(columnKeys, fileKey, fileAadPrefix);
+ }
+ }
+
+ public ByteBuffer aadPrefix() {
Review comment:
Should we name it as getAadPrefix()?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] shangxinli commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
shangxinli commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r715947193
##########
File path: core/src/main/java/org/apache/iceberg/encryption/Ciphers.java
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.encryption;
+
+import java.security.GeneralSecurityException;
+import java.security.SecureRandom;
+import javax.crypto.AEADBadTagException;
+import javax.crypto.Cipher;
+import javax.crypto.spec.GCMParameterSpec;
+import javax.crypto.spec.SecretKeySpec;
+
+public class Ciphers {
+
+ public static class AesGcmEncryptor {
+ public static final int NONCE_LENGTH = 12;
+ public static final int GCM_TAG_LENGTH = 16;
+ public static final int GCM_TAG_LENGTH_BITS = 8 * GCM_TAG_LENGTH;
+
+ private final SecretKeySpec aesKey;
+ private final Cipher cipher;
+ private final SecureRandom randomGenerator;
+
+ AesGcmEncryptor(byte[] keyBytes) {
+ int keyLength = keyBytes.length;
+ if (!(keyLength == 16 || keyLength == 24 || keyLength == 32)) {
+ throw new IllegalArgumentException("Wrong key length " + keyLength);
+ }
+ this.aesKey = new SecretKeySpec(keyBytes, "AES");
+
+ try {
+ this.cipher = Cipher.getInstance("AES/GCM/NoPadding");
+ } catch (GeneralSecurityException e) {
+ throw new RuntimeException("Failed to create GCM cipher", e);
+ }
+
+ this.randomGenerator = new SecureRandom();
+ }
+
+ public byte[] encrypt(byte[] plainText) {
+ byte[] nonce = new byte[NONCE_LENGTH];
+ randomGenerator.nextBytes(nonce);
+ int cipherTextLength = NONCE_LENGTH + plainText.length + GCM_TAG_LENGTH;
+ byte[] cipherText = new byte[cipherTextLength];
+
+ try {
+ GCMParameterSpec spec = new GCMParameterSpec(GCM_TAG_LENGTH_BITS, nonce);
+ cipher.init(Cipher.ENCRYPT_MODE, aesKey, spec);
+ cipher.doFinal(plainText, 0, plainText.length, cipherText, NONCE_LENGTH);
+ } catch (GeneralSecurityException e) {
+ throw new RuntimeException("Failed to encrypt", e);
+ }
+
+ // Add the nonce
+ System.arraycopy(nonce, 0, cipherText, 0, NONCE_LENGTH);
+
+ return cipherText;
+ }
+ }
+
+ public static class AesGcmDecryptor {
+ private final SecretKeySpec aesKey;
+ private final Cipher cipher;
+
+ AesGcmDecryptor(byte[] keyBytes) {
+ int keyLength = keyBytes.length;
+ if (!(keyLength == 16 || keyLength == 24 || keyLength == 32)) {
+ throw new IllegalArgumentException("Wrong key length " + keyLength);
+ }
+
+ this.aesKey = new SecretKeySpec(keyBytes, "AES");
+
+ try {
+ this.cipher = Cipher.getInstance("AES/GCM/NoPadding");
+ } catch (GeneralSecurityException e) {
+ throw new RuntimeException("Failed to create GCM cipher", e);
+ }
+ }
+
+ public byte[] decrypt(byte[] ciphertext) {
Review comment:
Same as above
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] shangxinli commented on pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
shangxinli commented on pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#issuecomment-921900828
@ggershinsky Thanks for working on it! I will see if I have time next week to review it.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ggershinsky commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
ggershinsky commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r740804992
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java
##########
@@ -82,6 +95,104 @@ public TableMetadata current() {
return currentMetadata;
}
+ @Override
+ @SuppressWarnings("checkstyle:CyclomaticComplexity")
+ public EncryptionManager encryption() {
+ // TODO run by single thread? or synchronize?
+ if (null != encryptionManager) {
+ return encryptionManager;
+ }
+
+ TableMetadata tableMetadata = current();
+ Map<String, String> tableProperties = tableMetadata.properties();
+
+ String keyManagerType = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_MANAGER_TYPE, TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT);
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT)) {
+ encryptionManager = new PlaintextEncryptionManager();
+ return encryptionManager;
+ }
+
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_SINGLE_ENVELOPE) ||
+ keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE)) {
+ boolean doubleWrap = keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE);
+ if (doubleWrap) {
+ throw new RuntimeException("Double envelope encryption is not supported yet");
+ }
+
+ Schema tableSchema = tableMetadata.schema();
+
+ String tableKeyId = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_TABLE_KEY, null);
+ if (null == tableKeyId) {
+ throw new RuntimeException("Table encryption key is not specified");
+ }
+
+ boolean pushdown = PropertyUtil.propertyAsBoolean(tableProperties,
+ TableProperties.ENCRYPTION_PUSHDOWN_ENABLED, TableProperties.ENCRYPTION_PUSHDOWN_ENABLED_DEFAULT);
+ // TODO since TableProperties.DEFAULT_FILE_FORMAT are overwritten eg in Spark,
+ // TODO check for pushdown in each data format and throw unsupported exception in Avro
+
+ String dataEncryptionAlgorithm = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_DATA_ALGORITHM, TableProperties.ENCRYPTION_DATA_ALGORITHM_DEFAULT);
+
+ EnvelopeConfig.Builder dataFileConfBuilder = EnvelopeConfig.builderFor(tableSchema)
+ .singleWrap(tableKeyId)
+ .useAlgorithm(EncryptionAlgorithm.valueOf(dataEncryptionAlgorithm));
+
+ String columnKeysProperty = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_COLUMN_KEYS, null);
+
+ if (null != columnKeysProperty) {
+ if (!pushdown) {
+ throw new RuntimeException("Column-specific master keys are supported only in pushdown mode");
+ }
+
+ // TODO
+ throw new RuntimeException("Column-specific master keys are not supported yet");
+ }
+
+ EnvelopeConfig dataFileConfig = dataFileConfBuilder.build();
+
+ String kmsClientImpl = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_KMS_CLIENT_IMPL, null);
+
+ // Pass custom kms configuration from table and Hadoop properties
+ Map<String, String> kmsProperties = new HashMap<>();
+ for (Map.Entry<String, String> property : tableProperties.entrySet()) {
+ if (property.getKey().contains("kms.client")) { // TODO
+ kmsProperties.put(property.getKey(), property.getValue());
+ }
+ }
+
+ for (Map.Entry<String, String> property : conf) {
+ if (property.getKey().contains("kms.client")) { // TODO
+ kmsProperties.put(property.getKey(), property.getValue());
+ }
+ }
Review comment:
Actually, passing the table properties pointer to a custom code, loaded at runtime, can be problematic. Passing a separate object with a copy of relevant properties seems to be the safest option (but TBD).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ggershinsky commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
ggershinsky commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r722187643
##########
File path: api/src/main/java/org/apache/iceberg/encryption/NativeFileDecryptParameters.java
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.encryption;
+
+import java.nio.ByteBuffer;
+import java.util.Map;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * Barebone decryption parameters, one object per content file.
+ * Carries the file and column encryption keys (and optional AAD prefix).
+ */
+public class NativeFileDecryptParameters {
+ private ByteBuffer fileAadPrefix;
+ private Map<String, ByteBuffer> columnKeys;
+ private ByteBuffer fileKey;
+
+ private NativeFileDecryptParameters(Map<String, ByteBuffer> columnKeys, ByteBuffer fileKey,
+ ByteBuffer fileAadPrefix) {
+ Preconditions.checkState((columnKeys != null && columnKeys.size() > 0) || fileKey != null,
+ "No file or column keys are supplied");
+ this.columnKeys = columnKeys;
+ this.fileKey = fileKey;
+ this.fileAadPrefix = fileAadPrefix;
+ }
+
+ /**
+ * Creates the builder.
+ * @param fileKey per-file encryption key. For example, used as "footer key" DEK in Parquet encryption.
+ */
+ public static Builder create(ByteBuffer fileKey) {
Review comment:
in Parquet, we need it for setting the file footer key.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ggershinsky commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
ggershinsky commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r722186703
##########
File path: api/src/main/java/org/apache/iceberg/encryption/NativeFileEncryptParameters.java
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.encryption;
Review comment:
Wow!! They used to be different, but then I made a change in the design, and changed the code, without realizing this makes them identical. Thanks for this observation!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ggershinsky commented on a change in pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
ggershinsky commented on a change in pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#discussion_r722178026
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java
##########
@@ -82,6 +95,104 @@ public TableMetadata current() {
return currentMetadata;
}
+ @Override
+ @SuppressWarnings("checkstyle:CyclomaticComplexity")
+ public EncryptionManager encryption() {
+ // TODO run by single thread? or synchronize?
+ if (null != encryptionManager) {
+ return encryptionManager;
+ }
+
+ TableMetadata tableMetadata = current();
+ Map<String, String> tableProperties = tableMetadata.properties();
+
+ String keyManagerType = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_MANAGER_TYPE, TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT);
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_PLAINTEXT)) {
+ encryptionManager = new PlaintextEncryptionManager();
+ return encryptionManager;
+ }
+
+ if (keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_SINGLE_ENVELOPE) ||
+ keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE)) {
+ boolean doubleWrap = keyManagerType.equals(TableProperties.ENCRYPTION_MANAGER_TYPE_DOUBLE_ENVELOPE);
+ if (doubleWrap) {
+ throw new RuntimeException("Double envelope encryption is not supported yet");
+ }
+
+ Schema tableSchema = tableMetadata.schema();
+
+ String tableKeyId = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_TABLE_KEY, null);
+ if (null == tableKeyId) {
+ throw new RuntimeException("Table encryption key is not specified");
+ }
+
+ boolean pushdown = PropertyUtil.propertyAsBoolean(tableProperties,
+ TableProperties.ENCRYPTION_PUSHDOWN_ENABLED, TableProperties.ENCRYPTION_PUSHDOWN_ENABLED_DEFAULT);
+ // TODO since TableProperties.DEFAULT_FILE_FORMAT are overwritten eg in Spark,
+ // TODO check for pushdown in each data format and throw unsupported exception in Avro
+
+ String dataEncryptionAlgorithm = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_DATA_ALGORITHM, TableProperties.ENCRYPTION_DATA_ALGORITHM_DEFAULT);
+
+ EnvelopeConfig.Builder dataFileConfBuilder = EnvelopeConfig.builderFor(tableSchema)
+ .singleWrap(tableKeyId)
+ .useAlgorithm(EncryptionAlgorithm.valueOf(dataEncryptionAlgorithm));
+
+ String columnKeysProperty = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_COLUMN_KEYS, null);
+
+ if (null != columnKeysProperty) {
+ if (!pushdown) {
+ throw new RuntimeException("Column-specific master keys are supported only in pushdown mode");
+ }
+
+ // TODO
+ throw new RuntimeException("Column-specific master keys are not supported yet");
+ }
+
+ EnvelopeConfig dataFileConfig = dataFileConfBuilder.build();
+
+ String kmsClientImpl = PropertyUtil.propertyAsString(tableProperties,
+ TableProperties.ENCRYPTION_KMS_CLIENT_IMPL, null);
+
+ // Pass custom kms configuration from table and Hadoop properties
+ Map<String, String> kmsProperties = new HashMap<>();
+ for (Map.Entry<String, String> property : tableProperties.entrySet()) {
+ if (property.getKey().contains("kms.client")) { // TODO
+ kmsProperties.put(property.getKey(), property.getValue());
+ }
+ }
+
+ for (Map.Entry<String, String> property : conf) {
+ if (property.getKey().contains("kms.client")) { // TODO
+ kmsProperties.put(property.getKey(), property.getValue());
+ }
+ }
Review comment:
`Configuration` is a Hadoop class, that should probably be transparent to other callers of `TableEnvelopeKeyManager.loadKmsClient `. Agree re table properties, that should be ok for all, will add it as a separate parameter to this method.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ggershinsky commented on pull request #3053: Data encryption MVP
Posted by GitBox <gi...@apache.org>.
ggershinsky commented on pull request #3053:
URL: https://github.com/apache/iceberg/pull/3053#issuecomment-934330695
> Thanks for working on it, Gidon! Did my first round review. Here are some questions:
>
> 1. Can we add tests for data read and write from a table with encryption enabled. We could use the Java APIs if Spark with encryption isn’t ready. Please refer to https://iceberg.apache.org/api/#tables.
> 2. Can we add tests for HadoopTableOperations::encryption()?
> 3. Can we add tests for KmsClient?
> 4. Are we leaving out the encryption() implementation intentionally for MetastoreTableOperations? The encryption() implementation of the PR is only for HadoopTableOpertation.
> 5. Are manifest files and manifest-list files encrypted?
> 6. An Iceberg table can contain data files with multiple formats, parquet, orc, avro, etc. How do we handle that case?
Thanks Yufei!
Affirmative answer to questions 1-3.
4: nope, no reason not to add it to other classes that implement TableOperations. HadoopTableOperations implements it directly, and is used in Spark, so I started with it; will add to others. I guess you mean BaseMetastoreTableOperations?
5. not yet, WIP
6. We start with formats that have a built-in encryption support - initially Parquet, then ORC. Avro doesn't have this capability, and is not a column format, so will be supported by generic encryption streams like the one in #2060
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org