ensure authenticated request bodies for Admin-API (#5984)

This commit adds a check to the server's admin-API such that it only
accepts Admin-API requests with authenticated bodies. Further this
commit updates the `madmin` package to always add the
`X-Amz-Content-Sha256` header.

This change improves the Admin-API security since the server does not
accept unauthenticated request bodies anymore.

After this commit `mc` must be updated to the new `madmin` api because
requests over TLS connections will fail.

cmd/auth-handler.go

@@ -114,7 +114,7 @@ func getRequestAuthType(r *http.Request) authType {
 // It does not accept presigned or JWT or anonymous requests.
 func checkAdminRequestAuthType(r *http.Request, region string) APIErrorCode {
 	s3Err := ErrAccessDenied
-	if getRequestAuthType(r) == authTypeSigned { // we only support V4 (no presign)
+	if _, ok := r.Header["X-Amz-Content-Sha256"]; ok && getRequestAuthType(r) == authTypeSigned && !skipContentSha256Cksum(r) { // we only support V4 (no presign) with auth. body
 		s3Err = isReqAuthenticated(r, region)
 	}
 	if s3Err != ErrNone {

pkg/madmin/api.go

@@ -18,7 +18,6 @@ package madmin
 
 import (
 	"bytes"
-	"encoding/base64"
 	"encoding/hex"
 	"fmt"
 	"io"
@@ -177,14 +176,8 @@ func (c *AdminClient) TraceOff() {
 type requestData struct {
 	customHeaders http.Header
 	queryValues   url.Values
-
+	relPath       string // Url path relative to admin API base endpoint
-	// Url path relative to admin API base endpoint
+	content       []byte
-	relPath string
-
-	contentBody        io.Reader
-	contentLength      int64
-	contentSHA256Bytes []byte
-	contentMD5Bytes    []byte
 }
 
 // Filter out signature value from Authorization header.
@@ -404,43 +397,17 @@ func (c AdminClient) newRequest(method string, reqData requestData) (req *http.R
 		return nil, err
 	}
 
-	// Set content body if available.
-	if reqData.contentBody != nil {
-		req.Body = ioutil.NopCloser(reqData.contentBody)
-	}
-
-	// Set 'User-Agent' header for the request.
 	c.setUserAgent(req)
-
-	// Set all headers.
 	for k, v := range reqData.customHeaders {
 		req.Header.Set(k, v[0])
 	}
-
+	if length := len(reqData.content); length > 0 {
-	// set incoming content-length.
+		req.ContentLength = int64(length)
-	if reqData.contentLength > 0 {
-		req.ContentLength = reqData.contentLength
 	}
+	req.Header.Set("X-Amz-Content-Sha256", hex.EncodeToString(sum256(reqData.content)))
+	req.Body = ioutil.NopCloser(bytes.NewReader(reqData.content))
 
-	shaHeader := unsignedPayload
-	if !c.secure {
-		if reqData.contentSHA256Bytes == nil {
-			shaHeader = hex.EncodeToString(sum256([]byte{}))
-		} else {
-			shaHeader = hex.EncodeToString(reqData.contentSHA256Bytes)
-		}
-	}
-	req.Header.Set("X-Amz-Content-Sha256", shaHeader)
-
-	// set md5Sum for content protection.
-	if reqData.contentMD5Bytes != nil {
-		req.Header.Set("Content-Md5", base64.StdEncoding.EncodeToString(reqData.contentMD5Bytes))
-	}
-
-	// Add signature version '4' authorization header.
 	req = s3signer.SignV4(*req, c.accessKeyID, c.secretAccessKey, "", location)
-
-	// Return request.
 	return req, nil
 }
 

pkg/madmin/config-commands.go

@@ -18,7 +18,6 @@
 package madmin
 
 import (
-	"bytes"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -66,8 +65,7 @@ func (adm *AdminClient) GetConfig() ([]byte, error) {
 
 // SetConfig - set config supplied as config.json for the setup.
 func (adm *AdminClient) SetConfig(config io.Reader) (r SetConfigResult, err error) {
-	// No TLS?
+	if !adm.secure { // No TLS?
-	if !adm.secure {
 		return r, fmt.Errorf("credentials/configuration cannot be updated over an insecure connection")
 	}
 
@@ -78,10 +76,8 @@ func (adm *AdminClient) SetConfig(config io.Reader) (r SetConfigResult, err erro
 	}
 
 	reqData := requestData{
-		relPath:            "/v1/config",
+		relPath: "/v1/config",
-		contentBody:        bytes.NewReader(configBytes),
+		content: configBytes,
-		contentMD5Bytes:    sumMD5(configBytes),
-		contentSHA256Bytes: sum256(configBytes),
 	}
 
 	// Execute PUT on /minio/admin/v1/config to set config.

pkg/madmin/generic-commands.go

@@ -18,7 +18,6 @@
 package madmin
 
 import (
-	"bytes"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -46,11 +45,8 @@ func (adm *AdminClient) SetCredentials(access, secret string) error {
 
 	// Setup new request
 	reqData := requestData{
-		relPath:            "/v1/config/credential",
+		relPath: "/v1/config/credential",
-		contentBody:        bytes.NewReader(body),
+		content: body,
-		contentLength:      int64(len(body)),
-		contentMD5Bytes:    sumMD5(body),
-		contentSHA256Bytes: sum256(body),
 	}
 
 	// Execute GET on bucket to list objects.

pkg/madmin/heal-commands.go

@@ -18,10 +18,8 @@
 package madmin
 
 import (
-	"bytes"
 	"encoding/json"
 	"fmt"
-	"io"
 	"io/ioutil"
 	"net/http"
 	"net/url"
@@ -194,23 +192,18 @@ func (adm *AdminClient) Heal(bucket, prefix string, healOpts HealOpts,
 
 	// execute POST request to heal api
 	queryVals := make(url.Values)
-	var contentBody io.Reader
 	if clientToken != "" {
 		queryVals.Set("clientToken", clientToken)
 		body = []byte{}
-	} else {
-		// Set a body only if clientToken is not given
-		contentBody = bytes.NewReader(body)
 	}
 	if forceStart {
 		queryVals.Set("forceStart", "true")
 	}
 
 	resp, err := adm.executeMethod("POST", requestData{
-		relPath:            path,
+		relPath:     path,
-		contentBody:        contentBody,
+		content:     body,
-		contentSHA256Bytes: sum256(body),
+		queryValues: queryVals,
-		queryValues:        queryVals,
 	})
 	defer closeResponse(resp)
 	if err != nil {

pkg/madmin/service-commands.go

@@ -18,7 +18,6 @@
 package madmin
 
 import (
-	"bytes"
 	"encoding/json"
 	"io/ioutil"
 	"net/http"
@@ -87,9 +86,8 @@ func (adm *AdminClient) ServiceSendAction(action ServiceActionValue) error {
 
 	// Request API to Restart server
 	resp, err := adm.executeMethod("POST", requestData{
-		relPath:            "/v1/service",
+		relPath: "/v1/service",
-		contentBody:        bytes.NewReader(body),
+		content: body,
-		contentSHA256Bytes: sum256(body),
 	})
 	defer closeResponse(resp)
 	if err != nil {