avoid unnecessary KMS requests during single-part PUT (#9220)

This commit fixes a performance issue caused
by too many calls to the external KMS - i.e.
for single-part PUT requests.

In general, the issue is caused by a sub-optimal
code structure. In particular, when the server
encrypts an object it requests a new data encryption
key from the KMS. With this key it does some key
derivation and encrypts the object content and
ETag.

However, to behave S3-compatible the MinIO server
has to return the plaintext ETag to the client
in case SSE-S3.
Therefore, the server code used to decrypt the
(previously encrypted) ETag again by requesting
the data encryption key (KMS decrypt API) from
the KMS.

This leads to 2 KMS API calls (1 generate key and
1 decrypt key) per PUT operation - while only
one KMS call is necessary.

This commit fixes this by fetching a data key only
once from the KMS and keeping the derived object
encryption key around (for the lifetime of the request).

This leads to a significant performance improvement
w.r.t. to PUT workloads:
```
Operation: PUT
Operations: 161 -> 239
Duration: 28s -> 29s
* Average: +47.56% (+25.8 MiB/s) throughput, +47.56% (+2.6) obj/s
* Fastest: +55.49% (+34.5 MiB/s) throughput, +55.49% (+3.5) obj/s
* 50% Median: +58.24% (+32.8 MiB/s) throughput, +58.24% (+3.3) obj/s
* Slowest: +1.83% (+0.6 MiB/s) throughput, +1.83% (+0.1) obj/s
```
This commit is contained in:
Andreas Auernhammer 2020-04-10 02:01:45 +02:00 committed by GitHub
parent cea078a593
commit db41953618
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 177 additions and 71 deletions

View file

@ -755,7 +755,7 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h
}
rawReader := hashReader
pReader := NewPutObjReader(rawReader, nil, nil)
var objectEncryptionKey []byte
var objectEncryptionKey crypto.ObjectKey
// Check if bucket encryption is enabled
_, encEnabled := globalBucketSSEConfigSys.Get(bucket)
@ -797,7 +797,7 @@ 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 = NewPutObjReader(rawReader, hashReader, &objectEncryptionKey)
}
}

View file

@ -29,6 +29,7 @@ import (
"net/http"
"path"
"strconv"
"strings"
"github.com/minio/minio-go/v6/pkg/encrypt"
"github.com/minio/minio/cmd/crypto"
@ -168,39 +169,39 @@ func rotateKey(oldKey []byte, newKey []byte, bucket, object string, metadata map
}
}
func newEncryptMetadata(key []byte, bucket, object string, metadata map[string]string, sseS3 bool) ([]byte, error) {
func newEncryptMetadata(key []byte, bucket, object string, metadata map[string]string, sseS3 bool) (crypto.ObjectKey, error) {
var sealedKey crypto.SealedKey
if sseS3 {
if GlobalKMS == nil {
return nil, errKMSNotConfigured
return crypto.ObjectKey{}, errKMSNotConfigured
}
key, encKey, err := GlobalKMS.GenerateKey(GlobalKMS.KeyID(), crypto.Context{bucket: path.Join(bucket, object)})
if err != nil {
return nil, err
return crypto.ObjectKey{}, err
}
objectKey := crypto.GenerateKey(key, rand.Reader)
sealedKey = objectKey.Seal(key, crypto.GenerateIV(rand.Reader), crypto.S3.String(), bucket, object)
crypto.S3.CreateMetadata(metadata, GlobalKMS.KeyID(), encKey, sealedKey)
return objectKey[:], nil
return objectKey, nil
}
var extKey [32]byte
copy(extKey[:], key)
objectKey := crypto.GenerateKey(extKey, rand.Reader)
sealedKey = objectKey.Seal(extKey, crypto.GenerateIV(rand.Reader), crypto.SSEC.String(), bucket, object)
crypto.SSEC.CreateMetadata(metadata, sealedKey)
return objectKey[:], nil
return objectKey, nil
}
func newEncryptReader(content io.Reader, key []byte, bucket, object string, metadata map[string]string, sseS3 bool) (r io.Reader, encKey []byte, err error) {
func newEncryptReader(content io.Reader, key []byte, bucket, object string, metadata map[string]string, sseS3 bool) (io.Reader, crypto.ObjectKey, error) {
objectEncryptionKey, err := newEncryptMetadata(key, bucket, object, metadata, sseS3)
if err != nil {
return nil, encKey, err
return nil, crypto.ObjectKey{}, err
}
reader, err := sio.EncryptReader(content, sio.Config{Key: objectEncryptionKey[:], MinVersion: sio.Version20})
if err != nil {
return nil, encKey, crypto.ErrInvalidCustomerKey
return nil, crypto.ObjectKey{}, crypto.ErrInvalidCustomerKey
}
return reader, objectEncryptionKey, nil
@ -225,23 +226,24 @@ func setEncryptionMetadata(r *http.Request, bucket, object string, metadata map[
// EncryptRequest takes the client provided content and encrypts the data
// with the client provided key. It also marks the object as client-side-encrypted
// and sets the correct headers.
func EncryptRequest(content io.Reader, r *http.Request, bucket, object string, metadata map[string]string) (reader io.Reader, objEncKey []byte, err error) {
var key []byte
func EncryptRequest(content io.Reader, r *http.Request, bucket, object string, metadata map[string]string) (io.Reader, crypto.ObjectKey, error) {
if crypto.S3.IsRequested(r.Header) && crypto.SSEC.IsRequested(r.Header) {
return nil, objEncKey, crypto.ErrIncompatibleEncryptionMethod
}
if crypto.SSEC.IsRequested(r.Header) {
key, err = ParseSSECustomerRequest(r)
if err != nil {
return nil, objEncKey, err
}
return nil, crypto.ObjectKey{}, crypto.ErrIncompatibleEncryptionMethod
}
if r.ContentLength > encryptBufferThreshold {
// The encryption reads in blocks of 64KB.
// We add a buffer on bigger files to reduce the number of syscalls upstream.
content = bufio.NewReaderSize(content, encryptBufferSize)
}
var key []byte
if crypto.SSEC.IsRequested(r.Header) {
var err error
key, err = ParseSSECustomerRequest(r)
if err != nil {
return nil, crypto.ObjectKey{}, err
}
}
return newEncryptReader(content, key, bucket, object, metadata, crypto.S3.IsRequested(r.Header))
}
@ -634,6 +636,47 @@ func (o *ObjectInfo) DecryptedSize() (int64, error) {
return size, nil
}
// DecryptETag decrypts the ETag that is part of given object
// with the given object encryption key.
//
// However, DecryptETag does not try to decrypt the ETag if
// it consists of a 128 bit hex value (32 hex chars) and exactly
// one '-' followed by a 32-bit number.
// This special case adresses randomly-generated ETags generated
// by the MinIO server when running in non-compat mode. These
// random ETags are not encrypt.
//
// Calling DecryptETag with a non-randomly generated ETag will
// fail.
func DecryptETag(key crypto.ObjectKey, object ObjectInfo) (string, error) {
if n := strings.Count(object.ETag, "-"); n > 0 {
if n != 1 {
return "", errObjectTampered
}
i := strings.IndexByte(object.ETag, '-')
if len(object.ETag[:i]) != 32 {
return "", errObjectTampered
}
if _, err := hex.DecodeString(object.ETag[:32]); err != nil {
return "", errObjectTampered
}
if _, err := strconv.ParseInt(object.ETag[i+1:], 10, 32); err != nil {
return "", errObjectTampered
}
return object.ETag, nil
}
etag, err := hex.DecodeString(object.ETag)
if err != nil {
return "", err
}
etag, err = key.UnsealETag(etag)
if err != nil {
return "", err
}
return hex.EncodeToString(etag), nil
}
// For encrypted objects, the ETag sent by client if available
// is stored in encrypted form in the backend. Decrypt the ETag
// if ETag was previously encrypted.

View file

@ -256,6 +256,78 @@ func TestDecryptObjectInfo(t *testing.T) {
}
}
var decryptETagTests = []struct {
ObjectKey crypto.ObjectKey
ObjectInfo ObjectInfo
ShouldFail bool
ETag string
}{
{
ObjectKey: [32]byte{},
ObjectInfo: ObjectInfo{ETag: "20000f00f27834c9a2654927546df57f9e998187496394d4ee80f3d9978f85f3c7d81f72600cdbe03d80dc5a13d69354"},
ETag: "8ad3fe6b84bf38489e95c701c84355b6",
},
{
ObjectKey: [32]byte{},
ObjectInfo: ObjectInfo{ETag: "20000f00f27834c9a2654927546df57f9e998187496394d4ee80f3d9978f85f3c7d81f72600cdbe03d80dc5a13d6935"},
ETag: "",
ShouldFail: true, // ETag is not a valid hex value
},
{
ObjectKey: [32]byte{},
ObjectInfo: ObjectInfo{ETag: "00000f00f27834c9a2654927546df57f9e998187496394d4ee80f3d9978f85f3c7d81f72600cdbe03d80dc5a13d69354"},
ETag: "",
ShouldFail: true, // modified ETag
},
// Special tests for ETags that end with a '-x'
{
ObjectKey: [32]byte{},
ObjectInfo: ObjectInfo{ETag: "916516b396f0f4d4f2a0e7177557bec4-1"},
ETag: "916516b396f0f4d4f2a0e7177557bec4-1",
},
{
ObjectKey: [32]byte{},
ObjectInfo: ObjectInfo{ETag: "916516b396f0f4d4f2a0e7177557bec4-738"},
ETag: "916516b396f0f4d4f2a0e7177557bec4-738",
},
{
ObjectKey: [32]byte{},
ObjectInfo: ObjectInfo{ETag: "916516b396f0f4d4f2a0e7177557bec4-Q"},
ETag: "",
ShouldFail: true, // Q is not a number
},
{
ObjectKey: [32]byte{},
ObjectInfo: ObjectInfo{ETag: "16516b396f0f4d4f2a0e7177557bec4-1"},
ETag: "",
ShouldFail: true, // ETag prefix is not a valid hex value
},
{
ObjectKey: [32]byte{},
ObjectInfo: ObjectInfo{ETag: "16516b396f0f4d4f2a0e7177557bec4-1-2"},
ETag: "",
ShouldFail: true, // ETag contains multiple: -
},
}
func TestDecryptETag(t *testing.T) {
for i, test := range decryptETagTests {
etag, err := DecryptETag(test.ObjectKey, test.ObjectInfo)
if err != nil && !test.ShouldFail {
t.Fatalf("Test %d: should succeed but failed: %v", i, err)
}
if err == nil && test.ShouldFail {
t.Fatalf("Test %d: should fail but succeeded", i)
}
if err == nil {
if etag != test.ETag {
t.Fatalf("Test %d: ETag mismatch: got %s - want %s", i, etag, test.ETag)
}
}
}
}
// Tests for issue reproduced when getting the right encrypted
// offset of the object.
func TestGetDecryptedRange_Issue50(t *testing.T) {

View file

@ -773,16 +773,13 @@ func (p *PutObjReader) MD5CurrentHexString() string {
// 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, encKey []byte) *PutObjReader {
func NewPutObjReader(rawReader *hash.Reader, encReader *hash.Reader, key *crypto.ObjectKey) *PutObjReader {
p := PutObjReader{Reader: rawReader, rawReader: rawReader}
if len(encKey) != 0 && encReader != nil {
var objKey crypto.ObjectKey
copy(objKey[:], encKey)
p.sealMD5Fn = sealETagFn(objKey)
if key != nil && encReader != nil {
p.sealMD5Fn = sealETagFn(*key)
p.Reader = encReader
}
return &p
}

View file

@ -19,8 +19,6 @@ package cmd
import (
"bufio"
"context"
"crypto/hmac"
"encoding/binary"
"encoding/hex"
"encoding/xml"
"io"
@ -49,7 +47,6 @@ import (
iampolicy "github.com/minio/minio/pkg/iam/policy"
"github.com/minio/minio/pkg/ioutil"
"github.com/minio/minio/pkg/s3select"
sha256 "github.com/minio/sha256-simd"
"github.com/minio/sio"
)
@ -905,7 +902,8 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
return
}
var oldKey, newKey, objEncKey []byte
var oldKey, newKey []byte
var objEncKey crypto.ObjectKey
sseCopyS3 := crypto.S3.IsEncrypted(srcInfo.UserDefined)
sseCopyC := crypto.SSEC.IsEncrypted(srcInfo.UserDefined) && crypto.SSECopy.IsRequested(r.Header)
sseC := crypto.SSEC.IsRequested(r.Header)
@ -995,7 +993,7 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
return
}
pReader = NewPutObjReader(rawReader, srcInfo.Reader, objEncKey)
pReader = NewPutObjReader(rawReader, srcInfo.Reader, &objEncKey)
}
}
@ -1355,26 +1353,28 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req
return
}
var objectEncryptionKey []byte
var objectEncryptionKey crypto.ObjectKey
if objectAPI.IsEncryptionSupported() {
if crypto.IsRequested(r.Header) && !HasSuffix(object, SlashSeparator) { // handle SSE requests
if crypto.SSECopy.IsRequested(r.Header) {
writeErrorResponse(ctx, w, toAPIError(ctx, errInvalidEncryptionParameters), r.URL, guessIsBrowserReq(r))
return
}
reader, objectEncryptionKey, err = EncryptRequest(hashReader, r, bucket, object, metadata)
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r))
return
}
info := ObjectInfo{Size: size}
// do not try to verify encrypted content
hashReader, err = hash.NewReader(reader, info.EncryptedSize(), "", "", size, globalCLIContext.StrictS3Compat)
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r))
return
}
pReader = NewPutObjReader(rawReader, hashReader, objectEncryptionKey)
pReader = NewPutObjReader(rawReader, hashReader, &objectEncryptionKey)
}
}
@ -1389,27 +1389,29 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req
}
etag := objInfo.ETag
if objInfo.IsCompressed() {
switch {
case objInfo.IsCompressed():
if !strings.HasSuffix(objInfo.ETag, "-1") {
etag = objInfo.ETag + "-1"
}
} else if crypto.IsRequested(r.Header) {
etag = getDecryptedETag(r.Header, objInfo, false)
}
w.Header()[xhttp.ETag] = []string{"\"" + etag + "\""}
case crypto.IsEncrypted(objInfo.UserDefined):
switch {
case crypto.S3.IsEncrypted(objInfo.UserDefined):
w.Header().Set(crypto.SSEHeader, crypto.SSEAlgorithmAES256)
etag, _ = DecryptETag(objectEncryptionKey, ObjectInfo{ETag: etag})
case crypto.SSEC.IsEncrypted(objInfo.UserDefined):
w.Header().Set(crypto.SSECAlgorithm, r.Header.Get(crypto.SSECAlgorithm))
w.Header().Set(crypto.SSECKeyMD5, r.Header.Get(crypto.SSECKeyMD5))
if objectAPI.IsEncryptionSupported() {
if crypto.IsEncrypted(objInfo.UserDefined) {
switch {
case crypto.S3.IsEncrypted(objInfo.UserDefined):
w.Header().Set(crypto.SSEHeader, crypto.SSEAlgorithmAES256)
case crypto.SSEC.IsRequested(r.Header):
w.Header().Set(crypto.SSECAlgorithm, r.Header.Get(crypto.SSECAlgorithm))
w.Header().Set(crypto.SSECKeyMD5, r.Header.Get(crypto.SSECKeyMD5))
if len(etag) >= 32 && strings.Count(etag, "-") != 1 {
etag = etag[len(etag)-32:]
}
}
}
// We must not use the http.Header().Set method here because some (broken)
// clients expect the ETag header key to be literally "ETag" - not "Etag" (case-sensitive).
// Therefore, we have to set the ETag directly as map entry.
w.Header()[xhttp.ETag] = []string{`"` + etag + `"`}
writeSuccessResponseHeadersOnly(w)
// Notify object created event.
@ -1812,7 +1814,7 @@ func (api objectAPIHandlers) CopyObjectPartHandler(w http.ResponseWriter, r *htt
pReader := NewPutObjReader(rawReader, nil, nil)
isEncrypted := false
var objectEncryptionKey []byte
var objectEncryptionKey crypto.ObjectKey
if objectAPI.IsEncryptionSupported() && !isCompressed {
li, lerr := objectAPI.ListObjectParts(ctx, dstBucket, dstObject, uploadID, 0, 1, dstOpts)
if lerr != nil {
@ -1843,19 +1845,15 @@ func (api objectAPIHandlers) CopyObjectPartHandler(w http.ResponseWriter, r *htt
return
}
}
objectEncryptionKey, err = decryptObjectInfo(key, dstBucket, dstObject, li.UserDefined)
key, err = decryptObjectInfo(key, dstBucket, dstObject, li.UserDefined)
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r))
return
}
copy(objectEncryptionKey[:], key)
var partIDbin [4]byte
binary.LittleEndian.PutUint32(partIDbin[:], uint32(partID)) // marshal part ID
mac := hmac.New(sha256.New, objectEncryptionKey) // derive part encryption key from part ID and object key
mac.Write(partIDbin[:])
partEncryptionKey := mac.Sum(nil)
reader, err = sio.EncryptReader(reader, sio.Config{Key: partEncryptionKey})
partEncryptionKey := objectEncryptionKey.DerivePartKey(uint32(partID))
reader, err = sio.EncryptReader(reader, sio.Config{Key: partEncryptionKey[:]})
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r))
return
@ -1867,7 +1865,7 @@ 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 = NewPutObjReader(rawReader, srcInfo.Reader, &objectEncryptionKey)
}
}
srcInfo.PutObjReader = pReader
@ -1881,7 +1879,7 @@ func (api objectAPIHandlers) CopyObjectPartHandler(w http.ResponseWriter, r *htt
}
if isEncrypted {
partInfo.ETag = tryDecryptETag(objectEncryptionKey, partInfo.ETag, crypto.SSEC.IsRequested(r.Header))
partInfo.ETag = tryDecryptETag(objectEncryptionKey[:], partInfo.ETag, crypto.SSEC.IsRequested(r.Header))
}
response := generateCopyObjectPartResponse(partInfo.ETag, partInfo.LastModified)
@ -2069,7 +2067,7 @@ func (api objectAPIHandlers) PutObjectPartHandler(w http.ResponseWriter, r *http
}
isEncrypted := false
var objectEncryptionKey []byte
var objectEncryptionKey crypto.ObjectKey
if objectAPI.IsEncryptionSupported() && !isCompressed {
var li ListPartsInfo
li, err = objectAPI.ListObjectParts(ctx, bucket, object, uploadID, 0, 1, ObjectOptions{})
@ -2101,25 +2099,21 @@ func (api objectAPIHandlers) PutObjectPartHandler(w http.ResponseWriter, r *http
}
// Calculating object encryption key
objectEncryptionKey, err = decryptObjectInfo(key, bucket, object, li.UserDefined)
key, err = decryptObjectInfo(key, bucket, object, li.UserDefined)
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r))
return
}
var partIDbin [4]byte
binary.LittleEndian.PutUint32(partIDbin[:], uint32(partID)) // marshal part ID
mac := hmac.New(sha256.New, objectEncryptionKey) // derive part encryption key from part ID and object key
mac.Write(partIDbin[:])
partEncryptionKey := mac.Sum(nil)
copy(objectEncryptionKey[:], key)
partEncryptionKey := objectEncryptionKey.DerivePartKey(uint32(partID))
in := io.Reader(hashReader)
if size > encryptBufferThreshold {
// The encryption reads in blocks of 64KB.
// We add a buffer on bigger files to reduce the number of syscalls upstream.
in = bufio.NewReaderSize(hashReader, encryptBufferSize)
}
reader, err = sio.EncryptReader(in, sio.Config{Key: partEncryptionKey})
reader, err = sio.EncryptReader(in, sio.Config{Key: partEncryptionKey[:]})
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r))
return
@ -2131,7 +2125,7 @@ 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 = NewPutObjReader(rawReader, hashReader, &objectEncryptionKey)
}
}
@ -2146,7 +2140,7 @@ func (api objectAPIHandlers) PutObjectPartHandler(w http.ResponseWriter, r *http
etag := partInfo.ETag
if isEncrypted {
etag = tryDecryptETag(objectEncryptionKey, partInfo.ETag, crypto.SSEC.IsRequested(r.Header))
etag = tryDecryptETag(objectEncryptionKey[:], partInfo.ETag, crypto.SSEC.IsRequested(r.Header))
}
w.Header()[xhttp.ETag] = []string{"\"" + etag + "\""}

View file

@ -1137,7 +1137,7 @@ func (web *webAPIHandlers) Upload(w http.ResponseWriter, r *http.Request) {
if objectAPI.IsEncryptionSupported() {
if crypto.IsRequested(r.Header) && !HasSuffix(object, SlashSeparator) { // handle SSE requests
rawReader := hashReader
var objectEncryptionKey []byte
var objectEncryptionKey crypto.ObjectKey
reader, objectEncryptionKey, err = EncryptRequest(hashReader, r, bucket, object, metadata)
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r))
@ -1150,7 +1150,7 @@ 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 = NewPutObjReader(rawReader, hashReader, &objectEncryptionKey)
}
}