From f8f290e8484fe002f4cb7e15046a33a5dbd3e0cf Mon Sep 17 00:00:00 2001 From: Klaus Post Date: Tue, 11 Aug 2020 08:29:29 -0700 Subject: [PATCH] security: Remove insecure custom headers (#10244) Background: https://github.com/google/security-research/security/advisories/GHSA-76wf-9vgp-pj7w Remove these custom headers from incoming and outgoing requests. --- cmd/api-headers.go | 5 +++++ cmd/api-response.go | 4 ++++ cmd/crypto/header.go | 4 ++++ cmd/crypto/header_test.go | 10 ++++++++++ cmd/crypto/metadata.go | 3 +++ cmd/handler-utils.go | 7 +++++++ cmd/http/headers.go | 3 +++ cmd/xl-storage-format-v2.go | 6 ++++++ 8 files changed, 42 insertions(+) diff --git a/cmd/api-headers.go b/cmd/api-headers.go index 8eeacf374..66ff7c078 100644 --- a/cmd/api-headers.go +++ b/cmd/api-headers.go @@ -128,6 +128,11 @@ func setObjectHeaders(w http.ResponseWriter, objInfo ObjectInfo, rs *HTTPRangeSp // values to client. continue } + + // https://github.com/google/security-research/security/advisories/GHSA-76wf-9vgp-pj7w + if strings.EqualFold(k, xhttp.AmzMetaUnencryptedContentLength) || strings.EqualFold(k, xhttp.AmzMetaUnencryptedContentMD5) { + continue + } var isSet bool for _, userMetadataPrefix := range userMetadataKeyPrefixes { if !strings.HasPrefix(k, userMetadataPrefix) { diff --git a/cmd/api-response.go b/cmd/api-response.go index 22df12678..41e9ce118 100644 --- a/cmd/api-response.go +++ b/cmd/api-response.go @@ -564,6 +564,10 @@ func generateListObjectsV2Response(bucket, prefix, token, nextToken, startAfter, // values to client. continue } + // https://github.com/google/security-research/security/advisories/GHSA-76wf-9vgp-pj7w + if strings.EqualFold(k, xhttp.AmzMetaUnencryptedContentLength) || strings.EqualFold(k, xhttp.AmzMetaUnencryptedContentMD5) { + continue + } content.UserMetadata[k] = v } } diff --git a/cmd/crypto/header.go b/cmd/crypto/header.go index b94ef4516..9f83a72f2 100644 --- a/cmd/crypto/header.go +++ b/cmd/crypto/header.go @@ -21,6 +21,8 @@ import ( "encoding/json" "net/http" "strings" + + xhttp "github.com/minio/minio/cmd/http" ) // SSEHeader is the general AWS SSE HTTP header key. @@ -81,6 +83,8 @@ const ( func RemoveSensitiveHeaders(h http.Header) { h.Del(SSECKey) h.Del(SSECopyKey) + h.Del(xhttp.AmzMetaUnencryptedContentLength) + h.Del(xhttp.AmzMetaUnencryptedContentMD5) } // IsRequested returns true if the HTTP headers indicates diff --git a/cmd/crypto/header_test.go b/cmd/crypto/header_test.go index 3f0a3d725..00fe33437 100644 --- a/cmd/crypto/header_test.go +++ b/cmd/crypto/header_test.go @@ -457,6 +457,16 @@ var removeSensitiveHeadersTests = []struct { "X-Amz-Meta-Test-1": []string{"Test-1"}, }, }, + { // https://github.com/google/security-research/security/advisories/GHSA-76wf-9vgp-pj7w + Header: http.Header{ + "X-Amz-Meta-X-Amz-Unencrypted-Content-Md5": []string{"value"}, + "X-Amz-Meta-X-Amz-Unencrypted-Content-Length": []string{"value"}, + "X-Amz-Meta-Test-1": []string{"Test-1"}, + }, + ExpectedHeader: http.Header{ + "X-Amz-Meta-Test-1": []string{"Test-1"}, + }, + }, } func TestRemoveSensitiveHeaders(t *testing.T) { diff --git a/cmd/crypto/metadata.go b/cmd/crypto/metadata.go index ee0864bfd..1ab5b7a2e 100644 --- a/cmd/crypto/metadata.go +++ b/cmd/crypto/metadata.go @@ -19,6 +19,7 @@ import ( "encoding/base64" "errors" + xhttp "github.com/minio/minio/cmd/http" "github.com/minio/minio/cmd/logger" ) @@ -38,6 +39,8 @@ func IsMultiPart(metadata map[string]string) bool { func RemoveSensitiveEntries(metadata map[string]string) { // The functions is tested in TestRemoveSensitiveHeaders for compatibility reasons delete(metadata, SSECKey) delete(metadata, SSECopyKey) + delete(metadata, xhttp.AmzMetaUnencryptedContentLength) + delete(metadata, xhttp.AmzMetaUnencryptedContentMD5) } // RemoveSSEHeaders removes all crypto-specific SSE diff --git a/cmd/handler-utils.go b/cmd/handler-utils.go index f470799b0..a4bc7079b 100644 --- a/cmd/handler-utils.go +++ b/cmd/handler-utils.go @@ -131,6 +131,13 @@ func extractMetadata(ctx context.Context, r *http.Request) (metadata map[string] metadata[strings.ToLower(xhttp.ContentType)] = "application/octet-stream" } + // https://github.com/google/security-research/security/advisories/GHSA-76wf-9vgp-pj7w + for k := range metadata { + if strings.EqualFold(k, xhttp.AmzMetaUnencryptedContentLength) || strings.EqualFold(k, xhttp.AmzMetaUnencryptedContentMD5) { + delete(metadata, k) + } + } + if contentEncoding, ok := metadata[strings.ToLower(xhttp.ContentEncoding)]; ok { contentEncoding = trimAwsChunkedContentEncoding(contentEncoding) if contentEncoding != "" { diff --git a/cmd/http/headers.go b/cmd/http/headers.go index c7ab9602d..dbed10e5c 100644 --- a/cmd/http/headers.go +++ b/cmd/http/headers.go @@ -102,6 +102,9 @@ const ( AmzSecurityToken = "X-Amz-Security-Token" AmzDecodedContentLength = "X-Amz-Decoded-Content-Length" + AmzMetaUnencryptedContentLength = "X-Amz-Meta-X-Amz-Unencrypted-Content-Length" + AmzMetaUnencryptedContentMD5 = "X-Amz-Meta-X-Amz-Unencrypted-Content-Md5" + // Signature v2 related constants AmzSignatureV2 = "Signature" AmzAccessKeyID = "AWSAccessKeyId" diff --git a/cmd/xl-storage-format-v2.go b/cmd/xl-storage-format-v2.go index 3aa2af165..6b925e8f3 100644 --- a/cmd/xl-storage-format-v2.go +++ b/cmd/xl-storage-format-v2.go @@ -24,6 +24,7 @@ import ( "time" "github.com/google/uuid" + xhttp "github.com/minio/minio/cmd/http" "github.com/minio/minio/cmd/logger" ) @@ -397,6 +398,11 @@ func (j xlMetaV2Object) ToFileInfo(volume, path string) (FileInfo, error) { } fi.Metadata = make(map[string]string, len(j.MetaUser)+len(j.MetaSys)) for k, v := range j.MetaUser { + // https://github.com/google/security-research/security/advisories/GHSA-76wf-9vgp-pj7w + if strings.EqualFold(k, xhttp.AmzMetaUnencryptedContentLength) || strings.EqualFold(k, xhttp.AmzMetaUnencryptedContentMD5) { + continue + } + fi.Metadata[k] = v } for k, v := range j.MetaSys {