api: xmlDecoder should honor contentLength. (#2226)

This is needed so that we avoid reading large amounts
of data from compromised clients.

bucket-handlers.go

@@ -322,10 +322,9 @@ func (api objectAPIHandlers) PutBucketHandler(w http.ResponseWriter, r *http.Req
 		}
 	}
 
-	// the location value in the request body should match the Region in serverConfig.
-	// other values of location are not accepted.
-	// make bucket fails in such cases.
-	errCode := isValidLocationContraint(r.Body, serverConfig.GetRegion())
+	// Validate if incoming location constraint is valid, reject
+	// requests which do not follow valid region requirements.
+	errCode := isValidLocationConstraint(r)
 	if errCode != ErrNone {
 		writeErrorResponse(w, r, errCode, r.URL.Path)
 		return

handler-utils.go

@@ -18,37 +18,43 @@ package main
 
 import (
 	"io"
+	"net/http"
 )
 
-// validates location constraint from the request body.
-// the location value in the request body should match the Region in serverConfig.
-// other values of location are not accepted.
-// make bucket fails in such cases.
-func isValidLocationContraint(reqBody io.Reader, serverRegion string) APIErrorCode {
-	var locationContraint createBucketLocationConfiguration
-	var errCode APIErrorCode
-	errCode = ErrNone
-	e := xmlDecoder(reqBody, &locationContraint)
-	if e != nil {
-		if e == io.EOF {
-			// Do nothing.
-			// failed due to empty body. The location will be set to default value from the serverConfig.
-			// this is valid.
-			errCode = ErrNone
-		} else {
-			// Failed due to malformed configuration.
-			errCode = ErrMalformedXML
-			//writeErrorResponse(w, r, ErrMalformedXML, r.URL.Path)
-		}
-	} else {
-		// Region obtained from the body.
-		// It should be equal to Region in serverConfig.
-		// Else ErrInvalidRegion returned.
-		// For empty value location will be to set to  default value from the serverConfig.
-		if locationContraint.Location != "" && serverRegion != locationContraint.Location {
-			//writeErrorResponse(w, r, ErrInvalidRegion, r.URL.Path)
-			errCode = ErrInvalidRegion
-		}
+// Validates location constraint in PutBucket request body.
+// The location value in the request body should match the
+// region configured at serverConfig, otherwise error is returned.
+func isValidLocationConstraint(r *http.Request) (s3Error APIErrorCode) {
+	serverRegion := serverConfig.GetRegion()
+	// If the request has no body with content-length set to 0,
+	// we do not have to validate location constraint. Bucket will
+	// be created at default region.
+	if r.ContentLength == 0 {
+		return ErrNone
 	}
-	return errCode
+	locationConstraint := createBucketLocationConfiguration{}
+	if err := xmlDecoder(r.Body, &locationConstraint, r.ContentLength); err != nil {
+		if err == io.EOF && r.ContentLength == -1 {
+			// EOF is a valid condition here when ContentLength is -1.
+			return ErrNone
+		}
+		errorIf(err, "Unable to xml decode location constraint")
+		// Treat all other failures as XML parsing errors.
+		return ErrMalformedXML
+	} // Successfully decoded, proceed to verify the region.
+
+	// Once region has been obtained we proceed to verify it.
+	incomingRegion := locationConstraint.Location
+	if incomingRegion == "" {
+		// Location constraint is empty for region "us-east-1",
+		// in accordance with protocol.
+		incomingRegion = "us-east-1"
+	}
+	// Return errInvalidRegion if location constraint does not match
+	// with configured region.
+	s3Error = ErrNone
+	if serverRegion != incomingRegion {
+		s3Error = ErrInvalidRegion
+	}
+	return s3Error
 }

handler-utils_test.go

@@ -26,6 +26,24 @@ import (
 
 // Tests validate bucket LocationConstraint.
 func TestIsValidLocationContraint(t *testing.T) {
+	savedServerConfig := serverConfig
+	defer func() {
+		serverConfig = savedServerConfig
+	}()
+	serverConfig = nil
+
+	// Test initialized config file.
+	path, err := ioutil.TempDir("", "minio-")
+	if err != nil {
+		t.Fatalf("Unable to create a temporary directory, %s", err)
+	}
+	defer removeAll(path)
+
+	setGlobalConfigPath(path)
+	if err := initConfig(); err != nil {
+		t.Fatalf("unable initialize config file, %s", err)
+	}
+
 	// generates the input request with XML bucket configuration set to the request body.
 	createExpectedRequest := func(req *http.Request, location string) (*http.Request, error) {
 		createBucketConfig := createBucketLocationConfiguration{}
@@ -37,6 +55,7 @@ func TestIsValidLocationContraint(t *testing.T) {
 		}
 		createBucketConfigBuffer := bytes.NewBuffer(createBucketConfigBytes)
 		req.Body = ioutil.NopCloser(createBucketConfigBuffer)
+		req.ContentLength = int64(createBucketConfigBuffer.Len())
 		return req, nil
 	}
 
@@ -58,7 +77,8 @@ func TestIsValidLocationContraint(t *testing.T) {
 		if e != nil {
 			t.Fatalf("Test %d: Failed to Marshal bucket configuration", i+1)
 		}
-		actualCode := isValidLocationContraint(inputRequest.Body, testCase.serverConfigRegion)
+		serverConfig.SetRegion(testCase.serverConfigRegion)
+		actualCode := isValidLocationConstraint(inputRequest)
 		if testCase.expectedCode != actualCode {
 			t.Errorf("Test %d: Expected the APIErrCode to be %d, but instead found %d", i+1, testCase.expectedCode, actualCode)
 		}

utils.go

@@ -24,8 +24,14 @@ import (
 )
 
 // xmlDecoder provide decoded value in xml.
-func xmlDecoder(body io.Reader, v interface{}) error {
-	d := xml.NewDecoder(body)
+func xmlDecoder(body io.Reader, v interface{}, size int64) error {
+	var lbody io.Reader
+	if size > 0 {
+		lbody = io.LimitReader(body, size)
+	} else {
+		lbody = body
+	}
+	d := xml.NewDecoder(lbody)
 	return d.Decode(v)
 }