Logged in as guest
Viewing Historical/4816 Full headers
Major security issue: yes no
Notes: wontfix Notification:
Date: Fri, 26 Jan 2007 15:43:45 GMT From: gael.roualland@oleane.net To: openldap-its@OpenLDAP.org Subject: slurpd generates invalid ldapadd/modify requests
Full_Name: Ga.l Roualland Version: 2.3.33 OS: Linux URL: ftp://ftp.openldap.org/incoming/gael.roualland-070126.diff Submission from: (NULL) (213.56.0.199) Hello, Slurpd has a bug when replicating, in the way it generates the LDAPMod array of the add and modify operations : if the same attribute is present several times (for add) or in multiple change blocks (for modify) in the replication log, it simply adds all occurences to the LDAPMod array, hence having one or more attributes repeated serveral times which violates the protocol. This is usually fine with slapd because there is code there to accept invalid queries from the updatedn, but it is rejected if you're not using updatedn (which happens to be our case in a custom floating master scenario). The uploaded patch fixes this in slurpd by replacing its LDAPmod logic by one based on ldapmodify which generates proper requests. I know slurpd is considered deprecated, however this might be useful to others still using it and would allow to remove the exception from slapd code. Regards,
Date: Sun, 28 Jan 2007 19:49:39 -0800 From: Howard Chu <hyc@symas.com> To: gael.roualland@oleane.net CC: openldap-its@openldap.org Subject: Re: (ITS#4816) slurpd generates invalid ldapadd/modify requests
gael.roualland@oleane.net wrote: > Full_Name: Ga.l Roualland > Version: 2.3.33 > OS: Linux > URL: ftp://ftp.openldap.org/incoming/gael.roualland-070126.diff > Submission from: (NULL) (213.56.0.199) > > > Hello, > > Slurpd has a bug when replicating, in the way it generates the LDAPMod array of > the add and modify operations : if the same attribute is present several times > (for add) or in multiple change blocks (for modify) in the replication log, it > simply adds all occurences to the LDAPMod array, hence having one or more > attributes repeated serveral times which violates the protocol. > > This is usually fine with slapd because there is code there to accept invalid > queries from the updatedn, but it is rejected if you're not using updatedn > (which happens to be our case in a custom floating master scenario). > > The uploaded patch fixes this in slurpd by replacing its LDAPmod logic by one > based on ldapmodify which generates proper requests. I know slurpd is considered > deprecated, however this might be useful to others still using it and would > allow to remove the exception from slapd code. > > Regards, As far as I can see, slurpd merely processes the changes in the replog. If it is generating requests out of sequence, then they must have been recorded out-of-sequence in the log. And yet I don't see any reason why slapd would generate the modifications out of sequence. What are the circumstances that cause these improper sequences to get into the replog in the first place? As for copying the code from ldapmodify.c into slurpd - probably the better solution would be to move the relevant code from ldapmodify.c into a library. When we still maintained libldif as its own entity it would have made sense to put it there, but now it would seem to best fit in libldap, and perhaps the other LDIF routines in liblutil should move there as well. This would also partially address ITS#4033. -- -- Howard Chu Chief Architect, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc OpenLDAP Core Team http://www.openldap.org/project/
Date: Mon, 29 Jan 2007 11:20:16 +0100 From: =?ISO-8859-1?Q?Ga=EBl_Roualland?= <gael.roualland@oleane.net> To: Howard Chu <hyc@symas.com> CC: openldap-its@openldap.org Subject: Re: (ITS#4816) slurpd generates invalid ldapadd/modify requests
Hello, >> Slurpd has a bug when replicating, in the way it generates the LDAPMod array of >> the add and modify operations : if the same attribute is present several times >> (for add) or in multiple change blocks (for modify) in the replication log, it >> simply adds all occurences to the LDAPMod array, hence having one or more >> attributes repeated serveral times which violates the protocol. >> [...] > As far as I can see, slurpd merely processes the changes in the replog. If it > is generating requests out of sequence, then they must have been recorded > out-of-sequence in the log. And yet I don't see any reason why slapd would > generate the modifications out of sequence. That's right for changetype=modify, since the original code aggregates multiple values of the same attributes in a signle LDAPmod if they're given in order in the replog file. But it doesn't do that for changetype=add, which toggled the bug. I choosed to use the same code path for both for simplicity when fixing add. > What are the circumstances that cause these improper sequences to get into > the replog in the first place? In our case, it was just "add" that was problematic. Although I could imagine the replog to be generated by something else than slapd, and I'd expect slurpd to be able to process it as long as it is valid ldif ? > As for copying the code from ldapmodify.c into slurpd - probably the better > solution would be to move the relevant code from ldapmodify.c into a library. Well, the relevant code is about 20 lines long, and adapted to the way slurpd handles memory. But sure, that could be shared. For the record, the patch I uploaded has an issue with empty changes blocks, so I uploaded a fixed version as ftp://ftp.openldap.org/incoming/gael.roualland-070129.diff -- Ga.l Roualland -+- gael.roualland@oleane.net
Date: Thu, 01 Mar 2007 18:49:03 +0100 From: =?ISO-8859-1?Q?Ga=EBl_Roualland?= <gael.roualland@oleane.net> To: Howard Chu <hyc@symas.com> CC: openldap-its@openldap.org Subject: Re: (ITS#4816) slurpd generates invalid ldapadd/modify requests
This is a multi-part message in MIME format. --------------090002080605050903080106 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit Hello, >> As far as I can see, slurpd merely processes the changes in the replog. If it >> is generating requests out of sequence, then they must have been recorded >> out-of-sequence in the log. And yet I don't see any reason why slapd would >> generate the modifications out of sequence. > > That's right for changetype=modify, since the original code aggregates > multiple values of the same attributes in a signle LDAPmod if they're > given in order in the replog file. > But it doesn't do that for changetype=add, which toggled the bug. I > choosed to use the same code path for both for simplicity when fixing add. > >> What are the circumstances that cause these improper sequences to get into >> the replog in the first place? > > In our case, it was just "add" that was problematic. Although I could > imagine the replog to be generated by something else than slapd, and I'd > expect slurpd to be able to process it as long as it is valid ldif ? The second patch I uploaded has an issue with deleting whole attributes. I have a fixed version of it, but I also wrote another patch to fix the initial issue only on the problematic case (entry adding), so that's a less intrusive change : instead of adding a new LDAPMod for each attribute/value pair, the previous LDAPMod (if any) is reused and the current value added to it if it's for the same attribute. The new patch is attached (no space left on ftp). Would it be possible for such a fix to be in mainline ? Thanks, -- Ga.l Roualland -+- gael.roualland@oleane.net --------------090002080605050903080106 Content-Type: text/x-patch; name="openldap2.3-slurpd-ldapadd.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="openldap2.3-slurpd-ldapadd.diff" --- openldap-2.3.33/servers/slurpd/ldap_op.c.orig 2007-01-25 12:01:46.000000000 +0100 +++ openldap-2.3.33/servers/slurpd/ldap_op.c 2007-03-01 18:05:44.315232000 +0100 @@ -52,7 +52,6 @@ #include "slurp.h" /* Forward references */ -static struct berval **make_singlevalued_berval LDAP_P(( char *, int )); static int op_ldap_add LDAP_P(( Ri *, Re *, char **, int * )); static int op_ldap_modify LDAP_P(( Ri *, Re *, char **, int * )); static int op_ldap_delete LDAP_P(( Ri *, Re *, char **, int * )); @@ -183,28 +182,42 @@ ) { Mi *mi; - int nattrs, rc = 0, i; + int nattrs, nvals, rc = 0, i; LDAPMod *ldm, **ldmarr; int lderr = 0; nattrs = i = 0; ldmarr = NULL; + ldm = NULL; /* * Construct a null-terminated array of LDAPMod structs. */ mi = re->re_mods; while ( mi[ i ].mi_type != NULL ) { - ldm = alloc_ldapmod(); - ldmarr = ( LDAPMod ** ) ch_realloc( ldmarr, - ( nattrs + 2 ) * sizeof( LDAPMod * )); - ldmarr[ nattrs ] = ldm; - ldm->mod_op = LDAP_MOD_BVALUES; - ldm->mod_type = mi[ i ].mi_type; - ldm->mod_bvalues = - make_singlevalued_berval( mi[ i ].mi_val, mi[ i ].mi_len ); + if (ldm == NULL || strcmp(ldm->mod_type, mi[ i ].mi_type) != 0) { + /* not the same attribute, add a new ldm */ + ldm = alloc_ldapmod(); + ldmarr = ( LDAPMod ** ) ch_realloc( ldmarr, + ( nattrs + 2 ) * sizeof( LDAPMod * )); + ldmarr[ nattrs ] = ldm; + nattrs++; + ldm->mod_op = LDAP_MOD_BVALUES; + ldm->mod_type = mi[ i ].mi_type; + nvals = 0; + } + + ldm->mod_bvalues = ( struct berval ** ) + ch_realloc( ldm->mod_bvalues, + ( nvals + 2 ) * sizeof( struct berval * )); + ldm->mod_bvalues[ nvals + 1 ] = NULL; + ldm->mod_bvalues[ nvals ] = ( struct berval * ) + ch_malloc( sizeof( struct berval )); + ldm->mod_bvalues[ nvals ]->bv_val = mi[ i ].mi_val; + ldm->mod_bvalues[ nvals ]->bv_len = mi[ i ].mi_len; + nvals++; + i++; - nattrs++; } if ( ldmarr != NULL ) { @@ -579,25 +592,6 @@ /* - * Create a berval with a single value. - */ -static struct berval ** -make_singlevalued_berval( -char *value, -int len ) -{ - struct berval **p; - - p = ( struct berval ** ) ch_malloc( 2 * sizeof( struct berval * )); - p[ 0 ] = ( struct berval * ) ch_malloc( sizeof( struct berval )); - p[ 1 ] = NULL; - p[ 0 ]->bv_val = value; - p[ 0 ]->bv_len = len; - return( p ); -} - - -/* * Given a modification type (string), return an enumerated type. * Avoids ugly copy in op_ldap_modify - lets us use a switch statement * there. --------------090002080605050903080106--
______________ © Copyright 2009, OpenLDAP Foundation, info@OpenLDAP.org
