[SSSD] [Patch] Improvements for the Kerberos locator plugin

Sumit Bose sbose at redhat.com
Wed Mar 10 08:37:43 UTC 2010


On Mon, Mar 08, 2010 at 03:53:03PM +0100, Martin Nagy wrote:
> On 03/08/2010 02:45 PM, Sumit Bose wrote:
> > On Mon, Mar 08, 2010 at 04:29:12AM +0100, Martin Nagy wrote:
> >> On 03/05/2010 08:19 PM, Sumit Bose wrote:
> >>> On Fri, Mar 05, 2010 at 12:00:41PM -0500, Dmitri Pal wrote:
> >>>> Sumit Bose wrote:
> >>>>> Hi,
> >>>>>
> >>>>> the following two patches are related to the Kerberos locator plugin.
> >>>>>
> >>>>> The first patch removes the name resolution from the locator plugin and
> >>>>> let it use the IP address found by the resolver code. This way we can be
> >>>>> sure to always talk to the same server until the resolver chooses a new
> >>>>> server.
> >>>>>
> >>>>> The second patch tries to make debugging of the locator plugin easier by
> >>>>> activating it automatically if the debug level of the Kerberos provider
> >>>>> is 5 or higher. Also the timestamp and debug-to-files flags are
> >>>>> respected.
> >>>>>
> >>>>> bye,
> >>>>> Sumit
> >>>>>
> >>>> I had a very quick glance.
> >>>> I think you are missing checks after strdup().
> >>>>
> >>>> ctx->kdc_addr = strdup((char *) buf);
> >>>>
> >>>>
> >>>> I am not familiar with the code so it is hard for me to comment on anything else.
> >>>>
> >>>
> >>> oops, new version attached.
> >>
> >> Hi, I can't apply the patch, can you please rebase?
> >>
> >> Thanks
> >> Martin
> >
> > sorry, this patch depends on 'Add expandable sequences to
> > krb5_ccachedir', but
> >
> > NACK to 0002, if at all debug-to-files needs to be handled differently.
> 
> ACK to 0001.
> 
> Martin

I just realised that I haven't split the fix for the missing strdup
check between the two patches. The new attached version of 0001 can be
applied to the current master without the withdrawn 0002. The only
change between the patches is:

-        PLUGIN_DEBUG(ctx, 5, "strdup failed.\n");
+        PLUGIN_DEBUG(("strdup failed.\n"));


bye,
Sumit
-------------- next part --------------
From afb12992a9a54298842ed7e99ccbe90d47bdc174 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Thu, 4 Mar 2010 12:44:21 +0100
Subject: [PATCH] Write the IP address of the KDC to the kdcinfo file

---
 src/krb5_plugin/sssd_krb5_locator_plugin.c |   81 ++++++++++++++-------------
 src/providers/ipa/ipa_common.c             |   14 ++++-
 src/providers/krb5/krb5_common.c           |   26 ++++------
 3 files changed, 63 insertions(+), 58 deletions(-)

diff --git a/src/krb5_plugin/sssd_krb5_locator_plugin.c b/src/krb5_plugin/sssd_krb5_locator_plugin.c
index 5e79733..8f32a31 100644
--- a/src/krb5_plugin/sssd_krb5_locator_plugin.c
+++ b/src/krb5_plugin/sssd_krb5_locator_plugin.c
@@ -45,7 +45,7 @@
 
 struct sssd_ctx {
     char *sssd_realm;
-    struct addrinfo *sssd_kdc_addrinfo;
+    char *kdc_addr;
     bool debug;
 };
 
@@ -122,14 +122,10 @@ static int get_kdcinfo(const char *realm, struct sssd_ctx *ctx)
     }
     PLUGIN_DEBUG(("Found kdcinfo [%s].\n", buf));
 
-    ret = getaddrinfo((char *) buf, "kerberos", NULL, &ctx->sssd_kdc_addrinfo);
-    if (ret != 0) {
-        PLUGIN_DEBUG(("getaddrinfo failed [%d][%s].\n", ret,
-                                                        gai_strerror(ret)));
-        if (ret == EAI_SYSTEM) {
-            PLUGIN_DEBUG(("getaddrinfo failed [%d][%s].\n", errno,
-                                                            strerror(errno)));
-        }
+    ctx->kdc_addr = strdup((char *) buf);
+    if (ctx->kdc_addr == NULL) {
+        PLUGIN_DEBUG(("strdup failed.\n"));
+        ret = ENOMEM;
         goto done;
     }
 
@@ -140,8 +136,6 @@ static int get_kdcinfo(const char *realm, struct sssd_ctx *ctx)
         goto done;
     }
 
-
-
 done:
     free(kdcinfo_name);
     return ret;
@@ -178,7 +172,7 @@ void sssd_krb5_locator_close(void *private_data)
     ctx = (struct sssd_ctx *) private_data;
     PLUGIN_DEBUG(("sssd_krb5_locator_close called\n"));
 
-    freeaddrinfo(ctx->sssd_kdc_addrinfo);
+    free(ctx->kdc_addr);
     free(ctx->sssd_realm);
     free(ctx);
     private_data = NULL;
@@ -197,14 +191,15 @@ krb5_error_code sssd_krb5_locator_lookup(void *private_data,
     int ret;
     struct addrinfo *ai;
     struct sssd_ctx *ctx;
-    char hostip[NI_MAXHOST];
+    struct addrinfo ai_hints;
+    const char *service = NULL;
 
     if (private_data == NULL) return KRB5_PLUGIN_NO_HANDLE;
     ctx = (struct sssd_ctx *) private_data;
 
     if (ctx->sssd_realm == NULL || strcmp(ctx->sssd_realm, realm) != 0) {
-        freeaddrinfo(ctx->sssd_kdc_addrinfo);
-        ctx->sssd_kdc_addrinfo = NULL;
+        free(ctx->kdc_addr);
+        ctx->kdc_addr = NULL;
         free(ctx->sssd_realm);
         ctx->sssd_realm = NULL;
         ret = get_kdcinfo(realm, ctx);
@@ -221,10 +216,15 @@ krb5_error_code sssd_krb5_locator_lookup(void *private_data,
     switch (svc) {
         case locate_service_kdc:
         case locate_service_master_kdc:
+            service = "kerberos";
+            break;
         case locate_service_kadmin:
+            service = "kerberos-adm";
             break;
-        case locate_service_krb524:
         case locate_service_kpasswd:
+            service = "kpasswd";
+            break;
+        case locate_service_krb524:
             return KRB5_PLUGIN_NO_HANDLE;
         default:
             return EINVAL;
@@ -250,32 +250,35 @@ krb5_error_code sssd_krb5_locator_lookup(void *private_data,
     if (strcmp(realm, ctx->sssd_realm) != 0)
         return KRB5_PLUGIN_NO_HANDLE;
 
-    for (ai = ctx->sssd_kdc_addrinfo; ai != NULL; ai = ai->ai_next) {
-        ret = getnameinfo(ai->ai_addr, ai->ai_addrlen, hostip, NI_MAXHOST,
-                          NULL, 0, NI_NUMERICHOST);
-        if (ret != 0) {
-            PLUGIN_DEBUG(("getnameinfo failed [%d][%s].\n", ret,
-                          gai_strerror(ret)));
-            if (ret == EAI_SYSTEM) {
-                PLUGIN_DEBUG(("getnameinfo failed [%d][%s].\n", errno,
-                              strerror(errno)));
-            }
+    memset(&ai_hints, 0, sizeof(struct addrinfo));
+    ai_hints.ai_flags = AI_NUMERICHOST;
+    ai_hints.ai_socktype = socktype;
+    ret = getaddrinfo(ctx->kdc_addr, service, &ai_hints, &ai);
+    if (ret != 0) {
+        PLUGIN_DEBUG(("getaddrinfo failed [%d][%s].\n", ret,
+                                                        gai_strerror(ret)));
+        if (ret == EAI_SYSTEM) {
+            PLUGIN_DEBUG(("getaddrinfo failed [%d][%s].\n", errno,
+                                                            strerror(errno)));
         }
-        PLUGIN_DEBUG(("addr[%s] family[%d] socktype[%d] - ", hostip,
-                      ai->ai_family, ai->ai_socktype));
-
-        if ((family == AF_UNSPEC || ai->ai_family == family) &&
-            ai->ai_socktype == socktype) {
-
-            ret = cbfunc(cbdata, socktype, ai->ai_addr);
-            if (ret != 0) {
-                PLUGIN_DEBUG(("\ncbfunc failed\n"));
-            } else {
-                PLUGIN_DEBUG(("used\n"));
-            }
+        return EFAULT;
+    }
+
+    PLUGIN_DEBUG(("addr[%s] family[%d] socktype[%d]\n", ctx->kdc_addr,
+                 ai->ai_family, ai->ai_socktype));
+
+    if ((family == AF_UNSPEC || ai->ai_family == family) &&
+        ai->ai_socktype == socktype) {
+
+        ret = cbfunc(cbdata, socktype, ai->ai_addr);
+        if (ret != 0) {
+            PLUGIN_DEBUG(("cbfunc failed\n"));
+            return ret;
         } else {
-            PLUGIN_DEBUG((" NOT used\n"));
+            PLUGIN_DEBUG(("[%s] used\n", ctx->kdc_addr));
         }
+    } else {
+        PLUGIN_DEBUG(("[%s] NOT used\n", ctx->kdc_addr));
     }
 
     return 0;
diff --git a/src/providers/ipa/ipa_common.c b/src/providers/ipa/ipa_common.c
index 92da9d1..a50b63b 100644
--- a/src/providers/ipa/ipa_common.c
+++ b/src/providers/ipa/ipa_common.c
@@ -24,6 +24,8 @@
 
 #include <netdb.h>
 #include <ctype.h>
+#include <arpa/inet.h>
+
 #include "providers/ipa/ipa_common.h"
 
 struct dp_option ipa_basic_opts[] = {
@@ -472,9 +474,15 @@ static void ipa_resolve_callback(void *private_data, struct fo_server *server)
         return;
     }
 
-    address = talloc_asprintf(service, "%s", srvaddr->h_name);
-    if (!address) {
-        DEBUG(1, ("Failed to copy address ...\n"));
+    address = talloc_zero_size(service, 128);
+    if (address == NULL) {
+        DEBUG(1, ("talloc_zero failed.\n"));
+        return;
+    }
+
+    if (inet_ntop(srvaddr->h_addrtype, srvaddr->h_addr_list[0],
+                  address, 128) == NULL) {
+        DEBUG(1, ("inet_ntop failed [%d][%s].\n", errno, strerror(errno)));
         return;
     }
 
diff --git a/src/providers/krb5/krb5_common.c b/src/providers/krb5/krb5_common.c
index 7619e6a..8c1c7fa 100644
--- a/src/providers/krb5/krb5_common.c
+++ b/src/providers/krb5/krb5_common.c
@@ -25,6 +25,7 @@
 #include <sys/stat.h>
 #include <unistd.h>
 #include <netdb.h>
+#include <arpa/inet.h>
 
 #include "providers/dp_backend.h"
 #include "providers/krb5/krb5_common.h"
@@ -47,7 +48,6 @@ errno_t check_and_export_options(struct dp_option *opts,
     char *value;
     const char *realm;
     const char *dummy;
-    char **list;
 
     realm = dp_opt_get_cstring(opts, KRB5_REALM);
     if (realm == NULL) {
@@ -68,18 +68,6 @@ errno_t check_and_export_options(struct dp_option *opts,
     dummy = dp_opt_get_cstring(opts, KRB5_KDC);
     if (dummy == NULL) {
         DEBUG(1, ("No KDC expicitly configured, using defaults"));
-    } else {
-        ret = split_on_separator(opts, dummy, ',', true, &list, NULL);
-        if (ret != EOK) {
-            DEBUG(1, ("Failed to parse server list!\n"));
-            return ret;
-        }
-        ret = write_kdcinfo_file(realm, list[0]);
-        if (ret != EOK) {
-            DEBUG(1, ("write_kdcinfo_file failed, "
-                      "using kerberos defaults from /etc/krb5.conf"));
-        }
-        talloc_free(list);
     }
 
     dummy = dp_opt_get_cstring(opts, KRB5_CCNAME_TMPL);
@@ -248,9 +236,15 @@ static void krb5_resolve_callback(void *private_data, struct fo_server *server)
         return;
     }
 
-    address = talloc_asprintf(krb5_service, "%s", srvaddr->h_name);
-    if (!address) {
-        DEBUG(1, ("Failed to copy address ...\n"));
+    address = talloc_zero_size(krb5_service, 128);
+    if (address == NULL) {
+        DEBUG(1, ("talloc_zero failed.\n"));
+        return;
+    }
+
+    if (inet_ntop(srvaddr->h_addrtype, srvaddr->h_addr_list[0],
+                  address, 128) == NULL) {
+        DEBUG(1, ("inet_ntop failed [%d][%s].\n", errno, strerror(errno)));
         return;
     }
 
-- 
1.6.6.1



More information about the sssd-devel mailing list