Re: [PATCH 4/4] GSSD: clean up machine credentials

[Jeff Layton <jlayton@poochiereds.net>]

On Mon, 21 Sep 2015 16:50:09 -0400
<andros@netapp.com> wrote:

> From: Andy Adamson <andros@netapp.com>
> 
> Since we no longer fork for uid 0, gssd_atexit() is only called when uid != 0,
> and fails as permissions on the /tmp/krb5ccmachine_REALM file prohibit
> the clean up of machine credentials (as it should).
> 
> Move the reaping of machine credentials back into a SIGINT sighandler so that
> <Ctrl C> destroyes machine credentials.
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  utils/gssd/gssd.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
> index 2a768ea..ebff860 100644
> --- a/utils/gssd/gssd.c
> +++ b/utils/gssd/gssd.c
> @@ -729,10 +729,12 @@ found:
>  }
>  
>  static void
> -gssd_atexit(void)
> +sig_die(int signal)
>  {
>  	if (root_uses_machine_creds)
>  		gssd_destroy_krb5_machine_creds();
> +	printerr(1, "exiting on signal %d\n", signal);
> +	exit(0);
>  }
>  
>  static void
> @@ -892,17 +894,13 @@ main(int argc, char *argv[])
>  		exit(EXIT_FAILURE);
>  	}
>  
> -	if (atexit(gssd_atexit)) {
> -		printerr(1, "ERROR: atexit failed: %s\n", strerror(errno));
> -		exit(EXIT_FAILURE);
> -	}
> -
>  	inotify_fd = inotify_init1(IN_NONBLOCK);
>  	if (inotify_fd == -1) {
>  		printerr(1, "ERROR: inotify_init1 failed: %s\n", strerror(errno));
>  		exit(EXIT_FAILURE);
>  	}
>  
> +	signal(SIGINT, sig_die);
>  	signal_set(&sighup_ev, SIGHUP, gssd_scan_cb, NULL);
>  	signal_add(&sighup_ev, NULL);
>  	event_set(&inotify_ev, inotify_fd, EV_READ | EV_PERSIST, gssd_inotify_cb, NULL);


Hmm, I don't know about this one. What if you die due to SIGTERM or
something (which is what systemd generally sends processes). They won't
get cleaned up in that case.

What may be better is to just keep the atexit handler but have it do a
getpid() and only do the cleanup if its the original pid of the daemon.
Just do a getpid early during gssd startup and store it in a global
variable somewhere.

That said, maybe I should take a step back and ask -- why does gssd
clean up this credcache in the first place? Is there some attack vector
that this prevents, or is it just to prevent a ton of credcaches piling
up in /tmp?

-- 
Jeff Layton <jlayton@poochiereds.net>