self-hosted/minio
self-hosted/minio
1
0
Fork 0
mirror of https://github.com/minio/minio synced 2024-10-14 11:52:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: IAM LDAP access key import bug (#19608)

Browse source 
When importing access keys (i.e. service accounts) for LDAP accounts,
we are requiring groups to exist under one of the configured group base
DNs. This is not correct. This change fixes this by only checking for
existence and storing the normalized form of the group DN - we do not
return an error if the group is not under a base DN.

Test is updated to illustrate an import failure that would happen
without this change.
This commit is contained in:
Aditya Manthramurthy 2024-04-25 08:50:16 -07:00 committed by GitHub
parent 3212d0c8cd
commit 62c3cdee75
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 62 additions and 38 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
cmd/admin-handlers-idp-ldap.go
View file

@ -222,9 +222,8 @@ func (a adminAPIHandlers) AddServiceAccountLDAP(w http.ResponseWriter, r *http.R
err error err error
) )
// If we are creating svc account for request sender, ensure // If we are creating svc account for request sender, ensure that targetUser
// that targetUser is a real user (i.e. not derived // is a real user (i.e. not derived credentials).
// credentials).
if isSvcAccForRequestor { if isSvcAccForRequestor {
if requestorIsDerivedCredential { if requestorIsDerivedCredential {
if requestorParentUser == "" { if requestorParentUser == "" {

5
cmd/admin-handlers-users.go
View file

@ -1632,9 +1632,10 @@ func (a adminAPIHandlers) SetPolicyForUserOrGroup(w http.ResponseWriter, r *http
var err error var err error
if isGroup { if isGroup {
var foundGroupDN string var foundGroupDN string
if foundGroupDN, err = globalIAMSys.LDAPConfig.GetValidatedGroupDN(nil, entityName); err != nil { var underBaseDN bool
if foundGroupDN, underBaseDN, err = globalIAMSys.LDAPConfig.GetValidatedGroupDN(nil, entityName); err != nil {
iamLogIf(ctx, err) iamLogIf(ctx, err)
} else if foundGroupDN == "" { } else if foundGroupDN == "" || !underBaseDN {
err = errNoSuchGroup err = errNoSuchGroup
} }
entityName = foundGroupDN entityName = foundGroupDN

27
cmd/iam.go
View file

@ -1503,12 +1503,14 @@ func (sys *IAMSys) NormalizeLDAPAccessKeypairs(ctx context.Context, accessKeyMap
hasDiff := false hasDiff := false
validatedParent, err := sys.LDAPConfig.GetValidatedUserDN(conn, parent) // For the parent value, we require that the parent exists in the LDAP
// server and is under a configured base DN.
validatedParent, isUnderBaseDN, err := sys.LDAPConfig.GetValidatedUserDN(conn, parent)
if err != nil { if err != nil {
collectedErrors = append(collectedErrors, fmt.Errorf("could not validate `%s` exists in LDAP directory: %w", parent, err)) collectedErrors = append(collectedErrors, fmt.Errorf("could not validate `%s` exists in LDAP directory: %w", parent, err))
continue continue
} }
if validatedParent == "" { if validatedParent == "" || !isUnderBaseDN {
err := fmt.Errorf("DN `%s` was not found in the LDAP directory", parent) err := fmt.Errorf("DN `%s` was not found in the LDAP directory", parent)
collectedErrors = append(collectedErrors, err) collectedErrors = append(collectedErrors, err)
continue continue
@ -1518,9 +1520,11 @@ func (sys *IAMSys) NormalizeLDAPAccessKeypairs(ctx context.Context, accessKeyMap
hasDiff = true hasDiff = true
} }
var validatedGroups []string var normalizedGroups []string
for _, group := range groups { for _, group := range groups {
validatedGroup, err := sys.LDAPConfig.GetValidatedGroupDN(conn, group) // For a group, we store the normalized DN even if it not under a
// configured base DN.
validatedGroup, _, err := sys.LDAPConfig.GetValidatedGroupDN(conn, group)
if err != nil { if err != nil {
collectedErrors = append(collectedErrors, fmt.Errorf("could not validate `%s` exists in LDAP directory: %w", group, err)) collectedErrors = append(collectedErrors, fmt.Errorf("could not validate `%s` exists in LDAP directory: %w", group, err))
continue continue
@ -1534,13 +1538,13 @@ func (sys *IAMSys) NormalizeLDAPAccessKeypairs(ctx context.Context, accessKeyMap
if validatedGroup != group { if validatedGroup != group {
hasDiff = true hasDiff = true
} }
validatedGroups = append(validatedGroups, validatedGroup) normalizedGroups = append(normalizedGroups, validatedGroup)
} }
if hasDiff { if hasDiff {
updatedCreateReq := createReq updatedCreateReq := createReq
updatedCreateReq.Parent = validatedParent updatedCreateReq.Parent = validatedParent
updatedCreateReq.Groups = validatedGroups updatedCreateReq.Groups = normalizedGroups
updatedKeysMap[ak] = updatedCreateReq updatedKeysMap[ak] = updatedCreateReq
} }
@ -1611,7 +1615,7 @@ func (sys *IAMSys) NormalizeLDAPMappingImport(ctx context.Context, isGroup bool,
// We map keys that correspond to LDAP DNs and validate that they exist in // We map keys that correspond to LDAP DNs and validate that they exist in
// the LDAP server. // the LDAP server.
var dnValidator func(*libldap.Conn, string) (string, error) = sys.LDAPConfig.GetValidatedUserDN var dnValidator func(*libldap.Conn, string) (string, bool, error) = sys.LDAPConfig.GetValidatedUserDN
if isGroup { if isGroup {
dnValidator = sys.LDAPConfig.GetValidatedGroupDN dnValidator = sys.LDAPConfig.GetValidatedGroupDN
} }
@ -1625,12 +1629,12 @@ func (sys *IAMSys) NormalizeLDAPMappingImport(ctx context.Context, isGroup bool,
// not a valid DN, ignore. // not a valid DN, ignore.
continue continue
} }
validatedDN, err := dnValidator(conn, k) validatedDN, underBaseDN, err := dnValidator(conn, k)
if err != nil { if err != nil {
collectedErrors = append(collectedErrors, fmt.Errorf("could not validate `%s` exists in LDAP directory: %w", k, err)) collectedErrors = append(collectedErrors, fmt.Errorf("could not validate `%s` exists in LDAP directory: %w", k, err))
continue continue
} }
if validatedDN == "" { if validatedDN == "" || !underBaseDN {
err := fmt.Errorf("DN `%s` was not found in the LDAP directory", k) err := fmt.Errorf("DN `%s` was not found in the LDAP directory", k)
collectedErrors = append(collectedErrors, err) collectedErrors = append(collectedErrors, err)
continue continue
@ -1949,10 +1953,11 @@ func (sys *IAMSys) PolicyDBUpdateLDAP(ctx context.Context, isAttach bool,
} else { } else {
if isAttach { if isAttach {
var foundGroupDN string var foundGroupDN string
if foundGroupDN, err = sys.LDAPConfig.GetValidatedGroupDN(nil, r.Group); err != nil { var underBaseDN bool
if foundGroupDN, underBaseDN, err = sys.LDAPConfig.GetValidatedGroupDN(nil, r.Group); err != nil {
iamLogIf(ctx, err) iamLogIf(ctx, err)
return return
} else if foundGroupDN == "" { } else if foundGroupDN == "" || !underBaseDN {
err = errNoSuchGroup err = errNoSuchGroup
return return
} }

5
cmd/site-replication.go
View file

@ -1441,9 +1441,10 @@ func (c *SiteReplicationSys) PeerPolicyMappingHandler(ctx context.Context, mappi
var err error var err error
if isGroup { if isGroup {
var foundGroupDN string var foundGroupDN string
if foundGroupDN, err = globalIAMSys.LDAPConfig.GetValidatedGroupDN(nil, entityName); err != nil { var underBaseDN bool
if foundGroupDN, underBaseDN, err = globalIAMSys.LDAPConfig.GetValidatedGroupDN(nil, entityName); err != nil {
iamLogIf(ctx, err) iamLogIf(ctx, err)
} else if foundGroupDN == "" { } else if foundGroupDN == "" || !underBaseDN {
err = errNoSuchGroup err = errNoSuchGroup
} }
entityName = foundGroupDN entityName = foundGroupDN

4
cmd/sts-handlers_test.go
View file

@ -829,12 +829,14 @@ func TestIAMImportAssetWithLDAP(t *testing.T) {
} }
} }
`, `,
// The `cn=projecty,..` group below is not under a configured DN, but we
// should still import without an error.
allSvcAcctsFile: `{ allSvcAcctsFile: `{
"u4ccRswj62HV3Ifwima7": { "u4ccRswj62HV3Ifwima7": {
"parent": "uid=svc.algorithm,OU=swengg,DC=min,DC=io", "parent": "uid=svc.algorithm,OU=swengg,DC=min,DC=io",
"accessKey": "u4ccRswj62HV3Ifwima7", "accessKey": "u4ccRswj62HV3Ifwima7",
"secretKey": "ZoEoZdLlzVbOlT9rbhD7ZN7TLyiYXSAlB79uGEge", "secretKey": "ZoEoZdLlzVbOlT9rbhD7ZN7TLyiYXSAlB79uGEge",
"groups": ["cn=project.c,ou=groups,OU=swengg,DC=min,DC=io"], "groups": ["cn=project.c,ou=groups,OU=swengg,DC=min,DC=io", "cn=projecty,ou=groups,ou=hwengg,dc=min,dc=io"],
"claims": { "claims": {
"accessKey": "u4ccRswj62HV3Ifwima7", "accessKey": "u4ccRswj62HV3Ifwima7",
"ldapUser": "uid=svc.algorithm,ou=swengg,dc=min,dc=io", "ldapUser": "uid=svc.algorithm,ou=swengg,dc=min,dc=io",

54
internal/config/identity/ldap/ldap.go
View file

@ -86,7 +86,7 @@ func (l *Config) GetValidatedDNForUsername(username string) (string, error) {
// the directory. // the directory.
bindDN, err := l.LDAP.LookupUserDN(conn, username) bindDN, err := l.LDAP.LookupUserDN(conn, username)
if err != nil { if err != nil {
if strings.Contains(err.Error(), "not found") { if strings.Contains(err.Error(), "User DN not found for") {
return "", nil return "", nil
} }
return "", fmt.Errorf("Unable to find user DN: %w", err) return "", fmt.Errorf("Unable to find user DN: %w", err)
@ -94,30 +94,36 @@ func (l *Config) GetValidatedDNForUsername(username string) (string, error) {
return bindDN, nil return bindDN, nil
} }
// Since the username is a valid DN, check that it is under a configured // Since the username parses as a valid DN, check that it exists and is
// base DN in the LDAP directory. // under a configured base DN in the LDAP directory.
return l.GetValidatedUserDN(conn, username) validDN, isUnderBaseDN, err := l.GetValidatedUserDN(conn, username)
if err == nil && !isUnderBaseDN {
return "", fmt.Errorf("Unable to find user DN: %w", err)
}
return validDN, err
} }
// GetValidatedUserDN validates the given user DN. Will error out if conn is nil. // GetValidatedUserDN validates the given user DN. Will error out if conn is nil. The returned
func (l *Config) GetValidatedUserDN(conn *ldap.Conn, userDN string) (string, error) { // boolean is true iff the user DN is found under one of the LDAP user base DNs.
func (l *Config) GetValidatedUserDN(conn *ldap.Conn, userDN string) (string, bool, error) {
return l.GetValidatedDNUnderBaseDN(conn, userDN, l.LDAP.UserDNSearchBaseDistNames) return l.GetValidatedDNUnderBaseDN(conn, userDN, l.LDAP.UserDNSearchBaseDistNames)
} }
// GetValidatedGroupDN validates the given group DN. If conn is nil, creates a // GetValidatedGroupDN validates the given group DN. If conn is nil, creates a
// connection. // connection. The returned boolean is true iff the group DN is found under one
func (l *Config) GetValidatedGroupDN(conn *ldap.Conn, groupDN string) (string, error) { // of the configured LDAP base DNs.
func (l *Config) GetValidatedGroupDN(conn *ldap.Conn, groupDN string) (string, bool, error) {
if conn == nil { if conn == nil {
var err error var err error
conn, err = l.LDAP.Connect() conn, err = l.LDAP.Connect()
if err != nil { if err != nil {
return "", err return "", false, err
} }
defer conn.Close() defer conn.Close()
// Bind to the lookup user account // Bind to the lookup user account
if err = l.LDAP.LookupBind(conn); err != nil { if err = l.LDAP.LookupBind(conn); err != nil {
return "", err return "", false, err
} }
} }
@ -128,22 +134,30 @@ func (l *Config) GetValidatedGroupDN(conn *ldap.Conn, groupDN string) (string, e
// and returns the DN value sent by the LDAP server. The value returned by the // and returns the DN value sent by the LDAP server. The value returned by the
// server may not be equal to the input DN, as LDAP equality is not a simple // server may not be equal to the input DN, as LDAP equality is not a simple
// Golang string equality. However, we assume the value returned by the LDAP // Golang string equality. However, we assume the value returned by the LDAP
// server is canonical. // server is canonical. Additionally, the attribute type names in the DN are
// lower-cased.
// //
// If the DN is not found in the LDAP directory, the returned string is empty // Return values:
// and err = nil. //
func (l *Config) GetValidatedDNUnderBaseDN(conn *ldap.Conn, dn string, baseDNList []xldap.BaseDNInfo) (string, error) { // If the DN is found, the normalized (string) value is returned and error is
// nil.
//
// If the DN is not found, the string returned is empty and the error is nil.
//
// The returned boolean is true iff the DN is found under one of the LDAP
// subtrees listed in `baseDNList`.
func (l *Config) GetValidatedDNUnderBaseDN(conn *ldap.Conn, dn string, baseDNList []xldap.BaseDNInfo) (string, bool, error) {
if len(baseDNList) == 0 { if len(baseDNList) == 0 {
return "", errors.New("no Base DNs given") return "", false, errors.New("no Base DNs given")
} }
// Check that DN exists in the LDAP directory. // Check that DN exists in the LDAP directory.
validatedDN, err := xldap.LookupDN(conn, dn) validatedDN, err := xldap.LookupDN(conn, dn)
if err != nil { if err != nil {
return "", fmt.Errorf("Error looking up DN %s: %w", dn, err) return "", false, fmt.Errorf("Error looking up DN %s: %w", dn, err)
} }
if validatedDN == "" { if validatedDN == "" {
return "", nil return "", false, nil
} }
// This will not return an error as the argument is validated to be a DN. // This will not return an error as the argument is validated to be a DN.
@ -153,10 +167,12 @@ func (l *Config) GetValidatedDNUnderBaseDN(conn *ldap.Conn, dn string, baseDNLis
// directory. // directory.
for _, baseDN := range baseDNList { for _, baseDN := range baseDNList {
if baseDN.Parsed.AncestorOf(pdn) { if baseDN.Parsed.AncestorOf(pdn) {
return validatedDN, nil return validatedDN, true, nil
} }
} }
return "", fmt.Errorf("DN %s is not under any configured base DN", validatedDN)
// Not under any configured base DN so return false.
return validatedDN, false, nil
} }
// Bind - binds to ldap, searches LDAP and returns the distinguished name of the // Bind - binds to ldap, searches LDAP and returns the distinguished name of the