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

Logged in as guest

Viewing Software Bugs/5048
Full headers

From: h.b.furuseth@usit.uio.no
Subject: Suspicious use of 'unchecked' limit syncprov
Compose comment
Download message
State:
0 replies:
8 followups: 1 2 3 4 5 6 7 8

Major security issue: yes  no

Notes:

Notification:


Date: Wed, 18 Jul 2007 14:36:24 GMT
From: h.b.furuseth@usit.uio.no
To: openldap-its@OpenLDAP.org
Subject: Suspicious use of 'unchecked' limit syncprov
Full_Name: Hallvard B Furuseth
Version: HEAD, RE23
OS: 
URL: 
Submission from: (NULL) (129.240.202.105)
Submitted by: hallvard


overlays/syncprov.c:syncprov_findcsn() sets an unchecked limit to 1.
findcsn_cb() says
	/* We just want to know that at least one exists, so it's OK if
	 * we exceed the unchecked limit or size limit.
	 */

This looks like it can return a false positive if two or more other
entries which the filter would eliminate have the same hash as the
value syncprov searches for.

Also syncprov_findcsn() passes fc_limits uninitialized outside of the
.lms_s_unchecked field to slapd.  Valgrind complains in test018 about
.lms_s_pr_hide in back-bdb/search.c:bdb_search().  Tested in HEAD.


Followup 1

Download message
Date: Fri, 20 Jul 2007 06:24:04 -0700
From: Howard Chu <hyc@symas.com>
To: h.b.furuseth@usit.uio.no
CC: openldap-its@openldap.org
Subject: Re: (ITS#5048) Suspicious use of 'unchecked' limit syncprov
h.b.furuseth@usit.uio.no wrote:
> Full_Name: Hallvard B Furuseth
> Version: HEAD, RE23
> OS: 
> URL: 
> Submission from: (NULL) (129.240.202.105)
> Submitted by: hallvard
> 
> 
> overlays/syncprov.c:syncprov_findcsn() sets an unchecked limit to 1.
> findcsn_cb() says
> 	/* We just want to know that at least one exists, so it's OK if
> 	 * we exceed the unchecked limit or size limit.
> 	 */
> 
> This looks like it can return a false positive if two or more other
> entries which the filter would eliminate have the same hash as the
> value syncprov searches for.

I don't believe this can cause any problem though. CSN indexing doesn't use a 
hash the way other indices do; the CSN timestamp is converted to binary form 
and saved as a 40 bit integer. Index collisions will only occur for multiple 
changes that occurred within the same 1-second interval.

The purpose of this search is to detect if a consumer's context CSN is stale, 
i.e., the provider no longer has any entries as old as the consumer's CSN. The 
fact that there is any index collision essentially proves that the CSN is not 
stale.

> Also syncprov_findcsn() passes fc_limits uninitialized outside of the
> .lms_s_unchecked field to slapd.  Valgrind complains in test018 about
> .lms_s_pr_hide in back-bdb/search.c:bdb_search().  Tested in HEAD.

Fixed in HEAD.
-- 
   -- 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: Tue, 11 Jan 2011 10:44:55 +0100
To: Howard Chu <hyc@symas.com>
Cc: openldap-its@openldap.org
Subject: Re: (ITS#5048) Suspicious use of 'unchecked' limit syncprov
Reopening this ITS...

Howard Chu writes:
>> Full_Name: Hallvard B Furuseth
>> 
>> overlays/syncprov.c:syncprov_findcsn() sets an unchecked limit to 1.
>> findcsn_cb() says
>> 	/* We just want to know that at least one exists, so it's OK if
>> 	 * we exceed the unchecked limit or size limit.
>> 	 */
>> 
>> This looks like it can return a false positive if two or more other
>> entries which the filter would eliminate have the same hash as the
>> value syncprov searches for.
> 
> I don't believe this can cause any problem though. CSN indexing doesn't use
a 
> hash the way other indices do; the CSN timestamp is converted to binary
form 
> and saved as a 40 bit integer. Index collisions will only occur for
multiple 
> changes that occurred within the same 1-second interval.

Only if entryCSN is indexed, which is recommended but not required in
man slapo-syncprov.  With un-indexed entryCSN it'll hit the unchecked
limit if there are two or more entries in scope for the search.

Also - another marginal case - findcsn_cb() assumes adminLimitExceeded
implies a size limit (.size or .unchecked).  It could also mean a hard
time limit.  After someone did ^Z on slapd while stepping through some
debugging, if nothing else.

-- 
Hallvard



Followup 3

Download message
Date: Tue, 11 Jan 2011 12:15:17 -0800
From: Howard Chu <hyc@symas.com>
To: Hallvard B Furuseth <h.b.furuseth@usit.uio.no>
CC: openldap-its@openldap.org
Subject: Re: (ITS#5048) Suspicious use of 'unchecked' limit syncprov
Hallvard B Furuseth wrote:
> Reopening this ITS...
>
> Howard Chu writes:
>>> Full_Name: Hallvard B Furuseth
>>>
>>> overlays/syncprov.c:syncprov_findcsn() sets an unchecked limit to
1.
>>> findcsn_cb() says
>>> 	/* We just want to know that at least one exists, so it's OK if
>>> 	 * we exceed the unchecked limit or size limit.
>>> 	 */
>>>
>>> This looks like it can return a false positive if two or more other
>>> entries which the filter would eliminate have the same hash as the
>>> value syncprov searches for.
>>
>> I don't believe this can cause any problem though. CSN indexing doesn't
use a
>> hash the way other indices do; the CSN timestamp is converted to binary
form
>> and saved as a 40 bit integer. Index collisions will only occur for
multiple
>> changes that occurred within the same 1-second interval.
>
> Only if entryCSN is indexed, which is recommended but not required in
> man slapo-syncprov.  With un-indexed entryCSN it'll hit the unchecked
> limit if there are two or more entries in scope for the search.
>
> Also - another marginal case - findcsn_cb() assumes adminLimitExceeded
> implies a size limit (.size or .unchecked).  It could also mean a hard
> time limit.  After someone did ^Z on slapd while stepping through some
> debugging, if nothing else.

Wrong. AdminLimitExceeded ONLY means unchecked here. Otherwise it would be 
SizeLimitExceeded (or TimeLimitExceeded).

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/



Followup 4

Download message
From: Hallvard B Furuseth <h.b.furuseth@usit.uio.no>
Date: Thu, 13 Jan 2011 10:24:09 +0100
To: hyc@symas.com
Cc: openldap-its@openldap.org
Subject: Re: (ITS#5048) Suspicious use of 'unchecked' limit syncprov
hyc@symas.com writes:
> Wrong. AdminLimitExceeded ONLY means unchecked here. Otherwise it would be 
> SizeLimitExceeded (or TimeLimitExceeded).

Okay.  Took me some digging to see that.  Anyway, the main problem is
that hitting the unchecked limit does not imply there is an entry which
even approximately matches the filter.  Not without indexes.

-- 
Hallvard



Followup 5

Download message
Date: Thu, 13 Jan 2011 01:34:18 -0800
From: Howard Chu <hyc@symas.com>
To: Hallvard B Furuseth <h.b.furuseth@usit.uio.no>
CC: openldap-its@openldap.org
Subject: Re: (ITS#5048) Suspicious use of 'unchecked' limit syncprov
Hallvard B Furuseth wrote:
> hyc@symas.com writes:
>> Wrong. AdminLimitExceeded ONLY means unchecked here. Otherwise it would
be
>> SizeLimitExceeded (or TimeLimitExceeded).
>
> Okay.  Took me some digging to see that.  Anyway, the main problem is
> that hitting the unchecked limit does not imply there is an entry which
> even approximately matches the filter.  Not without indexes.
>
I guess that's a fair point, but operating without an entryCSN index would be 
a config mistake anyway. (Note that the unchecked limit only has relevance on 
backends that support indexing in the first place.)

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/



Followup 6

Download message
From: Hallvard B Furuseth <h.b.furuseth@usit.uio.no>
Date: Thu, 13 Jan 2011 10:55:36 +0100
To: hyc@symas.com
Cc: openldap-its@openldap.org
Subject: Re: (ITS#5048) Suspicious use of 'unchecked' limit syncprov
hyc@symas.com writes:
> I guess that's a fair point, but operating without an entryCSN index would
be 
> a config mistake anyway. (Note that the unchecked limit only has relevance
on 
> backends that support indexing in the first place.)

As a user that's a config mistake from which I'd expect poor performance
or sizelimit warnings in syslog, not broken CSN handling.  In particular
when slapo-syncprov(5) does mention indexing without demanding it:

    "On databases that support inequality indexing, it is helpful to
    set an eq index on the entryCSN attribute when using this overlay."

-- 
Hallvard



Followup 7

Download message
From: Hallvard B Furuseth <h.b.furuseth@usit.uio.no>
Date: Thu, 13 Jan 2011 11:05:40 +0100
To: openldap-its@openldap.org
Subject: Re: (ITS#5048) Suspicious use of 'unchecked' limit syncprov
BTW, why do CSN times have 6-decimal precision when we only care
about 1/256 second?  Or doesn't it matter if this test is gives
false positives for small time differences?

-- 
Hallvard



Followup 8

Download message
Date: Thu, 13 Jan 2011 02:13:08 -0800
From: Howard Chu <hyc@symas.com>
To: h.b.furuseth@usit.uio.no
CC: openldap-its@openldap.org
Subject: Re: (ITS#5048) Suspicious use of 'unchecked' limit syncprov
h.b.furuseth@usit.uio.no wrote:
> BTW, why do CSN times have 6-decimal precision when we only care
> about 1/256 second?  Or doesn't it matter if this test is gives
> false positives for small time differences?
>
Yeah, we really just don't care. 6-decimal precision is for conflict 
resolution, this is just to determine if something is "new enough" where 
"enough" doesn't need to be exact.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/


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