The current RPCAUTH_LOOKUP_ROOTCREDS flag only works for AUTH_SYS
authentication, and then only as a special case in the code. This patch
removes the auth_sys special casing, and replaces it with generic code.
Signed-off-by: Trond Myklebust <[email protected]>
---
include/linux/sunrpc/auth.h | 4 ++-
net/sunrpc/auth.c | 35 +++++++++++++++------------
net/sunrpc/auth_unix.c | 56 ++++++++++++++++++-------------------------
net/sunrpc/sched.c | 4 ++-
4 files changed, 49 insertions(+), 50 deletions(-)
diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
index 84d5f3a..012566a 100644
--- a/include/linux/sunrpc/auth.h
+++ b/include/linux/sunrpc/auth.h
@@ -89,7 +89,6 @@ struct rpc_auth {
/* Flags for rpcauth_lookupcred() */
#define RPCAUTH_LOOKUP_NEW 0x01 /* Accept an uninitialised cred */
-#define RPCAUTH_LOOKUP_ROOTCREDS 0x02 /* This really ought to go! */
/*
* Client authentication ops
@@ -136,7 +135,8 @@ void rpcauth_release(struct rpc_auth *);
struct rpc_cred * rpcauth_lookup_credcache(struct rpc_auth *, struct auth_cred *, int);
void rpcauth_init_cred(struct rpc_cred *, const struct auth_cred *, struct rpc_auth *, const struct rpc_credops *);
struct rpc_cred * rpcauth_lookupcred(struct rpc_auth *, int);
-struct rpc_cred * rpcauth_bindcred(struct rpc_task *);
+void rpcauth_bindcred(struct rpc_task *);
+void rpcauth_bind_root_cred(struct rpc_task *);
void rpcauth_holdcred(struct rpc_task *);
void put_rpccred(struct rpc_cred *);
void rpcauth_unbindcred(struct rpc_task *);
diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index b38f6ee..b0f2b2e 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -285,9 +285,6 @@ rpcauth_lookup_credcache(struct rpc_auth *auth, struct auth_cred * acred,
nr = hash_long(acred->uid, RPC_CREDCACHE_HASHBITS);
- if (!(flags & RPCAUTH_LOOKUP_ROOTCREDS))
- nr = acred->uid & RPC_CREDCACHE_MASK;
-
rcu_read_lock();
hlist_for_each_entry_rcu(entry, pos, &cache->hashtable[nr], cr_hash) {
if (!entry->cr_ops->crmatch(acred, entry, flags))
@@ -378,30 +375,38 @@ rpcauth_init_cred(struct rpc_cred *cred, const struct auth_cred *acred,
}
EXPORT_SYMBOL_GPL(rpcauth_init_cred);
-struct rpc_cred *
-rpcauth_bindcred(struct rpc_task *task)
+void
+rpcauth_bind_root_cred(struct rpc_task *task)
{
struct rpc_auth *auth = task->tk_client->cl_auth;
struct auth_cred acred = {
- .uid = current->fsuid,
- .gid = current->fsgid,
- .group_info = current->group_info,
+ .uid = 0,
+ .gid = 0,
};
struct rpc_cred *ret;
- int flags = 0;
dprintk("RPC: %5u looking up %s cred\n",
task->tk_pid, task->tk_client->cl_auth->au_ops->au_name);
- get_group_info(acred.group_info);
- if (task->tk_flags & RPC_TASK_ROOTCREDS)
- flags |= RPCAUTH_LOOKUP_ROOTCREDS;
- ret = auth->au_ops->lookup_cred(auth, &acred, flags);
+ ret = auth->au_ops->lookup_cred(auth, &acred, 0);
+ if (!IS_ERR(ret))
+ task->tk_msg.rpc_cred = ret;
+ else
+ task->tk_status = PTR_ERR(ret);
+}
+
+void
+rpcauth_bindcred(struct rpc_task *task)
+{
+ struct rpc_auth *auth = task->tk_client->cl_auth;
+ struct rpc_cred *ret;
+
+ dprintk("RPC: %5u looking up %s cred\n",
+ task->tk_pid, auth->au_ops->au_name);
+ ret = rpcauth_lookupcred(auth, 0);
if (!IS_ERR(ret))
task->tk_msg.rpc_cred = ret;
else
task->tk_status = PTR_ERR(ret);
- put_group_info(acred.group_info);
- return ret;
}
void
diff --git a/net/sunrpc/auth_unix.c b/net/sunrpc/auth_unix.c
index 5ed91e5..b763710 100644
--- a/net/sunrpc/auth_unix.c
+++ b/net/sunrpc/auth_unix.c
@@ -60,7 +60,8 @@ static struct rpc_cred *
unx_create_cred(struct rpc_auth *auth, struct auth_cred *acred, int flags)
{
struct unx_cred *cred;
- int i;
+ unsigned int groups = 0;
+ unsigned int i;
dprintk("RPC: allocating UNIX cred for uid %d gid %d\n",
acred->uid, acred->gid);
@@ -70,21 +71,17 @@ unx_create_cred(struct rpc_auth *auth, struct auth_cred *acred, int flags)
rpcauth_init_cred(&cred->uc_base, acred, auth, &unix_credops);
cred->uc_base.cr_flags = 1UL << RPCAUTH_CRED_UPTODATE;
- if (flags & RPCAUTH_LOOKUP_ROOTCREDS) {
- cred->uc_uid = 0;
- cred->uc_gid = 0;
- cred->uc_gids[0] = NOGROUP;
- } else {
- int groups = acred->group_info->ngroups;
- if (groups > NFS_NGROUPS)
- groups = NFS_NGROUPS;
-
- cred->uc_gid = acred->gid;
- for (i = 0; i < groups; i++)
- cred->uc_gids[i] = GROUP_AT(acred->group_info, i);
- if (i < NFS_NGROUPS)
- cred->uc_gids[i] = NOGROUP;
- }
+
+ if (acred->group_info != NULL)
+ groups = acred->group_info->ngroups;
+ if (groups > NFS_NGROUPS)
+ groups = NFS_NGROUPS;
+
+ cred->uc_gid = acred->gid;
+ for (i = 0; i < groups; i++)
+ cred->uc_gids[i] = GROUP_AT(acred->group_info, i);
+ if (i < NFS_NGROUPS)
+ cred->uc_gids[i] = NOGROUP;
return &cred->uc_base;
}
@@ -118,26 +115,21 @@ static int
unx_match(struct auth_cred *acred, struct rpc_cred *rcred, int flags)
{
struct unx_cred *cred = container_of(rcred, struct unx_cred, uc_base);
- int i;
+ unsigned int groups = 0;
+ unsigned int i;
- if (!(flags & RPCAUTH_LOOKUP_ROOTCREDS)) {
- int groups;
- if (cred->uc_uid != acred->uid
- || cred->uc_gid != acred->gid)
- return 0;
+ if (cred->uc_uid != acred->uid || cred->uc_gid != acred->gid)
+ return 0;
+ if (acred->group_info != NULL)
groups = acred->group_info->ngroups;
- if (groups > NFS_NGROUPS)
- groups = NFS_NGROUPS;
- for (i = 0; i < groups ; i++)
- if (cred->uc_gids[i] != GROUP_AT(acred->group_info, i))
- return 0;
- return 1;
- }
- return (cred->uc_uid == 0
- && cred->uc_gid == 0
- && cred->uc_gids[0] == (gid_t) NOGROUP);
+ if (groups > NFS_NGROUPS)
+ groups = NFS_NGROUPS;
+ for (i = 0; i < groups ; i++)
+ if (cred->uc_gids[i] != GROUP_AT(acred->group_info, i))
+ return 0;
+ return 1;
}
/*
diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index cae219c..7db956f 100644
--- a/net/sunrpc/sched.c
+++ b/net/sunrpc/sched.c
@@ -821,8 +821,10 @@ static void rpc_init_task(struct rpc_task *task, const struct rpc_task_setup *ta
/* Bind the user cred */
if (task->tk_msg.rpc_cred != NULL)
rpcauth_holdcred(task);
- else
+ else if (!(task_setup_data->flags & RPC_TASK_ROOTCREDS))
rpcauth_bindcred(task);
+ else
+ rpcauth_bind_root_cred(task);
if (task->tk_action == NULL)
rpc_call_start(task);
}
Trond,
We were thinking of using RPCAUTH_LOOKUP_ROOTCREDS flag to acquire
machine creds for authenticated callback.
Trond Myklebust wrote:
> The current RPCAUTH_LOOKUP_ROOTCREDS flag only works for AUTH_SYS
> authentication, and then only as a special case in the code. This patch
> removes the auth_sys special casing, and replaces it with generic code.
>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
>
> include/linux/sunrpc/auth.h | 4 ++-
> net/sunrpc/auth.c | 35 +++++++++++++++------------
> net/sunrpc/auth_unix.c | 56 ++++++++++++++++++-------------------------
> net/sunrpc/sched.c | 4 ++-
> 4 files changed, 49 insertions(+), 50 deletions(-)
>
> diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
> index 84d5f3a..012566a 100644
> --- a/include/linux/sunrpc/auth.h
> +++ b/include/linux/sunrpc/auth.h
> @@ -89,7 +89,6 @@ struct rpc_auth {
>
> /* Flags for rpcauth_lookupcred() */
> #define RPCAUTH_LOOKUP_NEW 0x01 /* Accept an uninitialised cred */
> -#define RPCAUTH_LOOKUP_ROOTCREDS 0x02 /* This really ought to go! */
>
> /*
> * Client authentication ops
> @@ -136,7 +135,8 @@ void rpcauth_release(struct rpc_auth *);
> struct rpc_cred * rpcauth_lookup_credcache(struct rpc_auth *, struct auth_cred *, int);
> void rpcauth_init_cred(struct rpc_cred *, const struct auth_cred *, struct rpc_auth *, const struct rpc_credops *);
> struct rpc_cred * rpcauth_lookupcred(struct rpc_auth *, int);
> -struct rpc_cred * rpcauth_bindcred(struct rpc_task *);
> +void rpcauth_bindcred(struct rpc_task *);
> +void rpcauth_bind_root_cred(struct rpc_task *);
> void rpcauth_holdcred(struct rpc_task *);
> void put_rpccred(struct rpc_cred *);
> void rpcauth_unbindcred(struct rpc_task *);
> diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
> index b38f6ee..b0f2b2e 100644
> --- a/net/sunrpc/auth.c
> +++ b/net/sunrpc/auth.c
> @@ -285,9 +285,6 @@ rpcauth_lookup_credcache(struct rpc_auth *auth, struct auth_cred * acred,
>
> nr = hash_long(acred->uid, RPC_CREDCACHE_HASHBITS);
>
> - if (!(flags & RPCAUTH_LOOKUP_ROOTCREDS))
> - nr = acred->uid & RPC_CREDCACHE_MASK;
> -
> rcu_read_lock();
> hlist_for_each_entry_rcu(entry, pos, &cache->hashtable[nr], cr_hash) {
> if (!entry->cr_ops->crmatch(acred, entry, flags))
> @@ -378,30 +375,38 @@ rpcauth_init_cred(struct rpc_cred *cred, const struct auth_cred *acred,
> }
> EXPORT_SYMBOL_GPL(rpcauth_init_cred);
>
> -struct rpc_cred *
> -rpcauth_bindcred(struct rpc_task *task)
> +void
> +rpcauth_bind_root_cred(struct rpc_task *task)
> {
> struct rpc_auth *auth = task->tk_client->cl_auth;
> struct auth_cred acred = {
> - .uid = current->fsuid,
> - .gid = current->fsgid,
> - .group_info = current->group_info,
> + .uid = 0,
> + .gid = 0,
> };
> struct rpc_cred *ret;
> - int flags = 0;
>
> dprintk("RPC: %5u looking up %s cred\n",
> task->tk_pid, task->tk_client->cl_auth->au_ops->au_name);
> - get_group_info(acred.group_info);
> - if (task->tk_flags & RPC_TASK_ROOTCREDS)
> - flags |= RPCAUTH_LOOKUP_ROOTCREDS;
> - ret = auth->au_ops->lookup_cred(auth, &acred, flags);
> + ret = auth->au_ops->lookup_cred(auth, &acred, 0);
> + if (!IS_ERR(ret))
> + task->tk_msg.rpc_cred = ret;
> + else
> + task->tk_status = PTR_ERR(ret);
> +}
> +
> +void
> +rpcauth_bindcred(struct rpc_task *task)
> +{
> + struct rpc_auth *auth = task->tk_client->cl_auth;
> + struct rpc_cred *ret;
> +
> + dprintk("RPC: %5u looking up %s cred\n",
> + task->tk_pid, auth->au_ops->au_name);
> + ret = rpcauth_lookupcred(auth, 0);
> if (!IS_ERR(ret))
> task->tk_msg.rpc_cred = ret;
> else
> task->tk_status = PTR_ERR(ret);
> - put_group_info(acred.group_info);
> - return ret;
> }
>
> void
> diff --git a/net/sunrpc/auth_unix.c b/net/sunrpc/auth_unix.c
> index 5ed91e5..b763710 100644
> --- a/net/sunrpc/auth_unix.c
> +++ b/net/sunrpc/auth_unix.c
> @@ -60,7 +60,8 @@ static struct rpc_cred *
> unx_create_cred(struct rpc_auth *auth, struct auth_cred *acred, int flags)
> {
> struct unx_cred *cred;
> - int i;
> + unsigned int groups = 0;
> + unsigned int i;
>
> dprintk("RPC: allocating UNIX cred for uid %d gid %d\n",
> acred->uid, acred->gid);
> @@ -70,21 +71,17 @@ unx_create_cred(struct rpc_auth *auth, struct auth_cred *acred, int flags)
>
> rpcauth_init_cred(&cred->uc_base, acred, auth, &unix_credops);
> cred->uc_base.cr_flags = 1UL << RPCAUTH_CRED_UPTODATE;
> - if (flags & RPCAUTH_LOOKUP_ROOTCREDS) {
> - cred->uc_uid = 0;
> - cred->uc_gid = 0;
> - cred->uc_gids[0] = NOGROUP;
> - } else {
> - int groups = acred->group_info->ngroups;
> - if (groups > NFS_NGROUPS)
> - groups = NFS_NGROUPS;
> -
> - cred->uc_gid = acred->gid;
> - for (i = 0; i < groups; i++)
> - cred->uc_gids[i] = GROUP_AT(acred->group_info, i);
> - if (i < NFS_NGROUPS)
> - cred->uc_gids[i] = NOGROUP;
> - }
> +
> + if (acred->group_info != NULL)
> + groups = acred->group_info->ngroups;
> + if (groups > NFS_NGROUPS)
> + groups = NFS_NGROUPS;
> +
> + cred->uc_gid = acred->gid;
> + for (i = 0; i < groups; i++)
> + cred->uc_gids[i] = GROUP_AT(acred->group_info, i);
> + if (i < NFS_NGROUPS)
> + cred->uc_gids[i] = NOGROUP;
>
> return &cred->uc_base;
> }
> @@ -118,26 +115,21 @@ static int
> unx_match(struct auth_cred *acred, struct rpc_cred *rcred, int flags)
> {
> struct unx_cred *cred = container_of(rcred, struct unx_cred, uc_base);
> - int i;
> + unsigned int groups = 0;
> + unsigned int i;
>
> - if (!(flags & RPCAUTH_LOOKUP_ROOTCREDS)) {
> - int groups;
>
> - if (cred->uc_uid != acred->uid
> - || cred->uc_gid != acred->gid)
> - return 0;
> + if (cred->uc_uid != acred->uid || cred->uc_gid != acred->gid)
> + return 0;
>
> + if (acred->group_info != NULL)
> groups = acred->group_info->ngroups;
> - if (groups > NFS_NGROUPS)
> - groups = NFS_NGROUPS;
> - for (i = 0; i < groups ; i++)
> - if (cred->uc_gids[i] != GROUP_AT(acred->group_info, i))
> - return 0;
> - return 1;
> - }
> - return (cred->uc_uid == 0
> - && cred->uc_gid == 0
> - && cred->uc_gids[0] == (gid_t) NOGROUP);
> + if (groups > NFS_NGROUPS)
> + groups = NFS_NGROUPS;
> + for (i = 0; i < groups ; i++)
> + if (cred->uc_gids[i] != GROUP_AT(acred->group_info, i))
> + return 0;
> + return 1;
> }
>
> /*
> diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
> index cae219c..7db956f 100644
> --- a/net/sunrpc/sched.c
> +++ b/net/sunrpc/sched.c
> @@ -821,8 +821,10 @@ static void rpc_init_task(struct rpc_task *task, const struct rpc_task_setup *ta
> /* Bind the user cred */
> if (task->tk_msg.rpc_cred != NULL)
> rpcauth_holdcred(task);
> - else
> + else if (!(task_setup_data->flags & RPC_TASK_ROOTCREDS))
> rpcauth_bindcred(task);
> + else
> + rpcauth_bind_root_cred(task);
> if (task->tk_action == NULL)
> rpc_call_start(task);
> }
>
> --
> 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
>
>
On Thu, 2008-03-13 at 15:11 -0400, Olga Kornievskaia wrote:
> Trond,
>
> We were thinking of using RPCAUTH_LOOKUP_ROOTCREDS flag to acquire
> machine creds for authenticated callback.
I'd strongly suggest using a different flag for that purpose. The
function of RPCAUTH_LOOKUP_ROOTCREDS _today_ is to allow a future
swap-over-nfs to use root credentials when paging out memory.
That is not the same as machine creds...
Cheers
Trond
--
Trond Myklebust
NFS client maintainer
NetApp
[email protected]
http://www.netapp.com
On Thu, 2008-03-13 at 15:19 -0400, Trond Myklebust wrote:
> On Thu, 2008-03-13 at 15:11 -0400, Olga Kornievskaia wrote:
> > Trond,
> >
> > We were thinking of using RPCAUTH_LOOKUP_ROOTCREDS flag to acquire
> > machine creds for authenticated callback.
>
> I'd strongly suggest using a different flag for that purpose. The
> function of RPCAUTH_LOOKUP_ROOTCREDS _today_ is to allow a future
> swap-over-nfs to use root credentials when paging out memory.
>
> That is not the same as machine creds...
In fact, I'd strongly urge you to add the information for machine creds
to the 'struct auth_cred' instead. The latter is the lookup key for
'rpc_authops->lookup_cred()'.
Cheers
Trond
--
Trond Myklebust
NFS client maintainer
NetApp
[email protected]
http://www.netapp.com
Trond Myklebust wrote:
> On Thu, 2008-03-13 at 15:11 -0400, Olga Kornievskaia wrote:
>
>> Trond,
>>
>> We were thinking of using RPCAUTH_LOOKUP_ROOTCREDS flag to acquire
>> machine creds for authenticated callback..
>>
>
> I'd strongly suggest using a different flag for that purpose. The
> function of RPCAUTH_LOOKUP_ROOTCREDS _today_ is to allow a future
> swap-over-nfs to use root credentials when paging out memory.
>
> That is not the same as machine creds...
>
I apologize I forgot that I introduced a new flag instead,
RPCAUTH_USE_MACH_CRED. It should fit with the other flags such as
RPCAUTH_CRED_NEW in auth.h.
On Thu, 2008-03-13 at 15:30 -0400, Olga Kornievskaia wrote:
>
> Trond Myklebust wrote:
> > On Thu, 2008-03-13 at 15:11 -0400, Olga Kornievskaia wrote:
> >
> >> Trond,
> >>
> >> We were thinking of using RPCAUTH_LOOKUP_ROOTCREDS flag to acquire
> >> machine creds for authenticated callback..
> >>
> >
> > I'd strongly suggest using a different flag for that purpose. The
> > function of RPCAUTH_LOOKUP_ROOTCREDS _today_ is to allow a future
> > swap-over-nfs to use root credentials when paging out memory.
> >
> > That is not the same as machine creds...
> >
> I apologize I forgot that I introduced a new flag instead,
> RPCAUTH_USE_MACH_CRED. It should fit with the other flags such as
> RPCAUTH_CRED_NEW in auth.h.
As I said, I'd rather have that flag in 'struct auth_cred' so that it
works with the "generic" cred mechanism.
Cheers
Trond
--
Trond Myklebust
NFS client maintainer
NetApp
[email protected]
http://www.netapp.com
On Thu, Mar 13, 2008 at 03:25:38PM -0400, Trond Myklebust wrote:
>
> On Thu, 2008-03-13 at 15:19 -0400, Trond Myklebust wrote:
> > On Thu, 2008-03-13 at 15:11 -0400, Olga Kornievskaia wrote:
> > > Trond,
> > >
> > > We were thinking of using RPCAUTH_LOOKUP_ROOTCREDS flag to acquire
> > > machine creds for authenticated callback.
> >
> > I'd strongly suggest using a different flag for that purpose. The
> > function of RPCAUTH_LOOKUP_ROOTCREDS _today_ is to allow a future
> > swap-over-nfs to use root credentials when paging out memory.
> >
> > That is not the same as machine creds...
>
>
> In fact, I'd strongly urge you to add the information for machine creds
> to the 'struct auth_cred' instead. The latter is the lookup key for
> 'rpc_authops->lookup_cred()'.
So something like
diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
index 7a69ca3..d624169 100644
--- a/include/linux/sunrpc/auth.h
+++ b/include/linux/sunrpc/auth.h
@@ -26,6 +26,7 @@ struct auth_cred {
uid_t uid;
gid_t gid;
struct group_info *group_info;
+ int is_machine_cred;
};
? No objection, but it seems like mild overkill for a single bit when
the necessary functions already take a flag parameter. Or do you want
this information in that structure for future uses of the auth_cred as a
key into a more general cred cache?
--b.
On Thu, 2008-03-13 at 15:43 -0400, J. Bruce Fields wrote:
> On Thu, Mar 13, 2008 at 03:25:38PM -0400, Trond Myklebust wrote:
> >
> > On Thu, 2008-03-13 at 15:19 -0400, Trond Myklebust wrote:
> > > On Thu, 2008-03-13 at 15:11 -0400, Olga Kornievskaia wrote:
> > > > Trond,
> > > >
> > > > We were thinking of using RPCAUTH_LOOKUP_ROOTCREDS flag to acquire
> > > > machine creds for authenticated callback.
> > >
> > > I'd strongly suggest using a different flag for that purpose. The
> > > function of RPCAUTH_LOOKUP_ROOTCREDS _today_ is to allow a future
> > > swap-over-nfs to use root credentials when paging out memory.
> > >
> > > That is not the same as machine creds...
> >
> >
> > In fact, I'd strongly urge you to add the information for machine creds
> > to the 'struct auth_cred' instead. The latter is the lookup key for
> > 'rpc_authops->lookup_cred()'.
>
> So something like
>
> diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
> index 7a69ca3..d624169 100644
> --- a/include/linux/sunrpc/auth.h
> +++ b/include/linux/sunrpc/auth.h
> @@ -26,6 +26,7 @@ struct auth_cred {
> uid_t uid;
> gid_t gid;
> struct group_info *group_info;
> + int is_machine_cred;
> };
Yes. Unless we need more information. Will we ever be in a situation
where we want to specify a full
> ? No objection, but it seems like mild overkill for a single bit when
> the necessary functions already take a flag parameter.
The flag parameter should be for internal use by the RPC auth code to
pass around lookup intent information (see gss_match(), which is the
only remaining callback that actually uses that parameter).
The lookup key, OTOH, should be a single structure.
> Or do you want
> this information in that structure for future uses of the auth_cred as a
> key into a more general cred cache?
See the rest of the patch series...
Basically, if the administrator changes our security landscape on the
fly, either by migrating our filesystem, or by changing the supported
security flavours, then we're going to have to renegotiate security on
the fly. We really do _not_ want the NFS structures to cache RPC
credentials which are tied to a particular auth mechanism or rpcsec_gss
session.
The "generic" credential I'm proposing is the tool for carrying all the
information that is needed in order to lookup/create an on-the-wire
credential at the instant when we issue the RPC call.
--
Trond Myklebust
NFS client maintainer
NetApp
[email protected]
http://www.netapp.com
On Thu, 2008-03-13 at 16:36 -0400, Trond Myklebust wrote:
> On Thu, 2008-03-13 at 15:43 -0400, J. Bruce Fields wrote:
> > So something like
> >
> > diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
> > index 7a69ca3..d624169 100644
> > --- a/include/linux/sunrpc/auth.h
> > +++ b/include/linux/sunrpc/auth.h
> > @@ -26,6 +26,7 @@ struct auth_cred {
> > uid_t uid;
> > gid_t gid;
> > struct group_info *group_info;
> > + int is_machine_cred;
> > };
>
> Yes. Unless we need more information. Will we ever be in a situation
> where we want to specify a full
Sorry, I meant to say 'a full principal'. IOW: will we ever need to
specify principals beyond the simple 'uid/keyring' and 'machine cred'
distinction.
--
Trond Myklebust
NFS client maintainer
NetApp
[email protected]
http://www.netapp.com
On Thu, Mar 13, 2008 at 04:36:06PM -0400, Trond Myklebust wrote:
>
> On Thu, 2008-03-13 at 15:43 -0400, J. Bruce Fields wrote:
> > On Thu, Mar 13, 2008 at 03:25:38PM -0400, Trond Myklebust wrote:
> > >
> > > On Thu, 2008-03-13 at 15:19 -0400, Trond Myklebust wrote:
> > > > On Thu, 2008-03-13 at 15:11 -0400, Olga Kornievskaia wrote:
> > > > > Trond,
> > > > >
> > > > > We were thinking of using RPCAUTH_LOOKUP_ROOTCREDS flag to acquire
> > > > > machine creds for authenticated callback.
> > > >
> > > > I'd strongly suggest using a different flag for that purpose. The
> > > > function of RPCAUTH_LOOKUP_ROOTCREDS _today_ is to allow a future
> > > > swap-over-nfs to use root credentials when paging out memory.
> > > >
> > > > That is not the same as machine creds...
> > >
> > >
> > > In fact, I'd strongly urge you to add the information for machine creds
> > > to the 'struct auth_cred' instead. The latter is the lookup key for
> > > 'rpc_authops->lookup_cred()'.
> >
> > So something like
> >
> > diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
> > index 7a69ca3..d624169 100644
> > --- a/include/linux/sunrpc/auth.h
> > +++ b/include/linux/sunrpc/auth.h
> > @@ -26,6 +26,7 @@ struct auth_cred {
> > uid_t uid;
> > gid_t gid;
> > struct group_info *group_info;
> > + int is_machine_cred;
> > };
>
> Yes. Unless we need more information. Will we ever be in a situation
> where we want to specify a full
A full principal name, or something? No, I can't think of a reason for
now.
> > ? No objection, but it seems like mild overkill for a single bit when
> > the necessary functions already take a flag parameter.
>
> The flag parameter should be for internal use by the RPC auth code to
> pass around lookup intent information (see gss_match(), which is the
> only remaining callback that actually uses that parameter).
>
> The lookup key, OTOH, should be a single structure.
>
> > Or do you want
> > this information in that structure for future uses of the auth_cred as a
> > key into a more general cred cache?
>
> See the rest of the patch series...
Whoops, OK, will do.
> Basically, if the administrator changes our security landscape on the
> fly, either by migrating our filesystem, or by changing the supported
> security flavours, then we're going to have to renegotiate security on
> the fly. We really do _not_ want the NFS structures to cache RPC
> credentials which are tied to a particular auth mechanism or rpcsec_gss
> session.
> The "generic" credential I'm proposing is the tool for carrying all the
> information that is needed in order to lookup/create an on-the-wire
> credential at the instant when we issue the RPC call.
Got it; putting the information that we want a host credential into the
auth_cred sounds right. Thanks!
--b.