diff --git a/changelog/unreleased/use-ldappassword-exop.md b/changelog/unreleased/use-ldappassword-exop.md new file mode 100644 index 00000000000..5cacb713109 --- /dev/null +++ b/changelog/unreleased/use-ldappassword-exop.md @@ -0,0 +1,8 @@ +Bugfix: Store user passwords hashed in idm + +Support for hashing user passwords was added to libregraph/idm. The graph API will +now set userpasswords using the LDAP Modify Extended Operation (RFC3062). In the default +configuration passwords will be hashed using the argon2id algorithm. + +https://github.com/owncloud/ocis/issues/3778 +https://github.com/owncloud/ocis/pull/4053 diff --git a/go.mod b/go.mod index fd972634a6e..001e985f38c 100644 --- a/go.mod +++ b/go.mod @@ -38,7 +38,7 @@ require ( github.com/gorilla/mux v1.8.0 github.com/grpc-ecosystem/grpc-gateway/v2 v2.10.3 github.com/justinas/alice v1.2.0 - github.com/libregraph/idm v0.3.1-0.20220315094434-e9a5cff3dd05 + github.com/libregraph/idm v0.3.1-0.20220627110322-41a5a73f50b0 github.com/libregraph/lico v0.54.1-0.20220325072321-31efc3995d63 github.com/mitchellh/mapstructure v1.5.0 github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 @@ -250,7 +250,7 @@ require ( go.uber.org/zap v1.19.1 // indirect golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 // indirect golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect - golang.org/x/sys v0.0.0-20220412211240-33da011f77ad // indirect + golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a // indirect golang.org/x/text v0.3.7 // indirect golang.org/x/time v0.0.0-20220224211638-0e9765cccd65 // indirect golang.org/x/tools v0.1.10 // indirect diff --git a/go.sum b/go.sum index 8bfddbca426..088dd4bcf41 100644 --- a/go.sum +++ b/go.sum @@ -114,7 +114,7 @@ github.com/RoaringBitmap/roaring v0.9.4 h1:ckvZSX5gwCRaJYBNe7syNawCU5oruY9gQmjXl github.com/RoaringBitmap/roaring v0.9.4/go.mod h1:icnadbWcNyfEHlYdr+tDlOTih1Bf/h+rzPpv4sbomAA= github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWXgklEdEo= github.com/Shopify/toxiproxy v2.1.4+incompatible/go.mod h1:OXgGpZ6Cli1/URJOF1DMxUHB2q5Ap20/P/eIdh4G0pI= -github.com/Songmu/prompter v0.5.0/go.mod h1:S4Eg25l60kPlnfB2ttFVpvBKYw7RKJexzB3gzpAansY= +github.com/Songmu/prompter v0.5.1/go.mod h1:CS3jEPD6h9IaLaG6afrl1orTgII9+uDWuw95dr6xHSw= github.com/acomagu/bufpipe v1.0.3 h1:fxAGrHZTgQ9w5QqVItgzwj235/uYZYgbXitB+dLupOk= github.com/acomagu/bufpipe v1.0.3/go.mod h1:mxdxdup/WdsKVreO5GpW4+M/1CE2sMG4jeGJ2sYmHc4= github.com/agext/levenshtein v1.2.1/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558= @@ -295,8 +295,6 @@ github.com/crewjam/httperr v0.2.0/go.mod h1:Jlz+Sg/XqBQhyMjdDiC+GNNRzZTD7x39Gu3p github.com/crewjam/saml v0.4.6 h1:XCUFPkQSJLvzyl4cW9OvpWUbRf0gE7VUpU8ZnilbeM4= github.com/crewjam/saml v0.4.6/go.mod h1:ZBOXnNPFzB3CgOkRm7Nd6IVdkG+l/wF+0ZXLqD96t1A= github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e/go.mod h1:XJEZ3/EQuI3BXTp/6DUzFr850vlxq11I6satRtz0YQ4= -github.com/cs3org/reva/v2 v2.6.1-0.20220625133157-47ade515fb1e h1:/XaypNR4cVLC6jD2KQ6Z7D7Euyzj4hPHrLlQQad0bmo= -github.com/cs3org/reva/v2 v2.6.1-0.20220625133157-47ade515fb1e/go.mod h1:zAHqzr36X4lIalonDQeNbwrIXjn66C38lp5A+MTRS1c= github.com/cs3org/reva/v2 v2.6.1 h1:iVJlbKwUbjeTq17zZ8XGtD86yM+L64oHQ24r1HjzsQM= github.com/cs3org/reva/v2 v2.6.1/go.mod h1:zAHqzr36X4lIalonDQeNbwrIXjn66C38lp5A+MTRS1c= github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8 h1:Z9lwXumT5ACSmJ7WGnFl+OMLLjpz5uR2fyz7dC255FI= @@ -822,8 +820,8 @@ github.com/labbsr0x/goh v1.0.1/go.mod h1:8K2UhVoaWXcCU7Lxoa2omWnC8gyW8px7/lmO61c github.com/labstack/echo/v4 v4.1.11/go.mod h1:i541M3Fj6f76NZtHSj7TXnyM8n2gaodfvfxNnFqi74g= github.com/labstack/gommon v0.3.0/go.mod h1:MULnywXg0yavhxWKc+lOruYdAhDwPK9wf0OL7NoOu+k= github.com/leodido/go-urn v1.2.0/go.mod h1:+8+nEpDfqqsY+g338gtMEUOtuK+4dEMhiQEgxpxOKII= -github.com/libregraph/idm v0.3.1-0.20220315094434-e9a5cff3dd05 h1:/I4f6c7ZGw16oTBAyhCD9Tf+arBHGvmxL9Drs/KRkRc= -github.com/libregraph/idm v0.3.1-0.20220315094434-e9a5cff3dd05/go.mod h1:YQ21AOfZPcCZWX1uJYULZ8hNdrmxStg6egvXaS+ZvOM= +github.com/libregraph/idm v0.3.1-0.20220627110322-41a5a73f50b0 h1:YHYm9isSjHC50OyjrRPLCVN+6mWJTZwzjHX7WYlnDTE= +github.com/libregraph/idm v0.3.1-0.20220627110322-41a5a73f50b0/go.mod h1:ggVmYkaK5fu680QOnxkuyCRW5Wl5qzaYXgIiieNBOJE= github.com/libregraph/lico v0.54.1-0.20220325072321-31efc3995d63 h1:oPqyRePmq+59YF1tAur7WXuM/z/epRd+HGGyPPx2Vv8= github.com/libregraph/lico v0.54.1-0.20220325072321-31efc3995d63/go.mod h1:KZ4X+bEbOQMSV6iPysZEqVO/Pa5Mvo7xhhcLwUNPjmw= github.com/linode/linodego v0.25.3/go.mod h1:GSBKPpjoQfxEfryoCRcgkuUOCuVtGHWhzI8OMdycNTE= @@ -1364,7 +1362,6 @@ golang.org/x/crypto v0.0.0-20210513164829-c07d793c2f9a/go.mod h1:P+XmwS30IXTQdn5 golang.org/x/crypto v0.0.0-20210616213533-5ff15b29337e/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.0.0-20211117183948-ae814b36b871/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= -golang.org/x/crypto v0.0.0-20211215153901-e495a2d5b3d3/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.0.0-20211215165025-cf75a172585e/go.mod h1:P+XmwS30IXTQdn5tA2iutPOUgjI07+tq3H3K9MVA1s8= golang.org/x/crypto v0.0.0-20220112180741-5e0467b6c7ce/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.0.0-20220214200702-86341886e292/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= @@ -1600,7 +1597,6 @@ golang.org/x/sys v0.0.0-20210220050731-9a76102bfb43/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210303074136-134d130e1a04/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210305230114-8fe3ee5dd75b/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210315160823-c6e025ad8005/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210319071255-635bc2c9138d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210320140829-1e4c9ba3b0c4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210324051608-47abb6519492/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -1630,14 +1626,15 @@ golang.org/x/sys v0.0.0-20220114195835-da31bd327af9/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220209214540-3681064d5158/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220307203707-22a9840ba4d7/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220408201424-a24fb2fb8a0f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220412211240-33da011f77ad h1:ntjMns5wyP/fN65tdBD4g8J5w8n015+iIIs9rtjXkY0= golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a h1:dGzPydgVsqGcTRVwiLJ1jVbufYwmzD3LfVPLKsKg+0k= +golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201113234701-d7a72108b828/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= -golang.org/x/term v0.0.0-20210317153231-de623e64d2a6/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= -golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 h1:JGgROgKl9N8DuW20oFS5gxc+lE67/N3FcwmBPMe7ArY= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= +golang.org/x/term v0.0.0-20220526004731-065cf7ba2467 h1:CBpWXWQpIRjzmkkA+M7q9Fqnwd2mZr3AFqexg8YTfoM= +golang.org/x/term v0.0.0-20220526004731-065cf7ba2467/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/services/graph/pkg/config/config.go b/services/graph/pkg/config/config.go index 9abc881ecdb..f6e6628d13d 100644 --- a/services/graph/pkg/config/config.go +++ b/services/graph/pkg/config/config.go @@ -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"` diff --git a/services/graph/pkg/config/defaults/defaultconfig.go b/services/graph/pkg/config/defaults/defaultconfig.go index 508654e31ce..47379b777bd 100644 --- a/services/graph/pkg/config/defaults/defaultconfig.go +++ b/services/graph/pkg/config/defaults/defaultconfig.go @@ -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", diff --git a/services/graph/pkg/identity/ldap.go b/services/graph/pkg/identity/ldap.go index 959d62e741a..6ac51287f1d 100644 --- a/services/graph/pkg/identity/ldap.go +++ b/services/graph/pkg/identity/ldap.go @@ -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 diff --git a/services/graph/pkg/identity/ldap/reconnect.go b/services/graph/pkg/identity/ldap/reconnect.go index d2b2bedbb17..8a5c405f681 100644 --- a/services/graph/pkg/identity/ldap/reconnect.go +++ b/services/graph/pkg/identity/ldap/reconnect.go @@ -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")) }