[SSSD] [PATCH] Do not call nspr_nss_cleanup() in sss_password_decrypt()

Sumit Bose sbose at redhat.com
Thu Jan 6 12:22:47 UTC 2011


On Wed, Jan 05, 2011 at 08:31:57AM -0500, Stephen Gallagher wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 01/05/2011 06:12 AM, Sumit Bose wrote:
> > Hi,
> > 
> > Shanks found an issue with obfuscated password which led to
> > https://fedorahosted.org/sssd/ticket/762
> > 
> > This patch fixes the issue but might introduce some memory leaks. It
> > does not remove nspr_nss_cleanup() at any other places, because for me
> > the usage looks safe there. But we really should push
> > https://fedorahosted.org/sssd/ticket/752 to get a code audit for our NSS
> > usage.
> 
> 
> 
> Nack.
> 
> I think this solution is insufficient. We're not sure whether this will
> introduce a memory leak because we're not terminating our use of NSS. I
> think what we want to do is move the decryption to the initialization
> routine of the LDAP provider and store the decrypted password with the
> same mechanism we use to store deferred krb5 passwords.
> 
> This will also solve https://fedorahosted.org/sssd/ticket/761 at the
> same time.
> 
> There's no reason whatsoever that we need to be decrypting the
> obfuscated password every time we perform an authentication.
> 
> While we're in that code, it would probably be a good idea to make the
> non-obfuscated bind password use this mechanism as well.

This new patch converts the obfuscated password at startup and replaces
it with the cleartext version in the option array. This should fix
https://fedorahosted.org/sssd/ticket/761 and
https://fedorahosted.org/sssd/ticket/762

Would you mind filing a new ticket to create some common routines to
handle the Linux kernel keyring which can be used here and for deferred
authentication? (I have to admit that I think it doesn't make much sense
to use the kernel keyring here. Because I think the default LDAP
password is more a config option than a password which should kept
secret.)

bye,
Sumit

> 
> - -- 
> Stephen Gallagher
> RHCE 804006346421761
> 
> Delivering value year after year.
> Red Hat ranks #1 in value among software vendors.
> http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
> 
> iEYEARECAAYFAk0kcs0ACgkQeiVVYja6o6PsVgCeLk1PCJfCepRSFRgfQqZVNTM1
> a3MAnji3Gz4zhpBmaR/W92WoMcdqM3Jm
> =b67T
> -----END PGP SIGNATURE-----
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel
-------------- next part --------------
From bdf395e305bbac576ddf4fa79e1f02861388941c Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Thu, 6 Jan 2011 13:05:03 +0100
Subject: [PATCH] Convert obfuscated password once at startup

---
 src/providers/ldap/ldap_common.c           |   41 ++++++++++++++++++++++++++++
 src/providers/ldap/sdap_async_connection.c |   14 ---------
 2 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index f0db53f..c98dd4f 100644
--- a/src/providers/ldap/ldap_common.c
+++ b/src/providers/ldap/ldap_common.c
@@ -28,6 +28,7 @@
 #include "providers/krb5/krb5_common.h"
 
 #include "util/sss_krb5.h"
+#include "util/crypto/sss_crypto.h"
 
 /* a fd the child process would log into */
 int ldap_child_debug_fd = -1;
@@ -203,6 +204,9 @@ int ldap_get_options(TALLOC_CTX *memctx,
     const char *ldap_deref;
     int ldap_deref_val;
     int o;
+    const char *authtok_type;
+    struct dp_opt_blob authtok_blob;
+    char *cleartext;
     const int search_base_options[] = { SDAP_USER_SEARCH_BASE,
                                         SDAP_GROUP_SEARCH_BASE,
                                         SDAP_NETGROUP_SEARCH_BASE,
@@ -391,6 +395,43 @@ int ldap_get_options(TALLOC_CTX *memctx,
         goto done;
     }
 
+    authtok_type = dp_opt_get_string(opts->basic, SDAP_DEFAULT_AUTHTOK_TYPE);
+    if (authtok_type != NULL &&
+        strcasecmp(authtok_type,"obfuscated_password") == 0) {
+        DEBUG(9, ("Found obfuscated password, "
+                  "trying to convert to cleartext.\n"));
+
+        authtok_blob = dp_opt_get_blob(opts->basic, SDAP_DEFAULT_AUTHTOK);
+        if (authtok_blob.data == NULL || authtok_blob.length == 0) {
+            DEBUG(1, ("Missing obfuscated password string.\n"));
+            return EINVAL;
+        }
+
+        ret = sss_password_decrypt(memctx, (char *) authtok_blob.data,
+                                   &cleartext);
+        if (ret != EOK) {
+            DEBUG(1, ("Cannot convert the obfuscated "
+                      "password back to cleartext\n"));
+            return ret;
+        }
+
+        authtok_blob.data = (uint8_t *) cleartext;
+        authtok_blob.length = strlen(cleartext);
+        ret = dp_opt_set_blob(opts->basic, SDAP_DEFAULT_AUTHTOK, authtok_blob);
+        talloc_free(cleartext);
+        if (ret != EOK) {
+            DEBUG(1, ("dp_opt_set_string failed.\n"));
+            return ret;
+        }
+
+        ret = dp_opt_set_string(opts->basic, SDAP_DEFAULT_AUTHTOK_TYPE,
+                                "password");
+        if (ret != EOK) {
+            DEBUG(1, ("dp_opt_set_string failed.\n"));
+            return ret;
+        }
+    }
+
     ret = EOK;
     *_opts = opts;
 
diff --git a/src/providers/ldap/sdap_async_connection.c b/src/providers/ldap/sdap_async_connection.c
index 986a56c..ff8fb0d 100644
--- a/src/providers/ldap/sdap_async_connection.c
+++ b/src/providers/ldap/sdap_async_connection.c
@@ -24,7 +24,6 @@
 #include "util/util.h"
 #include "util/sss_krb5.h"
 #include "providers/ldap/sdap_async_private.h"
-#include "util/crypto/sss_crypto.h"
 
 #define LDAP_X_SSSD_PASSWORD_EXPIRED 0x555D
 
@@ -970,25 +969,12 @@ static int sdap_auth_get_authtok(TALLOC_CTX *mem_ctx,
                                  struct dp_opt_blob authtok,
                                  struct berval *pw)
 {
-    char *cleartext;
-    int ret;
-
     if (!authtok_type) return EOK;
     if (!pw) return EINVAL;
 
     if (strcasecmp(authtok_type,"password") == 0) {
         pw->bv_len = authtok.length;
         pw->bv_val = (char *) authtok.data;
-    } else if (strcasecmp(authtok_type,"obfuscated_password") == 0) {
-        ret = sss_password_decrypt(mem_ctx, (char *) authtok.data, &cleartext);
-        if (ret != EOK) {
-            DEBUG(1, ("Cannot convert the obfuscated "
-                      "password back to cleartext\n"));
-            return ret;
-        }
-
-        pw->bv_len = strlen(cleartext);
-        pw->bv_val = (char *) cleartext;
     } else {
         DEBUG(1, ("Authentication token type [%s] is not supported\n",
                   authtok_type));
-- 
1.7.3.3



More information about the sssd-devel mailing list