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

s3proxy: add initial implementation #2385

Merged
merged 2 commits into from
Oct 6, 2023
Merged

s3proxy: add initial implementation #2385

merged 2 commits into from
Oct 6, 2023

Conversation

derpsteb
Copy link
Member

@derpsteb derpsteb commented Sep 27, 2023

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:

  • e2e tests based on minio/mint: I would like to discuss again how we want to put this into the Constellation repo.
  • deployment automation as a bazel target: plan is to have a target that builds the image, pushes it and deploys s3proxy with the image into a cluster that was setup separately. This seems like the easiest development setup.
  • 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)

  • Add initial implementation of s3proxy

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.
  • Added Thomas as reviewer, but feedback from anyone else is welcome.

Checklist

  • Add labels (e.g., for changelog category)
  • Is PR title adequate for changelog?
  • Link to Milestone

@derpsteb derpsteb added the no changelog Change won't be listed in release changelog label Sep 27, 2023
@derpsteb derpsteb added this to the v2.12.0 milestone Sep 27, 2023
@derpsteb derpsteb force-pushed the feat/s3proxy/mvp branch 2 times, most recently from c7e2258 to 49061ef Compare September 27, 2023 11:11
@edgelesssys edgelesssys deleted a comment from netlify bot Sep 27, 2023
@edgelesssys edgelesssys deleted a comment from github-actions bot Sep 27, 2023
s3proxy/cmd/main.go Outdated Show resolved Hide resolved
s3proxy/internal/crypto/crypto.go Outdated Show resolved Hide resolved
s3proxy/internal/crypto/crypto.go Outdated Show resolved Hide resolved
s3proxy/internal/crypto/crypto.go Show resolved Hide resolved
s3proxy/internal/crypto/crypto.go Outdated Show resolved Hide resolved
s3proxy/internal/router/object.go Outdated Show resolved Hide resolved
s3proxy/internal/router/router.go Outdated Show resolved Hide resolved
s3proxy/deploy/create_cert.sh Outdated Show resolved Hide resolved
s3proxy/cmd/main.go Outdated Show resolved Hide resolved
s3proxy/deploy/deployment-s3proxy.yaml Outdated Show resolved Hide resolved
s3proxy/deploy/deployment-s3proxy.yaml Outdated Show resolved Hide resolved
s3proxy/internal/router/router.go Outdated Show resolved Hide resolved
s3proxy/internal/router/router.go Outdated Show resolved Hide resolved
s3proxy/internal/router/router.go Outdated Show resolved Hide resolved
s3proxy/cmd/main.go Outdated Show resolved Hide resolved
s3proxy/cmd/main.go Outdated Show resolved Hide resolved
@derpsteb derpsteb requested a review from 3u13r October 6, 2023 07:27
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.
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 derpsteb added feature This introduces new functionality and removed no changelog Change won't be listed in release changelog labels Oct 6, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2023

Coverage report

Package Old New Trend
s3proxy/cmd 0.00% [no test files] 🚨
s3proxy/internal/crypto 0.00% 70.40% 🆕
s3proxy/internal/kms 0.00% 85.70% 🆕
s3proxy/internal/router 0.00% 4.60% 🆕
s3proxy/internal/s3 0.00% [no test files] 🚨

Copy link
Member

@3u13r 3u13r left a 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?

@derpsteb
Copy link
Member Author

derpsteb commented Oct 6, 2023

Yes. Put into AB#3479 and AB#3480.

@derpsteb derpsteb merged commit 887dcda into main Oct 6, 2023
9 checks passed
@derpsteb derpsteb deleted the feat/s3proxy/mvp branch October 6, 2023 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This introduces new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants