Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NFS server in a zone uses keytab and gss service of global zone #963

Open
lhuedepohl opened this issue Nov 21, 2020 · 10 comments
Open

NFS server in a zone uses keytab and gss service of global zone #963

lhuedepohl opened this issue Nov 21, 2020 · 10 comments

Comments

@lhuedepohl
Copy link

It seems that when providing an NFS server form within a zone (cool that this work now, btw!) the keytab and GSS service (svc:/network/rpc/gss) from the the global zone are used. This was very surprising to me and is probably not intended, right?

I created a zone and set it up to be an NFS server, and was able to mount its exports (via zfs set sharenfs) on a Linux client with sec=sys. But when trying it with sec=krb5p (for example) the server replies with GSS Major/Minor 458752/2 - which I believe simply means that the server can not come up with the required credentials.

(Of course, this was only visible in a network trace. The Linux client declined to show me this error and instead downgraded to sec=sys and in the end only presented me with the Permission denied the server replied to that attempt. That sure helps debugging)

But as soon as I copy the kerberos ticket from the zone to the global zone

[root@hv ~]# cp /zones/1e2ddaa7-3f20-e286-c6e5-ab935cf8686b/root/etc/krb5/krb5.keytab /etc/krb5/

and activate the gss service there,

[root@hv ~]# svcadm enable svc:/network/rpc/gss

the mounts succeed. I can even remove the keytab from the zone completely and disable the GSS service there, the NFS mounts still work.

@danmcd
Copy link
Contributor

danmcd commented Nov 23, 2020

This is interesting, and likely overlooked because the kernel gssmod appears to be zone-aware. This should be filed as an illumos bug. I'll dive more deeply tomorrow, including filing said illumos bug.

@danmcd
Copy link
Contributor

danmcd commented Nov 23, 2020

I have small DTrace one-liner I'd like you to run, since you are able to reproduce this:

getgssd_handle:entry { @stacks[stack()] = count();}

Please run it in the foreground in the global zone at system start, PRIOR to any NFS traffic being received by your server. Then run some NFS traffic, making sure that GSSAPI services are being employed (from the global zone, even when the NFS server is in a non-global zone), and then press ^C to get the report of stacks. Please copy and paste that output here, or in a gist.

I'll use that as fodder for the illumos bug I'll filing in the morning (US/Eastern).

@lhuedepohl
Copy link
Author

What would be the exact DTrace command you want me to run? I can't seem to get it to work, I always get 'does not match any probes':

[root@hv ~]# dtrace -l -n 'getgssd_handle:entry'
   ID   PROVIDER            MODULE                          FUNCTION NAME
dtrace: failed to match ::getgssd_handle:entry: No probe matches description
[root@hv ~]# dtrace -l | grep gss
75245        fbt               nfs                 copy_sec_data_gss entry
75246        fbt               nfs                 copy_sec_data_gss return
75391        fbt            rpcsec                 gss_clnt_loadinfo entry
75392        fbt            rpcsec                 gss_clnt_loadinfo return

Forgot to include earlier:

[root@hv ~]# uname -a
SunOS hv 5.11 joyent_20201118T153314Z i86pc i386 i86pc

@lhuedepohl
Copy link
Author

OK, I got it now: After a reboot I of course needed to svcadm enable network/rpc/gss again in the global zone.

Now it worked, here is the output:

root@hv ~]# dtrace -n 'getgssd_handle:entry { @stacks[stack()] = count();}'
dtrace: description 'getgssd_handle:entry ' matched 1 probe
^C


              kgssapi`kgss_get_kmod+0x25
              kgssapi`__kgss_reset_mech+0x50
              kgssapi`kgss_accept_sec_context+0x118
              rpcsec_gss`do_gss_accept+0x176
              rpcsec_gss`svcrpcsec_gss_taskq_func+0x2d
              genunix`taskq_thread+0x2cd
              unix`thread_start+0xb
                1

              kgssapi`kgss_accept_sec_context_wrapped+0x8e
              kgssapi`kgss_accept_sec_context+0xd0
              rpcsec_gss`do_gss_accept+0x176
              rpcsec_gss`svcrpcsec_gss_taskq_func+0x2d
              genunix`taskq_thread+0x2cd
              unix`thread_start+0xb
                4

              kgssapi`kgsscred_expname_to_unix_cred+0xa8
              rpcsec_gss`rpc_gss_getcred+0x10d
              rpcsec`sec_svc_getcred+0x77
              nfssrv`rfs4_compound+0x112
              nfssrv`rfs4_dispatch+0x2d3
              nfssrv`common_dispatch+0x765
              nfssrv`rfs_dispatch+0x23
              rpcmod`svc_getreq+0x24e
              rpcmod`svc_run+0x170
              rpcmod`svc_do_run+0x8b
              nfs`nfssys+0xf9
              unix`_sys_sysenter_post_swapgs+0x159
                4

              kgssapi`kgss_export_sec_context_wrapped+0x4d
              kgssapi`kgss_export_sec_context+0x30
              rpcsec_gss`transfer_sec_context+0x37
              rpcsec_gss`do_gss_accept+0x382
              rpcsec_gss`svcrpcsec_gss_taskq_func+0x2d
              genunix`taskq_thread+0x2cd
              unix`thread_start+0xb
                4

              kgssapi`kgsscred_expname_to_unix_cred+0xa8
              rpcsec_gss`rpc_gss_getcred+0x10d
              rpcsec`sec_svc_getcred+0x77
              nfssrv`rfs4_compound+0x112
              nfssrv`rfs4_dispatch+0x1b6
              nfssrv`common_dispatch+0x765
              nfssrv`rfs_dispatch+0x23
              rpcmod`svc_getreq+0x24e
              rpcmod`svc_run+0x170
              rpcmod`svc_do_run+0x8b
              nfs`nfssys+0xf9
              unix`_sys_sysenter_post_swapgs+0x159
               24

Let me know if you need more tests to be done, this smartos instance is specifically set-up for this, in a VM.

@danmcd
Copy link
Contributor

danmcd commented Nov 23, 2020

Thank you. This will get me started. I will file an illumos bug once I have a sense of what precisely is wrong underneath. (FTR, a bug like this technically belongs in illumos-joyent, but this is not the time for nits like that. Again, thank you!)

@danmcd
Copy link
Contributor

danmcd commented Nov 23, 2020

I'm sorry I don't have better news, but this will end up being a big undertaking. See https://www.illumos.org/issues/13329 for details. I know I don't have the cycles to begin this myself right now (unless I get enough paying SmartOS customers complaining about it). The people in the illumos community who understand this problem as well are being added to the bug as watchers.

@danmcd
Copy link
Contributor

danmcd commented Nov 23, 2020

One last question important question: What PI are you running? This is Important. If you're running before release-20200910 you should try a PI that's 20200910 or more recent. There's a bugfix starting there that may be useful. I'm not sure if it's merely necessary, or if it's sufficient to fix the problem, however.

@lhuedepohl
Copy link
Author

Sorry, that got a bit lost in the end of one of my comments earlier:

[root@hv ~]# uname -a
SunOS hv 5.11 joyent_20201118T153314Z i86pc i386 i86pc

I prepared this installation with the (then) most recent .iso image in a VM specifically to test this issue.

I'm sorry I don't have better news, but this will end up being a big undertaking. See https://www.illumos.org/issues/13329 for details. I know I don't have the cycles to begin this myself right now (unless I get enough paying SmartOS customers complaining about it). The people in the illumos community who understand this problem as well are being added to the bug as watchers.

No worries! I can live with the "workaround" with GSS in the global zone for my use case. I am just happy that this is now reported somewhere and there is the possibility that it gets looked at - and that I didn't do something wrong ;)

@danmcd
Copy link
Contributor

danmcd commented Apr 8, 2022

1.5 years later... I mentioned illumos#13329 on the list, and I got a patch that's being used in NexentaStor. I'm attaching it here. No idea if this is sufficient to the task described above or not, however.

From 49ddab966fcebe1f4de96acf784004b9ddbd8737 Mon Sep 17 00:00:00 2001
From: Matt Barden <[email protected]>
Date: Wed, 9 Jun 2021 02:08:58 -0400
Subject: [PATCH] NEX-23049 rpcsec_gss always calls global-zone GSSD for
 gss_accept_sec_context

Reviewed by: Evan Layton <[email protected]>
Reviewed by: Gordon Ross <[email protected]>
---
 .../uts/common/rpc/sec_gss/svc_rpcsec_gss.c   | 140 ++++++++++++++++--
 1 file changed, 127 insertions(+), 13 deletions(-)

diff --git a/usr/src/uts/common/rpc/sec_gss/svc_rpcsec_gss.c b/usr/src/uts/common/rpc/sec_gss/svc_rpcsec_gss.c
index e35122119f..22c4df2625 100644
--- a/usr/src/uts/common/rpc/sec_gss/svc_rpcsec_gss.c
+++ b/usr/src/uts/common/rpc/sec_gss/svc_rpcsec_gss.c
@@ -49,6 +49,7 @@
 #include <rpc/rpcsec_defs.h>
 #include <sys/sunddi.h>
 #include <sys/atomic.h>
+#include <sys/disp.h>
 
 extern bool_t __rpc_gss_make_principal(rpc_gss_principal_t *, gss_buffer_t);
 
@@ -238,7 +239,6 @@ typedef struct svcrpcsec_gss_taskq_arg {
 
 /* gssd is single threaded, so 1 thread for the taskq is probably good/ok */
 int rpcsec_gss_init_taskq_nthreads = 1;
-static ddi_taskq_t *svcrpcsec_gss_init_taskq = NULL;
 
 extern struct rpc_msg *rpc_msg_dup(struct rpc_msg *);
 extern void rpc_msg_free(struct rpc_msg **, int);
@@ -253,6 +253,110 @@ struct udp_data {
 	mblk_t	*ud_inmp;			/* mblk chain of request */
 };
 
+static zone_key_t svc_gss_zone_key;
+static uint_t svc_gss_tsd_key;
+
+typedef struct svc_gss_zsd {
+	zoneid_t sgz_zoneid;
+	kmutex_t sgz_lock;
+	taskq_t *sgz_init_taskq;
+} svc_gss_zsd_t;
+
+static taskq_t *
+svc_gss_create_taskq(zone_t *zone)
+{
+	taskq_t *tq;
+
+	if (zone == NULL) {
+		cmn_err(CE_NOTE, "%s: couldn't find zone", __func__);
+		return (NULL);
+	}
+
+	/* Like ddi_taskq_create(), but for zones, just for now */
+	tq = taskq_create_proc("rpcsec_gss_init_taskq",
+	    rpcsec_gss_init_taskq_nthreads, minclsyspri,
+	    rpcsec_gss_init_taskq_nthreads, INT_MAX, zone->zone_zsched,
+	    TASKQ_PREPOPULATE);
+
+	if (tq == NULL)
+		cmn_err(CE_NOTE, "%s: taskq_create_proc failed", __func__);
+
+	return (tq);
+}
+
+static void *
+svc_gss_zone_init(zoneid_t zoneid)
+{
+	svc_gss_zsd_t *zsd;
+	zone_t *zone = curzone;
+
+	zsd = kmem_alloc(sizeof (*zsd), KM_SLEEP);
+	mutex_init(&zsd->sgz_lock, NULL, MUTEX_DEFAULT, NULL);
+	zsd->sgz_zoneid = zoneid;
+
+	if (zone->zone_id != zoneid)
+		zone = zone_find_by_id_nolock(zoneid);
+
+	zsd->sgz_init_taskq = svc_gss_create_taskq(zone);
+	return (zsd);
+}
+
+/*
+ * taskq_destroy() wakes all taskq threads and tells them to exit.
+ * It then cv_wait()'s for all of them to finish exiting.
+ * cv_wait() calls resume(), which accesses the target's process.
+ * That may be one of our taskq threads, which are attached to zone_zsched.
+ *
+ * If we do taskq_destroy() in the zsd_destroy callback, then zone_zsched
+ * will have exited and been destroyed before it runs, and we can panic
+ * in resume(). Our taskq threads are not accounted for in either
+ * zone_ntasks or zone_kthreads, which means zsched does not wait for
+ * taskq threads attached to it to complete before exiting.
+ *
+ * We therefore need to do this at shutdown time. At the point where
+ * the zsd_shutdown callback is invoked, all other zone tasks (processes)
+ * have exited, but zone_kthreads and other taskqs hanging off zsched have not.
+ *
+ * We need to be careful not to allow RPC services to be ran from
+ * zsched-attached taskqs or zone_kthreads.
+ */
+static void
+svc_gss_zone_shutdown(zoneid_t zoneid, void *arg)
+{
+	svc_gss_zsd_t *zsd = arg;
+
+	/* All non-zsched-hung threads should be finished. */
+	mutex_enter(&zsd->sgz_lock);
+	if (zsd->sgz_init_taskq != NULL) {
+		taskq_destroy(zsd->sgz_init_taskq);
+		zsd->sgz_init_taskq = NULL;
+	}
+	mutex_exit(&zsd->sgz_lock);
+}
+
+static void
+svc_gss_zone_fini(zoneid_t zoneid, void *arg)
+{
+	svc_gss_zsd_t *zsd = arg;
+
+	mutex_destroy(&zsd->sgz_lock);
+	kmem_free(zsd, sizeof (*zsd));
+}
+
+static svc_gss_zsd_t *
+svc_gss_get_zsd(void)
+{
+	svc_gss_zsd_t *zsd;
+
+	zsd = tsd_get(svc_gss_tsd_key);
+	if (zsd == NULL) {
+		zsd = zone_getspecific(svc_gss_zone_key, curzone);
+		(void) tsd_set(svc_gss_tsd_key, zsd);
+	}
+
+	return (zsd);
+}
+
 /*ARGSUSED*/
 static int
 svc_gss_data_create(void *buf, void *pdata, int kmflag)
@@ -305,14 +409,9 @@ svc_gss_init()
 	    svc_gss_data_reclaim,
 	    NULL, NULL, 0);
 
-	if (svcrpcsec_gss_init_taskq == NULL) {
-		svcrpcsec_gss_init_taskq = ddi_taskq_create(NULL,
-		    "rpcsec_gss_init_taskq", rpcsec_gss_init_taskq_nthreads,
-		    TASKQ_DEFAULTPRI, 0);
-		if (svcrpcsec_gss_init_taskq == NULL)
-			cmn_err(CE_NOTE,
-			    "svc_gss_init: ddi_taskq_create failed");
-	}
+	tsd_create(&svc_gss_tsd_key, NULL);
+	zone_key_create(&svc_gss_zone_key, svc_gss_zone_init,
+	    svc_gss_zone_shutdown, svc_gss_zone_fini);
 }
 
 /*
@@ -322,6 +421,8 @@ svc_gss_init()
 void
 svc_gss_fini()
 {
+	zone_key_delete(svc_gss_zone_key);
+	tsd_destroy(&svc_gss_tsd_key);
 	mutex_destroy(&cb_mutex);
 	mutex_destroy(&ctx_mutex);
 	rw_destroy(&cred_lock);
@@ -913,6 +1014,20 @@ rpcsec_gss_init(
 	svc_rpc_gss_data	*client_data;
 	int ret;
 	svcrpcsec_gss_taskq_arg_t *arg;
+	svc_gss_zsd_t *zsd = svc_gss_get_zsd();
+	taskq_t *tq = zsd->sgz_init_taskq;
+
+	if (tq == NULL) {
+		mutex_enter(&zsd->sgz_lock);
+		if (zsd->sgz_init_taskq == NULL)
+			zsd->sgz_init_taskq = svc_gss_create_taskq(curzone);
+		tq = zsd->sgz_init_taskq;
+		mutex_exit(&zsd->sgz_lock);
+		if (tq == NULL) {
+			cmn_err(CE_NOTE, "%s: no taskq available", __func__);
+			return (RPCSEC_GSS_FAILED);
+		}
+	}
 
 	if (creds.ctx_handle.length != 0) {
 		RPCGSS_LOG0(1, "_svcrpcsec_gss: ctx_handle not null\n");
@@ -977,10 +1092,9 @@ rpcsec_gss_init(
 	arg->cr_service = creds.service;
 
 	/* should be ok to hold clm lock as taskq will have new thread(s) */
-	ret = ddi_taskq_dispatch(svcrpcsec_gss_init_taskq,
-	    svcrpcsec_gss_taskq_func, arg, DDI_SLEEP);
-	if (ret == DDI_FAILURE) {
-		cmn_err(CE_NOTE, "rpcsec_gss_init: taskq dispatch fail");
+	if (taskq_dispatch(tq, svcrpcsec_gss_taskq_func, arg, TQ_SLEEP)
+	    == DDI_FAILURE) {
+		cmn_err(CE_NOTE, "%s: taskq dispatch fail", __func__);
 		ret = RPCSEC_GSS_FAILED;
 		rpc_msg_free(&arg->msg, MAX_AUTH_BYTES);
 		SVC_RELE(arg->rq_xprt, NULL, FALSE);
-- 
2.31.1

@danmcd
Copy link
Contributor

danmcd commented Apr 8, 2022

(To be clear it took me 1.5 years to see if anyone had been working on this problem or not.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants