diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index db942d755..e6f5c6871 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -393,6 +393,13 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h return } + // Require Content-Length to be set in the request + size := r.ContentLength + if size < 0 { + writeErrorResponse(w, ErrMissingContentLength, r.URL) + return + } + // Here the parameter is the size of the form data that should // be loaded in memory, the remaining being put in temporary files. reader, err := r.MultipartReader() @@ -402,12 +409,28 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h return } - fileBody, fileName, formValues, err := extractPostPolicyFormValues(reader) + // Read multipart data and save in memory and in the disk if needed + form, err := reader.ReadForm(maxFormMemory) + if err != nil { + errorIf(err, "Unable to initialize multipart reader.") + writeErrorResponse(w, ErrMalformedPOSTRequest, r.URL) + return + } + + // Remove all tmp files creating during multipart upload + defer form.RemoveAll() + + // Extract all form fields + fileBody, fileName, fileSize, formValues, err := extractPostPolicyFormValues(form) if err != nil { errorIf(err, "Unable to parse form values.") writeErrorResponse(w, ErrMalformedPOSTRequest, r.URL) return } + + // Close multipart file + defer fileBody.Close() + bucket := mux.Vars(r)["bucket"] formValues["Bucket"] = bucket @@ -443,21 +466,20 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h return } - // Use rangeReader to ensure that object size is within expected range. + // Ensure that the object size is within expected range, also the file size + // should not exceed the maximum single Put size (5 GiB) lengthRange := postPolicyForm.Conditions.ContentLengthRange if lengthRange.Valid { - // If policy restricted the size of the object. - fileBody = &rangeReader{ - Reader: fileBody, - Min: lengthRange.Min, - Max: lengthRange.Max, + if fileSize < lengthRange.Min { + errorIf(err, "Unable to create object.") + writeErrorResponse(w, toAPIErrorCode(errDataTooSmall), r.URL) + return } - } else { - // Default values of min/max size of the object. - fileBody = &rangeReader{ - Reader: fileBody, - Min: 0, - Max: maxObjectSize, + + if fileSize > lengthRange.Max || fileSize > maxObjectSize { + errorIf(err, "Unable to create object.") + writeErrorResponse(w, toAPIErrorCode(errDataTooLarge), r.URL) + return } } @@ -470,7 +492,7 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h objectLock.Lock() defer objectLock.Unlock() - objInfo, err := objectAPI.PutObject(bucket, object, -1, fileBody, metadata, sha256sum) + objInfo, err := objectAPI.PutObject(bucket, object, fileSize, fileBody, metadata, sha256sum) if err != nil { errorIf(err, "Unable to create object.") writeErrorResponse(w, toAPIErrorCode(err), r.URL) diff --git a/cmd/globals.go b/cmd/globals.go index f2ed99dd9..be77a78db 100644 --- a/cmd/globals.go +++ b/cmd/globals.go @@ -59,6 +59,9 @@ const ( // can reach that size according to https://aws.amazon.com/articles/1434 maxFormFieldSize = int64(1 * humanize.MiByte) + // Limit memory allocation to store multipart data + maxFormMemory = int64(5 * humanize.MiByte) + // The maximum allowed difference between the request generation time and the server processing time globalMaxSkewTime = 15 * time.Minute ) diff --git a/cmd/handler-utils.go b/cmd/handler-utils.go index a97f0f192..7b03a7e83 100644 --- a/cmd/handler-utils.go +++ b/cmd/handler-utils.go @@ -18,7 +18,6 @@ package cmd import ( "io" - "io/ioutil" "mime/multipart" "net/http" "strings" @@ -158,33 +157,49 @@ func extractMetadataFromForm(formValues map[string]string) map[string]string { } // Extract form fields and file data from a HTTP POST Policy -func extractPostPolicyFormValues(reader *multipart.Reader) (filePart io.Reader, fileName string, formValues map[string]string, err error) { +func extractPostPolicyFormValues(form *multipart.Form) (filePart io.ReadCloser, fileName string, fileSize int64, formValues map[string]string, err error) { /// HTML Form values formValues = make(map[string]string) fileName = "" - for err == nil { - var part *multipart.Part - part, err = reader.NextPart() - if part != nil { - canonicalFormName := http.CanonicalHeaderKey(part.FormName()) - if canonicalFormName != "File" { - var buffer []byte - limitReader := io.LimitReader(part, maxFormFieldSize+1) - buffer, err = ioutil.ReadAll(limitReader) - if err != nil { - return nil, "", nil, err - } - if int64(len(buffer)) > maxFormFieldSize { - return nil, "", nil, errSizeUnexpected - } - formValues[canonicalFormName] = string(buffer) - } else { - filePart = part - fileName = part.FileName() - // As described in S3 spec, we expect file to be the last form field - break + + // Iterate over form values + for k, v := range form.Value { + canonicalFormName := http.CanonicalHeaderKey(k) + // Check if value's field exceeds S3 limit + if int64(len(v[0])) > maxFormFieldSize { + return nil, "", 0, nil, errSizeUnexpected + } + // Set the form value + formValues[canonicalFormName] = v[0] + } + + // Iterator until we find a valid File field and break + for k, v := range form.File { + canonicalFormName := http.CanonicalHeaderKey(k) + if canonicalFormName == "File" { + if len(v) == 0 { + return nil, "", 0, nil, errInvalidArgument } + // Fetch fileHeader which has the uploaded file information + fileHeader := v[0] + // Set filename + fileName = fileHeader.Filename + // Open the uploaded part + filePart, err = fileHeader.Open() + // Compute file size + fileSize, err = filePart.(io.Seeker).Seek(0, 2) + if err != nil { + return nil, "", 0, nil, err + } + // Reset Seek to the beginning + _, err = filePart.(io.Seeker).Seek(0, 0) + if err != nil { + return nil, "", 0, nil, err + } + // File found and ready for reading + break } } - return filePart, fileName, formValues, nil + + return filePart, fileName, fileSize, formValues, nil } diff --git a/cmd/jwt.go b/cmd/jwt.go index c3d80a08f..f1f0b2373 100644 --- a/cmd/jwt.go +++ b/cmd/jwt.go @@ -106,13 +106,13 @@ func isAuthTokenValid(tokenString string) bool { } func isHTTPRequestValid(req *http.Request) bool { - return webReqestAuthenticate(req) == nil + return webRequestAuthenticate(req) == nil } // Check if the request is authenticated. // Returns nil if the request is authenticated. errNoAuthToken if token missing. // Returns errAuthentication for all other errors. -func webReqestAuthenticate(req *http.Request) error { +func webRequestAuthenticate(req *http.Request) error { jwtToken, err := jwtreq.ParseFromRequest(req, jwtreq.AuthorizationHeaderExtractor, keyFuncCallback) if err != nil { if err == jwtreq.ErrNoTokenInRequest { diff --git a/cmd/post-policy_test.go b/cmd/post-policy_test.go index 60333e3a3..14018979a 100644 --- a/cmd/post-policy_test.go +++ b/cmd/post-policy_test.go @@ -342,46 +342,66 @@ func testPostPolicyBucketHandler(obj ObjectLayer, instanceType string, t TestErr } testCases2 := []struct { - objectName string - data []byte - expectedRespStatus int - accessKey string - secretKey string - malformedBody bool + objectName string + data []byte + expectedRespStatus int + accessKey string + secretKey string + malformedBody bool + ignoreContentLength bool }{ // Success case. { - objectName: "test", - data: bytes.Repeat([]byte("a"), 1025), - expectedRespStatus: http.StatusNoContent, - accessKey: credentials.AccessKey, - secretKey: credentials.SecretKey, - malformedBody: false, + objectName: "test", + data: bytes.Repeat([]byte("a"), 1025), + expectedRespStatus: http.StatusNoContent, + accessKey: credentials.AccessKey, + secretKey: credentials.SecretKey, + malformedBody: false, + ignoreContentLength: false, + }, + // Failed with Content-Length not specified. + { + objectName: "test", + data: bytes.Repeat([]byte("a"), 1025), + expectedRespStatus: http.StatusNoContent, + accessKey: credentials.AccessKey, + secretKey: credentials.SecretKey, + malformedBody: false, + ignoreContentLength: true, }, // Failed with entity too small. { - objectName: "test", - data: bytes.Repeat([]byte("a"), 1023), - expectedRespStatus: http.StatusBadRequest, - accessKey: credentials.AccessKey, - secretKey: credentials.SecretKey, - malformedBody: false, + objectName: "test", + data: bytes.Repeat([]byte("a"), 1023), + expectedRespStatus: http.StatusBadRequest, + accessKey: credentials.AccessKey, + secretKey: credentials.SecretKey, + malformedBody: false, + ignoreContentLength: false, }, // Failed with entity too large. { - objectName: "test", - data: bytes.Repeat([]byte("a"), (1*humanize.MiByte)+1), - expectedRespStatus: http.StatusBadRequest, - accessKey: credentials.AccessKey, - secretKey: credentials.SecretKey, - malformedBody: false, + objectName: "test", + data: bytes.Repeat([]byte("a"), (1*humanize.MiByte)+1), + expectedRespStatus: http.StatusBadRequest, + accessKey: credentials.AccessKey, + secretKey: credentials.SecretKey, + malformedBody: false, + ignoreContentLength: false, }, } for i, testCase := range testCases2 { // initialize HTTP NewRecorder, this records any mutations to response writer inside the handler. rec := httptest.NewRecorder() - req, perr := newPostRequestV4WithContentLength("", bucketName, testCase.objectName, testCase.data, testCase.accessKey, testCase.secretKey) + var req *http.Request + var perr error + if testCase.ignoreContentLength { + req, perr = newPostRequestV4("", bucketName, testCase.objectName, testCase.data, testCase.accessKey, testCase.secretKey) + } else { + req, perr = newPostRequestV4WithContentLength("", bucketName, testCase.objectName, testCase.data, testCase.accessKey, testCase.secretKey) + } if perr != nil { t.Fatalf("Test %d: %s: Failed to create HTTP request for PostPolicyHandler: %v", i+1, instanceType, perr) } diff --git a/cmd/typed-errors.go b/cmd/typed-errors.go index e79ab0dd6..5a7568568 100644 --- a/cmd/typed-errors.go +++ b/cmd/typed-errors.go @@ -33,6 +33,9 @@ var errContentSHA256Mismatch = errors.New("Content checksum SHA256 mismatch") // used when we deal with data larger than expected var errSizeUnexpected = errors.New("Data size larger than expected") +// used when we deal with data with unknown size +var errSizeUnspecified = errors.New("Data size is unspecified") + // When upload object size is greater than 5G in a single PUT/POST operation. var errDataTooLarge = errors.New("Object size larger than allowed limit") diff --git a/cmd/web-handlers.go b/cmd/web-handlers.go index adf9f6db7..4943fa78c 100644 --- a/cmd/web-handlers.go +++ b/cmd/web-handlers.go @@ -151,7 +151,7 @@ func (web *webAPIHandlers) ListBuckets(r *http.Request, args *WebGenericArgs, re if objectAPI == nil { return toJSONError(errServerNotInitialized) } - authErr := webReqestAuthenticate(r) + authErr := webRequestAuthenticate(r) if authErr != nil { return toJSONError(authErr) } @@ -208,7 +208,7 @@ func (web *webAPIHandlers) ListObjects(r *http.Request, args *ListObjectsArgs, r prefix := args.Prefix + "test" // To test if GetObject/PutObject with the specified prefix is allowed. readable := isBucketActionAllowed("s3:GetObject", args.BucketName, prefix) writable := isBucketActionAllowed("s3:PutObject", args.BucketName, prefix) - authErr := webReqestAuthenticate(r) + authErr := webRequestAuthenticate(r) switch { case authErr == errAuthentication: return toJSONError(authErr) @@ -446,7 +446,7 @@ func (web *webAPIHandlers) Upload(w http.ResponseWriter, r *http.Request) { bucket := vars["bucket"] object := vars["object"] - authErr := webReqestAuthenticate(r) + authErr := webRequestAuthenticate(r) if authErr == errAuthentication { writeWebErrorResponse(w, errAuthentication) return @@ -456,6 +456,13 @@ func (web *webAPIHandlers) Upload(w http.ResponseWriter, r *http.Request) { return } + // Require Content-Length to be set in the request + size := r.ContentLength + if size < 0 { + writeWebErrorResponse(w, errSizeUnspecified) + return + } + // Extract incoming metadata if any. metadata := extractMetadataFromHeader(r.Header) @@ -465,7 +472,7 @@ func (web *webAPIHandlers) Upload(w http.ResponseWriter, r *http.Request) { defer objectLock.Unlock() sha256sum := "" - objInfo, err := objectAPI.PutObject(bucket, object, -1, r.Body, metadata, sha256sum) + objInfo, err := objectAPI.PutObject(bucket, object, size, r.Body, metadata, sha256sum) if err != nil { writeWebErrorResponse(w, err) return @@ -816,6 +823,12 @@ func toWebAPIError(err error) APIError { HTTPStatusCode: http.StatusForbidden, Description: err.Error(), } + } else if err == errSizeUnspecified { + return APIError{ + Code: "InvalidRequest", + HTTPStatusCode: http.StatusBadRequest, + Description: err.Error(), + } } // Convert error type to api error code. diff --git a/cmd/web-handlers_test.go b/cmd/web-handlers_test.go index fc72ab9ff..1f8c9e627 100644 --- a/cmd/web-handlers_test.go +++ b/cmd/web-handlers_test.go @@ -738,19 +738,24 @@ func testUploadWebHandler(obj ObjectLayer, instanceType string, t TestErrHandler objectName := "test.file" bucketName := getRandomBucketName() - test := func(token string) int { + test := func(token string, sendContentLength bool) int { rec := httptest.NewRecorder() - var req *http.Request - req, err = http.NewRequest("PUT", "/minio/upload/"+bucketName+"/"+objectName, nil) - if err != nil { - t.Fatalf("Cannot create upload request, %v", err) + req, rErr := http.NewRequest("PUT", "/minio/upload/"+bucketName+"/"+objectName, nil) + if rErr != nil { + t.Fatalf("Cannot create upload request, %v", rErr) } - req.Header.Set("Content-Length", strconv.Itoa(len(content))) req.Header.Set("x-amz-date", "20160814T114029Z") req.Header.Set("Accept", "*/*") + req.Body = ioutil.NopCloser(bytes.NewReader(content)) + if !sendContentLength { + req.ContentLength = -1 + } else { + req.ContentLength = int64(len(content)) + } + if token != "" { req.Header.Set("Authorization", "Bearer "+authorization) } @@ -765,7 +770,7 @@ func testUploadWebHandler(obj ObjectLayer, instanceType string, t TestErrHandler } // Authenticated upload should succeed. - code := test(authorization) + code := test(authorization, true) if code != http.StatusOK { t.Fatalf("Expected the response status to be 200, but instead found `%d`", code) } @@ -780,8 +785,14 @@ func testUploadWebHandler(obj ObjectLayer, instanceType string, t TestErrHandler t.Fatalf("The upload file is different from the download file") } + // Authenticated upload without content-length should fail + code = test(authorization, false) + if code != http.StatusBadRequest { + t.Fatalf("Expected the response status to be 200, but instead found `%d`", code) + } + // Unauthenticated upload should fail. - code = test("") + code = test("", true) if code != http.StatusForbidden { t.Fatalf("Expected the response status to be 403, but instead found `%d`", code) } @@ -794,13 +805,13 @@ func testUploadWebHandler(obj ObjectLayer, instanceType string, t TestErrHandler globalBucketPolicies.SetBucketPolicy(bucketName, policyChange{false, &policy}) // Unauthenticated upload with WRITE policy should succeed. - code = test("") + code = test("", true) if code != http.StatusOK { t.Fatalf("Expected the response status to be 200, but instead found `%d`", code) } } -// Wrapper for calling Upload Handler +// Wrapper for calling Download Handler func TestWebHandlerDownload(t *testing.T) { ExecObjectLayerTest(t, testDownloadWebHandler) }