fix: authenticate LDAP via actual DN instead of normalized DN (#19805)

fix: authenticate LDAP via actual DN instead of normalized DN

Normalized DN is only for internal representation, not for
external communication, any communication to LDAP must be
based on actual user DN. LDAP servers do not understand
normalized DN.

fixes #19757
This commit is contained in:
Harshavardhana 2024-05-25 06:43:06 -07:00 committed by GitHub
parent 7d75b1e758
commit 597a785253
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 118 additions and 20 deletions

View File

@ -282,6 +282,7 @@ func (a adminAPIHandlers) AddServiceAccountLDAP(w http.ResponseWriter, r *http.R
}
targetUser = lookupResult.NormDN
opts.claims[ldapUser] = targetUser // DN
opts.claims[ldapActualUser] = lookupResult.ActualDN
// Add LDAP attributes that were looked up into the claims.
for attribKey, attribValue := range lookupResult.Attributes {

View File

@ -709,6 +709,7 @@ func (a adminAPIHandlers) AddServiceAccount(w http.ResponseWriter, r *http.Reque
}
targetUser = lookupResult.NormDN
opts.claims[ldapUser] = targetUser // username DN
opts.claims[ldapActualUser] = lookupResult.ActualDN
// Add LDAP attributes that were looked up into the claims.
for attribKey, attribValue := range lookupResult.Attributes {

View File

@ -306,6 +306,7 @@ func (driver *ftpDriver) getMinIOClient(ctx *ftp.Context) (*minio.Client, error)
claims[expClaim] = UTCNow().Add(expiryDur).Unix()
claims[ldapUser] = lookupResult.NormDN
claims[ldapActualUser] = lookupResult.ActualDN
claims[ldapUserN] = ctx.Sess.LoginUser()
// Add LDAP attributes that were looked up into the claims.

View File

@ -1951,6 +1951,12 @@ func (store *IAMStoreSys) GetAllParentUsers() map[string]ParentUserInfo {
subClaimValue = subFromToken
}
}
if v, ok := claims[ldapActualUser]; ok {
subFromToken, ok := v.(string)
if ok {
subClaimValue = subFromToken
}
}
roleArn := openid.DummyRoleARN.String()
s, ok := claims[roleArnClaim]

View File

@ -1351,12 +1351,17 @@ func (sys *IAMSys) purgeExpiredCredentialsForExternalSSO(ctx context.Context) {
func (sys *IAMSys) purgeExpiredCredentialsForLDAP(ctx context.Context) {
parentUsers := sys.store.GetAllParentUsers()
var allDistNames []string
for parentUser := range parentUsers {
for parentUser, info := range parentUsers {
if !sys.LDAPConfig.IsLDAPUserDN(parentUser) {
continue
}
allDistNames = append(allDistNames, parentUser)
if info.subClaimValue != "" {
// we need to ask LDAP about the actual user DN not normalized DN.
allDistNames = append(allDistNames, info.subClaimValue)
} else {
allDistNames = append(allDistNames, parentUser)
}
}
expiredUsers, err := sys.LDAPConfig.GetNonEligibleUserDistNames(allDistNames)

View File

@ -248,8 +248,9 @@ func startSFTPServer(args []string) {
return nil, errAuthentication
}
criticalOptions := map[string]string{
ldapUser: targetUser,
ldapUserN: c.User(),
ldapUser: targetUser,
ldapActualUser: lookupResult.ActualDN,
ldapUserN: c.User(),
}
for attribKey, attribValue := range lookupResult.Attributes {
// we skip multi-value attributes here, as they cannot

View File

@ -74,8 +74,9 @@ const (
parentClaim = "parent"
// LDAP claim keys
ldapUser = "ldapUser" // this is a key name for a DN value
ldapUserN = "ldapUsername" // this is a key name for the short/login username
ldapUser = "ldapUser" // this is a key name for a normalized DN value
ldapActualUser = "ldapActualUser" // this is a key name for the actual DN value
ldapUserN = "ldapUsername" // this is a key name for the short/login username
// Claim key-prefix for LDAP attributes
ldapAttribPrefix = "ldapAttrib_"
@ -677,6 +678,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r *
return
}
ldapUserDN := lookupResult.NormDN
ldapActualUserDN := lookupResult.ActualDN
// Check if this user or their groups have a policy applied.
ldapPolicies, err := globalIAMSys.PolicyDBGet(ldapUserDN, groupDistNames...)
@ -687,7 +689,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r *
if len(ldapPolicies) == 0 && newGlobalAuthZPluginFn() == nil {
writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue,
fmt.Errorf("expecting a policy to be set for user `%s` or one of their groups: `%s` - rejecting this request",
ldapUserDN, strings.Join(groupDistNames, "`,`")))
ldapActualUserDN, strings.Join(groupDistNames, "`,`")))
return
}
@ -699,6 +701,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r *
claims[expClaim] = UTCNow().Add(expiryDur).Unix()
claims[ldapUser] = ldapUserDN
claims[ldapActualUser] = ldapActualUserDN
claims[ldapUserN] = ldapUsername
// Add lookup up LDAP attributes as claims.
for attrib, value := range lookupResult.Attributes {

View File

@ -723,6 +723,7 @@ func TestIAMWithLDAPServerSuite(t *testing.T) {
suite.TestLDAPSTSServiceAccountsWithUsername(c)
suite.TestLDAPSTSServiceAccountsWithGroups(c)
suite.TestLDAPAttributesLookup(c)
suite.TestLDAPCyrillicUser(c)
suite.TearDownSuite(c)
},
)
@ -1872,6 +1873,75 @@ func (s *TestSuiteIAM) TestLDAPSTSServiceAccountsWithGroups(c *check) {
c.mustNotCreateSvcAccount(ctx, globalActiveCred.AccessKey, userAdmClient)
}
func (s *TestSuiteIAM) TestLDAPCyrillicUser(c *check) {
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
_, err := s.adm.AttachPolicyLDAP(ctx, madmin.PolicyAssociationReq{
Policies: []string{"readwrite"},
User: "uid=Пользователь,ou=people,ou=swengg,dc=min,dc=io",
})
if err != nil {
c.Fatalf("Unable to set policy: %v", err)
}
cases := []struct {
username string
dn string
}{
{
username: "Пользователь",
dn: "uid=Пользователь,ou=people,ou=swengg,dc=min,dc=io",
},
}
conn, err := globalIAMSys.LDAPConfig.LDAP.Connect()
if err != nil {
c.Fatalf("LDAP connect failed: %v", err)
}
defer conn.Close()
for i, testCase := range cases {
ldapID := cr.LDAPIdentity{
Client: s.TestSuiteCommon.client,
STSEndpoint: s.endPoint,
LDAPUsername: testCase.username,
LDAPPassword: "example",
}
value, err := ldapID.Retrieve()
if err != nil {
c.Fatalf("Expected to generate STS creds, got err: %#v", err)
}
// Retrieve the STS account's credential object.
u, ok := globalIAMSys.GetUser(ctx, value.AccessKeyID)
if !ok {
c.Fatalf("Expected to find user %s", value.AccessKeyID)
}
if u.Credentials.AccessKey != value.AccessKeyID {
c.Fatalf("Expected access key %s, got %s", value.AccessKeyID, u.Credentials.AccessKey)
}
// Retrieve the credential's claims.
secret, err := getTokenSigningKey()
if err != nil {
c.Fatalf("Error getting token signing key: %v", err)
}
claims, err := getClaimsFromTokenWithSecret(value.SessionToken, secret)
if err != nil {
c.Fatalf("Error getting claims from token: %v", err)
}
// Validate claims.
dnClaim := claims[ldapActualUser].(string)
if dnClaim != testCase.dn {
c.Fatalf("Test %d: unexpected dn claim: %s", i+1, dnClaim)
}
}
}
func (s *TestSuiteIAM) TestLDAPAttributesLookup(c *check) {
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
@ -1942,7 +2012,7 @@ func (s *TestSuiteIAM) TestLDAPAttributesLookup(c *check) {
}
// Validate claims. Check if the sshPublicKey claim is present.
dnClaim := claims[ldapUser].(string)
dnClaim := claims[ldapActualUser].(string)
if dnClaim != testCase.dn {
c.Fatalf("Test %d: unexpected dn claim: %s", i+1, dnClaim)
}

4
go.mod
View File

@ -2,6 +2,8 @@ module github.com/minio/minio
go 1.21
replace github.com/minio/console => github.com/donatello/console v0.12.6-0.20240522161239-e2303ef9d681
require (
cloud.google.com/go/storage v1.40.0
github.com/Azure/azure-storage-blob-go v0.15.0
@ -55,7 +57,7 @@ require (
github.com/minio/madmin-go/v3 v3.0.52
github.com/minio/minio-go/v7 v7.0.70
github.com/minio/mux v1.9.0
github.com/minio/pkg/v3 v3.0.0
github.com/minio/pkg/v3 v3.0.1
github.com/minio/selfupdate v0.6.0
github.com/minio/simdjson-go v0.4.5
github.com/minio/sio v0.3.1

8
go.sum
View File

@ -128,6 +128,8 @@ github.com/decred/dcrd/dcrec/secp256k1/v4 v4.3.0 h1:rpfIENRNNilwHwZeG5+P150SMrnN
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.3.0/go.mod h1:v57UDF4pDQJcEfFUCRop3lJL149eHGSe9Jvczhzjo/0=
github.com/docker/go-units v0.5.0 h1:69rxXcBk27SvSaaxTtLh/8llcHD8vYHT7WSdRZ/jvr4=
github.com/docker/go-units v0.5.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk=
github.com/donatello/console v0.12.6-0.20240522161239-e2303ef9d681 h1:Cik6pFMXH35U5hje8pXhgtNVkcNwxQ1f1AzIXx1M878=
github.com/donatello/console v0.12.6-0.20240522161239-e2303ef9d681/go.mod h1:JyqeznIlKwgSx2Usz4CNq0i9WlDMJF75m8lbPV38p4I=
github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY=
github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto=
@ -425,8 +427,6 @@ github.com/minio/cli v1.24.2 h1:J+fCUh9mhPLjN3Lj/YhklXvxj8mnyE/D6FpFduXJ2jg=
github.com/minio/cli v1.24.2/go.mod h1:bYxnK0uS629N3Bq+AOZZ+6lwF77Sodk4+UL9vNuXhOY=
github.com/minio/colorjson v1.0.7 h1:n69M42mIuQHdzbsxlmwji1zxDypaw4o39rHjAmX4Dh4=
github.com/minio/colorjson v1.0.7/go.mod h1:9LGM5yybI+GuhSbuzAerbSgvFb4j8ux9NzyONR+NrAY=
github.com/minio/console v1.4.1 h1:P7hgyQi+36aYH90WPME3d/eLJ+a1jxnfhwxLjUOe9kY=
github.com/minio/console v1.4.1/go.mod h1:JyqeznIlKwgSx2Usz4CNq0i9WlDMJF75m8lbPV38p4I=
github.com/minio/csvparser v1.0.0 h1:xJEHcYK8ZAjeW4hNV9Zu30u+/2o4UyPnYgyjWp8b7ZU=
github.com/minio/csvparser v1.0.0/go.mod h1:lKXskSLzPgC5WQyzP7maKH7Sl1cqvANXo9YCto8zbtM=
github.com/minio/dnscache v0.1.1 h1:AMYLqomzskpORiUA1ciN9k7bZT1oB3YZN4cEIi88W5o=
@ -454,8 +454,8 @@ 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.17 h1:ndmGlitUj/eCVRPmfsAw3KlbtVNxqk0lQIvDXlcTHiQ=
github.com/minio/pkg/v2 v2.0.17/go.mod h1:V+OP/fKRD/qhJMQpdXXrCXcLYjGMpHKEE26zslthm5k=
github.com/minio/pkg/v3 v3.0.0 h1:0vOKHgwpya//mb7RH0i1lyPMH2IBBF5hJMNY5Bk2WlY=
github.com/minio/pkg/v3 v3.0.0/go.mod h1:53gkSUVHcfYoskOs5YAJ3D99nsd2SKru90rdE9whlXU=
github.com/minio/pkg/v3 v3.0.1 h1:qts6g9rYjAdeomRdwjnMc1IaQ6KbaJs3dwqBntXziaw=
github.com/minio/pkg/v3 v3.0.1/go.mod h1:53gkSUVHcfYoskOs5YAJ3D99nsd2SKru90rdE9whlXU=
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=

View File

@ -51,7 +51,7 @@ func (l *Config) LookupUserDN(username string) (*xldap.DNSearchResult, []string,
return nil, nil, errRet
}
groups, err := l.LDAP.SearchForUserGroups(conn, username, lookupRes.NormDN)
groups, err := l.LDAP.SearchForUserGroups(conn, username, lookupRes.ActualDN)
if err != nil {
return nil, nil, err
}
@ -200,9 +200,9 @@ func (l *Config) Bind(username, password string) (*xldap.DNSearchResult, []strin
}
// Authenticate the user credentials.
err = conn.Bind(lookupResult.NormDN, password)
err = conn.Bind(lookupResult.ActualDN, password)
if err != nil {
errRet := fmt.Errorf("LDAP auth failed for DN %s: %w", lookupResult.NormDN, err)
errRet := fmt.Errorf("LDAP auth failed for DN %s: %w", lookupResult.ActualDN, err)
return nil, nil, errRet
}
@ -212,7 +212,7 @@ func (l *Config) Bind(username, password string) (*xldap.DNSearchResult, []strin
}
// User groups lookup.
groups, err := l.LDAP.SearchForUserGroups(conn, username, lookupResult.NormDN)
groups, err := l.LDAP.SearchForUserGroups(conn, username, lookupResult.ActualDN)
if err != nil {
return nil, nil, err
}
@ -288,7 +288,7 @@ func (l *Config) GetNonEligibleUserDistNames(userDistNames []string) ([]string,
return nil, err
}
// Evaluate the filter again with generic wildcard instead of specific values
// Evaluate the filter again with generic wildcard instead of specific values
filter := strings.ReplaceAll(l.LDAP.UserDNSearchFilter, "%s", "*")
nonExistentUsers := []string{}
@ -305,7 +305,11 @@ func (l *Config) GetNonEligibleUserDistNames(userDistNames []string) ([]string,
if err != nil {
// Object does not exist error?
if ldap.IsErrorWithCode(err, 32) {
nonExistentUsers = append(nonExistentUsers, dn)
ndn, err := ldap.ParseDN(dn)
if err != nil {
return nil, err
}
nonExistentUsers = append(nonExistentUsers, ndn.String())
continue
}
return nil, err
@ -313,7 +317,11 @@ func (l *Config) GetNonEligibleUserDistNames(userDistNames []string) ([]string,
if len(searchResult.Entries) == 0 {
// DN was not found - this means this user account is
// expired.
nonExistentUsers = append(nonExistentUsers, dn)
ndn, err := ldap.ParseDN(dn)
if err != nil {
return nil, err
}
nonExistentUsers = append(nonExistentUsers, ndn.String())
}
}
return nonExistentUsers, nil