Full_Name: Hallvard B Furuseth Version: HEAD OS: URL: http://folk.uio.no/hbf/OpenLDAP/modrdn.diff Submission from: (NULL) (129.240.6.233) Submitted by: hallvard The slapd/modrdn.c check for affectsMultipleDSAs is insufficient, it requires newSuperior to be in the same backend. That does not catch moving "cn=x,cn=y" to another database's suffix "cn=z,cn=y". Also if a database has multiple suffixes it prevents moving an entry to one of the suffixes. slapd/modrdn.c can catch attempts to place an entry above/below itself. It doesn't need to send those to the database and hope that catches it. This patch puts it in the frontend (fe_op_modrdn); that needed to generate the destination DN anyway for the affectsMultipleDSAs check. Not sure if that's right or if it should go in do_modrdn() instead. (I'm thinking of DN rewriting in rwm, vs. global overlays.) In test005-modrdn, the "modrdn with newSuperior as child of target" check would not catch anything: It expects failure, but the newSuperior did not exist and would fail with noSuchObject in any case. This patch uses newSuperior=target instead, and expects unwillingToPerform. I'll apply the patch later, unless someone thinks it should be done differently.
h.b.furuseth@usit.uio.no wrote: > Full_Name: Hallvard B Furuseth > Version: HEAD > OS: > URL: http://folk.uio.no/hbf/OpenLDAP/modrdn.diff > Submission from: (NULL) (129.240.6.233) > Submitted by: hallvard > > > The slapd/modrdn.c check for affectsMultipleDSAs is insufficient, it > requires newSuperior to be in the same backend. That does not catch > moving "cn=x,cn=y" to another database's suffix "cn=z,cn=y". I don't see how it can miss this. > Also > if a database has multiple suffixes it prevents moving an entry > to one of the suffixes. OK, I see that. > slapd/modrdn.c can catch attempts to place an entry above/below itself. > It doesn't need to send those to the database and hope that catches it. > This patch puts it in the frontend (fe_op_modrdn); that needed to > generate the destination DN anyway for the affectsMultipleDSAs check. > Not sure if that's right or if it should go in do_modrdn() instead. > (I'm thinking of DN rewriting in rwm, vs. global overlays.) Probably should look at adding the dest_dn to the op struct, so each backend doesn't have to rebuild it. > In test005-modrdn, the "modrdn with newSuperior as child of target" check > would not catch anything: It expects failure, but the newSuperior did > not exist and would fail with noSuchObject in any case. This patch uses > newSuperior=target instead, and expects unwillingToPerform. > I'll apply the patch later, unless someone thinks it should be done > differently. -- -- Howard Chu Chief Architect, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
hyc@symas.com writes: >> The slapd/modrdn.c check for affectsMultipleDSAs is insufficient, it >> requires newSuperior to be in the same backend. That does not catch >> moving "cn=x,cn=y" to another database's suffix "cn=z,cn=y". > > I don't see how it can miss this. It checks if newSuperior is in another backend, but that move doesn't need a newSuperior. It keeps the same parent: database 1: suffix cn=y database 2: suffix cn=z,cn=y modify rdn: dn "cn=x,cn=y"; newrdn cn=z; newSuperior (if any) cn=y. > Probably should look at adding the dest_dn to the op struct, so each > backend doesn't have to rebuild it. Indeed. And dest_ndn. -- Hallvard
hyc@symas.com writes: > Probably should look at adding the dest_dn to the op struct, so each > backend doesn't have to rebuild it. Third-party overlays which modify newrdn or newSuperior would become incorrect, so maybe this change should wait for OpenLDAP 2.5. -- Hallvard
moved from Incoming to Software Bugs
h.b.furuseth@usit.uio.no wrote: > hyc@symas.com writes: >>> The slapd/modrdn.c check for affectsMultipleDSAs is insufficient, it >>> requires newSuperior to be in the same backend. That does not catch >>> moving "cn=x,cn=y" to another database's suffix "cn=z,cn=y". >> I don't see how it can miss this. > > It checks if newSuperior is in another backend, but that move doesn't > need a newSuperior. It keeps the same parent: > > database 1: suffix cn=y > database 2: suffix cn=z,cn=y > modify rdn: dn "cn=x,cn=y"; newrdn cn=z; newSuperior (if any) cn=y. > >> Probably should look at adding the dest_dn to the op struct, so each >> backend doesn't have to rebuild it. > > Indeed. And dest_ndn. You should apply your initial patch (to HEAD, 2.4), then modify HEAD to pass dest_dn/dest_ndn p. Ing. Pierangelo Masarati OpenLDAP Core Team SysNet s.r.l. via Dossi, 8 - 27100 Pavia - ITALIA http://www.sys-net.it --------------------------------------- Office: +39 02 23998309 Mobile: +39 333 4963172 Email: pierangelo.masarati@sys-net.it ---------------------------------------
changed notes changed state Open to Test
Pierangelo Masarati writes: > You should apply your initial patch (to HEAD, 2.4), Done. > then modify HEAD to pass dest_dn/dest_ndn Later. -- Hallvard
moved from Software Bugs to Development
changed notes changed state Test to Open
Fixed in HEAD, RE24. Todo (HEAD): add newDN field.
https://git.openldap.org/openldap/openldap/-/merge_requests/373
Do we document API changes for people maintaining external modules?
(In reply to Ondřej Kuzník from comment #12) > Do we document API changes for people maintaining external modules? slap.h is the documentation.
Commits: • 1cf39a85 by Ondřej Kuzník at 2021-08-06T15:30:47+01:00 ITS#5344 Record and maintain new DN on ModRDN ops
Commits: • 49ee5d9b by Howard Chu at 2021-08-13T21:09:28+01:00 ITS#5344 slapo-rwm: fix prev commit