OpenLDAP
Up to top level
Build   Contrib   Development   Documentation   Historical   Incoming   Software Bugs   Software Enhancements   Web  

Logged in as guest

Viewing Development/5344
Full headers

From: h.b.furuseth@usit.uio.no
Subject: Wrong check for bad Modify DN
Compose comment
Download message
State:
0 replies:
5 followups: 1 2 3 4 5

Major security issue: yes  no

Notes:

Notification:


Date: Wed, 30 Jan 2008 10:48:55 GMT
From: h.b.furuseth@usit.uio.no
To: openldap-its@OpenLDAP.org
Subject: Wrong check for bad Modify DN
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.


Followup 1

Download message
Date: Wed, 30 Jan 2008 03:10:55 -0800
From: Howard Chu <hyc@symas.com>
To: h.b.furuseth@usit.uio.no
CC: openldap-its@openldap.org
Subject: Re: (ITS#5344) Wrong check for bad Modify DN
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/



Followup 2

Download message
From: Hallvard B Furuseth <h.b.furuseth@usit.uio.no>
Date: Wed, 30 Jan 2008 12:25:54 +0100
To: hyc@symas.com
Cc: openldap-its@openldap.org
Subject: Re: (ITS#5344) Wrong check for bad Modify DN
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



Followup 3

Download message
From: Hallvard B Furuseth <h.b.furuseth@usit.uio.no>
Date: Wed, 30 Jan 2008 12:47:28 +0100
To: hyc@symas.com
Cc: openldap-its@openldap.org
Subject: Re: (ITS#5344) Wrong check for bad Modify DN
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



Followup 4

Download message
Date: Sat, 09 Feb 2008 12:00:41 +0100
From: Pierangelo Masarati <ando@sys-net.it>
To: h.b.furuseth@usit.uio.no
CC: openldap-its@openldap.org
Subject: Re: (ITS#5344) Wrong check for bad Modify DN
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
---------------------------------------




Followup 5

Download message
From: Hallvard B Furuseth <h.b.furuseth@usit.uio.no>
Date: Mon, 11 Feb 2008 22:11:46 +0100
To: Pierangelo Masarati <ando@sys-net.it>
Cc: openldap-its@openldap.org
Subject: Re: (ITS#5344) Wrong check for bad Modify DN
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


Up to top level
Build   Contrib   Development   Documentation   Historical   Incoming   Software Bugs   Software Enhancements   Web  

Logged in as guest


The OpenLDAP Issue Tracking System uses a hacked version of JitterBug

______________
© Copyright 2013, OpenLDAP Foundation, info@OpenLDAP.org