From 458f22f37c090121029795784f06e0f5013ced3c Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Wed, 31 May 2017 00:11:06 -0700 Subject: [PATCH] log: Fix printing of signature error request headers. (#4444) The following commit f44f2e341c931222a8f1e3f1e9911dda80bb2dc0 fix was incomplete and we still had presigned URLs printing in query strings in wrong fashion. This PR fixes this properly. Avoid double encoding percent encoded strings such as `s3%!!(MISSING)A(MISSING)` Print properly as json encoded. `s3%3AObjectCreated%3A%2A` --- cmd/auth-handler.go | 4 ++-- cmd/gateway-handlers.go | 26 +++++++++++++------------- cmd/object-handlers.go | 12 ++++++------ cmd/utils.go | 26 ++++++++++++++++---------- cmd/utils_test.go | 21 ++++++++++++++------- 5 files changed, 51 insertions(+), 38 deletions(-) diff --git a/cmd/auth-handler.go b/cmd/auth-handler.go index cdfbfe5a8..85fce51a6 100644 --- a/cmd/auth-handler.go +++ b/cmd/auth-handler.go @@ -113,13 +113,13 @@ func checkRequestAuthType(r *http.Request, bucket, policyAction, region string) // Signature V2 validation. s3Error := isReqAuthenticatedV2(r) if s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) } return s3Error case authTypeSigned, authTypePresigned: s3Error := isReqAuthenticated(r, region) if s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) } return s3Error } diff --git a/cmd/gateway-handlers.go b/cmd/gateway-handlers.go index 553de071b..4ce311f72 100644 --- a/cmd/gateway-handlers.go +++ b/cmd/gateway-handlers.go @@ -54,14 +54,14 @@ func (api gatewayAPIHandlers) GetObjectHandler(w http.ResponseWriter, r *http.Re // Signature V2 validation. s3Error := isReqAuthenticatedV2(r) if s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } case authTypeSigned, authTypePresigned: s3Error := isReqAuthenticated(r, serverConfig.GetRegion()) if s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } @@ -252,7 +252,7 @@ func (api gatewayAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Re // Initialize stream signature verifier. reader, s3Error := newSignV4ChunkedReader(r) if s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } @@ -260,14 +260,14 @@ func (api gatewayAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Re case authTypeSignedV2, authTypePresignedV2: s3Error := isReqAuthenticatedV2(r) if s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } objInfo, err = objectAPI.PutObject(bucket, object, size, r.Body, metadata, sha256sum) case authTypePresigned, authTypeSigned: if s3Error := reqSignatureV4Verify(r, serverConfig.GetRegion()); s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } @@ -313,14 +313,14 @@ func (api gatewayAPIHandlers) HeadObjectHandler(w http.ResponseWriter, r *http.R // Signature V2 validation. s3Error := isReqAuthenticatedV2(r) if s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } case authTypeSigned, authTypePresigned: s3Error := isReqAuthenticated(r, serverConfig.GetRegion()) if s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } @@ -727,14 +727,14 @@ func (api gatewayAPIHandlers) ListObjectsV1Handler(w http.ResponseWriter, r *htt // Signature V2 validation. s3Error := isReqAuthenticatedV2(r) if s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } case authTypeSigned, authTypePresigned: s3Error := isReqAuthenticated(r, serverConfig.GetRegion()) if s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } @@ -797,14 +797,14 @@ func (api gatewayAPIHandlers) HeadBucketHandler(w http.ResponseWriter, r *http.R // Signature V2 validation. s3Error := isReqAuthenticatedV2(r) if s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } case authTypeSigned, authTypePresigned: s3Error := isReqAuthenticated(r, serverConfig.GetRegion()) if s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } @@ -849,7 +849,7 @@ func (api gatewayAPIHandlers) GetBucketLocationHandler(w http.ResponseWriter, r // Signature V2 validation. s3Error := isReqAuthenticatedV2(r) if s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } @@ -860,7 +860,7 @@ func (api gatewayAPIHandlers) GetBucketLocationHandler(w http.ResponseWriter, r s3Error = isReqAuthenticated(r, serverConfig.GetRegion()) } if s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index bc432a097..3a1352fdf 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -507,7 +507,7 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req // Initialize stream signature verifier. reader, s3Error := newSignV4ChunkedReader(r) if s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } @@ -515,14 +515,14 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req case authTypeSignedV2, authTypePresignedV2: s3Error := isReqAuthenticatedV2(r) if s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } objInfo, err = objectAPI.PutObject(bucket, object, size, r.Body, metadata, sha256sum) case authTypePresigned, authTypeSigned: if s3Error := reqSignatureV4Verify(r, serverConfig.GetRegion()); s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } @@ -788,7 +788,7 @@ func (api objectAPIHandlers) PutObjectPartHandler(w http.ResponseWriter, r *http // Initialize stream signature verifier. reader, s3Error := newSignV4ChunkedReader(r) if s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } @@ -796,14 +796,14 @@ func (api objectAPIHandlers) PutObjectPartHandler(w http.ResponseWriter, r *http case authTypeSignedV2, authTypePresignedV2: s3Error := isReqAuthenticatedV2(r) if s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } partInfo, err = objectAPI.PutObjectPart(bucket, object, uploadID, partID, size, r.Body, incomingMD5, sha256sum) case authTypePresigned, authTypeSigned: if s3Error := reqSignatureV4Verify(r, serverConfig.GetRegion()); s3Error != ErrNone { - errorIf(errSignatureMismatch, dumpRequest(r)) + errorIf(errSignatureMismatch, "%s", dumpRequest(r)) writeErrorResponse(w, s3Error, r.URL) return } diff --git a/cmd/utils.go b/cmd/utils.go index 37e2ee292..398de1398 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -17,6 +17,7 @@ package cmd import ( + "bytes" "encoding/base64" "encoding/json" "encoding/xml" @@ -163,20 +164,25 @@ var globalProfiler interface { func dumpRequest(r *http.Request) string { header := cloneHeader(r.Header) header.Set("Host", r.Host) + // Replace all '%' to '%%' so that printer format parser + // to ignore URL encoded values. + rawURI := strings.Replace(r.RequestURI, "%", "%%", -1) req := struct { - Method string `json:"method"` - Path string `json:"path"` - Query string `json:"query"` - Header http.Header `json:"header"` - }{r.Method, getURLEncodedName(r.URL.Path), r.URL.RawQuery, header} - jsonBytes, err := json.Marshal(&req) - if err != nil { + Method string `json:"method"` + RequestURI string `json:"reqURI"` + Header http.Header `json:"header"` + }{r.Method, rawURI, header} + + var buffer bytes.Buffer + enc := json.NewEncoder(&buffer) + enc.SetEscapeHTML(false) + if err := enc.Encode(&req); err != nil { // Upon error just return Go-syntax representation of the value return fmt.Sprintf("%#v", req) } - // Replace all '%' to '%%' so that printer format parser - // to ignore URL encoded values. - return strings.Replace(string(jsonBytes), "%", "%%", -1) + + // Formatted string. + return strings.TrimSpace(string(buffer.Bytes())) } // isFile - returns whether given path is a file or not. diff --git a/cmd/utils_test.go b/cmd/utils_test.go index b67f8ea7e..f364ce4f7 100644 --- a/cmd/utils_test.go +++ b/cmd/utils_test.go @@ -248,17 +248,17 @@ func TestCheckURL(t *testing.T) { // Testing dumping request function. func TestDumpRequest(t *testing.T) { - req, err := http.NewRequest("GET", "http://localhost:9000?prefix=Hello%2AWorld%2A", nil) + req, err := http.NewRequest("GET", "http://localhost:9000?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=USWUXHGYZQYFYFFIT3RE%2F20170529%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20170529T190139Z&X-Amz-Expires=600&X-Amz-Signature=19b58080999df54b446fc97304eb8dda60d3df1812ae97f3e8783351bfd9781d&X-Amz-SignedHeaders=host&prefix=Hello%2AWorld%2A", nil) if err != nil { t.Fatal(err) } + req.RequestURI = "/?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=USWUXHGYZQYFYFFIT3RE%2F20170529%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20170529T190139Z&X-Amz-Expires=600&X-Amz-Signature=19b58080999df54b446fc97304eb8dda60d3df1812ae97f3e8783351bfd9781d&X-Amz-SignedHeaders=host&prefix=Hello%2AWorld%2A" req.Header.Set("content-md5", "====test") jsonReq := dumpRequest(req) type jsonResult struct { - Method string `json:"method"` - Path string `json:"path"` - Query string `json:"query"` - Header http.Header `json:"header"` + Method string `json:"method"` + RequestURI string `json:"reqURI"` + Header http.Header `json:"header"` } jsonReq = strings.Replace(jsonReq, "%%", "%", -1) res := jsonResult{} @@ -274,8 +274,15 @@ func TestDumpRequest(t *testing.T) { // Look for expected query values expectedQuery := url.Values{} expectedQuery.Set("prefix", "Hello*World*") - if !reflect.DeepEqual(res.Query, expectedQuery.Encode()) { - t.Fatalf("Expected %#v, got %#v", expectedQuery, res.Query) + expectedQuery.Set("X-Amz-Algorithm", "AWS4-HMAC-SHA256") + expectedQuery.Set("X-Amz-Credential", "USWUXHGYZQYFYFFIT3RE/20170529/us-east-1/s3/aws4_request") + expectedQuery.Set("X-Amz-Date", "20170529T190139Z") + expectedQuery.Set("X-Amz-Expires", "600") + expectedQuery.Set("X-Amz-SignedHeaders", "host") + expectedQuery.Set("X-Amz-Signature", "19b58080999df54b446fc97304eb8dda60d3df1812ae97f3e8783351bfd9781d") + expectedRequestURI := "/?" + expectedQuery.Encode() + if !reflect.DeepEqual(res.RequestURI, expectedRequestURI) { + t.Fatalf("Expected %#v, got %#v", expectedRequestURI, res.RequestURI) } // Look for expected header.