2013-05-14 21:12:55

by J. Bruce Fields

[permalink] [raw]
Subject:

The following is a minimal SP4_MACH_CRED implementation. I plan to
queue it up for 3.11.

It could use some testing at least, though.

--b.



2013-05-14 21:12:56

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 3/3] nfsd4: implement minimal SP4_MACH_CRED

From: "J. Bruce Fields" <[email protected]>

Do a minimal SP4_MACH_CRED implementation suggested by Trond, ignoring
the client-provided spo_must_* arrays and just enforcing credential
checks for the minimum required operations.

Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 66 +++++++++++++++++++++++++++++++++++++++++----------
fs/nfsd/nfs4xdr.c | 22 +++++++++++++++++
fs/nfsd/state.h | 1 +
3 files changed, 77 insertions(+), 12 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 52f9e92..4e50a2d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1265,6 +1265,23 @@ same_creds(struct svc_cred *cr1, struct svc_cred *cr2)
return 0 == strcmp(cr1->cr_principal, cr2->cr_principal);
}

+static bool mach_creds_match(struct nfs4_client *cl, struct svc_rqst *rqstp)
+{
+ struct svc_cred *cr = &rqstp->rq_cred;
+ u32 service;
+
+ if (!cl->cl_mach_cred)
+ return true;
+ if (cl->cl_cred.cr_gss_mech != cr->cr_gss_mech)
+ return false;
+ service = gss_pseudoflavor_to_service(cr->cr_gss_mech, cr->cr_flavor);
+ if (service != RPC_AUTH_GSS_KRB5I && service != RPC_AUTH_GSS_KRB5P)
+ return false;
+ if (!cr->cr_principal)
+ return false;
+ return 0 == strcmp(cl->cl_cred.cr_principal, cr->cr_principal);
+}
+
static void gen_clid(struct nfs4_client *clp, struct nfsd_net *nn)
{
static u32 current_clientid = 1;
@@ -1642,16 +1659,14 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
if (exid->flags & ~EXCHGID4_FLAG_MASK_A)
return nfserr_inval;

- /* Currently only support SP4_NONE */
switch (exid->spa_how) {
+ case SP4_MACH_CRED:
case SP4_NONE:
break;
default: /* checked by xdr code */
WARN_ON_ONCE(1);
case SP4_SSV:
return nfserr_encr_alg_unsupp;
- case SP4_MACH_CRED:
- return nfserr_serverfault; /* no excuse :-/ */
}

/* Cases below refer to rfc 5661 section 18.35.4: */
@@ -1666,6 +1681,10 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
status = nfserr_inval;
goto out;
}
+ if (!mach_creds_match(conf, rqstp)) {
+ status = nfserr_wrong_cred;
+ goto out;
+ }
if (!creds_match) { /* case 9 */
status = nfserr_perm;
goto out;
@@ -1713,6 +1732,7 @@ out_new:
goto out;
}
new->cl_minorversion = cstate->minorversion;
+ new->cl_mach_cred = (exid->spa_how == SP4_MACH_CRED);

gen_clid(new, nn);
add_to_unconfirmed(new);
@@ -1877,6 +1897,9 @@ nfsd4_create_session(struct svc_rqst *rqstp,
WARN_ON_ONCE(conf && unconf);

if (conf) {
+ status = nfserr_wrong_cred;
+ if (!mach_creds_match(conf, rqstp))
+ goto out_free_conn;
cs_slot = &conf->cl_cs_slot;
status = check_slot_seqid(cr_ses->seqid, cs_slot->sl_seqid, 0);
if (status == nfserr_replay_cache) {
@@ -1893,6 +1916,9 @@ nfsd4_create_session(struct svc_rqst *rqstp,
status = nfserr_clid_inuse;
goto out_free_conn;
}
+ status = nfserr_wrong_cred;
+ if (!mach_creds_match(unconf, rqstp))
+ goto out_free_conn;
cs_slot = &unconf->cl_cs_slot;
status = check_slot_seqid(cr_ses->seqid, cs_slot->sl_seqid, 0);
if (status) {
@@ -1989,6 +2015,9 @@ __be32 nfsd4_bind_conn_to_session(struct svc_rqst *rqstp,
status = nfserr_badsession;
if (!session)
goto out;
+ status = nfserr_wrong_cred;
+ if (!mach_creds_match(session->se_client, rqstp))
+ goto out;
status = nfsd4_map_bcts_dir(&bcts->dir);
if (status)
goto out;
@@ -2031,6 +2060,9 @@ nfsd4_destroy_session(struct svc_rqst *r,
status = nfserr_badsession;
if (!ses)
goto out_client_lock;
+ status = nfserr_wrong_cred;
+ if (!mach_creds_match(ses->se_client, r))
+ goto out_client_lock;
status = mark_session_dead_locked(ses);
if (status)
goto out_client_lock;
@@ -2061,26 +2093,31 @@ static struct nfsd4_conn *__nfsd4_find_conn(struct svc_xprt *xpt, struct nfsd4_s
return NULL;
}

-static void nfsd4_sequence_check_conn(struct nfsd4_conn *new, struct nfsd4_session *ses)
+static __be32 nfsd4_sequence_check_conn(struct nfsd4_conn *new, struct nfsd4_session *ses)
{
struct nfs4_client *clp = ses->se_client;
struct nfsd4_conn *c;
+ __be32 status = nfs_ok;
int ret;

spin_lock(&clp->cl_lock);
c = __nfsd4_find_conn(new->cn_xprt, ses);
- if (c) {
- spin_unlock(&clp->cl_lock);
- free_conn(new);
- return;
- }
+ if (c)
+ goto out_free;
+ status = nfserr_conn_not_bound_to_session;
+ if (clp->cl_mach_cred)
+ goto out_free;
__nfsd4_hash_conn(new, ses);
spin_unlock(&clp->cl_lock);
ret = nfsd4_register_conn(new);
if (ret)
/* oops; xprt is already down: */
nfsd4_conn_lost(&new->cn_xpt_user);
- return;
+ return nfs_ok;
+out_free:
+ spin_unlock(&clp->cl_lock);
+ free_conn(new);
+ return status;
}

static bool nfsd4_session_too_many_ops(struct svc_rqst *rqstp, struct nfsd4_session *session)
@@ -2172,8 +2209,10 @@ nfsd4_sequence(struct svc_rqst *rqstp,
if (status)
goto out_put_session;

- nfsd4_sequence_check_conn(conn, session);
+ status = nfsd4_sequence_check_conn(conn, session);
conn = NULL;
+ if (status)
+ goto out_put_session;

/* Success! bump slot seqid */
slot->sl_seqid = seq->seqid;
@@ -2235,7 +2274,10 @@ nfsd4_destroy_clientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *csta
status = nfserr_stale_clientid;
goto out;
}
-
+ if (!mach_creds_match(clp, rqstp)) {
+ status = nfserr_wrong_cred;
+ goto out;
+ }
expire_client(clp);
out:
nfs4_unlock_state();
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 9aeacdd..6b83465 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3227,6 +3227,14 @@ nfsd4_encode_write(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_w
return nfserr;
}

+static const u32 nfs4_minimal_spo_must_enforce[2] = {
+ [1] = 1 << (OP_BIND_CONN_TO_SESSION - 32) |
+ 1 << (OP_EXCHANGE_ID - 32) |
+ 1 << (OP_CREATE_SESSION - 32) |
+ 1 << (OP_DESTROY_SESSION - 32) |
+ 1 << (OP_DESTROY_CLIENTID - 32)
+};
+
static __be32
nfsd4_encode_exchange_id(struct nfsd4_compoundres *resp, __be32 nfserr,
struct nfsd4_exchange_id *exid)
@@ -3265,6 +3273,20 @@ nfsd4_encode_exchange_id(struct nfsd4_compoundres *resp, __be32 nfserr,
/* state_protect4_r. Currently only support SP4_NONE */
BUG_ON(exid->spa_how != SP4_NONE);
WRITE32(exid->spa_how);
+ switch (exid->spa_how) {
+ case SP4_NONE:
+ break;
+ case SP4_MACH_CRED:
+ /* spo_must_enforce bitmap: */
+ WRITE32(2);
+ WRITE32(nfs4_minimal_spo_must_enforce[0]);
+ WRITE32(nfs4_minimal_spo_must_enforce[1]);
+ /* empty spo_must_allow bitmap: */
+ WRITE32(0);
+ break;
+ default:
+ WARN_ON_ONCE(1);
+ }

/* The server_owner struct */
WRITE64(minor_id); /* Minor id */
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 274e2a1..424d8f5 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -246,6 +246,7 @@ struct nfs4_client {
nfs4_verifier cl_verifier; /* generated by client */
time_t cl_time; /* time of last lease renewal */
struct sockaddr_storage cl_addr; /* client ipaddress */
+ bool cl_mach_cred; /* SP4_MACH_CRED in force */
struct svc_cred cl_cred; /* setclientid principal */
clientid_t cl_clientid; /* generated by server */
nfs4_verifier cl_confirm; /* generated by server */
--
1.7.9.5


2013-05-14 21:46:12

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 3/3] nfsd4: implement minimal SP4_MACH_CRED

On Tue, May 14, 2013 at 05:12:53PM -0400, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <[email protected]>
>
> Do a minimal SP4_MACH_CRED implementation suggested by Trond, ignoring
> the client-provided spo_must_* arrays and just enforcing credential
> checks for the minimum required operations.

Reviewing myself:

>
> Signed-off-by: J. Bruce Fields <[email protected]>
> ---
> fs/nfsd/nfs4state.c | 66 +++++++++++++++++++++++++++++++++++++++++----------
> fs/nfsd/nfs4xdr.c | 22 +++++++++++++++++
> fs/nfsd/state.h | 1 +
> 3 files changed, 77 insertions(+), 12 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 52f9e92..4e50a2d 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1265,6 +1265,23 @@ same_creds(struct svc_cred *cr1, struct svc_cred *cr2)
> return 0 == strcmp(cr1->cr_principal, cr2->cr_principal);
> }
>
> +static bool mach_creds_match(struct nfs4_client *cl, struct svc_rqst *rqstp)
> +{
> + struct svc_cred *cr = &rqstp->rq_cred;
> + u32 service;
> +
> + if (!cl->cl_mach_cred)
> + return true;
> + if (cl->cl_cred.cr_gss_mech != cr->cr_gss_mech)
> + return false;
> + service = gss_pseudoflavor_to_service(cr->cr_gss_mech, cr->cr_flavor);
> + if (service != RPC_AUTH_GSS_KRB5I && service != RPC_AUTH_GSS_KRB5P)

Whoops, those constants are pseudoflavors, not service types.

Also, I'm assuming here that if cl_mach_cred is true then the client's
gss_mechanism and principal are non-null. But that's not necessarily
true since I forgot to enforce the requirement that the exchange_id also
be sent with integrity or privacy in the SP4_MACH_CRED case.

So, I need the following.

--b.

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 4e50a2d..293ffbe 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1265,17 +1265,25 @@ same_creds(struct svc_cred *cr1, struct svc_cred *cr2)
return 0 == strcmp(cr1->cr_principal, cr2->cr_principal);
}

-static bool mach_creds_match(struct nfs4_client *cl, struct svc_rqst *rqstp)
+static bool svc_rqst_integrity_protected(struct svc_rqst *rqstp)
{
struct svc_cred *cr = &rqstp->rq_cred;
u32 service;

+ service = gss_pseudoflavor_to_service(cr->cr_gss_mech, cr->cr_flavor);
+ return service == RPC_GSS_SVC_INTEGRITY ||
+ service == RPC_GSS_SVC_PRIVACY;
+}
+
+static bool mach_creds_match(struct nfs4_client *cl, struct svc_rqst *rqstp)
+{
+ struct svc_cred *cr = &rqstp->rq_cred;
+
if (!cl->cl_mach_cred)
return true;
if (cl->cl_cred.cr_gss_mech != cr->cr_gss_mech)
return false;
- service = gss_pseudoflavor_to_service(cr->cr_gss_mech, cr->cr_flavor);
- if (service != RPC_AUTH_GSS_KRB5I && service != RPC_AUTH_GSS_KRB5P)
+ if (!svc_rqst_integrity_protected(rqstp))
return false;
if (!cr->cr_principal)
return false;
@@ -1661,6 +1669,8 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,

switch (exid->spa_how) {
case SP4_MACH_CRED:
+ if (!svc_rqst_integrity_protected(rqstp))
+ return nfserr_inval;
case SP4_NONE:
break;
default: /* checked by xdr code */

2013-05-14 21:12:55

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 2/3] svcrpc: store gss mech in svc_cred

From: "J. Bruce Fields" <[email protected]>

Store a pointer to the gss mechanism used in the rq_cred and cl_cred.
This will make it easier to enforce SP4_MACH_CRED, which needs to
compare the mechanism used on the exchange_id with that used on
protected operations.

Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 3 +++
include/linux/sunrpc/gss_api.h | 2 ++
include/linux/sunrpc/svcauth.h | 4 ++++
net/sunrpc/auth_gss/gss_mech_switch.c | 5 ++++-
net/sunrpc/auth_gss/svcauth_gss.c | 4 +---
5 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 91ead0e..52f9e92 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1188,6 +1188,9 @@ static int copy_cred(struct svc_cred *target, struct svc_cred *source)
target->cr_gid = source->cr_gid;
target->cr_group_info = source->cr_group_info;
get_group_info(target->cr_group_info);
+ target->cr_gss_mech = source->cr_gss_mech;
+ if (source->cr_gss_mech)
+ gss_mech_get(source->cr_gss_mech);
return 0;
}

diff --git a/include/linux/sunrpc/gss_api.h b/include/linux/sunrpc/gss_api.h
index 161463e..1f911cc 100644
--- a/include/linux/sunrpc/gss_api.h
+++ b/include/linux/sunrpc/gss_api.h
@@ -151,6 +151,8 @@ struct gss_api_mech *gss_mech_get_by_pseudoflavor(u32);
/* Fill in an array with a list of supported pseudoflavors */
int gss_mech_list_pseudoflavors(rpc_authflavor_t *, int);

+struct gss_api_mech * gss_mech_get(struct gss_api_mech *);
+
/* For every successful gss_mech_get or gss_mech_get_by_* call there must be a
* corresponding call to gss_mech_put. */
void gss_mech_put(struct gss_api_mech *);
diff --git a/include/linux/sunrpc/svcauth.h b/include/linux/sunrpc/svcauth.h
index 95c9566..8d71d65 100644
--- a/include/linux/sunrpc/svcauth.h
+++ b/include/linux/sunrpc/svcauth.h
@@ -14,6 +14,7 @@
#include <linux/string.h>
#include <linux/sunrpc/msg_prot.h>
#include <linux/sunrpc/cache.h>
+#include <linux/sunrpc/gss_api.h>
#include <linux/hash.h>
#include <linux/cred.h>

@@ -23,6 +24,7 @@ struct svc_cred {
struct group_info *cr_group_info;
u32 cr_flavor; /* pseudoflavor */
char *cr_principal; /* for gss */
+ struct gss_api_mech *cr_gss_mech;
};

static inline void init_svc_cred(struct svc_cred *cred)
@@ -37,6 +39,8 @@ static inline void free_svc_cred(struct svc_cred *cred)
if (cred->cr_group_info)
put_group_info(cred->cr_group_info);
kfree(cred->cr_principal);
+ gss_mech_put(cred->cr_gss_mech);
+ init_svc_cred(cred);
}

struct svc_rqst; /* forward decl */
diff --git a/net/sunrpc/auth_gss/gss_mech_switch.c b/net/sunrpc/auth_gss/gss_mech_switch.c
index defa9d3..27ce262 100644
--- a/net/sunrpc/auth_gss/gss_mech_switch.c
+++ b/net/sunrpc/auth_gss/gss_mech_switch.c
@@ -139,11 +139,12 @@ void gss_mech_unregister(struct gss_api_mech *gm)
}
EXPORT_SYMBOL_GPL(gss_mech_unregister);

-static struct gss_api_mech *gss_mech_get(struct gss_api_mech *gm)
+struct gss_api_mech *gss_mech_get(struct gss_api_mech *gm)
{
__module_get(gm->gm_owner);
return gm;
}
+EXPORT_SYMBOL(gss_mech_get);

static struct gss_api_mech *
_gss_mech_get_by_name(const char *name)
@@ -360,6 +361,7 @@ gss_pseudoflavor_to_service(struct gss_api_mech *gm, u32 pseudoflavor)
}
return 0;
}
+EXPORT_SYMBOL(gss_pseudoflavor_to_service);

char *
gss_service_to_auth_domain_name(struct gss_api_mech *gm, u32 service)
@@ -379,6 +381,7 @@ gss_mech_put(struct gss_api_mech * gm)
if (gm)
module_put(gm->gm_owner);
}
+EXPORT_SYMBOL(gss_mech_put);

/* The mech could probably be determined from the token instead, but it's just
* as easy for now to pass it in. */
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index c59f875..9f0f017 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -391,7 +391,6 @@ update_rsc(struct cache_head *cnew, struct cache_head *ctmp)
memset(&new->seqdata, 0, sizeof(new->seqdata));
spin_lock_init(&new->seqdata.sd_lock);
new->cred = tmp->cred;
- new->cred.cr_principal = tmp->cred.cr_principal;
init_svc_cred(&tmp->cred);
}

@@ -485,7 +484,7 @@ static int rsc_parse(struct cache_detail *cd,
len = qword_get(&mesg, buf, mlen);
if (len < 0)
goto out;
- gm = gss_mech_get_by_name(buf);
+ gm = rsci.cred.cr_gss_mech = gss_mech_get_by_name(buf);
status = -EOPNOTSUPP;
if (!gm)
goto out;
@@ -515,7 +514,6 @@ static int rsc_parse(struct cache_detail *cd,
rscp = rsc_update(cd, &rsci, rscp);
status = 0;
out:
- gss_mech_put(gm);
rsc_free(&rsci);
if (rscp)
cache_put(&rscp->h, cd);
--
1.7.9.5


2013-05-14 21:12:55

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 1/3] svcrpc: introduce init_svc_cred

From: "J. Bruce Fields" <[email protected]>

Common helper to zero out fields of the svc_cred.

Signed-off-by: J. Bruce Fields <[email protected]>
---
include/linux/sunrpc/svcauth.h | 7 +++++++
net/sunrpc/auth_gss/svcauth_gss.c | 6 ++----
2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/include/linux/sunrpc/svcauth.h b/include/linux/sunrpc/svcauth.h
index ff374ab..95c9566 100644
--- a/include/linux/sunrpc/svcauth.h
+++ b/include/linux/sunrpc/svcauth.h
@@ -25,6 +25,13 @@ struct svc_cred {
char *cr_principal; /* for gss */
};

+static inline void init_svc_cred(struct svc_cred *cred)
+{
+ cred->cr_group_info = NULL;
+ cred->cr_principal = NULL;
+ cred->cr_gss_mech = NULL;
+}
+
static inline void free_svc_cred(struct svc_cred *cred)
{
if (cred->cr_group_info)
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 871c73c..c59f875 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -377,8 +377,7 @@ rsc_init(struct cache_head *cnew, struct cache_head *ctmp)
new->handle.data = tmp->handle.data;
tmp->handle.data = NULL;
new->mechctx = NULL;
- new->cred.cr_group_info = NULL;
- new->cred.cr_principal = NULL;
+ init_svc_cred(&new->cred);
}

static void
@@ -392,9 +391,8 @@ update_rsc(struct cache_head *cnew, struct cache_head *ctmp)
memset(&new->seqdata, 0, sizeof(new->seqdata));
spin_lock_init(&new->seqdata.sd_lock);
new->cred = tmp->cred;
- tmp->cred.cr_group_info = NULL;
new->cred.cr_principal = tmp->cred.cr_principal;
- tmp->cred.cr_principal = NULL;
+ init_svc_cred(&tmp->cred);
}

static struct cache_head *
--
1.7.9.5