On Tue, Oct 20, 2009 at 07:20:51PM -0400, Simo Sorce wrote:
> 
> On Tue, 2009-10-20 at 14:24 +0200, Sumit Bose wrote:
> > +    dummy = dp_opt_get_cstring(opts, KRB5_CHANGEPW_PRINC);
> > +    if (strchr(dummy, '@') == NULL) {
> > +        value = talloc_asprintf(opts, "%...@%s", dummy, realm);
> > +        if (value == NULL) {
> > +            DEBUG(7, ("talloc_asprintf failed.\n"));
> > +            return ENOMEM;
> > +        }
> 
> Same here too.
> Please always check what you get back.
> 
> Simo.
> 

ok, I've added checks where needed. New patches are attached.

bye,
Sumit
>From 0a2a6e9d52d1e90ff62876bcb479b0ad602ef931 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Tue, 20 Oct 2009 10:49:40 +0200
Subject: [PATCH 1/3] update krb5 option handling to new option scheme

---
 server/Makefile.am                  |   11 ++-
 server/providers/krb5/krb5_auth.c   |   62 +++++++++-----
 server/providers/krb5/krb5_auth.h   |    8 +--
 server/providers/krb5/krb5_child.c  |   41 +++++++++-
 server/providers/krb5/krb5_common.c |  155 +++++++++++++++++++++++++++++++++++
 server/providers/krb5/krb5_common.h |   20 +++++
 server/providers/krb5/krb5_init.c   |   86 +------------------
 server/providers/krb5/krb5_utils.c  |   26 ++++---
 server/tests/krb5_utils-tests.c     |   16 +++-
 9 files changed, 294 insertions(+), 131 deletions(-)
 create mode 100644 server/providers/krb5/krb5_common.c

diff --git a/server/Makefile.am b/server/Makefile.am
index 81f438d..480d57d 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -388,14 +388,16 @@ strtonum_tests_LDADD = \
     $(CHECK_LIBS)
 
 krb5_utils_tests_SOURCES = \
-    $(SSSD_DEBUG_OBJ) \
     tests/krb5_utils-tests.c \
-    providers/krb5/krb5_utils.c
+    providers/krb5/krb5_utils.c \
+    providers/krb5/krb5_common.c \
+    providers/data_provider_opts.c \
+    $(SSSD_UTIL_OBJ)
 krb5_utils_tests_CFLAGS = \
     $(CHECK_CFLAGS)
 krb5_utils_tests_LDADD = \
-    $(CHECK_LIBS) \
-    $(TALLOC_LIBS)
+    $(SSSD_LIBS)\
+    $(CHECK_LIBS)
 
 check_and_open_tests_SOURCES = \
     $(SSSD_DEBUG_OBJ) \
@@ -461,6 +463,7 @@ libsss_proxy_la_LDFLAGS = \
 libsss_krb5_la_SOURCES = \
     providers/krb5/krb5_utils.c \
     providers/krb5/krb5_auth.c \
+    providers/krb5/krb5_common.c \
     providers/krb5/krb5_init.c
 libsss_krb5_la_CFLAGS = \
     $(AM_CFLAGS) \
diff --git a/server/providers/krb5/krb5_auth.c 
b/server/providers/krb5/krb5_auth.c
index 1e689d9..1d699a8 100644
--- a/server/providers/krb5/krb5_auth.c
+++ b/server/providers/krb5/krb5_auth.c
@@ -210,7 +210,10 @@ static errno_t activate_child_timeout_handler(struct 
krb5child_req *kr)
     struct timeval tv;
 
     tv = tevent_timeval_current();
-    tv = tevent_timeval_add(&tv, kr->krb5_ctx->auth_timeout, 0);
+    tv = tevent_timeval_add(&tv,
+                            dp_opt_get_int(kr->krb5_ctx->opts,
+                                           KRB5_AUTH_TIMEOUT),
+                            0);
     kr->timeout_handler = tevent_add_timer(kr->req->be_ctx->ev, kr, tv,
                                            krb5_child_timeout, kr);
     if (kr->timeout_handler == NULL) {
@@ -278,7 +281,10 @@ static errno_t krb5_setup(struct be_req *req, struct 
krb5child_req **krb5_req,
     kr->krb5_ctx = krb5_ctx;
     kr->homedir = homedir;
 
-    kr->ccname = expand_ccname_template(kr, kr, krb5_ctx->ccname_template);
+    kr->ccname = expand_ccname_template(kr, kr,
+                                        dp_opt_get_cstring(krb5_ctx->opts,
+                                                           KRB5_CCNAME_TMPL)
+                                        );
     if (kr->ccname == NULL) {
         DEBUG(1, ("expand_ccname_template failed.\n"));
         err = EINVAL;
@@ -736,6 +742,7 @@ static void get_user_upn_done(void *pvt, int err, struct 
ldb_result *res)
     struct pam_data *pd;
     int pam_status=PAM_SYSTEM_ERR;
     const char *homedir = NULL;
+    const char *dummy;
 
     pd = talloc_get_type(be_req->req_data, struct pam_data);
     krb5_ctx = get_krb5_ctx(be_req);
@@ -760,9 +767,9 @@ static void get_user_upn_done(void *pvt, int err, struct 
ldb_result *res)
         pd->upn = ldb_msg_find_attr_as_string(res->msgs[0], SYSDB_UPN, NULL);
         if (pd->upn == NULL) {
             /* NOTE: this is a hack, works only in some environments */
-            if (krb5_ctx->realm != NULL) {
-                pd->upn = talloc_asprintf(be_req, "%...@%s", pd->user,
-                                      krb5_ctx->realm);
+            dummy = dp_opt_get_cstring(krb5_ctx->opts, KRB5_REALM);
+            if (dummy != NULL) {
+                pd->upn = talloc_asprintf(be_req, "%...@%s", pd->user, dummy);
                 if (pd->upn == NULL) {
                     DEBUG(1, ("failed to build simple upn.\n"));
                 }
@@ -829,6 +836,7 @@ static void krb5_pam_handler_done(struct tevent_req *req)
     char *password = NULL;
     char *env = NULL;
     int dp_err = DP_ERR_FATAL;
+    const char *dummy;
 
     pd->pam_status = PAM_SYSTEM_ERR;
     talloc_free(kr);
@@ -877,26 +885,34 @@ static void krb5_pam_handler_done(struct tevent_req *req)
     }
 
     if (*msg_status == PAM_SUCCESS && pd->cmd == SSS_PAM_AUTHENTICATE) {
-        env = talloc_asprintf(pd, "%s=%s", SSSD_KRB5_REALM, krb5_ctx->realm);
-        if (env == NULL) {
-            DEBUG(1, ("talloc_asprintf failed.\n"));
-            goto done;
-        }
-        ret = pam_add_response(pd, PAM_ENV_ITEM, strlen(env)+1, (uint8_t *) 
env);
-        if (ret != EOK) {
-            DEBUG(1, ("pam_add_response failed.\n"));
-            goto done;
+        dummy = dp_opt_get_cstring(krb5_ctx->opts, KRB5_REALM);
+        if (dummy != NULL) {
+            env = talloc_asprintf(pd, "%s=%s", SSSD_KRB5_REALM, dummy);
+            if (env == NULL) {
+                DEBUG(1, ("talloc_asprintf failed.\n"));
+                goto done;
+            }
+            ret = pam_add_response(pd, PAM_ENV_ITEM, strlen(env)+1,
+                                   (uint8_t *) env);
+            if (ret != EOK) {
+                DEBUG(1, ("pam_add_response failed.\n"));
+                goto done;
+            }
         }
 
-        env = talloc_asprintf(pd, "%s=%s", SSSD_KRB5_KDC, krb5_ctx->kdcip);
-        if (env == NULL) {
-            DEBUG(1, ("talloc_asprintf failed.\n"));
-            goto done;
-        }
-        ret = pam_add_response(pd, PAM_ENV_ITEM, strlen(env)+1, (uint8_t *) 
env);
-        if (ret != EOK) {
-            DEBUG(1, ("pam_add_response failed.\n"));
-            goto done;
+        dummy = dp_opt_get_cstring(krb5_ctx->opts, KRB5_KDC);
+        if (dummy != NULL) {
+            env = talloc_asprintf(pd, "%s=%s", SSSD_KRB5_KDC, dummy);
+            if (env == NULL) {
+                DEBUG(1, ("talloc_asprintf failed.\n"));
+                goto done;
+            }
+            ret = pam_add_response(pd, PAM_ENV_ITEM, strlen(env)+1,
+                                   (uint8_t *) env);
+            if (ret != EOK) {
+                DEBUG(1, ("pam_add_response failed.\n"));
+                goto done;
+            }
         }
     }
 
diff --git a/server/providers/krb5/krb5_auth.h 
b/server/providers/krb5/krb5_auth.h
index 9e9142c..95647e3 100644
--- a/server/providers/krb5/krb5_auth.h
+++ b/server/providers/krb5/krb5_auth.h
@@ -74,13 +74,7 @@ struct krb5_ctx {
     char* k4_cache_name;
 
     action_type action;
-
-    char *kdcip;
-    char *realm;
-    char *changepw_principle;
-    char *ccache_dir;
-    char *ccname_template;
-    int auth_timeout;
+    struct dp_option *opts;
     int child_debug_fd;
 };
 
diff --git a/server/providers/krb5/krb5_child.c 
b/server/providers/krb5/krb5_child.c
index 4e681b9..e67ff88 100644
--- a/server/providers/krb5/krb5_child.c
+++ b/server/providers/krb5/krb5_child.c
@@ -36,6 +36,41 @@
 
 #define IN_BUF_SIZE 512
 
+struct krb5_child_ctx {
+    /* opts taken from kinit */
+    /* in seconds */
+    krb5_deltat starttime;
+    krb5_deltat lifetime;
+    krb5_deltat rlife;
+
+    int forwardable;
+    int proxiable;
+    int addresses;
+
+    int not_forwardable;
+    int not_proxiable;
+    int no_addresses;
+
+    int verbose;
+
+    char* principal_name;
+    char* service_name;
+    char* keytab_name;
+    char* k5_cache_name;
+    char* k4_cache_name;
+
+    action_type action;
+
+    char *kdcip;
+    char *realm;
+    char *changepw_principle;
+    char *ccache_dir;
+    char *ccname_template;
+    int auth_timeout;
+
+    int child_debug_fd;
+};
+
 struct krb5_req {
     krb5_context ctx;
     krb5_ccache cc;
@@ -49,7 +84,7 @@ struct krb5_req {
 
     struct be_req *req;
     struct pam_data *pd;
-    struct krb5_ctx *krb5_ctx;
+    struct krb5_child_ctx *krb5_ctx;
     errno_t (*child_req)(int fd, struct krb5_req *kr);
 
     char *ccname;
@@ -522,7 +557,7 @@ static int krb5_cleanup(void *ptr)
         krb5_free_context(kr->ctx);
 
     if (kr->krb5_ctx != NULL) {
-        memset(kr->krb5_ctx, 0, sizeof(struct krb5_ctx));
+        memset(kr->krb5_ctx, 0, sizeof(struct krb5_child_ctx));
     }
     memset(kr, 0, sizeof(struct krb5_req));
 
@@ -543,7 +578,7 @@ static int krb5_setup(struct pam_data *pd, const char 
*user_princ_str,
     }
     talloc_set_destructor((TALLOC_CTX *) kr, krb5_cleanup);
 
-    kr->krb5_ctx = talloc_zero(kr, struct krb5_ctx);
+    kr->krb5_ctx = talloc_zero(kr, struct krb5_child_ctx);
     if (kr->krb5_ctx == NULL) {
         DEBUG(1, ("talloc failed.\n"));
         kerr = ENOMEM;
diff --git a/server/providers/krb5/krb5_common.c 
b/server/providers/krb5/krb5_common.c
new file mode 100644
index 0000000..a9d00ef
--- /dev/null
+++ b/server/providers/krb5/krb5_common.c
@@ -0,0 +1,155 @@
+/*
+    SSSD
+
+    Kerberos Provider Common Functions
+
+    Authors:
+        Sumit Bose <sb...@redhat.com>
+
+    Copyright (C) 2008-2009 Red Hat
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 3 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include "providers/dp_backend.h"
+#include "providers/krb5/krb5_common.h"
+
+struct dp_option default_krb5_opts[] = {
+    { "krb5_kdcip", DP_OPT_STRING, NULL_STRING, NULL_STRING },
+    { "krb5_realm", DP_OPT_STRING, NULL_STRING, NULL_STRING },
+    { "krb5_ccachedir", DP_OPT_STRING, { "/tmp" }, NULL_STRING },
+    { "krb5_ccname_tmpl", DP_OPT_STRING, { "FILE:%d/krb5cc_%U_XXXXXX" }, 
NULL_STRING},
+    { "krb5_changepw_principle", DP_OPT_STRING, { "kadmin/changepw" }, 
NULL_STRING },
+    { "krb5_auth_timeout", DP_OPT_NUMBER, { .number = 15 }, NULL_NUMBER },
+};
+
+errno_t check_and_export_options(struct dp_option *opts,
+                                 struct sss_domain_info *dom)
+{
+    int ret;
+    char *value;
+    const char *realm;
+    const char *dummy;
+    struct stat stat_buf;
+
+    dummy = dp_opt_get_cstring(opts, KRB5_KDC);
+    if (dummy == NULL) {
+        DEBUG(1, ("No KDC configured, "
+                  "using kerberos defaults from /etc/krb5.conf"));
+    } else {
+        ret = setenv(SSSD_KRB5_KDC, dummy, 1);
+        if (ret != EOK) {
+            DEBUG(2, ("setenv %s failed, authentication might fail.\n",
+                      SSSD_KRB5_KDC));
+        }
+    }
+
+    realm = dp_opt_get_cstring(opts, KRB5_REALM);
+    if (realm == NULL) {
+        ret = dp_opt_set_string(opts, KRB5_REALM, dom->name);
+        if (ret != EOK) {
+            DEBUG(1, ("dp_opt_set_string failed.\n"));
+            return ret;
+        }
+        realm = dom->name;
+    }
+    ret = setenv(SSSD_KRB5_REALM, realm, 1);
+    if (ret != EOK) {
+        DEBUG(2, ("setenv %s failed, authentication might fail.\n",
+                  SSSD_KRB5_REALM));
+    }
+
+    dummy = dp_opt_get_cstring(opts, KRB5_CCACHEDIR);
+    ret = lstat(dummy, &stat_buf);
+    if (ret != EOK) {
+        DEBUG(1, ("lstat for [%s] failed: [%d][%s].\n", dummy, errno,
+                  strerror(errno)));
+        return ret;
+    }
+    if ( !S_ISDIR(stat_buf.st_mode) ) {
+        DEBUG(1, ("Value of krb5ccache_dir [%s] is not a directory.\n", 
dummy));
+        return EINVAL;
+    }
+
+    dummy = dp_opt_get_cstring(opts, KRB5_CCNAME_TMPL);
+    if (dummy == NULL) {
+        DEBUG(1, ("Missing credential cache name template.\n"));
+        return EINVAL;
+    }
+    if (dummy[0] != '/' && strncmp(dummy, "FILE:", 5) != 0) {
+        DEBUG(1, ("Currently only file based credential caches are supported "
+                  "and krb5ccname_template must start with '/' or 'FILE:'\n"));
+        return EINVAL;
+    }
+
+    dummy = dp_opt_get_cstring(opts, KRB5_CHANGEPW_PRINC);
+    if (dummy == NULL) {
+        DEBUG(1, ("Missing change password principle.\n"));
+        return EINVAL;
+    }
+    if (strchr(dummy, '@') == NULL) {
+        value = talloc_asprintf(opts, "%...@%s", dummy, realm);
+        if (value == NULL) {
+            DEBUG(7, ("talloc_asprintf failed.\n"));
+            return ENOMEM;
+        }
+        ret = dp_opt_set_string(opts, KRB5_CHANGEPW_PRINC, value);
+        if (ret != EOK) {
+            DEBUG(1, ("dp_opt_set_string failed.\n"));
+            return ret;
+        }
+        dummy = value;
+    }
+
+    ret = setenv(SSSD_KRB5_CHANGEPW_PRINCIPLE, dummy, 1);
+    if (ret != EOK) {
+        DEBUG(2, ("setenv %s failed, password change might fail.\n",
+                  SSSD_KRB5_CHANGEPW_PRINCIPLE));
+    }
+
+    return EOK;
+}
+
+errno_t krb5_get_options(TALLOC_CTX *memctx, struct confdb_ctx *cdb,
+                         const char *conf_path, struct dp_option **_opts)
+{
+    int ret;
+    struct dp_option *opts;
+
+    opts = talloc_zero(memctx, struct dp_option);
+    if (opts == NULL) {
+        DEBUG(1, ("talloc_zero failed.\n"));
+        return ENOMEM;
+    }
+
+    ret = dp_get_options(opts, cdb, conf_path, default_krb5_opts,
+                         KRB5_OPTS, &opts);
+    if (ret != EOK) {
+        DEBUG(1, ("dp_get_options failed.\n"));
+        goto done;
+    }
+
+    *_opts = opts;
+    ret = EOK;
+
+done:
+    if (ret != EOK) {
+        talloc_zfree(opts);
+    }
+
+    return ret;
+}
diff --git a/server/providers/krb5/krb5_common.h 
b/server/providers/krb5/krb5_common.h
index 64e4360..5d784a5 100644
--- a/server/providers/krb5/krb5_common.h
+++ b/server/providers/krb5/krb5_common.h
@@ -34,7 +34,27 @@
 #include <krb5.h>
 #endif
 
+#include "providers/dp_backend.h"
+#include "util/util.h"
+
 #define SSSD_KRB5_KDC "SSSD_KRB5_KDC"
 #define SSSD_KRB5_REALM "SSSD_KRB5_REALM"
+#define SSSD_KRB5_CHANGEPW_PRINCIPLE "SSSD_KRB5_CHANGEPW_PRINCIPLE"
+
+enum krb5_opts {
+    KRB5_KDC = 0,
+    KRB5_REALM,
+    KRB5_CCACHEDIR,
+    KRB5_CCNAME_TMPL,
+    KRB5_CHANGEPW_PRINC,
+    KRB5_AUTH_TIMEOUT,
+
+    KRB5_OPTS
+};
+
+errno_t check_and_export_options(struct dp_option *opts,
+                                 struct sss_domain_info *dom);
 
+errno_t krb5_get_options(TALLOC_CTX *memctx, struct confdb_ctx *cdb,
+                         const char *conf_path, struct dp_option **_opts);
 #endif /* __KRB5_COMMON_H__ */
diff --git a/server/providers/krb5/krb5_init.c 
b/server/providers/krb5/krb5_init.c
index 676d078..26cf5e3 100644
--- a/server/providers/krb5/krb5_init.c
+++ b/server/providers/krb5/krb5_init.c
@@ -27,6 +27,7 @@
 #include <fcntl.h>
 #include <sys/stat.h>
 #include "providers/krb5/krb5_auth.h"
+#include "providers/krb5/krb5_common.h"
 
 struct bet_ops krb5_auth_ops = {
     .handler = krb5_pam_handler,
@@ -43,11 +44,8 @@ int sssm_krb5_auth_init(struct be_ctx *bectx,
                         void **pvt_auth_data)
 {
     struct krb5_ctx *ctx = NULL;
-    char *value = NULL;
-    int int_value;
     int ret;
     struct tevent_signal *sige;
-    struct stat stat_buf;
     unsigned v;
     FILE *debug_filep;
 
@@ -59,91 +57,17 @@ int sssm_krb5_auth_init(struct be_ctx *bectx,
 
     ctx->action = INIT_PW;
 
-    ret = confdb_get_string(bectx->cdb, ctx, bectx->conf_path,
-                            CONFDB_KRB5_KDCIP, NULL, &value);
-    if (ret != EOK) goto fail;
-    if (value == NULL) {
-        DEBUG(2, ("Missing krb5KDCIP, authentication might fail.\n"));
-    } else {
-        ret = setenv(SSSD_KRB5_KDC, value, 1);
-        if (ret != EOK) {
-            DEBUG(2, ("setenv %s failed, authentication might fail.\n",
-                      SSSD_KRB5_KDC));
-        }
-    }
-    ctx->kdcip = value;
-
-    ret = confdb_get_string(bectx->cdb, ctx, bectx->conf_path,
-                            CONFDB_KRB5_REALM, NULL, &value);
-    if (ret != EOK) goto fail;
-    if (value == NULL) {
-        DEBUG(4, ("Missing krb5REALM authentication might fail.\n"));
-    } else {
-        ret = setenv(SSSD_KRB5_REALM, value, 1);
-        if (ret != EOK) {
-            DEBUG(2, ("setenv %s failed, authentication might fail.\n",
-                      SSSD_KRB5_REALM));
-        }
-    }
-    ctx->realm = value;
-
-    ret = confdb_get_string(bectx->cdb, ctx, bectx->conf_path,
-                            CONFDB_KRB5_CCACHEDIR, "/tmp", &value);
-    if (ret != EOK) goto fail;
-    ret = lstat(value, &stat_buf);
+    ret = krb5_get_options(ctx, bectx->cdb, bectx->conf_path, &ctx->opts);
     if (ret != EOK) {
-        DEBUG(1, ("lstat for [%s] failed: [%d][%s].\n", value, errno,
-                  strerror(errno)));
-        goto fail;
-    }
-    if ( !S_ISDIR(stat_buf.st_mode) ) {
-        DEBUG(1, ("Value of krb5ccache_dir [%s] is not a directory.\n", 
value));
+        DEBUG(1, ("krb5_get_options failed.\n"));
         goto fail;
     }
-    ctx->ccache_dir = value;
-
-    ret = confdb_get_string(bectx->cdb, ctx, bectx->conf_path,
-                            CONFDB_KRB5_CCNAME_TMPL,
-                            "FILE:%d/krb5cc_%U_XXXXXX",
-                            &value);
-    if (ret != EOK) goto fail;
-    if (value[0] != '/' && strncmp(value, "FILE:", 5) != 0) {
-        DEBUG(1, ("Currently only file based credential caches are supported "
-                  "and krb5ccname_template must start with '/' or 'FILE:'\n"));
-        goto fail;
-    }
-    ctx->ccname_template = value;
-
-    ret = confdb_get_string(bectx->cdb, ctx, bectx->conf_path,
-                            CONFDB_KRB5_CHANGEPW_PRINC,
-                            "kadmin/changepw",
-                            &value);
-    if (ret != EOK) goto fail;
-    if (strchr(value, '@') == NULL) {
-        value = talloc_asprintf_append(value, "@%s", ctx->realm);
-        if (value == NULL) {
-            DEBUG(7, ("talloc_asprintf_append failed.\n"));
-            goto fail;
-        }
-    }
-    ctx->changepw_principle = value;
 
-    ret = setenv(SSSD_KRB5_CHANGEPW_PRINCIPLE, ctx->changepw_principle, 1);
+    ret = check_and_export_options(ctx->opts, bectx->domain);
     if (ret != EOK) {
-        DEBUG(2, ("setenv %s failed, password change might fail.\n",
-                  SSSD_KRB5_CHANGEPW_PRINCIPLE));
-    }
-
-    ret = confdb_get_int(bectx->cdb, ctx, bectx->conf_path,
-                         CONFDB_KRB5_AUTH_TIMEOUT, 15, &int_value);
-    if (ret != EOK) goto fail;
-    if (int_value <= 0) {
-        DEBUG(4, ("krb5auth_timeout has to be a positive value.\n"));
+        DEBUG(1, ("check_and_export_options failed.\n"));
         goto fail;
     }
-    ctx->auth_timeout = int_value;
-
-/* TODO: set options */
 
     sige = tevent_add_signal(bectx->ev, ctx, SIGCHLD, SA_SIGINFO,
                              krb5_child_sig_handler, NULL);
diff --git a/server/providers/krb5/krb5_utils.c 
b/server/providers/krb5/krb5_utils.c
index 68254ab..489030a 100644
--- a/server/providers/krb5/krb5_utils.c
+++ b/server/providers/krb5/krb5_utils.c
@@ -35,6 +35,12 @@ char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct 
krb5child_req *kr,
     char *p;
     char *n;
     char *result = NULL;
+    const char *dummy;
+
+    if (template == NULL) {
+        DEBUG(1, ("Missing template.\n"));
+        return NULL;
+    }
 
     copy = talloc_strdup(mem_ctx, template);
     if (copy == NULL) {
@@ -78,7 +84,7 @@ char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct 
krb5child_req *kr,
                 break;
             case 'p':
                 if (kr->pd->upn == NULL) {
-                    DEBUG(1, ("Cannot expand user principal name template "
+                    DEBUG(1, ("Cannot expand user principle name template "
                               "because upn is empty.\n"));
                     return NULL;
                 }
@@ -88,13 +94,12 @@ char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct 
krb5child_req *kr,
                 result = talloc_asprintf_append(result, "%s%%", p);
                 break;
             case 'r':
-                if (kr->krb5_ctx->realm == NULL) {
-                    DEBUG(1, ("Cannot expand realm template "
-                              "because value is not available.\n"));
+                dummy = dp_opt_get_string(kr->krb5_ctx->opts, KRB5_REALM);
+                if (dummy == NULL) {
+                    DEBUG(1, ("Missing kerberos realm.\n"));
                     return NULL;
                 }
-                result = talloc_asprintf_append(result, "%s%s", p,
-                                                kr->krb5_ctx->realm);
+                result = talloc_asprintf_append(result, "%s%s", p, dummy);
                 break;
             case 'h':
                 if (kr->homedir == NULL) {
@@ -105,13 +110,12 @@ char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct 
krb5child_req *kr,
                 result = talloc_asprintf_append(result, "%s%s", p, 
kr->homedir);
                 break;
             case 'd':
-                if (kr->krb5_ctx->ccache_dir == NULL) {
-                    DEBUG(1, ("Cannot expand ccache directory template "
-                              "because value is not available.\n"));
+                dummy = dp_opt_get_string(kr->krb5_ctx->opts, KRB5_CCACHEDIR);
+                if (dummy == NULL) {
+                    DEBUG(1, ("Missing credential cache directory.\n"));
                     return NULL;
                 }
-                result = talloc_asprintf_append(result, "%s%s", p,
-                                                kr->krb5_ctx->ccache_dir);
+                result = talloc_asprintf_append(result, "%s%s", p, dummy);
                 break;
             case 'P':
                 if (kr->pd->cli_pid == 0) {
diff --git a/server/tests/krb5_utils-tests.c b/server/tests/krb5_utils-tests.c
index 1ebebe6..aed1c3c 100644
--- a/server/tests/krb5_utils-tests.c
+++ b/server/tests/krb5_utils-tests.c
@@ -38,12 +38,15 @@
 #define CCACHE_DIR "/var/tmp"
 #define PID "4321"
 
+extern struct dp_option default_krb5_opts[];
+
 TALLOC_CTX *tmp_ctx = NULL;
 struct krb5child_req *kr;
 
 void setup_talloc_context(void)
 {
     int ret;
+    int i;
     struct pam_data *pd;
     struct krb5_ctx *krb5_ctx;
     fail_unless(tmp_ctx == NULL, "Talloc context already initialized.");
@@ -64,8 +67,17 @@ void setup_talloc_context(void)
     pd->upn = PRINCIPLE_NAME;
     pd->cli_pid = atoi(PID);
 
-    krb5_ctx->realm = REALM;
-    krb5_ctx->ccache_dir = CCACHE_DIR;
+    krb5_ctx->opts = talloc_zero_array(tmp_ctx, struct dp_option, KRB5_OPTS);
+    fail_unless(krb5_ctx->opts != NULL, "Cannot created options.");
+    for (i = 0; i < KRB5_OPTS; i++) {
+        krb5_ctx->opts[i].opt_name = default_krb5_opts[i].opt_name;
+        krb5_ctx->opts[i].type = default_krb5_opts[i].type;
+        krb5_ctx->opts[i].def_val = default_krb5_opts[i].def_val;
+    }
+    ret = dp_opt_set_string(krb5_ctx->opts, KRB5_REALM, REALM);
+    fail_unless(ret == EOK, "Failed to set Realm");
+    ret = dp_opt_set_string(krb5_ctx->opts, KRB5_CCACHEDIR, CCACHE_DIR);
+    fail_unless(ret == EOK, "Failed to set Ccache dir");
 
     kr->homedir = HOME_DIRECTORY;
 
-- 
1.6.2.5

>From aa05c255acce57902630c80b7c1fd4c5de86515a Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Tue, 20 Oct 2009 12:56:26 +0200
Subject: [PATCH 2/3] update ipa auth options to new option scheme

---
 server/Makefile.am                |    1 +
 server/providers/ipa/ipa_common.c |   79 +++++++++++++++++++
 server/providers/ipa/ipa_common.h |    7 ++
 server/providers/ipa/ipa_init.c   |  157 +++++++++++--------------------------
 4 files changed, 134 insertions(+), 110 deletions(-)

diff --git a/server/Makefile.am b/server/Makefile.am
index 480d57d..e115058 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -484,6 +484,7 @@ libsss_ipa_la_SOURCES = \
     providers/ldap/sdap.c \
     util/sss_ldap.c \
     providers/krb5/krb5_utils.c \
+    providers/krb5/krb5_common.c \
     providers/krb5/krb5_auth.c
 libsss_ipa_la_CFLAGS = \
     $(AM_CFLAGS) \
diff --git a/server/providers/ipa/ipa_common.c 
b/server/providers/ipa/ipa_common.c
index 799ac2f..6dba10c 100644
--- a/server/providers/ipa/ipa_common.c
+++ b/server/providers/ipa/ipa_common.c
@@ -104,6 +104,15 @@ struct sdap_id_map ipa_group_map[] = {
     { "ldap_group_modify_timestamp", "modifyTimestamp", SYSDB_ORIG_MODSTAMP, 
NULL }
 };
 
+struct dp_option ipa_def_krb5_opts[] = {
+    { "krb5_kdcip", DP_OPT_STRING, NULL_STRING, NULL_STRING },
+    { "krb5_realm", DP_OPT_STRING, NULL_STRING, NULL_STRING },
+    { "krb5_ccachedir", DP_OPT_STRING, { "/tmp" }, NULL_STRING },
+    { "krb5_ccname_tmpl", DP_OPT_STRING, { "FILE:%d/krb5cc_%U_XXXXXX" }, 
NULL_STRING},
+    { "krb5_changepw_princ", DP_OPT_STRING, { "kadmin/changepw" }, NULL_STRING 
},
+    { "krb5_auth_timeout", DP_OPT_NUMBER, { .number = 15 }, NULL_NUMBER },
+};
+
 int domain_to_basedn(TALLOC_CTX *memctx, const char *domain, char **basedn)
 {
     const char *s;
@@ -317,3 +326,73 @@ done:
     return ret;
 }
 
+/* the following preprocessor code is used to keep track of
+ * the options in the krb5 module, so that if they change and ipa
+ * is not updated correspondingly this will trigger a build error */
+#if KRB5_OPTS > 6
+#error There are krb5 options not accounted for
+#endif
+
+int ipa_get_auth_options(TALLOC_CTX *memctx,
+                         struct confdb_ctx *cdb,
+                         const char *conf_path,
+                         struct ipa_options *ipa_opts,
+                         struct dp_option **_opts)
+{
+    int ret;
+    int i;
+    TALLOC_CTX *tmpctx;
+    struct dp_option *opts;
+    char *value;
+
+    tmpctx = talloc_new(memctx);
+    if (!tmpctx) {
+        return ENOMEM;
+    }
+
+    opts = talloc_zero(memctx, struct dp_option);
+    if (opts == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    ret = dp_copy_options(ipa_opts, ipa_def_krb5_opts,
+                          KRB5_OPTS, &opts);
+    if (ret != EOK) {
+        goto done;
+    }
+
+    value = dp_opt_get_string(ipa_opts->basic, IPA_SERVER);
+    if (!value) {
+        ret = ENOMEM;
+        goto done;
+    }
+    ret = dp_opt_set_string(opts, KRB5_KDC, value);
+    if (ret != EOK) {
+        goto done;
+    }
+
+
+    value = dp_opt_get_string(ipa_opts->basic, IPA_DOMAIN);
+    if (!value) {
+        ret = ENOMEM;
+        goto done;
+    }
+    for (i = 0; value[i]; i++) {
+        value[i] = toupper(value[i]);
+    }
+    ret = dp_opt_set_string(opts, KRB5_REALM, value);
+    if (ret != EOK) {
+        goto done;
+    }
+
+    *_opts = opts;
+    ret = EOK;
+
+done:
+    talloc_zfree(tmpctx);
+    if (ret != EOK) {
+        talloc_zfree(opts);
+    }
+    return ret;
+}
diff --git a/server/providers/ipa/ipa_common.h 
b/server/providers/ipa/ipa_common.h
index 80f5294..f7d3ab8 100644
--- a/server/providers/ipa/ipa_common.h
+++ b/server/providers/ipa/ipa_common.h
@@ -25,6 +25,7 @@
 #include "util/util.h"
 #include "confdb/confdb.h"
 #include "providers/ldap/ldap_common.h"
+#include "providers/krb5/krb5_common.h"
 
 enum ipa_basic_opt {
     IPA_DOMAIN = 0,
@@ -58,4 +59,10 @@ int ipa_get_id_options(TALLOC_CTX *memctx,
                        struct ipa_options *ipa_opts,
                        struct sdap_options **_opts);
 
+int ipa_get_auth_options(TALLOC_CTX *memctx,
+                         struct confdb_ctx *cdb,
+                         const char *conf_path,
+                         struct ipa_options *ipa_opts,
+                         struct dp_option **_opts);
+
 #endif /* _IPA_COMMON_H_ */
diff --git a/server/providers/ipa/ipa_init.c b/server/providers/ipa/ipa_init.c
index 9cdcf4b..0c2eb2a 100644
--- a/server/providers/ipa/ipa_init.c
+++ b/server/providers/ipa/ipa_init.c
@@ -25,6 +25,8 @@
 #include <sys/types.h>
 #include <unistd.h>
 #include <sys/stat.h>
+#include <fcntl.h>
+
 #include "providers/ipa/ipa_common.h"
 #include "providers/krb5/krb5_auth.h"
 
@@ -100,141 +102,76 @@ done:
 
 int sssm_ipa_auth_init(struct be_ctx *bectx,
                        struct bet_ops **ops,
-                       void **pvt_auth_data)
+                       void **pvt_data)
 {
     struct krb5_ctx *ctx = NULL;
-    struct tevent_signal *sige;
-    struct stat stat_buf;
-    char *value = NULL;
-    int int_value;
     int ret;
+    struct tevent_signal *sige;
+    unsigned v;
+    FILE *debug_filep;
+
+    if (!ipa_options) {
+        ipa_get_options(bectx, bectx->cdb,
+                        bectx->conf_path,
+                        bectx->domain, &ipa_options);
+    }
+    if (!ipa_options) {
+        return ENOMEM;
+    }
 
     ctx = talloc_zero(bectx, struct krb5_ctx);
     if (!ctx) {
-        DEBUG(1, ("talloc failed.\n"));
         return ENOMEM;
     }
 
-    ctx->action = INIT_PW;
-
-    ret = confdb_get_string(bectx->cdb, ctx, bectx->conf_path,
-                            CONFDB_KRB5_KDCIP, NULL, &value);
+    ret = ipa_get_auth_options(ctx, bectx->cdb,
+                               bectx->conf_path,
+                               ipa_options, &ctx->opts);
     if (ret != EOK) {
-        goto fail;
+        goto done;
     }
 
-    if (value == NULL) {
-        DEBUG(2, ("Missing krb5KDCIP, authentication might fail.\n"));
-    } else {
-        ret = setenv(SSSD_KRB5_KDC, value, 1);
-        if (ret != EOK) {
-            DEBUG(2, ("setenv %s failed, authentication might fail.\n",
-                      SSSD_KRB5_KDC));
-        }
+    ret = check_and_export_options(ctx->opts, bectx->domain);
+    if (ret != EOK) {
+        DEBUG(1, ("check_and_export_opts failed.\n"));
+        goto done;
     }
-    ctx->kdcip = value;
 
-    ret = confdb_get_string(bectx->cdb, ctx, bectx->conf_path,
-                            CONFDB_KRB5_REALM, NULL, &value);
-    if (ret != EOK) {
-        goto fail;
+    sige = tevent_add_signal(bectx->ev, ctx, SIGCHLD, SA_SIGINFO,
+                             krb5_child_sig_handler, NULL);
+    if (sige == NULL) {
+        DEBUG(1, ("tevent_add_signal failed.\n"));
+        ret = ENOMEM;
+        goto done;
     }
 
-    if (value == NULL) {
-        DEBUG(4, ("Missing krb5REALM authentication might fail.\n"));
-    } else {
-        ret = setenv(SSSD_KRB5_REALM, value, 1);
+    if (debug_to_file != 0) {
+        ret = open_debug_file_ex("krb5_child", &debug_filep);
         if (ret != EOK) {
-            DEBUG(2, ("setenv %s failed, authentication might fail.\n",
-                      SSSD_KRB5_REALM));
+            DEBUG(0, ("Error setting up logging (%d) [%s]\n",
+                    ret, strerror(ret)));
+            goto done;
         }
-    }
-    ctx->realm = value;
 
-    ret = confdb_get_string(bectx->cdb, ctx, bectx->conf_path,
-                            CONFDB_KRB5_CCACHEDIR, "/tmp", &value);
-    if (ret != EOK) {
-        goto fail;
-    }
-
-    ret = lstat(value, &stat_buf);
-    if (ret != EOK) {
-        DEBUG(1, ("lstat for [%s] failed: [%d][%s].\n",
-                  value, errno, strerror(errno)));
-        goto fail;
-    }
-    if (!S_ISDIR(stat_buf.st_mode)) {
-        DEBUG(1, ("Value of krb5ccache_dir [%s] is not a directory.\n",
-                  value));
-        goto fail;
-    }
-    ctx->ccache_dir = value;
-
-    ret = confdb_get_string(bectx->cdb, ctx, bectx->conf_path,
-                            CONFDB_KRB5_CCNAME_TMPL,
-                            "FILE:%d/krb5cc_%U_XXXXXX",
-                            &value);
-    if (ret != EOK) {
-        goto fail;
-    }
-
-    if (value[0] != '/' && strncmp(value, "FILE:", 5) != 0) {
-        DEBUG(1, ("Currently only file based credential caches are supported "
-                  "and krb5ccname_template must start with '/' or 'FILE:'\n"));
-        goto fail;
-    }
-    ctx->ccname_template = value;
-
-    ret = confdb_get_string(bectx->cdb, ctx, bectx->conf_path,
-                            CONFDB_KRB5_CHANGEPW_PRINC,
-                            "kadmin/changepw",
-                            &value);
-    if (ret != EOK) {
-        goto fail;
-    }
-
-    if (strchr(value, '@') == NULL) {
-        value = talloc_asprintf_append(value, "@%s", ctx->realm);
-        if (value == NULL) {
-            DEBUG(7, ("talloc_asprintf_append failed.\n"));
-            goto fail;
+        ctx->child_debug_fd = fileno(debug_filep);
+        if (ctx->child_debug_fd == -1) {
+            DEBUG(0, ("fileno failed [%d][%s]\n", errno, strerror(errno)));
+            ret = errno;
+            goto done;
         }
-    }
-    ctx->changepw_principle = value;
-
-    ret = setenv(SSSD_KRB5_CHANGEPW_PRINCIPLE, ctx->changepw_principle, 1);
-    if (ret != EOK) {
-        DEBUG(2, ("setenv %s failed, password change might fail.\n",
-                  SSSD_KRB5_CHANGEPW_PRINCIPLE));
-    }
-
-    ret = confdb_get_int(bectx->cdb, ctx, bectx->conf_path,
-                         CONFDB_KRB5_AUTH_TIMEOUT, 15, &int_value);
-    if (ret != EOK) {
-        goto fail;
-    }
-    if (int_value <= 0) {
-        DEBUG(4, ("krb5auth_timeout has to be a positive value.\n"));
-        goto fail;
-    }
-    ctx->auth_timeout = int_value;
-
-/* TODO: set options */
 
-    sige = tevent_add_signal(bectx->ev, ctx, SIGCHLD, SA_SIGINFO,
-                             krb5_child_sig_handler, NULL);
-    if (sige == NULL) {
-        DEBUG(1, ("tevent_add_signal failed.\n"));
-        ret = ENOMEM;
-        goto fail;
+        v = fcntl(ctx->child_debug_fd, F_GETFD, 0);
+        fcntl(ctx->child_debug_fd, F_SETFD, v & ~FD_CLOEXEC);
     }
 
     *ops = &ipa_auth_ops;
-    *pvt_auth_data = ctx;
-    return EOK;
+    *pvt_data = ctx;
+    ret = EOK;
 
-fail:
-    talloc_free(ctx);
+done:
+    if (ret != EOK) {
+        talloc_free(ctx);
+    }
     return ret;
 }
 
-- 
1.6.2.5

>From 74a30ed29a11459f191f2149d8fbce4718b76cc5 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Tue, 20 Oct 2009 13:10:01 +0200
Subject: [PATCH 3/3] fix a compiler warning about redefinition of DEBUG

---
 server/krb5_plugin/sssd_krb5_locator_plugin.c |   34 ++++++++++++++-----------
 1 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/server/krb5_plugin/sssd_krb5_locator_plugin.c 
b/server/krb5_plugin/sssd_krb5_locator_plugin.c
index fc5f426..a30586c 100644
--- a/server/krb5_plugin/sssd_krb5_locator_plugin.c
+++ b/server/krb5_plugin/sssd_krb5_locator_plugin.c
@@ -35,7 +35,7 @@
 
 #define SSSD_KRB5_LOCATOR_DEBUG "SSSD_KRB5_LOCATOR_DEBUG"
 #define DEBUG_KEY "[sssd_krb5_locator] "
-#define DEBUG(body) do { \
+#define PLUGIN_DEBUG(body) do { \
     if (ctx->debug) { \
         debug_fn body; \
     } \
@@ -82,7 +82,7 @@ krb5_error_code sssd_krb5_locator_init(krb5_context context,
         ctx->debug = false;
     } else {
         ctx->debug = true;
-        DEBUG(("sssd_krb5_locator_init called\n"));
+        PLUGIN_DEBUG(("sssd_krb5_locator_init called\n"));
     }
 
     dummy = getenv(SSSD_KRB5_REALM);
@@ -95,9 +95,11 @@ krb5_error_code sssd_krb5_locator_init(krb5_context context,
 
     ret = getaddrinfo(dummy, "kerberos", NULL, &ctx->sssd_kdc_addrinfo);
     if (ret != 0) {
-        DEBUG(("getaddrinfo failed [%d][%s].\n", ret, gai_strerror(ret)));
+        PLUGIN_DEBUG(("getaddrinfo failed [%d][%s].\n", ret,
+                                                        gai_strerror(ret)));
         if (ret == EAI_SYSTEM) {
-            DEBUG(("getaddrinfo failed [%d][%s].\n", errno, strerror(errno)));
+            PLUGIN_DEBUG(("getaddrinfo failed [%d][%s].\n", errno,
+                                                            strerror(errno)));
         }
         goto failed;
     }
@@ -122,7 +124,7 @@ void sssd_krb5_locator_close(void *private_data)
     if (private_data == NULL) return;
 
     ctx = (struct sssd_ctx *) private_data;
-    DEBUG(("sssd_krb5_locator_close called\n"));
+    PLUGIN_DEBUG(("sssd_krb5_locator_close called\n"));
 
     freeaddrinfo(ctx->sssd_kdc_addrinfo);
     free(ctx->sssd_realm);
@@ -148,9 +150,9 @@ krb5_error_code sssd_krb5_locator_lookup(void *private_data,
     if (private_data == NULL) return KRB5_PLUGIN_NO_HANDLE;
     ctx = (struct sssd_ctx *) private_data;
 
-    DEBUG(("sssd_realm[%s] requested realm[%s] family[%d] socktype[%d] "
-          "locate_service[%d]\n", ctx->sssd_realm, realm, family, socktype,
-          svc));
+    PLUGIN_DEBUG(("sssd_realm[%s] requested realm[%s] family[%d] socktype[%d] "
+                  "locate_service[%d]\n", ctx->sssd_realm, realm, family,
+                                          socktype, svc));
 
     switch (svc) {
         case locate_service_kdc:
@@ -188,25 +190,27 @@ krb5_error_code sssd_krb5_locator_lookup(void 
*private_data,
         ret = getnameinfo(ai->ai_addr, ai->ai_addrlen, hostip, NI_MAXHOST,
                           NULL, 0, NI_NUMERICHOST);
         if (ret != 0) {
-            DEBUG(("getnameinfo failed [%d][%s].\n", ret, gai_strerror(ret)));
+            PLUGIN_DEBUG(("getnameinfo failed [%d][%s].\n", ret,
+                          gai_strerror(ret)));
             if (ret == EAI_SYSTEM) {
-                DEBUG(("getnameinfo failed [%d][%s].\n", errno, 
strerror(errno)));
+                PLUGIN_DEBUG(("getnameinfo failed [%d][%s].\n", errno,
+                              strerror(errno)));
             }
         }
-        DEBUG(("addr[%s] family[%d] socktype[%d] - ", hostip, ai->ai_family,
-                                                      ai->ai_socktype));
+        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) {
-                DEBUG(("\ncbfunc failed\n"));
+                PLUGIN_DEBUG(("\ncbfunc failed\n"));
             } else {
-                DEBUG(("used\n"));
+                PLUGIN_DEBUG(("used\n"));
             }
         } else {
-            DEBUG((" NOT used\n"));
+            PLUGIN_DEBUG((" NOT used\n"));
         }
     }
 
-- 
1.6.2.5

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to