MyFavMirrors/minio
1
0
Fork 0
mirror of https://github.com/minio/minio.git synced 2025-02-03 18:06:00 -05:00
Code Issues Packages Projects Releases Wiki Activity

ldap: improve normalization of DN values (#19358)

Browse Source 
Instead of relying on user input values, we use the DN value returned by
the LDAP server.

This handles cases like when a mapping is set on a DN value
`uid=svc.algorithm,OU=swengg,DC=min,DC=io` with a user input value (with
unicode variation) of `uid=svc﹒algorithm,OU=swengg,DC=min,DC=io`. The
LDAP server on lookup of this DN returns the normalized value where the
unicode dot character `SMALL FULL STOP` (in the user input), gets
replaced with regular full stop.
This commit is contained in:
Aditya Manthramurthy 2024-03-27 23:45:26 -07:00 committed by GitHub
parent 139a606f0a
commit 7e45d84ace
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 222 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
cmd/admin-handlers-idp-ldap.go
View File

@ -233,12 +233,12 @@ func (a adminAPIHandlers) AddServiceAccountLDAP(w http.ResponseWriter, r *http.R
targetGroups = requestorGroups
// Deny if the target user is not LDAP
isLDAP, err := globalIAMSys.LDAPConfig.DoesUsernameExist(targetUser)
foundLDAPDN, err := globalIAMSys.LDAPConfig.GetValidatedDNForUsername(targetUser)
if err != nil {
writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL)
return
}
if isLDAP == "" {
if foundLDAPDN == "" {
err := errors.New("Specified user does not exist on LDAP server")
APIErr := errorCodes.ToAPIErrWithErr(ErrAdminNoSuchUser, err)
writeErrorResponseJSON(ctx, w, APIErr, r.URL)
@ -374,7 +374,7 @@ func (a adminAPIHandlers) ListAccessKeysLDAP(w http.ResponseWriter, r *http.Requ
}
}
targetAccount, err := globalIAMSys.LDAPConfig.DoesUsernameExist(userDN)
targetAccount, err := globalIAMSys.LDAPConfig.GetValidatedDNForUsername(userDN)
if err != nil {
writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL)
return

18
cmd/iam.go
View File

@ -1701,7 +1701,7 @@ func (sys *IAMSys) PolicyDBUpdateLDAP(ctx context.Context, isAttach bool,
var dn string
var isGroup bool
if r.User != "" {
dn, err = sys.LDAPConfig.DoesUsernameExist(r.User)
dn, err = sys.LDAPConfig.GetValidatedDNForUsername(r.User)
if err != nil {
logger.LogIf(ctx, err)
return
@ -1718,22 +1718,26 @@ func (sys *IAMSys) PolicyDBUpdateLDAP(ctx context.Context, isAttach bool,
isGroup = false
} else {
if isAttach {
var exists bool
if exists, err = sys.LDAPConfig.DoesGroupDNExist(r.Group); err != nil {
var foundGroupDN string
if foundGroupDN, err = sys.LDAPConfig.GetValidatedGroupDN(r.Group); err != nil {
logger.LogIf(ctx, err)
return
} else if !exists {
} else if foundGroupDN == "" {
err = errNoSuchGroup
return
}
// We use the group DN returned by the LDAP server (this may not
// equal the input group name, but we assume it is canonical).
dn = foundGroupDN
} else {
dn = r.Group
}
dn = r.Group
isGroup = true
}
userType := stsUser
updatedAt, addedOrRemoved, effectivePolicies, err = sys.store.PolicyDBUpdate(ctx, dn, isGroup,
userType, r.Policies, isAttach)
updatedAt, addedOrRemoved, effectivePolicies, err = sys.store.PolicyDBUpdate(
ctx, dn, isGroup, userType, r.Policies, isAttach)
if err != nil {
return
}

167
cmd/sts-handlers_test.go
View File

@ -29,6 +29,8 @@ import (
minio "github.com/minio/minio-go/v7"
cr "github.com/minio/minio-go/v7/pkg/credentials"
"github.com/minio/minio-go/v7/pkg/set"
ldap "github.com/minio/pkg/v2/ldap"
"golang.org/x/exp/slices"
)
func runAllIAMSTSTests(suite *TestSuiteIAM, c *check) {
@ -681,6 +683,7 @@ func TestIAMWithLDAPServerSuite(t *testing.T) {
suite.SetUpSuite(c)
suite.SetUpLDAP(c, ldapServer)
suite.TestLDAPSTS(c)
suite.TestLDAPUnicodeVariations(c)
suite.TestLDAPSTSServiceAccounts(c)
suite.TestLDAPSTSServiceAccountsWithUsername(c)
suite.TestLDAPSTSServiceAccountsWithGroups(c)
@ -823,6 +826,170 @@ func (s *TestSuiteIAM) TestLDAPSTS(c *check) {
c.Assert(err.Error(), "Access Denied.")
}
func (s *TestSuiteIAM) TestLDAPUnicodeVariations(c *check) {
ctx, cancel := context.WithTimeout(context.Background(), testDefaultTimeout)
defer cancel()
bucket := getRandomBucketName()
err := s.client.MakeBucket(ctx, bucket, minio.MakeBucketOptions{})
if err != nil {
c.Fatalf("bucket create error: %v", err)
}
// Create policy
policy := "mypolicy"
policyBytes := []byte(fmt.Sprintf(`{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": [
"s3:PutObject",
"s3:GetObject",
"s3:ListBucket"
],
"Resource": [
"arn:aws:s3:::%s/*"
]
}
]
}`, bucket))
err = s.adm.AddCannedPolicy(ctx, policy, policyBytes)
if err != nil {
c.Fatalf("policy add error: %v", err)
}
ldapID := cr.LDAPIdentity{
Client: s.TestSuiteCommon.client,
STSEndpoint: s.endPoint,
LDAPUsername: "svc.algorithm",
LDAPPassword: "example",
}
_, err = ldapID.Retrieve()
if err == nil {
c.Fatalf("Expected to fail to create STS cred with no associated policy!")
}
mustNormalizeDN := func(dn string) string {
normalizedDN, err := ldap.NormalizeDN(dn)
if err != nil {
c.Fatalf("normalize err: %v", err)
}
return normalizedDN
}
actualUserDN := mustNormalizeDN("uid=svc.algorithm,OU=swengg,DC=min,DC=io")
// \uFE52 is the unicode dot SMALL FULL STOP used below:
userDNWithUnicodeDot := "uid=svc﹒algorithm,OU=swengg,DC=min,DC=io"
_, err = s.adm.AttachPolicyLDAP(ctx, madmin.PolicyAssociationReq{
Policies: []string{policy},
User: userDNWithUnicodeDot,
})
if err != nil {
c.Fatalf("Unable to set policy: %v", err)
}
value, err := ldapID.Retrieve()
if err != nil {
c.Fatalf("Expected to generate STS creds, got err: %#v", err)
}
usersList, err := s.adm.ListUsers(ctx)
if err != nil {
c.Fatalf("list users should not fail: %v", err)
}
if len(usersList) != 1 {
c.Fatalf("expected user listing output: %#v", usersList)
}
uinfo := usersList[actualUserDN]
if uinfo.PolicyName != policy || uinfo.Status != madmin.AccountEnabled {
c.Fatalf("expected user listing content: %v", uinfo)
}
minioClient, err := minio.New(s.endpoint, &minio.Options{
Creds: cr.NewStaticV4(value.AccessKeyID, value.SecretAccessKey, value.SessionToken),
Secure: s.secure,
Transport: s.TestSuiteCommon.client.Transport,
})
if err != nil {
c.Fatalf("Error initializing client: %v", err)
}
// Validate that the client from sts creds can access the bucket.
c.mustListObjects(ctx, minioClient, bucket)
// Validate that the client cannot remove any objects
err = minioClient.RemoveObject(ctx, bucket, "someobject", minio.RemoveObjectOptions{})
if err.Error() != "Access Denied." {
c.Fatalf("unexpected non-access-denied err: %v", err)
}
// Remove the policy assignment on the user DN:
_, err = s.adm.DetachPolicyLDAP(ctx, madmin.PolicyAssociationReq{
Policies: []string{policy},
User: userDNWithUnicodeDot,
})
if err != nil {
c.Fatalf("Unable to remove policy setting: %v", err)
}
_, err = ldapID.Retrieve()
if err == nil {
c.Fatalf("Expected to fail to create a user with no associated policy!")
}
// Set policy via group and validate policy assignment.
actualGroupDN := mustNormalizeDN("cn=project.c,ou=groups,ou=swengg,dc=min,dc=io")
groupDNWithUnicodeDot := "cn=project﹒c,ou=groups,ou=swengg,dc=min,dc=io"
_, err = s.adm.AttachPolicyLDAP(ctx, madmin.PolicyAssociationReq{
Policies: []string{policy},
Group: groupDNWithUnicodeDot,
})
if err != nil {
c.Fatalf("Unable to attach group policy: %v", err)
}
value, err = ldapID.Retrieve()
if err != nil {
c.Fatalf("Expected to generate STS creds, got err: %#v", err)
}
policyResult, err := s.adm.GetLDAPPolicyEntities(ctx, madmin.PolicyEntitiesQuery{
Policy: []string{policy},
})
if err != nil {
c.Fatalf("GetLDAPPolicyEntities should not fail: %v", err)
}
{
// Check that the mapping we created exists.
idx := slices.IndexFunc(policyResult.PolicyMappings, func(e madmin.PolicyEntities) bool {
return e.Policy == policy && slices.Contains(e.Groups, actualGroupDN)
})
if !(idx >= 0) {
c.Fatalf("expected groupDN (%s) to be present in mapping list: %#v", actualGroupDN, policyResult)
}
}
minioClient, err = minio.New(s.endpoint, &minio.Options{
Creds: cr.NewStaticV4(value.AccessKeyID, value.SecretAccessKey, value.SessionToken),
Secure: s.secure,
Transport: s.TestSuiteCommon.client.Transport,
})
if err != nil {
c.Fatalf("Error initializing client: %v", err)
}
// Validate that the client from sts creds can access the bucket.
c.mustListObjects(ctx, minioClient, bucket)
// Validate that the client cannot remove any objects
err = minioClient.RemoveObject(ctx, bucket, "someobject", minio.RemoveObjectOptions{})
c.Assert(err.Error(), "Access Denied.")
}
func (s *TestSuiteIAM) TestLDAPSTSServiceAccounts(c *check) {
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()

4
go.mod
View File

@ -54,7 +54,7 @@ require (
github.com/minio/madmin-go/v3 v3.0.50
github.com/minio/minio-go/v7 v7.0.69
github.com/minio/mux v1.9.0
github.com/minio/pkg/v2 v2.0.11
github.com/minio/pkg/v2 v2.0.14
github.com/minio/selfupdate v0.6.0
github.com/minio/sha256-simd v1.0.1
github.com/minio/simdjson-go v0.4.5
@ -76,6 +76,7 @@ require (
github.com/prometheus/client_model v0.6.0
github.com/prometheus/common v0.50.0
github.com/prometheus/procfs v0.13.0
github.com/puzpuzpuz/xsync/v3 v3.1.0
github.com/rabbitmq/amqp091-go v1.9.0
github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475
github.com/rs/cors v1.10.1
@ -219,7 +220,6 @@ require (
github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 // indirect
github.com/pquerna/cachecontrol v0.2.0 // indirect
github.com/prometheus/prom2json v1.3.3 // indirect
github.com/puzpuzpuz/xsync/v3 v3.1.0 // indirect
github.com/rivo/tview v0.0.0-20240307173318-e804876934a1 // indirect
github.com/rivo/uniseg v0.4.7 // indirect
github.com/rjeczalik/notify v0.9.3 // indirect

4
go.sum
View File

@ -458,8 +458,8 @@ github.com/minio/minio-go/v7 v7.0.69 h1:l8AnsQFyY1xiwa/DaQskY4NXSLA2yrGsW5iD9nRP
github.com/minio/minio-go/v7 v7.0.69/go.mod h1:XAvOPJQ5Xlzk5o3o/ArO2NMbhSGkimC+bpW/ngRKDmQ=
github.com/minio/mux v1.9.0 h1:dWafQFyEfGhJvK6AwLOt83bIG5bxKxKJnKMCi0XAaoA=
github.com/minio/mux v1.9.0/go.mod h1:1pAare17ZRL5GpmNL+9YmqHoWnLmMZF9C/ioUCfy0BQ=
github.com/minio/pkg/v2 v2.0.11 h1:nlFoFrnwBaNNtVmJJBJ+t1Nzp4jbPzyqdVuc7t9clcQ=
github.com/minio/pkg/v2 v2.0.11/go.mod h1:zbVATXCinLCo+L/4vsPyqgiA4OYPXCJb+/E4KfE396A=
github.com/minio/pkg/v2 v2.0.14 h1:tPhDYxgvv3LNqmfCSe2zsSXrcaIj4ANyL1VRGdEkahI=
github.com/minio/pkg/v2 v2.0.14/go.mod h1:zbVATXCinLCo+L/4vsPyqgiA4OYPXCJb+/E4KfE396A=
github.com/minio/selfupdate v0.6.0 h1:i76PgT0K5xO9+hjzKcacQtO7+MjJ4JKA8Ak8XQ9DDwU=
github.com/minio/selfupdate v0.6.0/go.mod h1:bO02GTIPCMQFTEvE5h4DjYB58bCoZ35XLeBf0buTDdM=
github.com/minio/sha256-simd v0.1.1/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM=

53
internal/config/identity/ldap/ldap.go
View File

@ -27,6 +27,7 @@ import (
ldap "github.com/go-ldap/ldap/v3"
"github.com/minio/minio-go/v7/pkg/set"
"github.com/minio/minio/internal/auth"
xldap "github.com/minio/pkg/v2/ldap"
)
// LookupUserDN searches for the full DN and groups of a given username
@ -57,11 +58,16 @@ func (l *Config) LookupUserDN(username string) (string, []string, error) {
return bindDN, groups, nil
}
// DoesUsernameExist checks if the given username exists in the LDAP directory.
// GetValidatedDNForUsername checks if the given username exists in the LDAP directory.
// The given username could be just the short "login" username or the full DN.
// When the username is found, the full DN is returned, otherwise the returned
// string is empty. If the user is not found, err = nil, otherwise, err != nil.
func (l *Config) DoesUsernameExist(username string) (string, error) {
//
// When the username/DN is found, the full DN returned by the **server** is
// returned, otherwise the returned string is empty. The value returned here is
// the value sent by the LDAP server and is used in minio as the server performs
// LDAP specific normalization (including Unicode normalization).
//
// If the user is not found, err = nil, otherwise, err != nil.
func (l *Config) GetValidatedDNForUsername(username string) (string, error) {
conn, err := l.LDAP.Connect()
if err != nil {
return "", err
@ -107,7 +113,11 @@ func (l *Config) DoesUsernameExist(username string) (string, error) {
return "", err
}
for _, entry := range searchResult.Entries {
foundDistName = append(foundDistName, entry.DN)
normDN, err := xldap.NormalizeDN(entry.DN)
if err != nil {
return "", err
}
foundDistName = append(foundDistName, normDN)
}
}
}
@ -123,26 +133,33 @@ func (l *Config) DoesUsernameExist(username string) (string, error) {
return "", nil
}
// DoesGroupDNExist checks if the given group DN exists in the LDAP directory.
func (l *Config) DoesGroupDNExist(groupDN string) (bool, error) {
// GetValidatedGroupDN checks if the given group DN exists in the LDAP directory
// and returns the group DN sent by the LDAP server. The value returned by the
// server may not be equal to the input group DN, as LDAP equality is not a
// simple Golang string equality. However, we assume the value returned by the
// LDAP server is canonical.
//
// If the group is not found in the LDAP directory, the returned string is empty
// and err = nil.
func (l *Config) GetValidatedGroupDN(groupDN string) (string, error) {
if len(l.LDAP.GroupSearchBaseDistNames) == 0 {
return false, errors.New("no group search Base DNs given")
return "", errors.New("no group search Base DNs given")
}
gdn, err := ldap.ParseDN(groupDN)
if err != nil {
return false, fmt.Errorf("Given group DN could not be parsed: %s", err)
return "", fmt.Errorf("Given group DN could not be parsed: %s", err)
}
conn, err := l.LDAP.Connect()
if err != nil {
return false, err
return "", err
}
defer conn.Close()
// Bind to the lookup user account
if err = l.LDAP.LookupBind(conn); err != nil {
return false, err
return "", err
}
var foundDistName []string
@ -158,22 +175,26 @@ func (l *Config) DoesGroupDNExist(groupDN string) (bool, error) {
if ldap.IsErrorWithCode(err, 32) {
continue
}
return false, err
return "", err
}
for _, entry := range searchResult.Entries {
foundDistName = append(foundDistName, entry.DN)
normDN, err := xldap.NormalizeDN(entry.DN)
if err != nil {
return "", err
}
foundDistName = append(foundDistName, normDN)
}
}
}
if len(foundDistName) == 1 {
return true, nil
return foundDistName[0], nil
} else if len(foundDistName) > 1 {
// FIXME: This error would happen if the multiple base DNs are given and
// some base DNs are subtrees of other base DNs - we should validate
// and error out in such cases.
return false, fmt.Errorf("found multiple DNs for the given group DN")
return "", fmt.Errorf("found multiple DNs for the given group DN")
}
return false, nil
return "", nil
}
// Bind - binds to ldap, searches LDAP and returns the distinguished name of the