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

Storage encryption proposal #603

Merged
merged 25 commits into from
Jan 20, 2023
Merged

Storage encryption proposal #603

merged 25 commits into from
Jan 20, 2023

Conversation

kasabe28
Copy link
Contributor

@kasabe28 kasabe28 commented Jan 10, 2023

Proposed Changes

Storage encryption proposal to enable onmetal volume encryption with user provided encryption key

@kasabe28 kasabe28 requested a review from a team as a code owner January 10, 2023 09:36
@github-actions github-actions bot added size/M documentation Improvements or additions to documentation labels Jan 10, 2023
@kasabe28 kasabe28 marked this pull request as draft January 10, 2023 10:05
Copy link
Member

@lukasfrank lukasfrank left a comment

Choose a reason for hiding this comment

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

For me is the overall flow a bit unclear. Can you elaborate the interplay of a VolumePool and a Volume. Do you implicit assume that a VolumePool is encrypted (that assigned volumes) if the secret in spec.encryption is given? What is expected if the Volume encryption flag is set to false and it is assigned to an "encrypted" pool?

docs/proposals/06-storage-encryption.md Outdated Show resolved Hide resolved
docs/proposals/06-storage-encryption.md Outdated Show resolved Hide resolved
@kasabe28
Copy link
Contributor Author

kasabe28 commented Jan 11, 2023

For me is the overall flow a bit unclear. Can you elaborate the interplay of a VolumePool and a Volume. Do you implicit assume that a VolumePool is encrypted (that assigned volumes) if the secret in spec.encryption is given? What is expected if the Volume encryption flag is set to false and it is assigned to an "encrypted" pool?

As per discussion now encryption key secret reference will be provided in Volume.

  • If encryption enabled flag is false, then volume will not be encrypted
  • If encryption enabled flag is set to true, then user can provide encryption key via secret reference to encrypt onmetal Volume.
  • If encryption enabled flag is set to true and user has not provided encryption key secret reference in Volume, then Volume will be encrypted with shared key of storage provider.

@kasabe28 kasabe28 marked this pull request as ready for review January 16, 2023 10:21
Copy link

@ManuStoessel ManuStoessel left a comment

Choose a reason for hiding this comment

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

Up for discussion: behaviour when a user does not set a secret for a encrypted volume explicitely. I would prefer auto-generating an individual secret instead of relying on a shared secret in that case.

docs/proposals/06-storage-encryption.md Outdated Show resolved Hide resolved
docs/proposals/06-storage-encryption.md Outdated Show resolved Hide resolved
docs/proposals/06-storage-encryption.md Outdated Show resolved Hide resolved
docs/proposals/06-storage-encryption.md Outdated Show resolved Hide resolved
docs/proposals/06-storage-encryption.md Outdated Show resolved Hide resolved
docs/proposals/06-storage-encryption.md Outdated Show resolved Hide resolved
docs/proposals/06-storage-encryption.md Outdated Show resolved Hide resolved
encryption key should be base64 encoded 256 bit key
@adracus adracus dismissed lukasfrank’s stale review January 20, 2023 12:45

Flow adjusted / concerns addressed.

@adracus adracus merged commit 341c8f7 into main Jan 20, 2023
@adracus adracus deleted the storage-encryption-proposal branch January 20, 2023 12:45
@afritzler afritzler added the enhancement New feature or request label Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants