* Russ Allbery [2015-04-16 14:08:56 -0700]:
> Sergio Gelato <sergio.gel...@astro.su.se> writes:
>
> > When only pam_open_session() and pam_close_session() are called, the
> > child session gets its own PAG, aklog tries to populate it with tokens,
> > and unlog can only destroy tokens in the child session's PAG; the parent
> > session's tokens are left alone.
>
> > When pam_setcred(PAM_REINITIALIZE_CRED) is called, no new PAG is created
> > by pam_afs_session so the child session inherits the PAG of its parent.
> > Then either pam_close_session() or pam_setcred(PAM_DELETE_CRED) runs
> > unlog in that same PAG, causing the parent to lose tokens.
>
> Could you try this patch? I think it fixes the problem, but OpenAFS
> doesn't like the current kernel and I don't have a good test environment
> set up right at the moment.
Patch successfully tested in an i386 jessie VM with kernel 3.16.7-ckt9-2
and OpenAFS module 1.6.9-2+deb8u2 . I've tried both possible settings of
the pam_setcred option in sudo. No apparent regression with sshd either.
> diff --git a/internal.h b/internal.h
> index d3c4c04..f0b3e26 100644
> --- a/internal.h
> +++ b/internal.h
> @@ -17,6 +17,7 @@
> #endif
> #include <portable/pam.h>
> #include <portable/macros.h>
> +#include <portable/stdbool.h>
>
> #include <stdarg.h>
>
> @@ -59,8 +60,8 @@ struct pam_args *pamafs_init(pam_handle_t *, int flags, int
> argc,
> void pamafs_free(struct pam_args *);
>
> /* Token manipulation functions. */
> -int pamafs_token_get(struct pam_args *args);
> -int pamafs_token_delete(struct pam_args *args);
> +int pamafs_token_get(struct pam_args *, bool reinitialize);
> +int pamafs_token_delete(struct pam_args *);
>
> /* Undo default visibility change. */
> #pragma GCC visibility pop
> diff --git a/public.c b/public.c
> index 3554f32..35f6406 100644
> --- a/public.c
> +++ b/public.c
> @@ -73,7 +73,7 @@ pam_sm_open_session(pam_handle_t *pamh, int flags, int argc,
>
> /* Get tokens. */
> if (!args->config->notokens)
> - pamret = pamafs_token_get(args);
> + pamret = pamafs_token_get(args, false);
>
> /* Error codes are returned for pam_setcred. Map to pam_open_sesssion.
> */
> if (pamret != PAM_SUCCESS && pamret != PAM_IGNORE)
> @@ -117,6 +117,7 @@ pam_sm_setcred(pam_handle_t *pamh, int flags, int argc,
> int status;
> int pamret = PAM_SUCCESS;
> const void *dummy;
> + bool reinitialize;
>
> args = pamafs_init(pamh, flags, argc, argv);
> if (args == NULL) {
> @@ -163,7 +164,8 @@ pam_sm_setcred(pam_handle_t *pamh, int flags, int argc,
> * we're reinitializing, we may be running in a screen saver or the like
> * and should use the existing PAG, so don't create a new PAG.
> */
> - if (!(flags & (PAM_REINITIALIZE_CRED | PAM_REFRESH_CRED))) {
> + reinitialize = (flags & (PAM_REINITIALIZE_CRED | PAM_REFRESH_CRED));
> + if (!reinitialize) {
> status = pam_get_data(pamh, "pam_afs_session", &dummy);
> if (status == PAM_SUCCESS) {
> if (!k_haspag() && !args->config->nopag)
> @@ -180,7 +182,7 @@ pam_sm_setcred(pam_handle_t *pamh, int flags, int argc,
> }
> }
> if (!args->config->notokens)
> - pamret = pamafs_token_get(args);
> + pamret = pamafs_token_get(args, reinitialize);
>
> done:
> EXIT(args, pamret);
> diff --git a/tokens.c b/tokens.c
> index 8f2ced5..bde5d2d 100644
> --- a/tokens.c
> +++ b/tokens.c
> @@ -345,16 +345,25 @@ maybe_destroy_cache(struct pam_args *args UNUSED, const
> char *cache UNUSED)
>
> /*
> * Obtain AFS tokens. Does various sanity checks first, ensuring that we
> have
> - * a K5 ticket cache, that we can resolve the username, and that we're not
> - * supposed to ignore this user. Sets our flag data item if tokens were
> - * successfully obtained.
> + * a Kerberos ticket cache, that we can resolve the username, and that we're
> + * not supposed to ignore this user.
> + *
> + * Normally, set our flag data item if tokens were successfully obtained.
> + * This prevents a subsequent setcred or open_session from doing anything and
> + * flags close_session to remove the token. However, don't do this if the
> + * reinitialize flag is set, since in that case we're refreshing a token
> we're
> + * not subsequently responsible for. This fixes problems with sudo when it
> + * has pam_setcred enabled, since it calls pam_setcred with
> + * PAM_REINITIALIZE_CRED first before calling pam_open_session, and we don't
> + * want to skip the pam_open_session or PAG creation or remove the
> credentials
> + * created in pam_setcred outside of the new session.
> *
> * Returns error codes for pam_setcred, since those are the most granular. A
> * caller implementing pam_open_session needs to map these (generally by
> * mapping all failures to PAM_SESSION_ERR).
> */
> int
> -pamafs_token_get(struct pam_args *args)
> +pamafs_token_get(struct pam_args *args, bool reinitialize)
> {
> int status;
> PAM_CONST char *user;
> @@ -407,16 +416,16 @@ pamafs_token_get(struct pam_args *args)
> #else
> status = pamafs_run_aklog(args, pwd);
> #endif
> - if (status == PAM_SUCCESS) {
> + if (status == PAM_SUCCESS && !reinitialize) {
> status = pam_set_data(args->pamh, "pam_afs_session", (char *) "yes",
> NULL);
> if (status != PAM_SUCCESS) {
> putil_err_pam(args, status, "cannot set success data");
> status = PAM_CRED_ERR;
> }
> - if (status == PAM_SUCCESS)
> - maybe_destroy_cache(args, cache);
> }
> + if (status == PAM_SUCCESS)
> + maybe_destroy_cache(args, cache);
> return PAM_SUCCESS;
> }
>
> --
> Russ Allbery (r...@debian.org) <http://www.eyrie.org/~eagle/>
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
