openldap-bugs

openldap-bugs@openldap.org

1 participants
20961 discussions

(ITS#6138) Bad Cancel/Abandon/"internal abandon"/Syncprov interactions
by h.b.furuseth＠usit.uio.no 22 May '09

22 May '09

Full_Name: Hallvard B Furuseth Version: HEAD OS: URL: Submission from: (NULL) (129.240.6.233) Submitted by: hallvard Might make this a tracking issue for Cancel/Abandon problems. And/or copy some to separate ITSes, but they all seem interconnected: Cancel(abandoned operation)-requests are not rejected. Thus slapd sets o_cancel and turns an active Abandon into a Cancel. Presumably that can confuse Cancel/Abandon handlers, like that in Syncprov. Similarly, Abandon(abandoned/cancelled operation) is not ignored. connection_abandon() re-abandons abandoned/cancelled operations too. However Syncprov:RefreshAndPersist abuses op->o_abandon: It sets it to mean "Suppress the response. A copy of this operation will send it." So if Cancel(op with o_abandon!=0) is fixed to respond protocolError "already abandoned", presumably that breaks Cancel(RefreshAndPersist). I'm not touching it with a flagpole - I don't know syncprov. Help? Overlay retcode does something similar - sends a response and then sets o_abandon. Cancel/Abandon can in any case fail by targeting the wrong operation though: A connection can have multiple messages with the same IDs when the response is sent and the client reuses the message ID, before the old operation in slapd can clean up and finish. syncprov_op_abandon() identifies messages by (connid, msgid). Can multiple messages with the same ID break that, or more importantly, break what gets sent/written with syncrepl? Anyway, the current code needs an o_abandon value which means "suppress response". Or maybe "abandoned, except as far as future Abandon and Cancel operations are concerned". Syncprov and Retcode need to handle the various possible orderings of Cancel/Abandon vs Suppress, including when they forward o_abandon from one operation to another. I haven't looked too closely at that code either. ITS#6104 (race condition with cancel operation) also calls for multiple o_abandon values (joining some or all of o_cancel into o_abandon), and must be considered when picking a value. However I suggest to consider just the various possible values first and get that right, and deal with the race condition later. Source and binary compatibility with third-party modules complicates this, if we're trying to support both and old compiled slapd + new compiled module and vice versa. Might try a minimal solution now.

1 0

Re: (ITS#6137) Cancel(pending operation) acts like Abandon
by h.b.furuseth＠usit.uio.no 22 May '09

22 May '09

Confirmed & fixed. Responds "too busy for Cancel, try Abandon instead". If someone wants to, they could improve that. As noted in the commit: /* TODO: We could instead remove the cancelled operation * from c_pending_ops like Abandon does, and send its * response here. Not if it is pending because of a * congested connection though. */ -- Hallvard

1 0

Re: (ITS#6137) Cancel(pending operation) acts like Abandon
by h.b.furuseth＠usit.uio.no 21 May '09

21 May '09

Correction: the Cancel operation responds, since the frontend sends the response to extended operations. But the cancelled operation doesn't. -- Hallvard

1 0

(ITS#6137) Cancel(pending operation) acts like Abandon
by h.b.furuseth＠usit.uio.no 21 May '09

21 May '09

Full_Name: Hallvard B Furuseth Version: HEAD OS: URL: Submission from: (NULL) (129.240.6.233) Submitted by: hallvard Looking at the code (not sure how to reproduce): Cancel(pending operation) abandons the operation, without responding to either Cancel or the cancelled operation. That code in slapd/cancel.c is copied from the equivalent abandon.c code. Cancel as currently written must wait for the cancelled operation to complete. But an active operation must not wait for a pending one, all threads could block. For now we can make Cancel respond with LDAP_CANNOT_CANCEL: "can't handle Cancel now, try Abandon". Repeating myself, a better fix (RE25?) would be to make the cancelled operation send the Cancel response. Don't how deep surgery that'd need to slapd.

1 0

Re: (ITS#5328) backends sending/setting results
by masarati＠aero.polimi.it 21 May '09

21 May '09

h.b.furuseth(a)usit.uio.no wrote: > Fixed relay_back_has_subordinates(). > I've lost track of what I thought to do with back-ldap. I think you should just ignore it, otherwise all you could do is to to perform a onelevel search for each entry (!) that requests the attribute. We (dunno if it's documented) do not guarantee hasSubordinates is correct, all in all. p.

1 0

(ITS#6136) replicated slapd.d interrupts syncrepl refresh
by mbackes＠symas.com 21 May '09

21 May '09

Full_Name: Matthew Backes Version: 2.4, probably head? OS: any URL: Submission from: (NULL) (12.49.250.67) If you modify an object in cn=config in a multi-master environment with replicated cn=config, other databases on replicating other-masters in the refresh-state will be interrupted when they receive the change. Applying any change to their local cn=config resumes the syncrepl refresh operation.

1 0

(ITS#6135) Changed TLS settings in cn=config require a restart
by hyc＠OpenLDAP.org 21 May '09

21 May '09

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.

1 0

(ITS#6134) Missing ITS state/directory documentation
by h.b.furuseth＠usit.uio.no 21 May '09

21 May '09

Full_Name: Hallvard B Furuseth Version: OS: URL: http://www.openldap.org/lists/openldap-commit/200902/msg00172.html Submission from: (NULL) (129.240.6.233) Submitted by: hallvard https://www.openldap.org/its/help.html does not document states Active and Partial. From the enclosed commit URL: Active - bug is being worked on Partial - a partial fix was released Also the help page's directory list shows "Archive.*", but the ITS system seems to have no match for that.

1 0

Re: (ITS#5328) backends sending/setting results
by h.b.furuseth＠usit.uio.no 21 May '09

21 May '09

Fixed relay_back_has_subordinates(). I've lost track of what I thought to do with back-ldap. -- Hallvard

1 0

(ITS#6133) back-relay issues
by h.b.furuseth＠usit.uio.no 21 May '09

21 May '09

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.

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

openldap-bugs