From b87fae0049265cf2f4a6c24cdb8c91be038609e6 Mon Sep 17 00:00:00 2001 From: Krishnan Parthasarathi Date: Wed, 10 Feb 2021 08:52:50 -0800 Subject: [PATCH] Simplify PutObjReader for plain-text reader usage (#11470) This change moves away from a unified constructor for plaintext and encrypted usage. NewPutObjReader is simplified for the plain-text reader use. For encrypted reader use, WithEncryption should be called on an initialized PutObjReader. Plaintext: func NewPutObjReader(rawReader *hash.Reader) *PutObjReader The hash.Reader is used to provide payload size and md5sum to the downstream consumers. This is different from the previous version in that there is no need to pass nil values for unused parameters. Encrypted: func WithEncryption(encReader *hash.Reader, key *crypto.ObjectKey) (*PutObjReader, error) This method sets up encrypted reader along with the key to seal the md5sum produced by the plain-text reader (already setup when NewPutObjReader was called). Usage: ``` pReader := NewPutObjReader(rawReader) // ... other object handler code goes here // Prepare the encrypted hashed reader pReader, err = pReader.WithEncryption(encReader, objEncKey) ``` --- cmd/bucket-handlers.go | 8 +++++-- cmd/bucket-lifecycle.go | 2 +- cmd/config-common.go | 2 +- cmd/data-crawler.go | 2 +- cmd/data-usage-cache.go | 2 +- cmd/data-usage.go | 2 +- cmd/disk-cache.go | 2 +- cmd/erasure-multipart.go | 2 +- cmd/erasure-server-pool.go | 2 +- cmd/erasure-sets.go | 2 +- cmd/gateway/s3/gateway-s3-metadata.go | 2 +- cmd/metacache-bucket.go | 2 +- cmd/metacache-set.go | 2 +- cmd/object-api-utils.go | 27 ++++++++++++++-------- cmd/object-handlers.go | 33 +++++++++++++++++++-------- cmd/test-utils_test.go | 2 +- cmd/web-handlers.go | 9 +++++--- 17 files changed, 66 insertions(+), 37 deletions(-) diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index fb021c6b0..770420d7f 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -919,7 +919,7 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h return } rawReader := hashReader - pReader := NewPutObjReader(rawReader, nil, nil) + pReader := NewPutObjReader(rawReader) var objectEncryptionKey crypto.ObjectKey // Check if bucket encryption is enabled @@ -964,7 +964,11 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) return } - pReader = NewPutObjReader(rawReader, hashReader, &objectEncryptionKey) + pReader, err = pReader.WithEncryption(hashReader, &objectEncryptionKey) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } } } diff --git a/cmd/bucket-lifecycle.go b/cmd/bucket-lifecycle.go index a26ccf57c..788fe8235 100644 --- a/cmd/bucket-lifecycle.go +++ b/cmd/bucket-lifecycle.go @@ -696,7 +696,7 @@ func restoreTransitionedObject(ctx context.Context, bucket, object string, objAP if err != nil { return err } - pReader := NewPutObjReader(hashReader, nil, nil) + pReader := NewPutObjReader(hashReader) opts := putRestoreOpts(bucket, object, rreq, objInfo) opts.UserDefined[xhttp.AmzRestore] = fmt.Sprintf("ongoing-request=%t, expiry-date=%s", false, restoreExpiry.Format(http.TimeFormat)) if _, err := objAPI.PutObject(ctx, bucket, object, pReader, opts); err != nil { diff --git a/cmd/config-common.go b/cmd/config-common.go index 9a4e82fc3..e8fa13f1d 100644 --- a/cmd/config-common.go +++ b/cmd/config-common.go @@ -69,7 +69,7 @@ func saveConfig(ctx context.Context, objAPI ObjectLayer, configFile string, data return err } - _, err = objAPI.PutObject(ctx, minioMetaBucket, configFile, NewPutObjReader(hashReader, nil, nil), ObjectOptions{}) + _, err = objAPI.PutObject(ctx, minioMetaBucket, configFile, NewPutObjReader(hashReader), ObjectOptions{}) return err } diff --git a/cmd/data-crawler.go b/cmd/data-crawler.go index da7ed4ef0..b608e4b30 100644 --- a/cmd/data-crawler.go +++ b/cmd/data-crawler.go @@ -134,7 +134,7 @@ func runDataCrawler(ctx context.Context, objAPI ObjectLayer) { continue } - _, err = objAPI.PutObject(ctx, dataUsageBucket, dataUsageBloomName, NewPutObjReader(r, nil, nil), ObjectOptions{}) + _, err = objAPI.PutObject(ctx, dataUsageBucket, dataUsageBloomName, NewPutObjReader(r), ObjectOptions{}) if !isErrBucketNotFound(err) { logger.LogIf(ctx, err) } diff --git a/cmd/data-usage-cache.go b/cmd/data-usage-cache.go index c6d7cf467..b8c11ca83 100644 --- a/cmd/data-usage-cache.go +++ b/cmd/data-usage-cache.go @@ -521,7 +521,7 @@ func (d *dataUsageCache) save(ctx context.Context, store objectIO, name string) _, err = store.PutObject(ctx, dataUsageBucket, name, - NewPutObjReader(r, nil, nil), + NewPutObjReader(r), ObjectOptions{}) if isErrBucketNotFound(err) { return nil diff --git a/cmd/data-usage.go b/cmd/data-usage.go index 5da9d8394..d4464829f 100644 --- a/cmd/data-usage.go +++ b/cmd/data-usage.go @@ -52,7 +52,7 @@ func storeDataUsageInBackend(ctx context.Context, objAPI ObjectLayer, dui <-chan logger.LogIf(ctx, err) continue } - _, err = objAPI.PutObject(ctx, dataUsageBucket, dataUsageObjName, NewPutObjReader(r, nil, nil), ObjectOptions{}) + _, err = objAPI.PutObject(ctx, dataUsageBucket, dataUsageObjName, NewPutObjReader(r), ObjectOptions{}) if !isErrBucketNotFound(err) { logger.LogIf(ctx, err) } diff --git a/cmd/disk-cache.go b/cmd/disk-cache.go index bb6f59a0b..385583c3e 100644 --- a/cmd/disk-cache.go +++ b/cmd/disk-cache.go @@ -711,7 +711,7 @@ func (c *cacheObjects) uploadObject(ctx context.Context, oi ObjectInfo) { var opts ObjectOptions opts.UserDefined = make(map[string]string) opts.UserDefined[xhttp.ContentMD5] = oi.UserDefined["content-md5"] - objInfo, err := c.InnerPutObjectFn(ctx, oi.Bucket, oi.Name, NewPutObjReader(hashReader, nil, nil), opts) + objInfo, err := c.InnerPutObjectFn(ctx, oi.Bucket, oi.Name, NewPutObjReader(hashReader), opts) wbCommitStatus := CommitComplete if err != nil { wbCommitStatus = CommitFailed diff --git a/cmd/erasure-multipart.go b/cmd/erasure-multipart.go index 30fe8f3e6..9856f8e2b 100644 --- a/cmd/erasure-multipart.go +++ b/cmd/erasure-multipart.go @@ -325,7 +325,7 @@ func (er erasureObjects) NewMultipartUpload(ctx context.Context, bucket, object // // Implements S3 compatible Upload Part Copy API. func (er erasureObjects) CopyObjectPart(ctx context.Context, srcBucket, srcObject, dstBucket, dstObject, uploadID string, partID int, startOffset int64, length int64, srcInfo ObjectInfo, srcOpts, dstOpts ObjectOptions) (pi PartInfo, e error) { - partInfo, err := er.PutObjectPart(ctx, dstBucket, dstObject, uploadID, partID, NewPutObjReader(srcInfo.Reader, nil, nil), dstOpts) + partInfo, err := er.PutObjectPart(ctx, dstBucket, dstObject, uploadID, partID, NewPutObjReader(srcInfo.Reader), dstOpts) if err != nil { return pi, toObjectErr(err, dstBucket, dstObject) } diff --git a/cmd/erasure-server-pool.go b/cmd/erasure-server-pool.go index cf851f0a6..aedbf8a4c 100644 --- a/cmd/erasure-server-pool.go +++ b/cmd/erasure-server-pool.go @@ -875,7 +875,7 @@ func (z *erasureServerPools) CopyObjectPart(ctx context.Context, srcBucket, srcO } return z.PutObjectPart(ctx, destBucket, destObject, uploadID, partID, - NewPutObjReader(srcInfo.Reader, nil, nil), dstOpts) + NewPutObjReader(srcInfo.Reader), dstOpts) } // PutObjectPart - writes part of an object to hashedSet based on the object name. diff --git a/cmd/erasure-sets.go b/cmd/erasure-sets.go index 05db578e1..57acdaa8f 100644 --- a/cmd/erasure-sets.go +++ b/cmd/erasure-sets.go @@ -1076,7 +1076,7 @@ func (s *erasureSets) CopyObjectPart(ctx context.Context, srcBucket, srcObject, startOffset int64, length int64, srcInfo ObjectInfo, srcOpts, dstOpts ObjectOptions) (partInfo PartInfo, err error) { destSet := s.getHashedSet(destObject) auditObjectErasureSet(ctx, destObject, destSet, s.poolNumber) - return destSet.PutObjectPart(ctx, destBucket, destObject, uploadID, partID, NewPutObjReader(srcInfo.Reader, nil, nil), dstOpts) + return destSet.PutObjectPart(ctx, destBucket, destObject, uploadID, partID, NewPutObjReader(srcInfo.Reader), dstOpts) } // PutObjectPart - writes part of an object to hashedSet based on the object name. diff --git a/cmd/gateway/s3/gateway-s3-metadata.go b/cmd/gateway/s3/gateway-s3-metadata.go index 0a534393b..61e547a45 100644 --- a/cmd/gateway/s3/gateway-s3-metadata.go +++ b/cmd/gateway/s3/gateway-s3-metadata.go @@ -172,5 +172,5 @@ func getGWMetadata(ctx context.Context, bucket, prefix string, gwMeta gwMetaV1) if err != nil { return nil, err } - return minio.NewPutObjReader(hashReader, nil, nil), nil + return minio.NewPutObjReader(hashReader), nil } diff --git a/cmd/metacache-bucket.go b/cmd/metacache-bucket.go index e958d7b9e..779c74ac3 100644 --- a/cmd/metacache-bucket.go +++ b/cmd/metacache-bucket.go @@ -180,7 +180,7 @@ func (b *bucketMetacache) save(ctx context.Context) error { if err != nil { return err } - _, err = objAPI.PutObject(ctx, minioMetaBucket, pathJoin("buckets", b.bucket, ".metacache", "index.s2"), NewPutObjReader(hr, nil, nil), ObjectOptions{}) + _, err = objAPI.PutObject(ctx, minioMetaBucket, pathJoin("buckets", b.bucket, ".metacache", "index.s2"), NewPutObjReader(hr), ObjectOptions{}) logger.LogIf(ctx, err) return err } diff --git a/cmd/metacache-set.go b/cmd/metacache-set.go index eb1dfa137..5e5e7ec7f 100644 --- a/cmd/metacache-set.go +++ b/cmd/metacache-set.go @@ -675,7 +675,7 @@ func (er *erasureObjects) listPath(ctx context.Context, o listPathOptions) (entr r, err := hash.NewReader(bytes.NewReader(b.data), int64(len(b.data)), "", "", int64(len(b.data)), false) logger.LogIf(ctx, err) custom := b.headerKV() - _, err = er.putObject(ctx, minioMetaBucket, o.objectPath(b.n), NewPutObjReader(r, nil, nil), ObjectOptions{ + _, err = er.putObject(ctx, minioMetaBucket, o.objectPath(b.n), NewPutObjReader(r), ObjectOptions{ UserDefined: custom, NoLock: true, // No need to hold namespace lock, each prefix caches uniquely. ParentIsObject: nil, diff --git a/cmd/object-api-utils.go b/cmd/object-api-utils.go index 0a0c10a8d..7e2760bab 100644 --- a/cmd/object-api-utils.go +++ b/cmd/object-api-utils.go @@ -20,6 +20,7 @@ import ( "bytes" "context" "encoding/hex" + "errors" "fmt" "io" "math/rand" @@ -865,17 +866,23 @@ func (p *PutObjReader) MD5CurrentHexString() string { return hex.EncodeToString(md5sumCurr) } -// NewPutObjReader returns a new PutObjReader and holds -// reference to underlying data stream from client and the encrypted -// data reader -func NewPutObjReader(rawReader *hash.Reader, encReader *hash.Reader, key *crypto.ObjectKey) *PutObjReader { - p := PutObjReader{Reader: rawReader, rawReader: rawReader} - - if key != nil && encReader != nil { - p.sealMD5Fn = sealETagFn(*key) - p.Reader = encReader +// WithEncryption sets up encrypted reader and the sealing for content md5sum +// using objEncKey. Unsealed md5sum is computed from the rawReader setup when +// NewPutObjReader was called. It returns an error if called on an uninitialized +// PutObjReader. +func (p *PutObjReader) WithEncryption(encReader *hash.Reader, objEncKey *crypto.ObjectKey) (*PutObjReader, error) { + if p.Reader == nil { + return nil, errors.New("put-object reader uninitialized") } - return &p + p.Reader = encReader + p.sealMD5Fn = sealETagFn(*objEncKey) + return p, nil +} + +// NewPutObjReader returns a new PutObjReader. It uses given hash.Reader's +// MD5Current method to construct md5sum when requested downstream. +func NewPutObjReader(rawReader *hash.Reader) *PutObjReader { + return &PutObjReader{Reader: rawReader, rawReader: rawReader} } func sealETag(encKey crypto.ObjectKey, md5CurrSum []byte) []byte { diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 86b521e85..a1a71ea6d 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -1032,8 +1032,7 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re return } - rawReader := srcInfo.Reader - pReader := NewPutObjReader(srcInfo.Reader, nil, nil) + pReader := NewPutObjReader(srcInfo.Reader) // Check if either the source is encrypted or the destination will be encrypted. _, objectEncryption := crypto.IsRequested(r.Header) @@ -1145,7 +1144,11 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re } if isTargetEncrypted { - pReader = NewPutObjReader(rawReader, srcInfo.Reader, &objEncKey) + pReader, err = pReader.WithEncryption(srcInfo.Reader, &objEncKey) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } } } } @@ -1495,7 +1498,7 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req } rawReader := hashReader - pReader := NewPutObjReader(rawReader, nil, nil) + pReader := NewPutObjReader(rawReader) // get gateway encryption options var opts ObjectOptions @@ -1564,7 +1567,11 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) return } - pReader = NewPutObjReader(rawReader, hashReader, &objectEncryptionKey) + pReader, err = pReader.WithEncryption(hashReader, &objectEncryptionKey) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } } } @@ -2002,7 +2009,7 @@ func (api objectAPIHandlers) CopyObjectPartHandler(w http.ResponseWriter, r *htt } rawReader := srcInfo.Reader - pReader := NewPutObjReader(rawReader, nil, nil) + pReader := NewPutObjReader(rawReader) _, isEncrypted := crypto.IsEncrypted(mi.UserDefined) var objectEncryptionKey crypto.ObjectKey @@ -2048,7 +2055,11 @@ func (api objectAPIHandlers) CopyObjectPartHandler(w http.ResponseWriter, r *htt writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) return } - pReader = NewPutObjReader(rawReader, srcInfo.Reader, &objectEncryptionKey) + pReader, err = pReader.WithEncryption(srcInfo.Reader, &objectEncryptionKey) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } } srcInfo.PutObjReader = pReader @@ -2242,7 +2253,7 @@ func (api objectAPIHandlers) PutObjectPartHandler(w http.ResponseWriter, r *http return } rawReader := hashReader - pReader := NewPutObjReader(rawReader, nil, nil) + pReader := NewPutObjReader(rawReader) _, isEncrypted := crypto.IsEncrypted(mi.UserDefined) var objectEncryptionKey crypto.ObjectKey @@ -2298,7 +2309,11 @@ func (api objectAPIHandlers) PutObjectPartHandler(w http.ResponseWriter, r *http writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) return } - pReader = NewPutObjReader(rawReader, hashReader, &objectEncryptionKey) + pReader, err = pReader.WithEncryption(hashReader, &objectEncryptionKey) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } } putObjectPart := objectAPI.PutObjectPart diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index 022a8e9b7..623f80956 100644 --- a/cmd/test-utils_test.go +++ b/cmd/test-utils_test.go @@ -164,7 +164,7 @@ func mustGetPutObjReader(t TestErrHandler, data io.Reader, size int64, md5hex, s if err != nil { t.Fatal(err) } - return NewPutObjReader(hr, nil, nil) + return NewPutObjReader(hr) } // calculateSignedChunkLength - calculates the length of the overall stream (data + metadata) diff --git a/cmd/web-handlers.go b/cmd/web-handlers.go index eab508c8e..a3c5eae34 100644 --- a/cmd/web-handlers.go +++ b/cmd/web-handlers.go @@ -1234,7 +1234,7 @@ func (web *webAPIHandlers) Upload(w http.ResponseWriter, r *http.Request) { if mustReplicate { metadata[xhttp.AmzBucketReplicationStatus] = string(replication.Pending) } - pReader = NewPutObjReader(hashReader, nil, nil) + pReader = NewPutObjReader(hashReader) // get gateway encryption options opts, err := putOpts(ctx, r, bucket, object, metadata) if err != nil { @@ -1244,7 +1244,6 @@ func (web *webAPIHandlers) Upload(w http.ResponseWriter, r *http.Request) { if objectAPI.IsEncryptionSupported() { if _, ok := crypto.IsRequested(r.Header); ok && !HasSuffix(object, SlashSeparator) { // handle SSE requests - rawReader := hashReader var objectEncryptionKey crypto.ObjectKey reader, objectEncryptionKey, err = EncryptRequest(hashReader, r, bucket, object, metadata) if err != nil { @@ -1258,7 +1257,11 @@ func (web *webAPIHandlers) Upload(w http.ResponseWriter, r *http.Request) { writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) return } - pReader = NewPutObjReader(rawReader, hashReader, &objectEncryptionKey) + pReader, err = pReader.WithEncryption(hashReader, &objectEncryptionKey) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } } }