[PATCH Version 4 0/2] Libtirpc changes for RPCSEC_GSS version 3

[PATCH Version 4 0/2] Libtirpc changes for RPCSEC_GSS version 3

[andros]

From: Andy Adamson <andros@netapp.com>

Version 4

Requires on Client:
-------------------
gssd patches:  "Version 4 GSSD changes for RPCSEC_GSS version 3"
        0001-GSSD-Add-RPCSEC_GSS-version-to-downcall.patch
	0002-GSSD-add-option-to-not-put-gss-version-in-downcall.patch

To use GSSv3:
-------------
kernel patches: "Version 6 RPCSEC_GSS Version 3 Full MOde MAC Labeling"
        SELINUX export security_current_sid_to_context
        SUNRPC GSSv3: base definitions
        SUNRPC AUTH_GSS get RPCSEC_GSS version from gssd downcall
        SUNRPC AUTH_GSS gss3 reply verifier
        SUNRPC AUTH_GSS RPCSEC_GSS_CREATE with label payload
        SUNRPC AUTH_GSS store and use gss3 label assertion
        SUNRPC-AUTH_GSS gss3_free_assertions
        SUNRPC SVCAUTH_GSS allow RPCSEC_GSS version 1 or 3
        SUNRPC SVCAUTH_GSS gss3 reply verifier
        SUNRPC SVCAUTH_GSS gss3 create label
        SUNRPC SVCAUTH_GSS set gss3 label on nfsd thread
        SUNRPC SVCAUTH_gss store gss3 child handles in parent rsc

Compatibility
-------------
GSSv3 enabled GSSD (libtirpc + gssd patches) is backwards compatible with
client kernels that do not support GSSv3. GSSD negotiates the GSS version,
starting with GSSv3 and falling back to GSSv1.

GSSD has a new option for turning off GSSv3 negotiation.

Andy Adamson (2):
  Use RPCSEC_GSS version 3
  RPCSEC_GSSv3 new reply verifier

 autogen.sh           |   0
 src/auth_gss.c       | 152 ++++++++++++++++++++++++++++++++++++++++++++++++---
 src/clnt_vc.c        |   1 +
 tirpc/rpc/auth_gss.h |   8 ++-
 4 files changed, 153 insertions(+), 8 deletions(-)
 mode change 100644 => 100755 autogen.sh

-- 
1.8.3.1

[PATCH Version 4 1/2] Use RPCSEC_GSS version 3

[andros]

From: Andy Adamson <andros@netapp.com>

Pass gss version to authgss_create
If version 3 fails, fall back to version 1

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 autogen.sh           |  0
 src/auth_gss.c       | 19 +++++++++++++++----
 tirpc/rpc/auth_gss.h |  8 +++++++-
 3 files changed, 22 insertions(+), 5 deletions(-)
 mode change 100644 => 100755 autogen.sh

diff --git a/autogen.sh b/autogen.sh
old mode 100644
new mode 100755
diff --git a/src/auth_gss.c b/src/auth_gss.c
index cf96ada..8f3da2c 100644
--- a/src/auth_gss.c
+++ b/src/auth_gss.c
@@ -132,6 +132,7 @@ char *p;
 	fprintf(stderr, "     qop: %d\n", ptr->qop);
 	fprintf(stderr, "     service: %d\n", ptr->svc);
 	fprintf(stderr, "     cred: %p\n", ptr->cred);
+	fprintf(stderr, "     gss version: %d\n", ptr->gss_vers);
 }
 
 struct rpc_gss_data {
@@ -156,9 +157,14 @@ authgss_create(CLIENT *clnt, gss_name_t name, struct rpc_gss_sec *sec)
 	AUTH			*auth, *save_auth;
 	struct rpc_gss_data	*gd;
 	OM_uint32		min_stat = 0;
+	int                     vers;
 
 	gss_log_debug("in authgss_create()");
 
+	/* Old gssd versions do not set gss_vers */
+	vers = sec->gss_vers == 0 ? RPCSEC_GSS_VERSION : sec->gss_vers;
+
+retry_gssv1:
 	memset(&rpc_createerr, 0, sizeof(rpc_createerr));
 
 	if ((auth = calloc(sizeof(*auth), 1)) == NULL) {
@@ -190,7 +196,7 @@ authgss_create(CLIENT *clnt, gss_name_t name, struct rpc_gss_sec *sec)
 	gd->ctx = GSS_C_NO_CONTEXT;
 	gd->sec = *sec;
 
-	gd->gc.gc_v = RPCSEC_GSS_VERSION;
+	gd->gc.gc_v = vers;
 	gd->gc.gc_proc = RPCSEC_GSS_INIT;
 	gd->gc.gc_svc = gd->sec.svc;
 
@@ -200,9 +206,13 @@ authgss_create(CLIENT *clnt, gss_name_t name, struct rpc_gss_sec *sec)
 	save_auth = clnt->cl_auth;
 	clnt->cl_auth = auth;
 
-	if (!authgss_refresh(auth, NULL))
-		auth = NULL;
-	else
+	if (!authgss_refresh(auth, NULL)) {
+		if (vers == RPCSEC_GSS3_VERSION) {
+			vers = RPCSEC_GSS_VERSION;
+			goto retry_gssv1;
+		} else
+			auth = NULL;
+	} else
 		auth_get(auth); /* Reference for caller */
 
 	clnt->cl_auth = save_auth;
@@ -263,6 +273,7 @@ authgss_get_private_data(AUTH *auth, struct authgss_private_data *pd)
 	pd->pd_ctx = gd->ctx;
 	pd->pd_ctx_hndl = gd->gc.gc_ctx;
 	pd->pd_seq_win = gd->win;
+	pd->pd_gss_vers = gd->gc.gc_v;
 	/*
 	 * We've given this away -- don't try to use it ourself any more
 	 * Caller should call authgss_free_private_data to free data.
diff --git a/tirpc/rpc/auth_gss.h b/tirpc/rpc/auth_gss.h
index a17b34b..a311e08 100644
--- a/tirpc/rpc/auth_gss.h
+++ b/tirpc/rpc/auth_gss.h
@@ -45,7 +45,10 @@ typedef enum {
 	RPCSEC_GSS_DATA = 0,
 	RPCSEC_GSS_INIT = 1,
 	RPCSEC_GSS_CONTINUE_INIT = 2,
-	RPCSEC_GSS_DESTROY = 3
+	RPCSEC_GSS_DESTROY = 3,
+	RPCSEC_GSS_BIND_CHANNEL = 4,	/* GSSv2, not used */
+	RPCSEC_GSS_CREATE = 5,		/* GSSv3 */
+	RPCSEC_GSS_LIST = 6		/* GSSv3 */
 } rpc_gss_proc_t;
 
 /* RPCSEC_GSS services. */
@@ -56,6 +59,7 @@ typedef enum {
 } rpc_gss_svc_t;
 
 #define RPCSEC_GSS_VERSION	1
+#define RPCSEC_GSS3_VERSION	3
 
 /* RPCSEC_GSS security triple. */
 struct rpc_gss_sec {
@@ -64,6 +68,7 @@ struct rpc_gss_sec {
 	rpc_gss_svc_t	svc;		/* service */
 	gss_cred_id_t	cred;		/* cred handle */
 	u_int		req_flags;	/* req flags for init_sec_context */
+	int		gss_vers;	/* highest supported gss version */
 };
 
 /* Private data required for kernel implementation */
@@ -71,6 +76,7 @@ struct authgss_private_data {
 	gss_ctx_id_t	pd_ctx;		/* Session context handle */
 	gss_buffer_desc	pd_ctx_hndl;	/* Credentials context handle */
 	u_int		pd_seq_win;	/* Sequence window */
+	u_int		pd_gss_vers;	/* RPCSEC_GSS version */
 };
 
 #define g_OID_equal(o1, o2) \
-- 
1.8.3.1

Re: [PATCH Version 4 1/2] Use RPCSEC_GSS version 3

[Chuck Lever]

> On Jul 28, 2017, at 4:49 PM, andros@netapp.com wrote:
> 
> From: Andy Adamson <andros@netapp.com>
> 
> Pass gss version to authgss_create
> If version 3 fails, fall back to version 1
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
> autogen.sh           |  0
> src/auth_gss.c       | 19 +++++++++++++++----
> tirpc/rpc/auth_gss.h |  8 +++++++-
> 3 files changed, 22 insertions(+), 5 deletions(-)
> mode change 100644 => 100755 autogen.sh
> 
> diff --git a/autogen.sh b/autogen.sh
> old mode 100644
> new mode 100755
> diff --git a/src/auth_gss.c b/src/auth_gss.c
> index cf96ada..8f3da2c 100644
> --- a/src/auth_gss.c
> +++ b/src/auth_gss.c
> @@ -132,6 +132,7 @@ char *p;
> 	fprintf(stderr, "     qop: %d\n", ptr->qop);
> 	fprintf(stderr, "     service: %d\n", ptr->svc);
> 	fprintf(stderr, "     cred: %p\n", ptr->cred);
> +	fprintf(stderr, "     gss version: %d\n", ptr->gss_vers);
> }
> 
> struct rpc_gss_data {
> @@ -156,9 +157,14 @@ authgss_create(CLIENT *clnt, gss_name_t name, struct rpc_gss_sec *sec)
> 	AUTH			*auth, *save_auth;
> 	struct rpc_gss_data	*gd;
> 	OM_uint32		min_stat = 0;
> +	int                     vers;
> 
> 	gss_log_debug("in authgss_create()");
> 
> +	/* Old gssd versions do not set gss_vers */
> +	vers = sec->gss_vers == 0 ? RPCSEC_GSS_VERSION : sec->gss_vers;
> +
> +retry_gssv1:
> 	memset(&rpc_createerr, 0, sizeof(rpc_createerr));
> 
> 	if ((auth = calloc(sizeof(*auth), 1)) == NULL) {
> @@ -190,7 +196,7 @@ authgss_create(CLIENT *clnt, gss_name_t name, struct rpc_gss_sec *sec)
> 	gd->ctx = GSS_C_NO_CONTEXT;
> 	gd->sec = *sec;
> 
> -	gd->gc.gc_v = RPCSEC_GSS_VERSION;
> +	gd->gc.gc_v = vers;
> 	gd->gc.gc_proc = RPCSEC_GSS_INIT;
> 	gd->gc.gc_svc = gd->sec.svc;
> 
> @@ -200,9 +206,13 @@ authgss_create(CLIENT *clnt, gss_name_t name, struct rpc_gss_sec *sec)
> 	save_auth = clnt->cl_auth;
> 	clnt->cl_auth = auth;
> 
> -	if (!authgss_refresh(auth, NULL))
> -		auth = NULL;
> -	else
> +	if (!authgss_refresh(auth, NULL)) {
> +		if (vers == RPCSEC_GSS3_VERSION) {
> +			vers = RPCSEC_GSS_VERSION;
> +			goto retry_gssv1;

How often does authgss_refresh fail for a reason other
than "GSS3_VERSION is not supported" ? Any such failure
will trigger a damp squib retry with GSSv1, which will
also fail.

Perhaps _rpc_gss_refresh should be changed to return an
error value instead of a boolean, and you should call that
here, directly, instead of authgss_refresh?

I suggest that because ah_refresh is part of a structure
of ops that is visible to library consumers; I'm not sure
it is feasible to change the synopsis of ah_refresh.


> +		} else
> +			auth = NULL;
> +	} else
> 		auth_get(auth); /* Reference for caller */
> 
> 	clnt->cl_auth = save_auth;
> @@ -263,6 +273,7 @@ authgss_get_private_data(AUTH *auth, struct authgss_private_data *pd)
> 	pd->pd_ctx = gd->ctx;
> 	pd->pd_ctx_hndl = gd->gc.gc_ctx;
> 	pd->pd_seq_win = gd->win;
> +	pd->pd_gss_vers = gd->gc.gc_v;
> 	/*
> 	 * We've given this away -- don't try to use it ourself any more
> 	 * Caller should call authgss_free_private_data to free data.
> diff --git a/tirpc/rpc/auth_gss.h b/tirpc/rpc/auth_gss.h
> index a17b34b..a311e08 100644
> --- a/tirpc/rpc/auth_gss.h
> +++ b/tirpc/rpc/auth_gss.h
> @@ -45,7 +45,10 @@ typedef enum {
> 	RPCSEC_GSS_DATA = 0,
> 	RPCSEC_GSS_INIT = 1,
> 	RPCSEC_GSS_CONTINUE_INIT = 2,
> -	RPCSEC_GSS_DESTROY = 3
> +	RPCSEC_GSS_DESTROY = 3,
> +	RPCSEC_GSS_BIND_CHANNEL = 4,	/* GSSv2, not used */
> +	RPCSEC_GSS_CREATE = 5,		/* GSSv3 */
> +	RPCSEC_GSS_LIST = 6		/* GSSv3 */
> } rpc_gss_proc_t;
> 
> /* RPCSEC_GSS services. */
> @@ -56,6 +59,7 @@ typedef enum {
> } rpc_gss_svc_t;
> 
> #define RPCSEC_GSS_VERSION	1
> +#define RPCSEC_GSS3_VERSION	3
> 
> /* RPCSEC_GSS security triple. */
> struct rpc_gss_sec {
> @@ -64,6 +68,7 @@ struct rpc_gss_sec {
> 	rpc_gss_svc_t	svc;		/* service */
> 	gss_cred_id_t	cred;		/* cred handle */
> 	u_int		req_flags;	/* req flags for init_sec_context */
> +	int		gss_vers;	/* highest supported gss version */
> };
> 
> /* Private data required for kernel implementation */
> @@ -71,6 +76,7 @@ struct authgss_private_data {
> 	gss_ctx_id_t	pd_ctx;		/* Session context handle */
> 	gss_buffer_desc	pd_ctx_hndl;	/* Credentials context handle */
> 	u_int		pd_seq_win;	/* Sequence window */
> +	u_int		pd_gss_vers;	/* RPCSEC_GSS version */
> };
> 
> #define g_OID_equal(o1, o2) \
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Chuck Lever

[PATCH Version 4 2/2] RPCSEC_GSSv3 new reply verifier

[andros]

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 src/auth_gss.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 src/clnt_vc.c  |   1 +
 2 files changed, 131 insertions(+), 3 deletions(-)

diff --git a/src/auth_gss.c b/src/auth_gss.c
index 8f3da2c..c015552 100644
--- a/src/auth_gss.c
+++ b/src/auth_gss.c
@@ -47,6 +47,7 @@
 #include <rpc/auth_gss.h>
 #include <rpc/rpcsec_gss.h>
 #include <rpc/clnt.h>
+#include <rpc/rpc_msg.h>
 #include <netinet/in.h>
 
 #include "debug.h"
@@ -209,6 +210,7 @@ retry_gssv1:
 	if (!authgss_refresh(auth, NULL)) {
 		if (vers == RPCSEC_GSS3_VERSION) {
 			vers = RPCSEC_GSS_VERSION;
+			gss_log_debug("authgss_create() RETRY with GSSv1\n");
 			goto retry_gssv1;
 		} else
 			auth = NULL;
@@ -436,17 +438,33 @@ static bool_t
 _rpc_gss_refresh(AUTH *auth, rpc_gss_options_ret_t *options_ret)
 {
 	struct rpc_gss_data	*gd;
+	struct rpc_gss_cred	pregc = {
+		.gc_v = 0,
+	};
 	struct rpc_gss_init_res	 gr;
 	gss_buffer_desc		*recv_tokenp, send_token;
 	OM_uint32		 maj_stat, min_stat, call_stat, ret_flags,
 				 time_ret;
 	gss_OID			 actual_mech_type;
 	char			*mechanism;
+	unsigned char		*buf = NULL;
+	int32_t			*ptr;
 
 	gss_log_debug("in authgss_refresh()");
 
 	gd = AUTH_PRIVATE(auth);
 
+	/** The RPCSEC_GSSv3 verifier is over the call header data caveat
+	 * the gss seq_num which is the current to be sent seq_num, and the
+	 * mtype which is changed from CALL to REPLY.
+	 * Save the input rpc_gss_cred to use values before they are changed.
+	 */
+	pregc = gd->gc;
+
+	gss_log_debug("PREGC gc_v %d gc_proc %d gc_svc %d  gc_ctx.length %d",
+			pregc.gc_v, pregc.gc_proc, pregc.gc_svc,
+			pregc.gc_ctx.length);
+
 	if (gd->established)
 		return (TRUE);
 
@@ -534,15 +552,124 @@ _rpc_gss_refresh(AUTH *auth, rpc_gss_options_ret_t *options_ret)
 			gss_buffer_desc   bufout;
 			u_int seq, qop_state = 0;
 
-			seq = htonl(gr.gr_win);
-			bufin.value = (unsigned char *)&seq;
-			bufin.length = sizeof(seq);
+			if (gd->gc.gc_v == RPCSEC_GSS_VERSION) {
+				seq = htonl(gr.gr_win);
+				bufin.value = (unsigned char *)&seq;
+				bufin.length = sizeof(seq);
+			}
+			if (gd->gc.gc_v == RPCSEC_GSS3_VERSION) {
+				int32_t dummy, crlen;
+				/*
+				 * GSSv3 draft: "compute the verifier using the
+				 * exact same input as is used to compute the
+				 * request verfier, except for the mtype is
+				 * changed from CALL to REPLY.
+				 *
+				 * NOTE: Need to add: the sequence number is
+				 * also different - as it is the seq number
+				 * for the reply. (same seq for gssv1)
+				 *
+				 * NOTE: RFC 2203: creation requests the
+				 * seq_num and the service fields are
+				 * undefined and must be ignored by the server.
+				 * So, send the same gc_svc as used in the call
+				 * as this is what the server should return??.
+				 *
+				 * 1.XID  CLNT_CONTROL(cl, CLGET_XID, <dest>)
+				 * gets the xid of the PREVIOUS call
+				 * see clnt_vc_control, CLGET_XID
+				 *
+				 * 2. direction	REPLY
+				 * 3. rpcvers	RPC_MSG_VERSION
+				 * 4.  prog		RPCBPROG
+				 * 5.  vers		RPCBVERS
+				 * 6.  proc             NULLPROC
+				 *
+				 * 	credential
+				 * 	NOTE: need to use pregc credential
+				 * 	as that is what was passed in CALL
+				 *
+				 * 7. flavor	RPCSEC_GSS
+				 * 8. length
+				 * 	xdr_rpc_gss_cred may do this for you
+				 * 	gd->gc
+				 * 9. gss version     gc_v
+				 * 10. gss proc	   gc_proc
+				 * 11. gss seq	gr.gr_win used above for v1
+				 * 12. gss service
+				 * --------------
+				 *  total 12 xdr units
+				 * gss ctx
+				 * 13. 	len  1 xdr unit
+				 * 	data
+				 */
+				crlen = ((5 * BYTES_PER_XDR_UNIT)
+					 + RNDUP(pregc.gc_ctx.length));
+
+				buf = (u_char *)malloc((8 * BYTES_PER_XDR_UNIT)
+					 + crlen);
+				if (buf == NULL)
+					return (FALSE);
+				ptr = (int32_t *)buf;
+
+				/* XID */
+				CLNT_CONTROL(gd->clnt, CLGET_XID, &dummy);
+				*ptr++ = dummy;  /* hmm, need htonl?*/
+
+				/* direction */
+				IXDR_PUT_ENUM(ptr, REPLY);
+
+				/* rpc vers */
+				IXDR_PUT_LONG(ptr, RPC_MSG_VERSION);
+
+				/* program (NFS) */
+				CLNT_CONTROL(gd->clnt, CLGET_PROG, &dummy);
+				*ptr++ = htonl(dummy);
+
+				/* version (NFS version 4) */
+				CLNT_CONTROL(gd->clnt, CLGET_VERS, &dummy);
+				*ptr++ = htonl(dummy);
+
+				/* NFS Program */
+				IXDR_PUT_LONG(ptr, NULLPROC);
+
+				/* credential */
+				/* flavor */
+				IXDR_PUT_LONG(ptr, RPCSEC_GSS);
+
+				/* cred length goes here */
+				IXDR_PUT_LONG(ptr, crlen);
+
+				/* gss version */
+				IXDR_PUT_LONG(ptr, gd->gc.gc_v);
+
+				/* gss proc from CALL */
+				IXDR_PUT_LONG(ptr, pregc.gc_proc);
+
+				/* gss seq */
+				IXDR_PUT_LONG(ptr, gr.gr_win);
+
+				/* gss service from CALL */
+				IXDR_PUT_LONG(ptr, pregc.gc_svc);
+
+				/* gss ctx len */
+				IXDR_PUT_LONG(ptr, pregc.gc_ctx.length);
+				if (pregc.gc_ctx.length > 0) {
+					memcpy(ptr, pregc.gc_ctx.value,
+						pregc.gc_ctx.length);
+				}
+				ptr += RNDUP(pregc.gc_ctx.length);
+				bufin.value = buf;
+				bufin.length = (8 * BYTES_PER_XDR_UNIT) + crlen;
+			}
 			bufout.value = (unsigned char *)gd->gc_wire_verf.value;
 			bufout.length = gd->gc_wire_verf.length;
 
 			maj_stat = gss_verify_mic(&min_stat, gd->ctx,
 				&bufin, &bufout, &qop_state);
 
+			if (buf && gd->gc.gc_v == RPCSEC_GSS3_VERSION)
+				free(buf);
 			if (maj_stat != GSS_S_COMPLETE
 					|| qop_state != gd->sec.qop) {
 				gss_log_status("authgss_refresh: gss_verify_mic", 
diff --git a/src/clnt_vc.c b/src/clnt_vc.c
index a72f9f7..e6b2ff1 100644
--- a/src/clnt_vc.c
+++ b/src/clnt_vc.c
@@ -574,6 +574,7 @@ clnt_vc_control(cl, request, info)
 		 * first element in the call structure
 		 * This will get the xid of the PREVIOUS call
 		 */
+		fprintf(stderr, "GETXID xid 0x%x\n", ntohl(ct->ct_u.ct_mcalli));
 		*(u_int32_t *)info =
 		    ntohl(*(u_int32_t *)(void *)&ct->ct_u.ct_mcalli);
 		break;
-- 
1.8.3.1

Re: [PATCH Version 4 2/2] RPCSEC_GSSv3 new reply verifier

[Chuck Lever]

Hi Andy-


> On Jul 28, 2017, at 4:50 PM, andros@netapp.com wrote:
> 
> From: Andy Adamson <andros@netapp.com>
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
> src/auth_gss.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> src/clnt_vc.c  |   1 +
> 2 files changed, 131 insertions(+), 3 deletions(-)
> 
> diff --git a/src/auth_gss.c b/src/auth_gss.c
> index 8f3da2c..c015552 100644
> --- a/src/auth_gss.c
> +++ b/src/auth_gss.c
> @@ -47,6 +47,7 @@
> #include <rpc/auth_gss.h>
> #include <rpc/rpcsec_gss.h>
> #include <rpc/clnt.h>
> +#include <rpc/rpc_msg.h>
> #include <netinet/in.h>
> 
> #include "debug.h"
> @@ -209,6 +210,7 @@ retry_gssv1:
> 	if (!authgss_refresh(auth, NULL)) {
> 		if (vers == RPCSEC_GSS3_VERSION) {
> 			vers = RPCSEC_GSS_VERSION;
> +			gss_log_debug("authgss_create() RETRY with GSSv1\n");

The debug message should be added in 1/2 instead.


> 			goto retry_gssv1;
> 		} else
> 			auth = NULL;
> @@ -436,17 +438,33 @@ static bool_t
> _rpc_gss_refresh(AUTH *auth, rpc_gss_options_ret_t *options_ret)
> {
> 	struct rpc_gss_data	*gd;
> +	struct rpc_gss_cred	pregc = {
> +		.gc_v = 0,
> +	};

pregc seems to be initialized unconditionally, just below.
Is there a reason also to have a static initializer here?


> 	struct rpc_gss_init_res	 gr;
> 	gss_buffer_desc		*recv_tokenp, send_token;
> 	OM_uint32		 maj_stat, min_stat, call_stat, ret_flags,
> 				 time_ret;
> 	gss_OID			 actual_mech_type;
> 	char			*mechanism;
> +	unsigned char		*buf = NULL;
> +	int32_t			*ptr;
> 
> 	gss_log_debug("in authgss_refresh()");
> 
> 	gd = AUTH_PRIVATE(auth);
> 
> +	/** The RPCSEC_GSSv3 verifier is over the call header data caveat
> +	 * the gss seq_num which is the current to be sent seq_num, and the
> +	 * mtype which is changed from CALL to REPLY.
> +	 * Save the input rpc_gss_cred to use values before they are changed.
> +	 */
> +	pregc = gd->gc;
> +
> +	gss_log_debug("PREGC gc_v %d gc_proc %d gc_svc %d  gc_ctx.length %d",
> +			pregc.gc_v, pregc.gc_proc, pregc.gc_svc,
> +			pregc.gc_ctx.length);
> +
> 	if (gd->established)
> 		return (TRUE);
> 
> @@ -534,15 +552,124 @@ _rpc_gss_refresh(AUTH *auth, rpc_gss_options_ret_t *options_ret)
> 			gss_buffer_desc   bufout;
> 			u_int seq, qop_state = 0;
> 
> -			seq = htonl(gr.gr_win);
> -			bufin.value = (unsigned char *)&seq;
> -			bufin.length = sizeof(seq);
> +			if (gd->gc.gc_v == RPCSEC_GSS_VERSION) {
> +				seq = htonl(gr.gr_win);
> +				bufin.value = (unsigned char *)&seq;
> +				bufin.length = sizeof(seq);
> +			}

OK, here IMO we need to have better checking of the value
in the gc_v field. What does this code do if GSS2_VERSION is
passed? What if there's garbage in that field?

Also I feel the source code could be better organized if
you use "switch (gd->gc.gc_v)" and call individual helper
functions (per version) that decode the relevant header
fields. I'm not certain that is going to be possible since
this ancient code tends to rely on function global variables.


> +			if (gd->gc.gc_v == RPCSEC_GSS3_VERSION) {
> +				int32_t dummy, crlen;
> +				/*
> +				 * GSSv3 draft: "compute the verifier using the

Love the in-source documentation! So, would it be correct to
say "RFC 7861" here instead of GSSv3 draft?


> +				 * exact same input as is used to compute the
> +				 * request verfier, except for the mtype is
> +				 * changed from CALL to REPLY.
> +				 *
> +				 * NOTE: Need to add: the sequence number is
> +				 * also different - as it is the seq number
> +				 * for the reply. (same seq for gssv1)
> +				 *
> +				 * NOTE: RFC 2203: creation requests the
> +				 * seq_num and the service fields are
> +				 * undefined and must be ignored by the server.
> +				 * So, send the same gc_svc as used in the call
> +				 * as this is what the server should return??.
> +				 *
> +				 * 1.XID  CLNT_CONTROL(cl, CLGET_XID, <dest>)
> +				 * gets the xid of the PREVIOUS call
> +				 * see clnt_vc_control, CLGET_XID
> +				 *
> +				 * 2. direction	REPLY
> +				 * 3. rpcvers	RPC_MSG_VERSION
> +				 * 4.  prog		RPCBPROG
> +				 * 5.  vers		RPCBVERS
> +				 * 6.  proc             NULLPROC
> +				 *
> +				 * 	credential
> +				 * 	NOTE: need to use pregc credential
> +				 * 	as that is what was passed in CALL
> +				 *
> +				 * 7. flavor	RPCSEC_GSS
> +				 * 8. length
> +				 * 	xdr_rpc_gss_cred may do this for you
> +				 * 	gd->gc
> +				 * 9. gss version     gc_v
> +				 * 10. gss proc	   gc_proc
> +				 * 11. gss seq	gr.gr_win used above for v1
> +				 * 12. gss service
> +				 * --------------
> +				 *  total 12 xdr units
> +				 * gss ctx
> +				 * 13. 	len  1 xdr unit
> +				 * 	data
> +				 */
> +				crlen = ((5 * BYTES_PER_XDR_UNIT)
> +					 + RNDUP(pregc.gc_ctx.length));
> +
> +				buf = (u_char *)malloc((8 * BYTES_PER_XDR_UNIT)
> +					 + crlen);

In the final version of this patch, use mem_alloc,
not malloc. There are a couple of spots where I didn't,
when recently adding new APIs, and those are "wrong".


> +				if (buf == NULL)
> +					return (FALSE);
> +				ptr = (int32_t *)buf;
> +
> +				/* XID */
> +				CLNT_CONTROL(gd->clnt, CLGET_XID, &dummy);
> +				*ptr++ = dummy;  /* hmm, need htonl?*/

Have you checked this? I think no htonl is necessary,
but I'd rather be certain and remove this comment.


> +
> +				/* direction */
> +				IXDR_PUT_ENUM(ptr, REPLY);
> +
> +				/* rpc vers */
> +				IXDR_PUT_LONG(ptr, RPC_MSG_VERSION);
> +
> +				/* program (NFS) */
> +				CLNT_CONTROL(gd->clnt, CLGET_PROG, &dummy);
> +				*ptr++ = htonl(dummy);
> +
> +				/* version (NFS version 4) */
> +				CLNT_CONTROL(gd->clnt, CLGET_VERS, &dummy);
> +				*ptr++ = htonl(dummy);
> +
> +				/* NFS Program */
> +				IXDR_PUT_LONG(ptr, NULLPROC);
> +
> +				/* credential */
> +				/* flavor */
> +				IXDR_PUT_LONG(ptr, RPCSEC_GSS);
> +
> +				/* cred length goes here */
> +				IXDR_PUT_LONG(ptr, crlen);
> +
> +				/* gss version */
> +				IXDR_PUT_LONG(ptr, gd->gc.gc_v);
> +
> +				/* gss proc from CALL */
> +				IXDR_PUT_LONG(ptr, pregc.gc_proc);
> +
> +				/* gss seq */
> +				IXDR_PUT_LONG(ptr, gr.gr_win);
> +
> +				/* gss service from CALL */
> +				IXDR_PUT_LONG(ptr, pregc.gc_svc);
> +
> +				/* gss ctx len */
> +				IXDR_PUT_LONG(ptr, pregc.gc_ctx.length);
> +				if (pregc.gc_ctx.length > 0) {
> +					memcpy(ptr, pregc.gc_ctx.value,
> +						pregc.gc_ctx.length);
> +				}
> +				ptr += RNDUP(pregc.gc_ctx.length);
> +				bufin.value = buf;
> +				bufin.length = (8 * BYTES_PER_XDR_UNIT) + crlen;
> +			}
> 			bufout.value = (unsigned char *)gd->gc_wire_verf.value;
> 			bufout.length = gd->gc_wire_verf.length;
> 
> 			maj_stat = gss_verify_mic(&min_stat, gd->ctx,
> 				&bufin, &bufout, &qop_state);
> 
> +			if (buf && gd->gc.gc_v == RPCSEC_GSS3_VERSION)
> +				free(buf);
> 			if (maj_stat != GSS_S_COMPLETE
> 					|| qop_state != gd->sec.qop) {
> 				gss_log_status("authgss_refresh: gss_verify_mic", 
> diff --git a/src/clnt_vc.c b/src/clnt_vc.c
> index a72f9f7..e6b2ff1 100644
> --- a/src/clnt_vc.c
> +++ b/src/clnt_vc.c
> @@ -574,6 +574,7 @@ clnt_vc_control(cl, request, info)
> 		 * first element in the call structure
> 		 * This will get the xid of the PREVIOUS call
> 		 */
> +		fprintf(stderr, "GETXID xid 0x%x\n", ntohl(ct->ct_u.ct_mcalli));

I assume this is for debugging, and thus should be removed before
this change is merged.


> 		*(u_int32_t *)info =
> 		    ntohl(*(u_int32_t *)(void *)&ct->ct_u.ct_mcalli);
> 		break;
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Chuck Lever