2018-08-20 17:55:13

by Chuck Lever III

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

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

Signed-off-by: Chuck Lever <[email protected]>
---
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);


2018-08-20 19:11:48

by Steve Dickson

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

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 <[email protected]>
> ---
> 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/[email protected]
> + */
> 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/[email protected]
> + */
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);
>

2018-08-20 22:21:49

by J. Bruce Fields

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

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 <[email protected]>
> ---
> 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);