[SSSD] [PATCH] Don't pass NULL to printf for TLS errors

Jakub Hrozek jhrozek at redhat.com
Wed Apr 6 05:39:05 UTC 2011


On Tue, Apr 05, 2011 at 12:53:16PM +0200, Jan Zelený wrote:
> Jakub Hrozek <jhrozek at redhat.com> wrote:
> > https://fedorahosted.org/sssd/ticket/643
> 
> Nack
> 
> In ldap_async_connection.c (on line 219) you missed one call of 
> ldap_get_option. Also add ldap_memfree at the end of 
> sss_ldap_get_diagnostic_msg(). Other than that the patch looks fine.
> 
> Jan

Thanks, new patch attached
-------------- next part --------------
>From 564f4aead0341975be15388b98e70bb0be4f0092 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 4 Mar 2011 22:17:55 +0100
Subject: [PATCH] Don't pass NULL to printf for TLS errors

https://fedorahosted.org/sssd/ticket/643
---
 src/providers/ldap/sdap.h                  |   10 -------
 src/providers/ldap/sdap_async.c            |    6 +---
 src/providers/ldap/sdap_async_connection.c |   41 +++++++++++++++-------------
 src/util/sss_ldap.c                        |   20 +++++++++++++-
 src/util/sss_ldap.h                        |   15 ++++++++++
 5 files changed, 58 insertions(+), 34 deletions(-)

diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
index 32dc344..01a00d9 100644
--- a/src/providers/ldap/sdap.h
+++ b/src/providers/ldap/sdap.h
@@ -26,16 +26,6 @@
 #include <ldap.h>
 #include "util/sss_ldap.h"
 
-#ifdef LDAP_OPT_DIAGNOSTIC_MESSAGE
-#define SDAP_DIAGNOSTIC_MESSAGE LDAP_OPT_DIAGNOSTIC_MESSAGE
-#else
-#ifdef LDAP_OPT_ERROR_STRING
-#define SDAP_DIAGNOSTIC_MESSAGE LDAP_OPT_ERROR_STRING
-#else
-#error No extended diagnostic message available
-#endif
-#endif
-
 struct sdap_msg {
     struct sdap_msg *next;
     LDAPMessage *msg;
diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
index ebd8d48..ed85a91 100644
--- a/src/providers/ldap/sdap_async.c
+++ b/src/providers/ldap/sdap_async.c
@@ -821,13 +821,11 @@ struct tevent_req *sdap_get_generic_send(TALLOC_CTX *memctx,
         DEBUG(3, ("ldap_search_ext failed: %s\n", ldap_err2string(lret)));
         if (lret == LDAP_SERVER_DOWN) {
             ret = ETIMEDOUT;
-            optret = ldap_get_option(state->sh->ldap,
-                                     SDAP_DIAGNOSTIC_MESSAGE,
-                                     (void*)&errmsg);
+            optret = sss_ldap_get_diagnostic_msg(state, state->sh->ldap,
+                                                 &errmsg);
             if (optret == LDAP_SUCCESS) {
                 DEBUG(3, ("Connection error: %s\n", errmsg));
                 sss_log(SSS_LOG_ERR, "LDAP connection error: %s", errmsg);
-                ldap_memfree(errmsg);
             }
             else {
                 sss_log(SSS_LOG_ERR, "LDAP connection error, %s",
diff --git a/src/providers/ldap/sdap_async_connection.c b/src/providers/ldap/sdap_async_connection.c
index d2eadfa..b6b0dd2 100644
--- a/src/providers/ldap/sdap_async_connection.c
+++ b/src/providers/ldap/sdap_async_connection.c
@@ -216,15 +216,13 @@ struct tevent_req *sdap_connect_send(TALLOC_CTX *memctx,
 
     lret = ldap_start_tls(state->sh->ldap, NULL, NULL, &msgid);
     if (lret != LDAP_SUCCESS) {
-        optret = ldap_get_option(state->sh->ldap,
-                                 SDAP_DIAGNOSTIC_MESSAGE,
-                                 (void*)&errmsg);
+        optret = sss_ldap_get_diagnostic_msg(state, state->sh->ldap,
+                                             &errmsg);
         if (optret == LDAP_SUCCESS) {
             DEBUG(3, ("ldap_start_tls failed: [%s] [%s]\n",
                       ldap_err2string(lret),
                       errmsg));
             sss_log(SSS_LOG_ERR, "Could not start TLS. %s", errmsg);
-            ldap_memfree(errmsg);
         }
         else {
             DEBUG(3, ("ldap_start_tls failed: [%s]\n",
@@ -303,15 +301,13 @@ static void sdap_connect_done(struct sdap_op *op,
     ret = ldap_install_tls(state->sh->ldap);
     if (ret != LDAP_SUCCESS) {
 
-        optret = ldap_get_option(state->sh->ldap,
-                                 SDAP_DIAGNOSTIC_MESSAGE,
-                                 (void*)&tlserr);
+        optret = sss_ldap_get_diagnostic_msg(state, state->sh->ldap,
+                                             &tlserr);
         if (optret == LDAP_SUCCESS) {
             DEBUG(3, ("ldap_install_tls failed: [%s] [%s]\n",
                       ldap_err2string(ret),
                       tlserr));
             sss_log(SSS_LOG_ERR, "Could not start TLS encryption. %s", tlserr);
-            ldap_memfree(tlserr);
         }
         else {
             DEBUG(3, ("ldap_install_tls failed: [%s]\n",
@@ -1480,30 +1476,34 @@ static int synchronous_tls_setup(LDAP *ldap)
     int msgid;
     char *errmsg = NULL;
     LDAPMessage *result;
+    TALLOC_CTX *tmp_ctx;
 
     DEBUG(4, ("Executing START TLS\n"));
 
+    tmp_ctx = talloc_new(NULL);
+    if (!tmp_ctx) return LDAP_NO_MEMORY;
+
     lret = ldap_start_tls(ldap, NULL, NULL, &msgid);
     if (lret != LDAP_SUCCESS) {
-        optret = ldap_get_option(ldap, SDAP_DIAGNOSTIC_MESSAGE, (void*)&errmsg);
+        optret = sss_ldap_get_diagnostic_msg(tmp_ctx, ldap, &errmsg);
         if (optret == LDAP_SUCCESS) {
             DEBUG(3, ("ldap_start_tls failed: [%s] [%s]\n",
                       ldap_err2string(lret), errmsg));
             sss_log(SSS_LOG_ERR, "Could not start TLS. %s", errmsg);
-            ldap_memfree(errmsg);
         } else {
             DEBUG(3, ("ldap_start_tls failed: [%s]\n", ldap_err2string(lret)));
             sss_log(SSS_LOG_ERR, "Could not start TLS. "
                                  "Check for certificate issues.");
         }
-        return lret;
+        goto done;
     }
 
     lret = ldap_result(ldap, msgid, 1, NULL, &result);
     if (lret != LDAP_RES_EXTENDED) {
         DEBUG(2, ("Unexpected ldap_result, expected [%d] got [%d].\n",
                   LDAP_RES_EXTENDED, lret));
-        return LDAP_PARAM_ERROR;
+        lret = LDAP_PARAM_ERROR;
+        goto done;
     }
 
     lret = ldap_parse_result(ldap, result, &ldaperr, NULL, &errmsg, NULL, NULL,
@@ -1511,7 +1511,7 @@ static int synchronous_tls_setup(LDAP *ldap)
     if (lret != LDAP_SUCCESS) {
         DEBUG(2, ("ldap_parse_result failed (%d) [%d][%s]\n", msgid, lret,
                   ldap_err2string(lret)));
-        return lret;
+        goto done;
     }
 
     DEBUG(3, ("START TLS result: %s(%d), %s\n",
@@ -1520,18 +1520,18 @@ static int synchronous_tls_setup(LDAP *ldap)
 
     if (ldap_tls_inplace(ldap)) {
         DEBUG(9, ("SSL/TLS handler already in place.\n"));
-        return LDAP_SUCCESS;
+        lret = LDAP_SUCCESS;
+        goto done;
     }
 
     lret = ldap_install_tls(ldap);
     if (lret != LDAP_SUCCESS) {
 
-        optret = ldap_get_option(ldap, SDAP_DIAGNOSTIC_MESSAGE, (void*)&errmsg);
+        optret = sss_ldap_get_diagnostic_msg(tmp_ctx, ldap, &errmsg);
         if (optret == LDAP_SUCCESS) {
             DEBUG(3, ("ldap_install_tls failed: [%s] [%s]\n",
                       ldap_err2string(lret), errmsg));
             sss_log(SSS_LOG_ERR, "Could not start TLS encryption. %s", errmsg);
-            ldap_memfree(errmsg);
         } else {
             DEBUG(3, ("ldap_install_tls failed: [%s]\n",
                       ldap_err2string(lret)));
@@ -1539,10 +1539,13 @@ static int synchronous_tls_setup(LDAP *ldap)
                                  "Check for certificate issues.");
         }
 
-        return lret;
+        goto done;
     }
 
-    return LDAP_SUCCESS;
+    lret = LDAP_SUCCESS;
+done:
+    talloc_zfree(tmp_ctx);
+    return lret;
 }
 
 static int sdap_rebind_proc(LDAP *ldap, LDAP_CONST char *url, ber_tag_t request,
@@ -1561,7 +1564,7 @@ static int sdap_rebind_proc(LDAP *ldap, LDAP_CONST char *url, ber_tag_t request,
 
     if (p->use_start_tls) {
         ret = synchronous_tls_setup(ldap);
-        if (ret != EOK) {
+        if (ret != LDAP_SUCCESS) {
             DEBUG(1, ("synchronous_tls_setup failed.\n"));
             return ret;
         }
diff --git a/src/util/sss_ldap.c b/src/util/sss_ldap.c
index f098e7d..a4d73a8 100644
--- a/src/util/sss_ldap.c
+++ b/src/util/sss_ldap.c
@@ -22,7 +22,25 @@
 #include "config.h"
 
 #include "util/sss_ldap.h"
+#include "util/util.h"
 
+int sss_ldap_get_diagnostic_msg(TALLOC_CTX *mem_ctx, LDAP *ld, char **_errmsg)
+{
+    char *errmsg = NULL;
+    int optret;
+
+    optret = ldap_get_option(ld, SDAP_DIAGNOSTIC_MESSAGE, (void*)&errmsg);
+    if (optret != LDAP_SUCCESS) {
+        return EINVAL;
+    }
+
+    *_errmsg = talloc_strdup(mem_ctx, errmsg ? errmsg : "unknown error");
+    ldap_memfree(errmsg);
+    if (*_errmsg == NULL) {
+        return ENOMEM;
+    }
+    return EOK;
+}
 
 int sss_ldap_control_create(const char *oid, int iscritical,
                             struct berval *value, int dupval,
diff --git a/src/util/sss_ldap.h b/src/util/sss_ldap.h
index 14747df..223f6ec 100644
--- a/src/util/sss_ldap.h
+++ b/src/util/sss_ldap.h
@@ -22,6 +22,21 @@
 #define __SSS_LDAP_H__
 
 #include <ldap.h>
+#include <talloc.h>
+
+#ifdef LDAP_OPT_DIAGNOSTIC_MESSAGE
+#define SDAP_DIAGNOSTIC_MESSAGE LDAP_OPT_DIAGNOSTIC_MESSAGE
+#else
+#ifdef LDAP_OPT_ERROR_STRING
+#define SDAP_DIAGNOSTIC_MESSAGE LDAP_OPT_ERROR_STRING
+#else
+#error No extended diagnostic message available
+#endif
+#endif
+
+int sss_ldap_get_diagnostic_msg(TALLOC_CTX *mem_ctx,
+                                LDAP *ld,
+                                char **_errmsg);
 
 int sss_ldap_control_create(const char *oid, int iscritical,
                             struct berval *value, int dupval,
-- 
1.7.4



More information about the sssd-devel mailing list