diff --git a/appveyor.yml b/appveyor.yml index 7488c103a..2e1dfd420 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -35,8 +35,8 @@ test_script: # Unit tests - ps: Add-AppveyorTest "Unit Tests" -Outcome Running - mkdir build\coverage - - go test -timeout 15m -v -race github.com/minio/minio/cmd... - - go test -v -race github.com/minio/minio/pkg... + - go test -timeout 15m -race github.com/minio/minio/cmd... + - go test -race github.com/minio/minio/pkg... - go test -coverprofile=build\coverage\coverage.txt -covermode=atomic github.com/minio/minio/cmd - ps: Update-AppveyorTest "Unit Tests" -Outcome Passed diff --git a/cmd/admin-handlers.go b/cmd/admin-handlers.go index b83f49652..5fdfbd47e 100644 --- a/cmd/admin-handlers.go +++ b/cmd/admin-handlers.go @@ -131,8 +131,8 @@ func (adminAPI adminAPIHandlers) ServiceCredentialsHandler(w http.ResponseWriter } // Avoid setting new credentials when they are already passed - // by the environnement - if globalEnvAccessKey != "" || globalEnvSecretKey != "" { + // by the environment. + if globalIsEnvCreds { writeErrorResponse(w, ErrMethodNotAllowed, r.URL) return } diff --git a/cmd/admin-handlers_test.go b/cmd/admin-handlers_test.go index 22c406d1e..dea04ef70 100644 --- a/cmd/admin-handlers_test.go +++ b/cmd/admin-handlers_test.go @@ -294,11 +294,9 @@ func TestServiceSetCreds(t *testing.T) { for i, testCase := range testCases { // Set or unset environement keys if !testCase.EnvKeysSet { - globalEnvAccessKey = "" - globalEnvSecretKey = "" + globalIsEnvCreds = false } else { - globalEnvAccessKey = testCase.Username - globalEnvSecretKey = testCase.Password + globalIsEnvCreds = true } // Construct setCreds request body diff --git a/cmd/auth-handler_test.go b/cmd/auth-handler_test.go index de9d8e124..fc48afc6c 100644 --- a/cmd/auth-handler_test.go +++ b/cmd/auth-handler_test.go @@ -315,7 +315,12 @@ func TestIsReqAuthenticated(t *testing.T) { } defer removeAll(path) - serverConfig.SetCredential(credential{"myuser", "mypassword"}) + creds, err := getCredential("myuser", "mypassword") + if err != nil { + t.Fatal(err) + } + + serverConfig.SetCredential(creds) // List of test cases for validating http request authentication. testCases := []struct { diff --git a/cmd/browser-peer-rpc.go b/cmd/browser-peer-rpc.go index 79f9e7b53..335e0b770 100644 --- a/cmd/browser-peer-rpc.go +++ b/cmd/browser-peer-rpc.go @@ -63,8 +63,13 @@ func (br *browserPeerAPIHandlers) SetAuthPeer(args SetAuthPeerArgs, reply *AuthR return err } + creds, err := getCredential(args.Creds.AccessKey, args.Creds.SecretKey) + if err != nil { + return err + } + // Update credentials in memory - serverConfig.SetCredential(args.Creds) + serverConfig.SetCredential(creds) // Save credentials to config file if err := serverConfig.Save(); err != nil { diff --git a/cmd/config-v13.go b/cmd/config-v13.go index 5ba8b78c6..7b014fb6c 100644 --- a/cmd/config-v13.go +++ b/cmd/config-v13.go @@ -90,10 +90,12 @@ func initConfig() (bool, error) { // Save config into file. return true, serverConfig.Save() } + configFile, err := getConfigFile() if err != nil { return false, err } + if _, err = os.Stat(configFile); err != nil { return false, err } @@ -103,7 +105,13 @@ func initConfig() (bool, error) { if err != nil { return false, err } - if err := qc.Load(configFile); err != nil { + + if err = qc.Load(configFile); err != nil { + return false, err + } + + srvCfg.Credential, err = getCredential(srvCfg.Credential.AccessKey, srvCfg.Credential.SecretKey) + if err != nil { return false, err } @@ -112,6 +120,7 @@ func initConfig() (bool, error) { // Save the loaded config globally. serverConfig = srvCfg serverConfigMu.Unlock() + // Set the version properly after the unmarshalled json is loaded. serverConfig.Version = globalMinioConfigVersion @@ -337,6 +346,7 @@ func (s *serverConfigV13) SetCredential(creds credential) { serverConfigMu.Lock() defer serverConfigMu.Unlock() + // Set updated credential. s.Credential = creds } diff --git a/cmd/credential.go b/cmd/credential.go index 4d84c9301..aad1749b1 100644 --- a/cmd/credential.go +++ b/cmd/credential.go @@ -19,6 +19,8 @@ package cmd import ( "crypto/rand" "encoding/base64" + + "golang.org/x/crypto/bcrypt" ) const ( @@ -65,14 +67,31 @@ func isSecretKeyValid(secretKey string) bool { // credential container for access and secret keys. type credential struct { - AccessKey string `json:"accessKey"` - SecretKey string `json:"secretKey"` + AccessKey string `json:"accessKey,omitempty"` + SecretKey string `json:"secretKey,omitempty"` + SecretKeyHash []byte `json:"secretKeyHash,omitempty"` +} + +// Generate a bcrypt hashed key for input secret key. +func mustGetHashedSecretKey(secretKey string) []byte { + hashedSecretKey, err := bcrypt.GenerateFromPassword([]byte(secretKey), bcrypt.DefaultCost) + if err != nil { + panic(err) + } + return hashedSecretKey } +// Initialize a new credential object. func newCredential() credential { - return credential{mustGetAccessKey(), mustGetSecretKey()} + secretKey := mustGetSecretKey() + accessKey := mustGetAccessKey() + + secretHash := mustGetHashedSecretKey(secretKey) + return credential{accessKey, secretKey, secretHash} } +// Converts accessKey and secretKeys into credential object which +// contains bcrypt secret key hash for future validation. func getCredential(accessKey, secretKey string) (credential, error) { if !isAccessKeyValid(accessKey) { return credential{}, errInvalidAccessKeyLength @@ -82,5 +101,6 @@ func getCredential(accessKey, secretKey string) (credential, error) { return credential{}, errInvalidSecretKeyLength } - return credential{accessKey, secretKey}, nil + secretHash := mustGetHashedSecretKey(secretKey) + return credential{accessKey, secretKey, secretHash}, nil } diff --git a/cmd/globals.go b/cmd/globals.go index eaa0508e5..4eed8972b 100644 --- a/cmd/globals.go +++ b/cmd/globals.go @@ -110,11 +110,8 @@ var ( // Minio server user agent string. globalServerUserAgent = "Minio/" + ReleaseTag + " (" + runtime.GOOS + "; " + runtime.GOARCH + ")" - // Global server's access key - globalEnvAccessKey = "" - - // Global server's secret key - globalEnvSecretKey = "" + // Set to true if credentials were passed from env, default is false. + globalIsEnvCreds = false // url.URL endpoints of disks that belong to the object storage. globalEndpoints = []*url.URL{} diff --git a/cmd/jwt.go b/cmd/jwt.go index 549617cab..c3d80a08f 100644 --- a/cmd/jwt.go +++ b/cmd/jwt.go @@ -65,8 +65,7 @@ func authenticateJWT(accessKey, secretKey string, expiry time.Duration) (string, // Validate secret key. // Using bcrypt to avoid timing attacks. - hashedSecretKey, _ := bcrypt.GenerateFromPassword([]byte(serverCred.SecretKey), bcrypt.DefaultCost) - if bcrypt.CompareHashAndPassword(hashedSecretKey, []byte(secretKey)) != nil { + if bcrypt.CompareHashAndPassword(serverCred.SecretKeyHash, []byte(secretKey)) != nil { return "", errAuthentication } diff --git a/cmd/jwt_test.go b/cmd/jwt_test.go index 32cdfa8fe..c8b531269 100644 --- a/cmd/jwt_test.go +++ b/cmd/jwt_test.go @@ -75,10 +75,40 @@ func testAuthenticate(authType string, t *testing.T) { } } -func TestNodeAuthenticate(t *testing.T) { +func TestAuthenticateNode(t *testing.T) { testAuthenticate("node", t) } -func TestWebAuthenticate(t *testing.T) { +func TestAuthenticateWeb(t *testing.T) { testAuthenticate("web", t) } + +func BenchmarkAuthenticateNode(b *testing.B) { + testPath, err := newTestConfig(globalMinioDefaultRegion) + if err != nil { + b.Fatalf("unable initialize config file, %s", err) + } + defer removeAll(testPath) + + creds := serverConfig.GetCredential() + b.ResetTimer() + b.ReportAllocs() + for i := 0; i < b.N; i++ { + authenticateNode(creds.AccessKey, creds.SecretKey) + } +} + +func BenchmarkAuthenticateWeb(b *testing.B) { + testPath, err := newTestConfig(globalMinioDefaultRegion) + if err != nil { + b.Fatalf("unable initialize config file, %s", err) + } + defer removeAll(testPath) + + creds := serverConfig.GetCredential() + b.ResetTimer() + b.ReportAllocs() + for i := 0; i < b.N; i++ { + authenticateWeb(creds.AccessKey, creds.SecretKey) + } +} diff --git a/cmd/main.go b/cmd/main.go index 0038042ea..57c42772a 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -190,20 +190,14 @@ func minioInit(ctx *cli.Context) { enableLoggers() // Fetch access keys from environment variables and update the config. - globalEnvAccessKey = os.Getenv("MINIO_ACCESS_KEY") - globalEnvSecretKey = os.Getenv("MINIO_SECRET_KEY") - if globalEnvAccessKey != "" && globalEnvSecretKey != "" { + accessKey := os.Getenv("MINIO_ACCESS_KEY") + secretKey := os.Getenv("MINIO_SECRET_KEY") + if accessKey != "" && secretKey != "" { + creds, err := getCredential(accessKey, secretKey) + fatalIf(err, "Credentials are invalid, please set proper credentials `minio server --help`") + // Set new credentials. - serverConfig.SetCredential(credential{ - AccessKey: globalEnvAccessKey, - SecretKey: globalEnvSecretKey, - }) - } - if !isAccessKeyValid(serverConfig.GetCredential().AccessKey) { - fatalIf(errInvalidArgument, "Invalid access key. Accept only a string starting with a alphabetic and containing from 5 to 20 characters.") - } - if !isSecretKeyValid(serverConfig.GetCredential().SecretKey) { - fatalIf(errInvalidArgument, "Invalid secret key. Accept only a string containing from 8 to 40 characters.") + serverConfig.SetCredential(creds) } // Init the error tracing module. diff --git a/cmd/web-handlers.go b/cmd/web-handlers.go index 8346ef944..adf9f6db7 100644 --- a/cmd/web-handlers.go +++ b/cmd/web-handlers.go @@ -363,16 +363,18 @@ func (web *webAPIHandlers) SetAuth(r *http.Request, args *SetAuthArgs, reply *Se } // As we already validated the authentication, we save given access/secret keys. - cred, err := getCredential(args.AccessKey, args.SecretKey) + creds, err := getCredential(args.AccessKey, args.SecretKey) if err != nil { return toJSONError(err) } // Notify all other Minio peers to update credentials - errsMap := updateCredsOnPeers(cred) + errsMap := updateCredsOnPeers(creds) // Update local credentials - serverConfig.SetCredential(cred) + serverConfig.SetCredential(creds) + + // Persist updated credentials. if err = serverConfig.Save(); err != nil { errsMap[globalMinioAddr] = err } @@ -506,14 +508,7 @@ func (web *webAPIHandlers) Download(w http.ResponseWriter, r *http.Request) { objectLock.RLock() defer objectLock.RUnlock() - objInfo, err := objectAPI.GetObjectInfo(bucket, object) - if err != nil { - writeWebErrorResponse(w, err) - return - } - offset := int64(0) - err = objectAPI.GetObject(bucket, object, offset, objInfo.Size, w) - if err != nil { + if err := objectAPI.GetObject(bucket, object, 0, -1, w); err != nil { /// No need to print error, response writer already written to. return } diff --git a/cmd/web-handlers_test.go b/cmd/web-handlers_test.go index 238713078..fc72ab9ff 100644 --- a/cmd/web-handlers_test.go +++ b/cmd/web-handlers_test.go @@ -1533,10 +1533,6 @@ func TestWebObjectLayerFaultyDisks(t *testing.T) { if rec.Code != http.StatusOK { t.Fatalf("Expected the response status to be 200, but instead found `%d`", rec.Code) } - resp := string(rec.Body.Bytes()) - if !strings.Contains(resp, "We encountered an internal error, please try again.") { - t.Fatalf("Unexpected error message, expected: `Invalid token`, found: `%s`", resp) - } // Test authorization of Web.Upload content := []byte("temporary file's content") @@ -1553,8 +1549,4 @@ func TestWebObjectLayerFaultyDisks(t *testing.T) { if rec.Code != http.StatusOK { t.Fatalf("Expected the response status to be 200, but instead found `%d`", rec.Code) } - resp = string(rec.Body.Bytes()) - if !strings.Contains(resp, "We encountered an internal error, please try again.") { - t.Fatalf("Unexpected error message, expected: `Invalid token`, found: `%s`", resp) - } }