(ITS#6135) Changed TLS settings in cn=config require a restart
by hyc＠OpenLDAP.org
Full_Name: Howard Chu
Version: 2.4.16/HEAD
OS: Solaris 10
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (76.91.220.157)
Submitted by: hyc
I thought we already had an ITS for this but didn't find it...
slapd's global TLS settings are stored in an SSL context that only gets
initialized at startup time. So changes to these settings via cn=config take no
effect until the next restart. Changes to the other TLS users (syncrepl,
back-ldap, back-meta) take effect immediately, as expected. bconfig.c's
config_tls_config needs to check whether slapd is online or not, and
reinitialize the global context after these changes if so.
14 years, 6 months
(ITS#6133) back-relay issues
by h.b.furuseth＠usit.uio.no
Full_Name: Hallvard B Furuseth
Version: HEAD
OS: Linux
URL:
Submission from: (NULL) (129.240.6.233)
Submitted by: hallvard
Back-relay issues - fixing some, awaiting confirmation/discussion of others:
* The handlers for Abandon, Cancel and connection-init/destroy
should not exist, as far as I can tell. They forward the call to the
underlying backend database, but that means it receives the call twice:
Slapd calls these handlers for all databases anyway (unless one returns
success in case of Cancel), since there's no DN on which to dispatch.
See abandon.c:fe_op_abandon(), cancel.c:cancel_extop(), and
backend.c:backend_connection_init()/backend_connection_destroy().
The Cancel handler seems broken anyway. It sets op->o_cancel =
LDAP_CANNOT_CANCEL on the Cancel operation. Presumably it intended
the to-be-cancelled operation.
I'm "#if 0"ing these out for now in case I've missed something.
Can remove the code later.
* Fixing RB_ERR (set rs->sr_err) and RB_SEND (send if error) handling:
relay_back_select_backend() did not obey (!(fail_mode & RB_ERR))
except when it caught recursion.
LDAP_SUCCESS makes no sense combined with these flags.
relay_back_operational() could set rs->sr_err = success on failure,
which seemed a bit excessive. (Though it should return success.)
Removing the RB_ERR flag.
relay_back_chk_referrals() can send success on failure. Except
the function is not used, so I'm just #if 0ing and commenting it.
This also means relay_back_select_backend() without these flags
could just as well get SlapReply rs == NULL, which again means
rs==NULL could function as a !RB_ERR flag. Not doing that yet,
in case the changes below lead to some reason not to.
* Fixing relay_back_select_backend() crash when select_backend()==NULL:
include servers/slapd/schema/core.schema
database relay
suffix cn=relay,cn=test
overlay rwm
rwm-suffixmassage cn=nowhere
ldapsearch -b cn=urgle,cn=relay,cn=test -s base
* back-relay can be configured to cause infinite recursion.
This dumps core with the above search:
include servers/slapd/schema/core.schema
database relay
subordinate
suffix cn=relay,cn=test
relay cn=relay,cn=test
database null
suffix cn=test
There are some tests to catch that, but half-hearted ones.
And some comments about it, like this:
/* FIXME: this test only works if there are no overlays, so
* it is nearly useless; if made stricter, no nested back-relays
* can be instantiated... too bad. */
if ( bd == NULL || bd == op->o_bd ) {
return LDAP_SUCCESS;
}
I don't know how that's worse in this case than in other cases,
nor what exactly how that snippet is supposed to work, so I don't
know if the suggestion below fixes it and the test can be removed.
The 'bd == op->o_bd' part can be removed since
relay_back_select_backend() now returns bd==NULL when that happens:
(bd->be_private == op->o_bd->be_private) implies (bd == op->o_bd).
Anyway, recursion can now be properly caught with op->o_extra. Wrap
calls to the underlying backend (e.g. in relay_back_op) in - untested:
OpExtraDB oex;
oex.oe_db = op->o_bd;
oex.oe.oe_key = op->o_bd->be_private;
LDAP_SLIST_INSERT_HEAD( &op->o_extra, &oex.oe, oe_next );
...
LDAP_SLIST_REMOVE( &op->o_extra, &oex.oe, OpExtra, oe_next );
and check in relay_back_select_backend() with
relay_back_info *ri = (relay_back_info *)op->o_bd->be_private;
OpExtra *oex;
LDAP_SLIST_FOREACH( oex, &op->o_extra, oe_next ) {
if ( oex->oe_key == ri )
<caught recursion, fail and return NULL>;
}
However, does the comment quoted above mean that it's desirable to
allow a few rounds of recursion (a back-relay instance calling
itself)?
If so, define a config parameter relay-max-recursion with default=0,
and define a struct with the OpExtra members + a counter which says
how many rounds of recursion the operation has had so far for this
database. See the OpExtra comment in slap.h.
14 years, 6 months
(ITS#6132) slapi_entry_has_children broken
by h.b.furuseth＠usit.uio.no
Full_Name: Hallvard B Furuseth
Version: HEAD
OS:
URL:
Submission from: (NULL) (129.240.6.233)
Submitted by: hallvard
slapi_entry_has_children() does not check for errors, and also calls
the database's be_has_subordinates handler even when it does not exist.
Finally, when there is no be_has_subordinates to call, it builds
and destroys a pblock which isn't used for anyting. I don't know
what a pblock is, but it looks like a waste of time.
Fixing.
14 years, 6 months
(ITS#6131) "TLSVerifyClient try" not working with GNU TLS
by subbarao＠computer.org
Full_Name: Kartik Subbarao
Version: 2.4.16
OS: Debian 5.0.1
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (76.99.175.5)
When TLSVerifyClient is set to "try", OpenLDAP improperly rejects SSL
connections without a client certificate. The problem appears to start with this
section of code in tls.c around line 1564:
#ifdef HAVE_GNUTLS
if ( ld->ld_options.ldo_tls_require_cert != LDAP_OPT_X_TLS_NEVER ) {
err = tls_cert_verify( ssl );
if ( err && ld->ld_options.ldo_tls_require_cert != LDAP_OPT_X_TLS_ALLOW
)
return err;
}
#endif
tls_cert_verify() calls gnutls_certificate_verify_peers2(), which appears to
return error 49 when no client certificate is presented. tls_cert_verify()
doesn't seem to distinguish between this case, and the case of an invalid client
certificate, returning -1 in both cases.
14 years, 6 months
Re: (ITS#6129) slapd seg faults on shut-down
by michael＠stroeder.com
Howard Chu wrote:
> This appears to be a Cyrus/SASL issue.
>
> In particular, your trace of thread 1 shows that slapd is shutting down
> the SASL library and SASL is trying to tear down libldap. Inside slapd,
> the Cyrus SASL library should never be touching libldap. As such, you
> should check your SASL configuration, you probably have a plugin enabled
> that shouldn't be.
You're right. Removing cyrus-sasl-ldap-auxprop solved this problem.
Sorry for the noise.
Ciao, Michael.
14 years, 6 months
Re: (ITS#6129) slapd seg faults on shut-down
by hyc＠symas.com
michael(a)stroeder.com wrote:
> Full_Name: Michael Ströder
> Version: RE24
> OS: openSUSE Linux 11.1
> URL:
> Submission from: (NULL) (84.163.72.172)
This appears to be a Cyrus/SASL issue.
In particular, your trace of thread 1 shows that slapd is shutting down the
SASL library and SASL is trying to tear down libldap. Inside slapd, the Cyrus
SASL library should never be touching libldap. As such, you should check your
SASL configuration, you probably have a plugin enabled that shouldn't be.
> gdb output:
>
> [New Thread 3953]
> Core was generated by `/opt/openldap-RE24/libexec/slapd -d 0 -h
> ldap://0.0.0.0:2072 -n slapd-ffg2009-2'.
> Program terminated with signal 11, Segmentation fault.
> #0 0xb799eef5 in free () from /lib/libc.so.6
> (gdb) info threads
> 2 Thread 3953 0xffffe430 in __kernel_vsyscall ()
> * 1 Thread 3951 0xb799eef5 in free () from /lib/libc.so.6
> (gdb) thread apply all bt
>
> Thread 2 (Thread 3953):
> #0 0xffffe430 in __kernel_vsyscall ()
> #1 0xb7ce37b9 in __lll_lock_wait () from /lib/libpthread.so.0
> #2 0xb7cdece0 in _L_lock_286 () from /lib/libpthread.so.0
> #3 0xb7cde705 in pthread_mutex_lock () from /lib/libpthread.so.0
> #4 0xb7a387c9 in dl_iterate_phdr () from /lib/libc.so.6
> #5 0xb7a3ada5 in _Unwind_Find_FDE () from /lib/libc.so.6
> #6 0xb544764d in ?? () from /lib/libgcc_s.so.1
> #7 0xb5447e88 in ?? () from /lib/libgcc_s.so.1
> #8 0xb544818e in _Unwind_ForcedUnwind () from /lib/libgcc_s.so.1
> #9 0xb7ce5aa6 in _Unwind_ForcedUnwind () from /lib/libpthread.so.0
> #10 0xb7ce3491 in __pthread_unwind () from /lib/libpthread.so.0
> #11 0xb7cdd5c0 in pthread_exit () from /lib/libpthread.so.0
> #12 0xb7ed1593 in ldap_pvt_thread_exit (retval=0x0) at thr_posix.c:186
> #13 0xb7ed0574 in ldap_int_thread_pool_wrapper (xpool=0x81d2fc8) at tpool.c:691
> #14 0xb7cdd1b5 in start_thread () from /lib/libpthread.so.0
> #15 0xb7a023be in clone () from /lib/libc.so.6
>
> Thread 1 (Thread 3951):
> #0 0xb799eef5 in free () from /lib/libc.so.6
> #1 0xb7ebad4d in ber_memfree_x (p=0x7fffffff, ctx=0x0) at memory.c:152
> #2 0xb77af997 in ?? () from /usr/lib/libldap-2.4.so.2
> #3 0xb7791a68 in ?? () from /usr/lib/libldap-2.4.so.2
> #4 0xb77c0210 in _fini () from /usr/lib/libldap-2.4.so.2
> #5 0xb7f28aff in ?? () from /lib/ld-linux.so.2
> #6 0xb7f29447 in ?? () from /lib/ld-linux.so.2
> #7 0xb7cb8cf4 in ?? () from /lib/libdl.so.2
> #8 0xb7f239f6 in ?? () from /lib/ld-linux.so.2
> #9 0xb7cb911c in ?? () from /lib/libdl.so.2
> #10 0xb7cb8d2a in dlclose () from /lib/libdl.so.2
> #11 0xb7ccf411 in _sasl_done_with_plugins () from /usr/lib/libsasl2.so.2
> #12 0xb7cc7c90 in sasl_done () from /usr/lib/libsasl2.so.2
> #13 0x080d9fb7 in slap_sasl_destroy ()
> #14 0x080aed1b in slap_destroy ()
> #15 0x08059fe2 in main ()
> (gdb) bt full
> #0 0xb799eef5 in free () from /lib/libc.so.6
> No symbol table info available.
> #1 0xb7ebad4d in ber_memfree_x (p=0x7fffffff, ctx=0x0) at memory.c:152
> __PRETTY_FUNCTION__ = "ber_memfree_x"
> #2 0xb77af997 in ?? () from /usr/lib/libldap-2.4.so.2
> No symbol table info available.
> #3 0xb7791a68 in ?? () from /usr/lib/libldap-2.4.so.2
> No symbol table info available.
> #4 0xb77c0210 in _fini () from /usr/lib/libldap-2.4.so.2
> No symbol table info available.
> #5 0xb7f28aff in ?? () from /lib/ld-linux.so.2
> No symbol table info available.
> #6 0xb7f29447 in ?? () from /lib/ld-linux.so.2
> No symbol table info available.
> #7 0xb7cb8cf4 in ?? () from /lib/libdl.so.2
> No symbol table info available.
> #8 0xb7f239f6 in ?? () from /lib/ld-linux.so.2
> No symbol table info available.
> #9 0xb7cb911c in ?? () from /lib/libdl.so.2
> No symbol table info available.
> #10 0xb7cb8d2a in dlclose () from /lib/libdl.so.2
> No symbol table info available.
> #11 0xb7ccf411 in _sasl_done_with_plugins () from /usr/lib/libsasl2.so.2
> No symbol table info available.
> #12 0xb7cc7c90 in sasl_done () from /usr/lib/libsasl2.so.2
> No symbol table info available.
> #13 0x080d9fb7 in slap_sasl_destroy ()
> No symbol table info available.
> #14 0x080aed1b in slap_destroy ()
> No symbol table info available.
> #15 0x08059fe2 in main ()
> No symbol table info available.
>
>
>
--
-- Howard Chu
CTO, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
14 years, 6 months
Re: (ITS#6104) race condition with cancel operation
by h.b.furuseth＠usit.uio.no
Howard Chu writes:
> I'd prefer to condense it all into a bitfield.
Right... keep both o_abandon and o_cancel (maybe renamed to o_dummy) for
RE24 though, otherwise we have a silent binary incompatible change with
already-compiled slapd modules. The change will in any case break
modules that play with o_cancel though.
> We already ensure that
> the c_mutex is held when setting these flags.
Not yet, but c_mutex it is:-) Must protect the following code.
Or rather, bitflag code which will replace it:
- o_cancel access in cancel.c, connection.c:connection_operation(),
back-relay/op.c.
- o_abandon setting in overlays/syncprov.c:syncprov_op_abandon(), and
in overlays/retcode.c where hopefully the first case should have been
if ( op2.o_abandon ) op->o_abandon = 1;
so that we can never reset o_abandon once it is set.
> It would be a lot of
> overhead to grab the mutex just to read the flag,
> and would require additional analysis to make sure the new locking
> behavior doesn't introduce new deadlocks.
Checking o_abandon!=0 works unmutexed. (Well, technically any
such unmutexed access is wrong, but in practice we should be OK.)
As long as we:
- never reset o_abandon like retcode above can do, and
- never unmutexed assume consistency betweeen abandon-related variables
set by different threads.
So the equivalent of this should be OK:
if ( op->o_abandon ) rs->sr_err = SLAPD_ABANDON;
...
if ( rs->sr_err == SLAPD_ABANDON ) {
lock c_mutex;
more careful o_abandon checks;
unlock c_mutex;
}
While I'm looking at this:
The ldap_pvt_thread_yield() in cancel.c is really ugly. There is no
cond to attach to it to without breaking binary compatibility, but even
a even a global mutex+cond might be better, with threads broadcasting on
it when it might be relevant.
It's still ugly though. Cancelling an expensive operation ought to be a
friendly operation, but instead it is unfriendly in that yet another
thread is occupied, solely to wait for that operation. What would make
sense (for RE25) would be to redo Cancel so that the cancelled thread is
responsible for sending the Cancel response or resubmitting an operation
which will.
Also, the be_cancel implementation in at least back-relay looks wrong.
Cancel isn't dispatched on DN, and back-relay with no 'relay' directive
configured cannot know where to dispatch. I haven't checked how it
works, but it seems a call to be_cancel means the Cancel operation
_might_ belong to this backend. But it might not, which means
back-relay shouldn't set op->o_cancel in this case. What it should do
I don't know though. And I don't know if syncprov is doing any better.
--
Hallvard
14 years, 6 months
