Restrict access keys for users and groups to not allow '=' or ',' (#19749)

* initial commit

* Add UTF check

---------

Co-authored-by: Harshavardhana <harsha@minio.io>

cmd/admin-handlers-users.go

@@ -29,6 +29,7 @@ import (
 	"sort"
 	"strconv"
 	"time"
+	"unicode/utf8"
 
 	"github.com/klauspost/compress/zip"
 	"github.com/minio/madmin-go/v3"
@@ -474,6 +475,11 @@ func (a adminAPIHandlers) AddUser(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if !utf8.ValidString(accessKey) {
+		writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAddUserValidUTF), r.URL)
+		return
+	}
+
 	checkDenyOnly := false
 	if accessKey == cred.AccessKey {
 		// Check that there is no explicit deny - otherwise it's allowed

cmd/api-errors.go

@@ -287,6 +287,7 @@ const (
 	ErrAdminNoSuchGroup
 	ErrAdminGroupNotEmpty
 	ErrAdminGroupDisabled
+	ErrAdminInvalidGroupName
 	ErrAdminNoSuchJob
 	ErrAdminNoSuchPolicy
 	ErrAdminPolicyChangeAlreadyApplied
@@ -425,6 +426,7 @@ const (
 	ErrAdminProfilerNotEnabled
 	ErrInvalidDecompressedSize
 	ErrAddUserInvalidArgument
+	ErrAddUserValidUTF
 	ErrAdminResourceInvalidArgument
 	ErrAdminAccountNotEligible
 	ErrAccountNotEligible
@@ -2101,6 +2103,16 @@ var errorCodes = errorCodeMap{
 		Description:    "Expected LDAP short username but was given full DN.",
 		HTTPStatusCode: http.StatusBadRequest,
 	},
+	ErrAdminInvalidGroupName: {
+		Code:           "XMinioInvalidGroupName",
+		Description:    "The group name is invalid.",
+		HTTPStatusCode: http.StatusBadRequest,
+	},
+	ErrAddUserValidUTF: {
+		Code:           "XMinioInvalidUTF",
+		Description:    "Invalid UTF-8 character detected.",
+		HTTPStatusCode: http.StatusBadRequest,
+	},
 }
 
 // toAPIErrorCode - Converts embedded errors. Convenience
@@ -2140,6 +2152,8 @@ func toAPIErrorCode(ctx context.Context, err error) (apiErr APIErrorCode) {
 		apiErr = ErrAdminNoSuchGroup
 	case errGroupNotEmpty:
 		apiErr = ErrAdminGroupNotEmpty
+	case errGroupNameContainsReservedChars:
+		apiErr = ErrAdminInvalidGroupName
 	case errNoSuchJob:
 		apiErr = ErrAdminNoSuchJob
 	case errNoPolicyToAttachOrDetach:
@@ -2154,6 +2168,8 @@ func toAPIErrorCode(ctx context.Context, err error) (apiErr APIErrorCode) {
 		apiErr = ErrEntityTooSmall
 	case errAuthentication:
 		apiErr = ErrAccessDenied
+	case auth.ErrContainsReservedChars:
+		apiErr = ErrAdminInvalidAccessKey
 	case auth.ErrInvalidAccessKeyLength:
 		apiErr = ErrAdminInvalidAccessKey
 	case auth.ErrInvalidSecretKeyLength:

cmd/iam.go

@@ -1273,6 +1273,10 @@ func (sys *IAMSys) CreateUser(ctx context.Context, accessKey string, ureq madmin
 		return updatedAt, auth.ErrInvalidAccessKeyLength
 	}
 
+	if auth.ContainsReservedChars(accessKey) {
+		return updatedAt, auth.ErrContainsReservedChars
+	}
+
 	if !auth.IsSecretKeyValid(ureq.SecretKey) {
 		return updatedAt, auth.ErrInvalidSecretKeyLength
 	}
@@ -1766,6 +1770,10 @@ func (sys *IAMSys) AddUsersToGroup(ctx context.Context, group string, members []
 		return updatedAt, errServerNotInitialized
 	}
 
+	if auth.ContainsReservedChars(group) {
+		return updatedAt, errGroupNameContainsReservedChars
+	}
+
 	updatedAt, err = sys.store.AddUsersToGroup(ctx, group, members)
 	if err != nil {
 		return updatedAt, err

cmd/typed-errors.go

@@ -125,3 +125,6 @@ var errSftpPublicKeyWithoutCert = errors.New("public key authentication without
 
 // error returned in SFTP when user used certificate which does not contain principal(s)
 var errSftpCertWithoutPrincipals = errors.New("certificates without principal(s) are not accepted")
+
+// error returned when group name contains reserved characters
+var errGroupNameContainsReservedChars = errors.New("Group name contains reserved characters '=' or ','")

internal/auth/credentials.go

@@ -54,6 +54,8 @@ const (
 
 	// Total length of the alpha numeric table.
 	alphaNumericTableLen = byte(len(alphaNumericTable))
+
+	reservedChars = "=,"
 )
 
 // Common errors generated for access and secret key validation.
@@ -62,11 +64,17 @@ var (
 	ErrInvalidSecretKeyLength   = fmt.Errorf("secret key length should be between %d and %d", secretKeyMinLen, secretKeyMaxLen)
 	ErrNoAccessKeyWithSecretKey = fmt.Errorf("access key must be specified if secret key is specified")
 	ErrNoSecretKeyWithAccessKey = fmt.Errorf("secret key must be specified if access key is specified")
+	ErrContainsReservedChars    = fmt.Errorf("access key contains one of reserved characters '=' or ','")
 )
 
 // AnonymousCredentials simply points to empty credentials
 var AnonymousCredentials = Credentials{}
 
+// ContainsReservedChars - returns whether the input string contains reserved characters.
+func ContainsReservedChars(s string) bool {
+	return strings.ContainsAny(s, reservedChars)
+}
+
 // IsAccessKeyValid - validate access key for right length.
 func IsAccessKeyValid(accessKey string) bool {
 	return len(accessKey) >= accessKeyMinLen