ldap/servers/slapd/plugin.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
New commits:
commit 7f8c64fb4bb4b0de68272a3908ae4d07afdba49c
Author: William Brown <firstyear(a)redhat.com>
Date: Wed Jun 8 09:29:05 2016 +1000
Ticket 48870 - Correct plugin execution order due to changes in exop
Bug Description: Due to changes to make exop handling more efficient, an
issue was introduced where the execution of plugins was re-ordered. This meant
the precedence rules when two plugins are equal, were not the same.
Fix Description: Return early when we find the plugin, rather than continuing
to loop to find the "last match"
https://fedorahosted.org/389/ticket/48870
Author: wibrown
Review by: nhosoi, tbordaz (Thanks, and sorry for breaking it ...)
diff --git a/ldap/servers/slapd/plugin.c b/ldap/servers/slapd/plugin.c
index a2b2d75..440be98 100644
--- a/ldap/servers/slapd/plugin.c
+++ b/ldap/servers/slapd/plugin.c
@@ -504,7 +504,13 @@ plugin_determine_exop_plugins( const char *oid, struct slapdplugin **plugin)
if ( strcasecmp( oid, p->plg_exoids[i] ) == 0 ) {
*plugin = p;
rc = p->plg_type;
- break;
+ /* break; */
+ /* WB - 48870, return early so that we can get the plugin
+ * that is set earlier in the list. This prevents the
+ * need to change add_plugin_to_list, which may have
+ * side effects.
+ */
+ return rc;
}
}
}
ldap/servers/slapd/backend_manager.c | 1 +
ldap/servers/slapd/daemon.c | 2 +-
ldap/servers/slapd/plugin.c | 12 +++++++++---
ldap/servers/slapd/proto-slap.h | 1 +
ldap/servers/slapd/util.c | 33 ++++++---------------------------
5 files changed, 18 insertions(+), 31 deletions(-)
New commits:
commit 0f4f838b29df3e33bf0751c80baf2d4e052856ef
Author: William Brown <firstyear(a)redhat.com>
Date: Thu Jun 2 11:04:05 2016 +1000
Ticket 48863 - remove check for vmsize from util_info_sys_pages
Bug Description: https://bugzilla.redhat.com/show_bug.cgi?id=1341709 and
on another instance could not allocate cache because it would exceed memory size
In both cases I suspect that that vmsize was low, and this causes the
util_info_sys_pages check to return a low pages number, which triggers the
checking to fail
Fix Description: Remove the checks related to vmsize. It's either too low,
or just too high and wrong. Either way, we never rely on it. Lets use reliable
data.
https://fedorahosted.org/389/ticket/48863
Author: wibrown
Review by: nhosoi (Thanks!)
diff --git a/ldap/servers/slapd/util.c b/ldap/servers/slapd/util.c
index 89c7579..3f17461 100644
--- a/ldap/servers/slapd/util.c
+++ b/ldap/servers/slapd/util.c
@@ -1538,7 +1538,7 @@ int util_info_sys_pages(size_t *pagesize, size_t *pages, size_t *procpages, size
* This space is a "funny number". It's a best effort based system
* where linux instead of telling us how much memory *actually* exists
* for us to use, gives us a virtual memory allocation which is the
- * value of ram + swap.
+ * value of ram + swap.... sometimes. Depends on platform.
*
* But none of these pages even exist or belong to us on the real system
* until will malloc them AND write a non-zero to them.
@@ -1569,13 +1569,12 @@ int util_info_sys_pages(size_t *pagesize, size_t *pages, size_t *procpages, size
* potentially allocate: generally, this will be MemAvailable.
*/
- size_t vmsize = 0;
size_t freesize = 0;
size_t rlimsize = 0;
*pagesize = getpagesize();
- /* Get the amount of freeram, rss, and the vmsize */
+ /* Get the amount of freeram, rss */
FILE *f;
char fn[40], s[80];
@@ -1596,9 +1595,6 @@ int util_info_sys_pages(size_t *pagesize, size_t *pages, size_t *procpages, size
/* VmRSS shows us what we are ACTUALLY using for proc pages
* Rather than "funny" pages.
*/
- if (strncmp(s, "VmSize:", 7) == 0) {
- sscanf(s+7, "%lu", (long unsigned int *)&vmsize);
- }
if (strncmp(s, "VmRSS:", 6) == 0) {
sscanf(s+6, "%lu", (long unsigned int *)procpages);
}
@@ -1641,29 +1637,11 @@ int util_info_sys_pages(size_t *pagesize, size_t *pages, size_t *procpages, size
/* This is in bytes, make it pages */
rlimsize = rlimsize / *pagesize;
}
- /* Now we have vmsize, the availpages from getrlimit, our freesize */
- vmsize /= (*pagesize / 1024);
- /* Pages is the total ram on the system. We should smaller of:
- * - vmsize
- * - pages
- */
- LDAPDebug(LDAP_DEBUG_TRACE,"util_info_sys_pages pages=%lu, vmsize=%lu, \n",
- (unsigned long) *pages, (unsigned long) vmsize,0);
-#if __GNUC__
-#if __x86_64__ || __ppc64__
- /* On 64bit platforms, vmsize is set high (VmSize), and doesn't change. IE 17tb */
- if (vmsize < *pages) {
- LDAPDebug(LDAP_DEBUG_TRACE,"util_info_sys_pages using vmsize for pages \n",0,0,0);
- *pages = vmsize;
- } else {
- LDAPDebug(LDAP_DEBUG_TRACE,"util_info_sys_pages using pages for pages \n",0,0,0);
- }
-#else
- /* On 32bit platforms, vmsize is set low (VmSize) and grows. */
+ /* Pages is the total ram on the system. */
+ LDAPDebug(LDAP_DEBUG_TRACE,"util_info_sys_pages pages=%lu, \n",
+ (unsigned long) *pages, 0,0);
LDAPDebug(LDAP_DEBUG_TRACE,"util_info_sys_pages using pages for pages \n",0,0,0);
-#endif
-#endif
/* Availpages is how much we *could* alloc. We should take the smallest:
* - pages
@@ -1809,6 +1787,7 @@ int util_is_cachesize_sane(size_t *cachesize)
* the remaining system mem to the cachesize instead, and log a warning
*/
*cachesize = (size_t)((availpages * 0.75 ) * pagesize);
+ slapi_log_error(SLAPI_LOG_FATAL, "util_is_cachesize_sane", "Available pages %lu, requested pages %lu, pagesize %lu\n", (unsigned long)availpages, (unsigned long)cachepages, (unsigned long)pagesize);
slapi_log_error(SLAPI_LOG_FATAL, "util_is_cachesize_sane", "WARNING adjusted cachesize to %lu\n", (unsigned long)*cachesize);
}
#else
commit e7aabb539a8f05bc9c1c5e90c4e4a2cbfbc826cb
Author: William Brown <firstyear(a)redhat.com>
Date: Tue Jun 7 13:49:20 2016 +1000
Ticket 48872 - Fix segfault and use after free in plugin shutdown
Bug Description: Plugin_closeall would free the plugin password name scheme
value *even* if the plugin was not a password scheme.
backend manager would call be_cleanup on the plugin *after* plugin_closeall was
run. This means plugin_closeall now does not close backends, and we call
plugin_free from the backend to do this.
Fix Description: Check the union type to make sure we free the pwdscheme
only on password plugins.
Expose plugin_free, and call it from the backend manager. Don't close backends
during the call to plugin_closeall.
https://fedorahosted.org/389/ticket/48872
Author: wibrown
Review by: mreynolds (Thanks!)
diff --git a/ldap/servers/slapd/backend_manager.c b/ldap/servers/slapd/backend_manager.c
index 9084d95..979144a 100644
--- a/ldap/servers/slapd/backend_manager.c
+++ b/ldap/servers/slapd/backend_manager.c
@@ -323,6 +323,7 @@ be_cleanupall()
slapi_pblock_set( &pb, SLAPI_BACKEND, backends[i] );
(*backends[i]->be_cleanup)( &pb );
+ plugin_free(backends[i]->be_database);
slapi_be_free(&backends[i]);
}
}
diff --git a/ldap/servers/slapd/daemon.c b/ldap/servers/slapd/daemon.c
index 862f6e7..90d1f7d 100644
--- a/ldap/servers/slapd/daemon.c
+++ b/ldap/servers/slapd/daemon.c
@@ -1318,7 +1318,7 @@ void slapd_daemon( daemon_ports_t *ports )
uniqueIDGenCleanup ();
}
- plugin_closeall( 1 /* Close Backends */, 1 /* Close Globals */);
+ plugin_closeall( 0 /* Close Backends */, 1 /* Close Globals */);
if ( ! in_referral_mode ) {
/* Close SNMP collator after the plugins closed...
diff --git a/ldap/servers/slapd/plugin.c b/ldap/servers/slapd/plugin.c
index 39bfcef..a2b2d75 100644
--- a/ldap/servers/slapd/plugin.c
+++ b/ldap/servers/slapd/plugin.c
@@ -52,7 +52,6 @@ static PRBool plugin_matches_operation (Slapi_DN *target_spec, PluginTargetData
static void plugin_config_init (struct pluginconfig *config);
static void plugin_config_cleanup (struct pluginconfig *config);
-static void plugin_free(struct slapdplugin *plugin);
static int plugin_config_set_action (int *action, char *value);
static struct pluginconfig* plugin_get_config (struct slapdplugin *plugin);
static void default_plugin_init();
@@ -1951,6 +1950,10 @@ plugin_closeall(int close_backends, int close_globals)
iterp = dep_plugin_entries;
while (iterp) {
nextp = iterp->next;
+ if (close_backends == 0 && iterp->plugin->plg_type == SLAPI_PLUGIN_DATABASE) {
+ iterp = nextp;
+ continue;
+ }
slapi_entry_free(iterp->e);
plugin_free(iterp->plugin);
slapi_ch_free((void **)&iterp);
@@ -2703,15 +2706,18 @@ plugin_add_descriptive_attributes( Slapi_Entry *e, struct slapdplugin *plugin )
/*
clean up the memory associated with the plugin
*/
-static void
+void
plugin_free(struct slapdplugin *plugin)
{
+ slapi_log_error(SLAPI_LOG_TRACE, "plugin_free", "Freeing %s \n", plugin->plg_name );
charray_free(plugin->plg_argv);
slapi_ch_free_string(&plugin->plg_libpath);
slapi_ch_free_string(&plugin->plg_initfunc);
slapi_ch_free_string(&plugin->plg_name);
slapi_ch_free_string(&plugin->plg_dn);
- slapi_ch_free_string(&plugin->plg_pwdstorageschemename);
+ if (plugin->plg_type == SLAPI_PLUGIN_PWD_STORAGE_SCHEME) {
+ slapi_ch_free_string(&plugin->plg_pwdstorageschemename);
+ }
release_componentid(plugin->plg_identity);
slapi_counter_destroy(&plugin->plg_op_counter);
if (!plugin->plg_group)
diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h
index b8d7b86..1de1e38 100644
--- a/ldap/servers/slapd/proto-slap.h
+++ b/ldap/servers/slapd/proto-slap.h
@@ -879,6 +879,7 @@ void global_plugin_init();
int plugin_call_plugins( Slapi_PBlock *, int );
int plugin_setup(Slapi_Entry *plugin_entry, struct slapi_componentid *group,
slapi_plugin_init_fnptr initfunc, int add_to_dit, char *returntext);
+void plugin_free(struct slapdplugin *plugin);
int plugin_determine_exop_plugins( const char *oid, struct slapdplugin **plugin );
int plugin_call_exop_plugins( Slapi_PBlock *pb, struct slapdplugin *p );
Slapi_Backend * plugin_extended_op_getbackend( Slapi_PBlock *pb, struct slapdplugin *p);
ldap/servers/slapd/filterentry.c | 208 +++++++++++++++++++++++++--------------
1 file changed, 138 insertions(+), 70 deletions(-)
New commits:
commit 169d0ab5e8d2d61d39585c3a8981a3707578cae8
Author: Ludwig Krispenz <lkrispen(a)redhat.com>
Date: Tue Jun 7 15:38:54 2016 +0200
Ticket 48275 - search returns no entry when OR filter component contains non readable attribute
Bug Description: If an OR filter with more than one components did contain one
component without access the complete filter was set to non matching
even if the component did not contribute to the result of the evaluation
Fix Description: Handle filter components, so that it can be distinguished if
the component did not match aor if there was no access, components
without access are completely ignored in OR filters and evaluation is
only based on components with access
https://fedorahosted.org/389/ticket/48275
Reviewed by: Noriko, Thanks
diff --git a/ldap/servers/slapd/filterentry.c b/ldap/servers/slapd/filterentry.c
index ceeae79..9963c09 100644
--- a/ldap/servers/slapd/filterentry.c
+++ b/ldap/servers/slapd/filterentry.c
@@ -22,7 +22,8 @@
static int test_filter_list();
static int test_extensible_filter();
-static int vattr_test_filter_list();
+static int vattr_test_filter_list_and();
+static int vattr_test_filter_list_or();
static int test_filter_access( Slapi_PBlock *pb, Slapi_Entry*e,
char * attr_type, struct berval *attr_val);
static int slapi_vattr_filter_test_ext_internal( Slapi_PBlock *pb, Slapi_Entry *e,
@@ -762,9 +763,40 @@ slapi_vattr_filter_test(
/*
* vattr_filter_test_ext - full-feature filter test function
*
- * returns 0 filter matched
- * -1 filter did not match
- * >0 an ldap error code
+ * the filter test functions can be run in three different modes:
+ * - verify that the filter matches the entry
+ * - verify that the bound user has access to the attributes used in the filter
+ * - verify that the filter matches and the user has access to the attributes
+ *
+ * A special situation is the case of OR filters, eg
+ * (|(attr1=xxx)(attr2=yyy)(attr3=zzz))
+ * and the case to verify access and filter matching. An or filter is true if any
+ * of the filter components in the or filter is true. So if (attr2=yyy) matches
+ * and the user has access to attr2 the complete filter should match.
+ * But to prevent using a mixture of matching filter components and components with
+ * granted access to guess values of components without access filter evaluation needs
+ * to handle access and matching synchronously.
+ * It is also not sufficient to set a component without access to false because in cases
+ * where this component is part of a NOT filter it would be negated to true.
+ * Filter components withou access to the attribute need to be completely ignored.
+ *
+ * The implementation uses a three valued logic where the result of the evaluation of a
+ * filter component can be:
+ * - true: access to attribute and filter matches
+ * - false: access to attribute and filter does not match
+ * - undefined: no access to the attribute or any error during filter evaluatio
+ *
+ * The rules for complex filters are:
+ * (!(undefined)) --> undefined
+ * (|(undefined)(true)) --> true
+ * (|(undefined)(false)) --> false
+ * (&(undefined)(true)) --> undefined
+ * (&(undefined)(false)) --> false
+ *
+ * This is reflected in the return codes:
+ * 0 filter matched (true)
+ * -1 filter did not match (false)
+ * >0 undefined (or an ldap error code)
*/
int
slapi_vattr_filter_test_ext(
@@ -778,39 +810,11 @@ slapi_vattr_filter_test_ext(
int rc = 0; /* a no op request succeeds */
int access_check_done = 0;
- switch ( f->f_choice ) {
- case LDAP_FILTER_AND:
- case LDAP_FILTER_OR:
- case LDAP_FILTER_NOT:
- /*
- * optimize acl checking by only doing it once it is
- * known that the whole filter passes and so the entry
- * is eligible to be returned.
- * then we check the filter only for access
- *
- * complex filters really benefit from
- * separate stages, filter eval, followed by acl check...
- */
- if(!only_check_access)
- {
- rc = slapi_vattr_filter_test_ext_internal(pb,e,f,0,0, &access_check_done);
- }
-
- if(rc == 0 && verify_access)
- {
- rc = slapi_vattr_filter_test_ext_internal(pb,e,f,-1,-1, &access_check_done);
- }
-
- break;
-
- default:
- /*
- * ...but simple filters are better off doing eval and
- * acl check at once
- */
- rc = slapi_vattr_filter_test_ext_internal(pb,e,f,verify_access,only_check_access, &access_check_done);
- break;
- }
+ /* Fix for ticket 48275
+ * If we want to handle or components which can contain nonmatching components without access propoerly
+ * always filter verification and access check have to be done together for each component
+ */
+ rc = slapi_vattr_filter_test_ext_internal(pb,e,f,verify_access,only_check_access, &access_check_done);
return rc;
}
@@ -925,35 +929,53 @@ slapi_vattr_filter_test_ext_internal(
case LDAP_FILTER_AND:
LDAPDebug( LDAP_DEBUG_FILTER, " AND\n", 0, 0, 0 );
- rc = vattr_test_filter_list( pb, e, f->f_and,
+ rc = vattr_test_filter_list_and( pb, e, f->f_and,
LDAP_FILTER_AND , verify_access, only_check_access, access_check_done);
break;
case LDAP_FILTER_OR:
LDAPDebug( LDAP_DEBUG_FILTER, " OR\n", 0, 0, 0 );
- rc = vattr_test_filter_list( pb, e, f->f_or,
+ rc = vattr_test_filter_list_or( pb, e, f->f_or,
LDAP_FILTER_OR , verify_access, only_check_access, access_check_done);
break;
case LDAP_FILTER_NOT:
LDAPDebug( LDAP_DEBUG_FILTER, " NOT\n", 0, 0, 0 );
rc = slapi_vattr_filter_test_ext_internal( pb, e, f->f_not , verify_access, only_check_access, access_check_done);
- if(!(verify_access && only_check_access)) /* dont play with access control return codes */
+ if(verify_access && only_check_access)
{
- if(verify_access && !rc && !(*access_check_done))
- {
+ /* dont play with access control return codes
+ * do not negate return code */
+ break;
+ }
+ if (rc > 0) {
+ /* an error occurred or access denied, don't negate */
+ break;
+ }
+ if(verify_access)
+ {
+ int rc2;
+ if (!(*access_check_done)) {
/* the filter failed so access control was not checked
* for NOT filters this is significant so we must ensure
* access control is checked
*/
/* check access control only */
- rc = slapi_vattr_filter_test_ext_internal( pb, e, f->f_not , verify_access, -1 /*only_check_access*/, access_check_done);
+ rc2 = slapi_vattr_filter_test_ext_internal( pb, e, f->f_not , verify_access, -1 /*only_check_access*/, access_check_done);
/* preserve error code if any */
- if(!rc)
- rc = !rc;
+ if (rc2) {
+ rc = rc2;
+ } else {
+ rc = (rc==0)?-1:0;
+ }
+ } else {
+ rc = (rc==0)?-1:0;
}
- else
- rc = !rc;
+ }
+ else
+ {
+ /* filter verification only, no error */
+ rc = (rc==0)?-1:0;
}
break;
@@ -987,7 +1009,7 @@ static int test_filter_access( Slapi_PBlock *pb,
}
static int
-vattr_test_filter_list(
+vattr_test_filter_list_and(
Slapi_PBlock *pb,
Slapi_Entry *e,
struct slapi_filter *flist,
@@ -997,37 +1019,83 @@ vattr_test_filter_list(
int *access_check_done
)
{
- int nomatch;
+ int nomatch = -1;
+ int undefined = 0;
+ int rc = 0;
struct slapi_filter *f;
- LDAPDebug( LDAP_DEBUG_FILTER, "=> vattr_test_filter_list\n", 0, 0, 0 );
+ LDAPDebug( LDAP_DEBUG_FILTER, "=> vattr_test_filter_list_and\n", 0, 0, 0 );
- nomatch = 1;
for ( f = flist; f != NULL; f = f->f_next ) {
- if ( slapi_vattr_filter_test_ext_internal( pb, e, f, verify_access, only_check_access, access_check_done ) != 0 ) {
- /* optimize AND evaluation */
- if ( ftype == LDAP_FILTER_AND || verify_access) {
- /* one false is failure
- * for AND all components need to match
- * and for AND and OR access to ALL filter attributes is required
- */
- nomatch = 1;
- break;
- }
+ rc = slapi_vattr_filter_test_ext_internal( pb, e, f, verify_access, only_check_access, access_check_done );
+ if ( rc > 0 ) {
+ undefined = rc;
+ } else if (rc < 0) {
+ undefined = 0;
+ nomatch = -1;
+ break;
} else {
- nomatch = 0;
+ if (!verify_access || (*access_check_done)) {
+ nomatch = 0;
+ } else {
+ /* check access */
+ rc = slapi_vattr_filter_test_ext_internal( pb, e, f, verify_access, 1, access_check_done );
+ if (rc) undefined = rc;
+ }
+ }
+ }
- /* optimize OR evaluation too */
- if ( ftype == LDAP_FILTER_OR && !verify_access) {
- /* access to all atributes needs to be evaluated
- * for filter matching
- * only one needs to be true
- */
- break;
+ LDAPDebug( LDAP_DEBUG_FILTER, "<= test_filter_list_and %d\n", nomatch, 0, 0 );
+ if (undefined) return undefined;
+ return( nomatch );
+}
+
+static int
+vattr_test_filter_list_or(
+ Slapi_PBlock *pb,
+ Slapi_Entry *e,
+ struct slapi_filter *flist,
+ int ftype,
+ int verify_access,
+ int only_check_access,
+ int *access_check_done
+)
+{
+ int nomatch = 1;
+ int undefined = 0;
+ int rc = 0;
+ struct slapi_filter *f;
+
+ LDAPDebug( LDAP_DEBUG_FILTER, "=> vattr_test_filter_list_or\n", 0, 0, 0 );
+
+ for ( f = flist; f != NULL; f = f->f_next ) {
+ if ( verify_access) {
+ /* we do access check first */
+ rc = slapi_vattr_filter_test_ext_internal( pb, e, f, verify_access, -1, access_check_done );
+ if (rc != 0 ) {
+ /* no access to this component, ignore it */
+ undefined = rc;
+ continue;
}
}
+ if (only_check_access) continue;
+ /* now check if filter matches */
+ undefined = 0;
+ rc = slapi_vattr_filter_test_ext_internal( pb, e, f, 0, 0, access_check_done );
+ if ( rc == 0 ) {
+ undefined = 0;
+ nomatch = 0;
+ break;
+ } else if (rc > 0) {
+ undefined = rc;
+ } else {
+ /* filter didn't match, but we have one or component evaluated */
+ nomatch = -1;
+ }
}
- LDAPDebug( LDAP_DEBUG_FILTER, "<= test_filter_list %d\n", nomatch, 0, 0 );
+ LDAPDebug( LDAP_DEBUG_FILTER, "<= test_filter_list_or %d\n", nomatch, 0, 0 );
+
+ if (nomatch == 1) return undefined;
return( nomatch );
}