[SSSD] [PATCH] Generate ccache name at the beginning of auth process

Sumit Bose sbose at redhat.com
Mon Nov 22 09:18:00 UTC 2010


On Sat, Nov 20, 2010 at 12:25:02AM +0100, Jan Zeleny wrote:
> After some complications I finally made the patch solving ticket #533 (different 
> ccache files during multiple simultaneous logins of the same user). It is based 
> on Simo's idea to determine the ccache file name in advance and count 
> references to it.
> 
> Jan

I'm not sure if it is worth writing the reference counter and the new
file to the cache. Having them in a hash should be sufficient and
the reference counter does not have to be reseted at startup (which is
missing).

The reference counter is set in krb5_auth_send() but decremented in
krb5_child_done(). This should be moved to krb5_auth_done() before any
error checking to make sure that it is done under all circumstances.

bye,
Sumit

> From 4a7090fc49073ff671024ca6dbc01d654949670a Mon Sep 17 00:00:00 2001
> From: Jan Zeleny <jzeleny at redhat.com>
> Date: Sat, 20 Nov 2010 00:18:37 +0100
> Subject: [PATCH] Generate ccache name at the beginning of auth
> 
> In order to avoid situation when user logging to multiple
> terminals simultaneously gets different ccache files in each
> terminal (if random ccache files are used), the ccache file names
> are determined at the very beginning of authorization process.
> 
> Ticket: #533
> ---
>  src/db/sysdb.h                 |    2 +
>  src/providers/krb5/krb5_auth.c |  242 ++++++++++++++++++++++++++++++----------
>  src/providers/krb5/krb5_auth.h |    2 +
>  3 files changed, 189 insertions(+), 57 deletions(-)
> 
> diff --git a/src/db/sysdb.h b/src/db/sysdb.h
> index ed100b6..858c837 100644
> --- a/src/db/sysdb.h
> +++ b/src/db/sysdb.h
> @@ -88,6 +88,8 @@
>  #define SYSDB_UUID "uniqueID"
>  #define SYSDB_UPN "userPrincipalName"
>  #define SYSDB_CCACHE_FILE "ccacheFile"
> +#define SYSDB_CCACHE_NFILE "ccacheNFile"
> +#define SYSDB_CCACHE_NFILE_REF "ccacheNFileRef"
>  
>  #define SYSDB_ORIG_DN "originalDN"
>  #define SYSDB_ORIG_MODSTAMP "originalModifyTimestamp"
> diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
> index 9d5005a..eac595b 100644
> --- a/src/providers/krb5/krb5_auth.c
> +++ b/src/providers/krb5/krb5_auth.c
> @@ -24,6 +24,7 @@
>  
>  #include <errno.h>
>  #include <sys/time.h>
> +#include <unistd.h>
>  
>  #include <sys/types.h>
>  #include <sys/wait.h>
> @@ -147,13 +148,15 @@ static int krb5_save_ccname(TALLOC_CTX *mem_ctx,
>                              struct sysdb_ctx *sysdb,
>                              struct sss_domain_info *domain,
>                              const char *name,
> -                            const char *ccname)
> +                            const char *attr_name,
> +                            const char *ccname,
> +                            const int refcount)
>  {
>      TALLOC_CTX *tmpctx;
>      struct sysdb_attrs *attrs;
>      int ret;
>  
> -    if (name == NULL || ccname == NULL) {
> +    if (name == NULL || (ccname == NULL && strcmp(attr_name, SYSDB_CCACHE_FILE) == 0)) {
>          DEBUG(1, ("Missing user or ccache name.\n"));
>          return EINVAL;
>      }
> @@ -169,10 +172,24 @@ static int krb5_save_ccname(TALLOC_CTX *mem_ctx,
>          goto done;
>      }
>  
> -    ret = sysdb_attrs_add_string(attrs, SYSDB_CCACHE_FILE, ccname);
> -    if (ret != EOK) {
> -        DEBUG(1, ("sysdb_attrs_add_string failed.\n"));
> -        goto done;
> +    if (ccname) {
> +        ret = sysdb_attrs_add_string(attrs, attr_name, ccname);
> +        if (ret != EOK) {
> +            DEBUG(1, ("sysdb_attrs_add_string failed.\n"));
> +            goto done;
> +        }
> +    }
> +
> +    if (strcmp(attr_name, SYSDB_CCACHE_NFILE) == 0) {
> +        if (refcount >= 0) {
> +            ret = sysdb_attrs_add_long(attrs, SYSDB_CCACHE_NFILE_REF, refcount);
> +            if (ret != EOK) {
> +                DEBUG(1, ("sysdb_attrs_add_string failed.\n"));
> +                goto done;
> +            }
> +        } else {
> +            DEBUG(3, ("New ccache file ref. counter < 0. Skipping ...\n"));
> +        }
>      }
>  
>      ret = sysdb_transaction_start(sysdb);
> @@ -199,6 +216,67 @@ done:
>      return ret;
>  }
>  
> +/**
> + * Decrease reference counter of the potential new ccname
> + *
> + * When the counter is 0, no auth request for given user should be in progress
> + * and the new ccname candidate will be generated during the next auth request
> + */
> +static int krb5_ccname_decref(TALLOC_CTX *mem_ctx,
> +                              struct sysdb_ctx *sysdb,
> +                              struct sss_domain_info *domain,
> +                              const char *name)
> +{
> +    int ccache_file_new_ref = 0;
> +    struct ldb_result *res = NULL;
> +    const char **attrs = NULL;
> +    int ret = EOK;
> +
> +    attrs = talloc_array(mem_ctx, const char *, 2);
> +    if (attrs == NULL) {
> +        return ENOMEM;
> +    }
> +
> +    attrs[0] = SYSDB_CCACHE_NFILE_REF;
> +    attrs[1] = NULL;
> +
> +    ret = sysdb_get_user_attr(mem_ctx, sysdb, domain, name, attrs, &res);
> +    if (ret) {
> +        DEBUG(5, ("sysdb search for upn of user [%s] failed.\n", name));
> +        goto done;
> +    }
> +
> +    switch (res->count) {
> +    case 0:
> +        DEBUG(5, ("No attributes for user [%s] found.\n", name));
> +        ret = ENOENT;
> +        goto done;
> +
> +    case 1:
> +        ccache_file_new_ref = ldb_msg_find_attr_as_int(res->msgs[0],
> +                                                       SYSDB_CCACHE_NFILE_REF,
> +                                                       0);
> +        if (ccache_file_new_ref < 1) {
> +            DEBUG(3, ("Something strange happened - cannot decrease ccache refcount.\n"));
> +            ret = EINVAL;
> +            goto done;
> +        }
> +
> +        ret = krb5_save_ccname(mem_ctx, sysdb, domain, name, SYSDB_CCACHE_NFILE, NULL,
> +                               ccache_file_new_ref-1);
> +        break;
> +
> +    default:
> +        DEBUG(1, ("User search for (%s) returned > 1 results!\n", name));
> +        ret = EINVAL;
> +        goto done;
> +    }
> +
> +done:
> +    talloc_free(attrs);
> +    return ret;
> +}
> +
>  static struct krb5_ctx *get_krb5_ctx(struct be_req *be_req)
>  {
>      struct pam_data *pd;
> @@ -302,11 +380,17 @@ struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx,
>      struct ldb_result *res;
>      struct krb5child_req *kr = NULL;
>      const char *ccache_file = NULL;
> +    const char *ccache_file_new = NULL;
> +    int ccache_file_new_ref = 0;
> +    bool private_path = false;
> +    char *cc_file_name = NULL;
> +    size_t ccname_len = 0;
>      const char *realm;
>      krb5_error_code kerr;
>      struct tevent_req *req;
>      struct tevent_req *subreq;
>      int ret;
> +    int fd;
>  
>      req = tevent_req_create(mem_ctx, &state, struct krb5_auth_state);
>      if (req == NULL) {
> @@ -352,7 +436,7 @@ struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx,
>          goto done;
>      }
>  
> -    attrs = talloc_array(state, const char *, 6);
> +    attrs = talloc_array(state, const char *, 8);
>      if (attrs == NULL) {
>          ret = ENOMEM;
>          goto done;
> @@ -361,9 +445,11 @@ struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx,
>      attrs[0] = SYSDB_UPN;
>      attrs[1] = SYSDB_HOMEDIR;
>      attrs[2] = SYSDB_CCACHE_FILE;
> -    attrs[3] = SYSDB_UIDNUM;
> -    attrs[4] = SYSDB_GIDNUM;
> -    attrs[5] = NULL;
> +    attrs[3] = SYSDB_CCACHE_NFILE;
> +    attrs[4] = SYSDB_CCACHE_NFILE_REF;
> +    attrs[5] = SYSDB_UIDNUM;
> +    attrs[6] = SYSDB_GIDNUM;
> +    attrs[7] = NULL;
>  
>      ret = krb5_setup(state, pd, krb5_ctx, &state->kr);
>      if (ret != EOK) {
> @@ -425,6 +511,78 @@ struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx,
>              goto done;
>          }
>  
> +        ccache_file_new = ldb_msg_find_attr_as_string(res->msgs[0],
> +                                                      SYSDB_CCACHE_NFILE,
> +                                                      NULL);
> +        ccache_file_new_ref = ldb_msg_find_attr_as_int(res->msgs[0],
> +                                                       SYSDB_CCACHE_NFILE_REF,
> +                                                       0);
> +        DEBUG(8, ("New ccache file: %s (%d references)\n", ccache_file_new,
> +                                                           ccache_file_new_ref));
> +        if (!ccache_file_new || !ccache_file_new_ref) {
> +            cc_file_name = expand_ccname_template(kr, kr,
> +                                                  dp_opt_get_cstring(kr->krb5_ctx->opts,
> +                                                                     KRB5_CCNAME_TMPL),
> +                                                  true, &private_path);
> +            if (cc_file_name == NULL) {
> +                DEBUG(1, ("expand_ccname_template failed.\n"));
> +                ret = ENOMEM;
> +                goto done;
> +            }
> +
> +            ret = create_ccache_dir(kr, cc_file_name,
> +                                    kr->krb5_ctx->illegal_path_re,
> +                                    kr->uid, kr->gid, private_path);
> +            if (ret != EOK) {
> +                DEBUG(1, ("create_ccache_dir failed.\n"));
> +                goto done;
> +            }
> +
> +            /* A little hack here - assigning to const char *
> +             * even though we know that we will be modifiyng the content
> +             * a few lines below
> +             */
> +            ccache_file_new = cc_file_name;
> +            if (strncmp(cc_file_name, "FILE:", 5) == 0) {
> +                cc_file_name = cc_file_name + 5;
> +            }
> +
> +            if (cc_file_name[0] != '/') {
> +                DEBUG(1, ("Ccache filename is not an absolute path.\n"));
> +                ret = EINVAL;
> +                goto done;
> +            }
> +
> +            ccname_len = strlen(cc_file_name);
> +            if (ccname_len >= 6 && strcmp(cc_file_name + (ccname_len-6), "XXXXXX")==0 ) {
> +                fd = mkstemp(cc_file_name);
> +                if (fd == -1) {
> +                    DEBUG(1, ("mkstemp failed [%d][%s].\n", errno, strerror(errno)));
> +                    ret = errno;
> +                    goto done;
> +                }
> +                close(fd);
> +                unlink(cc_file_name);
> +            }
> +
> +            ccache_file_new_ref = 1;
> +        } else {
> +            ccache_file_new_ref++;
> +        }
> +
> +        kr->new_ccname = ccache_file_new;
> +        kr->new_ccname_ref = ccache_file_new_ref;
> +        DEBUG(8, ("Storing new ccache file: %s (%d references)\n",
> +                  ccache_file_new, ccache_file_new_ref));
> +        ret = krb5_save_ccname(kr, state->be_ctx->sysdb,
> +                               state->be_ctx->domain,
> +                               kr->pd->user, SYSDB_CCACHE_NFILE,
> +                               ccache_file_new, ccache_file_new_ref);
> +        if (ret) {
> +            DEBUG(1, ("krb5_save_ccname_send failed.\n"));
> +            goto done;
> +        }
> +
>          ccache_file = ldb_msg_find_attr_as_string(res->msgs[0],
>                                                    SYSDB_CCACHE_FILE,
>                                                    NULL);
> @@ -566,7 +724,6 @@ static void krb5_find_ccache_step(struct tevent_req *req)
>      struct krb5child_req *kr = state->kr;
>      struct pam_data *pd = kr->pd;
>      char *msg;
> -    bool private_path = false;
>      struct tevent_req *subreq = NULL;
>  
>      if (!kr->is_offline) {
> @@ -588,24 +745,8 @@ static void krb5_find_ccache_step(struct tevent_req *req)
>          (kr->is_offline && !kr->active_ccache_present &&
>              !kr->valid_tgt_present) ||
>          (!kr->is_offline && !kr->active_ccache_present)) {
> -            DEBUG(9, ("Recreating  ccache file.\n"));
> -            kr->ccname = expand_ccname_template(kr, kr,
> -                                          dp_opt_get_cstring(kr->krb5_ctx->opts,
> -                                                             KRB5_CCNAME_TMPL),
> -                                                true, &private_path);
> -            if (kr->ccname == NULL) {
> -                DEBUG(1, ("expand_ccname_template failed.\n"));
> -                ret = ENOMEM;
> -                goto done;
> -            }
> -
> -            ret = create_ccache_dir(kr, kr->ccname,
> -                                    kr->krb5_ctx->illegal_path_re,
> -                                    kr->uid, kr->gid, private_path);
> -            if (ret != EOK) {
> -                DEBUG(1, ("create_ccache_dir failed.\n"));
> -                goto done;
> -            }
> +            DEBUG(9, ("Using new ccache file.\n"));
> +            kr->ccname = kr->new_ccname;
>      }
>  
>      if (kr->is_offline) {
> @@ -615,6 +756,13 @@ static void krb5_find_ccache_step(struct tevent_req *req)
>              DEBUG(9, ("Valid TGT available or "
>                        "ccache file is already in use.\n"));
>              kr->ccname = kr->old_ccname;
> +            kr->new_ccname = NULL;
> +            kr->new_ccname_ref = 0;
> +            ret = krb5_save_ccname(kr, state->be_ctx->sysdb,
> +                                   state->be_ctx->domain,
> +                                   kr->pd->user, SYSDB_CCACHE_NFILE,
> +                                   kr->new_ccname, kr->new_ccname_ref);
> +
>              msg = talloc_asprintf(pd, "%s=%s", CCACHE_ENV_NAME, kr->ccname);
>              if (msg == NULL) {
>                  DEBUG(1, ("talloc_asprintf failed.\n"));
> @@ -683,7 +831,6 @@ static void krb5_child_done(struct tevent_req *subreq)
>      int ret;
>      uint8_t *buf = NULL;
>      ssize_t len = -1;
> -    ssize_t pref_len;
>      size_t p;
>      int32_t msg_status;
>      int32_t msg_type;
> @@ -735,22 +882,6 @@ static void krb5_child_done(struct tevent_req *subreq)
>              goto done;
>          }
>  
> -        /* We need to save the name of the credential cache file. To find it
> -         * we check if the data part of a message starts with
> -         * CCACHE_ENV_NAME"=". pref_len also counts the trailing '=' because
> -         * sizeof() counts the trailing '\0' of a string. */
> -        pref_len = sizeof(CCACHE_ENV_NAME);
> -        if (msg_len > pref_len &&
> -            strncmp((const char *) &buf[p], CCACHE_ENV_NAME"=", pref_len) == 0) {
> -            kr->ccname = talloc_strndup(kr, (char *) &buf[p+pref_len],
> -                                        msg_len-pref_len);
> -            if (kr->ccname == NULL) {
> -                DEBUG(1, ("talloc_strndup failed.\n"));
> -                ret = ENOMEM;
> -                goto done;
> -            }
> -        }
> -
>          ret = pam_add_response(pd, msg_type, msg_len, &buf[p]);
>          if (ret != EOK) {
>              /* This is not a fatal error */
> @@ -818,16 +949,7 @@ static void krb5_child_done(struct tevent_req *subreq)
>          fo_set_port_status(kr->srv, PORT_WORKING);
>      }
>  
> -    /* Now only a successful authentication or password change is left.
> -     *
> -     * We expect that one of the messages in the received buffer contains
> -     * the name of the credential cache file. */
> -    if (kr->ccname == NULL) {
> -        DEBUG(1, ("Missing ccache name in child response.\n"));
> -        ret = EINVAL;
> -        goto done;
> -    }
> -
> +    /* Now only a successful authentication or password change is left. */
>      if (kr->old_ccname != NULL) {
>          ret = safe_remove_old_ccache_file(kr->old_ccname, kr->ccname);
>          if (ret != EOK) {
> @@ -837,12 +959,18 @@ static void krb5_child_done(struct tevent_req *subreq)
>  
>      ret = krb5_save_ccname(state, state->be_ctx->sysdb,
>                             state->be_ctx->domain,
> -                           pd->user, kr->ccname);
> +                           pd->user, SYSDB_CCACHE_FILE, kr->ccname, 0);
>      if (ret) {
>          DEBUG(1, ("krb5_save_ccname_send failed.\n"));
>          goto done;
>      }
>  
> +    ret = krb5_ccname_decref(state, state->be_ctx->sysdb, state->be_ctx->domain,
> +                             pd->user);
> +    if (ret) {
> +        DEBUG(3, ("krb5_ccname_decref failed.\n"));
> +    }
> +
>      krb5_save_ccname_done(req);
>      return;
>  
> diff --git a/src/providers/krb5/krb5_auth.h b/src/providers/krb5/krb5_auth.h
> index 130e85d..ae984f8 100644
> --- a/src/providers/krb5/krb5_auth.h
> +++ b/src/providers/krb5/krb5_auth.h
> @@ -43,6 +43,8 @@ struct krb5child_req {
>  
>      const char *ccname;
>      const char *old_ccname;
> +    const char *new_ccname;
> +    int new_ccname_ref;
>      const char *homedir;
>      const char *upn;
>      uid_t uid;
> -- 
> 1.7.3.1
> 

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




More information about the sssd-devel mailing list