[PATCH] sunrpc: Add comment defining gssd upcall API keywords

[PATCH] sunrpc: Add comment defining gssd upcall API keywords

[Chuck Lever]

During review, it was found that the target, service, and srchost
keywords are easily conflated. Add an explainer.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/auth_gss.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Hi Bruce-

I've received no objections to this patch. Can you include it in
v4.19 with the other patches I've already submitted?


diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index 1943e11..2460759 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -461,12 +461,28 @@ static int gss_encode_v1_msg(struct gss_upcall_msg *gss_msg,
 	buflen -= len;
 	p += len;
 	gss_msg->msg.len = len;
+
+	/*
+	 * target= is a full service principal that names the remote
+	 * identity that we are authenticating to.
+	 */
 	if (target_name) {
 		len = scnprintf(p, buflen, "target=%s ", target_name);
 		buflen -= len;
 		p += len;
 		gss_msg->msg.len += len;
 	}
+
+	/*
+	 * gssd uses service= and srchost= to select a matching key from
+	 * the system's keytab to use as the source principal.
+	 *
+	 * service= is the service name part of the source principal,
+	 * or "*" (meaning choose any).
+	 *
+	 * srchost= is the hostname part of the source principal. When
+	 * not provided, gssd uses the local hostname.
+	 */
 	if (service_name) {
 		char *c = strchr(service_name, '@');
 
@@ -482,6 +498,7 @@ static int gss_encode_v1_msg(struct gss_upcall_msg *gss_msg,
 		p += len;
 		gss_msg->msg.len += len;
 	}
+
 	if (mech->gm_upcall_enctypes) {
 		len = scnprintf(p, buflen, "enctypes=%s ",
 				mech->gm_upcall_enctypes);

Re: [PATCH] sunrpc: Add comment defining gssd upcall API keywords

[Steve Dickson]

Hello,

I found your examples in the "Enable the kernel to specify the hostname..."
thread to be very useful. See below.

On 08/20/2018 10:39 AM, Chuck Lever wrote:
> During review, it was found that the target, service, and srchost
> keywords are easily conflated. Add an explainer.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  net/sunrpc/auth_gss/auth_gss.c |   17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> Hi Bruce-
> 
> I've received no objections to this patch. Can you include it in
> v4.19 with the other patches I've already submitted?
> 
> 
> diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
> index 1943e11..2460759 100644
> --- a/net/sunrpc/auth_gss/auth_gss.c
> +++ b/net/sunrpc/auth_gss/auth_gss.c
> @@ -461,12 +461,28 @@ static int gss_encode_v1_msg(struct gss_upcall_msg *gss_msg,
>  	buflen -= len;
>  	p += len;
>  	gss_msg->msg.len = len;
> +
> +	/*
> +	 * target= is a full service principal that names the remote
> +	 * identity that we are authenticating to.
         *  In full, the target is:
         *     host/manet.example.net@EXAMPLE.NET
> +	 */
>  	if (target_name) {
>  		len = scnprintf(p, buflen, "target=%s ", target_name);
>  		buflen -= len;
>  		p += len;
>  		gss_msg->msg.len += len;
>  	}
> +
> +	/*
> +	 * gssd uses service= and srchost= to select a matching key from
> +	 * the system's keytab to use as the source principal.
> +	 *
> +	 * service= is the service name part of the source principal,
> +	 * or "*" (meaning choose any).
> +	 *
> +	 * srchost= is the hostname part of the source principal. When
> +	 * not provided, gssd uses the local hostname. 
         * Thus the full source principal has to be:
         *      nfs/klimt.ib.example.net@EXAMPLE.NET
> +	 */
Just a suggestion... 

steved.
>  	if (service_name) {
>  		char *c = strchr(service_name, '@');
>  
> @@ -482,6 +498,7 @@ static int gss_encode_v1_msg(struct gss_upcall_msg *gss_msg,
>  		p += len;
>  		gss_msg->msg.len += len;
>  	}
> +
>  	if (mech->gm_upcall_enctypes) {
>  		len = scnprintf(p, buflen, "enctypes=%s ",
>  				mech->gm_upcall_enctypes);
> 

Re: [PATCH] sunrpc: Add comment defining gssd upcall API keywords

[J. Bruce Fields]

On Mon, Aug 20, 2018 at 10:39:16AM -0400, Chuck Lever wrote:
> During review, it was found that the target, service, and srchost
> keywords are easily conflated. Add an explainer.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  net/sunrpc/auth_gss/auth_gss.c |   17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> Hi Bruce-
> 
> I've received no objections to this patch. Can you include it in
> v4.19 with the other patches I've already submitted?

Will do, it'll probably just be another week or so before I publish a
4.19 branch.

--b.

> 
> 
> diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
> index 1943e11..2460759 100644
> --- a/net/sunrpc/auth_gss/auth_gss.c
> +++ b/net/sunrpc/auth_gss/auth_gss.c
> @@ -461,12 +461,28 @@ static int gss_encode_v1_msg(struct gss_upcall_msg *gss_msg,
>  	buflen -= len;
>  	p += len;
>  	gss_msg->msg.len = len;
> +
> +	/*
> +	 * target= is a full service principal that names the remote
> +	 * identity that we are authenticating to.
> +	 */
>  	if (target_name) {
>  		len = scnprintf(p, buflen, "target=%s ", target_name);
>  		buflen -= len;
>  		p += len;
>  		gss_msg->msg.len += len;
>  	}
> +
> +	/*
> +	 * gssd uses service= and srchost= to select a matching key from
> +	 * the system's keytab to use as the source principal.
> +	 *
> +	 * service= is the service name part of the source principal,
> +	 * or "*" (meaning choose any).
> +	 *
> +	 * srchost= is the hostname part of the source principal. When
> +	 * not provided, gssd uses the local hostname.
> +	 */
>  	if (service_name) {
>  		char *c = strchr(service_name, '@');
>  
> @@ -482,6 +498,7 @@ static int gss_encode_v1_msg(struct gss_upcall_msg *gss_msg,
>  		p += len;
>  		gss_msg->msg.len += len;
>  	}
> +
>  	if (mech->gm_upcall_enctypes) {
>  		len = scnprintf(p, buflen, "enctypes=%s ",
>  				mech->gm_upcall_enctypes);