From 11aa393ba755d9fa7a385856894af6267c8795aa Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Sun, 23 Aug 2020 22:06:22 -0700 Subject: [PATCH] Allow region errors to be dynamic (#10323) remove other FIXMEs as we are not planning to fix these, instead we will add dynamism case by case basis. fixes #10250 --- cmd/api-errors.go | 21 +-------------- cmd/api-response.go | 40 ++++------------------------ cmd/bucket-handlers.go | 2 +- cmd/post-policy_test.go | 4 +-- cmd/signature-v4-parser.go | 54 +++++++++++++++++++------------------- cmd/signature-v4.go | 6 ++--- cmd/signature-v4_test.go | 2 +- 7 files changed, 40 insertions(+), 89 deletions(-) diff --git a/cmd/api-errors.go b/cmd/api-errors.go index 509fa0160..937885a29 100644 --- a/cmd/api-errors.go +++ b/cmd/api-errors.go @@ -663,20 +663,9 @@ var errorCodes = errorCodeMap{ Description: "X-Amz-Date must be in the ISO8601 Long Format \"yyyyMMdd'T'HHmmss'Z'\"", HTTPStatusCode: http.StatusBadRequest, }, - // FIXME: Should contain the invalid param set as seen in https://github.com/minio/minio/issues/2385. - // right Description: "Error parsing the X-Amz-Credential parameter; incorrect date format \"%s\". This date in the credential must be in the format \"yyyyMMdd\".", - // Need changes to make sure variable messages can be constructed. ErrMalformedCredentialDate: { Code: "AuthorizationQueryParametersError", - Description: "Error parsing the X-Amz-Credential parameter; incorrect date format \"%s\". This date in the credential must be in the format \"yyyyMMdd\".", - HTTPStatusCode: http.StatusBadRequest, - }, - // FIXME: Should contain the invalid param set as seen in https://github.com/minio/minio/issues/2385. - // right Description: "Error parsing the X-Amz-Credential parameter; the region 'us-east-' is wrong; expecting 'us-east-1'". - // Need changes to make sure variable messages can be constructed. - ErrMalformedCredentialRegion: { - Code: "AuthorizationQueryParametersError", - Description: "Error parsing the X-Amz-Credential parameter; the region is wrong;", + Description: "Error parsing the X-Amz-Credential parameter; incorrect date format. This date in the credential must be in the format \"yyyyMMdd\".", HTTPStatusCode: http.StatusBadRequest, }, ErrInvalidRegion: { @@ -684,9 +673,6 @@ var errorCodes = errorCodeMap{ Description: "Region does not match.", HTTPStatusCode: http.StatusBadRequest, }, - // FIXME: Should contain the invalid param set as seen in https://github.com/minio/minio/issues/2385. - // right Description: "Error parsing the X-Amz-Credential parameter; incorrect service \"s4\". This endpoint belongs to \"s3\".". - // Need changes to make sure variable messages can be constructed. ErrInvalidServiceS3: { Code: "AuthorizationParametersError", Description: "Error parsing the Credential/X-Amz-Credential parameter; incorrect service. This endpoint belongs to \"s3\".", @@ -697,9 +683,6 @@ var errorCodes = errorCodeMap{ Description: "Error parsing the Credential parameter; incorrect service. This endpoint belongs to \"sts\".", HTTPStatusCode: http.StatusBadRequest, }, - // FIXME: Should contain the invalid param set as seen in https://github.com/minio/minio/issues/2385. - // Description: "Error parsing the X-Amz-Credential parameter; incorrect terminal "aws4_reque". This endpoint uses "aws4_request". - // Need changes to make sure variable messages can be constructed. ErrInvalidRequestVersion: { Code: "AuthorizationQueryParametersError", Description: "Error parsing the X-Amz-Credential parameter; incorrect terminal. This endpoint uses \"aws4_request\".", @@ -770,8 +753,6 @@ var errorCodes = errorCodeMap{ Description: "Your key is too long", HTTPStatusCode: http.StatusBadRequest, }, - - // FIXME: Actual XML error response also contains the header which missed in list of signed header parameters. ErrUnsignedHeaders: { Code: "AccessDenied", Description: "There were headers present in the request which were not signed", diff --git a/cmd/api-response.go b/cmd/api-response.go index 41e9ce118..b97409aa1 100644 --- a/cmd/api-response.go +++ b/cmd/api-response.go @@ -20,6 +20,7 @@ import ( "context" "encoding/base64" "encoding/xml" + "fmt" "net/http" "net/url" "path" @@ -763,6 +764,10 @@ func writeErrorResponse(ctx context.Context, w http.ResponseWriter, err APIError // Set retry-after header to indicate user-agents to retry request after 120secs. // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After w.Header().Set(xhttp.RetryAfter, "120") + case "InvalidRegion": + err.Description = fmt.Sprintf("Region does not match; expecting '%s'.", globalServerRegion) + case "AuthorizationHeaderMalformed": + err.Description = fmt.Sprintf("The authorization header is malformed; the region is wrong; expecting '%s'.", globalServerRegion) case "AccessDenied": // The request is from browser and also if browser // is enabled we need to redirect. @@ -821,38 +826,3 @@ func writeCustomErrorResponseJSON(ctx context.Context, w http.ResponseWriter, er encodedErrorResponse := encodeResponseJSON(errorResponse) writeResponse(w, err.HTTPStatusCode, encodedErrorResponse, mimeJSON) } - -// writeCustomErrorResponseXML - similar to writeErrorResponse, -// but accepts the error message directly (this allows messages to be -// dynamically generated.) -func writeCustomErrorResponseXML(ctx context.Context, w http.ResponseWriter, err APIError, errBody string, reqURL *url.URL, browser bool) { - - switch err.Code { - case "SlowDown", "XMinioServerNotInitialized", "XMinioReadQuorum", "XMinioWriteQuorum": - // Set retry-after header to indicate user-agents to retry request after 120secs. - // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After - w.Header().Set(xhttp.RetryAfter, "120") - case "AccessDenied": - // The request is from browser and also if browser - // is enabled we need to redirect. - if browser && globalBrowserEnabled { - w.Header().Set(xhttp.Location, minioReservedBucketPath+reqURL.Path) - w.WriteHeader(http.StatusTemporaryRedirect) - return - } - } - - reqInfo := logger.GetReqInfo(ctx) - errorResponse := APIErrorResponse{ - Code: err.Code, - Message: errBody, - Resource: reqURL.Path, - BucketName: reqInfo.BucketName, - Key: reqInfo.ObjectName, - RequestID: w.Header().Get(xhttp.AmzRequestID), - HostID: globalDeploymentID, - } - - encodedErrorResponse := encodeResponse(errorResponse) - writeResponse(w, err.HTTPStatusCode, encodedErrorResponse, mimeXML) -} diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index 9934591ce..bc0a89fde 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -762,7 +762,7 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h // Make sure formValues adhere to policy restrictions. if err = checkPostPolicy(formValues, postPolicyForm); err != nil { - writeCustomErrorResponseXML(ctx, w, errorCodes.ToAPIErr(ErrAccessDenied), err.Error(), r.URL, guessIsBrowserReq(r)) + writeErrorResponse(ctx, w, errorCodes.ToAPIErrWithErr(ErrAccessDenied, err), r.URL, guessIsBrowserReq(r)) return } diff --git a/cmd/post-policy_test.go b/cmd/post-policy_test.go index 9a37c86c3..1edf9b9a4 100644 --- a/cmd/post-policy_test.go +++ b/cmd/post-policy_test.go @@ -196,7 +196,7 @@ func testPostPolicyBucketHandler(obj ObjectLayer, instanceType string, t TestErr { objectName: "test", data: []byte("Hello, World"), - expectedRespStatus: http.StatusBadRequest, + expectedRespStatus: http.StatusForbidden, accessKey: "", secretKey: "", malformedBody: false, @@ -293,7 +293,7 @@ func testPostPolicyBucketHandler(obj ObjectLayer, instanceType string, t TestErr { objectName: "test", data: []byte("Hello, World"), - expectedRespStatus: http.StatusBadRequest, + expectedRespStatus: http.StatusForbidden, accessKey: "", secretKey: "", dates: []interface{}{}, diff --git a/cmd/signature-v4-parser.go b/cmd/signature-v4-parser.go index c4baccd8b..67f7aba1b 100644 --- a/cmd/signature-v4-parser.go +++ b/cmd/signature-v4-parser.go @@ -49,17 +49,17 @@ func (c credentialHeader) getScope() string { } func getReqAccessKeyV4(r *http.Request, region string, stype serviceType) (auth.Credentials, bool, APIErrorCode) { - ch, err := parseCredentialHeader("Credential="+r.URL.Query().Get(xhttp.AmzCredential), region, stype) - if err != ErrNone { + ch, s3Err := parseCredentialHeader("Credential="+r.URL.Query().Get(xhttp.AmzCredential), region, stype) + if s3Err != ErrNone { // Strip off the Algorithm prefix. v4Auth := strings.TrimPrefix(r.Header.Get("Authorization"), signV4Algorithm) authFields := strings.Split(strings.TrimSpace(v4Auth), ",") if len(authFields) != 3 { return auth.Credentials{}, false, ErrMissingFields } - ch, err = parseCredentialHeader(authFields[0], region, stype) - if err != ErrNone { - return auth.Credentials{}, false, err + ch, s3Err = parseCredentialHeader(authFields[0], region, stype) + if s3Err != ErrNone { + return auth.Credentials{}, false, s3Err } } return checkKeyValid(ch.accessKey) @@ -192,9 +192,9 @@ func doesV4PresignParamsExist(query url.Values) APIErrorCode { // Parses all the presigned signature values into separate elements. func parsePreSignV4(query url.Values, region string, stype serviceType) (psv preSignValues, aec APIErrorCode) { // verify whether the required query params exist. - err := doesV4PresignParamsExist(query) - if err != ErrNone { - return psv, err + aec = doesV4PresignParamsExist(query) + if aec != ErrNone { + return psv, aec } // Verify if the query algorithm is supported or not. @@ -206,9 +206,9 @@ func parsePreSignV4(query url.Values, region string, stype serviceType) (psv pre preSignV4Values := preSignValues{} // Save credential. - preSignV4Values.Credential, err = parseCredentialHeader("Credential="+query.Get(xhttp.AmzCredential), region, stype) - if err != ErrNone { - return psv, err + preSignV4Values.Credential, aec = parseCredentialHeader("Credential="+query.Get(xhttp.AmzCredential), region, stype) + if aec != ErrNone { + return psv, aec } var e error @@ -234,15 +234,15 @@ func parsePreSignV4(query url.Values, region string, stype serviceType) (psv pre } // Save signed headers. - preSignV4Values.SignedHeaders, err = parseSignedHeader("SignedHeaders=" + query.Get(xhttp.AmzSignedHeaders)) - if err != ErrNone { - return psv, err + preSignV4Values.SignedHeaders, aec = parseSignedHeader("SignedHeaders=" + query.Get(xhttp.AmzSignedHeaders)) + if aec != ErrNone { + return psv, aec } // Save signature. - preSignV4Values.Signature, err = parseSignature("Signature=" + query.Get(xhttp.AmzSignature)) - if err != ErrNone { - return psv, err + preSignV4Values.Signature, aec = parseSignature("Signature=" + query.Get(xhttp.AmzSignature)) + if aec != ErrNone { + return psv, aec } // Return structed form of signature query string. @@ -280,23 +280,23 @@ func parseSignV4(v4Auth string, region string, stype serviceType) (sv signValues // Initialize signature version '4' structured header. signV4Values := signValues{} - var err APIErrorCode + var s3Err APIErrorCode // Save credentail values. - signV4Values.Credential, err = parseCredentialHeader(strings.TrimSpace(credElement), region, stype) - if err != ErrNone { - return sv, err + signV4Values.Credential, s3Err = parseCredentialHeader(strings.TrimSpace(credElement), region, stype) + if s3Err != ErrNone { + return sv, s3Err } // Save signed headers. - signV4Values.SignedHeaders, err = parseSignedHeader(authFields[1]) - if err != ErrNone { - return sv, err + signV4Values.SignedHeaders, s3Err = parseSignedHeader(authFields[1]) + if s3Err != ErrNone { + return sv, s3Err } // Save signature. - signV4Values.Signature, err = parseSignature(authFields[2]) - if err != ErrNone { - return sv, err + signV4Values.Signature, s3Err = parseSignature(authFields[2]) + if s3Err != ErrNone { + return sv, s3Err } // Return the structure here. diff --git a/cmd/signature-v4.go b/cmd/signature-v4.go index ffb01c9d6..f7b280fda 100644 --- a/cmd/signature-v4.go +++ b/cmd/signature-v4.go @@ -173,9 +173,9 @@ func doesPolicySignatureV4Match(formValues http.Header) APIErrorCode { region := globalServerRegion // Parse credential tag. - credHeader, err := parseCredentialHeader("Credential="+formValues.Get(xhttp.AmzCredential), region, serviceS3) - if err != ErrNone { - return ErrMissingFields + credHeader, s3Err := parseCredentialHeader("Credential="+formValues.Get(xhttp.AmzCredential), region, serviceS3) + if s3Err != ErrNone { + return s3Err } cred, _, s3Err := checkKeyValid(credHeader.accessKey) diff --git a/cmd/signature-v4_test.go b/cmd/signature-v4_test.go index f9fd3e133..b7dc57831 100644 --- a/cmd/signature-v4_test.go +++ b/cmd/signature-v4_test.go @@ -46,7 +46,7 @@ func TestDoesPolicySignatureMatch(t *testing.T) { // (0) It should fail if 'X-Amz-Credential' is missing. { form: http.Header{}, - expected: ErrMissingFields, + expected: ErrCredMalformed, }, // (1) It should fail if the access key is incorrect. {