From 0f401b67ad37a22abdf213eec9d706c8030ae290 Mon Sep 17 00:00:00 2001 From: ebozduman Date: Thu, 3 Aug 2017 20:03:37 -0700 Subject: [PATCH] Removes max limit requirement on accessKey and secretKey length (#4730) --- cmd/credential.go | 47 +++++++++++++++++++++++++----------------- cmd/credential_test.go | 20 +++++++----------- cmd/jwt_test.go | 28 +++++++++++-------------- cmd/server-main.go | 4 ++-- docs/config/README.md | 4 ++-- 5 files changed, 52 insertions(+), 51 deletions(-) diff --git a/cmd/credential.go b/cmd/credential.go index d3ff0c618..306e36c04 100644 --- a/cmd/credential.go +++ b/cmd/credential.go @@ -29,6 +29,7 @@ const ( accessKeyMinLen = 5 // Maximum length for Minio access key. + // There is no max length enforcement for access keys accessKeyMaxLen = 20 // Minimum length for Minio secret key for both server and gateway mode. @@ -36,11 +37,8 @@ const ( // Maximum secret key length for Minio, this // is used when autogenerating new credentials. - secretKeyMaxLenMinio = 40 - - // Maximum secret key length allowed from client side - // caters for both server and gateway mode. - secretKeyMaxLen = 100 + // There is no max length enforcement for secret keys + secretKeyMaxLen = 40 // Alpha numeric table used for generating access keys. alphaNumericTable = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ" @@ -51,18 +49,18 @@ const ( // Common errors generated for access and secret key validation. var ( - errInvalidAccessKeyLength = errors.New("Invalid access key, access key should be 5 to 20 characters in length") - errInvalidSecretKeyLength = errors.New("Invalid secret key, secret key should be 8 to 100 characters in length") + errInvalidAccessKeyLength = errors.New("Invalid access key, access key should be minimum 5 characters in length") + errInvalidSecretKeyLength = errors.New("Invalid secret key, secret key should be minimum 8 characters in length") ) // isAccessKeyValid - validate access key for right length. func isAccessKeyValid(accessKey string) bool { - return len(accessKey) >= accessKeyMinLen && len(accessKey) <= accessKeyMaxLen + return len(accessKey) >= accessKeyMinLen } // isSecretKeyValid - validate secret key for right length. func isSecretKeyValid(secretKey string) bool { - return len(secretKey) >= secretKeyMinLen && len(secretKey) <= secretKeyMaxLen + return len(secretKey) >= secretKeyMinLen } // credential container for access and secret keys. @@ -116,24 +114,35 @@ func createCredential(accessKey, secretKey string) (cred credential, err error) } // Initialize a new credential object -func mustGetNewCredential() credential { +func getNewCredential(accessKeyLen, secretKeyLen int) (cred credential, err error) { // Generate access key. - keyBytes := make([]byte, accessKeyMaxLen) - _, err := rand.Read(keyBytes) - fatalIf(err, "Unable to generate access key.") - for i := 0; i < accessKeyMaxLen; i++ { + keyBytes := make([]byte, accessKeyLen) + _, err = rand.Read(keyBytes) + if err != nil { + return cred, err + } + + for i := 0; i < accessKeyLen; i++ { keyBytes[i] = alphaNumericTable[keyBytes[i]%alphaNumericTableLen] } accessKey := string(keyBytes) // Generate secret key. - keyBytes = make([]byte, secretKeyMaxLenMinio) + keyBytes = make([]byte, secretKeyLen) _, err = rand.Read(keyBytes) - fatalIf(err, "Unable to generate secret key.") - secretKey := string([]byte(base64.StdEncoding.EncodeToString(keyBytes))[:secretKeyMaxLenMinio]) + if err != nil { + return cred, err + } - cred, err := createCredential(accessKey, secretKey) - fatalIf(err, "Unable to generate new credential.") + secretKey := string([]byte(base64.StdEncoding.EncodeToString(keyBytes))[:secretKeyLen]) + cred, err = createCredential(accessKey, secretKey) + return cred, err +} + +func mustGetNewCredential() credential { + // Generate Minio credentials with Minio key max lengths. + cred, err := getNewCredential(accessKeyMaxLen, secretKeyMaxLen) + fatalIf(err, "Unable to generate new credentials.") return cred } diff --git a/cmd/credential_test.go b/cmd/credential_test.go index ebf19e787..ac04c66ce 100644 --- a/cmd/credential_test.go +++ b/cmd/credential_test.go @@ -23,8 +23,8 @@ func TestMustGetNewCredential(t *testing.T) { if !cred.IsValid() { t.Fatalf("Failed to get new valid credential") } - if len(cred.SecretKey) != secretKeyMaxLenMinio { - t.Fatalf("Invalid length %d of the secretKey credential generated, expected %d", len(cred.SecretKey), secretKeyMaxLenMinio) + if len(cred.SecretKey) != secretKeyMaxLen { + t.Fatalf("Invalid length %d of the secretKey credential generated, expected %d", len(cred.SecretKey), secretKeyMaxLen) } } @@ -36,18 +36,14 @@ func TestCreateCredential(t *testing.T) { expectedResult bool expectedErr error }{ - // Access key too small. + // Access key too small (min 5 chars). {"user", "pass", false, errInvalidAccessKeyLength}, - // Access key too long. - {"user12345678901234567", "pass", false, errInvalidAccessKeyLength}, - // Access key contains unsuppported characters. - {"!@#$", "pass", false, errInvalidAccessKeyLength}, - // Secret key too small. + // Long access key is ok. + {"user123456789012345678901234567890", "password", true, nil}, + // Secret key too small (min 8 chars). {"myuser", "pass", false, errInvalidSecretKeyLength}, - // Secret key too long. - {"myuser", "pass1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890", false, errInvalidSecretKeyLength}, - // Success when access key contains leading/trailing spaces. - {" user ", cred.SecretKey, true, nil}, + // Long secret key is ok. + {"myuser", "pass1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890", true, nil}, {"myuser", "mypassword", true, nil}, {cred.AccessKey, cred.SecretKey, true, nil}, } diff --git a/cmd/jwt_test.go b/cmd/jwt_test.go index ac0c512d1..dd69e9520 100644 --- a/cmd/jwt_test.go +++ b/cmd/jwt_test.go @@ -24,8 +24,12 @@ func testAuthenticate(authType string, t *testing.T) { t.Fatalf("unable initialize config file, %s", err) } defer removeAll(testPath) - - serverCred := serverConfig.GetCredential() + // Create access and secret keys in length, 300 and 600 + cred, err := getNewCredential(300, 600) + if err != nil { + t.Fatalf("unable to get new credential, %v", err) + } + serverConfig.SetCredential(cred) // Define test cases. testCases := []struct { @@ -33,24 +37,16 @@ func testAuthenticate(authType string, t *testing.T) { 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}, - // Success when access key contains leading/trailing spaces. - {" " + serverCred.AccessKey + " ", serverCred.SecretKey, errInvalidAccessKeyLength}, - // Secret key too small. - {"myuser", "pass", errInvalidSecretKeyLength}, - // Secret key too long. - {"myuser", "pass1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890", errInvalidSecretKeyLength}, + // Access key (less than 5 chrs) too small. + {"user", cred.SecretKey, errInvalidAccessKeyLength}, + // Secret key (less than 8 chrs) too small. + {cred.AccessKey, "pass", errInvalidSecretKeyLength}, // Authentication error. {"myuser", "mypassword", errInvalidAccessKeyID}, // Authentication error. - {serverCred.AccessKey, "mypassword", errAuthentication}, + {cred.AccessKey, "mypassword", errAuthentication}, // Success. - {serverCred.AccessKey, serverCred.SecretKey, nil}, + {cred.AccessKey, cred.SecretKey, nil}, } // Run tests. diff --git a/cmd/server-main.go b/cmd/server-main.go index dadf2df30..930d7b352 100644 --- a/cmd/server-main.go +++ b/cmd/server-main.go @@ -52,8 +52,8 @@ FLAGS: {{end}}{{end}} ENVIRONMENT VARIABLES: ACCESS: - MINIO_ACCESS_KEY: Custom username or access key of 5 to 20 characters in length. - MINIO_SECRET_KEY: Custom password or secret key of 8 to 40 characters in length. + MINIO_ACCESS_KEY: Custom username or access key of minimum 5 characters in length. + MINIO_SECRET_KEY: Custom password or secret key of minimum 8 characters in length. BROWSER: MINIO_BROWSER: To disable web browser access, set this value to "off". diff --git a/docs/config/README.md b/docs/config/README.md index 4ba4f5061..68ea5d468 100644 --- a/docs/config/README.md +++ b/docs/config/README.md @@ -34,8 +34,8 @@ $ tree ~/.minio |Field|Type|Description| |:---|:---|:---| |``credential``| | Auth credential for object storage and web access.| -|``credential.accessKey`` | _string_ | Access key of 5 to 20 characters in length. You may override this field with `MINIO_ACCESS_KEY` environment variable.| -|``credential.secretKey`` | _string_ | Secret key of 8 to 40 characters in length. You may override this field with `MINIO_SECRET_KEY` environment variable.| +|``credential.accessKey`` | _string_ | Access key of minimum 5 characters in length. You may override this field with `MINIO_ACCESS_KEY` environment variable.| +|``credential.secretKey`` | _string_ | Secret key of minimum 8 characters in length. You may override this field with `MINIO_SECRET_KEY` environment variable.| Example: