From c0a1369b736d37ac551a6a372a2012a447834bbd Mon Sep 17 00:00:00 2001 From: Praveen raj Mani Date: Wed, 6 Mar 2019 01:40:47 +0530 Subject: [PATCH] Construct dynamic XML error responses for postpolicyform validation (#7321) Fixes #7314 --- cmd/api-errors.go | 6 ------ cmd/api-response.go | 35 +++++++++++++++++++++++++++++++++++ cmd/bucket-handlers.go | 4 ++-- cmd/post-policy_test.go | 2 +- cmd/postpolicyform.go | 19 ++++++++++--------- cmd/postpolicyform_test.go | 26 +++++++++++++++----------- 6 files changed, 63 insertions(+), 29 deletions(-) diff --git a/cmd/api-errors.go b/cmd/api-errors.go index 67c201cd0..75fa424f3 100644 --- a/cmd/api-errors.go +++ b/cmd/api-errors.go @@ -114,7 +114,6 @@ const ( ErrInvalidRequestVersion ErrMissingSignTag ErrMissingSignHeadersTag - ErrPolicyAlreadyExpired ErrMalformedDate ErrMalformedPresignedDate ErrMalformedCredentialDate @@ -625,11 +624,6 @@ var errorCodes = errorCodeMap{ Description: "Signature header missing SignedHeaders field.", HTTPStatusCode: http.StatusBadRequest, }, - ErrPolicyAlreadyExpired: { - Code: "AccessDenied", - Description: "Invalid according to Policy: Policy expired.", - HTTPStatusCode: http.StatusBadRequest, - }, ErrMalformedExpires: { Code: "AuthorizationQueryParametersError", Description: "X-Amz-Expires should be a number", diff --git a/cmd/api-response.go b/cmd/api-response.go index 0be2d8352..f77ae5c57 100644 --- a/cmd/api-response.go +++ b/cmd/api-response.go @@ -640,3 +640,38 @@ 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("Retry-After", "120") + case "AccessDenied": + // The request is from browser and also if browser + // is enabled we need to redirect. + if browser && globalIsBrowserEnabled { + w.Header().Set("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(responseRequestIDKey), + HostID: w.Header().Get(responseDeploymentIDKey), + } + + encodedErrorResponse := encodeResponse(errorResponse) + writeResponse(w, err.HTTPStatusCode, encodedErrorResponse, mimeXML) +} diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index a4b15491b..7fc55cd80 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -595,8 +595,8 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h } // Make sure formValues adhere to policy restrictions. - if errCode = checkPostPolicy(formValues, postPolicyForm); errCode != ErrNone { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(errCode), r.URL, guessIsBrowserReq(r)) + if err = checkPostPolicy(formValues, postPolicyForm); err != nil { + writeCustomErrorResponseXML(ctx, w, errorCodes.ToAPIErr(ErrAccessDenied), err.Error(), r.URL, guessIsBrowserReq(r)) return } diff --git a/cmd/post-policy_test.go b/cmd/post-policy_test.go index 490e75fd6..6473725c9 100644 --- a/cmd/post-policy_test.go +++ b/cmd/post-policy_test.go @@ -300,7 +300,7 @@ func testPostPolicyBucketHandler(obj ObjectLayer, instanceType string, t TestErr { objectName: "test", data: []byte("Hello, World"), - expectedRespStatus: http.StatusBadRequest, + expectedRespStatus: http.StatusForbidden, accessKey: credentials.AccessKey, secretKey: credentials.SecretKey, dates: []interface{}{curTime.Add(-1 * time.Minute * 5).Format(expirationDateFormat), curTime.Format(iso8601DateFormat), curTime.Format(yyyymmdd)}, diff --git a/cmd/postpolicyform.go b/cmd/postpolicyform.go index 8c8c1719d..246a8059a 100644 --- a/cmd/postpolicyform.go +++ b/cmd/postpolicyform.go @@ -217,10 +217,10 @@ func checkPolicyCond(op string, input1, input2 string) bool { // checkPostPolicy - apply policy conditions and validate input values. // (http://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-HTTPPOSTConstructPolicy.html) -func checkPostPolicy(formValues http.Header, postPolicyForm PostPolicyForm) APIErrorCode { +func checkPostPolicy(formValues http.Header, postPolicyForm PostPolicyForm) error { // Check if policy document expiry date is still not reached if !postPolicyForm.Expiration.After(UTCNow()) { - return ErrPolicyAlreadyExpired + return fmt.Errorf("Invalid according to Policy: Policy expired") } // Flag to indicate if all policies conditions are satisfied @@ -237,22 +237,23 @@ func checkPostPolicy(formValues http.Header, postPolicyForm PostPolicyForm) APIE if startsWithSupported, condFound := startsWithConds[cond]; condFound { // Check if the current condition supports starts-with operator if op == policyCondStartsWith && !startsWithSupported { - return ErrAccessDenied + return fmt.Errorf("Invalid according to Policy: Policy Condition failed") } // Check if current policy condition is satisfied - condPassed = checkPolicyCond(op, formValues.Get(formCanonicalName), v.Value) + if !condPassed { + return fmt.Errorf("Invalid according to Policy: Policy Condition failed") + } } else { // This covers all conditions X-Amz-Meta-* and X-Amz-* if strings.HasPrefix(cond, "$x-amz-meta-") || strings.HasPrefix(cond, "$x-amz-") { // Check if policy condition is satisfied condPassed = checkPolicyCond(op, formValues.Get(formCanonicalName), v.Value) + if !condPassed { + return fmt.Errorf("Invalid according to Policy: Policy Condition failed: [%s, %s, %s]", op, cond, v.Value) + } } } - // Check if current policy condition is satisfied, quit immediately otherwise - if !condPassed { - return ErrAccessDenied - } } - return ErrNone + return nil } diff --git a/cmd/postpolicyform_test.go b/cmd/postpolicyform_test.go index 26cb185a9..170a9ae0f 100644 --- a/cmd/postpolicyform_test.go +++ b/cmd/postpolicyform_test.go @@ -18,6 +18,7 @@ package cmd import ( "encoding/base64" + "fmt" "net/http" "testing" @@ -45,26 +46,28 @@ func TestPostPolicyForm(t *testing.T) { SuccessActionStatus string Policy string Expired bool - ErrCode APIErrorCode + expectedErr error } testCases := []testCase{ // Everything is fine with this test - {Bucket: "testbucket", Key: "user/user1/filename/${filename}/myfile.txt", XAmzMetaUUID: "14365123651274", SuccessActionStatus: "201", XAmzCredential: "KVGKMDUQ23TCZXTLTHLP/20160727/us-east-1/s3/aws4_request", XAmzDate: "20160727T000000Z", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "image/jpeg", ErrCode: ErrNone}, + {Bucket: "testbucket", Key: "user/user1/filename/${filename}/myfile.txt", XAmzMetaUUID: "14365123651274", SuccessActionStatus: "201", XAmzCredential: "KVGKMDUQ23TCZXTLTHLP/20160727/us-east-1/s3/aws4_request", XAmzDate: "20160727T000000Z", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "image/jpeg", expectedErr: nil}, // Expired policy document - {Bucket: "testbucket", Key: "user/user1/filename/${filename}/myfile.txt", XAmzMetaUUID: "14365123651274", SuccessActionStatus: "201", XAmzCredential: "KVGKMDUQ23TCZXTLTHLP/20160727/us-east-1/s3/aws4_request", XAmzDate: "20160727T000000Z", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "image/jpeg", Expired: true, ErrCode: ErrPolicyAlreadyExpired}, + {Bucket: "testbucket", Key: "user/user1/filename/${filename}/myfile.txt", XAmzMetaUUID: "14365123651274", SuccessActionStatus: "201", XAmzCredential: "KVGKMDUQ23TCZXTLTHLP/20160727/us-east-1/s3/aws4_request", XAmzDate: "20160727T000000Z", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "image/jpeg", Expired: true, expectedErr: fmt.Errorf("Invalid according to Policy: Policy expired")}, // Different AMZ date - {Bucket: "testbucket", Key: "user/user1/filename/${filename}/myfile.txt", XAmzMetaUUID: "14365123651274", XAmzDate: "20160727T000000Z", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "image/jpeg", ErrCode: ErrAccessDenied}, + {Bucket: "testbucket", Key: "user/user1/filename/${filename}/myfile.txt", XAmzMetaUUID: "14365123651274", XAmzDate: "2017T000000Z", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "image/jpeg", expectedErr: fmt.Errorf("Invalid according to Policy: Policy Condition failed")}, // Key which doesn't start with user/user1/filename - {Bucket: "testbucket", Key: "myfile.txt", XAmzDate: "20160727T000000Z", XAmzMetaUUID: "14365123651274", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "image/jpeg", ErrCode: ErrAccessDenied}, + {Bucket: "testbucket", Key: "myfile.txt", XAmzDate: "20160727T000000Z", XAmzMetaUUID: "14365123651274", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "image/jpeg", expectedErr: fmt.Errorf("Invalid according to Policy: Policy Condition failed")}, // Incorrect bucket name. - {Bucket: "incorrect", Key: "user/user1/filename/myfile.txt", XAmzMetaUUID: "14365123651274", XAmzDate: "20160727T000000Z", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "image/jpeg", ErrCode: ErrAccessDenied}, + {Bucket: "incorrect", Key: "user/user1/filename/myfile.txt", XAmzMetaUUID: "14365123651274", XAmzDate: "20160727T000000Z", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "image/jpeg", expectedErr: fmt.Errorf("Invalid according to Policy: Policy Condition failed")}, // Incorrect key name - {Bucket: "testbucket", Key: "incorrect", XAmzDate: "20160727T000000Z", XAmzMetaUUID: "14365123651274", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "image/jpeg", ErrCode: ErrAccessDenied}, + {Bucket: "testbucket", Key: "incorrect", XAmzDate: "20160727T000000Z", XAmzMetaUUID: "14365123651274", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "image/jpeg", expectedErr: fmt.Errorf("Invalid according to Policy: Policy Condition failed")}, // Incorrect date - {Bucket: "testbucket", Key: "user/user1/filename/${filename}/myfile.txt", XAmzMetaUUID: "14365123651274", XAmzDate: "incorrect", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "image/jpeg", ErrCode: ErrAccessDenied}, + {Bucket: "testbucket", Key: "user/user1/filename/${filename}/myfile.txt", XAmzMetaUUID: "14365123651274", XAmzDate: "incorrect", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "image/jpeg", expectedErr: fmt.Errorf("Invalid according to Policy: Policy Condition failed")}, // Incorrect ContentType - {Bucket: "testbucket", Key: "user/user1/filename/${filename}/myfile.txt", XAmzMetaUUID: "14365123651274", XAmzDate: "20160727T000000Z", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "incorrect", ErrCode: ErrAccessDenied}, + {Bucket: "testbucket", Key: "user/user1/filename/${filename}/myfile.txt", XAmzMetaUUID: "14365123651274", XAmzDate: "20160727T000000Z", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "incorrect", expectedErr: fmt.Errorf("Invalid according to Policy: Policy Condition failed")}, + // Incorrect Metadata + {Bucket: "testbucket", Key: "user/user1/filename/${filename}/myfile.txt", XAmzMetaUUID: "151274", SuccessActionStatus: "201", XAmzCredential: "KVGKMDUQ23TCZXTLTHLP/20160727/us-east-1/s3/aws4_request", XAmzDate: "20160727T000000Z", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "image/jpeg", expectedErr: fmt.Errorf("Invalid according to Policy: Policy Condition failed: [eq, $x-amz-meta-uuid, 14365123651274]")}, } // Validate all the test cases. for i, tt := range testCases { @@ -96,8 +99,9 @@ func TestPostPolicyForm(t *testing.T) { t.Fatal(err) } - if errCode := checkPostPolicy(formValues, postPolicyForm); tt.ErrCode != errCode { - t.Fatalf("Test %d:, Expected %d, got %d", i+1, tt.ErrCode, errCode) + err = checkPostPolicy(formValues, postPolicyForm) + if err != nil && tt.expectedErr != nil && err.Error() != tt.expectedErr.Error() { + t.Fatalf("Test %d:, Expected %s, got %s", i+1, tt.expectedErr.Error(), err.Error()) } } }