mirror of
https://git.openldap.org/openldap/openldap.git
synced 2025-12-25 09:09:54 -05:00
Preliminary Fixes for ITS#24, ITS#26, and ldbm_back_add race condition.
Resolved deadlock by passing target entry to be_group and using this if dn same as bdn. It might actually be safer to check entry ids instead of dns. Resolved bogus add to cache after failed acl check by deferring cache add until after parent/acl checks have successful been completed. Eliminated race condition caused by concurrent adds of same dn by adding 'li_add_mutex' around the critical section of code (most of ldbm_back_add). This code is preliminary and still needs significant testing.
This commit is contained in:
parent
595bf86635
commit
64cd7d3346
8 changed files with 89 additions and 48 deletions
|
|
@ -360,7 +360,9 @@ acl_access_allowed(
|
|||
/* see if asker is listed in dnattr */
|
||||
string_expand(buf, sizeof(buf), b->a_group, edn, matches);
|
||||
|
||||
if (be_group(be, buf, odn, b->a_objectclassvalue, b->a_groupattrname) == 0) {
|
||||
if (be_group(be, e, buf, odn,
|
||||
b->a_objectclassvalue, b->a_groupattrname) == 0)
|
||||
{
|
||||
Debug( LDAP_DEBUG_ACL,
|
||||
"<= acl_access_allowed: matched by clause #%d (group) access granted\n",
|
||||
i, 0, 0 );
|
||||
|
|
|
|||
|
|
@ -28,22 +28,22 @@ ldbm_back_add(
|
|||
|
||||
Debug(LDAP_DEBUG_ARGS, "==> ldbm_back_add: %s\n", dn, 0, 0);
|
||||
|
||||
pthread_mutex_lock(&li->li_add_mutex);
|
||||
|
||||
if ( ( dn2id( be, dn ) ) != NOID ) {
|
||||
pthread_mutex_unlock(&li->li_add_mutex);
|
||||
entry_free( e );
|
||||
free( dn );
|
||||
send_ldap_result( conn, op, LDAP_ALREADY_EXISTS, "", "" );
|
||||
return( -1 );
|
||||
}
|
||||
|
||||
/* XXX race condition here til we cache_add_entry_lock below XXX */
|
||||
|
||||
if ( global_schemacheck && oc_schema_check( e ) != 0 ) {
|
||||
pthread_mutex_unlock(&li->li_add_mutex);
|
||||
|
||||
Debug( LDAP_DEBUG_TRACE, "entry failed schema check\n",
|
||||
0, 0, 0 );
|
||||
|
||||
/* XXX this should be ok, no other thread should have access
|
||||
* because e hasn't been added to the cache yet
|
||||
*/
|
||||
entry_free( e );
|
||||
free( dn );
|
||||
send_ldap_result( conn, op, LDAP_OBJECT_CLASS_VIOLATION, "",
|
||||
|
|
@ -51,28 +51,6 @@ ldbm_back_add(
|
|||
return( -1 );
|
||||
}
|
||||
|
||||
/*
|
||||
* Try to add the entry to the cache, assign it a new dnid
|
||||
* and mark it locked. This should only fail if the entry
|
||||
* already exists.
|
||||
*/
|
||||
|
||||
e->e_id = next_id( be );
|
||||
if ( cache_add_entry_lock( &li->li_cache, e, ENTRY_STATE_CREATING )
|
||||
!= 0 ) {
|
||||
Debug( LDAP_DEBUG_ANY, "cache_add_entry_lock failed\n", 0, 0,
|
||||
0 );
|
||||
next_id_return( be, e->e_id );
|
||||
|
||||
/* XXX this should be ok, no other thread should have access
|
||||
* because e hasn't been added to the cache yet
|
||||
*/
|
||||
entry_free( e );
|
||||
free( dn );
|
||||
send_ldap_result( conn, op, LDAP_ALREADY_EXISTS, "", "" );
|
||||
return( -1 );
|
||||
}
|
||||
|
||||
/*
|
||||
* Get the parent dn and see if the corresponding entry exists.
|
||||
* If the parent does not exist, only allow the "root" user to
|
||||
|
|
@ -85,6 +63,7 @@ ldbm_back_add(
|
|||
|
||||
/* get entry with reader lock */
|
||||
if ( (p = dn2entry_r( be, pdn, &matched )) == NULL ) {
|
||||
pthread_mutex_unlock(&li->li_add_mutex);
|
||||
Debug( LDAP_DEBUG_TRACE, "parent does not exist\n", 0,
|
||||
0, 0 );
|
||||
send_ldap_result( conn, op, LDAP_NO_SUCH_OBJECT,
|
||||
|
|
@ -94,32 +73,62 @@ ldbm_back_add(
|
|||
free( matched );
|
||||
}
|
||||
|
||||
rc = -1;
|
||||
goto return_results;
|
||||
entry_free( e );
|
||||
free( dn );
|
||||
return -1;
|
||||
}
|
||||
|
||||
if ( ! access_allowed( be, conn, op, p, "children", NULL,
|
||||
op->o_dn, ACL_WRITE ) ) {
|
||||
op->o_dn, ACL_WRITE ) )
|
||||
{
|
||||
pthread_mutex_unlock(&li->li_add_mutex);
|
||||
Debug( LDAP_DEBUG_TRACE, "no access to parent\n", 0,
|
||||
0, 0 );
|
||||
send_ldap_result( conn, op, LDAP_INSUFFICIENT_ACCESS,
|
||||
"", "" );
|
||||
|
||||
rc = -1;
|
||||
goto return_results;
|
||||
entry_free( e );
|
||||
free( dn );
|
||||
return -1;
|
||||
}
|
||||
|
||||
} else {
|
||||
if ( ! be_isroot( be, op->o_dn ) ) {
|
||||
pthread_mutex_unlock(&li->li_add_mutex);
|
||||
Debug( LDAP_DEBUG_TRACE, "no parent & not root\n", 0,
|
||||
0, 0 );
|
||||
send_ldap_result( conn, op, LDAP_INSUFFICIENT_ACCESS,
|
||||
"", "" );
|
||||
|
||||
rc = -1;
|
||||
goto return_results;
|
||||
entry_free( e );
|
||||
free( dn );
|
||||
return -1;
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* Try to add the entry to the cache, assign it a new dnid
|
||||
* and mark it locked. This should only fail if the entry
|
||||
* already exists.
|
||||
*/
|
||||
|
||||
e->e_id = next_id( be );
|
||||
if ( cache_add_entry_lock( &li->li_cache, e, ENTRY_STATE_CREATING ) != 0 ) {
|
||||
pthread_mutex_unlock(&li->li_add_mutex);
|
||||
|
||||
Debug( LDAP_DEBUG_ANY, "cache_add_entry_lock failed\n", 0, 0,
|
||||
0 );
|
||||
next_id_return( be, e->e_id );
|
||||
|
||||
/* XXX this should be ok, no other thread should have access
|
||||
* because e hasn't been added to the cache yet
|
||||
*/
|
||||
entry_free( e );
|
||||
free( dn );
|
||||
send_ldap_result( conn, op, LDAP_ALREADY_EXISTS, "", "" );
|
||||
return( -1 );
|
||||
}
|
||||
|
||||
/*
|
||||
* add it to the id2children index for the parent
|
||||
*/
|
||||
|
|
@ -192,5 +201,8 @@ return_results:;
|
|||
cache_return_entry_r( &li->li_cache, p );
|
||||
}
|
||||
|
||||
/* it might actually be okay to release this lock sooner */
|
||||
pthread_mutex_unlock(&li->li_add_mutex);
|
||||
|
||||
return( rc );
|
||||
}
|
||||
|
|
|
|||
|
|
@ -106,6 +106,7 @@ struct attrinfo {
|
|||
|
||||
struct ldbminfo {
|
||||
ID li_nextid;
|
||||
pthread_mutex_t li_add_mutex;
|
||||
pthread_mutex_t li_nextid_mutex;
|
||||
int li_mode;
|
||||
char *li_directory;
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@
|
|||
int
|
||||
ldbm_back_group(
|
||||
Backend *be,
|
||||
Entry *target,
|
||||
char *bdn,
|
||||
char *edn,
|
||||
char *objectclassValue,
|
||||
|
|
@ -28,6 +29,7 @@ ldbm_back_group(
|
|||
{
|
||||
struct ldbminfo *li = (struct ldbminfo *) be->be_private;
|
||||
Entry *e;
|
||||
char *tdn;
|
||||
char *matched;
|
||||
Attribute *objectClass;
|
||||
Attribute *member;
|
||||
|
|
@ -38,15 +40,30 @@ ldbm_back_group(
|
|||
Debug( LDAP_DEBUG_TRACE, "=> ldbm_back_group: objectClass: %s attrName: %s\n",
|
||||
objectclassValue, groupattrName, 0 );
|
||||
|
||||
/* can we find bdn entry with reader lock */
|
||||
if ((e = dn2entry_r(be, bdn, &matched )) == NULL) {
|
||||
Debug( LDAP_DEBUG_TRACE, "=> ldbm_back_group: cannot find bdn: %s matched: %s\n", bdn, (matched ? matched : ""), 0 );
|
||||
if (matched != NULL)
|
||||
free(matched);
|
||||
return( 1 );
|
||||
tdn = dn_normalize_case( ch_strdup( target->e_dn ) );
|
||||
if (strcmp(tdn, bdn) == 0) {
|
||||
/* we already have a LOCKED copy of the entry */
|
||||
e = target;
|
||||
Debug( LDAP_DEBUG_ARGS,
|
||||
"=> ldbm_back_group: target is bdn: %s\n",
|
||||
bdn, 0, 0 );
|
||||
} else {
|
||||
/* can we find bdn entry with reader lock */
|
||||
if ((e = dn2entry_r(be, bdn, &matched )) == NULL) {
|
||||
Debug( LDAP_DEBUG_TRACE,
|
||||
"=> ldbm_back_group: cannot find bdn: %s matched: %s\n",
|
||||
bdn, (matched ? matched : ""), 0 );
|
||||
if (matched != NULL)
|
||||
free(matched);
|
||||
free(tdn);
|
||||
return( 1 );
|
||||
}
|
||||
Debug( LDAP_DEBUG_ARGS,
|
||||
"=> ldbm_back_group: found bdn: %s\n",
|
||||
bdn, 0, 0 );
|
||||
}
|
||||
free(tdn);
|
||||
|
||||
Debug( LDAP_DEBUG_ARGS, "=> ldbm_back_group: found bdn: %s\n", bdn, 0, 0 );
|
||||
|
||||
/* check for deleted */
|
||||
|
||||
|
|
@ -90,8 +107,10 @@ ldbm_back_group(
|
|||
}
|
||||
}
|
||||
|
||||
/* free entry and reader lock */
|
||||
cache_return_entry_r( &li->li_cache, e );
|
||||
if( target != e ) {
|
||||
/* free entry and reader lock */
|
||||
cache_return_entry_r( &li->li_cache, e );
|
||||
}
|
||||
Debug( LDAP_DEBUG_ARGS, "ldbm_back_group: rc: %d\n", rc, 0, 0 );
|
||||
return(rc);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -63,6 +63,7 @@ ldbm_back_init(
|
|||
free( argv[ 1 ] );
|
||||
|
||||
/* initialize various mutex locks & condition variables */
|
||||
pthread_mutex_init( &li->li_add_mutex, pthread_mutexattr_default );
|
||||
pthread_mutex_init( &li->li_cache.c_mutex, pthread_mutexattr_default );
|
||||
pthread_mutex_init( &li->li_nextid_mutex, pthread_mutexattr_default );
|
||||
pthread_mutex_init( &li->li_dbcache_mutex, pthread_mutexattr_default );
|
||||
|
|
|
|||
|
|
@ -261,6 +261,7 @@ be_unbind(
|
|||
int
|
||||
be_group(
|
||||
Backend *be,
|
||||
Entry *e,
|
||||
char *bdn,
|
||||
char *edn,
|
||||
char *objectclassValue,
|
||||
|
|
@ -268,7 +269,8 @@ be_group(
|
|||
)
|
||||
{
|
||||
if (be->be_group)
|
||||
return(be->be_group(be, bdn, edn, objectclassValue, groupattrName));
|
||||
return(be->be_group(be, e, bdn, edn,
|
||||
objectclassValue, groupattrName));
|
||||
else
|
||||
return(1);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -256,7 +256,8 @@ extern struct acl *global_acl;
|
|||
extern struct objclass *global_oc;
|
||||
extern time_t currenttime;
|
||||
|
||||
extern int be_group LDAP_P((Backend *be, char *bdn, char *edn, char *objectclassValue, char *groupattrName));
|
||||
extern int be_group LDAP_P((Backend *be, Entry *e,
|
||||
char *bdn, char *edn, char *objectclassValue, char *groupattrName));
|
||||
extern void init LDAP_P((void));
|
||||
extern void be_unbind LDAP_P((Connection *conn, Operation *op));
|
||||
extern void config_info LDAP_P((Connection *conn, Operation *op));
|
||||
|
|
@ -295,7 +296,8 @@ extern void ldbm_back_abandon LDAP_P((Backend *be, Connection *c, Operation *o,
|
|||
extern void ldbm_back_config LDAP_P((Backend *be, char *fname, int lineno, int argc, char **argv ));
|
||||
extern void ldbm_back_init LDAP_P((Backend *be));
|
||||
extern void ldbm_back_close LDAP_P((Backend *be));
|
||||
extern int ldbm_back_group LDAP_P((Backend *be, char *bdn, char *edn, char *objectclassValue, char *groupattrName ));
|
||||
extern int ldbm_back_group LDAP_P((Backend *be, Entry *target,
|
||||
char *bdn, char *edn, char *objectclassValue, char *groupattrName ));
|
||||
#endif
|
||||
|
||||
#ifdef SLAPD_PASSWD
|
||||
|
|
|
|||
|
|
@ -249,7 +249,9 @@ struct backend {
|
|||
void (*be_close) LDAP_P((Backend *be));
|
||||
|
||||
#ifdef SLAPD_ACLGROUPS
|
||||
int (*be_group) LDAP_P((Backend *be, char *bdn, char *edn, char *objectclassValue, char *groupattrName ));
|
||||
int (*be_group) LDAP_P((Backend *be, Entry *e,
|
||||
char *bdn, char *edn,
|
||||
char *objectclassValue, char *groupattrName ));
|
||||
#endif
|
||||
};
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue