Skip to content
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

S3 client-side encryption support #3218

Open
invidian opened this issue Jan 2, 2021 · 11 comments · May be fixed by #6480
Open

S3 client-side encryption support #3218

invidian opened this issue Jan 2, 2021 · 11 comments · May be fixed by #6480
Labels
Enhancement/User End-User Enhancement to Velero kind/requirement Needs Product Blocked needing input or feedback from Product Reviewed Q2 2021

Comments

@invidian
Copy link
Contributor

invidian commented Jan 2, 2021

Describe the problem/challenge you have
In addition to server-side encryption requested in #1782, it would be great to also support client-side encryption with client-provided master key.

This would allow using not fully trusted Minio instance for example. As far as I know, right now data is encrypted using static key which does not add any protection.

Describe the solution you'd like
Add support for specifying client-side encryption master key for S3 backup storage location.

Anything else you would like to add:

S3 documentation: https://docs.aws.amazon.com/AmazonS3/latest/dev/UsingClientSideEncryption.html

I guess this could be workaround using some sort of S3 proxy, which would handle the encryption independent from Velero.

Vote on this issue!

This is an invitation to the Velero community to vote on issues, you can see the project's top voted issues listed here.
Use the "reaction smiley face" up to the right of this comment to vote.

  • 👍 for "The project would be better with this feature added"
  • 👎 for "This feature will not enhance the project in a meaningful way"
@zubron zubron added Needs Product Blocked needing input or feedback from Product Needs investigation labels Jan 4, 2021
@nrb nrb added the Enhancement/User End-User Enhancement to Velero label Jan 11, 2021
@stale
Copy link

stale bot commented Jul 11, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the staled label Jul 11, 2021
@invidian
Copy link
Contributor Author

Not stale.

@stale stale bot removed the staled label Jul 11, 2021
@reasonerjt reasonerjt added kind/requirement Reviewed Q2 2021 Needs Product Blocked needing input or feedback from Product Enhancement/User End-User Enhancement to Velero and removed Enhancement/User End-User Enhancement to Velero Needs Product Blocked needing input or feedback from Product Reviewed Q2 2021 labels May 20, 2022
@eleanor-millman
Copy link
Contributor

@Lyndon-Li am I right to assume that this would come after Kopia integration?

@Lyndon-Li
Copy link
Contributor

@eleanor-millman
Yes, but more accurately speaking, should be after we have the Unified Data Path.

More Details:

  1. After Kopia Integration, everything writes to Kopia will be encrypted before going to the storage (i.e. minio or S3, etc.), this is the so-called Client-Side Encryption
  2. Kopia is using its own mechanism for the encryption, simply speaking: generate master key from repository password->generate data key from master key->encrypt data by data key. As you can see, everything is from the user provided repository password, but it is not used for encryption directly, this is similar with AWS's Client-Side Encryption. For more details, visit https://kopia.io/docs/advanced/encryption/.

This is to say we will have the good enough Client-Side Encryption after Kopia Integration, but we don't use AWS's Client-Side Encryption, because AWS's solution is vendor specific, we want to have a generic solution that work for every backup storage.

Gaps after Kopia Integration:

  1. Currently, Velero has a repository password (for Restic), but it is not changeable by users. This is a necessary and relatively small change, we can discuss whether to add this in next release @reasonerjt
  2. Not all data goes to Kopia, for example, the K8s metadata. This kind of data is not encrypted until we have the Unified Repository

To make it better in future:
Comparing with AWS's Client-Side Encryption, we miss the capability to integrate with KMS solutions, for example, AWS KMS, Azure Key Vault, HashiCorp Valut, etc. This is feasible technically, so we may add this in future.

@eleanor-millman
Copy link
Contributor

@Lyndon-Li Thanks so much for the detail! It is clear then that we won't tackle this for 1.10, but I'm happy that the other 1.10+ work (described above by you) will move us towards implementing this issue.

@jelemux
Copy link

jelemux commented Jan 9, 2023

Hi, we are interested in having a changeable Restic repository password.
If nobody else is working on this, we'd like to provide a PR to implement this feature.
We have several approaches and would like to ask which of them suits your architecture:

  1. Storing the backup repository password as a Secret on the cluster.
  2. Providing the backup repository password on each backup and restore call. We're are not sure if this is possible for scheduled backups etc.
  3. Adding a Plugin API for retrieving the backup repository password. A plugin for (1) could be the default here. Additional plugins for various KMS solutions can be added later on.

Do you have any more ideas or suggestions?

@jelemux
Copy link

jelemux commented Jan 9, 2023

Was (1) already implemented with #5167? To us, it seems that way.

EDIT: Strange, it seems that the files stored on S3 are not encrypted at all. Could it be that Restic is only used for the in-cluster storage of the backups?

@Lyndon-Li
Copy link
Contributor

Yes, it is implemented and we will document it, follow #5443

@jelemux
Copy link

jelemux commented Jan 10, 2023

Thanks for the quick response!
Why aren't the files stored on S3 encrypted then?

We're currently looking into adding encryption support into the AWS S3 Velero Plugin as it's the easiest solution. A rough prototype can be found here: https:/cloudogu/velero-plugin-for-aws/tree/feature/encrypted_aws
However the cleaner solution would be to integrate this directly into Velero. We'd like to implement this if it's ok with you.

@Lyndon-Li
Copy link
Contributor

There are two kinds of data in Velero's backup object store:

  1. The kubenetes metadata and backup metadata, this part is not encrypted
  2. The PV data backed up by pod volume backup, this part is encrypted

For 1, personally I don't see any technical blockings, just architecture change and efforts.
It is not a good idea to implement it in plugins, otherwise, we have to implement it in every plugin.

@jelemux
Copy link

jelemux commented Jan 10, 2023

Ah, I see. As we're using Longhorn as a storage provider, (2) doesn't do anything for us.

Yeah, I realize that implementing it in the plugin is not the best solution. It was just a quick PoC.
If we contribute this feature, we'll implement it directly in Velero.

Thanks for clearing things up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement/User End-User Enhancement to Velero kind/requirement Needs Product Blocked needing input or feedback from Product Reviewed Q2 2021
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants