From ee0172dfe419febe371f21d50835b577338cadfa Mon Sep 17 00:00:00 2001 From: Bala FA Date: Tue, 27 Dec 2016 21:58:10 +0530 Subject: [PATCH] Have simpler JWT authentication. (#3501) --- cmd/admin-rpc-server.go | 4 +- cmd/auth-handler.go | 2 +- cmd/auth-rpc-client.go | 24 ---- cmd/browser-peer-rpc.go | 14 +-- cmd/credential.go | 12 ++ cmd/jwt.go | 116 ++++++++++++++++++ cmd/jwt_test.go | 84 +++++++++++++ cmd/lock-rpc-server-common.go | 2 +- cmd/login-server.go | 10 +- cmd/post-policy_test.go | 4 +- cmd/s3-peer-rpc-handlers.go | 8 +- cmd/signature-jwt.go | 103 ---------------- cmd/signature-jwt_test.go | 214 --------------------------------- cmd/storage-rpc-server.go | 26 ++-- cmd/storage-rpc-server_test.go | 13 +- cmd/test-utils_test.go | 4 +- cmd/web-handlers.go | 132 +++++--------------- 17 files changed, 277 insertions(+), 495 deletions(-) create mode 100644 cmd/jwt.go create mode 100644 cmd/jwt_test.go delete mode 100644 cmd/signature-jwt.go delete mode 100644 cmd/signature-jwt_test.go diff --git a/cmd/admin-rpc-server.go b/cmd/admin-rpc-server.go index 3948901d1..af5cd0467 100644 --- a/cmd/admin-rpc-server.go +++ b/cmd/admin-rpc-server.go @@ -32,7 +32,7 @@ type serviceCmd struct { // Shutdown - Shutdown this instance of minio server. func (s *serviceCmd) Shutdown(args *GenericArgs, reply *GenericReply) error { - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } globalServiceSignalCh <- serviceStop @@ -41,7 +41,7 @@ func (s *serviceCmd) Shutdown(args *GenericArgs, reply *GenericReply) error { // Restart - Restart this instance of minio server. func (s *serviceCmd) Restart(args *GenericArgs, reply *GenericReply) error { - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } globalServiceSignalCh <- serviceRestart diff --git a/cmd/auth-handler.go b/cmd/auth-handler.go index a1708c7ef..c048698bb 100644 --- a/cmd/auth-handler.go +++ b/cmd/auth-handler.go @@ -224,7 +224,7 @@ func (a authHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } else if aType == authTypeJWT { // Validate Authorization header if its valid for JWT request. - if !isJWTReqAuthenticated(r) { + if !isHTTPRequestValid(r) { w.WriteHeader(http.StatusUnauthorized) return } diff --git a/cmd/auth-rpc-client.go b/cmd/auth-rpc-client.go index 978c9382f..9284e0784 100644 --- a/cmd/auth-rpc-client.go +++ b/cmd/auth-rpc-client.go @@ -17,12 +17,9 @@ package cmd import ( - "fmt" "net/rpc" "sync" "time" - - jwtgo "github.com/dgrijalva/jwt-go" ) // GenericReply represents any generic RPC reply. @@ -63,27 +60,6 @@ type RPCLoginReply struct { ServerVersion string } -// Validates if incoming token is valid. -func isRPCTokenValid(tokenStr string) bool { - jwt, err := newJWT(defaultInterNodeJWTExpiry, serverConfig.GetCredential()) - if err != nil { - errorIf(err, "Unable to initialize JWT") - return false - } - token, err := jwtgo.Parse(tokenStr, func(token *jwtgo.Token) (interface{}, error) { - if _, ok := token.Method.(*jwtgo.SigningMethodHMAC); !ok { - return nil, fmt.Errorf("Unexpected signing method: %v", token.Header["alg"]) - } - return []byte(jwt.SecretKey), nil - }) - if err != nil { - errorIf(err, "Unable to parse JWT token string") - return false - } - // Return if token is valid. - return token.Valid -} - // Auth config represents authentication credentials and Login method name to be used // for fetching JWT tokens from the RPC server. type authConfig struct { diff --git a/cmd/browser-peer-rpc.go b/cmd/browser-peer-rpc.go index 3a76e8ff6..17d9254f6 100644 --- a/cmd/browser-peer-rpc.go +++ b/cmd/browser-peer-rpc.go @@ -25,17 +25,11 @@ import ( // Login handler implements JWT login token generator, which upon login request // along with username and password is generated. func (br *browserPeerAPIHandlers) LoginHandler(args *RPCLoginArgs, reply *RPCLoginReply) error { - jwt, err := newJWT(defaultInterNodeJWTExpiry, serverConfig.GetCredential()) - if err != nil { - return err - } - if err = jwt.Authenticate(args.Username, args.Password); err != nil { - return err - } - token, err := jwt.GenerateToken(args.Username) + token, err := authenticateWeb(args.Username, args.Password) if err != nil { return err } + reply.Token = token reply.ServerVersion = Version reply.Timestamp = time.Now().UTC() @@ -54,13 +48,13 @@ type SetAuthPeerArgs struct { // SetAuthPeer - Update to new credentials sent from a peer Minio // server. Since credentials are already validated on the sending // peer, here we just persist to file and update in-memory config. All -// subsequently running isRPCTokenValid() calls will fail, and clients +// subsequently running isAuthTokenValid() calls will fail, and clients // will be forced to re-establish connections. Connections will be // re-established only when the sending client has also updated its // credentials. func (br *browserPeerAPIHandlers) SetAuthPeer(args SetAuthPeerArgs, reply *GenericReply) error { // Check auth - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } diff --git a/cmd/credential.go b/cmd/credential.go index 3e2c7d576..4d84c9301 100644 --- a/cmd/credential.go +++ b/cmd/credential.go @@ -72,3 +72,15 @@ type credential struct { func newCredential() credential { return credential{mustGetAccessKey(), mustGetSecretKey()} } + +func getCredential(accessKey, secretKey string) (credential, error) { + if !isAccessKeyValid(accessKey) { + return credential{}, errInvalidAccessKeyLength + } + + if !isSecretKeyValid(secretKey) { + return credential{}, errInvalidSecretKeyLength + } + + return credential{accessKey, secretKey}, nil +} diff --git a/cmd/jwt.go b/cmd/jwt.go new file mode 100644 index 000000000..c87e15ffa --- /dev/null +++ b/cmd/jwt.go @@ -0,0 +1,116 @@ +/* + * Minio Cloud Storage, (C) 2016 Minio, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package cmd + +import ( + "errors" + "fmt" + "net/http" + "strings" + "time" + + jwtgo "github.com/dgrijalva/jwt-go" + jwtreq "github.com/dgrijalva/jwt-go/request" + "golang.org/x/crypto/bcrypt" +) + +const ( + jwtAlgorithm = "Bearer" + + // Default JWT token for web handlers is one day. + defaultJWTExpiry = 24 * time.Hour + + // Inter-node JWT token expiry is 100 years approx. + defaultInterNodeJWTExpiry = 100 * 365 * 24 * time.Hour +) + +var errInvalidAccessKeyLength = errors.New("Invalid access key, access key should be 5 to 20 characters in length") +var errInvalidSecretKeyLength = errors.New("Invalid secret key, secret key should be 8 to 40 characters in length") + +var errInvalidAccessKeyID = errors.New("The access key ID you provided does not exist in our records") +var errAuthentication = errors.New("Authentication failed, check your access credentials") + +func authenticateJWT(accessKey, secretKey string, expiry time.Duration) (string, error) { + // Trim spaces. + accessKey = strings.TrimSpace(accessKey) + + if !isAccessKeyValid(accessKey) { + return "", errInvalidAccessKeyLength + } + if !isSecretKeyValid(secretKey) { + return "", errInvalidSecretKeyLength + } + + serverCred := serverConfig.GetCredential() + + // Validate access key. + if accessKey != serverCred.AccessKey { + return "", errInvalidAccessKeyID + } + + // Validate secret key. + // Using bcrypt to avoid timing attacks. + hashedSecretKey, _ := bcrypt.GenerateFromPassword([]byte(serverCred.SecretKey), bcrypt.DefaultCost) + if bcrypt.CompareHashAndPassword(hashedSecretKey, []byte(secretKey)) != nil { + return "", errAuthentication + } + + utcNow := time.Now().UTC() + token := jwtgo.NewWithClaims(jwtgo.SigningMethodHS512, jwtgo.MapClaims{ + "exp": utcNow.Add(expiry).Unix(), + "iat": utcNow.Unix(), + "sub": accessKey, + }) + + return token.SignedString([]byte(serverCred.SecretKey)) +} + +func authenticateNode(accessKey, secretKey string) (string, error) { + return authenticateJWT(accessKey, secretKey, defaultInterNodeJWTExpiry) +} + +func authenticateWeb(accessKey, secretKey string) (string, error) { + return authenticateJWT(accessKey, secretKey, defaultJWTExpiry) +} + +func keyFuncCallback(jwtToken *jwtgo.Token) (interface{}, error) { + if _, ok := jwtToken.Method.(*jwtgo.SigningMethodHMAC); !ok { + return nil, fmt.Errorf("Unexpected signing method: %v", jwtToken.Header["alg"]) + } + + return []byte(serverConfig.GetCredential().SecretKey), nil +} + +func isAuthTokenValid(tokenString string) bool { + jwtToken, err := jwtgo.Parse(tokenString, keyFuncCallback) + if err != nil { + errorIf(err, "Unable to parse JWT token string") + return false + } + + return jwtToken.Valid +} + +func isHTTPRequestValid(req *http.Request) bool { + jwtToken, err := jwtreq.ParseFromRequest(req, jwtreq.AuthorizationHeaderExtractor, keyFuncCallback) + if err != nil { + errorIf(err, "Unable to parse JWT token string") + return false + } + + return jwtToken.Valid +} diff --git a/cmd/jwt_test.go b/cmd/jwt_test.go new file mode 100644 index 000000000..341ecd591 --- /dev/null +++ b/cmd/jwt_test.go @@ -0,0 +1,84 @@ +/* + * Minio Cloud Storage, (C) 2016 Minio, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package cmd + +import "testing" + +func testAuthenticate(authType string, t *testing.T) { + testPath, err := newTestConfig("us-east-1") + if err != nil { + t.Fatalf("unable initialize config file, %s", err) + } + defer removeAll(testPath) + + serverCred := serverConfig.GetCredential() + + // Define test cases. + testCases := []struct { + accessKey string + secretKey string + expectedErr error + }{ + // Access key too small. + {"user", "pass", errInvalidAccessKeyLength}, + // Access key too long. + {"user12345678901234567", "pass", errInvalidAccessKeyLength}, + // Access key contains unsuppported characters. + {"!@#$", "pass", errInvalidAccessKeyLength}, + // Secret key too small. + {"myuser", "pass", errInvalidSecretKeyLength}, + // Secret key too long. + {"myuser", "pass1234567890123456789012345678901234567", errInvalidSecretKeyLength}, + // Authentication error. + {"myuser", "mypassword", errInvalidAccessKeyID}, + // Authentication error. + {serverCred.AccessKey, "mypassword", errAuthentication}, + // Success. + {serverCred.AccessKey, serverCred.SecretKey, nil}, + // Success when access key contains leading/trailing spaces. + {" " + serverCred.AccessKey + " ", serverCred.SecretKey, nil}, + } + + // Run tests. + for _, testCase := range testCases { + var err error + if authType == "node" { + _, err = authenticateNode(testCase.accessKey, testCase.secretKey) + } else if authType == "web" { + _, err = authenticateWeb(testCase.accessKey, testCase.secretKey) + } + + if testCase.expectedErr != nil { + if err == nil { + t.Fatalf("%+v: expected: %s, got: ", testCase, testCase.expectedErr) + } + if testCase.expectedErr.Error() != err.Error() { + t.Fatalf("%+v: expected: %s, got: %s", testCase, testCase.expectedErr, err) + } + } else if err != nil { + t.Fatalf("%+v: expected: , got: %s", testCase, err) + } + } +} + +func TestNodeAuthenticate(t *testing.T) { + testAuthenticate("node", t) +} + +func TestWebAuthenticate(t *testing.T) { + testAuthenticate("web", t) +} diff --git a/cmd/lock-rpc-server-common.go b/cmd/lock-rpc-server-common.go index 716028de6..c0e9a11ad 100644 --- a/cmd/lock-rpc-server-common.go +++ b/cmd/lock-rpc-server-common.go @@ -65,7 +65,7 @@ func (l *lockServer) validateLockArgs(args *LockArgs) error { if curTime.Sub(args.Timestamp) > globalMaxSkewTime || args.Timestamp.Sub(curTime) > globalMaxSkewTime { return errServerTimeMismatch } - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } return nil diff --git a/cmd/login-server.go b/cmd/login-server.go index 0de184100..223288609 100644 --- a/cmd/login-server.go +++ b/cmd/login-server.go @@ -23,17 +23,11 @@ type loginServer struct { // LoginHandler - Handles JWT based RPC logic. func (b loginServer) LoginHandler(args *RPCLoginArgs, reply *RPCLoginReply) error { - jwt, err := newJWT(defaultInterNodeJWTExpiry, serverConfig.GetCredential()) - if err != nil { - return err - } - if err = jwt.Authenticate(args.Username, args.Password); err != nil { - return err - } - token, err := jwt.GenerateToken(args.Username) + token, err := authenticateNode(args.Username, args.Password) if err != nil { return err } + reply.Token = token reply.Timestamp = time.Now().UTC() reply.ServerVersion = Version diff --git a/cmd/post-policy_test.go b/cmd/post-policy_test.go index 1b9b891d9..52bd96058 100644 --- a/cmd/post-policy_test.go +++ b/cmd/post-policy_test.go @@ -545,7 +545,7 @@ func buildGenericPolicy(t time.Time, accessKey, bucketName, objectName string, c // Expire the request five minutes from now. expirationTime := t.Add(time.Minute * 5) - credStr := getCredential(accessKey, serverConfig.GetRegion(), t) + credStr := getCredentialString(accessKey, serverConfig.GetRegion(), t) // Create a new post policy. policy := newPostPolicyBytesV4(credStr, bucketName, objectName, expirationTime) if contentLengthRange { @@ -557,7 +557,7 @@ func buildGenericPolicy(t time.Time, accessKey, bucketName, objectName string, c func newPostRequestV4Generic(endPoint, bucketName, objectName string, objData []byte, accessKey, secretKey string, t time.Time, policy []byte, addFormData map[string]string, corruptedB64 bool, corruptedMultipart bool) (*http.Request, error) { // Get the user credential. - credStr := getCredential(accessKey, serverConfig.GetRegion(), t) + credStr := getCredentialString(accessKey, serverConfig.GetRegion(), t) // Only need the encoding. encodedPolicy := base64.StdEncoding.EncodeToString(policy) diff --git a/cmd/s3-peer-rpc-handlers.go b/cmd/s3-peer-rpc-handlers.go index 46b7d9611..769d84f76 100644 --- a/cmd/s3-peer-rpc-handlers.go +++ b/cmd/s3-peer-rpc-handlers.go @@ -37,7 +37,7 @@ func (s *SetBucketNotificationPeerArgs) BucketUpdate(client BucketMetaState) err func (s3 *s3PeerAPIHandlers) SetBucketNotificationPeer(args *SetBucketNotificationPeerArgs, reply *GenericReply) error { // check auth - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } @@ -64,7 +64,7 @@ func (s *SetBucketListenerPeerArgs) BucketUpdate(client BucketMetaState) error { func (s3 *s3PeerAPIHandlers) SetBucketListenerPeer(args *SetBucketListenerPeerArgs, reply *GenericReply) error { // check auth - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } @@ -86,7 +86,7 @@ type EventArgs struct { // submit an event to the receiving server. func (s3 *s3PeerAPIHandlers) Event(args *EventArgs, reply *GenericReply) error { // check auth - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } @@ -114,7 +114,7 @@ func (s *SetBucketPolicyPeerArgs) BucketUpdate(client BucketMetaState) error { // tell receiving server to update a bucket policy func (s3 *s3PeerAPIHandlers) SetBucketPolicyPeer(args *SetBucketPolicyPeerArgs, reply *GenericReply) error { // check auth - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } diff --git a/cmd/signature-jwt.go b/cmd/signature-jwt.go deleted file mode 100644 index ab2d1b4cd..000000000 --- a/cmd/signature-jwt.go +++ /dev/null @@ -1,103 +0,0 @@ -/* - * Minio Cloud Storage, (C) 2016 Minio, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package cmd - -import ( - "errors" - "strings" - "time" - - jwtgo "github.com/dgrijalva/jwt-go" - "golang.org/x/crypto/bcrypt" -) - -const jwtAlgorithm = "Bearer" - -// JWT - jwt auth backend -type JWT struct { - credential - expiry time.Duration -} - -const ( - // Default JWT token for web handlers is one day. - defaultJWTExpiry time.Duration = time.Hour * 24 - - // Inter-node JWT token expiry is 100 years. - defaultInterNodeJWTExpiry time.Duration = time.Hour * 24 * 365 * 100 -) - -// newJWT - returns new JWT object. -func newJWT(expiry time.Duration, cred credential) (*JWT, error) { - if !isAccessKeyValid(cred.AccessKey) { - return nil, errInvalidAccessKeyLength - } - if !isSecretKeyValid(cred.SecretKey) { - return nil, errInvalidSecretKeyLength - } - return &JWT{cred, expiry}, nil -} - -var errInvalidAccessKeyLength = errors.New("Invalid access key, access key should be 5 to 20 characters in length") -var errInvalidSecretKeyLength = errors.New("Invalid secret key, secret key should be 8 to 40 characters in length") - -// GenerateToken - generates a new Json Web Token based on the incoming access key. -func (jwt *JWT) GenerateToken(accessKey string) (string, error) { - // Trim spaces. - accessKey = strings.TrimSpace(accessKey) - - if !isAccessKeyValid(accessKey) { - return "", errInvalidAccessKeyLength - } - - tUTCNow := time.Now().UTC() - token := jwtgo.NewWithClaims(jwtgo.SigningMethodHS512, jwtgo.MapClaims{ - // Token expires in 10hrs. - "exp": tUTCNow.Add(jwt.expiry).Unix(), - "iat": tUTCNow.Unix(), - "sub": accessKey, - }) - return token.SignedString([]byte(jwt.SecretKey)) -} - -var errInvalidAccessKeyID = errors.New("The access key ID you provided does not exist in our records") -var errAuthentication = errors.New("Authentication failed, check your access credentials") - -// Authenticate - authenticates incoming access key and secret key. -func (jwt *JWT) Authenticate(accessKey, secretKey string) error { - // Trim spaces. - accessKey = strings.TrimSpace(accessKey) - - if !isAccessKeyValid(accessKey) { - return errInvalidAccessKeyLength - } - if !isSecretKeyValid(secretKey) { - return errInvalidSecretKeyLength - } - - if accessKey != jwt.AccessKey { - return errInvalidAccessKeyID - } - - hashedSecretKey, _ := bcrypt.GenerateFromPassword([]byte(jwt.SecretKey), bcrypt.DefaultCost) - if bcrypt.CompareHashAndPassword(hashedSecretKey, []byte(secretKey)) != nil { - return errAuthentication - } - - // Success. - return nil -} diff --git a/cmd/signature-jwt_test.go b/cmd/signature-jwt_test.go deleted file mode 100644 index b3421deb1..000000000 --- a/cmd/signature-jwt_test.go +++ /dev/null @@ -1,214 +0,0 @@ -/* - * Minio Cloud Storage, (C) 2016 Minio, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package cmd - -import ( - "io/ioutil" - "os" - "path" - "testing" -) - -// Tests newJWT() -func TestNewJWT(t *testing.T) { - savedServerConfig := serverConfig - defer func() { - serverConfig = savedServerConfig - }() - serverConfig = nil - - // Test non-existent config directory. - path1, err := ioutil.TempDir(globalTestTmpDir, "minio-") - if err != nil { - t.Fatalf("Unable to create a temporary directory, %s", err) - } - defer removeAll(path1) - - // Test empty config directory. - path2, err := ioutil.TempDir(globalTestTmpDir, "minio-") - if err != nil { - t.Fatalf("Unable to create a temporary directory, %s", err) - } - defer removeAll(path2) - - // Test empty config file. - path3, err := ioutil.TempDir(globalTestTmpDir, "minio-") - if err != nil { - t.Fatalf("Unable to create a temporary directory, %s", err) - } - defer removeAll(path3) - - if err = ioutil.WriteFile(path.Join(path3, "config.json"), []byte{}, os.ModePerm); err != nil { - t.Fatalf("unable to create config file, %s", err) - } - - // Test initialized config file. - path4, err := ioutil.TempDir(globalTestTmpDir, "minio-") - if err != nil { - t.Fatalf("Unable to create a temporary directory, %s", err) - } - defer removeAll(path4) - - // Define test cases. - testCases := []struct { - dirPath string - init bool - cred *credential - expectedErr error - }{ - // Test initialized config file. - {path4, true, nil, nil}, - // Test to read already created config file. - {path4, true, nil, nil}, - // Access key is too small. - {path4, false, &credential{"user", "pass"}, errInvalidAccessKeyLength}, - // Access key is too long. - {path4, false, &credential{"user12345678901234567", "pass"}, errInvalidAccessKeyLength}, - // Secret key is too small. - {path4, false, &credential{"myuser", "pass"}, errInvalidSecretKeyLength}, - // Secret key is too long. - {path4, false, &credential{"myuser", "pass1234567890123456789012345678901234567"}, errInvalidSecretKeyLength}, - // Valid access/secret keys. - {path4, false, &credential{"myuser", "mypassword"}, nil}, - } - - // Run tests. - for _, testCase := range testCases { - setGlobalConfigPath(testCase.dirPath) - if testCase.init { - if _, err := initConfig(); err != nil { - t.Fatalf("unable initialize config file, %s", err) - } - } - if testCase.cred != nil { - serverConfig.SetCredential(*testCase.cred) - } - _, err := newJWT(defaultJWTExpiry, serverConfig.GetCredential()) - if testCase.expectedErr != nil { - if err == nil { - t.Fatalf("%+v: expected: %s, got: ", testCase, testCase.expectedErr) - } - if testCase.expectedErr.Error() != err.Error() { - t.Fatalf("%+v: expected: %s, got: %s", testCase, testCase.expectedErr, err) - } - } else if err != nil { - t.Fatalf("%+v: expected: , got: %s", testCase, err) - } - } -} - -// Tests JWT.GenerateToken() -func TestGenerateToken(t *testing.T) { - testPath, err := newTestConfig("us-east-1") - if err != nil { - t.Fatalf("unable initialize config file, %s", err) - } - defer removeAll(testPath) - - jwt, err := newJWT(defaultJWTExpiry, serverConfig.GetCredential()) - if err != nil { - t.Fatalf("unable get new JWT, %s", err) - } - - // Define test cases. - testCases := []struct { - accessKey string - expectedErr error - }{ - // Access key is too small. - {"user", errInvalidAccessKeyLength}, - // Access key is too long. - {"user12345678901234567", errInvalidAccessKeyLength}, - // Access key contains unsupported characters. - {"!@#$", errInvalidAccessKeyLength}, - // Valid access key. - {"myuser", nil}, - // Valid access key with leading/trailing spaces. - {" myuser ", nil}, - } - - // Run tests. - for _, testCase := range testCases { - _, err := jwt.GenerateToken(testCase.accessKey) - if testCase.expectedErr != nil { - if err == nil { - t.Fatalf("%+v: expected: %s, got: ", testCase, testCase.expectedErr) - } - - if testCase.expectedErr.Error() != err.Error() { - t.Fatalf("%+v: expected: %s, got: %s", testCase, testCase.expectedErr, err) - } - } else if err != nil { - t.Fatalf("%+v: expected: , got: %s", testCase, err) - } - } -} - -// Tests JWT.Authenticate() -func TestAuthenticate(t *testing.T) { - testPath, err := newTestConfig("us-east-1") - if err != nil { - t.Fatalf("unable initialize config file, %s", err) - } - defer removeAll(testPath) - - jwt, err := newJWT(defaultJWTExpiry, serverConfig.GetCredential()) - if err != nil { - t.Fatalf("unable get new JWT, %s", err) - } - - // Define test cases. - testCases := []struct { - accessKey string - secretKey string - expectedErr error - }{ - // Access key too small. - {"user", "pass", errInvalidAccessKeyLength}, - // Access key too long. - {"user12345678901234567", "pass", errInvalidAccessKeyLength}, - // Access key contains unsuppported characters. - {"!@#$", "pass", errInvalidAccessKeyLength}, - // Secret key too small. - {"myuser", "pass", errInvalidSecretKeyLength}, - // Secret key too long. - {"myuser", "pass1234567890123456789012345678901234567", errInvalidSecretKeyLength}, - // Authentication error. - {"myuser", "mypassword", errInvalidAccessKeyID}, - // Authentication error. - {serverConfig.GetCredential().AccessKey, "mypassword", errAuthentication}, - // Success. - {serverConfig.GetCredential().AccessKey, serverConfig.GetCredential().SecretKey, nil}, - // Success when access key contains leading/trailing spaces. - {" " + serverConfig.GetCredential().AccessKey + " ", serverConfig.GetCredential().SecretKey, nil}, - } - - // Run tests. - for _, testCase := range testCases { - err := jwt.Authenticate(testCase.accessKey, testCase.secretKey) - if testCase.expectedErr != nil { - if err == nil { - t.Fatalf("%+v: expected: %s, got: ", testCase, testCase.expectedErr) - } - if testCase.expectedErr.Error() != err.Error() { - t.Fatalf("%+v: expected: %s, got: %s", testCase, testCase.expectedErr, err) - } - } else if err != nil { - t.Fatalf("%+v: expected: , got: %s", testCase, err) - } - } -} diff --git a/cmd/storage-rpc-server.go b/cmd/storage-rpc-server.go index 66dfc294e..0210a4c8b 100644 --- a/cmd/storage-rpc-server.go +++ b/cmd/storage-rpc-server.go @@ -39,7 +39,7 @@ type storageServer struct { // DiskInfoHandler - disk info handler is rpc wrapper for DiskInfo operation. func (s *storageServer) DiskInfoHandler(args *GenericArgs, reply *disk.Info) error { - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } info, err := s.storage.DiskInfo() @@ -51,7 +51,7 @@ func (s *storageServer) DiskInfoHandler(args *GenericArgs, reply *disk.Info) err // MakeVolHandler - make vol handler is rpc wrapper for MakeVol operation. func (s *storageServer) MakeVolHandler(args *GenericVolArgs, reply *GenericReply) error { - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } return s.storage.MakeVol(args.Vol) @@ -59,7 +59,7 @@ func (s *storageServer) MakeVolHandler(args *GenericVolArgs, reply *GenericReply // ListVolsHandler - list vols handler is rpc wrapper for ListVols operation. func (s *storageServer) ListVolsHandler(args *GenericArgs, reply *ListVolsReply) error { - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } vols, err := s.storage.ListVols() @@ -72,7 +72,7 @@ func (s *storageServer) ListVolsHandler(args *GenericArgs, reply *ListVolsReply) // StatVolHandler - stat vol handler is a rpc wrapper for StatVol operation. func (s *storageServer) StatVolHandler(args *GenericVolArgs, reply *VolInfo) error { - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } volInfo, err := s.storage.StatVol(args.Vol) @@ -86,7 +86,7 @@ func (s *storageServer) StatVolHandler(args *GenericVolArgs, reply *VolInfo) err // DeleteVolHandler - delete vol handler is a rpc wrapper for // DeleteVol operation. func (s *storageServer) DeleteVolHandler(args *GenericVolArgs, reply *GenericReply) error { - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } return s.storage.DeleteVol(args.Vol) @@ -96,7 +96,7 @@ func (s *storageServer) DeleteVolHandler(args *GenericVolArgs, reply *GenericRep // StatFileHandler - stat file handler is rpc wrapper to stat file. func (s *storageServer) StatFileHandler(args *StatFileArgs, reply *FileInfo) error { - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } fileInfo, err := s.storage.StatFile(args.Vol, args.Path) @@ -109,7 +109,7 @@ func (s *storageServer) StatFileHandler(args *StatFileArgs, reply *FileInfo) err // ListDirHandler - list directory handler is rpc wrapper to list dir. func (s *storageServer) ListDirHandler(args *ListDirArgs, reply *[]string) error { - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } entries, err := s.storage.ListDir(args.Vol, args.Path) @@ -122,7 +122,7 @@ func (s *storageServer) ListDirHandler(args *ListDirArgs, reply *[]string) error // ReadAllHandler - read all handler is rpc wrapper to read all storage API. func (s *storageServer) ReadAllHandler(args *ReadFileArgs, reply *[]byte) error { - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } buf, err := s.storage.ReadAll(args.Vol, args.Path) @@ -135,7 +135,7 @@ func (s *storageServer) ReadAllHandler(args *ReadFileArgs, reply *[]byte) error // ReadFileHandler - read file handler is rpc wrapper to read file. func (s *storageServer) ReadFileHandler(args *ReadFileArgs, reply *[]byte) (err error) { - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } @@ -154,7 +154,7 @@ func (s *storageServer) ReadFileHandler(args *ReadFileArgs, reply *[]byte) (err // PrepareFileHandler - prepare file handler is rpc wrapper to prepare file. func (s *storageServer) PrepareFileHandler(args *PrepareFileArgs, reply *GenericReply) error { - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } return s.storage.PrepareFile(args.Vol, args.Path, args.Size) @@ -162,7 +162,7 @@ func (s *storageServer) PrepareFileHandler(args *PrepareFileArgs, reply *Generic // AppendFileHandler - append file handler is rpc wrapper to append file. func (s *storageServer) AppendFileHandler(args *AppendFileArgs, reply *GenericReply) error { - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } return s.storage.AppendFile(args.Vol, args.Path, args.Buffer) @@ -170,7 +170,7 @@ func (s *storageServer) AppendFileHandler(args *AppendFileArgs, reply *GenericRe // DeleteFileHandler - delete file handler is rpc wrapper to delete file. func (s *storageServer) DeleteFileHandler(args *DeleteFileArgs, reply *GenericReply) error { - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } return s.storage.DeleteFile(args.Vol, args.Path) @@ -178,7 +178,7 @@ func (s *storageServer) DeleteFileHandler(args *DeleteFileArgs, reply *GenericRe // RenameFileHandler - rename file handler is rpc wrapper to rename file. func (s *storageServer) RenameFileHandler(args *RenameFileArgs, reply *GenericReply) error { - if !isRPCTokenValid(args.Token) { + if !isAuthTokenValid(args.Token) { return errInvalidToken } return s.storage.RenameFile(args.SrcVol, args.SrcPath, args.DstVol, args.DstPath) diff --git a/cmd/storage-rpc-server_test.go b/cmd/storage-rpc-server_test.go index 14ce5daa4..a64ebf07c 100644 --- a/cmd/storage-rpc-server_test.go +++ b/cmd/storage-rpc-server_test.go @@ -40,17 +40,8 @@ func createTestStorageServer(t *testing.T) *testStorageRPCServer { t.Fatalf("unable initialize config file, %s", err) } - jwt, err := newJWT(defaultInterNodeJWTExpiry, serverConfig.GetCredential()) - if err != nil { - t.Fatalf("unable to get new JWT, %s", err) - } - - err = jwt.Authenticate(serverConfig.GetCredential().AccessKey, serverConfig.GetCredential().SecretKey) - if err != nil { - t.Fatalf("unable for JWT to authenticate, %s", err) - } - - token, err := jwt.GenerateToken(serverConfig.GetCredential().AccessKey) + serverCred := serverConfig.GetCredential() + token, err := authenticateNode(serverCred.AccessKey, serverCred.SecretKey) if err != nil { t.Fatalf("unable for JWT to generate token, %s", err) } diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index b427a6ae2..c6c7a530f 100644 --- a/cmd/test-utils_test.go +++ b/cmd/test-utils_test.go @@ -1077,8 +1077,8 @@ func signRequestV4(req *http.Request, accessKey, secretKey string) error { return nil } -// getCredential generate a credential string. -func getCredential(accessKeyID, location string, t time.Time) string { +// getCredentialString generate a credential string. +func getCredentialString(accessKeyID, location string, t time.Time) string { return accessKeyID + "/" + getScope(t, location) } diff --git a/cmd/web-handlers.go b/cmd/web-handlers.go index 2f4324566..881abf7fc 100644 --- a/cmd/web-handlers.go +++ b/cmd/web-handlers.go @@ -29,8 +29,6 @@ import ( "strings" "time" - jwtgo "github.com/dgrijalva/jwt-go" - jwtreq "github.com/dgrijalva/jwt-go/request" "github.com/dustin/go-humanize" "github.com/gorilla/mux" "github.com/gorilla/rpc/v2/json2" @@ -38,30 +36,6 @@ import ( "github.com/minio/miniobrowser" ) -// isJWTReqAuthenticated validates if any incoming request to be a -// valid JWT authenticated request. -func isJWTReqAuthenticated(req *http.Request) bool { - jwt, err := newJWT(defaultJWTExpiry, serverConfig.GetCredential()) - if err != nil { - errorIf(err, "unable to initialize a new JWT") - return false - } - - var reqCallback jwtgo.Keyfunc - reqCallback = func(token *jwtgo.Token) (interface{}, error) { - if _, ok := token.Method.(*jwtgo.SigningMethodHMAC); !ok { - return nil, fmt.Errorf("Unexpected signing method: %v", token.Header["alg"]) - } - return []byte(jwt.SecretKey), nil - } - token, err := jwtreq.ParseFromRequest(req, jwtreq.AuthorizationHeaderExtractor, reqCallback) - if err != nil { - errorIf(err, "token parsing failed") - return false - } - return token.Valid -} - // WebGenericArgs - empty struct for calls that don't accept arguments // for ex. ServerInfo, GenerateAuth type WebGenericArgs struct{} @@ -84,7 +58,7 @@ type ServerInfoRep struct { // ServerInfo - get server info. func (web *webAPIHandlers) ServerInfo(r *http.Request, args *WebGenericArgs, reply *ServerInfoRep) error { - if !isJWTReqAuthenticated(r) { + if !isHTTPRequestValid(r) { return toJSONError(errAuthentication) } host, err := os.Hostname() @@ -125,7 +99,7 @@ func (web *webAPIHandlers) StorageInfo(r *http.Request, args *GenericArgs, reply if objectAPI == nil { return toJSONError(errServerNotInitialized) } - if !isJWTReqAuthenticated(r) { + if !isHTTPRequestValid(r) { return toJSONError(errAuthentication) } reply.StorageInfo = objectAPI.StorageInfo() @@ -144,7 +118,7 @@ func (web *webAPIHandlers) MakeBucket(r *http.Request, args *MakeBucketArgs, rep if objectAPI == nil { return toJSONError(errServerNotInitialized) } - if !isJWTReqAuthenticated(r) { + if !isHTTPRequestValid(r) { return toJSONError(errAuthentication) } bucketLock := globalNSMutex.NewNSLock(args.BucketName, "") @@ -177,7 +151,7 @@ func (web *webAPIHandlers) ListBuckets(r *http.Request, args *WebGenericArgs, re if objectAPI == nil { return toJSONError(errServerNotInitialized) } - if !isJWTReqAuthenticated(r) { + if !isHTTPRequestValid(r) { return toJSONError(errAuthentication) } buckets, err := objectAPI.ListBuckets() @@ -227,7 +201,7 @@ func (web *webAPIHandlers) ListObjects(r *http.Request, args *ListObjectsArgs, r if objectAPI == nil { return toJSONError(errServerNotInitialized) } - if !isJWTReqAuthenticated(r) { + if !isHTTPRequestValid(r) { return toJSONError(errAuthentication) } marker := "" @@ -271,7 +245,7 @@ func (web *webAPIHandlers) RemoveObject(r *http.Request, args *RemoveObjectArgs, if objectAPI == nil { return toJSONError(errServerNotInitialized) } - if !isJWTReqAuthenticated(r) { + if !isHTTPRequestValid(r) { return toJSONError(errAuthentication) } @@ -318,19 +292,11 @@ type LoginRep struct { // Login - user login handler. func (web *webAPIHandlers) Login(r *http.Request, args *LoginArgs, reply *LoginRep) error { - jwt, err := newJWT(defaultJWTExpiry, serverConfig.GetCredential()) + token, err := authenticateWeb(args.Username, args.Password) if err != nil { return toJSONError(err) } - if err = jwt.Authenticate(args.Username, args.Password); err != nil { - return toJSONError(err) - } - - token, err := jwt.GenerateToken(args.Username) - if err != nil { - return toJSONError(err) - } reply.Token = token reply.UIVersion = miniobrowser.UIVersion return nil @@ -344,7 +310,7 @@ type GenerateAuthReply struct { } func (web webAPIHandlers) GenerateAuth(r *http.Request, args *WebGenericArgs, reply *GenerateAuthReply) error { - if !isJWTReqAuthenticated(r) { + if !isHTTPRequestValid(r) { return toJSONError(errAuthentication) } cred := newCredential() @@ -369,41 +335,16 @@ type SetAuthReply struct { // SetAuth - Set accessKey and secretKey credentials. func (web *webAPIHandlers) SetAuth(r *http.Request, args *SetAuthArgs, reply *SetAuthReply) error { - if !isJWTReqAuthenticated(r) { + if !isHTTPRequestValid(r) { return toJSONError(errAuthentication) } - // Initialize jwt with the new access keys, fail if not possible. - jwt, err := newJWT(defaultJWTExpiry, credential{ - AccessKey: args.AccessKey, - SecretKey: args.SecretKey, - }) // JWT Expiry set to 24Hrs. + // As we already validated the authentication, we save given access/secret keys. + cred, err := getCredential(args.AccessKey, args.SecretKey) if err != nil { return toJSONError(err) } - // Authenticate the secret key properly. - if err = jwt.Authenticate(args.AccessKey, args.SecretKey); err != nil { - return toJSONError(err) - - } - - unexpErrsMsg := "Unexpected error(s) occurred - please check minio server logs." - gaveUpMsg := func(errMsg error, moreErrors bool) *json2.Error { - msg := fmt.Sprintf( - "We gave up due to: '%s', but there were more errors. Please check minio server logs.", - errMsg.Error(), - ) - var err *json2.Error - if moreErrors { - err = toJSONError(errors.New(msg)) - } else { - err = toJSONError(errMsg) - } - return err - } - - cred := credential{args.AccessKey, args.SecretKey} // Notify all other Minio peers to update credentials errsMap := updateCredsOnPeers(cred) @@ -427,19 +368,21 @@ func (web *webAPIHandlers) SetAuth(r *http.Request, args *SetAuthArgs, reply *Se // Since the error message may be very long to display // on the browser, we tell the user to check the // server logs. - return toJSONError(errors.New(unexpErrsMsg)) + return toJSONError(errors.New("unexpected error(s) occurred - please check minio server logs")) } - // Did we have peer errors? - var moreErrors bool - if len(errsMap) > 0 { - moreErrors = true - } - - // Generate a JWT token. - token, err := jwt.GenerateToken(args.AccessKey) + // As we have updated access/secret key, generate new auth token. + token, err := authenticateWeb(args.AccessKey, args.SecretKey) if err != nil { - return gaveUpMsg(err, moreErrors) + // Did we have peer errors? + if len(errsMap) > 0 { + err = fmt.Errorf( + "we gave up due to: '%s', but there were more errors. Please check minio server logs", + err.Error(), + ) + } + + return toJSONError(err) } reply.Token = token @@ -456,7 +399,7 @@ type GetAuthReply struct { // GetAuth - return accessKey and secretKey credentials. func (web *webAPIHandlers) GetAuth(r *http.Request, args *WebGenericArgs, reply *GetAuthReply) error { - if !isJWTReqAuthenticated(r) { + if !isHTTPRequestValid(r) { return toJSONError(errAuthentication) } creds := serverConfig.GetCredential() @@ -474,7 +417,7 @@ func (web *webAPIHandlers) Upload(w http.ResponseWriter, r *http.Request) { return } - if !isJWTReqAuthenticated(r) { + if !isHTTPRequestValid(r) { writeWebErrorResponse(w, errAuthentication) return } @@ -519,24 +462,13 @@ func (web *webAPIHandlers) Download(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) bucket := vars["bucket"] object := vars["object"] - tokenStr := r.URL.Query().Get("token") + token := r.URL.Query().Get("token") - jwt, err := newJWT(defaultJWTExpiry, serverConfig.GetCredential()) // Expiry set to 24Hrs. - if err != nil { - errorIf(err, "error in getting new JWT") - return - } - - token, e := jwtgo.Parse(tokenStr, func(token *jwtgo.Token) (interface{}, error) { - if _, ok := token.Method.(*jwtgo.SigningMethodHMAC); !ok { - return nil, fmt.Errorf("Unexpected signing method: %v", token.Header["alg"]) - } - return []byte(jwt.SecretKey), nil - }) - if e != nil || !token.Valid { + if !isAuthTokenValid(token) { writeWebErrorResponse(w, errAuthentication) return } + // Add content disposition. w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=\"%s\"", path.Base(object))) @@ -601,7 +533,7 @@ func (web *webAPIHandlers) GetBucketPolicy(r *http.Request, args *GetBucketPolic return toJSONError(errServerNotInitialized) } - if !isJWTReqAuthenticated(r) { + if !isHTTPRequestValid(r) { return toJSONError(errAuthentication) } @@ -640,7 +572,7 @@ func (web *webAPIHandlers) ListAllBucketPolicies(r *http.Request, args *ListAllB return toJSONError(errServerNotInitialized) } - if !isJWTReqAuthenticated(r) { + if !isHTTPRequestValid(r) { return toJSONError(errAuthentication) } @@ -673,7 +605,7 @@ func (web *webAPIHandlers) SetBucketPolicy(r *http.Request, args *SetBucketPolic return toJSONError(errServerNotInitialized) } - if !isJWTReqAuthenticated(r) { + if !isHTTPRequestValid(r) { return toJSONError(errAuthentication) } @@ -741,7 +673,7 @@ type PresignedGetRep struct { // PresignedGET - returns presigned-Get url. func (web *webAPIHandlers) PresignedGet(r *http.Request, args *PresignedGetArgs, reply *PresignedGetRep) error { - if !isJWTReqAuthenticated(r) { + if !isHTTPRequestValid(r) { return toJSONError(errAuthentication) }