From 70f0810cb115216a06164fa13252815211845070 Mon Sep 17 00:00:00 2001 From: Johannes Hoppenstedt <5921150+jhoppenstedt@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:54:58 +0100 Subject: [PATCH 1/2] feat: support SSE-C encryption in S3 blobstores --- pkg/storage/fs/decomposeds3/README.md | 30 ++++++ .../fs/decomposeds3/blobstore/blobstore.go | 19 +++- .../decomposeds3/blobstore/blobstore_test.go | 97 +++++++++++++++++++ pkg/storage/fs/decomposeds3/decomposeds3.go | 2 +- .../fs/decomposeds3/decomposeds3_test.go | 7 ++ pkg/storage/fs/decomposeds3/option.go | 3 + pkg/storage/fs/decomposeds3/option_test.go | 2 + pkg/storage/fs/s3ng/blobstore/blobstore.go | 19 +++- .../fs/s3ng/blobstore/blobstore_test.go | 97 +++++++++++++++++++ pkg/storage/fs/s3ng/option.go | 3 + pkg/storage/fs/s3ng/option_test.go | 2 + pkg/storage/fs/s3ng/s3ng.go | 2 +- pkg/storage/fs/s3ng/s3ng_test.go | 7 ++ pkg/storage/utils/s3utils/encryption.go | 31 ++++++ pkg/storage/utils/s3utils/encryption_test.go | 53 ++++++++++ 15 files changed, 366 insertions(+), 8 deletions(-) create mode 100644 pkg/storage/fs/decomposeds3/README.md create mode 100644 pkg/storage/fs/decomposeds3/blobstore/blobstore_test.go create mode 100644 pkg/storage/fs/s3ng/blobstore/blobstore_test.go create mode 100644 pkg/storage/utils/s3utils/encryption.go create mode 100644 pkg/storage/utils/s3utils/encryption_test.go diff --git a/pkg/storage/fs/decomposeds3/README.md b/pkg/storage/fs/decomposeds3/README.md new file mode 100644 index 00000000000..e2c8dc1b819 --- /dev/null +++ b/pkg/storage/fs/decomposeds3/README.md @@ -0,0 +1,30 @@ +# S3 customer-provided encryption keys (SSE-C) + +Both `decomposeds3` and the legacy `s3ng` driver accept an optional +`s3.sse_c_key` alongside the bucket configuration: + +```toml +[grpc.services.storageprovider.drivers.decomposeds3] +"s3.endpoint" = "https://s3.example.com" +"s3.region" = "us-east-1" +"s3.bucket" = "files" +"s3.access_key" = "access-key" +"s3.secret_key" = "secret-key" +"s3.sse_c_key" = "" +``` + +Generate a random key with `openssl rand -base64 32`. Keep it in secret +configuration and retain a secure backup: the same key is needed for subsequent +reads, including metadata requests and historical file versions. It is separate +from the S3 access credentials. The endpoint must use HTTPS and the S3 service +must permit SSE-C uploads. + +An empty setting preserves the existing behavior. A configured key applies to +both upload paths, including multipart uploads, and to downloads and their +metadata requests. The SDK generates AES256, key and key-MD5 headers. Listing +and deleting objects do not send customer encryption headers. Incorrect or +missing keys fail without an unencrypted retry. + +Setting or changing the key does not migrate or re-encrypt existing objects. +Before enabling SSE-C on an existing bucket, plan its data migration separately. +Key rotation and multiple keys within one bucket configuration are not provided. diff --git a/pkg/storage/fs/decomposeds3/blobstore/blobstore.go b/pkg/storage/fs/decomposeds3/blobstore/blobstore.go index 11cbafb8598..b3e3b018e67 100644 --- a/pkg/storage/fs/decomposeds3/blobstore/blobstore.go +++ b/pkg/storage/fs/decomposeds3/blobstore/blobstore.go @@ -29,14 +29,17 @@ import ( "github.com/minio/minio-go/v7" "github.com/minio/minio-go/v7/pkg/credentials" + "github.com/minio/minio-go/v7/pkg/encrypt" "github.com/opencloud-eu/reva/v2/pkg/storage/pkg/decomposedfs/lookup" "github.com/opencloud-eu/reva/v2/pkg/storage/pkg/decomposedfs/node" + "github.com/opencloud-eu/reva/v2/pkg/storage/utils/s3utils" "github.com/pkg/errors" ) // Blobstore provides an interface to an s3 compatible blobstore type Blobstore struct { - client *minio.Client + client *minio.Client + encryption encrypt.ServerSide defaultPutOptions Options @@ -53,12 +56,17 @@ type Options struct { } // New returns a new Blobstore -func New(endpoint, region, bucket, accessKey, secretKey string, defaultPutOptions Options) (*Blobstore, error) { +// ssecKey is an optional base64-encoded 32-byte customer encryption key. +func New(endpoint, region, bucket, accessKey, secretKey, ssecKey string, defaultPutOptions Options) (*Blobstore, error) { u, err := url.Parse(endpoint) if err != nil { return nil, errors.Wrap(err, "failed to parse s3 endpoint") } + encryption, err := s3utils.NewSSEC(ssecKey, u.Scheme == "https") + if err != nil { + return nil, err + } useSSL := u.Scheme != "http" client, err := minio.New(u.Host, &minio.Options{ Region: region, @@ -71,6 +79,7 @@ func New(endpoint, region, bucket, accessKey, secretKey string, defaultPutOption return &Blobstore{ client: client, + encryption: encryption, bucket: bucket, defaultPutOptions: defaultPutOptions, }, nil @@ -86,6 +95,7 @@ func (bs *Blobstore) Upload(node *node.Node, source, _copyTarget string) error { _, err = bs.client.PutObject(context.Background(), bs.bucket, bs.Path(node), reader, node.Blobsize, minio.PutObjectOptions{ ContentType: "application/octet-stream", + ServerSideEncryption: bs.encryption, SendContentMd5: bs.defaultPutOptions.SendContentMd5, ConcurrentStreamParts: bs.defaultPutOptions.ConcurrentStreamParts, NumThreads: bs.defaultPutOptions.NumThreads, @@ -104,6 +114,7 @@ func (bs *Blobstore) Upload(node *node.Node, source, _copyTarget string) error { func (bs *Blobstore) UploadFromReader(node *node.Node, reader io.Reader, size int64) error { _, err := bs.client.PutObject(context.Background(), bs.bucket, bs.Path(node), reader, size, minio.PutObjectOptions{ ContentType: "application/octet-stream", + ServerSideEncryption: bs.encryption, SendContentMd5: bs.defaultPutOptions.SendContentMd5, ConcurrentStreamParts: bs.defaultPutOptions.ConcurrentStreamParts, NumThreads: bs.defaultPutOptions.NumThreads, @@ -119,17 +130,19 @@ func (bs *Blobstore) UploadFromReader(node *node.Node, reader io.Reader, size in // Download retrieves a blob from the blobstore for reading func (bs *Blobstore) Download(node *node.Node) (io.ReadCloser, error) { - reader, err := bs.client.GetObject(context.Background(), bs.bucket, bs.Path(node), minio.GetObjectOptions{}) + reader, err := bs.client.GetObject(context.Background(), bs.bucket, bs.Path(node), minio.GetObjectOptions{ServerSideEncryption: bs.encryption}) if err != nil { return nil, errors.Wrapf(err, "could not download object '%s' from bucket '%s'", bs.Path(node), bs.bucket) } stat, err := reader.Stat() if err != nil { + reader.Close() return nil, errors.Wrapf(err, "blob path: %s", bs.Path(node)) } if node.Blobsize != stat.Size { + reader.Close() return nil, fmt.Errorf("blob has unexpected size. %d bytes expected, got %d bytes", node.Blobsize, stat.Size) } diff --git a/pkg/storage/fs/decomposeds3/blobstore/blobstore_test.go b/pkg/storage/fs/decomposeds3/blobstore/blobstore_test.go new file mode 100644 index 00000000000..831d01c9324 --- /dev/null +++ b/pkg/storage/fs/decomposeds3/blobstore/blobstore_test.go @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: Apache-2.0 + +package blobstore + +import ( + "bytes" + "encoding/base64" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/minio/minio-go/v7" + "github.com/minio/minio-go/v7/pkg/credentials" + "github.com/opencloud-eu/reva/v2/pkg/storage/pkg/decomposedfs/node" +) + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +func TestSSECHeaders(t *testing.T) { + source := filepath.Join(t.TempDir(), "blob") + if err := os.WriteFile(source, []byte("blob"), 0600); err != nil { + t.Fatal(err) + } + key := base64.StdEncoding.EncodeToString(bytes.Repeat([]byte{42}, 32)) + for _, tc := range []struct{ name, key string }{{"unset", ""}, {"SSE-C", key}} { + t.Run(tc.name, func(t *testing.T) { + opts := Options{DisableMultipart: true, SendContentMd5: true} + if tc.key != "" { + if _, err := New("http://s3.example.com", "us-east-1", "bucket", "ak", "sk", tc.key, opts); err == nil { + t.Fatal("accepted an HTTP endpoint with SSE-C enabled") + } + } + bs, err := New("https://s3.example.com", "us-east-1", "bucket", "ak", "sk", tc.key, opts) + if err != nil { + t.Fatal(err) + } + methods := map[string]int{} + // Intercept SDK requests to check Reva's option wiring without a + // network server or emulating multipart behavior owned by the SDK. + bs.client, err = minio.New("s3.example.com", &minio.Options{ + Region: "us-east-1", Secure: true, + Creds: credentials.NewStaticV4("ak", "sk", ""), + Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + methods[r.Method]++ + if r.Header.Get("X-Amz-Server-Side-Encryption-Customer-Key") != tc.key { + t.Errorf("%s request has the wrong customer key", r.Method) + } + algorithm := "" + if tc.key != "" { + algorithm = "AES256" + } + if r.Header.Get("X-Amz-Server-Side-Encryption-Customer-Algorithm") != algorithm { + t.Errorf("%s request has the wrong encryption algorithm", r.Method) + } + if r.Body != nil { + defer r.Body.Close() + if _, err := io.Copy(io.Discard, r.Body); err != nil { + return nil, err + } + } + headers := http.Header{} + headers.Set("Content-Length", "4") + headers.Set("ETag", `"etag"`) + headers.Set("Last-Modified", "Mon, 26 Oct 2026 12:00:00 GMT") + return &http.Response{StatusCode: http.StatusOK, Header: headers, + ContentLength: 4, Body: io.NopCloser(strings.NewReader("blob")), Request: r}, nil + }), + }) + if err != nil { + t.Fatal(err) + } + n := &node.Node{BaseNode: node.BaseNode{SpaceID: "space"}, BlobID: "blob", Blobsize: 4} + if err := bs.Upload(n, source, ""); err != nil { + t.Fatalf("Upload: %v", err) + } + if err := bs.UploadFromReader(n, strings.NewReader("blob"), 4); err != nil { + t.Fatalf("UploadFromReader: %v", err) + } + reader, err := bs.Download(n) + if err != nil { + t.Fatalf("Download: %v", err) + } + defer reader.Close() + if _, err := io.Copy(io.Discard, reader); err != nil { + t.Fatal(err) + } + if methods[http.MethodPut] != 2 || methods[http.MethodHead] != 1 || methods[http.MethodGet] != 1 { + t.Fatalf("unexpected requests: %v", methods) + } + }) + } +} diff --git a/pkg/storage/fs/decomposeds3/decomposeds3.go b/pkg/storage/fs/decomposeds3/decomposeds3.go index eabf69b42e3..9abc4b379ff 100644 --- a/pkg/storage/fs/decomposeds3/decomposeds3.go +++ b/pkg/storage/fs/decomposeds3/decomposeds3.go @@ -60,7 +60,7 @@ func New(m map[string]interface{}, stream events.Stream, log *zerolog.Logger) (s PartSize: o.PartSize, } - bs, err := blobstore.New(o.S3Endpoint, o.S3Region, o.S3Bucket, o.S3AccessKey, o.S3SecretKey, defaultPutOptions) + bs, err := blobstore.New(o.S3Endpoint, o.S3Region, o.S3Bucket, o.S3AccessKey, o.S3SecretKey, o.S3SSECKey, defaultPutOptions) if err != nil { return nil, err } diff --git a/pkg/storage/fs/decomposeds3/decomposeds3_test.go b/pkg/storage/fs/decomposeds3/decomposeds3_test.go index ea7dce09d3b..69cd0d13c23 100644 --- a/pkg/storage/fs/decomposeds3/decomposeds3_test.go +++ b/pkg/storage/fs/decomposeds3/decomposeds3_test.go @@ -64,6 +64,13 @@ var _ = Describe("decomposeds3", func() { Expect(err).To(MatchError("S3 configuration incomplete")) }) + It("rejects invalid SSE-C configuration before creating storage", func() { + options["s3.endpoint"] = "https://localhost:9000" + options["s3.sse_c_key"] = "invalid-secret!" + _, err := decomposeds3.New(options, nil, &zerolog.Logger{}) + Expect(err).To(MatchError("SSE-C key must be base64-encoded")) + }) + It("works with complete configuration", func() { _, err := decomposeds3.New(options, nil, &zerolog.Logger{}) Expect(err).ToNot(HaveOccurred()) diff --git a/pkg/storage/fs/decomposeds3/option.go b/pkg/storage/fs/decomposeds3/option.go index 211fe43c4aa..f648bc980ed 100644 --- a/pkg/storage/fs/decomposeds3/option.go +++ b/pkg/storage/fs/decomposeds3/option.go @@ -38,6 +38,9 @@ type Options struct { // Bucket of the s3 blobstore S3Bucket string `mapstructure:"s3.bucket"` + // Base64-encoded AES-256 customer key for server-side encryption (optional) + S3SSECKey string `mapstructure:"s3.sse_c_key"` + // Access key for the s3 blobstore S3AccessKey string `mapstructure:"s3.access_key"` diff --git a/pkg/storage/fs/decomposeds3/option_test.go b/pkg/storage/fs/decomposeds3/option_test.go index 592b1af5f9c..109a6477dcb 100644 --- a/pkg/storage/fs/decomposeds3/option_test.go +++ b/pkg/storage/fs/decomposeds3/option_test.go @@ -40,6 +40,7 @@ var _ = Describe("Options", func() { "s3.bucket": "the-bucket", "s3.access_key": "foo", "s3.secret_key": "bar", + "s3.sse_c_key": "customer-key", } o = &decomposeds3.Options{} err := mapstructure.Decode(raw, o) @@ -51,6 +52,7 @@ var _ = Describe("Options", func() { Expect(o.S3Region).To(Equal("default")) Expect(o.S3AccessKey).To(Equal("foo")) Expect(o.S3SecretKey).To(Equal("bar")) + Expect(o.S3SSECKey).To(Equal("customer-key")) }) Describe("S3ConfigComplete", func() { diff --git a/pkg/storage/fs/s3ng/blobstore/blobstore.go b/pkg/storage/fs/s3ng/blobstore/blobstore.go index 628105a1833..1c8ef826272 100644 --- a/pkg/storage/fs/s3ng/blobstore/blobstore.go +++ b/pkg/storage/fs/s3ng/blobstore/blobstore.go @@ -29,14 +29,17 @@ import ( "github.com/minio/minio-go/v7" "github.com/minio/minio-go/v7/pkg/credentials" + "github.com/minio/minio-go/v7/pkg/encrypt" "github.com/opencloud-eu/reva/v2/pkg/storage/utils/decomposedfs/lookup" "github.com/opencloud-eu/reva/v2/pkg/storage/utils/decomposedfs/node" + "github.com/opencloud-eu/reva/v2/pkg/storage/utils/s3utils" "github.com/pkg/errors" ) // Blobstore provides an interface to an s3 compatible blobstore type Blobstore struct { - client *minio.Client + client *minio.Client + encryption encrypt.ServerSide defaultPutOptions Options @@ -53,12 +56,17 @@ type Options struct { } // New returns a new Blobstore -func New(endpoint, region, bucket, accessKey, secretKey string, defaultPutOptions Options) (*Blobstore, error) { +// ssecKey is an optional base64-encoded 32-byte customer encryption key. +func New(endpoint, region, bucket, accessKey, secretKey, ssecKey string, defaultPutOptions Options) (*Blobstore, error) { u, err := url.Parse(endpoint) if err != nil { return nil, errors.Wrap(err, "failed to parse s3 endpoint") } + encryption, err := s3utils.NewSSEC(ssecKey, u.Scheme == "https") + if err != nil { + return nil, err + } useSSL := u.Scheme != "http" client, err := minio.New(u.Host, &minio.Options{ Region: region, @@ -71,6 +79,7 @@ func New(endpoint, region, bucket, accessKey, secretKey string, defaultPutOption return &Blobstore{ client: client, + encryption: encryption, bucket: bucket, defaultPutOptions: defaultPutOptions, }, nil @@ -87,6 +96,7 @@ func (bs *Blobstore) Upload(node *node.Node, source string) error { _, err = bs.client.PutObject(context.Background(), bs.bucket, bs.Path(node), reader, node.Blobsize, minio.PutObjectOptions{ ContentType: "application/octet-stream", + ServerSideEncryption: bs.encryption, SendContentMd5: bs.defaultPutOptions.SendContentMd5, ConcurrentStreamParts: bs.defaultPutOptions.ConcurrentStreamParts, NumThreads: bs.defaultPutOptions.NumThreads, @@ -105,6 +115,7 @@ func (bs *Blobstore) Upload(node *node.Node, source string) error { func (bs *Blobstore) UploadFromReader(node *node.Node, r io.Reader, size int64) error { _, err := bs.client.PutObject(context.Background(), bs.bucket, bs.Path(node), r, size, minio.PutObjectOptions{ ContentType: "application/octet-stream", + ServerSideEncryption: bs.encryption, SendContentMd5: bs.defaultPutOptions.SendContentMd5, ConcurrentStreamParts: bs.defaultPutOptions.ConcurrentStreamParts, NumThreads: bs.defaultPutOptions.NumThreads, @@ -120,17 +131,19 @@ func (bs *Blobstore) UploadFromReader(node *node.Node, r io.Reader, size int64) // Download retrieves a blob from the blobstore for reading func (bs *Blobstore) Download(node *node.Node) (io.ReadCloser, error) { - reader, err := bs.client.GetObject(context.Background(), bs.bucket, bs.Path(node), minio.GetObjectOptions{}) + reader, err := bs.client.GetObject(context.Background(), bs.bucket, bs.Path(node), minio.GetObjectOptions{ServerSideEncryption: bs.encryption}) if err != nil { return nil, errors.Wrapf(err, "could not download object '%s' from bucket '%s'", bs.Path(node), bs.bucket) } stat, err := reader.Stat() if err != nil { + reader.Close() return nil, errors.Wrapf(err, "blob path: %s", bs.Path(node)) } if node.Blobsize != stat.Size { + reader.Close() return nil, fmt.Errorf("blob has unexpected size. %d bytes expected, got %d bytes", node.Blobsize, stat.Size) } diff --git a/pkg/storage/fs/s3ng/blobstore/blobstore_test.go b/pkg/storage/fs/s3ng/blobstore/blobstore_test.go new file mode 100644 index 00000000000..94f53a892aa --- /dev/null +++ b/pkg/storage/fs/s3ng/blobstore/blobstore_test.go @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: Apache-2.0 + +package blobstore + +import ( + "bytes" + "encoding/base64" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/minio/minio-go/v7" + "github.com/minio/minio-go/v7/pkg/credentials" + "github.com/opencloud-eu/reva/v2/pkg/storage/utils/decomposedfs/node" +) + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +func TestSSECHeaders(t *testing.T) { + source := filepath.Join(t.TempDir(), "blob") + if err := os.WriteFile(source, []byte("blob"), 0600); err != nil { + t.Fatal(err) + } + key := base64.StdEncoding.EncodeToString(bytes.Repeat([]byte{42}, 32)) + for _, tc := range []struct{ name, key string }{{"unset", ""}, {"SSE-C", key}} { + t.Run(tc.name, func(t *testing.T) { + opts := Options{DisableMultipart: true, SendContentMd5: true} + if tc.key != "" { + if _, err := New("http://s3.example.com", "us-east-1", "bucket", "ak", "sk", tc.key, opts); err == nil { + t.Fatal("accepted an HTTP endpoint with SSE-C enabled") + } + } + bs, err := New("https://s3.example.com", "us-east-1", "bucket", "ak", "sk", tc.key, opts) + if err != nil { + t.Fatal(err) + } + methods := map[string]int{} + // Intercept SDK requests to check Reva's option wiring without a + // network server or emulating multipart behavior owned by the SDK. + bs.client, err = minio.New("s3.example.com", &minio.Options{ + Region: "us-east-1", Secure: true, + Creds: credentials.NewStaticV4("ak", "sk", ""), + Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + methods[r.Method]++ + if r.Header.Get("X-Amz-Server-Side-Encryption-Customer-Key") != tc.key { + t.Errorf("%s request has the wrong customer key", r.Method) + } + algorithm := "" + if tc.key != "" { + algorithm = "AES256" + } + if r.Header.Get("X-Amz-Server-Side-Encryption-Customer-Algorithm") != algorithm { + t.Errorf("%s request has the wrong encryption algorithm", r.Method) + } + if r.Body != nil { + defer r.Body.Close() + if _, err := io.Copy(io.Discard, r.Body); err != nil { + return nil, err + } + } + headers := http.Header{} + headers.Set("Content-Length", "4") + headers.Set("ETag", `"etag"`) + headers.Set("Last-Modified", "Mon, 26 Oct 2026 12:00:00 GMT") + return &http.Response{StatusCode: http.StatusOK, Header: headers, + ContentLength: 4, Body: io.NopCloser(strings.NewReader("blob")), Request: r}, nil + }), + }) + if err != nil { + t.Fatal(err) + } + n := &node.Node{SpaceID: "space", BlobID: "blob", Blobsize: 4} + if err := bs.Upload(n, source); err != nil { + t.Fatalf("Upload: %v", err) + } + if err := bs.UploadFromReader(n, strings.NewReader("blob"), 4); err != nil { + t.Fatalf("UploadFromReader: %v", err) + } + reader, err := bs.Download(n) + if err != nil { + t.Fatalf("Download: %v", err) + } + defer reader.Close() + if _, err := io.Copy(io.Discard, reader); err != nil { + t.Fatal(err) + } + if methods[http.MethodPut] != 2 || methods[http.MethodHead] != 1 || methods[http.MethodGet] != 1 { + t.Fatalf("unexpected requests: %v", methods) + } + }) + } +} diff --git a/pkg/storage/fs/s3ng/option.go b/pkg/storage/fs/s3ng/option.go index 3f0f3201fd3..6b200548859 100644 --- a/pkg/storage/fs/s3ng/option.go +++ b/pkg/storage/fs/s3ng/option.go @@ -38,6 +38,9 @@ type Options struct { // Bucket of the s3 blobstore S3Bucket string `mapstructure:"s3.bucket"` + // Base64-encoded AES-256 customer key for server-side encryption (optional) + S3SSECKey string `mapstructure:"s3.sse_c_key"` + // Access key for the s3 blobstore S3AccessKey string `mapstructure:"s3.access_key"` diff --git a/pkg/storage/fs/s3ng/option_test.go b/pkg/storage/fs/s3ng/option_test.go index 3d0b421ac52..a7bdd1c1ce9 100644 --- a/pkg/storage/fs/s3ng/option_test.go +++ b/pkg/storage/fs/s3ng/option_test.go @@ -40,6 +40,7 @@ var _ = Describe("Options", func() { "s3.bucket": "the-bucket", "s3.access_key": "foo", "s3.secret_key": "bar", + "s3.sse_c_key": "customer-key", } o = &s3ng.Options{} err := mapstructure.Decode(raw, o) @@ -51,6 +52,7 @@ var _ = Describe("Options", func() { Expect(o.S3Region).To(Equal("default")) Expect(o.S3AccessKey).To(Equal("foo")) Expect(o.S3SecretKey).To(Equal("bar")) + Expect(o.S3SSECKey).To(Equal("customer-key")) }) Describe("S3ConfigComplete", func() { diff --git a/pkg/storage/fs/s3ng/s3ng.go b/pkg/storage/fs/s3ng/s3ng.go index 903e224bd0f..b61026ff0cb 100644 --- a/pkg/storage/fs/s3ng/s3ng.go +++ b/pkg/storage/fs/s3ng/s3ng.go @@ -54,7 +54,7 @@ func New(m map[string]interface{}, stream events.Stream, log *zerolog.Logger) (s PartSize: o.PartSize, } - bs, err := blobstore.New(o.S3Endpoint, o.S3Region, o.S3Bucket, o.S3AccessKey, o.S3SecretKey, defaultPutOptions) + bs, err := blobstore.New(o.S3Endpoint, o.S3Region, o.S3Bucket, o.S3AccessKey, o.S3SecretKey, o.S3SSECKey, defaultPutOptions) if err != nil { return nil, err } diff --git a/pkg/storage/fs/s3ng/s3ng_test.go b/pkg/storage/fs/s3ng/s3ng_test.go index 8ee883d7d55..28bd5870b89 100644 --- a/pkg/storage/fs/s3ng/s3ng_test.go +++ b/pkg/storage/fs/s3ng/s3ng_test.go @@ -64,6 +64,13 @@ var _ = Describe("S3ng", func() { Expect(err).To(MatchError("S3 configuration incomplete")) }) + It("rejects invalid SSE-C configuration before creating storage", func() { + options["s3.endpoint"] = "https://localhost:9000" + options["s3.sse_c_key"] = "invalid-secret!" + _, err := s3ng.New(options, nil, &zerolog.Logger{}) + Expect(err).To(MatchError("SSE-C key must be base64-encoded")) + }) + It("works with complete configuration", func() { _, err := s3ng.New(options, nil, &zerolog.Logger{}) Expect(err).ToNot(HaveOccurred()) diff --git a/pkg/storage/utils/s3utils/encryption.go b/pkg/storage/utils/s3utils/encryption.go new file mode 100644 index 00000000000..64e704f3a1b --- /dev/null +++ b/pkg/storage/utils/s3utils/encryption.go @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Package s3utils provides shared configuration helpers for S3 storage clients. +package s3utils + +import ( + "encoding/base64" + "fmt" + + "github.com/minio/minio-go/v7/pkg/encrypt" +) + +// NewSSEC validates a base64-encoded AES-256 customer key and creates the SDK +// encryption options. An empty key leaves server-side encryption unspecified. +// Customer keys must only be sent over HTTPS. +func NewSSEC(key string, secure bool) (encrypt.ServerSide, error) { + if key == "" { + return nil, nil + } + if !secure { + return nil, fmt.Errorf("SSE-C requires an HTTPS S3 endpoint") + } + decoded, err := base64.StdEncoding.Strict().DecodeString(key) + if err != nil { + return nil, fmt.Errorf("SSE-C key must be base64-encoded") + } + if len(decoded) != 32 { + return nil, fmt.Errorf("SSE-C key must decode to exactly 32 bytes") + } + return encrypt.NewSSEC(decoded) +} diff --git a/pkg/storage/utils/s3utils/encryption_test.go b/pkg/storage/utils/s3utils/encryption_test.go new file mode 100644 index 00000000000..690bd47547b --- /dev/null +++ b/pkg/storage/utils/s3utils/encryption_test.go @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: Apache-2.0 + +package s3utils_test + +import ( + "bytes" + "encoding/base64" + "net/http" + "strings" + "testing" + + "github.com/opencloud-eu/reva/v2/pkg/storage/utils/s3utils" +) + +func TestNewSSEC(t *testing.T) { + key := base64.StdEncoding.EncodeToString(bytes.Repeat([]byte{42}, 32)) + for _, tc := range []struct { + name, key string + secure, valid bool + }{ + {"unset HTTP", "", false, true}, + {"unset HTTPS", "", true, true}, + {"valid", key, true, true}, + {"HTTP", key, false, false}, + {"invalid base64", "secret!", true, false}, + {"short", base64.StdEncoding.EncodeToString(bytes.Repeat([]byte{1}, 31)), true, false}, + {"long", base64.StdEncoding.EncodeToString(bytes.Repeat([]byte{1}, 33)), true, false}, + } { + t.Run(tc.name, func(t *testing.T) { + enc, err := s3utils.NewSSEC(tc.key, tc.secure) + if (err == nil) != tc.valid { + t.Fatalf("unexpected validation result: %v", err) + } + if err != nil && strings.Contains(err.Error(), tc.key) { + t.Fatal("error exposes customer key") + } + if !tc.valid { + return + } + if tc.key == "" { + if enc != nil { + t.Fatal("empty key enabled encryption") + } + return + } + h := http.Header{} + enc.Marshal(h) + if h.Get("X-Amz-Server-Side-Encryption-Customer-Key") != key { + t.Fatal("encryption options contain the wrong customer key") + } + }) + } +} From 62a4396f7a5db1821987e33d35265d6aefec3048 Mon Sep 17 00:00:00 2001 From: Johannes Hoppenstedt <5921150+jhoppenstedt@users.noreply.github.com> Date: Wed, 28 Oct 2026 20:27:40 +0100 Subject: [PATCH 2/2] revert: s3ng changes --- pkg/storage/fs/s3ng/blobstore/blobstore.go | 19 +--- .../fs/s3ng/blobstore/blobstore_test.go | 97 ------------------- pkg/storage/fs/s3ng/option.go | 3 - pkg/storage/fs/s3ng/option_test.go | 2 - pkg/storage/fs/s3ng/s3ng.go | 2 +- pkg/storage/fs/s3ng/s3ng_test.go | 7 -- 6 files changed, 4 insertions(+), 126 deletions(-) delete mode 100644 pkg/storage/fs/s3ng/blobstore/blobstore_test.go diff --git a/pkg/storage/fs/s3ng/blobstore/blobstore.go b/pkg/storage/fs/s3ng/blobstore/blobstore.go index 1c8ef826272..628105a1833 100644 --- a/pkg/storage/fs/s3ng/blobstore/blobstore.go +++ b/pkg/storage/fs/s3ng/blobstore/blobstore.go @@ -29,17 +29,14 @@ import ( "github.com/minio/minio-go/v7" "github.com/minio/minio-go/v7/pkg/credentials" - "github.com/minio/minio-go/v7/pkg/encrypt" "github.com/opencloud-eu/reva/v2/pkg/storage/utils/decomposedfs/lookup" "github.com/opencloud-eu/reva/v2/pkg/storage/utils/decomposedfs/node" - "github.com/opencloud-eu/reva/v2/pkg/storage/utils/s3utils" "github.com/pkg/errors" ) // Blobstore provides an interface to an s3 compatible blobstore type Blobstore struct { - client *minio.Client - encryption encrypt.ServerSide + client *minio.Client defaultPutOptions Options @@ -56,17 +53,12 @@ type Options struct { } // New returns a new Blobstore -// ssecKey is an optional base64-encoded 32-byte customer encryption key. -func New(endpoint, region, bucket, accessKey, secretKey, ssecKey string, defaultPutOptions Options) (*Blobstore, error) { +func New(endpoint, region, bucket, accessKey, secretKey string, defaultPutOptions Options) (*Blobstore, error) { u, err := url.Parse(endpoint) if err != nil { return nil, errors.Wrap(err, "failed to parse s3 endpoint") } - encryption, err := s3utils.NewSSEC(ssecKey, u.Scheme == "https") - if err != nil { - return nil, err - } useSSL := u.Scheme != "http" client, err := minio.New(u.Host, &minio.Options{ Region: region, @@ -79,7 +71,6 @@ func New(endpoint, region, bucket, accessKey, secretKey, ssecKey string, default return &Blobstore{ client: client, - encryption: encryption, bucket: bucket, defaultPutOptions: defaultPutOptions, }, nil @@ -96,7 +87,6 @@ func (bs *Blobstore) Upload(node *node.Node, source string) error { _, err = bs.client.PutObject(context.Background(), bs.bucket, bs.Path(node), reader, node.Blobsize, minio.PutObjectOptions{ ContentType: "application/octet-stream", - ServerSideEncryption: bs.encryption, SendContentMd5: bs.defaultPutOptions.SendContentMd5, ConcurrentStreamParts: bs.defaultPutOptions.ConcurrentStreamParts, NumThreads: bs.defaultPutOptions.NumThreads, @@ -115,7 +105,6 @@ func (bs *Blobstore) Upload(node *node.Node, source string) error { func (bs *Blobstore) UploadFromReader(node *node.Node, r io.Reader, size int64) error { _, err := bs.client.PutObject(context.Background(), bs.bucket, bs.Path(node), r, size, minio.PutObjectOptions{ ContentType: "application/octet-stream", - ServerSideEncryption: bs.encryption, SendContentMd5: bs.defaultPutOptions.SendContentMd5, ConcurrentStreamParts: bs.defaultPutOptions.ConcurrentStreamParts, NumThreads: bs.defaultPutOptions.NumThreads, @@ -131,19 +120,17 @@ func (bs *Blobstore) UploadFromReader(node *node.Node, r io.Reader, size int64) // Download retrieves a blob from the blobstore for reading func (bs *Blobstore) Download(node *node.Node) (io.ReadCloser, error) { - reader, err := bs.client.GetObject(context.Background(), bs.bucket, bs.Path(node), minio.GetObjectOptions{ServerSideEncryption: bs.encryption}) + reader, err := bs.client.GetObject(context.Background(), bs.bucket, bs.Path(node), minio.GetObjectOptions{}) if err != nil { return nil, errors.Wrapf(err, "could not download object '%s' from bucket '%s'", bs.Path(node), bs.bucket) } stat, err := reader.Stat() if err != nil { - reader.Close() return nil, errors.Wrapf(err, "blob path: %s", bs.Path(node)) } if node.Blobsize != stat.Size { - reader.Close() return nil, fmt.Errorf("blob has unexpected size. %d bytes expected, got %d bytes", node.Blobsize, stat.Size) } diff --git a/pkg/storage/fs/s3ng/blobstore/blobstore_test.go b/pkg/storage/fs/s3ng/blobstore/blobstore_test.go deleted file mode 100644 index 94f53a892aa..00000000000 --- a/pkg/storage/fs/s3ng/blobstore/blobstore_test.go +++ /dev/null @@ -1,97 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 - -package blobstore - -import ( - "bytes" - "encoding/base64" - "io" - "net/http" - "os" - "path/filepath" - "strings" - "testing" - - "github.com/minio/minio-go/v7" - "github.com/minio/minio-go/v7/pkg/credentials" - "github.com/opencloud-eu/reva/v2/pkg/storage/utils/decomposedfs/node" -) - -type roundTripFunc func(*http.Request) (*http.Response, error) - -func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } - -func TestSSECHeaders(t *testing.T) { - source := filepath.Join(t.TempDir(), "blob") - if err := os.WriteFile(source, []byte("blob"), 0600); err != nil { - t.Fatal(err) - } - key := base64.StdEncoding.EncodeToString(bytes.Repeat([]byte{42}, 32)) - for _, tc := range []struct{ name, key string }{{"unset", ""}, {"SSE-C", key}} { - t.Run(tc.name, func(t *testing.T) { - opts := Options{DisableMultipart: true, SendContentMd5: true} - if tc.key != "" { - if _, err := New("http://s3.example.com", "us-east-1", "bucket", "ak", "sk", tc.key, opts); err == nil { - t.Fatal("accepted an HTTP endpoint with SSE-C enabled") - } - } - bs, err := New("https://s3.example.com", "us-east-1", "bucket", "ak", "sk", tc.key, opts) - if err != nil { - t.Fatal(err) - } - methods := map[string]int{} - // Intercept SDK requests to check Reva's option wiring without a - // network server or emulating multipart behavior owned by the SDK. - bs.client, err = minio.New("s3.example.com", &minio.Options{ - Region: "us-east-1", Secure: true, - Creds: credentials.NewStaticV4("ak", "sk", ""), - Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { - methods[r.Method]++ - if r.Header.Get("X-Amz-Server-Side-Encryption-Customer-Key") != tc.key { - t.Errorf("%s request has the wrong customer key", r.Method) - } - algorithm := "" - if tc.key != "" { - algorithm = "AES256" - } - if r.Header.Get("X-Amz-Server-Side-Encryption-Customer-Algorithm") != algorithm { - t.Errorf("%s request has the wrong encryption algorithm", r.Method) - } - if r.Body != nil { - defer r.Body.Close() - if _, err := io.Copy(io.Discard, r.Body); err != nil { - return nil, err - } - } - headers := http.Header{} - headers.Set("Content-Length", "4") - headers.Set("ETag", `"etag"`) - headers.Set("Last-Modified", "Mon, 26 Oct 2026 12:00:00 GMT") - return &http.Response{StatusCode: http.StatusOK, Header: headers, - ContentLength: 4, Body: io.NopCloser(strings.NewReader("blob")), Request: r}, nil - }), - }) - if err != nil { - t.Fatal(err) - } - n := &node.Node{SpaceID: "space", BlobID: "blob", Blobsize: 4} - if err := bs.Upload(n, source); err != nil { - t.Fatalf("Upload: %v", err) - } - if err := bs.UploadFromReader(n, strings.NewReader("blob"), 4); err != nil { - t.Fatalf("UploadFromReader: %v", err) - } - reader, err := bs.Download(n) - if err != nil { - t.Fatalf("Download: %v", err) - } - defer reader.Close() - if _, err := io.Copy(io.Discard, reader); err != nil { - t.Fatal(err) - } - if methods[http.MethodPut] != 2 || methods[http.MethodHead] != 1 || methods[http.MethodGet] != 1 { - t.Fatalf("unexpected requests: %v", methods) - } - }) - } -} diff --git a/pkg/storage/fs/s3ng/option.go b/pkg/storage/fs/s3ng/option.go index 6b200548859..3f0f3201fd3 100644 --- a/pkg/storage/fs/s3ng/option.go +++ b/pkg/storage/fs/s3ng/option.go @@ -38,9 +38,6 @@ type Options struct { // Bucket of the s3 blobstore S3Bucket string `mapstructure:"s3.bucket"` - // Base64-encoded AES-256 customer key for server-side encryption (optional) - S3SSECKey string `mapstructure:"s3.sse_c_key"` - // Access key for the s3 blobstore S3AccessKey string `mapstructure:"s3.access_key"` diff --git a/pkg/storage/fs/s3ng/option_test.go b/pkg/storage/fs/s3ng/option_test.go index a7bdd1c1ce9..3d0b421ac52 100644 --- a/pkg/storage/fs/s3ng/option_test.go +++ b/pkg/storage/fs/s3ng/option_test.go @@ -40,7 +40,6 @@ var _ = Describe("Options", func() { "s3.bucket": "the-bucket", "s3.access_key": "foo", "s3.secret_key": "bar", - "s3.sse_c_key": "customer-key", } o = &s3ng.Options{} err := mapstructure.Decode(raw, o) @@ -52,7 +51,6 @@ var _ = Describe("Options", func() { Expect(o.S3Region).To(Equal("default")) Expect(o.S3AccessKey).To(Equal("foo")) Expect(o.S3SecretKey).To(Equal("bar")) - Expect(o.S3SSECKey).To(Equal("customer-key")) }) Describe("S3ConfigComplete", func() { diff --git a/pkg/storage/fs/s3ng/s3ng.go b/pkg/storage/fs/s3ng/s3ng.go index b61026ff0cb..903e224bd0f 100644 --- a/pkg/storage/fs/s3ng/s3ng.go +++ b/pkg/storage/fs/s3ng/s3ng.go @@ -54,7 +54,7 @@ func New(m map[string]interface{}, stream events.Stream, log *zerolog.Logger) (s PartSize: o.PartSize, } - bs, err := blobstore.New(o.S3Endpoint, o.S3Region, o.S3Bucket, o.S3AccessKey, o.S3SecretKey, o.S3SSECKey, defaultPutOptions) + bs, err := blobstore.New(o.S3Endpoint, o.S3Region, o.S3Bucket, o.S3AccessKey, o.S3SecretKey, defaultPutOptions) if err != nil { return nil, err } diff --git a/pkg/storage/fs/s3ng/s3ng_test.go b/pkg/storage/fs/s3ng/s3ng_test.go index 28bd5870b89..8ee883d7d55 100644 --- a/pkg/storage/fs/s3ng/s3ng_test.go +++ b/pkg/storage/fs/s3ng/s3ng_test.go @@ -64,13 +64,6 @@ var _ = Describe("S3ng", func() { Expect(err).To(MatchError("S3 configuration incomplete")) }) - It("rejects invalid SSE-C configuration before creating storage", func() { - options["s3.endpoint"] = "https://localhost:9000" - options["s3.sse_c_key"] = "invalid-secret!" - _, err := s3ng.New(options, nil, &zerolog.Logger{}) - Expect(err).To(MatchError("SSE-C key must be base64-encoded")) - }) - It("works with complete configuration", func() { _, err := s3ng.New(options, nil, &zerolog.Logger{}) Expect(err).ToNot(HaveOccurred())