From 35e541e0b10836067dabf1c8f1b7b77677461cb7 Mon Sep 17 00:00:00 2001 From: Krishna Srinivas Date: Tue, 25 Oct 2016 12:17:03 +0530 Subject: [PATCH] content-length-range policy should be honored for the uploaded object sizes. (#3076) --- cmd/bucket-handlers.go | 27 ++++++++++++++++++++- cmd/handler-utils.go | 2 +- cmd/object-errors.go | 14 +++++++++++ cmd/object-utils.go | 7 +++++- cmd/object-utils_test.go | 11 +++++---- cmd/signature-v4-postpolicyform.go | 32 ++++++++----------------- cmd/signature-v4-postpolicyform_test.go | 17 +++++++++---- cmd/typed-errors.go | 3 +++ 8 files changed, 79 insertions(+), 34 deletions(-) diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index fb91ebf0c..9a801d911 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -17,6 +17,7 @@ package cmd import ( + "encoding/base64" "encoding/xml" "io" "net/http" @@ -450,11 +451,35 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h writeErrorResponse(w, r, apiErr, r.URL.Path) return } - if apiErr = checkPostPolicy(formValues); apiErr != ErrNone { + + policyBytes, err := base64.StdEncoding.DecodeString(formValues["Policy"]) + if err != nil { + writeErrorResponse(w, r, ErrMalformedPOSTRequest, r.URL.Path) + return + } + + postPolicyForm, err := parsePostPolicyFormV4(string(policyBytes)) + if err != nil { + writeErrorResponse(w, r, ErrMalformedPOSTRequest, r.URL.Path) + return + } + + // Make sure formValues adhere to policy restrictions. + if apiErr = checkPostPolicy(formValues, postPolicyForm); apiErr != ErrNone { writeErrorResponse(w, r, apiErr, r.URL.Path) return } + // Use limitReader to ensure that object size is within expected range. + lengthRange := postPolicyForm.Conditions.ContentLengthRange + if lengthRange.Valid { + // If policy restricted the size of the object. + fileBody = limitReader(fileBody, int64(lengthRange.Min), int64(lengthRange.Max)) + } else { + // Default values of min/max size of the object. + fileBody = limitReader(fileBody, 0, maxObjectSize) + } + // Save metadata. metadata := make(map[string]string) // Nothing to store right now. diff --git a/cmd/handler-utils.go b/cmd/handler-utils.go index bee01ee27..a087e1b40 100644 --- a/cmd/handler-utils.go +++ b/cmd/handler-utils.go @@ -114,7 +114,7 @@ func extractPostPolicyFormValues(reader *multipart.Reader) (filePart io.Reader, } formValues[canonicalFormName] = string(buffer) } else { - filePart = limitReader(part, maxObjectSize) + filePart = part fileName = part.FileName() // As described in S3 spec, we expect file to be the last form field break diff --git a/cmd/object-errors.go b/cmd/object-errors.go index aa3657230..6e7c524a7 100644 --- a/cmd/object-errors.go +++ b/cmd/object-errors.go @@ -81,6 +81,13 @@ func toObjectErr(err error, params ...string) error { Object: params[1], } } + case errDataTooSmall: + if len(params) >= 2 { + err = ObjectTooSmall{ + Bucket: params[0], + Object: params[1], + } + } case errXLReadQuorum: err = InsufficientReadQuorum{} case errXLWriteQuorum: @@ -266,6 +273,13 @@ func (e ObjectTooLarge) Error() string { return "size of the object greater than what is allowed(5G)" } +// ObjectTooSmall error returned when the size of the object < what is expected. +type ObjectTooSmall GenericError + +func (e ObjectTooSmall) Error() string { + return "size of the object less than what is expected" +} + /// Multipart related errors. // MalformedUploadID malformed upload id. diff --git a/cmd/object-utils.go b/cmd/object-utils.go index 85aec3474..d43ed80e3 100644 --- a/cmd/object-utils.go +++ b/cmd/object-utils.go @@ -169,17 +169,22 @@ func (d byBucketName) Less(i, j int) bool { return d[i].Name < d[j].Name } // The underlying implementation is a *LimitedReader. type limitedReader struct { R io.Reader // underlying reader + M int64 // min bytes remaining N int64 // max bytes remaining } -func limitReader(r io.Reader, n int64) io.Reader { return &limitedReader{r, n} } +func limitReader(r io.Reader, m, n int64) io.Reader { return &limitedReader{r, m, n} } func (l *limitedReader) Read(p []byte) (n int, err error) { n, err = l.R.Read(p) l.N -= int64(n) + l.M -= int64(n) if l.N < 0 { // If more data is available than what is expected we return error. return 0, errDataTooLarge } + if err == io.EOF && l.M > 0 { + return 0, errDataTooSmall + } return } diff --git a/cmd/object-utils_test.go b/cmd/object-utils_test.go index b57d0960e..3df4823fe 100644 --- a/cmd/object-utils_test.go +++ b/cmd/object-utils_test.go @@ -120,17 +120,20 @@ func TestIsValidObjectName(t *testing.T) { func TestLimitReader(t *testing.T) { testCases := []struct { data string + minLen int64 maxLen int64 err error }{ - {"1234567890", 15, nil}, - {"1234567890", 10, nil}, - {"1234567890", 5, errDataTooLarge}, + {"1234567890", 0, 15, nil}, + {"1234567890", 0, 10, nil}, + {"1234567890", 0, 5, errDataTooLarge}, + {"123", 5, 10, errDataTooSmall}, + {"123", 2, 10, nil}, } for i, test := range testCases { r := strings.NewReader(test.data) - _, err := ioutil.ReadAll(limitReader(r, test.maxLen)) + _, err := ioutil.ReadAll(limitReader(r, test.minLen, test.maxLen)) if err != test.err { t.Fatalf("test %d failed: expected %v, got %v", i+1, test.err, err) } diff --git a/cmd/signature-v4-postpolicyform.go b/cmd/signature-v4-postpolicyform.go index 4f7011535..ad89a9548 100644 --- a/cmd/signature-v4-postpolicyform.go +++ b/cmd/signature-v4-postpolicyform.go @@ -17,7 +17,6 @@ package cmd import ( - "encoding/base64" "encoding/json" "fmt" "reflect" @@ -52,6 +51,13 @@ func isString(val interface{}) bool { return false } +// ContentLengthRange - policy content-length-range field. +type contentLengthRange struct { + Min int + Max int + Valid bool // If content-length-range was part of policy +} + // PostPolicyForm provides strict static type conversion and validation for Amazon S3's POST policy JSON string. type PostPolicyForm struct { Expiration time.Time // Expiration date and time of the POST policy. @@ -60,10 +66,7 @@ type PostPolicyForm struct { Operator string Value string } - ContentLengthRange struct { - Min int - Max int - } + ContentLengthRange contentLengthRange } } @@ -133,13 +136,7 @@ func parsePostPolicyFormV4(policy string) (PostPolicyForm, error) { Value: value, } case "content-length-range": - parsedPolicy.Conditions.ContentLengthRange = struct { - Min int - Max int - }{ - Min: toInteger(condt[1]), - Max: toInteger(condt[2]), - } + parsedPolicy.Conditions.ContentLengthRange = contentLengthRange{toInteger(condt[1]), toInteger(condt[2]), true} default: // Condition should be valid. return parsedPolicy, fmt.Errorf("Unknown type %s of conditional field value %s found in POST policy form.", reflect.TypeOf(condt).String(), condt) @@ -152,16 +149,7 @@ func parsePostPolicyFormV4(policy string) (PostPolicyForm, error) { } // checkPostPolicy - apply policy conditions and validate input values. -func checkPostPolicy(formValues map[string]string) APIErrorCode { - /// Decoding policy - policyBytes, err := base64.StdEncoding.DecodeString(formValues["Policy"]) - if err != nil { - return ErrMalformedPOSTRequest - } - postPolicyForm, err := parsePostPolicyFormV4(string(policyBytes)) - if err != nil { - return ErrMalformedPOSTRequest - } +func checkPostPolicy(formValues map[string]string, postPolicyForm PostPolicyForm) APIErrorCode { if !postPolicyForm.Expiration.After(time.Now().UTC()) { return ErrPolicyAlreadyExpired } diff --git a/cmd/signature-v4-postpolicyform_test.go b/cmd/signature-v4-postpolicyform_test.go index b49e06d48..a74f3ae51 100644 --- a/cmd/signature-v4-postpolicyform_test.go +++ b/cmd/signature-v4-postpolicyform_test.go @@ -17,6 +17,7 @@ package cmd import ( + "encoding/base64" "testing" ) @@ -33,8 +34,6 @@ func TestPostPolicyForm(t *testing.T) { ErrCode APIErrorCode } testCases := []testCase{ - // Mal formed policy document - {Bucket: "testbucket", Key: "user/user1/filename/${filename}/myfile.txt", XAmzDate: "20160727T000000Z", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "image/jpeg", Policy: "==", ErrCode: ErrMalformedPOSTRequest}, // Different AMZ date {Bucket: "testbucket", Key: "user/user1/filename/${filename}/myfile.txt", XAmzDate: "20160727T000000Z", XAmzAlgorithm: "AWS4-HMAC-SHA256", ContentType: "image/jpeg", Policy: "eyAiZXhwaXJhdGlvbiI6ICIyMDE3LTEyLTMwVDEyOjAwOjAwLjAwMFoiLCAiY29uZGl0aW9ucyI6IFsgeyJidWNrZXQiOiAidGVzdGJ1Y2tldCJ9LCBbInN0YXJ0cy13aXRoIiwgIiRrZXkiLCAidXNlci91c2VyMS9maWxlbmFtZSJdLCB7ImFjbCI6ICJwdWJsaWMtcmVhZCJ9LCB7InN1Y2Nlc3NfYWN0aW9uX3JlZGlyZWN0IjogImh0dHA6Ly8xMjcuMC4wLjE6OTAwMC8ifSwgWyJzdGFydHMtd2l0aCIsICIkQ29udGVudC1UeXBlIiwgImltYWdlLyJdLCB7IngtYW16LW1ldGEtdXVpZCI6ICIxNDM2NTEyMzY1MTI3NCJ9LCB7IngtYW16LXNlcnZlci1zaWRlLWVuY3J5cHRpb24iOiAiQUVTMjU2In0sIFsic3RhcnRzLXdpdGgiLCAiJHgtYW16LW1ldGEtdGFnIiwgIiJdLCB7IngtYW16LWNyZWRlbnRpYWwiOiAiS1ZHS01EVVEyM1RDWlhUTFRITFAvMjAxNjA3MjcvdXMtZWFzdC0xL3MzL2F3czRfcmVxdWVzdCJ9LCB7IngtYW16LWFsZ29yaXRobSI6ICJBV1M0LUhNQUMtU0hBMjU2In0sIHsieC1hbXotZGF0ZSI6ICIyMDIwMDcyN1QwMDAwMDBaIiB9IF0gfQ==", ErrCode: ErrAccessDenied}, // Key which doesn't start with user/user1/filename @@ -51,10 +50,18 @@ func TestPostPolicyForm(t *testing.T) { formValues["X-Amz-Algorithm"] = tt.XAmzAlgorithm formValues["Content-Type"] = tt.ContentType formValues["Policy"] = tt.Policy + policyBytes, err := base64.StdEncoding.DecodeString(tt.Policy) + if err != nil { + t.Fatal(err) + } - err := checkPostPolicy(formValues) - if tt.ErrCode != err { - t.Errorf("Test %d:, Expected %d, got %d", i+1, tt.ErrCode, err) + postPolicyForm, err := parsePostPolicyFormV4(string(policyBytes)) + if err != nil { + t.Fatal(err) + } + errCode := checkPostPolicy(formValues, postPolicyForm) + if tt.ErrCode != errCode { + t.Errorf("Test %d:, Expected %d, got %d", i+1, tt.ErrCode, errCode) } } } diff --git a/cmd/typed-errors.go b/cmd/typed-errors.go index 332930eb8..a41693674 100644 --- a/cmd/typed-errors.go +++ b/cmd/typed-errors.go @@ -38,3 +38,6 @@ var errSizeUnexpected = errors.New("Data size larger than expected") // When upload object size is greater than 5G in a single PUT/POST operation. var errDataTooLarge = errors.New("Object size larger than allowed limit") + +// When upload object size is less than what was expected. +var errDataTooSmall = errors.New("Object size smaller than expected")