-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[core] Introduce encryption interface for paimon #2903
Conversation
.withDescription( | ||
"The kms client for encryption, if the user has enabled encryption, the kms client must be specified."); | ||
|
||
public static final ConfigOption<String> HADOOP_SECURITY = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a table option?
I think this can be configured from catalog or evironment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated it
|
||
/** The kms client for encryption. */ | ||
public enum EncryptionKmsClient implements DescribedEnum { | ||
MEMORY("memory", "Use memory kms for encryption, this is only for test."), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is only for test, why it need to be a public option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not just for test, it is a variable of the EncryptionManager
, if the user has enabled encryption, the kms client must be specified
# limitations under the License | ||
|
||
org.apache.paimon.encryption.PlaintextEncryptionManager | ||
org.apache.paimon.encryption.EnvelopeEncryptionManager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this? No implementation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is my issue, I forgot to submit the EnvelopeEncryptionManager
|
||
private final String keyId; | ||
private final byte[] encryptedKey; | ||
private final byte[] plaintextKey; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we need plaintextKey
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When using the file writer to write data, the plaintext data key is required for encryption. EncryptionManager#createLocalDataKey
method is used to get a wrapper for the data key ,KeyMetadata
will be passed as a parameter between methods
KeyMetadata
will be persisted, but the plaintext data key cannot be stored.
ae727b7
to
657b2d9
Compare
657b2d9
to
c3a8fa7
Compare
@@ -1084,6 +1085,33 @@ public class CoreOptions implements Serializable { | |||
"Whether to enable deletion vectors mode. In this mode, index files containing deletion" | |||
+ " vectors are generated when data is written, which marks the data for deletion." | |||
+ " During read operations, by applying these index files, merging can be avoided."); | |||
public static final ConfigOption<EncryptionMechanism> ENCRYPTION_MECHANISM = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a line above
|
||
/** KMS client interface, provide common operations for KMS. */ | ||
public interface KmsClient extends Serializable, Closeable { | ||
void configure(Options options); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a line above
|
||
/** Provide base operations for KMS. */ | ||
public abstract class KmsClientBase implements KmsClient { | ||
public static final int NONCE_LENGTH = 12; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a line above
|
||
@Override | ||
public byte[] decryptDataKey(byte[] ciphertext, byte[] kek) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove empty line
fields.add(new DataField(0, "_KEY_ID", newStringType(false))); | ||
fields.add(new DataField(1, "_ENCRYPTED_KEY", newBytesType(false))); | ||
fields.add(new DataField(2, "_AAD_PREFIX", newBytesType(true))); | ||
return new RowType(fields); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here does not have plaintextKey
?
|
||
private final String keyId; | ||
private final byte[] encryptedKey; | ||
private final byte[] plaintextKey; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still can not get it, iceberg StandardKeyMetadata
has no this?
75d7221
to
3a66f2a
Compare
3a66f2a
to
c885771
Compare
encryption needs lots of design, close this first. |
Purpose
Linked issue: (#2113)
Tests
API and Format
Documentation