LinuxLists.cc - [PATCH RFC 0/4] Use correct NFSv4.0 callback credential

[permalink] [raw]

Subject: [PATCH RFC 1/4] sunrpc: Enable the kernel to specify the hostname part of service principals

A multi-homed NFS server may have more than one "nfs" key in its
keytab. Enable the kernel to pick the key it wants as a machine
credential when establishing a GSS context.

This is useful for GSS-protected NFSv4.0 callbacks, which are
required by RFC 7530 S3.3.3 to use the same principal as the service
principal the client used when establishing its lease.

A complementary modification to rpc.gssd is required to fully enable
this feature.

Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/auth_gss/auth_gss.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index 9463af4..818c695 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -284,7 +284,12 @@ struct gss_auth {
return p;
}

-#define UPCALL_BUF_LEN 128
+/* XXX: Need some documentation about why UPCALL_BUF_LEN is so small.
+ * Is user space expecting no more than UPCALL_BUF_LEN bytes?
+ * Note that there are now _two_ NI_MAXHOST sized data items
+ * being passed in this string.
+ */
+#define UPCALL_BUF_LEN 256

struct gss_upcall_msg {
refcount_t count;
@@ -462,8 +467,17 @@ static int gss_encode_v1_msg(struct gss_upcall_msg *gss_msg,
p += len;
gss_msg->msg.len += len;
}
- if (service_name != NULL) {
- len = scnprintf(p, buflen, "service=%s ", service_name);
+ if (service_name) {
+ char *c = strchr(service_name, '@');
+
+ if (!c)
+ len = scnprintf(p, buflen, "service=%s ",
+ service_name);
+ else
+ len = scnprintf(p, buflen,
+ "service=%.*s srchost=%s ",
+ (int)(c - service_name),
+ service_name, c + 1);
buflen -= len;
p += len;
gss_msg->msg.len += len;

2018-05-18 15:39:24

[permalink] [raw]

Subject: [PATCH RFC 2/4] sunrpc: Extract target name into svc_cred

NFSv4.0 callback needs to know the GSS target name the client used
when it established its lease. That information is available from
the GSS context created by gssproxy. Make it available in each
svc_cred.

Note this will also give us access to the real target service
principal name (which is typically "nfs", but spec does not require
that).

Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/nfs4state.c | 7 ++-
include/linux/sunrpc/svcauth.h | 3 +
net/sunrpc/auth_gss/gss_rpc_upcall.c | 70 ++++++++++++++++++++++------------
3 files changed, 53 insertions(+), 27 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index fc74d6f..e56abb7 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1978,8 +1978,10 @@ static int copy_cred(struct svc_cred *target, struct svc_cred *source)
target->cr_principal = kstrdup(source->cr_principal, GFP_KERNEL);
target->cr_raw_principal = kstrdup(source->cr_raw_principal,
GFP_KERNEL);
- if ((source->cr_principal && ! target->cr_principal) ||
- (source->cr_raw_principal && ! target->cr_raw_principal))
+ target->cr_targ_princ = kstrdup(source->cr_targ_princ, GFP_KERNEL);
+ if ((source->cr_principal && !target->cr_principal) ||
+ (source->cr_raw_principal && !target->cr_raw_principal) ||
+ (source->cr_targ_princ && !target->cr_targ_princ))
return -ENOMEM;

target->cr_flavor = source->cr_flavor;
@@ -2056,6 +2058,7 @@ static bool is_gss_cred(struct svc_cred *cr)
|| (!gid_eq(cr1->cr_gid, cr2->cr_gid))
|| !groups_equal(cr1->cr_group_info, cr2->cr_group_info))
return false;
+ /* XXX: check that cr_targ_princ fields match ? */
if (cr1->cr_principal == cr2->cr_principal)
return true;
if (!cr1->cr_principal || !cr2->cr_principal)
diff --git a/include/linux/sunrpc/svcauth.h b/include/linux/sunrpc/svcauth.h
index 7c36565..04e404a 100644
--- a/include/linux/sunrpc/svcauth.h
+++ b/include/linux/sunrpc/svcauth.h
@@ -31,6 +31,7 @@ struct svc_cred {
/* name of form servicetype@hostname, passed down by
* rpc.svcgssd, or computed from the above: */
char *cr_principal;
+ char *cr_targ_princ;
struct gss_api_mech *cr_gss_mech;
};

@@ -39,6 +40,7 @@ static inline void init_svc_cred(struct svc_cred *cred)
cred->cr_group_info = NULL;
cred->cr_raw_principal = NULL;
cred->cr_principal = NULL;
+ cred->cr_targ_princ = NULL;
cred->cr_gss_mech = NULL;
}

@@ -48,6 +50,7 @@ static inline void free_svc_cred(struct svc_cred *cred)
put_group_info(cred->cr_group_info);
kfree(cred->cr_raw_principal);
kfree(cred->cr_principal);
+ kfree(cred->cr_targ_princ);
gss_mech_put(cred->cr_gss_mech);
init_svc_cred(cred);
}
diff --git a/net/sunrpc/auth_gss/gss_rpc_upcall.c b/net/sunrpc/auth_gss/gss_rpc_upcall.c
index 46b295e..6c0862c 100644
--- a/net/sunrpc/auth_gss/gss_rpc_upcall.c
+++ b/net/sunrpc/auth_gss/gss_rpc_upcall.c
@@ -234,6 +234,35 @@ static int gssp_alloc_receive_pages(struct gssx_arg_accept_sec_context *arg)
return 0;
}

+static char *gssp_stringify(struct xdr_netobj *netobj)
+{
+ return kstrndup(netobj->data, netobj->len, GFP_KERNEL);
+}
+
+static void gssp_hostbased_service(char **principal)
+{
+ char *c;
+
+ if (!*principal)
+ return;
+
+ /* terminate and remove realm part */
+ c = strchr(*principal, '@');
+ if (c) {
+ *c = '\0';
+
+ /* change service-hostname delimiter */
+ c = strchr(*principal, '/');
+ if (c)
+ *c = '@';
+ }
+ if (!c) {
+ /* not a service principal */
+ kfree(*principal);
+ *principal = NULL;
+ }
+}
+
/*
* Public functions
*/
@@ -262,6 +291,7 @@ int gssp_accept_sec_context_upcall(struct net *net,
*/
.exported_context_token.len = GSSX_max_output_handle_sz,
.mech.len = GSS_OID_MAX_LEN,
+ .targ_name.display_name.len = GSSX_max_princ_sz,
.src_name.display_name.len = GSSX_max_princ_sz
};
struct gssx_res_accept_sec_context res = {
@@ -275,6 +305,7 @@ int gssp_accept_sec_context_upcall(struct net *net,
.rpc_cred = NULL, /* FIXME ? */
};
struct xdr_netobj client_name = { 0 , NULL };
+ struct xdr_netobj target_name = { 0, NULL };
int ret;

if (data->in_handle.len != 0)
@@ -285,8 +316,6 @@ int gssp_accept_sec_context_upcall(struct net *net,
if (ret)
return ret;

- /* use nfs/ for targ_name ? */
-
ret = gssp_call(net, &msg);

gssp_free_receive_pages(&arg);
@@ -302,6 +331,7 @@ int gssp_accept_sec_context_upcall(struct net *net,
memcpy(data->mech_oid.data, rctxh.mech.data,
data->mech_oid.len);
client_name = rctxh.src_name.display_name;
+ target_name = rctxh.targ_name.display_name;
}

if (res.options.count == 1) {
@@ -323,32 +353,22 @@ int gssp_accept_sec_context_upcall(struct net *net,
}

/* convert to GSS_NT_HOSTBASED_SERVICE form and set into creds */
- if (data->found_creds && client_name.data != NULL) {
- char *c;
-
- data->creds.cr_raw_principal = kstrndup(client_name.data,
- client_name.len, GFP_KERNEL);
-
- data->creds.cr_principal = kstrndup(client_name.data,
- client_name.len, GFP_KERNEL);
- if (data->creds.cr_principal) {
- /* terminate and remove realm part */
- c = strchr(data->creds.cr_principal, '@');
- if (c) {
- *c = '\0';
-
- /* change service-hostname delimiter */
- c = strchr(data->creds.cr_principal, '/');
- if (c) *c = '@';
- }
- if (!c) {
- /* not a service principal */
- kfree(data->creds.cr_principal);
- data->creds.cr_principal = NULL;
- }
+ if (data->found_creds) {
+ if (client_name.data) {
+ data->creds.cr_raw_principal =
+ gssp_stringify(&client_name);
+ data->creds.cr_principal =
+ gssp_stringify(&client_name);
+ gssp_hostbased_service(&data->creds.cr_principal);
+ }
+ if (target_name.data) {
+ data->creds.cr_targ_princ =
+ gssp_stringify(&target_name);
+ gssp_hostbased_service(&data->creds.cr_targ_princ);
}
}
kfree(client_name.data);
+ kfree(target_name.data);

return ret;
}

2018-05-18 15:39:29

[permalink] [raw]

Subject: [PATCH RFC 3/4] nfsd: Use correct credential for NFSv4.0 callback with GSS

I've had trouble when operating a multi-homed Linux NFS server with
Kerberos using NFSv4.0. Lately, I've seen my clients reporting
this (and then hanging):

May 9 11:43:26 manet kernel: NFS: NFSv4 callback contains invalid cred

The client-side commit f11b2a1cfbf5 ("nfs4: copy acceptor name from
context to nfs_client") appears to be related, but I suspect this
problem has been going on for some time before that.

RFC 7530 Section 3.3.3 says:
> For Kerberos V5, nfs/hostname would be a server principal in the
> Kerberos Key Distribution Center database. This is the same
> principal the client acquired a GSS-API context for when it issued
> the SETCLIENTID operation ...

In other words, an NFSv4.0 client expects that the server will use
the same GSS principal for callback that the client used to
establish its lease. For example, if the client used the service
principal "[email protected]" to establish its lease, the server
is required to use "[email protected]" when performing NFSv4.0
callback operations.

The Linux NFS server currently does not. It uses a common service
principal for all callback connections. Sometimes this works as
expected, and other times -- for example, when the server is
accessible via multiple hostnames -- it won't work at all.

This patch scrapes the target name from the client credential,
and uses that for the NFSv4.0 callback credential. That should
be correct much more often.

Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/nfs4callback.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 1f04d2a..0e9ff86 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -769,7 +769,14 @@ void cleanup_callback_cred(void)
static struct rpc_cred *get_backchannel_cred(struct nfs4_client *clp, struct rpc_clnt *client, struct nfsd4_session *ses)
{
if (clp->cl_minorversion == 0) {
- return get_rpccred(callback_cred);
+ char *principal = clp->cl_cred.cr_targ_princ ?
+ clp->cl_cred.cr_targ_princ : "nfs";
+ struct rpc_cred *cred;
+
+ cred = rpc_lookup_machine_cred(principal);
+ if (!IS_ERR(cred))
+ get_rpccred(cred);
+ return cred;
} else {
struct rpc_auth *auth = client->cl_auth;
struct auth_cred acred = {};

2018-05-18 15:39:35

[permalink] [raw]

Subject: [PATCH RFC 4/4] nfsd: Remove callback_cred

Clean up: The global callback_cred is no longer used, so it can be
removed.

Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/nfs4callback.c | 20 --------------------
fs/nfsd/nfs4state.c | 10 ++--------
fs/nfsd/state.h | 2 --
3 files changed, 2 insertions(+), 30 deletions(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 0e9ff86..c088f97 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -746,26 +746,6 @@ static int max_cb_time(struct net *net)
return max(nn->nfsd4_lease/10, (time_t)1) * HZ;
}

-static struct rpc_cred *callback_cred;
-
-int set_callback_cred(void)
-{
- if (callback_cred)
- return 0;
- callback_cred = rpc_lookup_machine_cred("nfs");
- if (!callback_cred)
- return -ENOMEM;
- return 0;
-}
-
-void cleanup_callback_cred(void)
-{
- if (callback_cred) {
- put_rpccred(callback_cred);
- callback_cred = NULL;
- }
-}
-
static struct rpc_cred *get_backchannel_cred(struct nfs4_client *clp, struct rpc_clnt *client, struct nfsd4_session *ses)
{
if (clp->cl_minorversion == 0) {
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index e56abb7..88dc890 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7195,14 +7195,10 @@ static int nfs4_state_create_net(struct net *net)
{
int ret;

- ret = set_callback_cred();
- if (ret)
- return ret;
-
laundry_wq = alloc_workqueue("%s", WQ_UNBOUND, 0, "nfsd4");
if (laundry_wq == NULL) {
ret = -ENOMEM;
- goto out_cleanup_cred;
+ goto out;
}
ret = nfsd4_create_callback_queue();
if (ret)
@@ -7213,8 +7209,7 @@ static int nfs4_state_create_net(struct net *net)

out_free_laundry:
destroy_workqueue(laundry_wq);
-out_cleanup_cred:
- cleanup_callback_cred();
+out:
return ret;
}

@@ -7251,7 +7246,6 @@ static int nfs4_state_create_net(struct net *net)
{
destroy_workqueue(laundry_wq);
nfsd4_destroy_callback_queue();
- cleanup_callback_cred();
}

static void
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index f3772ea..0b15dac 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -617,8 +617,6 @@ extern struct nfs4_client_reclaim *nfsd4_find_reclaim_client(const char *recdir,
struct nfsd_net *nn);
extern __be32 nfs4_check_open_reclaim(clientid_t *clid,
struct nfsd4_compound_state *cstate, struct nfsd_net *nn);
-extern int set_callback_cred(void);
-extern void cleanup_callback_cred(void);
extern void nfsd4_probe_callback(struct nfs4_client *clp);
extern void nfsd4_probe_callback_sync(struct nfs4_client *clp);
extern void nfsd4_change_callback(struct nfs4_client *clp, struct nfs4_cb_conn *);

2018-05-18 16:03:10

[permalink] [raw]

Subject: Re: [PATCH RFC 0/4] Use correct NFSv4.0 callback credential

On Fri, 2018-05-18 at 11:39 -0400, Chuck Lever wrote:
> I've been experimenting with this series that modifies NFSD to
> discover and use the correct GSS service principal when constructing
> its NFSv4.0 callback channels. I'm interested in review of this
> approach. There are a couple of code comments marked with XXX that
> also need some attention.
>
> The rpc.gssd change mentioned in 1/4 is unremarkable and will be
> made available once there is consensus about the kernel changes
> in this series. No gssproxy changes are necessary.
>
> ---
>
> Chuck Lever (4):
> sunrpc: Enable the kernel to specify the hostname part of service principals
> sunrpc: Extract target name into svc_cred
> nfsd: Use correct credential for NFSv4.0 callback with GSS
> nfsd: Remove callback_cred
>
>
> fs/nfsd/nfs4callback.c | 29 ++++----------
> fs/nfsd/nfs4state.c | 17 +++-----
> fs/nfsd/state.h | 2 -
> include/linux/sunrpc/svcauth.h | 3 +
> net/sunrpc/auth_gss/auth_gss.c | 20 ++++++++--
> net/sunrpc/auth_gss/gss_rpc_upcall.c | 70 ++++++++++++++++++++++------------
> 6 files changed, 80 insertions(+), 61 deletions(-)
>
> --
> Chuck Lever

Ack for the sunrpc gssp changes.

The one thing I am unsure of is whether always using the source name
as the callback target is going to work properly, and what happens
when it is not.

Machines mounting with NFSv4.0 but without machine credentials (ie:
root kinits to [email protected] and uses those creds to mount) will
always fail to establish a callback because the NFS client's kernel
does not have access to the user long term key. So even if the KDC
would decide to allow you to get a ticket for a user principal, the
client would not be able to complete context establishment.

Maybe a fallback behavior where it tries to guess at a possible
machine service name would be valuable for cases where a machine
credential is actually available on the client host even though
for whatever reason the mount was done using some user credential.

Simo.

--
Simo Sorce
Sr. Principal Software Engineer
Red Hat, Inc

2018-05-18 16:53:29

[permalink] [raw]

Subject: Re: [PATCH RFC 0/4] Use correct NFSv4.0 callback credential

> On May 18, 2018, at 12:03 PM, Simo Sorce <[email protected]> wrote:
>=20
> On Fri, 2018-05-18 at 11:39 -0400, Chuck Lever wrote:
>> I've been experimenting with this series that modifies NFSD to
>> discover and use the correct GSS service principal when constructing
>> its NFSv4.0 callback channels. I'm interested in review of this
>> approach. There are a couple of code comments marked with XXX that
>> also need some attention.
>>=20
>> The rpc.gssd change mentioned in 1/4 is unremarkable and will be
>> made available once there is consensus about the kernel changes
>> in this series. No gssproxy changes are necessary.
>>=20
>> ---
>>=20
>> Chuck Lever (4):
>> sunrpc: Enable the kernel to specify the hostname part of =
service principals
>> sunrpc: Extract target name into svc_cred
>> nfsd: Use correct credential for NFSv4.0 callback with GSS
>> nfsd: Remove callback_cred
>>=20
>>=20
>> fs/nfsd/nfs4callback.c | 29 ++++----------
>> fs/nfsd/nfs4state.c | 17 +++-----
>> fs/nfsd/state.h | 2 -
>> include/linux/sunrpc/svcauth.h | 3 +
>> net/sunrpc/auth_gss/auth_gss.c | 20 ++++++++--
>> net/sunrpc/auth_gss/gss_rpc_upcall.c | 70 =
++++++++++++++++++++++------------
>> 6 files changed, 80 insertions(+), 61 deletions(-)
>>=20
>> --
>> Chuck Lever
>=20
> Ack for the sunrpc gssp changes.

Thanks!

> The one thing I am unsure of is whether always using the source name
> as the callback target is going to work properly, and what happens
> when it is not.

The series does not change the client principals in play, it
merely ensures that the source principal the server uses to call
the client back will match the principal the client used to
establish the GSS context for the forward channel. It doesn't
seem to me like that would make any case worse (unless there are
bugs, of course).

The forward channel GSS context can be established only if the
client chooses a target principal that has a key in the server's
keytab already. So the server was choosing the wrong key for the
callback channel in some cases before, but now will be choosing
a source principal that the client is already aware of and should
expect.

The server will continue to use a target principal for the
callback channel that the client provided when it authenticated.

> Machines mounting with NFSv4.0 but without machine credentials (ie:
> root kinits to [email protected] and uses those creds to mount) will
> always fail to establish a callback because the NFS client's kernel
> does not have access to the user long term key. So even if the KDC
> would decide to allow you to get a ticket for a user principal, the
> client would not be able to complete context establishment.

This might be a little outside the scope of my series.

I think currently the server will test the callback channel when
the client first OPENs a file, and it will not offer a delegation
if the channel cannot be established. So what we need here is a
reliably quick failure in the cases that will not work. That will
enable the first OPEN to proceed without undue delay.

My mount point was hanging here, but I'm not sure why. I was
focused on addressing the authentication mismatch, which fixed
the hang, but perhaps there's another bug.

> Maybe a fallback behavior where it tries to guess at a possible
> machine service name would be valuable for cases where a machine
> credential is actually available on the client host even though
> for whatever reason the mount was done using some user credential.

We could pick a few likely candidates for the server to try if
the client-provided target principal does not work for the callback
channel, but I wonder if that is an heroic effort that is not worth
the additional complexity.

Is it impossible to give the client's kernel access to that user
key for the callback channel? Alternately, using NFSv4.1 might
be helpful.

Also, in the user credential-only case would the client even use
GSS for lease management. If it does not, then the server would
use AUTH_UNIX and not GSS for NFSv4.0 callbacks.

--
Chuck Lever

2018-05-18 17:07:52

[permalink] [raw]

Subject: Re: [PATCH RFC 0/4] Use correct NFSv4.0 callback credential

On Fri, 2018-05-18 at 12:53 -0400, Chuck Lever wrote:
> > On May 18, 2018, at 12:03 PM, Simo Sorce <[email protected]> wrote:
> >
> > On Fri, 2018-05-18 at 11:39 -0400, Chuck Lever wrote:
> > > I've been experimenting with this series that modifies NFSD to
> > > discover and use the correct GSS service principal when constructing
> > > its NFSv4.0 callback channels. I'm interested in review of this
> > > approach. There are a couple of code comments marked with XXX that
> > > also need some attention.
> > >
> > > The rpc.gssd change mentioned in 1/4 is unremarkable and will be
> > > made available once there is consensus about the kernel changes
> > > in this series. No gssproxy changes are necessary.
> > >
> > > ---
> > >
> > > Chuck Lever (4):
> > > sunrpc: Enable the kernel to specify the hostname part of service principals
> > > sunrpc: Extract target name into svc_cred
> > > nfsd: Use correct credential for NFSv4.0 callback with GSS
> > > nfsd: Remove callback_cred
> > >
> > >
> > > fs/nfsd/nfs4callback.c | 29 ++++----------
> > > fs/nfsd/nfs4state.c | 17 +++-----
> > > fs/nfsd/state.h | 2 -
> > > include/linux/sunrpc/svcauth.h | 3 +
> > > net/sunrpc/auth_gss/auth_gss.c | 20 ++++++++--
> > > net/sunrpc/auth_gss/gss_rpc_upcall.c | 70 ++++++++++++++++++++++------------
> > > 6 files changed, 80 insertions(+), 61 deletions(-)
> > >
> > > --
> > > Chuck Lever
> >
> > Ack for the sunrpc gssp changes.
>
> Thanks!
>
>
> > The one thing I am unsure of is whether always using the source name
> > as the callback target is going to work properly, and what happens
> > when it is not.
>
> The series does not change the client principals in play, it
> merely ensures that the source principal the server uses to call
> the client back will match the principal the client used to
> establish the GSS context for the forward channel. It doesn't
> seem to me like that would make any case worse (unless there are
> bugs, of course).
>
> The forward channel GSS context can be established only if the
> client chooses a target principal that has a key in the server's
> keytab already. So the server was choosing the wrong key for the
> callback channel in some cases before, but now will be choosing
> a source principal that the client is already aware of and should
> expect.
>
> The server will continue to use a target principal for the
> callback channel that the client provided when it authenticated.

This is fine, I am in fact ok with the changes there.

>
> > Machines mounting with NFSv4.0 but without machine credentials (ie:
> > root kinits to [email protected] and uses those creds to mount) will
> > always fail to establish a callback because the NFS client's kernel
> > does not have access to the user long term key. So even if the KDC
> > would decide to allow you to get a ticket for a user principal, the
> > client would not be able to complete context establishment.
>
> This might be a little outside the scope of my series.
>
> I think currently the server will test the callback channel when
> the client first OPENs a file, and it will not offer a delegation
> if the channel cannot be established. So what we need here is a
> reliably quick failure in the cases that will not work. That will
> enable the first OPEN to proceed without undue delay.

If this is an ok degradation for clients that use "unexpected"
credentials to mount, I am ok with that.

> My mount point was hanging here, but I'm not sure why. I was
> focused on addressing the authentication mismatch, which fixed
> the hang, but perhaps there's another bug.

I was just mentioning that callback calls may fail, but as long as that
is a supportable situation we are probably ok (assuming that doesn't
cause hangs of course).

> > Maybe a fallback behavior where it tries to guess at a possible
> > machine service name would be valuable for cases where a machine
> > credential is actually available on the client host even though
> > for whatever reason the mount was done using some user credential.
>
> We could pick a few likely candidates for the server to try if
> the client-provided target principal does not work for the callback
> channel, but I wonder if that is an heroic effort that is not worth
> the additional complexity.

I do not know that it is worth, proper logging (so admins can
understand what is going on and fix their clients) may be sufficient.

> Is it impossible to give the client's kernel access to that user
> key for the callback channel?

No, not in the general case. The long term key is the user's kerberos
password in the simple case, and sometimes may not even exist. No key
may be stored in the KDC database when Pkinit (smartcards) are used or
in some cases Pass+OTP forwarded to external servers, etc...

> Alternately, using NFSv4.1 might be helpful.

Yes, 4.1 is better.

> Also, in the user credential-only case would the client even use
> GSS for lease management. If it does not, then the server would
> use AUTH_UNIX and not GSS for NFSv4.0 callbacks.

I am not sure why it couldn't, but there is some "arbitrary" principal
selection in some of the gss code in NFS, so I do not have a good
answer here without carefully looking at the client code.

Simo.

--
Simo Sorce
Sr. Principal Software Engineer
Red Hat, Inc

2018-05-18 18:53:41

[permalink] [raw]

Subject: Re: [PATCH RFC 0/4] Use correct NFSv4.0 callback credential

Hi Chuck,

I'm not convinced that "srchost=" is necessary. I believe that
everything that is needed is suppose to be encoded in the "target="
option.

I thought target just needed to correctly identify the domain for
which authentication is taking place. Then I think more changes should
be in nfs-utils to make sure that we find credentials for that
particular domain instead of going by the gethostbyname() results.

On Fri, May 18, 2018 at 11:39 AM, Chuck Lever <[email protected]> wrote:
> I've been experimenting with this series that modifies NFSD to
> discover and use the correct GSS service principal when constructing
> its NFSv4.0 callback channels. I'm interested in review of this
> approach. There are a couple of code comments marked with XXX that
> also need some attention.
>
> The rpc.gssd change mentioned in 1/4 is unremarkable and will be
> made available once there is consensus about the kernel changes
> in this series. No gssproxy changes are necessary.
>
> ---
>
> Chuck Lever (4):
> sunrpc: Enable the kernel to specify the hostname part of service principals
> sunrpc: Extract target name into svc_cred
> nfsd: Use correct credential for NFSv4.0 callback with GSS
> nfsd: Remove callback_cred
>
>
> fs/nfsd/nfs4callback.c | 29 ++++----------
> fs/nfsd/nfs4state.c | 17 +++-----
> fs/nfsd/state.h | 2 -
> include/linux/sunrpc/svcauth.h | 3 +
> net/sunrpc/auth_gss/auth_gss.c | 20 ++++++++--
> net/sunrpc/auth_gss/gss_rpc_upcall.c | 70 ++++++++++++++++++++++------------
> 6 files changed, 80 insertions(+), 61 deletions(-)
>
> --
> Chuck Lever
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2018-05-18 19:05:09

[permalink] [raw]

Subject: Re: [PATCH RFC 0/4] Use correct NFSv4.0 callback credential

On Fri, 2018-05-18 at 14:53 -0400, Olga Kornievskaia wrote:
> Hi Chuck,
>
> I'm not convinced that "srchost=" is necessary. I believe that
> everything that is needed is suppose to be encoded in the "target="
> option.
>
> I thought target just needed to correctly identify the domain for
> which authentication is taking place. Then I think more changes should
> be in nfs-utils to make sure that we find credentials for that
> particular domain instead of going by the gethostbyname() results.

What do you mean by "domain" here? Realm or hostname ?
What if the multihomed service is part of multiple realms and even
serves with multiple different hostnames ?

Simo.

>
> On Fri, May 18, 2018 at 11:39 AM, Chuck Lever <[email protected]> wrote:
> > I've been experimenting with this series that modifies NFSD to
> > discover and use the correct GSS service principal when constructing
> > its NFSv4.0 callback channels. I'm interested in review of this
> > approach. There are a couple of code comments marked with XXX that
> > also need some attention.
> >
> > The rpc.gssd change mentioned in 1/4 is unremarkable and will be
> > made available once there is consensus about the kernel changes
> > in this series. No gssproxy changes are necessary.
> >
> > ---
> >
> > Chuck Lever (4):
> > sunrpc: Enable the kernel to specify the hostname part of service principals
> > sunrpc: Extract target name into svc_cred
> > nfsd: Use correct credential for NFSv4.0 callback with GSS
> > nfsd: Remove callback_cred
> >
> >
> > fs/nfsd/nfs4callback.c | 29 ++++----------
> > fs/nfsd/nfs4state.c | 17 +++-----
> > fs/nfsd/state.h | 2 -
> > include/linux/sunrpc/svcauth.h | 3 +
> > net/sunrpc/auth_gss/auth_gss.c | 20 ++++++++--
> > net/sunrpc/auth_gss/gss_rpc_upcall.c | 70 ++++++++++++++++++++++------------
> > 6 files changed, 80 insertions(+), 61 deletions(-)
> >
> > --
> > Chuck Lever
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Simo Sorce
Sr. Principal Software Engineer
Red Hat, Inc

2018-05-18 19:24:18

[permalink] [raw]

Subject: Re: [PATCH RFC 0/4] Use correct NFSv4.0 callback credential

> On May 18, 2018, at 2:53 PM, Olga Kornievskaia <[email protected]> wrote:
>=20
> Hi Chuck,
>=20
> I'm not convinced that "srchost=3D" is necessary. I believe that
> everything that is needed is suppose to be encoded in the "target=3D"
> option.

I don't believe target=3D has what we want. For NFSv4.0 callback:

A. The callback source principal needs to be the same as the client's
NFS server target principal.

B. The callback target principal needs to be the same as the client's
NFS server source principal.

Today, for NFSv4.0 callbacks, the kernel passes to gssd:

target=3D it uses B for this

service=3D it always sets this to "nfs"

And gssd "makes up" the hostname part of A using gethostname(3), which
is bogus for multi-homed NFS servers.

So my patch series does the following for NFSv4.0 callbacks:

1. It acquires the actual target principal the client used to establish
it's lease. This is A above.

2. Instead of always passing "nfs" as the service=3D value, it passes
the non-hostname part of A. That should be "nfs" but it doesn't
have to be.

3. Instead of letting gssd make up the hostname part of the source
principal, it passes the hostname part of A.

That should provide the correct source principal in the multi-home
case, and it should be backwards-compatible with older gssd's.

Now here's why it's not enough to use getsockname(3) on target=3D
to predict the correct source hostname:

If the client has established the lease when mounting from one
particular hostname, and then mounts from another, it will have
to continue managing the lease using the principal it has already
established with the first hostname. NFSD knows what that is, and
can tell gssd what it needs to be. gssd, using only getsockname(3),
would probably pick something that is wrong.

> I thought target just needed to correctly identify the domain for
> which authentication is taking place.

That's what srchost=3D does. We can call it something else if that
helps.

> Then I think more changes should
> be in nfs-utils to make sure that we find credentials for that
> particular domain instead of going by the gethostbyname() results.

That's a patch to gssd that I didn't post. I will post that later if
we all agree srchost=3D is OK.

srchost=3D is optional. If it's not present in the upcall, gssd will
continue to use the result of gethostname(3) to construct the source
principal.

> On Fri, May 18, 2018 at 11:39 AM, Chuck Lever <[email protected]> =
wrote:
>> I've been experimenting with this series that modifies NFSD to
>> discover and use the correct GSS service principal when constructing
>> its NFSv4.0 callback channels. I'm interested in review of this
>> approach. There are a couple of code comments marked with XXX that
>> also need some attention.
>>=20
>> The rpc.gssd change mentioned in 1/4 is unremarkable and will be
>> made available once there is consensus about the kernel changes
>> in this series. No gssproxy changes are necessary.
>>=20
>> ---
>>=20
>> Chuck Lever (4):
>> sunrpc: Enable the kernel to specify the hostname part of =
service principals
>> sunrpc: Extract target name into svc_cred
>> nfsd: Use correct credential for NFSv4.0 callback with GSS
>> nfsd: Remove callback_cred
>>=20
>>=20
>> fs/nfsd/nfs4callback.c | 29 ++++----------
>> fs/nfsd/nfs4state.c | 17 +++-----
>> fs/nfsd/state.h | 2 -
>> include/linux/sunrpc/svcauth.h | 3 +
>> net/sunrpc/auth_gss/auth_gss.c | 20 ++++++++--
>> net/sunrpc/auth_gss/gss_rpc_upcall.c | 70 =
++++++++++++++++++++++------------
>> 6 files changed, 80 insertions(+), 61 deletions(-)
>>=20
>> --
>> Chuck Lever
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" =
in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" =
in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Chuck Lever

2018-05-18 20:02:45

[permalink] [raw]

Subject: Re: [PATCH RFC 0/4] Use correct NFSv4.0 callback credential

On Fri, May 18, 2018 at 3:05 PM, Simo Sorce <[email protected]> wrote:
> On Fri, 2018-05-18 at 14:53 -0400, Olga Kornievskaia wrote:
>> Hi Chuck,
>>
>> I'm not convinced that "srchost=" is necessary. I believe that
>> everything that is needed is suppose to be encoded in the "target="
>> option.
>>
>> I thought target just needed to correctly identify the domain for
>> which authentication is taking place. Then I think more changes should
>> be in nfs-utils to make sure that we find credentials for that
>> particular domain instead of going by the gethostbyname() results.
>
> What do you mean by "domain" here? Realm or hostname ?
> What if the multihomed service is part of multiple realms and even
> serves with multiple different hostnames ?

I meant the DNS domain name.

Let's set a side multi-kerberos domain scenarios here as the
implementation assumes a single domain right now. Otherwise we need to
send down in which kerberos realm authentication is happening.

I thought the problem is that nfs-utils looking for credentials quires
gethostbyname() which on a multihome machine returns an answer which
is insufficient accurately match among the key tab entries. I think we
instead need to be parsing the domain out of the
"target=<principal>@<targethost>.domain" and the look for the
"nfs@<srchost>.domain" match instead of looking for
"nfs@gethostbyname()" . Also kernel needs to make sure it's supplying
the correct "target=" entry.

> Simo.
>
>>
>> On Fri, May 18, 2018 at 11:39 AM, Chuck Lever <[email protected]> wrote:
>> > I've been experimenting with this series that modifies NFSD to
>> > discover and use the correct GSS service principal when constructing
>> > its NFSv4.0 callback channels. I'm interested in review of this
>> > approach. There are a couple of code comments marked with XXX that
>> > also need some attention.
>> >
>> > The rpc.gssd change mentioned in 1/4 is unremarkable and will be
>> > made available once there is consensus about the kernel changes
>> > in this series. No gssproxy changes are necessary.
>> >
>> > ---
>> >
>> > Chuck Lever (4):
>> > sunrpc: Enable the kernel to specify the hostname part of service principals
>> > sunrpc: Extract target name into svc_cred
>> > nfsd: Use correct credential for NFSv4.0 callback with GSS
>> > nfsd: Remove callback_cred
>> >
>> >
>> > fs/nfsd/nfs4callback.c | 29 ++++----------
>> > fs/nfsd/nfs4state.c | 17 +++-----
>> > fs/nfsd/state.h | 2 -
>> > include/linux/sunrpc/svcauth.h | 3 +
>> > net/sunrpc/auth_gss/auth_gss.c | 20 ++++++++--
>> > net/sunrpc/auth_gss/gss_rpc_upcall.c | 70 ++++++++++++++++++++++------------
>> > 6 files changed, 80 insertions(+), 61 deletions(-)
>> >
>> > --
>> > Chuck Lever
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> > the body of a message to [email protected]
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Simo Sorce
> Sr. Principal Software Engineer
> Red Hat, Inc
>

2018-05-18 20:11:24

[permalink] [raw]

Subject: Re: [PATCH RFC 0/4] Use correct NFSv4.0 callback credential

On Fri, May 18, 2018 at 3:23 PM, Chuck Lever <[email protected]> wrote:
>
>
>> On May 18, 2018, at 2:53 PM, Olga Kornievskaia <[email protected]> wrote:
>>
>> Hi Chuck,
>>
>> I'm not convinced that "srchost=" is necessary. I believe that
>> everything that is needed is suppose to be encoded in the "target="
>> option.
>
> I don't believe target= has what we want. For NFSv4.0 callback:
>
> A. The callback source principal needs to be the same as the client's
> NFS server target principal.

Correct and by specifying "nfs" and "*" I think that signals to the
gssd to pick the NFS server principal (which is what the client would
used as the target for the client to server authentication).

> B. The callback target principal needs to be the same as the client's
> NFS server source principal.
>
> Today, for NFSv4.0 callbacks, the kernel passes to gssd:
>
> target= it uses B for this
>
> service= it always sets this to "nfs"
>
> And gssd "makes up" the hostname part of A using gethostname(3), which
> is bogus for multi-homed NFS servers.

I agree that's bogus. Instead we should grab the domain from the
"target=" string.

> So my patch series does the following for NFSv4.0 callbacks:
>
> 1. It acquires the actual target principal the client used to establish
> it's lease. This is A above.
>
> 2. Instead of always passing "nfs" as the service= value, it passes
> the non-hostname part of A. That should be "nfs" but it doesn't
> have to be.

Ok so yes it has historically have always been "nfs" (even thought
it's only RECOMMENDED by the spec) and I don't think this is the case
to add complexity to change a well established behavior?

> 3. Instead of letting gssd make up the hostname part of the source
> principal, it passes the hostname part of A.

I'm arguing that hostname part of A should have been a part of the "target="

>
> That should provide the correct source principal in the multi-home
> case, and it should be backwards-compatible with older gssd's.
>
>
> Now here's why it's not enough to use getsockname(3) on target=
> to predict the correct source hostname:
>
> If the client has established the lease when mounting from one
> particular hostname, and then mounts from another, it will have
> to continue managing the lease using the principal it has already
> established with the first hostname. NFSD knows what that is, and
> can tell gssd what it needs to be. gssd, using only getsockname(3),
> would probably pick something that is wrong.
>
>
>> I thought target just needed to correctly identify the domain for
>> which authentication is taking place.
>
> That's what srchost= does. We can call it something else if that
> helps.
>
>
>> Then I think more changes should
>> be in nfs-utils to make sure that we find credentials for that
>> particular domain instead of going by the gethostbyname() results.
>
> That's a patch to gssd that I didn't post. I will post that later if
> we all agree srchost= is OK.
>
> srchost= is optional. If it's not present in the upcall, gssd will
> continue to use the result of gethostname(3) to construct the source
> principal.
>
>
>> On Fri, May 18, 2018 at 11:39 AM, Chuck Lever <[email protected]> wrote:
>>> I've been experimenting with this series that modifies NFSD to
>>> discover and use the correct GSS service principal when constructing
>>> its NFSv4.0 callback channels. I'm interested in review of this
>>> approach. There are a couple of code comments marked with XXX that
>>> also need some attention.
>>>
>>> The rpc.gssd change mentioned in 1/4 is unremarkable and will be
>>> made available once there is consensus about the kernel changes
>>> in this series. No gssproxy changes are necessary.
>>>
>>> ---
>>>
>>> Chuck Lever (4):
>>> sunrpc: Enable the kernel to specify the hostname part of service principals
>>> sunrpc: Extract target name into svc_cred
>>> nfsd: Use correct credential for NFSv4.0 callback with GSS
>>> nfsd: Remove callback_cred
>>>
>>>
>>> fs/nfsd/nfs4callback.c | 29 ++++----------
>>> fs/nfsd/nfs4state.c | 17 +++-----
>>> fs/nfsd/state.h | 2 -
>>> include/linux/sunrpc/svcauth.h | 3 +
>>> net/sunrpc/auth_gss/auth_gss.c | 20 ++++++++--
>>> net/sunrpc/auth_gss/gss_rpc_upcall.c | 70 ++++++++++++++++++++++------------
>>> 6 files changed, 80 insertions(+), 61 deletions(-)
>>>
>>> --
>>> Chuck Lever
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Chuck Lever
>
>
>

2018-05-18 20:19:36

[permalink] [raw]

Subject: Re: [PATCH RFC 0/4] Use correct NFSv4.0 callback credential

On Fri, May 18, 2018 at 4:11 PM, Olga Kornievskaia <[email protected]> wrote:
> On Fri, May 18, 2018 at 3:23 PM, Chuck Lever <[email protected]> wrote:
>>
>>
>>> On May 18, 2018, at 2:53 PM, Olga Kornievskaia <[email protected]> wrote:
>>>
>>> Hi Chuck,
>>>
>>> I'm not convinced that "srchost=" is necessary. I believe that
>>> everything that is needed is suppose to be encoded in the "target="
>>> option.
>>
>> I don't believe target= has what we want. For NFSv4.0 callback:
>>
>> A. The callback source principal needs to be the same as the client's
>> NFS server target principal.
>
> Correct and by specifying "nfs" and "*" I think that signals to the
> gssd to pick the NFS server principal (which is what the client would
> used as the target for the client to server authentication).
>
>> B. The callback target principal needs to be the same as the client's
>> NFS server source principal.
>>
>> Today, for NFSv4.0 callbacks, the kernel passes to gssd:
>>
>> target= it uses B for this
>>
>> service= it always sets this to "nfs"
>>
>> And gssd "makes up" the hostname part of A using gethostname(3), which
>> is bogus for multi-homed NFS servers.
>
> I agree that's bogus. Instead we should grab the domain from the
> "target=" string.
>
>
>> So my patch series does the following for NFSv4.0 callbacks:
>>
>> 1. It acquires the actual target principal the client used to establish
>> it's lease. This is A above.
>>
>> 2. Instead of always passing "nfs" as the service= value, it passes
>> the non-hostname part of A. That should be "nfs" but it doesn't
>> have to be.
>
> Ok so yes it has historically have always been "nfs" (even thought
> it's only RECOMMENDED by the spec) and I don't think this is the case
> to add complexity to change a well established behavior?
>
>> 3. Instead of letting gssd make up the hostname part of the source
>> principal, it passes the hostname part of A.
>
> I'm arguing that hostname part of A should have been a part of the "target="
>
>>
>> That should provide the correct source principal in the multi-home
>> case, and it should be backwards-compatible with older gssd's.
>>
>>
>> Now here's why it's not enough to use getsockname(3) on target=
>> to predict the correct source hostname:
>>
>> If the client has established the lease when mounting from one
>> particular hostname, and then mounts from another, it will have
>> to continue managing the lease using the principal it has already
>> established with the first hostname. NFSD knows what that is, and
>> can tell gssd what it needs to be. gssd, using only getsockname(3),
>> would probably pick something that is wrong.
>>
>>
>>> I thought target just needed to correctly identify the domain for
>>> which authentication is taking place.
>>
>> That's what srchost= does. We can call it something else if that
>> helps.
>>
>>
>>> Then I think more changes should
>>> be in nfs-utils to make sure that we find credentials for that
>>> particular domain instead of going by the gethostbyname() results.
>>
>> That's a patch to gssd that I didn't post. I will post that later if
>> we all agree srchost= is OK.
>>
>> srchost= is optional. If it's not present in the upcall, gssd will
>> continue to use the result of gethostname(3) to construct the source
>> principal.
>>

I think the srchost is needed only if we are going to deal with doing
something cross-DNS domains so say if
[email protected] will authenticate to
[email protected], then the target's domain would not be
sufficient to match which source domain name to use. So if this the
use case, then sorry I just had to think out-loud to get here, then we
do need srcdomain (and it's really a domain isn't it and not host
that's of interest?).

>>
>>> On Fri, May 18, 2018 at 11:39 AM, Chuck Lever <[email protected]> wrote:
>>>> I've been experimenting with this series that modifies NFSD to
>>>> discover and use the correct GSS service principal when constructing
>>>> its NFSv4.0 callback channels. I'm interested in review of this
>>>> approach. There are a couple of code comments marked with XXX that
>>>> also need some attention.
>>>>
>>>> The rpc.gssd change mentioned in 1/4 is unremarkable and will be
>>>> made available once there is consensus about the kernel changes
>>>> in this series. No gssproxy changes are necessary.
>>>>
>>>> ---
>>>>
>>>> Chuck Lever (4):
>>>> sunrpc: Enable the kernel to specify the hostname part of service principals
>>>> sunrpc: Extract target name into svc_cred
>>>> nfsd: Use correct credential for NFSv4.0 callback with GSS
>>>> nfsd: Remove callback_cred
>>>>
>>>>
>>>> fs/nfsd/nfs4callback.c | 29 ++++----------
>>>> fs/nfsd/nfs4state.c | 17 +++-----
>>>> fs/nfsd/state.h | 2 -
>>>> include/linux/sunrpc/svcauth.h | 3 +
>>>> net/sunrpc/auth_gss/auth_gss.c | 20 ++++++++--
>>>> net/sunrpc/auth_gss/gss_rpc_upcall.c | 70 ++++++++++++++++++++++------------
>>>> 6 files changed, 80 insertions(+), 61 deletions(-)
>>>>
>>>> --
>>>> Chuck Lever
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>>> the body of a message to [email protected]
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>> --
>> Chuck Lever
>>
>>
>>

2018-05-18 20:40:00

[permalink] [raw]

Subject: Re: [PATCH RFC 0/4] Use correct NFSv4.0 callback credential

On Fri, 2018-05-18 at 16:11 -0400, Olga Kornievskaia wrote:
> On Fri, May 18, 2018 at 3:23 PM, Chuck Lever <[email protected]> wrote:
> >
> >
> > > On May 18, 2018, at 2:53 PM, Olga Kornievskaia <[email protected]> wrote:
> > >
> > > Hi Chuck,
> > >
> > > I'm not convinced that "srchost=" is necessary. I believe that
> > > everything that is needed is suppose to be encoded in the "target="
> > > option.
> >
> > I don't believe target= has what we want. For NFSv4.0 callback:
> >
> > A. The callback source principal needs to be the same as the client's
> > NFS server target principal.
>
> Correct and by specifying "nfs" and "*" I think that signals to the
> gssd to pick the NFS server principal (which is what the client would
> used as the target for the client to server authentication).
>
> > B. The callback target principal needs to be the same as the client's
> > NFS server source principal.
> >
> > Today, for NFSv4.0 callbacks, the kernel passes to gssd:
> >
> > target= it uses B for this
> >
> > service= it always sets this to "nfs"
> >
> > And gssd "makes up" the hostname part of A using gethostname(3), which
> > is bogus for multi-homed NFS servers.
>
> I agree that's bogus. Instead we should grab the domain from the
> "target=" string.
>
>
> > So my patch series does the following for NFSv4.0 callbacks:
> >
> > 1. It acquires the actual target principal the client used to establish
> > it's lease. This is A above.
> >
> > 2. Instead of always passing "nfs" as the service= value, it passes
> > the non-hostname part of A. That should be "nfs" but it doesn't
> > have to be.
>
> Ok so yes it has historically have always been "nfs" (even thought
> it's only RECOMMENDED by the spec) and I don't think this is the case
> to add complexity to change a well established behavior?

Not so well established, historically "host/" has been used as well.

> > 3. Instead of letting gssd make up the hostname part of the source
> > principal, it passes the hostname part of A.
>
> I'm arguing that hostname part of A should have been a part of the "target="

You can't use the target name for the source, I do not understand this
comment.

The problem here is that you need to know what server name the client
used.

If the server is multihomed and has three keys in the keytab:
nfs/a.nfs.domain
nfs/b.nfs.domain
nfs/c.nfs.domain

you need to know which creds to acquire in gssd in order to connect
back to the client with the "right" ones.

What is "right" depends on what the client (nfs/client.domain) used to
connect to you, and that is not in target=

HTH,
Simo.

> >
> > That should provide the correct source principal in the multi-home
> > case, and it should be backwards-compatible with older gssd's.
> >
> >
> > Now here's why it's not enough to use getsockname(3) on target=
> > to predict the correct source hostname:
> >
> > If the client has established the lease when mounting from one
> > particular hostname, and then mounts from another, it will have
> > to continue managing the lease using the principal it has already
> > established with the first hostname. NFSD knows what that is, and
> > can tell gssd what it needs to be. gssd, using only getsockname(3),
> > would probably pick something that is wrong.
> >
> >
> > > I thought target just needed to correctly identify the domain for
> > > which authentication is taking place.
> >
> > That's what srchost= does. We can call it something else if that
> > helps.
> >
> >
> > > Then I think more changes should
> > > be in nfs-utils to make sure that we find credentials for that
> > > particular domain instead of going by the gethostbyname() results.
> >
> > That's a patch to gssd that I didn't post. I will post that later if
> > we all agree srchost= is OK.
> >
> > srchost= is optional. If it's not present in the upcall, gssd will
> > continue to use the result of gethostname(3) to construct the source
> > principal.
> >
> >
> > > On Fri, May 18, 2018 at 11:39 AM, Chuck Lever <[email protected]> wrote:
> > > > I've been experimenting with this series that modifies NFSD to
> > > > discover and use the correct GSS service principal when constructing
> > > > its NFSv4.0 callback channels. I'm interested in review of this
> > > > approach. There are a couple of code comments marked with XXX that
> > > > also need some attention.
> > > >
> > > > The rpc.gssd change mentioned in 1/4 is unremarkable and will be
> > > > made available once there is consensus about the kernel changes
> > > > in this series. No gssproxy changes are necessary.
> > > >
> > > > ---
> > > >
> > > > Chuck Lever (4):
> > > > sunrpc: Enable the kernel to specify the hostname part of service principals
> > > > sunrpc: Extract target name into svc_cred
> > > > nfsd: Use correct credential for NFSv4.0 callback with GSS
> > > > nfsd: Remove callback_cred
> > > >
> > > >
> > > > fs/nfsd/nfs4callback.c | 29 ++++----------
> > > > fs/nfsd/nfs4state.c | 17 +++-----
> > > > fs/nfsd/state.h | 2 -
> > > > include/linux/sunrpc/svcauth.h | 3 +
> > > > net/sunrpc/auth_gss/auth_gss.c | 20 ++++++++--
> > > > net/sunrpc/auth_gss/gss_rpc_upcall.c | 70 ++++++++++++++++++++++------------
> > > > 6 files changed, 80 insertions(+), 61 deletions(-)
> > > >
> > > > --
> > > > Chuck Lever
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > > > the body of a message to [email protected]
> > > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > > the body of a message to [email protected]
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> > --
> > Chuck Lever
> >
> >
> >

--
Simo Sorce
Sr. Principal Software Engineer
Red Hat, Inc

2018-05-18 20:42:40

[permalink] [raw]

Subject: Re: [PATCH RFC 0/4] Use correct NFSv4.0 callback credential

On Fri, 2018-05-18 at 16:19 -0400, Olga Kornievskaia wrote:
> I think the srchost is needed only if we are going to deal with doing
> something cross-DNS domains so say if
> [email protected] will authenticate to
> [email protected], then the target's domain would not be
> sufficient to match which source domain name to use. So if this the
> use case, then sorry I just had to think out-loud to get here, then we
> do need srcdomain (and it's really a domain isn't it and not host
> that's of interest?).

I am confused about your use of the term "domain" here.

Can you use either REALM or hostname appropriately ?

Or are you using the term "domain" to mean FQDN ?

Simo.

--
Simo Sorce
Sr. Principal Software Engineer
Red Hat, Inc

2018-05-18 20:56:42

[permalink] [raw]

Subject: Re: [PATCH RFC 0/4] Use correct NFSv4.0 callback credential

> On May 18, 2018, at 4:11 PM, Olga Kornievskaia <[email protected]> wrote:
>=20
> On Fri, May 18, 2018 at 3:23 PM, Chuck Lever <[email protected]> =
wrote:
>>=20
>>=20
>>> On May 18, 2018, at 2:53 PM, Olga Kornievskaia <[email protected]> =
wrote:
>>>=20
>>> Hi Chuck,
>>>=20
>>> I'm not convinced that "srchost=3D" is necessary. I believe that
>>> everything that is needed is suppose to be encoded in the "target=3D"
>>> option.
>>=20
>> I don't believe target=3D has what we want. For NFSv4.0 callback:
>>=20
>> A. The callback source principal needs to be the same as the =
client's
>> NFS server target principal.
>=20
> Correct and by specifying "nfs" and "*" I think that signals to the
> gssd to pick the NFS server principal (which is what the client would
> used as the target for the client to server authentication).
>=20
>> B. The callback target principal needs to be the same as the =
client's
>> NFS server source principal.
>>=20
>> Today, for NFSv4.0 callbacks, the kernel passes to gssd:
>>=20
>> target=3D it uses B for this
>>=20
>> service=3D it always sets this to "nfs"
>>=20
>> And gssd "makes up" the hostname part of A using gethostname(3), =
which
>> is bogus for multi-homed NFS servers.
>=20
> I agree that's bogus. Instead we should grab the domain from the
> "target=3D" string.
>=20
>=20
>> So my patch series does the following for NFSv4.0 callbacks:
>>=20
>> 1. It acquires the actual target principal the client used to =
establish
>> it's lease. This is A above.
>>=20
>> 2. Instead of always passing "nfs" as the service=3D value, it passes
>> the non-hostname part of A. That should be "nfs" but it doesn't
>> have to be.
>=20
> Ok so yes it has historically have always been "nfs" (even thought
> it's only RECOMMENDED by the spec) and I don't think this is the case
> to add complexity to change a well established behavior?
>=20
>> 3. Instead of letting gssd make up the hostname part of the source
>> principal, it passes the hostname part of A.
>=20
> I'm arguing that hostname part of A should have been a part of the =
"target=3D"

For a callback credential upcall:

target name=3D client's principal @ client's hostname
source name=3D server's principal @ server's hostname

I don't see how gssd on a multi-homed server could derive "server's
hostname" correctly from "client's hostname" in every case. As I
explained below, the kernel retains the correct "server's hostname"
string in clp->cl_cred even if the client should connect from another
hostname within the lease expiry period.

Example:

Given a client manet that has

- an interface named manet.domain
- an interface named manet.other.domain
- one key in its keytab: host/manet.domain@REALM

And given a server klimt that has

- an interface named klimt.domain
- an interface named klimt.other.domain
- a key in its keytab: nfs/klimt.domain@REALM
- a key in its keytab: nfs/klimt.other.domain@REAM

Now suppose the client manet mounts klimt.other with NFSv4.0 and
sec=3Dkrb5. The client establishes a GSS context using:

source=3Dhost/manet.domain@REALM
target=3Dnfs/klimt.other.domain@REALM

and we want the server to establish a GSS context for the callback
channel that looks like:

source=3Dnfs/klimt.other.domain@REALM
target=3Dhost/manet.domain@REALM

How does gssd construct the correct source principal given the
target host/manet.domain@REALM ? Today it picks
nfs/klimt.domain@REALM , which is incorrect.

Further, if the client then connects via klimt.domain but continues
to use the original GSS context and lease, how does gssd know it
needs to use source=3Dnfs/klimt.other.domain@REALM if it needs to
re-establish the callback context for some reason?

The only way gssd can know these things for certain is if the
kernel explicitly requests "nfs/klimt.other.domain@REALM" as the
source principal for this GSS context. srchost=3D enables the
kernel to request a specific hostname for the source principal.

Currently our upcall does not specify a REALM for either principal,
but that should be straightforward to add.

I'm in favor of not having magic in gssd that figures this stuff
out based on a heuristic, but rather I'd like the kernel ask for
precisely what it needs every time.

>> That should provide the correct source principal in the multi-home
>> case, and it should be backwards-compatible with older gssd's.
>>=20
>>=20
>> Now here's why it's not enough to use getsockname(3) on target=3D
>> to predict the correct source hostname:
>>=20
>> If the client has established the lease when mounting from one
>> particular hostname, and then mounts from another, it will have
>> to continue managing the lease using the principal it has already
>> established with the first hostname. NFSD knows what that is, and
>> can tell gssd what it needs to be. gssd, using only getsockname(3),
>> would probably pick something that is wrong.
>>=20
>>=20
>>> I thought target just needed to correctly identify the domain for
>>> which authentication is taking place.
>>=20
>> That's what srchost=3D does. We can call it something else if that
>> helps.
>>=20
>>=20
>>> Then I think more changes should
>>> be in nfs-utils to make sure that we find credentials for that
>>> particular domain instead of going by the gethostbyname() results.
>>=20
>> That's a patch to gssd that I didn't post. I will post that later if
>> we all agree srchost=3D is OK.
>>=20
>> srchost=3D is optional. If it's not present in the upcall, gssd will
>> continue to use the result of gethostname(3) to construct the source
>> principal.
>>=20
>>=20
>>> On Fri, May 18, 2018 at 11:39 AM, Chuck Lever =
<[email protected]> wrote:
>>>> I've been experimenting with this series that modifies NFSD to
>>>> discover and use the correct GSS service principal when =
constructing
>>>> its NFSv4.0 callback channels. I'm interested in review of this
>>>> approach. There are a couple of code comments marked with XXX that
>>>> also need some attention.
>>>>=20
>>>> The rpc.gssd change mentioned in 1/4 is unremarkable and will be
>>>> made available once there is consensus about the kernel changes
>>>> in this series. No gssproxy changes are necessary.
>>>>=20
>>>> ---
>>>>=20
>>>> Chuck Lever (4):
>>>> sunrpc: Enable the kernel to specify the hostname part of =
service principals
>>>> sunrpc: Extract target name into svc_cred
>>>> nfsd: Use correct credential for NFSv4.0 callback with GSS
>>>> nfsd: Remove callback_cred
>>>>=20
>>>>=20
>>>> fs/nfsd/nfs4callback.c | 29 ++++----------
>>>> fs/nfsd/nfs4state.c | 17 +++-----
>>>> fs/nfsd/state.h | 2 -
>>>> include/linux/sunrpc/svcauth.h | 3 +
>>>> net/sunrpc/auth_gss/auth_gss.c | 20 ++++++++--
>>>> net/sunrpc/auth_gss/gss_rpc_upcall.c | 70 =
++++++++++++++++++++++------------
>>>> 6 files changed, 80 insertions(+), 61 deletions(-)
>>>>=20
>>>> --
>>>> Chuck Lever
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe =
linux-nfs" in
>>>> the body of a message to [email protected]
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" =
in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>=20
>> --
>> Chuck Lever

--
Chuck Lever

2018-05-18 21:02:46