* 6 svcauth_unix patches to make export table lookups optional [not found] <20040916230555.GA13415@fieldses.org> @ 2004-09-16 23:07 ` J. Bruce Fields 2004-09-16 23:16 ` [PATCH 1 of 6] svcrpc: auth_null fixes J. Bruce Fields 0 siblings, 1 reply; 22+ messages in thread From: J. Bruce Fields @ 2004-09-16 23:07 UTC (permalink / raw) To: Neil Brown; +Cc: nfs, Trond Myklebust Currently lockd and nfs4 callbacks have the problem that the svcauth_unix and svcauth_null code insists on always doing the ip_map_lookup stuff for all rpc services. Of the six following patches, the first two are just cleanup, and should be OK to apply now. The third through sixth are first drafts posted to give other people a chance to look them over--they'll need review at least from Trond. In particular, I haven't actually tested either the lockd or the nfs4 callbacks. ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1 of 6] svcrpc: auth_null fixes 2004-09-16 23:07 ` 6 svcauth_unix patches to make export table lookups optional J. Bruce Fields @ 2004-09-16 23:16 ` J. Bruce Fields 2004-09-16 23:16 ` [PATCH 2 of 6] svcrpc: share code duplicated between auth_unix and auth_null J. Bruce Fields 0 siblings, 1 reply; 22+ messages in thread From: J. Bruce Fields @ 2004-09-16 23:16 UTC (permalink / raw) To: Neil Brown; +Cc: nfs, Trond Myklebust Fix some discrepencies between the server-side auth_null and auth_unix rpc code: in particular, make sure we return an auth error in the auth_null case instead of dropping when we fail to match an export entry, and make sure such responses are encoded correctly. Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu> --- linux-2.6.9-rc2-bfields/net/sunrpc/svcauth_unix.c | 30 +++++++++++++--------- 1 files changed, 18 insertions(+), 12 deletions(-) diff -puN net/sunrpc/svcauth_unix.c~svcrpc_auth_null_fixes net/sunrpc/svcauth_unix.c --- linux-2.6.9-rc2/net/sunrpc/svcauth_unix.c~svcrpc_auth_null_fixes 2004-09-16 15:40:33.000000000 -0400 +++ linux-2.6.9-rc2-bfields/net/sunrpc/svcauth_unix.c 2004-09-16 16:38:26.000000000 -0400 @@ -335,9 +335,13 @@ svcauth_null_accept(struct svc_rqst *rqs { struct kvec *argv = &rqstp->rq_arg.head[0]; struct kvec *resv = &rqstp->rq_res.head[0]; + struct svc_cred *cred = &rqstp->rq_cred; int rv=0; struct ip_map key, *ipm; + cred->cr_group_info = NULL; + rqstp->rq_client = NULL; + if (argv->iov_len < 3*4) return SVC_GARBAGE; @@ -353,23 +357,17 @@ svcauth_null_accept(struct svc_rqst *rqs } /* Signal that mapping to nobody uid/gid is required */ - rqstp->rq_cred.cr_uid = (uid_t) -1; - rqstp->rq_cred.cr_gid = (gid_t) -1; - rqstp->rq_cred.cr_group_info = groups_alloc(0); - if (rqstp->rq_cred.cr_group_info == NULL) + cred->cr_uid = (uid_t) -1; + cred->cr_gid = (gid_t) -1; + cred->cr_group_info = groups_alloc(0); + if (cred->cr_group_info == NULL) return SVC_DROP; /* kmalloc failure - client must retry */ - /* Put NULL verifier */ - svc_putu32(resv, RPC_AUTH_NULL); - svc_putu32(resv, 0); - strcpy(key.m_class, rqstp->rq_server->sv_program->pg_class); key.m_addr = rqstp->rq_addr.sin_addr; ipm = ip_map_lookup(&key, 0); - rqstp->rq_client = NULL; - if (ipm) switch (cache_check(&ip_map_cache, &ipm->h, &rqstp->rq_chandle)) { case -EAGAIN: @@ -388,10 +386,18 @@ svcauth_null_accept(struct svc_rqst *rqs } else rv = SVC_DROP; - if (rqstp->rq_client == NULL && rqstp->rq_proc != 0) - *authp = rpc_autherr_badcred; + if (rv == SVC_OK && rqstp->rq_client == NULL && rqstp->rq_proc != 0) + goto badcred; + + /* Put NULL verifier */ + svc_putu32(resv, RPC_AUTH_NULL); + svc_putu32(resv, 0); return rv; + +badcred: + *authp = rpc_autherr_badcred; + return SVC_DENIED; } static int _ ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2 of 6] svcrpc: share code duplicated between auth_unix and auth_null 2004-09-16 23:16 ` [PATCH 1 of 6] svcrpc: auth_null fixes J. Bruce Fields @ 2004-09-16 23:16 ` J. Bruce Fields 2004-09-16 23:16 ` [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method J. Bruce Fields 0 siblings, 1 reply; 22+ messages in thread From: J. Bruce Fields @ 2004-09-16 23:16 UTC (permalink / raw) To: Neil Brown; +Cc: nfs, Trond Myklebust Call a helper function from svcauth_unix_accept() and svcauth_null_accept() instead of duplicating code. Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu> --- linux-2.6.9-rc2-bfields/net/sunrpc/svcauth_unix.c | 89 +++++++++------------- 1 files changed, 37 insertions(+), 52 deletions(-) diff -puN net/sunrpc/svcauth_unix.c~svcrpc_unix_ip_mapping_cleanup net/sunrpc/svcauth_unix.c --- linux-2.6.9-rc2/net/sunrpc/svcauth_unix.c~svcrpc_unix_ip_mapping_cleanup 2004-09-16 16:40:49.000000000 -0400 +++ linux-2.6.9-rc2-bfields/net/sunrpc/svcauth_unix.c 2004-09-16 16:40:49.000000000 -0400 @@ -329,6 +329,39 @@ void svcauth_unix_purge(void) cache_purge(&auth_domain_cache); } +int +svcauth_unix_set_client(struct svc_rqst *rqstp) +{ + struct ip_map key, *ipm; + + rqstp->rq_client = NULL; + if (rqstp->rq_proc == 0) + return SVC_OK; + + strcpy(key.m_class, rqstp->rq_server->sv_program->pg_class); + key.m_addr = rqstp->rq_addr.sin_addr; + + ipm = ip_map_lookup(&key, 0); + + if (ipm == NULL) + return SVC_DENIED; + + switch (cache_check(&ip_map_cache, &ipm->h, &rqstp->rq_chandle)) { + case -EAGAIN: + return SVC_DROP; + case -ENOENT: + return SVC_DENIED; + case 0: + rqstp->rq_client = &ipm->m_client->h; + cache_get(&rqstp->rq_client->h); + ip_map_put(&ipm->h, &ip_map_cache); + return SVC_OK; + default: + BUG(); + } + /* shut up gcc: */ + return -1; +} static int svcauth_null_accept(struct svc_rqst *rqstp, u32 *authp) @@ -337,7 +370,6 @@ svcauth_null_accept(struct svc_rqst *rqs struct kvec *resv = &rqstp->rq_res.head[0]; struct svc_cred *cred = &rqstp->rq_cred; int rv=0; - struct ip_map key, *ipm; cred->cr_group_info = NULL; rqstp->rq_client = NULL; @@ -363,30 +395,8 @@ svcauth_null_accept(struct svc_rqst *rqs if (cred->cr_group_info == NULL) return SVC_DROP; /* kmalloc failure - client must retry */ - strcpy(key.m_class, rqstp->rq_server->sv_program->pg_class); - key.m_addr = rqstp->rq_addr.sin_addr; - - ipm = ip_map_lookup(&key, 0); - - if (ipm) - switch (cache_check(&ip_map_cache, &ipm->h, &rqstp->rq_chandle)) { - case -EAGAIN: - rv = SVC_DROP; - break; - case -ENOENT: - rv = SVC_OK; /* rq_client is NULL */ - break; - case 0: - rqstp->rq_client = &ipm->m_client->h; - cache_get(&rqstp->rq_client->h); - ip_map_put(&ipm->h, &ip_map_cache); - rv = SVC_OK; - break; - default: BUG(); - } - else rv = SVC_DROP; - - if (rv == SVC_OK && rqstp->rq_client == NULL && rqstp->rq_proc != 0) + rv = svcauth_unix_set_client(rqstp); + if (rv == SVC_DENIED) goto badcred; /* Put NULL verifier */ @@ -432,7 +442,6 @@ svcauth_unix_accept(struct svc_rqst *rqs u32 slen, i; int len = argv->iov_len; int rv=0; - struct ip_map key, *ipm; cred->cr_group_info = NULL; rqstp->rq_client = NULL; @@ -464,32 +473,8 @@ svcauth_unix_accept(struct svc_rqst *rqs return SVC_DENIED; } - - strcpy(key.m_class, rqstp->rq_server->sv_program->pg_class); - key.m_addr = rqstp->rq_addr.sin_addr; - - - ipm = ip_map_lookup(&key, 0); - - if (ipm) - switch (cache_check(&ip_map_cache, &ipm->h, &rqstp->rq_chandle)) { - case -EAGAIN: - rv = SVC_DROP; - break; - case -ENOENT: - rv = SVC_OK; /* rq_client is NULL */ - break; - case 0: - rqstp->rq_client = &ipm->m_client->h; - cache_get(&rqstp->rq_client->h); - ip_map_put(&ipm->h, &ip_map_cache); - rv = SVC_OK; - break; - default: BUG(); - } - else rv = SVC_DROP; - - if (rv == SVC_OK && rqstp->rq_client == NULL && rqstp->rq_proc != 0) + rv = svcauth_unix_set_client(rqstp); + if (rv == SVC_DENIED) goto badcred; /* Put NULL verifier */ _ ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method 2004-09-16 23:16 ` [PATCH 2 of 6] svcrpc: share code duplicated between auth_unix and auth_null J. Bruce Fields @ 2004-09-16 23:16 ` J. Bruce Fields 2004-09-16 23:16 ` [PATCH 4 of 6] nfs4: use new pg_set_client method to simplify nfs4 callback authentication J. Bruce Fields ` (2 more replies) 0 siblings, 3 replies; 22+ messages in thread From: J. Bruce Fields @ 2004-09-16 23:16 UTC (permalink / raw) To: Neil Brown; +Cc: nfs, Trond Myklebust svcauth_null_accept() and svcauth_unix_accept() are currently hard-wired to check the source ip address on an incoming request against the export table, which make sense for nfsd but not necessarily for other rpc-based services. So instead we have svcauth_null_accept() and svcauth_unix_accept() call a program-specific pg_add_client() method. Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu> --- linux-2.6.9-rc2-bfields/fs/lockd/svc.c | 11 +++++++++++ linux-2.6.9-rc2-bfields/fs/nfsd/nfssvc.c | 11 +++++++++++ linux-2.6.9-rc2-bfields/include/linux/sunrpc/svc.h | 1 + linux-2.6.9-rc2-bfields/include/linux/sunrpc/svcauth.h | 2 ++ linux-2.6.9-rc2-bfields/net/sunrpc/svcauth_unix.c | 8 ++------ 5 files changed, 27 insertions(+), 6 deletions(-) diff -puN net/sunrpc/svcauth_unix.c~svcrpc_unix_ip_mapping_method net/sunrpc/svcauth_unix.c --- linux-2.6.9-rc2/net/sunrpc/svcauth_unix.c~svcrpc_unix_ip_mapping_method 2004-09-16 16:40:53.000000000 -0400 +++ linux-2.6.9-rc2-bfields/net/sunrpc/svcauth_unix.c 2004-09-16 16:40:53.000000000 -0400 @@ -334,10 +334,6 @@ svcauth_unix_set_client(struct svc_rqst { struct ip_map key, *ipm; - rqstp->rq_client = NULL; - if (rqstp->rq_proc == 0) - return SVC_OK; - strcpy(key.m_class, rqstp->rq_server->sv_program->pg_class); key.m_addr = rqstp->rq_addr.sin_addr; @@ -395,7 +391,7 @@ svcauth_null_accept(struct svc_rqst *rqs if (cred->cr_group_info == NULL) return SVC_DROP; /* kmalloc failure - client must retry */ - rv = svcauth_unix_set_client(rqstp); + rv = rqstp->rq_server->sv_program->pg_set_client(rqstp); if (rv == SVC_DENIED) goto badcred; @@ -473,7 +469,7 @@ svcauth_unix_accept(struct svc_rqst *rqs return SVC_DENIED; } - rv = svcauth_unix_set_client(rqstp); + rv = rqstp->rq_server->sv_program->pg_set_client(rqstp); if (rv == SVC_DENIED) goto badcred; diff -puN net/sunrpc/svc.c~svcrpc_unix_ip_mapping_method net/sunrpc/svc.c diff -puN include/linux/sunrpc/svc.h~svcrpc_unix_ip_mapping_method include/linux/sunrpc/svc.h --- linux-2.6.9-rc2/include/linux/sunrpc/svc.h~svcrpc_unix_ip_mapping_method 2004-09-16 16:40:53.000000000 -0400 +++ linux-2.6.9-rc2-bfields/include/linux/sunrpc/svc.h 2004-09-16 16:40:53.000000000 -0400 @@ -253,6 +253,7 @@ struct svc_program { struct svc_stat * pg_stats; /* rpc statistics */ /* Override authentication. NULL means use default */ int (*pg_authenticate)(struct svc_rqst *, u32 *); + int (*pg_set_client)(struct svc_rqst *); }; /* diff -L fs/nsfd/nfssvc.c -puN /dev/null /dev/null diff -puN fs/lockd/svc.c~svcrpc_unix_ip_mapping_method fs/lockd/svc.c --- linux-2.6.9-rc2/fs/lockd/svc.c~svcrpc_unix_ip_mapping_method 2004-09-16 16:40:53.000000000 -0400 +++ linux-2.6.9-rc2-bfields/fs/lockd/svc.c 2004-09-16 16:40:53.000000000 -0400 @@ -398,6 +398,16 @@ static int param_set_##name(const char * return 0; \ } +static int lockd_set_client(struct svc_rqst *rqstp) +{ + rqstp->rq_client = NULL; + if (rqstp->rq_proc == 0) /* XXX not quite right. */ + return SVC_OK; + else + return svcauth_unix_set_client(rqstp); +} + + param_set_min_max(port, int, simple_strtol, 0, 65535) param_set_min_max(grace_period, unsigned long, simple_strtoul, nlm_grace_period_min, nlm_grace_period_max) @@ -478,4 +488,5 @@ struct svc_program nlmsvc_program = { .pg_name = "lockd", /* service name */ .pg_class = "nfsd", /* share authentication with nfsd */ .pg_stats = &nlmsvc_stats, /* stats table */ + .pg_set_client = &lockd_set_client /* XXX export authentication */ }; diff -puN include/linux/sunrpc/svcauth.h~svcrpc_unix_ip_mapping_method include/linux/sunrpc/svcauth.h --- linux-2.6.9-rc2/include/linux/sunrpc/svcauth.h~svcrpc_unix_ip_mapping_method 2004-09-16 16:40:53.000000000 -0400 +++ linux-2.6.9-rc2-bfields/include/linux/sunrpc/svcauth.h 2004-09-16 16:40:53.000000000 -0400 @@ -119,6 +119,8 @@ extern struct auth_domain *auth_unix_loo extern int auth_unix_forget_old(struct auth_domain *dom); extern void svcauth_unix_purge(void); +extern int svcauth_unix_set_client(struct svc_rqst *); + static inline unsigned long hash_str(char *name, int bits) { unsigned long hash = 0; diff -puN fs/nfsd/nfssvc.c~svcrpc_unix_ip_mapping_method fs/nfsd/nfssvc.c --- linux-2.6.9-rc2/fs/nfsd/nfssvc.c~svcrpc_unix_ip_mapping_method 2004-09-16 16:40:53.000000000 -0400 +++ linux-2.6.9-rc2-bfields/fs/nfsd/nfssvc.c 2004-09-16 16:40:53.000000000 -0400 @@ -359,6 +359,15 @@ nfsd_dispatch(struct svc_rqst *rqstp, u3 return 1; } +static int nfsd_set_client(struct svc_rqst *rqstp) +{ + rqstp->rq_client = NULL; + if (rqstp->rq_proc == 0) + return SVC_OK; + else + return svcauth_unix_set_client(rqstp); +} + extern struct svc_version nfsd_version2, nfsd_version3, nfsd_version4; static struct svc_version * nfsd_version[] = { @@ -379,4 +388,6 @@ struct svc_program nfsd_program = { .pg_name = "nfsd", /* program name */ .pg_class = "nfsd", /* authentication class */ .pg_stats = &nfsd_svcstats, /* version table */ + .pg_set_client = nfsd_set_client, /* export authentication */ + }; _ ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4 of 6] nfs4: use new pg_set_client method to simplify nfs4 callback authentication 2004-09-16 23:16 ` [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method J. Bruce Fields @ 2004-09-16 23:16 ` J. Bruce Fields 2004-09-16 23:16 ` [PATCH 5 of 6] lockd: don't try to match callback requests against export table J. Bruce Fields 2004-09-16 23:38 ` [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method Trond Myklebust 2004-09-17 1:18 ` Trond Myklebust 2 siblings, 1 reply; 22+ messages in thread From: J. Bruce Fields @ 2004-09-16 23:16 UTC (permalink / raw) To: Neil Brown; +Cc: nfs, Trond Myklebust Use new pg_set_client method to simplify nfs4 callback authentication. This also has the effect of changing the error return from rejectedcred to badcred. I believe the change is correct. Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu> --- linux-2.6.9-rc2-bfields/fs/nfs/callback.c | 155 +----------------------------- 1 files changed, 8 insertions(+), 147 deletions(-) diff -puN fs/nfs/callback.c~nfs4_simplify_callback_auth fs/nfs/callback.c --- linux-2.6.9-rc2/fs/nfs/callback.c~nfs4_simplify_callback_auth 2004-09-16 16:18:54.000000000 -0400 +++ linux-2.6.9-rc2-bfields/fs/nfs/callback.c 2004-09-16 16:23:21.000000000 -0400 @@ -139,133 +139,10 @@ out: return ret; } -/* - * AUTH_NULL authentication - */ -static int nfs_callback_null_accept(struct svc_rqst *rqstp, u32 *authp) -{ - struct kvec *argv = &rqstp->rq_arg.head[0]; - struct kvec *resv = &rqstp->rq_res.head[0]; - - if (argv->iov_len < 3*4) - return SVC_GARBAGE; - - if (svc_getu32(argv) != 0) { - dprintk("svc: bad null cred\n"); - *authp = rpc_autherr_badcred; - return SVC_DENIED; - } - if (svc_getu32(argv) != RPC_AUTH_NULL || svc_getu32(argv) != 0) { - dprintk("svc: bad null verf\n"); - *authp = rpc_autherr_badverf; - return SVC_DENIED; - } - - /* Signal that mapping to nobody uid/gid is required */ - rqstp->rq_cred.cr_uid = (uid_t) -1; - rqstp->rq_cred.cr_gid = (gid_t) -1; - rqstp->rq_cred.cr_group_info = groups_alloc(0); - if (rqstp->rq_cred.cr_group_info == NULL) - return SVC_DROP; /* kmalloc failure - client must retry */ - - /* Put NULL verifier */ - svc_putu32(resv, RPC_AUTH_NULL); - svc_putu32(resv, 0); - dprintk("%s: success, returning %d!\n", __FUNCTION__, SVC_OK); - return SVC_OK; -} - -static int nfs_callback_null_release(struct svc_rqst *rqstp) -{ - if (rqstp->rq_cred.cr_group_info) - put_group_info(rqstp->rq_cred.cr_group_info); - rqstp->rq_cred.cr_group_info = NULL; - return 0; /* don't drop */ -} - -static struct auth_ops nfs_callback_auth_null = { - .name = "null", - .flavour = RPC_AUTH_NULL, - .accept = nfs_callback_null_accept, - .release = nfs_callback_null_release, -}; - -/* - * AUTH_SYS authentication - */ -static int nfs_callback_unix_accept(struct svc_rqst *rqstp, u32 *authp) -{ - struct kvec *argv = &rqstp->rq_arg.head[0]; - struct kvec *resv = &rqstp->rq_res.head[0]; - struct svc_cred *cred = &rqstp->rq_cred; - u32 slen, i; - int len = argv->iov_len; - - dprintk("%s: start\n", __FUNCTION__); - cred->cr_group_info = NULL; - rqstp->rq_client = NULL; - if ((len -= 3*4) < 0) - return SVC_GARBAGE; - - /* Get length, time stamp and machine name */ - svc_getu32(argv); - svc_getu32(argv); - slen = XDR_QUADLEN(ntohl(svc_getu32(argv))); - if (slen > 64 || (len -= (slen + 3)*4) < 0) - goto badcred; - argv->iov_base = (void*)((u32*)argv->iov_base + slen); - argv->iov_len -= slen*4; - - cred->cr_uid = ntohl(svc_getu32(argv)); - cred->cr_gid = ntohl(svc_getu32(argv)); - slen = ntohl(svc_getu32(argv)); - if (slen > 16 || (len -= (slen + 2)*4) < 0) - goto badcred; - cred->cr_group_info = groups_alloc(slen); - if (cred->cr_group_info == NULL) - return SVC_DROP; - for (i = 0; i < slen; i++) - GROUP_AT(cred->cr_group_info, i) = ntohl(svc_getu32(argv)); - - if (svc_getu32(argv) != RPC_AUTH_NULL || svc_getu32(argv) != 0) { - *authp = rpc_autherr_badverf; - return SVC_DENIED; - } - /* Put NULL verifier */ - svc_putu32(resv, RPC_AUTH_NULL); - svc_putu32(resv, 0); - dprintk("%s: success, returning %d!\n", __FUNCTION__, SVC_OK); - return SVC_OK; -badcred: - *authp = rpc_autherr_badcred; - return SVC_DENIED; -} - -static int nfs_callback_unix_release(struct svc_rqst *rqstp) -{ - if (rqstp->rq_cred.cr_group_info) - put_group_info(rqstp->rq_cred.cr_group_info); - rqstp->rq_cred.cr_group_info = NULL; - return 0; -} - -static struct auth_ops nfs_callback_auth_unix = { - .name = "unix", - .flavour = RPC_AUTH_UNIX, - .accept = nfs_callback_unix_accept, - .release = nfs_callback_unix_release, -}; - -/* - * Hook the authentication protocol - */ -static int nfs_callback_auth(struct svc_rqst *rqstp, u32 *authp) +static int nfs_callback_set_client(struct svc_rqst *rqstp) { struct in_addr *addr = &rqstp->rq_addr.sin_addr; struct nfs4_client *clp; - struct kvec *argv = &rqstp->rq_arg.head[0]; - int flavour; - int retval; /* Don't talk to strangers */ clp = nfs4_find_client(addr); @@ -273,34 +150,18 @@ static int nfs_callback_auth(struct svc_ return SVC_DROP; dprintk("%s: %u.%u.%u.%u NFSv4 callback!\n", __FUNCTION__, NIPQUAD(addr)); nfs4_put_client(clp); - flavour = ntohl(svc_getu32(argv)); - switch(flavour) { + switch (rqstp->rq_authop->flavour) { case RPC_AUTH_NULL: - if (rqstp->rq_proc != CB_NULL) { - *authp = rpc_autherr_tooweak; - retval = SVC_DENIED; - break; - } - rqstp->rq_authop = &nfs_callback_auth_null; - retval = nfs_callback_null_accept(rqstp, authp); + if (rqstp->rq_proc != CB_NULL) + return SVC_DENIED; break; case RPC_AUTH_UNIX: - /* Eat the authentication flavour */ - rqstp->rq_authop = &nfs_callback_auth_unix; - retval = nfs_callback_unix_accept(rqstp, authp); break; default: - /* FIXME: need to add RPCSEC_GSS upcalls */ -#if 0 - svc_ungetu32(argv); - retval = svc_authenticate(rqstp, authp); -#else - *authp = rpc_autherr_rejectedcred; - retval = SVC_DENIED; -#endif + /* FIXME: RPCSEC_GSS handling? */ + return SVC_DENIED; } - dprintk("%s: flavour %d returning error %d\n", __FUNCTION__, flavour, retval); - return retval; + return SVC_OK; } /* @@ -321,5 +182,5 @@ static struct svc_program nfs4_callback_ .pg_name = "NFSv4 callback", /* service name */ .pg_class = "nfs", /* authentication class */ .pg_stats = &nfs4_callback_stats, - .pg_authenticate = nfs_callback_auth, + .pg_set_client = nfs_callback_set_client, }; _ ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 5 of 6] lockd: don't try to match callback requests against export table 2004-09-16 23:16 ` [PATCH 4 of 6] nfs4: use new pg_set_client method to simplify nfs4 callback authentication J. Bruce Fields @ 2004-09-16 23:16 ` J. Bruce Fields 2004-09-16 23:16 ` [PATCH 6 of 6] nfsd: remove pg_authenticate field J. Bruce Fields 2004-09-16 23:34 ` [PATCH 5 of 6] lockd: don't try to match callback requests against export table Trond Myklebust 0 siblings, 2 replies; 22+ messages in thread From: J. Bruce Fields @ 2004-09-16 23:16 UTC (permalink / raw) To: Neil Brown; +Cc: nfs, Trond Myklebust On lockd callbacks, we're a client, and the source address is that of a server, so we shouldn't be trying to match the source address of the callback request against our export table. Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu> --- linux-2.6.9-rc2-bfields/fs/lockd/svc.c | 21 +++++++++++++++++++-- 1 files changed, 19 insertions(+), 2 deletions(-) diff -puN fs/lockd/svc.c~lockd_fix_authentication fs/lockd/svc.c --- linux-2.6.9-rc2/fs/lockd/svc.c~lockd_fix_authentication 2004-09-16 16:23:55.000000000 -0400 +++ linux-2.6.9-rc2-bfields/fs/lockd/svc.c 2004-09-16 16:23:55.000000000 -0400 @@ -398,12 +398,29 @@ static int param_set_##name(const char * return 0; \ } +static inline int is_callback(u32 proc) +{ + /* XXX: correct list? Add field to proc? */ + /* XXX: SMPROC_NOTIFY may be a special case. */ + /* XXX: double-check: make sure the procedure numbers make sense + * across all nlm versions. Check NOTIFY in particular. */ + return proc == NLMPROC_GRANTED + || proc == NLMPROC_TEST_RES + || proc == NLMPROC_LOCK_RES + || proc == NLMPROC_CANCEL_RES + || proc == NLMPROC_UNLOCK_RES + || proc == NLMPROC_GRANTED_RES; +} + static int lockd_set_client(struct svc_rqst *rqstp) { rqstp->rq_client = NULL; - if (rqstp->rq_proc == 0) /* XXX not quite right. */ + if (is_callback(rqstp->rq_proc)) { + /* XXX: how do I authenticate callbacks? + * Call nlmsvc_lookup_host(rqstp), or just leave that + * to the individual procedures? */ return SVC_OK; - else + } else return svcauth_unix_set_client(rqstp); } _ ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 6 of 6] nfsd: remove pg_authenticate field 2004-09-16 23:16 ` [PATCH 5 of 6] lockd: don't try to match callback requests against export table J. Bruce Fields @ 2004-09-16 23:16 ` J. Bruce Fields 2004-09-16 23:34 ` [PATCH 5 of 6] lockd: don't try to match callback requests against export table Trond Myklebust 1 sibling, 0 replies; 22+ messages in thread From: J. Bruce Fields @ 2004-09-16 23:16 UTC (permalink / raw) To: Neil Brown; +Cc: nfs, Trond Myklebust The pg_authenticate callback was only being used by the nfs4 client callback code to circumvent the svcauth_unix code's insistence on checking all requests against the export table. With that problem solved, we no longer need pg_authenticate. Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu> --- linux-2.6.9-rc2-bfields/include/linux/sunrpc/svc.h | 1 - linux-2.6.9-rc2-bfields/net/sunrpc/svc.c | 5 +---- 2 files changed, 1 insertion(+), 5 deletions(-) diff -puN net/sunrpc/svc.c~nfsd_remove_pg_authenticate net/sunrpc/svc.c --- linux-2.6.9-rc2/net/sunrpc/svc.c~nfsd_remove_pg_authenticate 2004-09-16 16:29:22.000000000 -0400 +++ linux-2.6.9-rc2-bfields/net/sunrpc/svc.c 2004-09-16 16:29:22.000000000 -0400 @@ -311,10 +311,7 @@ svc_process(struct svc_serv *serv, struc * We do this before anything else in order to get a decent * auth verifier. */ - if (progp->pg_authenticate != NULL) - auth_res = progp->pg_authenticate(rqstp, &auth_stat); - else - auth_res = svc_authenticate(rqstp, &auth_stat); + auth_res = svc_authenticate(rqstp, &auth_stat); switch (auth_res) { case SVC_OK: break; diff -puN include/linux/sunrpc/svc.h~nfsd_remove_pg_authenticate include/linux/sunrpc/svc.h --- linux-2.6.9-rc2/include/linux/sunrpc/svc.h~nfsd_remove_pg_authenticate 2004-09-16 16:29:22.000000000 -0400 +++ linux-2.6.9-rc2-bfields/include/linux/sunrpc/svc.h 2004-09-16 16:29:22.000000000 -0400 @@ -252,7 +252,6 @@ struct svc_program { char * pg_class; /* class name: services sharing authentication */ struct svc_stat * pg_stats; /* rpc statistics */ /* Override authentication. NULL means use default */ - int (*pg_authenticate)(struct svc_rqst *, u32 *); int (*pg_set_client)(struct svc_rqst *); }; _ ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 5 of 6] lockd: don't try to match callback requests against export table 2004-09-16 23:16 ` [PATCH 5 of 6] lockd: don't try to match callback requests against export table J. Bruce Fields 2004-09-16 23:16 ` [PATCH 6 of 6] nfsd: remove pg_authenticate field J. Bruce Fields @ 2004-09-16 23:34 ` Trond Myklebust 2004-09-24 3:55 ` Neil Brown 1 sibling, 1 reply; 22+ messages in thread From: Trond Myklebust @ 2004-09-16 23:34 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Neil Brown, nfs P=E5 to , 16/09/2004 klokka 19:16, skreiv J. Bruce Fields: > + /* XXX: correct list? Add field to proc? */ > + /* XXX: SMPROC_NOTIFY may be a special case. */ > + /* XXX: double-check: make sure the procedure numbers make sense > + * across all nlm versions. Check NOTIFY in particular. */ > + return proc =3D=3D NLMPROC_GRANTED > + || proc =3D=3D NLMPROC_TEST_RES > + || proc =3D=3D NLMPROC_LOCK_RES > + || proc =3D=3D NLMPROC_CANCEL_RES > + || proc =3D=3D NLMPROC_UNLOCK_RES > + || proc =3D=3D NLMPROC_GRANTED_RES; || proc =3D=3D NLMPROC_NSM_NOTIFY Cheers, Trond ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 5 of 6] lockd: don't try to match callback requests against export table 2004-09-16 23:34 ` [PATCH 5 of 6] lockd: don't try to match callback requests against export table Trond Myklebust @ 2004-09-24 3:55 ` Neil Brown 0 siblings, 0 replies; 22+ messages in thread From: Neil Brown @ 2004-09-24 3:55 UTC (permalink / raw) To: Trond Myklebust; +Cc: J. Bruce Fields, nfs On Thursday September 16, trond.myklebust@fys.uio.no wrote: > P=E5 to , 16/09/2004 klokka 19:16, skreiv J. Bruce Fields: > > +=09/* XXX: correct list? Add field to proc? */ > > +=09/* XXX: SMPROC_NOTIFY may be a special case. */ > > +=09/* XXX: double-check: make sure the procedure numbers make sens= e > > +=09 * across all nlm versions. Check NOTIFY in particular. */ > > +=09return proc =3D=3D NLMPROC_GRANTED > > +=09=09|| proc =3D=3D NLMPROC_TEST_RES > > +=09=09|| proc =3D=3D NLMPROC_LOCK_RES > > +=09=09|| proc =3D=3D NLMPROC_CANCEL_RES > > +=09=09|| proc =3D=3D NLMPROC_UNLOCK_RES > > +=09=09|| proc =3D=3D NLMPROC_GRANTED_RES; >=20 > || proc =3D=3D NLMPROC_NSM_NOTIFY || proc =3D=3D NLMPROC_GRANTED_MSG which is the case that is actually causing problems.....(at least in my testing) :-) NeilBrown >=20 > Cheers, > Trond ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method 2004-09-16 23:16 ` [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method J. Bruce Fields 2004-09-16 23:16 ` [PATCH 4 of 6] nfs4: use new pg_set_client method to simplify nfs4 callback authentication J. Bruce Fields @ 2004-09-16 23:38 ` Trond Myklebust 2004-09-17 1:11 ` J. Bruce Fields 2004-09-17 1:18 ` Trond Myklebust 2 siblings, 1 reply; 22+ messages in thread From: Trond Myklebust @ 2004-09-16 23:38 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Neil Brown, nfs P=E5 to , 16/09/2004 klokka 19:16, skreiv J. Bruce Fields: > diff -L fs/nsfd/nfssvc.c -puN /dev/null /dev/null > diff -puN fs/lockd/svc.c~svcrpc_unix_ip_mapping_method fs/lockd/svc.c > --- linux-2.6.9-rc2/fs/lockd/svc.c~svcrpc_unix_ip_mapping_method 2004-09-= 16 16:40:53.000000000 -0400 > +++ linux-2.6.9-rc2-bfields/fs/lockd/svc.c 2004-09-16 16:40:53.000000000 = -0400 > @@ -398,6 +398,16 @@ static int param_set_##name(const char * > return 0; \ > } > =20 > +static int lockd_set_client(struct svc_rqst *rqstp) > +{ > + rqstp->rq_client =3D NULL; > + if (rqstp->rq_proc =3D=3D 0) /* XXX not quite right. */ > + return SVC_OK; > + else > + return svcauth_unix_set_client(rqstp); > +} > + Is svcauth_unix_set_client() actually exported anywhere? Cheers, Trond ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method 2004-09-16 23:38 ` [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method Trond Myklebust @ 2004-09-17 1:11 ` J. Bruce Fields 0 siblings, 0 replies; 22+ messages in thread From: J. Bruce Fields @ 2004-09-17 1:11 UTC (permalink / raw) To: Trond Myklebust; +Cc: Neil Brown, nfs On Thu, Sep 16, 2004 at 07:38:51PM -0400, Trond Myklebust wrote: > På to , 16/09/2004 klokka 19:16, skreiv J. Bruce Fields: > > > diff -L fs/nsfd/nfssvc.c -puN /dev/null /dev/null > > diff -puN fs/lockd/svc.c~svcrpc_unix_ip_mapping_method fs/lockd/svc.c > > --- linux-2.6.9-rc2/fs/lockd/svc.c~svcrpc_unix_ip_mapping_method 2004-09-16 16:40:53.000000000 -0400 > > +++ linux-2.6.9-rc2-bfields/fs/lockd/svc.c 2004-09-16 16:40:53.000000000 -0400 > > @@ -398,6 +398,16 @@ static int param_set_##name(const char * > > return 0; \ > > } > > > > +static int lockd_set_client(struct svc_rqst *rqstp) > > +{ > > + rqstp->rq_client = NULL; > > + if (rqstp->rq_proc == 0) /* XXX not quite right. */ > > + return SVC_OK; > > + else > > + return svcauth_unix_set_client(rqstp); > > +} > > + > > Is svcauth_unix_set_client() actually exported anywhere? Nope! Fixed, thanks.--b. ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method 2004-09-16 23:16 ` [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method J. Bruce Fields 2004-09-16 23:16 ` [PATCH 4 of 6] nfs4: use new pg_set_client method to simplify nfs4 callback authentication J. Bruce Fields 2004-09-16 23:38 ` [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method Trond Myklebust @ 2004-09-17 1:18 ` Trond Myklebust 2004-09-17 2:20 ` J. Bruce Fields 2 siblings, 1 reply; 22+ messages in thread From: Trond Myklebust @ 2004-09-17 1:18 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Neil Brown, nfs P=E5 to , 16/09/2004 klokka 19:16, skreiv J. Bruce Fields: > diff -puN net/sunrpc/svcauth_unix.c~svcrpc_unix_ip_mapping_method net/sun= rpc/svcauth_unix.c > --- linux-2.6.9-rc2/net/sunrpc/svcauth_unix.c~svcrpc_unix_ip_mapping_meth= od 2004-09-16 16:40:53.000000000 -0400 > +++ linux-2.6.9-rc2-bfields/net/sunrpc/svcauth_unix.c 2004-09-16 16:40:53= .000000000 -0400 > @@ -334,10 +334,6 @@ svcauth_unix_set_client(struct svc_rqst=20 > { > struct ip_map key, *ipm; > =20 > - rqstp->rq_client =3D NULL; > - if (rqstp->rq_proc =3D=3D 0) > - return SVC_OK; > - > strcpy(key.m_class, rqstp->rq_server->sv_program->pg_class); > key.m_addr =3D rqstp->rq_addr.sin_addr; > =20 > @@ -395,7 +391,7 @@ svcauth_null_accept(struct svc_rqst *rqs > if (cred->cr_group_info =3D=3D NULL) > return SVC_DROP; /* kmalloc failure - client must retry */ > =20 > - rv =3D svcauth_unix_set_client(rqstp); > + rv =3D rqstp->rq_server->sv_program->pg_set_client(rqstp); > if (rv =3D=3D SVC_DENIED) > goto badcred; > =20 > @@ -473,7 +469,7 @@ svcauth_unix_accept(struct svc_rqst *rqs > return SVC_DENIED; > } > =20 > - rv =3D svcauth_unix_set_client(rqstp); > + rv =3D rqstp->rq_server->sv_program->pg_set_client(rqstp); > if (rv =3D=3D SVC_DENIED) > goto badcred; > =20 This is rather unclean... You are making a special method that is really very specific to svcauth_unix and svcauth_null, yet the pg_set_client() appears as a generic method in the generic svc_program object. Firstly, I'd strongly suggest that we call this callback pg_set_domain so that there is no confusion about what it does. Secondly, please explain why we're leaving RPCSEC_GSS as a special case here? Isn't the current implementation also calling up to "rpc.mountd" in order to check "/etc/exports"? Ideally, all the *_accept() methods should be calling the same function to set the domain (or not to set it as the case may be). Better still: could we defer calling pg_set_domain() until after the call to svc_authenticate? Finally, please could we move the domain_release() method out of struct auth_ops and into struct auth_domain itself? Cheers, Trond ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method 2004-09-17 1:18 ` Trond Myklebust @ 2004-09-17 2:20 ` J. Bruce Fields 2004-09-22 6:54 ` Neil Brown 0 siblings, 1 reply; 22+ messages in thread From: J. Bruce Fields @ 2004-09-17 2:20 UTC (permalink / raw) To: Trond Myklebust; +Cc: Neil Brown, nfs On Thu, Sep 16, 2004 at 09:18:39PM -0400, Trond Myklebust wrote: > You are making a special method that is really very specific to > svcauth_unix and svcauth_null, yet the pg_set_client() appears as a > generic method in the generic svc_program object. > > Firstly, I'd strongly suggest that we call this callback pg_set_domain > so that there is no confusion about what it does. After considering that, I decided that "client" (which is used in rq_client, in the nfsctl's (add_client), etc., in the exportfs documentation, etc.) makes more sense than "domain" (used only in the type struct auth_domain). > Secondly, please explain why we're leaving RPCSEC_GSS as a special case > here? Isn't the current implementation also calling up to "rpc.mountd" > in order to check "/etc/exports"? Yes, but that doesn't happen till later--we have to have a filehandle for that. The mistake was probably referring to "the export table" in the patch comments--we're not really looking at that yet, we're only looking up the name of this client--it's not much more than a reverse dns lookup. (So in the worst case, in auth_unix, there are *two* upcalls--one here, to get the name of the client, then one later to actually see whether something's exported to that client.) So this upcall really is auth_unix/auth_null-specific. But still there's some odd asymmetry here, I agree--I need to think about the auth_gss case. > Ideally, all the *_accept() methods > should be calling the same function to set the domain (or not to set it > as the case may be). Better still: could we defer calling > pg_set_domain() until after the call to svc_authenticate? That would be nice. > Finally, please could we move the domain_release() method out of struct > auth_ops and into struct auth_domain itself? Yeah, that's probably a good idea. Thanks for the comments. --b. ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method 2004-09-17 2:20 ` J. Bruce Fields @ 2004-09-22 6:54 ` Neil Brown 2004-09-22 10:10 ` Olaf Kirch 2004-09-23 21:46 ` J. Bruce Fields 0 siblings, 2 replies; 22+ messages in thread From: Neil Brown @ 2004-09-22 6:54 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Trond Myklebust, nfs On Thursday September 16, bfields@fieldses.org wrote: > On Thu, Sep 16, 2004 at 09:18:39PM -0400, Trond Myklebust wrote: > > You are making a special method that is really very specific to > > svcauth_unix and svcauth_null, yet the pg_set_client() appears as a > > generic method in the generic svc_program object. > > > > Firstly, I'd strongly suggest that we call this callback pg_set_domain > > so that there is no confusion about what it does. > > After considering that, I decided that "client" (which is used in > rq_client, in the nfsctl's (add_client), etc., in the exportfs > documentation, etc.) makes more sense than "domain" (used only in the > type struct auth_domain). > > > Secondly, please explain why we're leaving RPCSEC_GSS as a special case > > here? Isn't the current implementation also calling up to "rpc.mountd" > > in order to check "/etc/exports"? > > Yes, but that doesn't happen till later--we have to have a filehandle > for that. The mistake was probably referring to "the export table" in > the patch comments--we're not really looking at that yet, we're only > looking up the name of this client--it's not much more than a reverse > dns lookup. (So in the worst case, in auth_unix, there are *two* > upcalls--one here, to get the name of the client, then one later to > actually see whether something's exported to that client.) > > So this upcall really is auth_unix/auth_null-specific. > > But still there's some odd asymmetry here, I agree--I need to think > about the auth_gss case. I suspect that is an excellent idea. Work out the auth_gss case which is likely to be more general, and then make auth_{unix,null} fit in with that. For nfsd, there are three up-calls that can be made for an auth_unix authenticated request. 1 - lookup IP address and get 'client' identifier 2 - lookup filehandle (prefix) and get filesystem 3 - lookup client + filesystem and get export flags. (1) could be seen as an "authentication" step, while (3) is an "authorisation" step. ( (2) is just an internal lookup). (1) doesn't look much like authentication, but the equivalent with auth_gss does (I think). For call-backs, the authorisation and any other lookup would be handled inside the kernel, but the authentication could still need an up-call - to start a gss session for example. In 2.4, lockd authentication piggy-backed on nfsd authentication for just those requests that were thought to need it. The rest were unauthenticated. Presumably we should be doing the same in 2.6 - not require auth for some requests, and just leave nfsd auth for the remaining. One approach is the patches you produced. Another might be a per-svc_procedure flag to say whether auth was required or not. There are doubtlessly others. Alternately, the code which causes a call-back to be meaningful (e.g. nlmclnt_lock in lockd (??)) could insert the relevant information into the auth cache in advance. For gss, I presume a callback needs the same authentication as a regular service so it would be nice to use the same mechanism. Presumably this couldn't be satisfied completely within the kernel without adding too much extra gss_init code. i.e. the kernel shouldn't need upcalls to find out keys, but only to perform the context-establishment handshake. But who's upcall mechanism do we use here?? I've rambled a bit, hopefully not to much. To summarise: - I'm not in favour of the pg_set_client method in it's current form. - I think that for AUTH_UNIX call-backs, the best approach would be do have the caller insert the relevant information into the current ip-lookup cache, but that due to the vagaries of multi-homed hosts(*), that might not work. Second best would be to ignore the IP address (as 2.4 lockd does) and trust to the contents of the request being hard to fake. - For gss, an upcall will be needed to authenticate the call-back, so use the current kernel-side infrastructure for request authentication. (*) I note that nfs/callback.c find the client based in the IP address. Is there anything in the code or the NFSv4 spec that allows for the fact that a server might have multiple IP addresses and that the callback might come from a different address than requests are being sent to?? > > > Ideally, all the *_accept() methods > > should be calling the same function to set the domain (or not to set it > > as the case may be). Better still: could we defer calling > > pg_set_domain() until after the call to svc_authenticate? > > That would be nice. I don't think that makes sense. It is the client that is being authenticated. The end-product of authentication is knowing who the client is. How can you separate one for the other? > > > Finally, please could we move the domain_release() method out of struct > > auth_ops and into struct auth_domain itself? > > Yeah, that's probably a good idea. Agreed. NeilBrown ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Re: [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method 2004-09-22 6:54 ` Neil Brown @ 2004-09-22 10:10 ` Olaf Kirch 2004-09-23 21:46 ` J. Bruce Fields 1 sibling, 0 replies; 22+ messages in thread From: Olaf Kirch @ 2004-09-22 10:10 UTC (permalink / raw) To: Neil Brown; +Cc: J. Bruce Fields, Trond Myklebust, nfs On Wed, Sep 22, 2004 at 04:54:12PM +1000, Neil Brown wrote: > One approach is the patches you produced. Another might be a > per-svc_procedure flag to say whether auth was required or not. There > are doubtlessly others. I think the code in svcauth*.c should not concern itself with authorization and make security policy decisions. It should parse the credentials and verifier and call service specific routines that make this decision. Specifically, stuff like ip_map_lookup() should occur inside the policy routine, not in svcauth_unix_accept(). I also see no problem with having specific callback routines per authentication flavor: int (*pg_auth_null)(struct svc_rqst *); int (*pg_auth_unix)(struct svc_rqst *, struct svc_auth_unix_data *); int (*pg_auth_gss)(struct svc_rqst *, struct svc_auth_gss_data *); If a program leaves one of these pointers NULL means "reject any request with this auth flavor" Of course, it makes sense to provide default implementations for all auth flavors so that you don't have to cut'n'paste them for every rpc service. Olaf -- Olaf Kirch | Things that make Monday morning interesting, #1: okir@suse.de | "I want to use NFS over AX25, can you help me?" ---------------+ ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method 2004-09-22 6:54 ` Neil Brown 2004-09-22 10:10 ` Olaf Kirch @ 2004-09-23 21:46 ` J. Bruce Fields 2004-09-24 4:04 ` Neil Brown 1 sibling, 1 reply; 22+ messages in thread From: J. Bruce Fields @ 2004-09-23 21:46 UTC (permalink / raw) To: Neil Brown; +Cc: Trond Myklebust, nfs On Wed, Sep 22, 2004 at 04:54:12PM +1000, Neil Brown wrote: > Alternately, the code which causes a call-back to be meaningful > (e.g. nlmclnt_lock in lockd (??)) could insert the relevant > information into the auth cache in advance. There's not really a reliable way to do that with the current code since userspace can flush the auth_domain cache at whim. --b. ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method 2004-09-23 21:46 ` J. Bruce Fields @ 2004-09-24 4:04 ` Neil Brown 2004-09-24 7:42 ` Olaf Kirch 2004-09-28 22:00 ` J. Bruce Fields 0 siblings, 2 replies; 22+ messages in thread From: Neil Brown @ 2004-09-24 4:04 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Trond Myklebust, nfs On Thursday September 23, bfields@fieldses.org wrote: > On Wed, Sep 22, 2004 at 04:54:12PM +1000, Neil Brown wrote: > > Alternately, the code which causes a call-back to be meaningful > > (e.g. nlmclnt_lock in lockd (??)) could insert the relevant > > information into the auth cache in advance. > > There's not really a reliable way to do that with the current code since > userspace can flush the auth_domain cache at whim. Hmm... yes. Thanks. I'm thinking that you really need to either involve user-space in authenticating callbacks (issues like multi-homed servers mean that the kernel cannot cope by itself at all) or not require RPC authentication at all (the way 2.4 works). Does anyone have objections to the following patch, which presumes the svcauth_unix_set_client patch from Bruce. With it, locking starts working again. NeilBrown ==================================== Fix call-back authentication problems with lockd. Currently, lockd callbacks (e.g. GRANT, from server to client) fail because they cannot be authenticated. In 2.4, we simply don't bother authenticating callbacks, and this is probably the easiest approach. Even checking the source address in the kernel would not be enough because the server might be multi-homed. So we introduce a per-procedure flag to say that authentication isn't needed, make sure the procedure is known to svc_authenticate, and have the AUTH_UNIX and AUTH_NULL authentication routines allow such requests through. Signed-off-by: Neil Brown <neilb@cse.unsw.edu.au> ### Diffstat output ./fs/lockd/svc4proc.c | 51 +++++++++++++++++++++---------------------- ./fs/lockd/svcproc.c | 51 +++++++++++++++++++++---------------------- ./include/linux/sunrpc/svc.h | 6 +++++ ./net/sunrpc/svc.c | 20 +++++++++++----- ./net/sunrpc/svcauth_unix.c | 4 ++- 5 files changed, 75 insertions(+), 57 deletions(-) diff ./fs/lockd/svc4proc.c~current~ ./fs/lockd/svc4proc.c --- ./fs/lockd/svc4proc.c~current~ 2004-09-24 12:10:40.000000000 +1000 +++ ./fs/lockd/svc4proc.c 2004-09-24 13:43:10.000000000 +1000 @@ -537,7 +537,7 @@ nlm4svc_callback_exit(struct rpc_task *t struct nlm_void { int dummy; }; -#define PROC(name, xargt, xrest, argt, rest, respsize) \ +#define PROC(name, xargt, xrest, argt, rest, respsize, flags) \ { .pc_func = (svc_procfunc) nlm4svc_proc_##name, \ .pc_decode = (kxdrproc_t) nlm4svc_decode_##xargt, \ .pc_encode = (kxdrproc_t) nlm4svc_encode_##xrest, \ @@ -545,36 +545,37 @@ struct nlm_void { int dummy; }; .pc_argsize = sizeof(struct nlm_##argt), \ .pc_ressize = sizeof(struct nlm_##rest), \ .pc_xdrressize = respsize, \ + .pc_flags = flags, \ } #define Ck (1+8) /* cookie */ #define No (1+1024/4) /* netobj */ #define St 1 /* status */ #define Rg 4 /* range (offset + length) */ struct svc_procedure nlmsvc_procedures4[] = { - PROC(null, void, void, void, void, 1), - PROC(test, testargs, testres, args, res, Ck+St+2+No+Rg), - PROC(lock, lockargs, res, args, res, Ck+St), - PROC(cancel, cancargs, res, args, res, Ck+St), - PROC(unlock, unlockargs, res, args, res, Ck+St), - PROC(granted, testargs, res, args, res, Ck+St), - PROC(test_msg, testargs, norep, args, void, 1), - PROC(lock_msg, lockargs, norep, args, void, 1), - PROC(cancel_msg, cancargs, norep, args, void, 1), - PROC(unlock_msg, unlockargs, norep, args, void, 1), - PROC(granted_msg, testargs, norep, args, void, 1), - PROC(test_res, testres, norep, res, void, 1), - PROC(lock_res, lockres, norep, res, void, 1), - PROC(cancel_res, cancelres, norep, res, void, 1), - PROC(unlock_res, unlockres, norep, res, void, 1), - PROC(granted_res, res, norep, res, void, 1), + PROC(null, void, void, void, void, 1, PC_NO_AUTH_NEEDED), + PROC(test, testargs, testres, args, res, Ck+St+2+No+Rg, 0), + PROC(lock, lockargs, res, args, res, Ck+St, 0), + PROC(cancel, cancargs, res, args, res, Ck+St, 0), + PROC(unlock, unlockargs, res, args, res, Ck+St, 0), + PROC(granted, testargs, res, args, res, Ck+St, PC_NO_AUTH_NEEDED), + PROC(test_msg, testargs, norep, args, void, 1, 0), + PROC(lock_msg, lockargs, norep, args, void, 1, 0), + PROC(cancel_msg, cancargs, norep, args, void, 1, 0), + PROC(unlock_msg, unlockargs, norep, args, void, 1, 0), + PROC(granted_msg, testargs, norep, args, void, 1, PC_NO_AUTH_NEEDED), + PROC(test_res, testres, norep, res, void, 1, PC_NO_AUTH_NEEDED), + PROC(lock_res, lockres, norep, res, void, 1, PC_NO_AUTH_NEEDED), + PROC(cancel_res, cancelres, norep, res, void, 1, PC_NO_AUTH_NEEDED), + PROC(unlock_res, unlockres, norep, res, void, 1, PC_NO_AUTH_NEEDED), + PROC(granted_res, res, norep, res, void, 1, PC_NO_AUTH_NEEDED), /* statd callback */ - PROC(sm_notify, reboot, void, reboot, void, 1), - PROC(none, void, void, void, void, 0), - PROC(none, void, void, void, void, 0), - PROC(none, void, void, void, void, 0), - PROC(share, shareargs, shareres, args, res, Ck+St+1), - PROC(unshare, shareargs, shareres, args, res, Ck+St+1), - PROC(nm_lock, lockargs, res, args, res, Ck+St), - PROC(free_all, notify, void, args, void, 1), + PROC(sm_notify, reboot, void, reboot, void, 1, PC_NO_AUTH_NEEDED), + PROC(none, void, void, void, void, 0, 0), + PROC(none, void, void, void, void, 0, 0), + PROC(none, void, void, void, void, 0, 0), + PROC(share, shareargs, shareres, args, res, Ck+St+1, 0), + PROC(unshare, shareargs, shareres, args, res, Ck+St+1, 0), + PROC(nm_lock, lockargs, res, args, res, Ck+St, 0), + PROC(free_all, notify, void, args, void, 1, 0), }; diff ./fs/lockd/svcproc.c~current~ ./fs/lockd/svcproc.c --- ./fs/lockd/svcproc.c~current~ 2004-09-24 12:11:21.000000000 +1000 +++ ./fs/lockd/svcproc.c 2004-09-24 13:43:16.000000000 +1000 @@ -561,7 +561,7 @@ nlmsvc_callback_exit(struct rpc_task *ta struct nlm_void { int dummy; }; -#define PROC(name, xargt, xrest, argt, rest, respsize) \ +#define PROC(name, xargt, xrest, argt, rest, respsize, flags) \ { .pc_func = (svc_procfunc) nlmsvc_proc_##name, \ .pc_decode = (kxdrproc_t) nlmsvc_decode_##xargt, \ .pc_encode = (kxdrproc_t) nlmsvc_encode_##xrest, \ @@ -569,6 +569,7 @@ struct nlm_void { int dummy; }; .pc_argsize = sizeof(struct nlm_##argt), \ .pc_ressize = sizeof(struct nlm_##rest), \ .pc_xdrressize = respsize, \ + .pc_flags = flags, \ } #define Ck (1+8) /* cookie */ @@ -577,30 +578,30 @@ struct nlm_void { int dummy; }; #define Rg 2 /* range - offset + size */ struct svc_procedure nlmsvc_procedures[] = { - PROC(null, void, void, void, void, 1), - PROC(test, testargs, testres, args, res, Ck+St+2+No+Rg), - PROC(lock, lockargs, res, args, res, Ck+St), - PROC(cancel, cancargs, res, args, res, Ck+St), - PROC(unlock, unlockargs, res, args, res, Ck+St), - PROC(granted, testargs, res, args, res, Ck+St), - PROC(test_msg, testargs, norep, args, void, 1), - PROC(lock_msg, lockargs, norep, args, void, 1), - PROC(cancel_msg, cancargs, norep, args, void, 1), - PROC(unlock_msg, unlockargs, norep, args, void, 1), - PROC(granted_msg, testargs, norep, args, void, 1), - PROC(test_res, testres, norep, res, void, 1), - PROC(lock_res, lockres, norep, res, void, 1), - PROC(cancel_res, cancelres, norep, res, void, 1), - PROC(unlock_res, unlockres, norep, res, void, 1), - PROC(granted_res, res, norep, res, void, 1), + PROC(null, void, void, void, void, 1, PC_NO_AUTH_NEEDED), + PROC(test, testargs, testres, args, res, Ck+St+2+No+Rg, 0), + PROC(lock, lockargs, res, args, res, Ck+St, 0), + PROC(cancel, cancargs, res, args, res, Ck+St, 0), + PROC(unlock, unlockargs, res, args, res, Ck+St, 0), + PROC(granted, testargs, res, args, res, Ck+St, PC_NO_AUTH_NEEDED), + PROC(test_msg, testargs, norep, args, void, 1, 0), + PROC(lock_msg, lockargs, norep, args, void, 1, 0), + PROC(cancel_msg, cancargs, norep, args, void, 1, 0), + PROC(unlock_msg, unlockargs, norep, args, void, 1, 0), + PROC(granted_msg, testargs, norep, args, void, 1, PC_NO_AUTH_NEEDED), + PROC(test_res, testres, norep, res, void, 1, PC_NO_AUTH_NEEDED), + PROC(lock_res, lockres, norep, res, void, 1, PC_NO_AUTH_NEEDED), + PROC(cancel_res, cancelres, norep, res, void, 1, PC_NO_AUTH_NEEDED), + PROC(unlock_res, unlockres, norep, res, void, 1, PC_NO_AUTH_NEEDED), + PROC(granted_res, res, norep, res, void, 1, PC_NO_AUTH_NEEDED), /* statd callback */ - PROC(sm_notify, reboot, void, reboot, void, 1), - PROC(none, void, void, void, void, 1), - PROC(none, void, void, void, void, 1), - PROC(none, void, void, void, void, 1), - PROC(share, shareargs, shareres, args, res, Ck+St+1), - PROC(unshare, shareargs, shareres, args, res, Ck+St+1), - PROC(nm_lock, lockargs, res, args, res, Ck+St), - PROC(free_all, notify, void, args, void, 0), + PROC(sm_notify, reboot, void, reboot, void, 1, PC_NO_AUTH_NEEDED), + PROC(none, void, void, void, void, 1, 0), + PROC(none, void, void, void, void, 1, 0), + PROC(none, void, void, void, void, 1, 0), + PROC(share, shareargs, shareres, args, res, Ck+St+1, 0), + PROC(unshare, shareargs, shareres, args, res, Ck+St+1, 0), + PROC(nm_lock, lockargs, res, args, res, Ck+St, 0), + PROC(free_all, notify, void, args, void, 0, 0), }; diff ./include/linux/sunrpc/svc.h~current~ ./include/linux/sunrpc/svc.h --- ./include/linux/sunrpc/svc.h~current~ 2004-09-24 11:49:40.000000000 +1000 +++ ./include/linux/sunrpc/svc.h 2004-09-24 11:51:18.000000000 +1000 @@ -285,8 +285,14 @@ struct svc_procedure { unsigned int pc_count; /* call count */ unsigned int pc_cachetype; /* cache info (NFS) */ unsigned int pc_xdrressize; /* maximum size of XDR reply */ + unsigned long pc_flags; /* various per-procedude flags */ }; +/* pc_flags values */ +#define PC_NO_AUTH_NEEDED 1 /* an authenticate request is not required for + * this proceedure. e.g. NULL and lockd call-backs + */ + /* * This is the RPC server thread function prototype */ diff ./net/sunrpc/svc.c~current~ ./net/sunrpc/svc.c --- ./net/sunrpc/svc.c~current~ 2004-09-24 11:52:04.000000000 +1000 +++ ./net/sunrpc/svc.c 2004-09-24 12:03:21.000000000 +1000 @@ -305,7 +305,19 @@ svc_process(struct svc_serv *serv, struc rqstp->rq_vers = vers = ntohl(svc_getu32(argv)); /* version number */ rqstp->rq_proc = proc = ntohl(svc_getu32(argv)); /* procedure number */ + /* find version and procedure before calling svc_authenticate, + * is the later might need details of the former, but don't + * return errors yet - if there is an auth error it but over-ride any + * others to avoid information leakage + */ progp = serv->sv_program; + if (vers < progp->pg_nvers) + versp = progp->pg_vers[vers]; + if (versp && proc < versp->vs_nproc) + procp = versp->vs_proc + proc; + rqstp->rq_server = serv; + rqstp->rq_procinfo = procp; + /* * Decode auth data, and add verifier to reply buffer. * We do this before anything else in order to get a decent @@ -335,15 +347,11 @@ svc_process(struct svc_serv *serv, struc if (prog != progp->pg_prog) goto err_bad_prog; - if (vers >= progp->pg_nvers || - !(versp = progp->pg_vers[vers])) + if (!versp) goto err_bad_vers; - procp = versp->vs_proc + proc; - if (proc >= versp->vs_nproc || !procp->pc_func) + if (!procp || !procp->pc_func) goto err_bad_proc; - rqstp->rq_server = serv; - rqstp->rq_procinfo = procp; /* Syntactic check complete */ serv->sv_stats->rpccnt++; diff ./net/sunrpc/svcauth_unix.c~current~ ./net/sunrpc/svcauth_unix.c --- ./net/sunrpc/svcauth_unix.c~current~ 2004-09-24 11:49:09.000000000 +1000 +++ ./net/sunrpc/svcauth_unix.c 2004-09-24 12:28:26.000000000 +1000 @@ -335,7 +335,9 @@ svcauth_unix_set_client(struct svc_rqst struct ip_map key, *ipm; rqstp->rq_client = NULL; - if (rqstp->rq_proc == 0) + if (rqstp->rq_proc == 0 || + (rqstp->rq_procinfo && + (rqstp->rq_procinfo->pc_flags & PC_NO_AUTH_NEEDED))) return SVC_OK; strcpy(key.m_class, rqstp->rq_server->sv_program->pg_class); ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Re: [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method 2004-09-24 4:04 ` Neil Brown @ 2004-09-24 7:42 ` Olaf Kirch 2004-09-24 20:58 ` J. Bruce Fields 2004-09-28 22:00 ` J. Bruce Fields 1 sibling, 1 reply; 22+ messages in thread From: Olaf Kirch @ 2004-09-24 7:42 UTC (permalink / raw) To: Neil Brown; +Cc: J. Bruce Fields, Trond Myklebust, nfs [-- Attachment #1: Type: text/plain, Size: 837 bytes --] On Fri, Sep 24, 2004 at 02:04:16PM +1000, Neil Brown wrote: > Does anyone have objections to the following patch, which presumes the > svcauth_unix_set_client patch from Bruce. With it, locking starts > working again. It looks to me as if the patch forgets to include the NULL verifier for those procedures that don't need authentication. I think you also want to do the same in svcauth_null_accept, as some lockd implementations actually use AUTH_NULL. For what it's worth, I added a similar patch to our tree yesterday to fix this problem. (This is mostly FYI - the patch won't apply cleanly to the vanilla kernel, as it is based on top of the kernel-statd stuff) Olaf -- Olaf Kirch | Things that make Monday morning interesting, #1: okir@suse.de | "I want to use NFS over AX25, can you help me?" ---------------+ [-- Attachment #2: sunrpc-optional-auth --] [-- Type: text/plain, Size: 4702 bytes --] Index: linux-2.6.5/fs/lockd/svc.c =================================================================== --- linux-2.6.5.orig/fs/lockd/svc.c 2004-09-22 15:57:04.000000000 +0200 +++ linux-2.6.5/fs/lockd/svc.c 2004-09-22 15:58:34.000000000 +0200 @@ -211,6 +211,32 @@ module_put_and_exit(0); } +static int +lockd_rqst_needs_auth(struct svc_rqst *rqstp) +{ + u32 proc = rqstp->rq_proc; + + if (proc == 0 + || proc == NLMPROC_GRANTED + || proc == NLMPROC_TEST_RES + || proc == NLMPROC_LOCK_RES + || proc == NLMPROC_CANCEL_RES + || proc == NLMPROC_UNLOCK_RES + || proc == NLMPROC_GRANTED_RES + || proc == NLMPROC_NSM_NOTIFY) + return 0; + return 1; +} + +#ifdef CONFIG_STATD +static int +statd_rqst_needs_auth(struct svc_rqst *rqstp) +{ + /* statd is unauthenticated */ + return 0; +} +#endif + /* * Bring up the lockd process if it's not already up. */ @@ -480,6 +506,8 @@ .pg_name = "statd", /* service name */ .pg_class = "nfsd", /* share authentication with nfsd */ .pg_stats = &nsmsvc_stats, /* stats table */ + + .pg_need_auth = statd_rqst_needs_auth, }; #define nsmsvc_program_p &nsmsvc_program @@ -529,4 +557,6 @@ .pg_name = "lockd", /* service name */ .pg_class = "nfsd", /* share authentication with nfsd */ .pg_stats = &nlmsvc_stats, /* stats table */ + + .pg_need_auth = lockd_rqst_needs_auth, }; Index: linux-2.6.5/fs/nfsd/nfssvc.c =================================================================== --- linux-2.6.5.orig/fs/nfsd/nfssvc.c 2004-09-22 15:57:04.000000000 +0200 +++ linux-2.6.5/fs/nfsd/nfssvc.c 2004-09-22 15:58:34.000000000 +0200 @@ -360,6 +360,14 @@ return 1; } +static int +nfsd_rqst_needs_auth(struct svc_rqst *rqstp) +{ + if (rqstp->rq_proc == 0) + return 0; + return 1; +} + extern struct svc_version nfsd_version2, nfsd_version3, nfsd_version4; static struct svc_version * nfsd_version[] = { @@ -386,6 +394,8 @@ .pg_vers = nfsd_acl_version, .pg_name = "nfsd", .pg_stats = &nfsd_acl_svcstats, + + .pg_need_auth = nfsd_rqst_needs_auth, }; # define nfsd_acl_program_p &nfsd_acl_program #else @@ -401,4 +411,6 @@ .pg_name = "nfsd", /* program name */ .pg_class = "nfsd", /* authentication class */ .pg_stats = &nfsd_svcstats, /* version table */ + + .pg_need_auth = nfsd_rqst_needs_auth, }; Index: linux-2.6.5/include/linux/sunrpc/svc.h =================================================================== --- linux-2.6.5.orig/include/linux/sunrpc/svc.h 2004-09-22 15:57:04.000000000 +0200 +++ linux-2.6.5/include/linux/sunrpc/svc.h 2004-09-22 15:58:56.000000000 +0200 @@ -264,6 +264,8 @@ char * pg_name; /* service name */ char * pg_class; /* class name: services sharing authentication */ struct svc_stat * pg_stats; /* rpc statistics */ + + int (*pg_need_auth)(struct svc_rqst *); }; /* Index: linux-2.6.5/net/sunrpc/svcauth_unix.c =================================================================== --- linux-2.6.5.orig/net/sunrpc/svcauth_unix.c 2004-09-22 15:57:04.000000000 +0200 +++ linux-2.6.5/net/sunrpc/svcauth_unix.c 2004-09-22 15:59:20.000000000 +0200 @@ -337,6 +337,7 @@ static int svcauth_null_accept(struct svc_rqst *rqstp, u32 *authp) { + struct svc_program *prog = rqstp->rq_server->sv_program; struct iovec *argv = &rqstp->rq_arg.head[0]; struct iovec *resv = &rqstp->rq_res.head[0]; int rv=0; @@ -363,9 +364,10 @@ if (rqstp->rq_cred.cr_group_info == NULL) return SVC_DROP; /* kmalloc failure - client must retry */ - /* Put NULL verifier */ - svc_putu32(resv, RPC_AUTH_NULL); - svc_putu32(resv, 0); + if (prog->pg_need_auth && !prog->pg_need_auth(rqstp)) { + rv = SVC_OK; + goto accepted; + } key.m_class = rqstp->rq_server->sv_program->pg_class; key.m_addr = rqstp->rq_addr.sin_addr; @@ -395,6 +397,11 @@ if (rqstp->rq_client == NULL && rqstp->rq_proc != 0) *authp = rpc_autherr_badcred; +accepted: + /* Put NULL verifier */ + svc_putu32(resv, RPC_AUTH_NULL); + svc_putu32(resv, 0); + return rv; } @@ -420,6 +427,7 @@ int svcauth_unix_accept(struct svc_rqst *rqstp, u32 *authp) { + struct svc_program *prog = rqstp->rq_server->sv_program; struct iovec *argv = &rqstp->rq_arg.head[0]; struct iovec *resv = &rqstp->rq_res.head[0]; struct svc_cred *cred = &rqstp->rq_cred; @@ -462,6 +470,10 @@ key.m_class = rqstp->rq_server->sv_program->pg_class; key.m_addr = rqstp->rq_addr.sin_addr; + if (prog->pg_need_auth && !prog->pg_need_auth(rqstp)) { + rv = SVC_OK; + goto accepted; + } ipm = ip_map_lookup(&key, 0); @@ -486,6 +498,7 @@ if (rv == SVC_OK && rqstp->rq_client == NULL && rqstp->rq_proc != 0) goto badcred; +accepted: /* Put NULL verifier */ svc_putu32(resv, RPC_AUTH_NULL); svc_putu32(resv, 0); ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Re: [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method 2004-09-24 7:42 ` Olaf Kirch @ 2004-09-24 20:58 ` J. Bruce Fields 0 siblings, 0 replies; 22+ messages in thread From: J. Bruce Fields @ 2004-09-24 20:58 UTC (permalink / raw) To: Olaf Kirch; +Cc: Neil Brown, Trond Myklebust, nfs On Fri, Sep 24, 2004 at 09:42:09AM +0200, Olaf Kirch wrote: > On Fri, Sep 24, 2004 at 02:04:16PM +1000, Neil Brown wrote: > > Does anyone have objections to the following patch, which presumes the > > svcauth_unix_set_client patch from Bruce. With it, locking starts > > working again. > > It looks to me as if the patch forgets to include the NULL verifier > for those procedures that don't need authentication. > > I think you also want to do the same in svcauth_null_accept, as some > lockd implementations actually use AUTH_NULL. Note that the place where the check is added is in svcauth_unix_set_client, which (after my 2 svcauth_unix patches) is called from both null_accept and unix_accept. Both encode a NULL verifier if svcauth_unix_set_client fails. --b. ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method 2004-09-24 4:04 ` Neil Brown 2004-09-24 7:42 ` Olaf Kirch @ 2004-09-28 22:00 ` J. Bruce Fields 2004-09-28 22:11 ` Trond Myklebust 1 sibling, 1 reply; 22+ messages in thread From: J. Bruce Fields @ 2004-09-28 22:00 UTC (permalink / raw) To: Neil Brown; +Cc: Trond Myklebust, nfs On Fri, Sep 24, 2004 at 02:04:16PM +1000, Neil Brown wrote: > On Thursday September 23, bfields@fieldses.org wrote: > > On Wed, Sep 22, 2004 at 04:54:12PM +1000, Neil Brown wrote: > > > Alternately, the code which causes a call-back to be meaningful > > > (e.g. nlmclnt_lock in lockd (??)) could insert the relevant > > > information into the auth cache in advance. > > > > There's not really a reliable way to do that with the current code since > > userspace can flush the auth_domain cache at whim. > > > Hmm... yes. Thanks. > > I'm thinking that you really need to either involve user-space in > authenticating callbacks (issues like multi-homed servers mean that > the kernel cannot cope by itself at all) I can't find anything definitive on this in any rfc, but I don't believe that clients are required to be able to deal with callbacks from a different IP address (or that a server should allow such a thing to happen). Can anyone find evidence to the contrary? > or not require RPC authentication at all (the way 2.4 works). I believe the ip-address checking done by the nfsv4 callback service and by lockd are correct. Over rpcsec_gss, nfsv4 callbacks also have to be authenticated (by checking that the principal making the callback is the server's). > Does anyone have objections to the following patch, which presumes the > svcauth_unix_set_client patch from Bruce. With it, locking starts > working again. This patch would suffice if we're content to postpone authentication until the dispatch or procedure-specific code. If we do that, then we end up returning an NFS or NLM error instead of having the call rejected at the rpc layer. So, do we require the ability to e.g return an rpc auth error on authentication failure? If not, then the cleaner solution would be to do the same thing for nfsd--don't do any upcalls at accept() time, and move that stuff into, say, nfsd_dispatch(), instead. If we *do* need to allow programs to reject rpc calls at the rpc layer, then we need something like the program-specific hook I propose (with some modifications to make it flavor-independent, so it works for gss as well). --b. ------------------------------------------------------- This SF.net email is sponsored by: IT Product Guide on ITManagersJournal Use IT products in your business? Tell us what you think of them. Give us Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more http://productguide.itmanagersjournal.com/guidepromo.tmpl _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method 2004-09-28 22:00 ` J. Bruce Fields @ 2004-09-28 22:11 ` Trond Myklebust 2004-09-28 22:37 ` Trond Myklebust 0 siblings, 1 reply; 22+ messages in thread From: Trond Myklebust @ 2004-09-28 22:11 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Neil Brown, nfs P=E5 on , 29/09/2004 klokka 00:00, skreiv J. Bruce Fields: > =20 > > I'm thinking that you really need to either involve user-space in > > authenticating callbacks (issues like multi-homed servers mean that > > the kernel cannot cope by itself at all) >=20 > I can't find anything definitive on this in any rfc, but I don't believe > that clients are required to be able to deal with callbacks from a > different IP address (or that a server should allow such a thing to > happen). Can anyone find evidence to the contrary? Agreed. If the client talks to the server on one IP-address, but the server replies on another, then there is no reason for the client to trust the reply. NFSv4.1 session extensions will allow the client and server agree on alternative transports for the callback channel, but there is nothing to allow this for earlier versions of NFS or NLM. Cheers, Trond ------------------------------------------------------- This SF.net email is sponsored by: IT Product Guide on ITManagersJournal Use IT products in your business? Tell us what you think of them. Give us Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more http://productguide.itmanagersjournal.com/guidepromo.tmpl _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method 2004-09-28 22:11 ` Trond Myklebust @ 2004-09-28 22:37 ` Trond Myklebust 0 siblings, 0 replies; 22+ messages in thread From: Trond Myklebust @ 2004-09-28 22:37 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Neil Brown, nfs P=E5 on , 29/09/2004 klokka 00:11, skreiv Trond Myklebust: > >=20 > > I can't find anything definitive on this in any rfc, but I don't believ= e > > that clients are required to be able to deal with callbacks from a > > different IP address (or that a server should allow such a thing to > > happen). Can anyone find evidence to the contrary? >=20 > Agreed. If the client talks to the server on one IP-address, but the > server replies on another, then there is no reason for the client to > trust the reply. To put the argument slightly differently: Although you might argue that when I type "mount bar:/foo", I'm accepting replies from the DNS entry "bar" rather than an IP address, however if I type "mount 192.168.0.1:/foo" then I might not want to rely on DNS to tell me that 192.168.0.2 is an alias for "bar" which is an alias for 192.168.0.1. The server doesn't know what I've typed on my command line, so it must take the conservative view that if I'm talking to 192.168.0.1, then I'm only accepting replies from that address. Cheers, Trond ------------------------------------------------------- This SF.net email is sponsored by: IT Product Guide on ITManagersJournal Use IT products in your business? Tell us what you think of them. Give us Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more http://productguide.itmanagersjournal.com/guidepromo.tmpl _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2004-09-28 22:37 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20040916230555.GA13415@fieldses.org>
2004-09-16 23:07 ` 6 svcauth_unix patches to make export table lookups optional J. Bruce Fields
2004-09-16 23:16 ` [PATCH 1 of 6] svcrpc: auth_null fixes J. Bruce Fields
2004-09-16 23:16 ` [PATCH 2 of 6] svcrpc: share code duplicated between auth_unix and auth_null J. Bruce Fields
2004-09-16 23:16 ` [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method J. Bruce Fields
2004-09-16 23:16 ` [PATCH 4 of 6] nfs4: use new pg_set_client method to simplify nfs4 callback authentication J. Bruce Fields
2004-09-16 23:16 ` [PATCH 5 of 6] lockd: don't try to match callback requests against export table J. Bruce Fields
2004-09-16 23:16 ` [PATCH 6 of 6] nfsd: remove pg_authenticate field J. Bruce Fields
2004-09-16 23:34 ` [PATCH 5 of 6] lockd: don't try to match callback requests against export table Trond Myklebust
2004-09-24 3:55 ` Neil Brown
2004-09-16 23:38 ` [PATCH 3 of 6] svcrpc: move export table checks to a per-program pg_add_client method Trond Myklebust
2004-09-17 1:11 ` J. Bruce Fields
2004-09-17 1:18 ` Trond Myklebust
2004-09-17 2:20 ` J. Bruce Fields
2004-09-22 6:54 ` Neil Brown
2004-09-22 10:10 ` Olaf Kirch
2004-09-23 21:46 ` J. Bruce Fields
2004-09-24 4:04 ` Neil Brown
2004-09-24 7:42 ` Olaf Kirch
2004-09-24 20:58 ` J. Bruce Fields
2004-09-28 22:00 ` J. Bruce Fields
2004-09-28 22:11 ` Trond Myklebust
2004-09-28 22:37 ` Trond Myklebust
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.