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.