389-ds-base

Clone
Source Code
GIT

#50793 Ticket 50789: Add err-log for filter verification warning
Closed 3 years ago by spichugi. Opened 4 years ago by tbordaz.
tbordaz/389-ds-base ticket_50789  into  master

file modified
+29 -1 
@@ -35,7 +35,7 @@ 
 

  void
 

  do_search(Slapi_PBlock *pb)
 

  {
 

-     Slapi_Operation *operation;
 

+     Slapi_Operation *operation = NULL;
 

      BerElement *ber;
 

      int i, err = 0, attrsonly;
 

      ber_int_t scope, deref, sizelimit, timelimit;
 
@@ -220,6 +220,34 @@ 
 

          send_ldap_result(pb, err, NULL, errtxt, 0, NULL);
 

          goto free_and_return;
 

      }
 

+     if (r == FILTER_SCHEMA_WARNING) {
 

+         /* A notes=F will be logged in access log
 

+          * Anyway make it noisy with a log in error log
 

+          * as the behavior will change in upcoming release =>
 

+          * it needs to be fixed
 

+          */
 

+         if (config_get_verify_filter_schema() == FILTER_POLICY_WARNING) {
 

+             /* A component with unknown attribute was possibly processed
 

+              * with an unindexed scan
 

+              */
 

+             slapi_log_err(SLAPI_LOG_WARNING, "do_search",
 

+                     "Search filter \"%s\" contains unknown attribute. Possible performance impact (conn=%d op=%d).\n",
 

+                     fstr ? fstr : "NULL",
 

+                     operation ? operation->o_connid : "unknown",
 

+                     operation ? operation->o_opid : "unknown");
 

+         } else if (config_get_verify_filter_schema() == FILTER_POLICY_PROTECT) {
 

+             /* A component with unknown attribute was translated in
 

+              * a idl=0 (no entry matching). It protects the server against
 

+              * unindexed scan but the return result may ignore some
 

+              * matching entries
 

+              */
 

+             slapi_log_err(SLAPI_LOG_WARNING, "do_search",
 

+                     "Search filter \"%s\" contains unknown attribute. Possible invalid result set (conn=%d op=%d).\n",
 

+                     fstr ? fstr : "NULL",
 

+                     operation ? operation->o_connid : "unknown",
 

+                     operation ? operation->o_opid : "unknown");
 

+         }
 

+     }
 

  

      /* attributes */
 

      attrs = NULL;

Bug Description:
A filter component containing unknown attribute will (1.4.3)
match no entry. It can return a truncated set of matching entries.
This is notify in access logs (notes=F) but not in error logs.
To help admin to detect these problematic filters it need to be
log in error logs as well

Fix Description:
add a log when schema checking leads to note=F (FILTER_SCHEMA_WARNING)

https://pagure.io/389-ds-base/issue/50789

Reviewed by: ?

Platforms tested: F30

Flag Day: no

Doc impact: no

I think if we log an error it should be as precise as possible. your logging is in do_search where we only have the filter and do not know which attribute was rejected. Couldn't the logging occur in "slapi_filter_schema_check_inner()" ?

Thanks @lkrispen good point. The rework of the patch is not immediate and will need further tests.

Yeah, the check in schema_check_inner makes more sense, but we want to check that the schema transaction lock is a rwlock not a mutex, else the err log write could cause a delay.

Okay, I can confirm that the schema locks in question are from attr_syntax_read_lock() which calls slapi_rwlock_rdlock(). So this is fine.

The second concern is that the slapi_log_err does hit a single mutex, so this could cause delays in operations if there is a high number of invalid filters being issued to the server. I think perhaps that leaving this as notes=F, and the result message we send to the client is enough, we may not need an extra log as we already provide one in the access log .....

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/4317

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi
3 years ago
Metadata
None
No Tags
Changes Summary 1
+29 -1
file changed
ldap/servers/slapd/search.c
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.