-
Notifications
You must be signed in to change notification settings - Fork 55
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
s3proxy: add initial implementation #2385
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
derpsteb
force-pushed
the
feat/s3proxy/mvp
branch
2 times, most recently
from
September 27, 2023 11:11
c7e2258
to
49061ef
Compare
derpsteb
force-pushed
the
feat/s3proxy/mvp
branch
from
September 27, 2023 11:27
49061ef
to
cad9b7a
Compare
3u13r
requested changes
Sep 27, 2023
derpsteb
force-pushed
the
feat/s3proxy/mvp
branch
from
October 2, 2023 07:07
cad9b7a
to
f6eb31e
Compare
3 tasks
derpsteb
force-pushed
the
feat/s3proxy/mvp
branch
from
October 4, 2023 07:30
f6eb31e
to
bf24452
Compare
daniel-weisse
requested changes
Oct 5, 2023
daniel-weisse
approved these changes
Oct 5, 2023
3u13r
requested changes
Oct 5, 2023
INSECURE! The proxy intercepts GetObject and PutObject. A manual deployment guide is included. The decryption only relies on a hardcoded, static key. Do not use with sensitive data; testing only. * Ticket to track ranged GetObject: AB#3466.
derpsteb
force-pushed
the
feat/s3proxy/mvp
branch
from
October 6, 2023 08:06
c17ca57
to
59287a2
Compare
Encrypt each object with a random DEK and attach the encrypted DEK as object metadata. Encrpt the DEK with a key from the keyservice. All objects use the same KEK until a keyrotation takes place.
derpsteb
added
feature
This introduces new functionality
and removed
no changelog
Change won't be listed in release changelog
labels
Oct 6, 2023
Coverage report
|
3u13r
approved these changes
Oct 6, 2023
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.
LGTM, should we add e2e test tickets to the backlog?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
EDIT: This PR now includes keyservice integration and proper envelope encryption (#2398).
This PR adds a new service to Constellation called s3proxy. It can be used to transparently encrypt and decrypt data when using AWS S3. Data that is created while using s3proxy is tagged with
constellation-encryption
and the body is encrypted using AES-256-GCM. Data that is retrieved while using s3proxy is decrypted if has an encrypted data encryption key in it's metadata.This PR is missing:
integration with Constellation keyservice to derive keys: immediate next step.I decided to leave these things out to get some eyes on the implementation as it is now working with the e2e tests and Filestash.
There are no unittests so far as I didn't know what the correct behavior was until now. The implementation is validated through the minio/mint e2e tests and Filestash so far. Ideally I would like to revisit unittests after we have the MVP in the documentation and rely on e2e tests until then.
Proposed change(s)
Additional information
Tagging as no-changelog as at least the keyservice integration is required before users should be pointed to this. That separate PR can link to this one to have the full story.Tagging as feature as this now includes the full MVP.Checklist