sevenhill/QSfera
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Use LDAP Modify Password ExtOp for updating passwords

Browse Source 
By default the graph API will now use the LDAP Password Modify Extended
Operation for setting user passwords. By this we make sure that the
LDAP server can e.g. properly hash the password with and algorithm that
it supports.

This can be reverted to the old behaviour (using "normal" LDAP modify
requests) by setting GRAPH_LDAP_SERVER_USE_PASSWORD_MODIFY_EXOP=false

Fixes: #3778
This commit is contained in:
Ralf Haferkamp
2022-06-30 14:17:24 +02:00
committed by Ralf Haferkamp
parent 4e07daef53
commit 2cabf5bf5d
7 changed files with 103 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
services/graph/pkg/config/config.go
+8 -7
View File
@@ -37,13 +37,14 @@ type Spaces struct {
}
type LDAP struct {
URI string `yaml:"uri" env:"LDAP_URI;GRAPH_LDAP_URI" desc:"URI of the LDAP Server to connect to. Supported URI schemes are 'ldaps://' and 'ldap://'"`
CACert string `yaml:"cacert" env:"LDAP_CACERT;GRAPH_LDAP_CACERT" desc:"The certificate to verify TLS connections"`
Insecure bool `yaml:"insecure" env:"LDAP_INSECURE;GRAPH_LDAP_INSECURE"`
BindDN string `yaml:"bind_dn" env:"LDAP_BIND_DN;GRAPH_LDAP_BIND_DN"`
BindPassword string `yaml:"bind_password" env:"LDAP_BIND_PASSWORD;GRAPH_LDAP_BIND_PASSWORD"`
UseServerUUID bool `yaml:"use_server_uuid" env:"GRAPH_LDAP_SERVER_UUID"`
WriteEnabled bool `yaml:"write_enabled" env:"GRAPH_LDAP_SERVER_WRITE_ENABLED"`
URI string `yaml:"uri" env:"LDAP_URI;GRAPH_LDAP_URI" desc:"URI of the LDAP Server to connect to. Supported URI schemes are 'ldaps://' and 'ldap://'"`
CACert string `yaml:"cacert" env:"LDAP_CACERT;GRAPH_LDAP_CACERT" desc:"The certificate to verify TLS connections"`
Insecure bool `yaml:"insecure" env:"LDAP_INSECURE;GRAPH_LDAP_INSECURE"`
BindDN string `yaml:"bind_dn" env:"LDAP_BIND_DN;GRAPH_LDAP_BIND_DN"`
BindPassword string `yaml:"bind_password" env:"LDAP_BIND_PASSWORD;GRAPH_LDAP_BIND_PASSWORD"`
UseServerUUID bool `yaml:"use_server_uuid" env:"GRAPH_LDAP_SERVER_UUID"`
UsePasswordModExOp bool `yaml:"use_password_modify_exop" env:"GRAPH_LDAP_SERVER_USE_PASSWORD_MODIFY_EXOP" desc:"User the Password Modify Extended Operation for updating user passwords"`
WriteEnabled bool `yaml:"write_enabled" env:"GRAPH_LDAP_SERVER_WRITE_ENABLED"`
UserBaseDN string `yaml:"user_base_dn" env:"LDAP_USER_BASE_DN;GRAPH_LDAP_USER_BASE_DN"`
UserSearchScope string `yaml:"user_search_scope" env:"LDAP_USER_SCOPE;GRAPH_LDAP_USER_SCOPE"`
services/graph/pkg/config/defaults/defaultconfig.go
+1
View File
@@ -46,6 +46,7 @@ func DefaultConfig() *config.Config {
CACert: path.Join(defaults.BaseDataPath(), "idm", "ldap.crt"),
BindDN: "uid=libregraph,ou=sysusers,o=libregraph-idm",
UseServerUUID: false,
UsePasswordModExOp: true,
WriteEnabled: true,
UserBaseDN: "ou=users,o=libregraph-idm",
UserSearchScope: "sub",
services/graph/pkg/identity/ldap.go
+52 -12
View File
@@ -23,8 +23,9 @@ var (
)
type LDAP struct {
useServerUUID bool
writeEnabled bool
useServerUUID bool
writeEnabled bool
usePwModifyExOp bool
userBaseDN string
userFilter string
@@ -90,6 +91,7 @@ func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LD
return &LDAP{
useServerUUID: config.UseServerUUID,
usePwModifyExOp: config.UsePasswordModExOp,
userBaseDN: config.UserBaseDN,
userFilter: config.UserFilter,
userObjectClass: config.UserObjectClass,
@@ -138,11 +140,10 @@ func (i *LDAP) CreateUser(ctx context.Context, user libregraph.User) (*libregrap
objectClasses := []string{"inetOrgPerson", "organizationalPerson", "person", "top"}
if user.PasswordProfile != nil && user.PasswordProfile.Password != nil {
// TODO? This relies to the LDAP server to properly hash the password.
// We might want to add support for the Password Modify LDAP Extended
// Operation for servers that implement it. (Or implement client-side
// hashing here.
if !i.usePwModifyExOp && user.PasswordProfile != nil && user.PasswordProfile.Password != nil {
// Depending on the LDAP server implementation this might cause the
// password to be stored in cleartext in the LDAP database. Using the
// "Password Modify LDAP Extended Operation" is recommended.
ar.Attribute("userPassword", []string{*user.PasswordProfile.Password})
}
if !i.useServerUUID {
@@ -172,6 +173,12 @@ func (i *LDAP) CreateUser(ctx context.Context, user libregraph.User) (*libregrap
return nil, err
}
if i.usePwModifyExOp && user.PasswordProfile != nil && user.PasswordProfile.Password != nil {
if err := i.updateUserPassowrd(ar.DN, user.PasswordProfile.GetPassword()); err != nil {
return nil, err
}
}
// Read back user from LDAP to get the generated UUID
e, err := i.getUserByDN(ar.DN)
if err != nil {
@@ -224,6 +231,8 @@ func (i *LDAP) UpdateUser(ctx context.Context, nameOrID string, user libregraph.
return nil, err
}
var updateNeeded bool
// Don't allow updates of the ID
if user.Id != nil && *user.Id != "" {
if e.GetEqualFoldAttributeValue(i.userAttributeMap.id) != *user.Id {
@@ -243,21 +252,32 @@ func (i *LDAP) UpdateUser(ctx context.Context, nameOrID string, user libregraph.
if user.DisplayName != nil && *user.DisplayName != "" {
if e.GetEqualFoldAttributeValue(i.userAttributeMap.displayName) != *user.DisplayName {
mr.Replace(i.userAttributeMap.displayName, []string{*user.DisplayName})
updateNeeded = true
}
}
if user.Mail != nil && *user.Mail != "" {
if e.GetEqualFoldAttributeValue(i.userAttributeMap.mail) != *user.Mail {
mr.Replace(i.userAttributeMap.mail, []string{*user.Mail})
updateNeeded = true
}
}
if user.PasswordProfile != nil && user.PasswordProfile.Password != nil && *user.PasswordProfile.Password != "" {
// password are hashed server side there is no need to check if the new password
// is actually different from the old one.
mr.Replace("userPassword", []string{*user.PasswordProfile.Password})
if i.usePwModifyExOp {
if err := i.updateUserPassowrd(e.DN, user.PasswordProfile.GetPassword()); err != nil {
return nil, err
}
} else {
// password are hashed server side there is no need to check if the new password
// is actually different from the old one.
mr.Replace("userPassword", []string{*user.PasswordProfile.Password})
updateNeeded = true
}
}
if err := i.conn.Modify(&mr); err != nil {
return nil, err
if updateNeeded {
if err := i.conn.Modify(&mr); err != nil {
return nil, err
}
}
// Read back user from LDAP to get the generated UUID
@@ -808,6 +828,26 @@ func (i *LDAP) RemoveMemberFromGroup(ctx context.Context, groupID string, member
return nil
}
func (i *LDAP) updateUserPassowrd(dn, password string) error {
pwMod := ldap.PasswordModifyRequest{
UserIdentity: dn,
NewPassword: password,
}
// Note: We can ignore the result message here, as it were only relevant if we requested
// the server to generate a new Password
_, err := i.conn.PasswordModify(&pwMod)
if err != nil {
var lerr *ldap.Error
i.logger.Debug().Err(err).Msg("error setting password for user")
if errors.As(err, &lerr) {
if lerr.ResultCode == ldap.LDAPResultEntryAlreadyExists {
err = errorcode.New(errorcode.NameAlreadyExists, lerr.Error())
}
}
}
return err
}
func (i *LDAP) createUserModelFromLDAP(e *ldap.Entry) *libregraph.User {
if e == nil {
return nil
services/graph/pkg/identity/ldap/reconnect.go
+25 -4
View File
@@ -146,6 +146,31 @@ func (c ConnWithReconnect) Modify(m *ldap.ModifyRequest) error {
return ldap.NewError(ldap.ErrorNetwork, errMaxRetries)
}
func (c ConnWithReconnect) PasswordModify(m *ldap.PasswordModifyRequest) (*ldap.PasswordModifyResult, error) {
conn, err := c.GetConnection()
if err != nil {
return nil, err
}
var res *ldap.PasswordModifyResult
for try := 0; try <= c.retries; try++ {
res, err = conn.PasswordModify(m)
if !ldap.IsErrorWithCode(err, ldap.ErrorNetwork) {
// non network error, return it to the client
return res, err
}
c.logger.Debug().Msgf("Network Error. attempt %d", try)
conn, err = c.reconnect(conn)
if err != nil {
return nil, err
}
c.logger.Debug().Msg("retrying LDAP Password Modify")
}
// if we get here we reached the maximum retries. So return an error
return nil, ldap.NewError(ldap.ErrorNetwork, errMaxRetries)
}
func (c ConnWithReconnect) ModifyDN(m *ldap.ModifyDNRequest) error {
conn, err := c.GetConnection()
if err != nil {
@@ -289,10 +314,6 @@ func (c ConnWithReconnect) Compare(dn, attribute, value string) (bool, error) {
return false, ldap.NewError(ldap.LDAPResultNotSupported, fmt.Errorf("not implemented"))
}
func (c ConnWithReconnect) PasswordModify(*ldap.PasswordModifyRequest) (*ldap.PasswordModifyResult, error) {
return nil, ldap.NewError(ldap.LDAPResultNotSupported, fmt.Errorf("not implemented"))
}
func (c ConnWithReconnect) SearchWithPaging(searchRequest *ldap.SearchRequest, pagingSize uint32) (*ldap.SearchResult, error) {
return nil, ldap.NewError(ldap.LDAPResultNotSupported, fmt.Errorf("not implemented"))
}