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.

Sign up for GitHub

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

Jump to bottom

Refactor init & recovery to use KMS URI instead of mastersecret #987

Merged
merged 3 commits into from
Jan 19, 2023
Merged

Refactor init & recovery to use KMS URI instead of mastersecret #987

merged 3 commits into from
Jan 19, 2023

Conversation

derpsteb
Copy link
Member

Proposed change(s)

  • Move code related to communication with KMS backends to internal/kms
  • Use internal/kms in all places where we need to generate new secrets.

Additional info

  • This refactoring is happening to improve our external KMS support. After streamlining the code that relies on KMS interaction in this PR, an upcoming PR will expose eKMS configs in the config and improve useability of the existing eKMS backends.
  • See commit messages for more background

Checklist

  • Add labels (e.g., for changelog category)
  • Link to Milestone

@derpsteb derpsteb added the no changelog Change won't be listed in release changelog label Jan 16, 2023
@derpsteb derpsteb added this to the v.2.5.0 milestone Jan 16, 2023
@netlify
Copy link

netlify bot commented Jan 16, 2023 •
edited
Loading

✅ Deploy Preview for constellation-docs canceled.

Name Link
🔨 Latest commit 70b910e
🔍 Latest deploy log https://app.netlify.com/sites/constellation-docs/deploys/63c91b3c611ed100084f678e

@m1ghtym0 m1ghtym0 mentioned this pull request Jan 17, 2023
Open
8 tasks
daniel-weisse
daniel-weisse requested changes Jan 17, 2023
View reviewed changes
bootstrapper/internal/initserver/initserver.go Outdated Show resolved Hide resolved
cli/internal/cmd/init.go Outdated Show resolved Hide resolved
cli/internal/cmd/init_test.go Outdated Show resolved Hide resolved
cli/internal/cmd/recover.go Outdated Show resolved Hide resolved
disk-mapper/internal/recoveryserver/server_test.go Outdated Show resolved Hide resolved
disk-mapper/recoverproto/recover.proto Outdated Show resolved Hide resolved
internal/kms/kms/cluster/cluster.go Outdated Show resolved Hide resolved
internal/kms/setup/setup.go Outdated Show resolved Hide resolved
internal/kms/setup/setup.go Outdated Show resolved Hide resolved
@3u13r 3u13r removed their request for review January 17, 2023 11:14
@katexochen katexochen removed their request for review January 17, 2023 13:15
daniel-weisse
daniel-weisse reviewed Jan 18, 2023
View reviewed changes
bootstrapper/internal/initserver/initserver.go Outdated Show resolved Hide resolved
cli/internal/cmd/recover.go Outdated Show resolved Hide resolved
@derpsteb
keyservice: move kms code to internal/kms
71eaa30 
Recovery (disk-mapper) and init (bootstrapper)
will have to work with multiple external KMSes
in the future.
@derpsteb
Refactor init/recovery to use kms URI
e319cd1 
So far the masterSecret was sent to the initial bootstrapper
on init/recovery. With this commit this information is encoded
in the kmsURI that is sent during init.
For recover, the communication with the recoveryserver is
changed. Before a streaming gRPC call was used to
exchanges UUID for measurementSecret and state disk key.
Now a standard gRPC is made that includes the same kmsURI &
storageURI that are sent during init.
@derpsteb derpsteb merged commit a0ac957 into main Jan 19, 2023
@derpsteb derpsteb deleted the ref/ckms branch January 19, 2023 12:14
@derpsteb
Copy link
Member Author

Successful e2e test on azure: https://github.com/edgelesssys/constellation/actions/runs/3957539412/jobs/6778033767

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daniel-weisse daniel-weisse daniel-weisse approved these changes

@thomasten thomasten Awaiting requested review from thomasten thomasten is a code owner

Assignees
No one assigned
Labels
no changelog Change won't be listed in release changelog
Projects
None yet
Milestone
v.2.5.0
Development

Successfully merging this pull request may close these issues.
2 participants
@derpsteb @daniel-weisse