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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On Sep 22, 2015, at 7:24 AM, Jeff Layton <jlayton@xxxxxxxxxxxxxxx> wrote:
> 
> On Mon, 21 Sep 2015 16:50:09 -0400
> <andros@xxxxxxxxxx> wrote:
> 
>> From: Andy Adamson <andros@xxxxxxxxxx>
>> 
>> 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@xxxxxxxxxx>
>> ---
>> 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.

Yes - the code prior to gssd_atexit() called the sig_die for SIGTERM as well.
> 
> 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.

I tried that before submitting this patch - gssd_atexit is not called when systemctl stop rpc-gssd.service stops rpc.gssd. Nor is it called on SIGINT, SIGKILL, or SIGTERM. AFAICS, it is never called.
> 
> 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?

Yeah, it’s not a big problem - cred caches piling up in /tmp. Mostly it’s due to admin’s thinking that if the user of kerberos credentials is not running, then the kerberos credentials should not be present. The same reason kdestroy is used. Note that the /tmp/krb5ccmachine_REALM file survives reboot ….

I think I should simply use sig_die for SIGTERM and SIGKILL as well as SIGINT. Note that with the sig_die() approach, I do not need to check the pid.

WIth the addition of SIGTERM and SIGKILL, the machine credential file is ‘kdestroyed’ upon rpc.gssd exit.

—>Andy
> 
> -- 
> Jeff Layton <jlayton@xxxxxxxxxxxxxxx>

��.n��������+%������w��{.n�����{��w���jg��������ݢj����G�������j:+v���w�m������w�������h�����٥




[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux