From 3f09c17bfe5027fb913cb17cc7a07dcdd675b7fe Mon Sep 17 00:00:00 2001 From: Andreas Auernhammer Date: Wed, 17 Jan 2018 19:36:25 +0100 Subject: [PATCH] fix authentication bypass against Admin-API (#5412) This change fixes an authentication bypass attack against the minio Admin-API. Therefore the Admin-API rejects now all types of requests except valid signature V2 and signature V4 requests - this includes signature V2/V4 pre-signed requests. Fixes #5411 --- cmd/admin-handlers.go | 26 ++++++++--------- cmd/auth-handler.go | 14 +++++++++ cmd/auth-handler_test.go | 63 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 13 deletions(-) diff --git a/cmd/admin-handlers.go b/cmd/admin-handlers.go index bc97ec790..b51b885ec 100644 --- a/cmd/admin-handlers.go +++ b/cmd/admin-handlers.go @@ -72,7 +72,7 @@ type ServerStatus struct { // Fetches server status information like total disk space available // to use, online disks, offline disks and quorum threshold. func (adminAPI adminAPIHandlers) ServiceStatusHandler(w http.ResponseWriter, r *http.Request) { - adminAPIErr := checkRequestAuthType(r, "", "", "") + adminAPIErr := checkAdminRequestAuthType(r, "") if adminAPIErr != ErrNone { writeErrorResponse(w, adminAPIErr, r.URL) return @@ -114,7 +114,7 @@ func (adminAPI adminAPIHandlers) ServiceStatusHandler(w http.ResponseWriter, r * // Restarts minio server gracefully. In a distributed setup, restarts // all the servers in the cluster. func (adminAPI adminAPIHandlers) ServiceRestartHandler(w http.ResponseWriter, r *http.Request) { - adminAPIErr := checkRequestAuthType(r, "", "", "") + adminAPIErr := checkAdminRequestAuthType(r, "") if adminAPIErr != ErrNone { writeErrorResponse(w, adminAPIErr, r.URL) return @@ -139,7 +139,7 @@ type setCredsReq struct { // in the cluster. func (adminAPI adminAPIHandlers) ServiceCredentialsHandler(w http.ResponseWriter, r *http.Request) { // Authenticate request - adminAPIErr := checkRequestAuthType(r, "", "", "") + adminAPIErr := checkAdminRequestAuthType(r, "") if adminAPIErr != ErrNone { writeErrorResponse(w, adminAPIErr, r.URL) return @@ -257,7 +257,7 @@ type ServerInfo struct { // Get server information func (adminAPI adminAPIHandlers) ServerInfoHandler(w http.ResponseWriter, r *http.Request) { // Authenticate request - adminAPIErr := checkRequestAuthType(r, "", "", "") + adminAPIErr := checkAdminRequestAuthType(r, "") if adminAPIErr != ErrNone { writeErrorResponse(w, adminAPIErr, r.URL) return @@ -341,7 +341,7 @@ func validateLockQueryParams(vars url.Values) (string, string, time.Duration, AP // --------- // Lists locks held on a given bucket, prefix and duration it was held for. func (adminAPI adminAPIHandlers) ListLocksHandler(w http.ResponseWriter, r *http.Request) { - adminAPIErr := checkRequestAuthType(r, "", "", "") + adminAPIErr := checkAdminRequestAuthType(r, "") if adminAPIErr != ErrNone { writeErrorResponse(w, adminAPIErr, r.URL) return @@ -383,7 +383,7 @@ func (adminAPI adminAPIHandlers) ListLocksHandler(w http.ResponseWriter, r *http // --------- // Clear locks held on a given bucket, prefix and duration it was held for. func (adminAPI adminAPIHandlers) ClearLocksHandler(w http.ResponseWriter, r *http.Request) { - adminAPIErr := checkRequestAuthType(r, "", "", "") + adminAPIErr := checkAdminRequestAuthType(r, "") if adminAPIErr != ErrNone { writeErrorResponse(w, adminAPIErr, r.URL) return @@ -467,7 +467,7 @@ func (adminAPI adminAPIHandlers) ListObjectsHealHandler(w http.ResponseWriter, r } // Validate request signature. - adminAPIErr := checkRequestAuthType(r, "", "", "") + adminAPIErr := checkAdminRequestAuthType(r, "") if adminAPIErr != ErrNone { writeErrorResponse(w, adminAPIErr, r.URL) return @@ -509,7 +509,7 @@ func (adminAPI adminAPIHandlers) ListBucketsHealHandler(w http.ResponseWriter, r } // Validate request signature. - adminAPIErr := checkRequestAuthType(r, "", "", "") + adminAPIErr := checkAdminRequestAuthType(r, "") if adminAPIErr != ErrNone { writeErrorResponse(w, adminAPIErr, r.URL) return @@ -546,7 +546,7 @@ func (adminAPI adminAPIHandlers) HealBucketHandler(w http.ResponseWriter, r *htt } // Validate request signature. - adminAPIErr := checkRequestAuthType(r, "", "", "") + adminAPIErr := checkAdminRequestAuthType(r, "") if adminAPIErr != ErrNone { writeErrorResponse(w, adminAPIErr, r.URL) return @@ -641,7 +641,7 @@ func (adminAPI adminAPIHandlers) HealObjectHandler(w http.ResponseWriter, r *htt } // Validate request signature. - adminAPIErr := checkRequestAuthType(r, "", "", "") + adminAPIErr := checkAdminRequestAuthType(r, "") if adminAPIErr != ErrNone { writeErrorResponse(w, adminAPIErr, r.URL) return @@ -705,7 +705,7 @@ func (adminAPI adminAPIHandlers) HealFormatHandler(w http.ResponseWriter, r *htt } // Validate request signature. - adminAPIErr := checkRequestAuthType(r, "", "", "") + adminAPIErr := checkAdminRequestAuthType(r, "") if adminAPIErr != ErrNone { writeErrorResponse(w, adminAPIErr, r.URL) return @@ -772,7 +772,7 @@ func (adminAPI adminAPIHandlers) HealFormatHandler(w http.ResponseWriter, r *htt // Get config.json of this minio setup. func (adminAPI adminAPIHandlers) GetConfigHandler(w http.ResponseWriter, r *http.Request) { // Validate request signature. - adminAPIErr := checkRequestAuthType(r, "", "", "") + adminAPIErr := checkAdminRequestAuthType(r, "") if adminAPIErr != ErrNone { writeErrorResponse(w, adminAPIErr, r.URL) return @@ -865,7 +865,7 @@ func (adminAPI adminAPIHandlers) SetConfigHandler(w http.ResponseWriter, r *http } // Validate request signature. - adminAPIErr := checkRequestAuthType(r, "", "", "") + adminAPIErr := checkAdminRequestAuthType(r, "") if adminAPIErr != ErrNone { writeErrorResponse(w, adminAPIErr, r.URL) return diff --git a/cmd/auth-handler.go b/cmd/auth-handler.go index 6d2ed5ff7..ae632da4b 100644 --- a/cmd/auth-handler.go +++ b/cmd/auth-handler.go @@ -18,6 +18,7 @@ package cmd import ( "bytes" + "errors" "io/ioutil" "net/http" "strings" @@ -101,6 +102,19 @@ func getRequestAuthType(r *http.Request) authType { return authTypeUnknown } +// checkAdminRequestAuthType checks whether the request is a valid signature V2 or V4 request. +// 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) + s3Err = isReqAuthenticated(r, region) + } + if s3Err != ErrNone { + errorIf(errors.New(getAPIError(s3Err).Description), "%s", dumpRequest(r)) + } + return s3Err +} + func checkRequestAuthType(r *http.Request, bucket, policyAction, region string) APIErrorCode { reqAuthType := getRequestAuthType(r) diff --git a/cmd/auth-handler_test.go b/cmd/auth-handler_test.go index be7118fcb..ff5678626 100644 --- a/cmd/auth-handler_test.go +++ b/cmd/auth-handler_test.go @@ -23,6 +23,7 @@ import ( "net/url" "os" "testing" + "time" "github.com/minio/minio/pkg/auth" ) @@ -277,6 +278,39 @@ func mustNewSignedRequest(method string, urlStr string, contentLength int64, bod return req } +// This is similar to mustNewRequest but additionally the request +// is signed with AWS Signature V2, fails if not able to do so. +func mustNewSignedV2Request(method string, urlStr string, contentLength int64, body io.ReadSeeker, t *testing.T) *http.Request { + req := mustNewRequest(method, urlStr, contentLength, body, t) + cred := globalServerConfig.GetCredential() + if err := signRequestV2(req, cred.AccessKey, cred.SecretKey); err != nil { + t.Fatalf("Unable to inititalized new signed http request %s", err) + } + return req +} + +// This is similar to mustNewRequest but additionally the request +// is presigned with AWS Signature V2, fails if not able to do so. +func mustNewPresignedV2Request(method string, urlStr string, contentLength int64, body io.ReadSeeker, t *testing.T) *http.Request { + req := mustNewRequest(method, urlStr, contentLength, body, t) + cred := globalServerConfig.GetCredential() + if err := preSignV2(req, cred.AccessKey, cred.SecretKey, time.Now().Add(10*time.Minute).Unix()); err != nil { + t.Fatalf("Unable to inititalized new signed http request %s", err) + } + return req +} + +// This is similar to mustNewRequest but additionally the request +// is presigned with AWS Signature V4, fails if not able to do so. +func mustNewPresignedRequest(method string, urlStr string, contentLength int64, body io.ReadSeeker, t *testing.T) *http.Request { + req := mustNewRequest(method, urlStr, contentLength, body, t) + cred := globalServerConfig.GetCredential() + if err := preSignV4(req, cred.AccessKey, cred.SecretKey, time.Now().Add(10*time.Minute).Unix()); err != nil { + t.Fatalf("Unable to inititalized new signed http request %s", err) + } + return req +} + func mustNewSignedBadMD5Request(method string, urlStr string, contentLength int64, body io.ReadSeeker, t *testing.T) *http.Request { req := mustNewRequest(method, urlStr, contentLength, body, t) req.Header.Set("Content-Md5", "YWFhYWFhYWFhYWFhYWFhCg==") @@ -324,3 +358,32 @@ func TestIsReqAuthenticated(t *testing.T) { } } } +func TestCheckAdminRequestAuthType(t *testing.T) { + path, err := newTestConfig(globalMinioDefaultRegion) + if err != nil { + t.Fatalf("unable initialize config file, %s", err) + } + defer os.RemoveAll(path) + + creds, err := auth.CreateCredentials("myuser", "mypassword") + if err != nil { + t.Fatalf("unable create credential, %s", err) + } + + globalServerConfig.SetCredential(creds) + testCases := []struct { + Request *http.Request + ErrCode APIErrorCode + }{ + {Request: mustNewRequest("GET", "http://127.0.0.1:9000", 0, nil, t), ErrCode: ErrAccessDenied}, + {Request: mustNewSignedRequest("GET", "http://127.0.0.1:9000", 0, nil, t), ErrCode: ErrNone}, + {Request: mustNewSignedV2Request("GET", "http://127.0.0.1:9000", 0, nil, t), ErrCode: ErrAccessDenied}, + {Request: mustNewPresignedV2Request("GET", "http://127.0.0.1:9000", 0, nil, t), ErrCode: ErrAccessDenied}, + {Request: mustNewPresignedRequest("GET", "http://127.0.0.1:9000", 0, nil, t), ErrCode: ErrAccessDenied}, + } + for i, testCase := range testCases { + if s3Error := checkAdminRequestAuthType(testCase.Request, globalServerConfig.GetRegion()); s3Error != testCase.ErrCode { + t.Errorf("Test %d: Unexpected s3error returned wanted %d, got %d", i, testCase.ErrCode, s3Error) + } + } +}