[SSSD] [PATCH] Allow more libldap debugging

Jakub Hrozek jhrozek at redhat.com
Wed Nov 28 13:22:58 UTC 2012


On Wed, Nov 28, 2012 at 07:40:32AM -0500, Stephen Gallagher wrote:
> On Wed 28 Nov 2012 02:46:43 AM EST, Jakub Hrozek wrote:
> >On Tue, Nov 27, 2012 at 11:27:03PM -0500, Simo Sorce wrote:
> >>On Wed, 2011-08-10 at 15:29 +0200, Jan Zelený wrote:
> >>>>On Wed, Aug 10, 2011 at 10:14:14AM +0200, Jan Zelený wrote:
> >>>>>>This patch should not be pushed to master, but I would like to get it
> >>>>>>reviewed anyway.
> >>>>>>
> >>>>>>It should be used to provide a custom build for users experiencing
> >>>>>>cases where ldap_search_ext would block (c.f.
> >>>>>>https://bugzilla.redhat.com/show_bug.cgi?id=728343)
> >>>>>>
> >>>>>>For example:
> >>>>>>export SSSD_DEBUG_LDAP_SEARCH="0xffff"
> >>>>>>would set LDAP_DEBUG_ANY
> >>>>>>
> >>>>>>The attached patch applies cleanly on the RHEL6.1 branch. I also have a
> >>>>>>version that applies on master/1.5 if needed.
> >>>>>
> >>>>>I have three things:
> >>>>>On line 98 you close the original stderr - this is not a problem, but it
> >>>>>is redundant
> >>>>>
> >>>>>On line 162 you are using strtol and testing that it did not fail. Please
> >>>>>consider two possibilities that you haven't considered in your patch:
> >>>>>- the function did not fail, but non-zero value remains in errno from
> >>>>>previous program run
> >>>>>- the env. variable SSSD_DEBUG_LDAP_SEARCH contains empty string (I'm not
> >>>>>sure whether it is possible)
> >>>>>
> >>>>>Other than that the patch looks fine.
> >>>>>
> >>>>>Jan
> >>>>
> >>>>Thanks, a new patch is attached.
> >>>
> >>>Looks ok now,
> >>>Ack
> >>
> >>It looks like this patch, although acked was never pushed upstream ?
> >>Has it been dropped offline ?
> >>
> >>Simo.
> >>
> >>--
> >>Simo Sorce * Red Hat, Inc * New York
> >>
> >
> >See this sentence in my original mail:
> >
> >>This patch should not be pushed to master, but I would like to
> >>get it
> >>reviewed anyway.
> >>
> >>It should be used to provide a custom build for users
> >>experiencing
> >>cases where ldap_search_ext would block (c.f.
> >>https://bugzilla.redhat.com/show_bug.cgi?id=728343)
> >
> >So no, it was never pushed to master. It provides a *lot* of debugging
> >info and may easily flood the logs. We may want to reconsider the
> >submission, but I would do so only if we dedicate one log level to
> >low level tracing such as ldb transactions or libldb error output.
> 
> That is supposed to be the point of SSSDBG_TRACE_ALL. I realize
> we're not always consistent on that, though.

OK, attached is the original acked patch, just rebased on master.

Right now, the debug is controlled by a special environment variable. If
we were to use the patch upstream, I would propose just turning the
libldap debug based on an the debug_level option value. 
-------------- next part --------------
>From f422634931ee731e30343e910f4ebd28c4a0f229 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Wed, 28 Nov 2012 14:16:52 +0100
Subject: [PATCH] Enable libldap debugging

---
 src/providers/ldap/sdap_async.c            | 13 +++++
 src/providers/ldap/sdap_async_connection.c | 10 ++++
 src/util/debug.c                           | 52 +++++++++++++++++++
 src/util/sss_ldap.c                        | 82 ++++++++++++++++++++++++++++++
 src/util/sss_ldap.h                        |  3 ++
 src/util/util.h                            |  3 ++
 6 files changed, 163 insertions(+)

diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
index e0440625d279fbbaa1cc2e6343b73f5a247371f7..88fc6b936488ee4aa77d0b2523f1a8b7b66e7d60 100644
--- a/src/providers/ldap/sdap_async.c
+++ b/src/providers/ldap/sdap_async.c
@@ -1111,6 +1111,7 @@ struct sdap_get_generic_ext_state {
     void *cb_data;
 
     bool allow_paging;
+    int old_ldap_debug;
 };
 
 static errno_t sdap_get_generic_ext_step(struct tevent_req *req);
@@ -1274,6 +1275,13 @@ static errno_t sdap_get_generic_ext_step(struct tevent_req *req)
         state->serverctrls[state->nserverctrls+1] = NULL;
     }
 
+    ret = sss_ldap_set_debug(&state->old_ldap_debug);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_MINOR_FAILURE,
+               ("Could not set extra LDAP debugging\n"));
+        /* Not fatal, carry on */
+    }
+
     lret = ldap_search_ext(state->sh->ldap, state->search_base,
                            state->scope, state->filter,
                            discard_const(state->attrs),
@@ -1314,6 +1322,9 @@ static errno_t sdap_get_generic_ext_step(struct tevent_req *req)
     }
 
 done:
+    if (ret != EOK) {
+        sss_ldap_reset_debug(state->old_ldap_debug);
+    }
     return ret;
 }
 
@@ -1333,6 +1344,8 @@ static void sdap_get_generic_ext_done(struct sdap_op *op,
     LDAPControl **returned_controls = NULL;
     LDAPControl *page_control;
 
+    sss_ldap_reset_debug(state->old_ldap_debug);
+
     if (error) {
         tevent_req_error(req, error);
         return;
diff --git a/src/providers/ldap/sdap_async_connection.c b/src/providers/ldap/sdap_async_connection.c
index ff99248433026608f06fc548824fa2dd25aaacaa..02ce81f7dc126c97c5f3d98a072e2e22c16dd725 100644
--- a/src/providers/ldap/sdap_async_connection.c
+++ b/src/providers/ldap/sdap_async_connection.c
@@ -70,6 +70,7 @@ struct sdap_connect_state {
 
     struct sdap_msg *reply;
     int result;
+    int old_ldap_debug;
 };
 
 static void sdap_sys_connect_done(struct tevent_req *subreq);
@@ -129,6 +130,13 @@ struct tevent_req *sdap_connect_send(TALLOC_CTX *memctx,
         goto fail;
     }
 
+    ret = sss_ldap_set_debug(&state->old_ldap_debug);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_MINOR_FAILURE,
+               ("Could not set extra LDAP debugging\n"));
+        /* Not fatal, carry on */
+    }
+
     tevent_req_set_callback(subreq, sdap_sys_connect_done, req);
     return req;
 
@@ -425,6 +433,8 @@ int sdap_connect_recv(struct tevent_req *req,
     struct sdap_connect_state *state = tevent_req_data(req,
                                                   struct sdap_connect_state);
 
+    sss_ldap_reset_debug(state->old_ldap_debug);
+
     TEVENT_REQ_RETURN_ON_ERROR(req);
 
     *sh = talloc_steal(memctx, state->sh);
diff --git a/src/util/debug.c b/src/util/debug.c
index cb90d65ac788366cbb732c924f0e7bffd7676ab6..f1a8bbcd97ecf69d784c2da64d01cf0b3916eef3 100644
--- a/src/util/debug.c
+++ b/src/util/debug.c
@@ -261,3 +261,55 @@ void talloc_log_fn(const char *message)
 {
     DEBUG(SSSDBG_FATAL_FAILURE, (message));
 }
+
+int reopen_stderr_for_libldap(const char *filename)
+{
+    int ret;
+    char *logpath;
+    const char *log_file;
+    FILE *new_stderr;
+    TALLOC_CTX *tmp_ctx;
+
+    if (!debug_to_file) return EOK;
+
+    tmp_ctx = talloc_new(NULL);
+    if (!tmp_ctx) return ENOMEM;
+
+    log_file = filename ? filename : debug_log_file;
+
+    logpath = talloc_asprintf(tmp_ctx, "%s/%s.log", LOG_PATH, log_file);
+    if (logpath == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    fclose(stderr);
+    new_stderr = freopen(logpath, "a", stderr);
+    if (!new_stderr) {
+        ret = errno;
+        DEBUG(SSSDBG_OP_FAILURE, ("Couldn't reopen stderr to logfile\n"));
+        goto done;
+    }
+
+    ret = EOK;
+done:
+    talloc_free(tmp_ctx);
+    return ret;
+}
+
+int reopen_stderr_after_libldap(void)
+{
+    FILE *new_stderr;
+    int ret;
+
+    if (!debug_to_file) return EOK;
+
+    new_stderr = freopen("/dev/null", "a", stderr);
+    if (!new_stderr) {
+        ret = errno;
+        DEBUG(SSSDBG_OP_FAILURE, ("Couldn't reopen stderr to logfile\n"));
+        return ret;
+    }
+
+    return EOK;
+}
diff --git a/src/util/sss_ldap.c b/src/util/sss_ldap.c
index 060aacf9e55e79724a701fa391f72933416ef5d4..878ed4a632efd3cf93c9e5109619d6838c277fd9 100644
--- a/src/util/sss_ldap.c
+++ b/src/util/sss_ldap.c
@@ -590,3 +590,85 @@ done:
     talloc_free(filter);
     return ret;
 }
+
+int sss_ldap_set_debug(int *old_debug)
+{
+    const char *dbg;
+    static const char dbg_var[] = "SSSD_DEBUG_LDAP_SEARCH";
+    int ret;
+    int new_debug;
+    int ldap_old_debug;
+    char *endptr;
+
+    dbg = getenv(dbg_var);
+    if (!dbg){
+        DEBUG(SSSDBG_TRACE_LIBS, ("No extra LDAP debugging set\n"));
+        return EOK;
+    }
+
+    errno = 0;
+    new_debug = strtol(dbg, &endptr, 0);
+    if (errno != 0) {
+        ret = errno;
+        DEBUG(SSSDBG_OP_FAILURE,
+               ("strtol failed on [%s]: [%d][%s].\n", dbg, ret, strerror(ret)));
+        return ret;
+    }
+
+    if (*endptr != '\0') {
+        DEBUG(SSSDBG_OP_FAILURE,
+              ("Found additional characters [%s] in debug level [%s].\n",
+               endptr, dbg));
+        return EINVAL;
+    }
+
+    ret = reopen_stderr_for_libldap(NULL);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Could not redirect stderr to log file\n"));
+        return ret;
+    }
+
+    ret = ldap_get_option(NULL, LDAP_OPT_DEBUG_LEVEL,
+                          &ldap_old_debug);
+    if (ret != LDAP_OPT_SUCCESS) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Could not save old LDAP debug level\n"));
+        return EIO;
+    }
+
+    DEBUG(8, ("LDAP debug level was %#X setting to %#X\n",
+              ldap_old_debug, new_debug));
+
+    ret = ldap_set_option(NULL, LDAP_OPT_DEBUG_LEVEL,
+                          &new_debug);
+    if (ret != LDAP_OPT_SUCCESS) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Could not set LDAP debugging\n"));
+        return EIO;
+    }
+
+    if (old_debug) *old_debug = ldap_old_debug;
+
+    return EOK;
+}
+
+int sss_ldap_reset_debug(int ldap_old_debug)
+{
+    int ret;
+
+    ret = reopen_stderr_after_libldap();
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE,
+               ("Could not redirect stderr back to /dev/null\n"));
+        return ret;
+    }
+
+    ret = ldap_set_option(NULL, LDAP_OPT_DEBUG_LEVEL,
+                          &ldap_old_debug);
+    if (ret != LDAP_SUCCESS) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Could not reset LDAP debugging\n"));
+        return EIO;
+    }
+
+    DEBUG(SSSDBG_TRACE_LIBS,
+          ("Resetting LDAP debug level to %#X\n", ldap_old_debug));
+    return EOK;
+}
diff --git a/src/util/sss_ldap.h b/src/util/sss_ldap.h
index 46829259aedcf4a4f2ba3f94fc059c343c0e9ba6..de1cf6a4861eb2f1ac9597c152f520017804d9b0 100644
--- a/src/util/sss_ldap.h
+++ b/src/util/sss_ldap.h
@@ -76,4 +76,7 @@ bool sss_ldap_dn_in_search_bases(TALLOC_CTX *mem_ctx,
                                  struct sdap_search_base **search_bases,
                                  char **_filter);
 
+int sss_ldap_set_debug(int *old_debug);
+int sss_ldap_reset_debug(int ldap_old_debug);
+
 #endif /* __SSS_LDAP_H__ */
diff --git a/src/util/util.h b/src/util/util.h
index c15ca668392105447d073c40666953a0145d375a..b12ce5ad7a36cf1fdb3f557f38080e97efe0186a 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -67,6 +67,9 @@ int debug_get_level(int old_level);
 int debug_convert_old_level(int old_level);
 errno_t set_debug_file_from_fd(const int fd);
 
+int reopen_stderr_for_libldap(const char *filename);
+int reopen_stderr_after_libldap(void);
+
 #define SSSDBG_FATAL_FAILURE  0x0010   /* level 0 */
 #define SSSDBG_CRIT_FAILURE   0x0020   /* level 1 */
 #define SSSDBG_OP_FAILURE     0x0040   /* level 2 */
-- 
1.8.0



More information about the sssd-devel mailing list