From: Andy Adamson <[email protected]>
Jeff Layton worked on this patch set with me.
patch 0001 and 0002 clean up process_krb5_upcall() by moving the two cases into
helper functions.
patch 0003 is the heart of this patch set.
commit f9cac65972da588d5218236de60a7be11247a8aa added the fork to
process_krb5_upcall so that the child assumes the uid of the principal
requesting service. This is good for the reasons listed in the commit.
When machine credentials are used, a gssd_k5_kt_princ entry is added to
a global list and used by future upcalls to note when valid machine credentials
have been obtained. When a child process performs this task, the entry to the
global list is lost upon exit, and all upcalls for machine credentials re-fetch
a TGT, even when a valid TGT is in the machine kerberos credential cache.
Since forking is not necessary when the principal has uid=0, solve the
gssd_k5_kt_princ_list issue by only forking when the uid != 0.
Please do more testing. Comments welcome.
Andy Adamson (4):
GSSD: move process_krb5_upcall machine cred case to helper function
GSSD: move process_krb5_updcall non machine cred case to helper
function
GSSD only fork when uid is not zeo
GSSD: clean up machine credentials
utils/gssd/gssd.c | 10 +-
utils/gssd/gssd_proc.c | 244 ++++++++++++++++++++++++++++++-------------------
2 files changed, 154 insertions(+), 100 deletions(-)
--
1.8.3.1
From: Andy Adamson <[email protected]>
Signed-off-by: Andy Adamson <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
utils/gssd/gssd_proc.c | 107 ++++++++++++++++++++++++++++---------------------
1 file changed, 62 insertions(+), 45 deletions(-)
diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 03afc8b..f5a9ce1 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -482,6 +482,64 @@ change_identity(uid_t uid)
return 0;
}
+AUTH *
+krb5_use_machine_creds(struct clnt_info *clp, uid_t uid, char *tgtname,
+ char *service, CLIENT **rpc_clnt)
+{
+ AUTH *auth = NULL;
+ char **credlist = NULL;
+ char **ccname;
+ int nocache = 0;
+ int success = 0;
+
+ do {
+ gssd_refresh_krb5_machine_credential(clp->servername, NULL,
+ service);
+ /*
+ * Get a list of credential cache names and try each
+ * of them until one works or we've tried them all
+ */
+ if (gssd_get_krb5_machine_cred_list(&credlist)) {
+ printerr(0, "ERROR: No credentials found "
+ "for connection to server %s\n",
+ clp->servername);
+ goto out;
+ }
+ for (ccname = credlist; ccname && *ccname; ccname++) {
+ gssd_setup_krb5_machine_gss_ccache(*ccname);
+ if ((create_auth_rpc_client(clp, tgtname, rpc_clnt,
+ &auth, uid,
+ AUTHTYPE_KRB5,
+ GSS_C_NO_CREDENTIAL)) == 0) {
+ /* Success! */
+ success++;
+ break;
+ }
+ printerr(2, "WARNING: Failed to create machine krb5"
+ "context with cred cache %s for server %s\n",
+ *ccname, clp->servername);
+ }
+ gssd_free_krb5_machine_cred_list(credlist);
+ if (!success) {
+ if(nocache == 0) {
+ nocache++;
+ printerr(2, "WARNING: Machine cache prematurely" "expired or corrupted trying to"
+ "recreate cache for server %s\n",
+ clp->servername);
+ } else {
+ printerr(1, "WARNING: Failed to create machine"
+ "krb5 context with any credentials"
+ "cache for server %s\n",
+ clp->servername);
+ goto out;
+ }
+ }
+ } while(!success);
+
+out:
+ return auth;
+}
+
/*
* this code uses the userland rpcsec gss library to create a krb5
* context on behalf of the kernel
@@ -494,8 +552,6 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
AUTH *auth = NULL;
struct authgss_private_data pd;
gss_buffer_desc token;
- char **credlist = NULL;
- char **ccname;
char **dirname;
int create_resp = -1;
int err, downcall_err = -EACCES;
@@ -587,49 +643,10 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
if (create_resp != 0) {
if (uid == 0 && (root_uses_machine_creds == 1 ||
service != NULL)) {
- int nocache = 0;
- int success = 0;
- do {
- gssd_refresh_krb5_machine_credential(clp->servername,
- NULL, service);
- /*
- * Get a list of credential cache names and try each
- * of them until one works or we've tried them all
- */
- if (gssd_get_krb5_machine_cred_list(&credlist)) {
- printerr(0, "ERROR: No credentials found "
- "for connection to server %s\n",
- clp->servername);
- goto out_return_error;
- }
- for (ccname = credlist; ccname && *ccname; ccname++) {
- gssd_setup_krb5_machine_gss_ccache(*ccname);
- if ((create_auth_rpc_client(clp, tgtname, &rpc_clnt,
- &auth, uid,
- AUTHTYPE_KRB5,
- GSS_C_NO_CREDENTIAL)) == 0) {
- /* Success! */
- success++;
- break;
- }
- printerr(2, "WARNING: Failed to create machine krb5 context "
- "with credentials cache %s for server %s\n",
- *ccname, clp->servername);
- }
- gssd_free_krb5_machine_cred_list(credlist);
- if (!success) {
- if(nocache == 0) {
- nocache++;
- printerr(2, "WARNING: Machine cache is prematurely expired or corrupted "
- "trying to recreate cache for server %s\n", clp->servername);
- } else {
- printerr(1, "WARNING: Failed to create machine krb5 context "
- "with any credentials cache for server %s\n",
- clp->servername);
- goto out_return_error;
- }
- }
- } while(!success);
+ auth = krb5_use_machine_creds(clp, uid, tgtname,
+ service, &rpc_clnt);
+ if (auth == NULL)
+ goto out_return_error;
} else {
printerr(1, "WARNING: Failed to create krb5 context "
"for user with uid %d for server %s\n",
--
1.9.3 (Apple Git-50)
From: Andy Adamson <[email protected]>
commit f9cac65972da588d5218236de60a7be11247a8aa added the fork to
process_krb5_upcall so that the child assumes the uid of the principal
requesting service.
When machine credentials are used, a gssd_k5_kt_princ entry is added to
a global list and used by future upcalls to note when valid machine credentials
have been obtained. When a child process performs this task, the entry to the
global list is lost upon exit, and all upcalls for machine credentials re-fetch
a TGT, even when a valid TGT is in the machine kerberos credential cache.
Since forking is not necessary when the principal has uid=0, solve the
gssd_k5_kt_princ_list issue by only forking when the uid != 0.
Signed-off-by: Andy Adamson <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
utils/gssd/gssd_proc.c | 67 ++++++++++++++++++++++++++++++++------------------
1 file changed, 43 insertions(+), 24 deletions(-)
diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 0e04570..7cf8f0c 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -597,33 +597,11 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
gss_buffer_desc token;
int err, downcall_err = -EACCES;
OM_uint32 maj_stat, min_stat, lifetime_rec;
- pid_t pid;
+ pid_t pid, childpid = -1;
gss_name_t gacceptor = GSS_C_NO_NAME;
gss_OID mech;
gss_buffer_desc acceptor = {0};
- pid = fork();
- switch(pid) {
- case 0:
- /* Child: fall through to rest of function */
- break;
- case -1:
- /* fork() failed! */
- printerr(0, "WARNING: unable to fork() to handle upcall: %s\n",
- strerror(errno));
- return;
- default:
- /* Parent: just wait on child to exit and return */
- do {
- pid = wait(&err);
- } while(pid == -1 && errno != -ECHILD);
-
- if (WIFSIGNALED(err))
- printerr(0, "WARNING: forked child was killed with signal %d\n",
- WTERMSIG(err));
- return;
- }
-
printerr(1, "handling krb5 upcall (%s)\n", clp->relpath);
token.length = 0;
@@ -655,6 +633,38 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
service ? service : "<null>");
if (uid != 0 || (uid == 0 && root_uses_machine_creds == 0 &&
service == NULL)) {
+
+ /* already running as uid 0 */
+ if (uid == 0)
+ goto no_fork;
+
+ printerr(1, "Forking\n");
+ pid = fork();
+ switch(pid) {
+ case 0:
+ /* Child: fall through to rest of function */
+ childpid = getpid();
+ printerr(1, "CHILD forked pid %d \n", childpid);
+ break;
+ case -1:
+ /* fork() failed! */
+ printerr(0, "WARNING: unable to fork() to handle"
+ "upcall: %s\n", strerror(errno));
+ return;
+ default:
+ /* Parent: just wait on child to exit and return */
+ do {
+ pid = wait(&err);
+ } while(pid == -1 && errno != -ECHILD);
+
+ if (WIFSIGNALED(err))
+ printerr(0, "WARNING: forked child was killed"
+ "with signal %d\n", WTERMSIG(err));
+ return;
+ }
+no_fork:
+
+ printerr(1, "pid %d calling krb5_not_machine_creds\n", getpid());
auth = krb5_not_machine_creds(clp, uid, tgtname, &downcall_err,
&err, &rpc_clnt);
if (err)
@@ -717,11 +727,20 @@ out:
if (pd.pd_ctx_hndl.length != 0 || pd.pd_ctx != 0)
authgss_free_private_data(&pd);
#endif
+ printerr(1, "OUT auth %p rpc_clnt %p pid %d\n", auth, rpc_clnt,
+ getpid());
if (auth)
AUTH_DESTROY(auth);
if (rpc_clnt)
clnt_destroy(rpc_clnt);
- exit(0);
+ pid = getpid();
+ if (pid == childpid) {
+ printerr(1, "pid %d CALLING EXIT\n\n\n", pid);
+ exit(0);
+ } else {
+ printerr(1, "pid %d CALLING RETURN\n\n\n", pid);
+ return;
+ }
out_return_error:
do_error_downcall(fd, uid, downcall_err);
--
1.9.3 (Apple Git-50)
From: Andy Adamson <[email protected]>
Signed-off-by: Andy Adamson <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
utils/gssd/gssd_proc.c | 74 ++++++++++++++++++++++++++++++++------------------
1 file changed, 47 insertions(+), 27 deletions(-)
diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index f5a9ce1..0e04570 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -483,6 +483,49 @@ change_identity(uid_t uid)
}
AUTH *
+krb5_not_machine_creds(struct clnt_info *clp, uid_t uid, char *tgtname,
+ int *downcall_err, int *chg_err, CLIENT **rpc_clnt)
+{
+ AUTH *auth = NULL;
+ gss_cred_id_t gss_cred;
+ char **dname;
+ int err, resp = -1;
+
+ *chg_err = change_identity(uid);
+ if (*chg_err) {
+ printerr(0, "WARNING: failed to change identity: %s",
+ strerror(*chg_err));
+ goto out;
+ }
+
+ /** Tell krb5 gss which credentials cache to use.
+ * Try first to acquire credentials directly via GSSAPI
+ */
+ err = gssd_acquire_user_cred(&gss_cred);
+ if (err == 0)
+ resp = create_auth_rpc_client(clp, tgtname, rpc_clnt,
+ &auth, uid,
+ AUTHTYPE_KRB5, gss_cred);
+
+ /** if create_auth_rplc_client fails try the traditional
+ * method of trolling for credentials
+ */
+ for (dname = ccachesearch; resp != 0 && *dname != NULL; dname++) {
+ err = gssd_setup_krb5_user_gss_ccache(uid, clp->servername,
+ *dname);
+ if (err == -EKEYEXPIRED)
+ *downcall_err = -EKEYEXPIRED;
+ else if (err == 0)
+ resp = create_auth_rpc_client(clp, tgtname, rpc_clnt,
+ &auth, uid,AUTHTYPE_KRB5,
+ GSS_C_NO_CREDENTIAL);
+ }
+
+out:
+ return auth;
+}
+
+AUTH *
krb5_use_machine_creds(struct clnt_info *clp, uid_t uid, char *tgtname,
char *service, CLIENT **rpc_clnt)
{
@@ -552,10 +595,7 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
AUTH *auth = NULL;
struct authgss_private_data pd;
gss_buffer_desc token;
- char **dirname;
- int create_resp = -1;
int err, downcall_err = -EACCES;
- gss_cred_id_t gss_cred;
OM_uint32 maj_stat, min_stat, lifetime_rec;
pid_t pid;
gss_name_t gacceptor = GSS_C_NO_NAME;
@@ -615,32 +655,12 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
service ? service : "<null>");
if (uid != 0 || (uid == 0 && root_uses_machine_creds == 0 &&
service == NULL)) {
-
- err = change_identity(uid);
- if (err) {
- printerr(0, "WARNING: failed to change identity: %s",
- strerror(err));
+ auth = krb5_not_machine_creds(clp, uid, tgtname, &downcall_err,
+ &err, &rpc_clnt);
+ if (err)
goto out_return_error;
- }
-
- /* Tell krb5 gss which credentials cache to use */
- /* Try first to acquire credentials directly via GSSAPI */
- err = gssd_acquire_user_cred(&gss_cred);
- if (!err)
- create_resp = create_auth_rpc_client(clp, tgtname, &rpc_clnt, &auth, uid,
- AUTHTYPE_KRB5, gss_cred);
- /* if create_auth_rplc_client fails try the traditional method of
- * trolling for credentials */
- for (dirname = ccachesearch; create_resp != 0 && *dirname != NULL; dirname++) {
- err = gssd_setup_krb5_user_gss_ccache(uid, clp->servername, *dirname);
- if (err == -EKEYEXPIRED)
- downcall_err = -EKEYEXPIRED;
- else if (!err)
- create_resp = create_auth_rpc_client(clp, tgtname, &rpc_clnt, &auth, uid,
- AUTHTYPE_KRB5, GSS_C_NO_CREDENTIAL);
- }
}
- if (create_resp != 0) {
+ if (auth == NULL) {
if (uid == 0 && (root_uses_machine_creds == 1 ||
service != NULL)) {
auth = krb5_use_machine_creds(clp, uid, tgtname,
--
1.9.3 (Apple Git-50)
From: Andy Adamson <[email protected]>
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 <[email protected]>
---
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);
--
1.9.3 (Apple Git-50)
On Mon, 21 Sep 2015 16:50:09 -0400
<[email protected]> wrote:
> From: Andy Adamson <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]>
DQo+IE9uIFNlcCAyMiwgMjAxNSwgYXQgNzoyNCBBTSwgSmVmZiBMYXl0b24gPGpsYXl0b25AcG9v
Y2hpZXJlZHMubmV0PiB3cm90ZToNCj4gDQo+IE9uIE1vbiwgMjEgU2VwIDIwMTUgMTY6NTA6MDkg
LTA0MDANCj4gPGFuZHJvc0BuZXRhcHAuY29tPiB3cm90ZToNCj4gDQo+PiBGcm9tOiBBbmR5IEFk
YW1zb24gPGFuZHJvc0BuZXRhcHAuY29tPg0KPj4gDQo+PiBTaW5jZSB3ZSBubyBsb25nZXIgZm9y
ayBmb3IgdWlkIDAsIGdzc2RfYXRleGl0KCkgaXMgb25seSBjYWxsZWQgd2hlbiB1aWQgIT0gMCwN
Cj4+IGFuZCBmYWlscyBhcyBwZXJtaXNzaW9ucyBvbiB0aGUgL3RtcC9rcmI1Y2NtYWNoaW5lX1JF
QUxNIGZpbGUgcHJvaGliaXQNCj4+IHRoZSBjbGVhbiB1cCBvZiBtYWNoaW5lIGNyZWRlbnRpYWxz
IChhcyBpdCBzaG91bGQpLg0KPj4gDQo+PiBNb3ZlIHRoZSByZWFwaW5nIG9mIG1hY2hpbmUgY3Jl
ZGVudGlhbHMgYmFjayBpbnRvIGEgU0lHSU5UIHNpZ2hhbmRsZXIgc28gdGhhdA0KPj4gPEN0cmwg
Qz4gZGVzdHJveWVzIG1hY2hpbmUgY3JlZGVudGlhbHMuDQo+PiANCj4+IFNpZ25lZC1vZmYtYnk6
IEFuZHkgQWRhbXNvbiA8YW5kcm9zQG5ldGFwcC5jb20+DQo+PiAtLS0NCj4+IHV0aWxzL2dzc2Qv
Z3NzZC5jIHwgMTAgKysrKy0tLS0tLQ0KPj4gMSBmaWxlIGNoYW5nZWQsIDQgaW5zZXJ0aW9ucygr
KSwgNiBkZWxldGlvbnMoLSkNCj4+IA0KPj4gZGlmZiAtLWdpdCBhL3V0aWxzL2dzc2QvZ3NzZC5j
IGIvdXRpbHMvZ3NzZC9nc3NkLmMNCj4+IGluZGV4IDJhNzY4ZWEuLmViZmY4NjAgMTAwNjQ0DQo+
PiAtLS0gYS91dGlscy9nc3NkL2dzc2QuYw0KPj4gKysrIGIvdXRpbHMvZ3NzZC9nc3NkLmMNCj4+
IEBAIC03MjksMTAgKzcyOSwxMiBAQCBmb3VuZDoNCj4+IH0NCj4+IA0KPj4gc3RhdGljIHZvaWQN
Cj4+IC1nc3NkX2F0ZXhpdCh2b2lkKQ0KPj4gK3NpZ19kaWUoaW50IHNpZ25hbCkNCj4+IHsNCj4+
IAlpZiAocm9vdF91c2VzX21hY2hpbmVfY3JlZHMpDQo+PiAJCWdzc2RfZGVzdHJveV9rcmI1X21h
Y2hpbmVfY3JlZHMoKTsNCj4+ICsJcHJpbnRlcnIoMSwgImV4aXRpbmcgb24gc2lnbmFsICVkXG4i
LCBzaWduYWwpOw0KPj4gKwlleGl0KDApOw0KPj4gfQ0KPj4gDQo+PiBzdGF0aWMgdm9pZA0KPj4g
QEAgLTg5MiwxNyArODk0LDEzIEBAIG1haW4oaW50IGFyZ2MsIGNoYXIgKmFyZ3ZbXSkNCj4+IAkJ
ZXhpdChFWElUX0ZBSUxVUkUpOw0KPj4gCX0NCj4+IA0KPj4gLQlpZiAoYXRleGl0KGdzc2RfYXRl
eGl0KSkgew0KPj4gLQkJcHJpbnRlcnIoMSwgIkVSUk9SOiBhdGV4aXQgZmFpbGVkOiAlc1xuIiwg
c3RyZXJyb3IoZXJybm8pKTsNCj4+IC0JCWV4aXQoRVhJVF9GQUlMVVJFKTsNCj4+IC0JfQ0KPj4g
LQ0KPj4gCWlub3RpZnlfZmQgPSBpbm90aWZ5X2luaXQxKElOX05PTkJMT0NLKTsNCj4+IAlpZiAo
aW5vdGlmeV9mZCA9PSAtMSkgew0KPj4gCQlwcmludGVycigxLCAiRVJST1I6IGlub3RpZnlfaW5p
dDEgZmFpbGVkOiAlc1xuIiwgc3RyZXJyb3IoZXJybm8pKTsNCj4+IAkJZXhpdChFWElUX0ZBSUxV
UkUpOw0KPj4gCX0NCj4+IA0KPj4gKwlzaWduYWwoU0lHSU5ULCBzaWdfZGllKTsNCj4+IAlzaWdu
YWxfc2V0KCZzaWdodXBfZXYsIFNJR0hVUCwgZ3NzZF9zY2FuX2NiLCBOVUxMKTsNCj4+IAlzaWdu
YWxfYWRkKCZzaWdodXBfZXYsIE5VTEwpOw0KPj4gCWV2ZW50X3NldCgmaW5vdGlmeV9ldiwgaW5v
dGlmeV9mZCwgRVZfUkVBRCB8IEVWX1BFUlNJU1QsIGdzc2RfaW5vdGlmeV9jYiwgTlVMTCk7DQo+
IA0KPiANCj4gSG1tLCBJIGRvbid0IGtub3cgYWJvdXQgdGhpcyBvbmUuIFdoYXQgaWYgeW91IGRp
ZSBkdWUgdG8gU0lHVEVSTSBvcg0KPiBzb21ldGhpbmcgKHdoaWNoIGlzIHdoYXQgc3lzdGVtZCBn
ZW5lcmFsbHkgc2VuZHMgcHJvY2Vzc2VzKS4gVGhleSB3b24ndA0KPiBnZXQgY2xlYW5lZCB1cCBp
biB0aGF0IGNhc2UuDQoNClllcyAtIHRoZSBjb2RlIHByaW9yIHRvIGdzc2RfYXRleGl0KCkgY2Fs
bGVkIHRoZSBzaWdfZGllIGZvciBTSUdURVJNIGFzIHdlbGwuDQo+IA0KPiBXaGF0IG1heSBiZSBi
ZXR0ZXIgaXMgdG8ganVzdCBrZWVwIHRoZSBhdGV4aXQgaGFuZGxlciBidXQgaGF2ZSBpdCBkbyBh
DQo+IGdldHBpZCgpIGFuZCBvbmx5IGRvIHRoZSBjbGVhbnVwIGlmIGl0cyB0aGUgb3JpZ2luYWwg
cGlkIG9mIHRoZSBkYWVtb24uDQo+IEp1c3QgZG8gYSBnZXRwaWQgZWFybHkgZHVyaW5nIGdzc2Qg
c3RhcnR1cCBhbmQgc3RvcmUgaXQgaW4gYSBnbG9iYWwNCj4gdmFyaWFibGUgc29tZXdoZXJlLg0K
DQpJIHRyaWVkIHRoYXQgYmVmb3JlIHN1Ym1pdHRpbmcgdGhpcyBwYXRjaCAtIGdzc2RfYXRleGl0
IGlzIG5vdCBjYWxsZWQgd2hlbiBzeXN0ZW1jdGwgc3RvcCBycGMtZ3NzZC5zZXJ2aWNlIHN0b3Bz
IHJwYy5nc3NkLiBOb3IgaXMgaXQgY2FsbGVkIG9uIFNJR0lOVCwgU0lHS0lMTCwgb3IgU0lHVEVS
TS4gQUZBSUNTLCBpdCBpcyBuZXZlciBjYWxsZWQuDQo+IA0KPiBUaGF0IHNhaWQsIG1heWJlIEkg
c2hvdWxkIHRha2UgYSBzdGVwIGJhY2sgYW5kIGFzayAtLSB3aHkgZG9lcyBnc3NkDQo+IGNsZWFu
IHVwIHRoaXMgY3JlZGNhY2hlIGluIHRoZSBmaXJzdCBwbGFjZT8gSXMgdGhlcmUgc29tZSBhdHRh
Y2sgdmVjdG9yDQo+IHRoYXQgdGhpcyBwcmV2ZW50cywgb3IgaXMgaXQganVzdCB0byBwcmV2ZW50
IGEgdG9uIG9mIGNyZWRjYWNoZXMgcGlsaW5nDQo+IHVwIGluIC90bXA/DQoNClllYWgsIGl04oCZ
cyBub3QgYSBiaWcgcHJvYmxlbSAtIGNyZWQgY2FjaGVzIHBpbGluZyB1cCBpbiAvdG1wLiBNb3N0
bHkgaXTigJlzIGR1ZSB0byBhZG1pbuKAmXMgdGhpbmtpbmcgdGhhdCBpZiB0aGUgdXNlciBvZiBr
ZXJiZXJvcyBjcmVkZW50aWFscyBpcyBub3QgcnVubmluZywgdGhlbiB0aGUga2VyYmVyb3MgY3Jl
ZGVudGlhbHMgc2hvdWxkIG5vdCBiZSBwcmVzZW50LiBUaGUgc2FtZSByZWFzb24ga2Rlc3Ryb3kg
aXMgdXNlZC4gTm90ZSB0aGF0IHRoZSAvdG1wL2tyYjVjY21hY2hpbmVfUkVBTE0gZmlsZSBzdXJ2
aXZlcyByZWJvb3Qg4oCmLg0KDQpJIHRoaW5rIEkgc2hvdWxkIHNpbXBseSB1c2Ugc2lnX2RpZSBm
b3IgU0lHVEVSTSBhbmQgU0lHS0lMTCBhcyB3ZWxsIGFzIFNJR0lOVC4gTm90ZSB0aGF0IHdpdGgg
dGhlIHNpZ19kaWUoKSBhcHByb2FjaCwgSSBkbyBub3QgbmVlZCB0byBjaGVjayB0aGUgcGlkLg0K
DQpXSXRoIHRoZSBhZGRpdGlvbiBvZiBTSUdURVJNIGFuZCBTSUdLSUxMLCB0aGUgbWFjaGluZSBj
cmVkZW50aWFsIGZpbGUgaXMg4oCYa2Rlc3Ryb3llZOKAmSB1cG9uIHJwYy5nc3NkIGV4aXQuDQoN
CuKAlD5BbmR5DQo+IA0KPiAtLSANCj4gSmVmZiBMYXl0b24gPGpsYXl0b25AcG9vY2hpZXJlZHMu
bmV0Pg0KDQo=
On Tue, 22 Sep 2015 16:38:41 +0000
"Adamson, Andy" <[email protected]> wrote:
>
> > On Sep 22, 2015, at 7:24 AM, Jeff Layton <[email protected]> wrote:
> >
> > On Mon, 21 Sep 2015 16:50:09 -0400
> > <[email protected]> wrote:
> >
> >> From: Andy Adamson <[email protected]>
> >>
> >> 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 <[email protected]>
> >> ---
> >> 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.
> >
Ahh, right:
The atexit() function registers the given function to be called at nor‐
mal process termination, either via exit(3) or via return from the pro‐
gram's main().
...signals don't qualify as normal process termination.
> > 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.
>
If signals are the only way to take the daemon down then that should be OK.
> WIth the addition of SIGTERM and SIGKILL, the machine credential file
> is ‘kdestroyed’ upon rpc.gssd exit.
>
FWIW, you can't override SIGKILL. SIGTERM should be doable though.
--
Jeff Layton <[email protected]>