From 838d4dafbd207ae50c90e155440ea2383846a290 Mon Sep 17 00:00:00 2001 From: Andreas Auernhammer Date: Tue, 2 Feb 2021 08:02:08 +0100 Subject: [PATCH] gateway: don't use encrypted ETags for If-Match (#11400) This commit fixes a bug in the S3 gateway that causes GET requests to fail when the object is encrypted by the gateway itself. The gateway was not able to GET the object since it always specified a `If-Match` pre-condition checking that the object ETag matches an expected ETag - even for encrypted ETags. The problem is that an encrypted ETag will never match the ETag computed by the backend causing the `If-Match` pre-condition to fail. This commit fixes this by not sending an `If-Match` header when the ETag is encrypted. This is acceptable because: 1. A gateway-encrypted object consists of two objects at the backend and there is no way to provide a concurrency-safe implementation of two consecutive S3 GETs in the deployment model of the S3 gateway. Ref: S3 gateways are self-contained and isolated - and there may be multiple instances at the same time (no lock across instances). 2. Even if the data object changes (concurrent PUT) while gateway A has download the metadata object (but not issued the GET to the data object => data race) then we don't return invalid data to the client since the decryption (of the currently uploaded data) will fail - given the metadata of the previous object. --- cmd/gateway/s3/gateway-s3-sse.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/cmd/gateway/s3/gateway-s3-sse.go b/cmd/gateway/s3/gateway-s3-sse.go index 5e35172bc..096610c66 100644 --- a/cmd/gateway/s3/gateway-s3-sse.go +++ b/cmd/gateway/s3/gateway-s3-sse.go @@ -316,7 +316,7 @@ func (l *s3EncObjects) GetObjectNInfo(ctx context.Context, bucket, object string if err != nil { return l.s3Objects.GetObjectNInfo(ctx, bucket, object, rs, h, lockType, opts) } - fn, off, length, err := minio.NewGetObjectReader(rs, objInfo, o) + fn, off, length, err := minio.NewGetObjectReader(rs, objInfo, opts) if err != nil { return nil, minio.ErrorRespToObjectError(err, bucket, object) } @@ -325,7 +325,20 @@ func (l *s3EncObjects) GetObjectNInfo(ctx context.Context, bucket, object string } pr, pw := io.Pipe() go func() { - err := l.getObject(ctx, bucket, object, off, length, pw, objInfo.ETag, opts) + // Do not set an `If-Match` header for the ETag when + // the ETag is encrypted. The ETag at the backend never + // matches an encrypted ETag and there is in any case + // no way to make two consecutive S3 calls safe for concurrent + // access. + // However, the encrypted object changes concurrently then the + // gateway will not be able to decrypt it since the key (obtained + // from dare.meta) will not work for any new created object. Therefore, + // we will in any case not return invalid data to the client. + etag := objInfo.ETag + if len(etag) > 32 && strings.Count(etag, "-") == 0 { + etag = "" + } + err := l.getObject(ctx, bucket, object, off, length, pw, etag, opts) pw.CloseWithError(err) }()