Skip to content

Conversation

@andersonm-ibm
Copy link

@andersonm-ibm andersonm-ibm commented Feb 9, 2022

An implementation of the KmsClient API where master encryption keys are managed in Hashicorp Vault by its transit engine.
The KMS client is configured with table properties:

kms.client.vault.access.token.file
kms.client.vault.instance.url

The token is passed as a table property and not as an environment variable because of consistency. Security of the token is less of an issue here than in a file keystore, since it should be a non-root token that has a time-to-live (TTL). However, a RAM disk or tmpfs can be used to store the file with the token.

This PR depends on the encryption PRs, in particular the KMS client interface : #3470.

@ggershinsky

Encryption keys managed in Hashicorp Vault by its
transit engine.
@liujinhui1994
Copy link

Why can't it be used in production, and what if I want to use it in production? Can you help me understand?

@andersonm-ibm

@andersonm-ibm
Copy link
Author

Why can't it be used in production, and what if I want to use it in production? Can you help me understand?

@andersonm-ibm

Hi @liujinhui1994 . This can be used as the basis for the production-grade KMS client. However, some of the reasons for this not being production-ready, in no specific order:

  • We won't be supporting changes in Vault API, which might break this client , or maintaining different versions of this client
  • More flexibility might be needed in the definition of the path to the transit engine, which in this example is hardcoded to "/v1/transit"
  • The final decision on how to pass the access token might depend on the production environment and company policies
  • Error handling can be made more tailored to specific use cases. For example, if you need to differentiate the cases where Vault doesn't grant access to the keys, then you would define a specific exception
  • More rigorous testing would be required to cover all the scenarios relevant to a production environment

@liujinhui1994
Copy link

@andersonm-ibm Please help me understand #3470 why not use parquet KMSClient here but wrap one myself.
org.apache.parquet.crypto.keytools.KmsClient
thanks~

@andersonm-ibm
Copy link
Author

@andersonm-ibm Please help me understand #3470 why not use parquet KMSClient here but wrap one myself. org.apache.parquet.crypto.keytools.KmsClient thanks~

If I understand correctly your question, you're asking why do we have a separate KmsClient interface in Iceberg, different from the one defined in parquet.
Then @ggershinsky would be the best person to answer you.

@andersonm-ibm
Copy link
Author

Why can't it be used in production, and what if I want to use it in production? Can you help me understand?
@andersonm-ibm

Hi @liujinhui1994 . This can be used as the basis for the production-grade KMS client. However, some of the reasons for this not being production-ready, in no specific order:

* We won't be supporting changes in Vault API, which might break this client , or maintaining different versions of this client

* More flexibility might be needed in the definition of the path to the transit engine, which in this example is hardcoded to  "/v1/transit"

* The final decision on how to pass the access token might depend on the production environment and company policies

* Error handling can be made more tailored to specific use cases. For example, if you need to differentiate the cases where Vault doesn't grant access to the keys, then you would define a specific exception

* More rigorous testing would be required to cover all the scenarios relevant to a production environment

And, of course, a large and important missing part here is token lifecycle management.
It would be interesting to hear about various usage scenarios in order to understand which token lifecycle events to support.

@ggershinsky
Copy link
Contributor

ggershinsky commented Feb 10, 2022

The KMS client is configured with Hadoop configuration properties:

I think the url is better set as a table property

/**
* Property name for Vault access token.
*/
public static final String ACCESS_TOKEN_PROP = "keystore.kms.client.access.token";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"keystore" -> "vault" ?

/**
* Property name for Vault instance URL
*/
public static final String KMS_INSTANCE_URL_PROP = "keystore.kms.client.instance.url";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"keystore" -> "vault" ? or even "kms.client.vault.instance.url"?

@ggershinsky
Copy link
Contributor

The token is passed as a hadoop property and not as an environment variable because of consistency. Security of the token is less of an issue here than in a file keystore, since it should be a non-root token that has a time-to-live (TTL).

Agreed, though something like a ramdisk file can be better than a hadoop property (easier to refresh the token contents once in TTL, and less dependency on hadoop; the latter though should not be a big issue in a sample)

@ggershinsky
Copy link
Contributor

why do we have a separate KmsClient interface in Iceberg, different from the one defined in parquet.

We build a general key management layer in Iceberg encryption, that will work for all Iceberg data formats, so it should not depend on parquet. Also, this KmsClient is cleaner and more intuitive than what we have in parquet-mr.

@github-actions
Copy link

github-actions bot commented Aug 7, 2024

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.

@github-actions github-actions bot added the stale label Aug 7, 2024
@github-actions
Copy link

This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

@github-actions github-actions bot closed this Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants