nfs-utils patches for v2/v3 security negotiation
Subsystems (2)
This is two bits of small cleanup (including the previously sent m_path field cleanup), followed by a patch that fixes mountd's security negotiation. (Till now mountd has just advertised just a static list of pseudoflavors, which works just well enough most of the time, but we actually want to use the list of security flavors associated with the given export.)
--b.
PATCH 1/3 Remove redundant m_path field
From: J. Bruce Fields <[email protected]>
Contrary to the comment above its definition, the field m_path always has the same value as e_path: the *only* modifications of m_path are all of the form:
strncpy(exp->m_export.m_path, exp->m_export.e_path, sizeof (exp->m_export.m_path) - 1); exp->m_export.m_path[sizeof (exp->m_export.m_path) - 1] = '\0';
So m_path is always just a copy of e_path. In places where we need to store a path to a submount of a CROSSMNT-exported filesystem, as in cache.c, we just use a local variable.
--- support/export/export.c | 12 ------------ support/export/nfsctl.c | 4 ++-- support/include/nfslib.h | 5 ----- support/nfs/exports.c | 6 ------ utils/mountd/mountd.c | 12 ++---------- utils/mountd/rmtab.c | 4 +--- 6 files changed, 5 insertions(+), 38 deletions(-)
support/export/export.c +0 -12
@@ -255,15 +255,3 @@ export_freeall(void) }client_freeall (); } - -void -export_reset (nfs_export *exp) -{ -if (!exp) -return ; - -/* Restore m_path. */ -strncpy (exp->m_export.m_path, exp->m_export.e_path, -sizeof (exp->m_export.m_path) -1 ); - exp->m_export.m_path[sizeof (exp->m_export.m_path) -1 ] ='\0' ; -}
support/export/nfsctl.c +2 -2
@@ -89,11 +89,11 @@ expsetup(struct nfsctl_export *exparg, nfs_export *exp, int unexport)nfs_client *clp = exp->m_client;struct stat stb; -if (stat (exp->m_export.m_path, &stb) <0 ) +if (stat (exp->m_export.e_path, &stb) <0 )return 0 ;memset (exparg,0 ,sizeof (*exparg)); -strncpy (exparg->ex_path, exp->m_export.m_path, +strncpy (exparg->ex_path, exp->m_export.e_path,sizeof (exparg->ex_path) -1 );strncpy (exparg->ex_client, clp->m_hostname,sizeof (exparg->ex_client) -1 );
support/include/nfslib.h +0 -5
@@ -67,11 +67,6 @@ struct sec_entry {struct exportent {char * e_hostname;char e_path[NFS_MAXPATHLEN +1 ]; -/* The mount path may be different from the exported path due -to submount. It may change for every mount. The idea is we -set m_path every time when we process a mount. We should not -use it for anything else. */ -char m_path[NFS_MAXPATHLEN +1 ];int e_flags;int e_anonuid;int e_anongid;
support/nfs/exports.c +0 -6
@@ -127,8 +127,6 @@ getexportent(int fromkernel, int fromexports)if (ok <=0 )return NULL ; -strncpy (def_ee.m_path, def_ee.e_path,sizeof (def_ee.m_path) -1 ); - def_ee.m_path [sizeof (def_ee.m_path) -1 ] ='\0' ; ok =getexport (exp,sizeof (exp)); }if (ok <0 ) {
@@ -187,8 +185,6 @@ getexportent(int fromkernel, int fromexports) rpath[sizeof (rpath) -1 ] ='\0' ;strncpy (ee.e_path, rpath,sizeof (ee.e_path) -1 ); ee.e_path[sizeof (ee.e_path) -1 ] ='\0' ; -strncpy (ee.m_path, ee.e_path,sizeof (ee.m_path) -1 ); - ee.m_path [sizeof (ee.m_path) -1 ] ='\0' ; }return ⅇ
@@ -360,8 +356,6 @@ mkexportent(char *hname, char *path, char *options) }strncpy (ee.e_path, path,sizeof (ee.e_path)); ee.e_path[sizeof (ee.e_path) -1 ] ='\0' ; -strncpy (ee.m_path, ee.e_path,sizeof (ee.m_path) -1 ); - ee.m_path [sizeof (ee.m_path) -1 ] ='\0' ;if (parseopts (options, &ee,0 ,NULL ) <0 )return NULL ;return ⅇ
utils/mountd/mountd.c +2 -10
@@ -230,9 +230,6 @@ mount_umnt_1_svc(struct svc_req *rqstp, dirpath *argp, void *resp)return 1 ; } -if (!new_cache) -export_reset (exp); -mountlist_del (inet_ntoa (sin->sin_addr), p);return 1 ; }
@@ -312,7 +309,6 @@ mount_pathconf_2_svc(struct svc_req *rqstp, dirpath *path, ppathcnf *res) }else if (stat (p, &stb) <0 ) {xlog (L_WARNING ,"can't stat exported dir %s: %s" , p,strerror (errno)); -export_reset (exp);return 1 ; }
@@ -328,8 +324,6 @@ mount_pathconf_2_svc(struct svc_req *rqstp, dirpath *path, ppathcnf *res) res->pc_mask[0 ] =0 ; res->pc_mask[1 ] =0 ; -export_reset (exp); -return 1 ; }
@@ -457,13 +451,11 @@ get_rootfh(struct svc_req *rqstp, dirpath *path, mountstat3 *error, int v3)if (fh !=NULL ) {mountlist_add (inet_ntoa (sin->sin_addr), p); *error =NFS_OK ; -export_reset (exp);return fh; }xlog (L_WARNING ,"getfh failed: %s" ,strerror (errno)); *error =NFSERR_ACCES ; } -export_reset (exp);return NULL ; }
@@ -499,14 +491,14 @@ get_exportlist(void)for (i =0 ; i <MCL_MAXTYPES ; i++) {for (exp = exportlist[i]; exp; exp = exp->m_next) {for (e = elist; e !=NULL ; e = e->ex_next) { -if (!strcmp (exp->m_export.m_path, e->ex_dir)) +if (!strcmp (exp->m_export.e_path, e->ex_dir))break ; }if (!e) { e = (struct exportnode *)xmalloc (sizeof (*e)); e->ex_next = elist; e->ex_groups =NULL ; - e->ex_dir =xstrdup (exp->m_export.m_path); + e->ex_dir =xstrdup (exp->m_export.e_path); elist = e; }
utils/mountd/rmtab.c +1 -3
@@ -162,10 +162,8 @@ mountlist_del_all(struct sockaddr_in *sin) }while ((rep =getrmtabent (1 ,NULL )) !=NULL ) {if (strcmp (rep->r_client, hp->h_name) ==0 && - (exp =auth_authenticate ("umountall" , sin, rep->r_path))) { -export_reset (exp); + (exp =auth_authenticate ("umountall" , sin, rep->r_path)))continue ; - }fputrmtabent (fp, rep,NULL ); }if (slink_safe_rename (_PATH_RMTABTMP, _PATH_RMTAB) <0 ) {
Signature · 1 line
1.5.5.rc1
PATCH 2/3 Minor mountd.c cleanup
From: J. Bruce Fields <[email protected]>
I find it more readable to have the normal (non-error) case unindented, and to keep conditionals relatively simple, as is the usual kernel style. Fix some inconsistent indentation while we're there.
--- utils/mountd/mountd.c | 87 +++++++++++++++++++++++++++--------------------- 1 files changed, 49 insertions(+), 38 deletions(-)
utils/mountd/mountd.c +49 -38
@@ -175,9 +175,9 @@ killer (int sig)static void sig_hup (int sig) { -/* don't exit on SIGHUP */ -xlog (L_NOTICE ,"Received SIGHUP... Ignoring.\n" , sig); -return ; +/* don't exit on SIGHUP */ +xlog (L_NOTICE ,"Received SIGHUP... Ignoring.\n" , sig); +return ; } bool_t
@@ -192,7 +192,8 @@ mount_mnt_1_svc(struct svc_req *rqstp, dirpath *path, fhstatus *res)struct nfs_fh_len *fh;xlog (D_CALL ,"MNT1(%s) called" , *path); -if ((fh =get_rootfh (rqstp, path, &res->fhs_status,0 )) !=NULL ) + fh =get_rootfh (rqstp, path, &res->fhs_status,0 ); +if (fh)memcpy (&res->fhstatus_u.fhs_fhandle, fh->fh_handle,32 );return 1 ; }
@@ -304,7 +305,8 @@ mount_pathconf_2_svc(struct svc_req *rqstp, dirpath *path, ppathcnf *res) }/* Now authenticate the intruder... */ -if (!(exp =auth_authenticate ("pathconf" , sin, p))) { + exp =auth_authenticate ("pathconf" , sin, p); +if (!exp) {return 1 ; }else if (stat (p, &stb) <0 ) { xlog(L_WARNING, "can't stat exported dir %s: %s",
@@ -344,18 +346,18 @@ mount_mnt_3_svc(struct svc_req *rqstp, dirpath *path, mountres3 *res)* issue with older Linux clients, who inspect the list in reversed * order. */ +struct mountres3_ok *ok = &res->mountres3_u.mountinfo;struct nfs_fh_len *fh;xlog (D_CALL ,"MNT3(%s) called" , *path); -if ((fh =get_rootfh (rqstp, path, &res->fhs_status,1 )) !=NULL ) { -struct mountres3_ok *ok = &res->mountres3_u.mountinfo; - - ok->fhandle.fhandle3_len = fh->fh_size; - ok->fhandle.fhandle3_val = (char *)fh->fh_handle; - ok->auth_flavors.auth_flavors_len - =sizeof (flavors)/sizeof (flavors[0 ]); - ok->auth_flavors.auth_flavors_val = flavors; - } + fh =get_rootfh (rqstp, path, &res->fhs_status,1 ); +if (!fh) +return 1 ; + + ok->fhandle.fhandle3_len = fh->fh_size; + ok->fhandle.fhandle3_val = (char *)fh->fh_handle; + ok->auth_flavors.auth_flavors_len =sizeof (flavors)/sizeof (flavors[0 ]); + ok->auth_flavors.auth_flavors_val = flavors;return 1 ; }
@@ -366,6 +368,7 @@ get_rootfh(struct svc_req *rqstp, dirpath *path, mountstat3 *error, int v3) (struct sockaddr_in *)svc_getcaller (rqstp->rq_xprt);struct stat stb, estb;nfs_export *exp; +struct nfs_fh_len *fh;char rpath[MAXPATHLEN +1 ];char *p = *path;
@@ -382,57 +385,65 @@ get_rootfh(struct svc_req *rqstp, dirpath *path, mountstat3 *error, int v3) }/* Now authenticate the intruder... */ -if (!(exp =auth_authenticate ("mount" , sin, p))) { + exp =auth_authenticate ("mount" , sin, p); +if (!exp) { *error =NFSERR_ACCES ; - }else if (stat (p, &stb) <0 ) { +return NULL ; + } +if (stat (p, &stb) <0 ) {xlog (L_WARNING ,"can't stat exported dir %s: %s" , p,strerror (errno));if (errno ==ENOENT ) *error =NFSERR_NOENT ;else *error =NFSERR_ACCES ; - }else if (!S_ISDIR (stb.st_mode) && !S_ISREG (stb.st_mode)) { +return NULL ; + } +if (!S_ISDIR (stb.st_mode) && !S_ISREG (stb.st_mode)) {xlog (L_WARNING ,"%s is not a directory or regular file" , p); *error =NFSERR_NOTDIR ; - }else if (stat (exp->m_export.e_path, &estb) <0 ) { +return NULL ; + } +if (stat (exp->m_export.e_path, &estb) <0 ) {xlog (L_WARNING ,"can't stat export point %s: %s" , p,strerror (errno)); *error =NFSERR_NOENT ; - }else if (estb.st_dev != stb.st_dev - && (!new_cache || !(exp->m_export.e_flags &NFSEXP_CROSSMOUNT )) - ) { +return NULL ; + } +if (estb.st_dev != stb.st_dev + && (!new_cache + || !(exp->m_export.e_flags &NFSEXP_CROSSMOUNT ))) {xlog (L_WARNING ,"request to export directory %s below nearest filesystem %s" , p, exp->m_export.e_path); *error =NFSERR_ACCES ; - }else if (exp->m_export.e_mountpoint && +return NULL ; + } +if (exp->m_export.e_mountpoint && !is_mountpoint (exp->m_export.e_mountpoint[0 ]? exp->m_export.e_mountpoint: exp->m_export.e_path)) {xlog (L_WARNING ,"request to export an unmounted filesystem: %s" , p); *error =NFSERR_NOENT ; - }else if (new_cache) { +return NULL ; + } + +if (new_cache) {/* This will be a static private nfs_export with just one * address. We feed it to kernel then extract the filehandle, * */ -struct nfs_fh_len *fh;if (cache_export (exp, p)) { *error =NFSERR_ACCES ;return NULL ; } fh =cache_get_filehandle (exp, v3?64 :32 , p); -if (fh ==NULL ) +if (fh ==NULL ) { *error =NFSERR_ACCES ; -else { - *error =NFS_OK ; -mountlist_add (inet_ntoa (sin->sin_addr), p); +return NULL ; } -return fh; }else { -struct nfs_fh_len *fh; -if (exp->m_exported<1 )export_export (exp);if (!exp->m_xtabent)
@@ -448,15 +459,15 @@ get_rootfh(struct svc_req *rqstp, dirpath *path, mountstat3 *error, int v3) fh =getfh_old ((struct sockaddr *) sin, stb.st_dev, stb.st_ino); } -if (fh !=NULL ) { -mountlist_add (inet_ntoa (sin->sin_addr), p); - *error =NFS_OK ; -return fh; +if (fh ==NULL ) { +xlog (L_WARNING ,"getfh failed: %s" ,strerror (errno)); + *error =NFSERR_ACCES ; +return NULL ; } -xlog (L_WARNING ,"getfh failed: %s" ,strerror (errno)); - *error =NFSERR_ACCES ; } -return NULL ; + *error =NFS_OK ; +mountlist_add (inet_ntoa (sin->sin_addr), p); +return fh; } static exports
Signature · 1 line
1.5.5.rc1
PATCH 3/3 Determine supported pseudoflavors from export
From: J. Bruce Fields <[email protected]>
Instead of using a static list of supported flavors, we should be taking the list from the export.
--- utils/mountd/mountd.c | 55 +++++++++++++++++++++++++++++++++--------------- 1 files changed, 38 insertions(+), 17 deletions(-)
utils/mountd/mountd.c +38 -17
@@ -26,6 +26,7 @@#include "misc.h" #include "mountd.h" #include "rpcmisc.h" +#include "pseudoflavors.h" extern void cache_open (void );extern struct nfs_fh_len *cache_get_filehandle (nfs_export *exp,int len,char *p);
@@ -35,7 +36,7 @@ extern void my_svc_run(void);static void usage (const char *,int exitcode);static exports get_exportlist (void ); -static struct nfs_fh_len *get_rootfh (struct svc_req *,dirpath *,mountstat3 *,int v3); +static struct nfs_fh_len *get_rootfh (struct svc_req *,dirpath *,nfs_export **,mountstat3 *,int v3);int reverse_resolve =0 ;int new_cache =0 ;
@@ -192,7 +193,7 @@ mount_mnt_1_svc(struct svc_req *rqstp, dirpath *path, fhstatus *res)struct nfs_fh_len *fh;xlog (D_CALL ,"MNT1(%s) called" , *path); - fh =get_rootfh (rqstp, path, &res->fhs_status,0 ); + fh =get_rootfh (rqstp, path,NULL , &res->fhs_status,0 );if (fh)memcpy (&res->fhstatus_u.fhs_fhandle, fh->fh_handle,32 );return 1 ;
@@ -330,39 +331,57 @@ mount_pathconf_2_svc(struct svc_req *rqstp, dirpath *path, ppathcnf *res) }/* +* We should advertise the preferred flavours first. (See RFC 2623 +* section 2.7.) We leave that to the administrator, by advertising +* flavours in the order they were listed in /etc/exports. AUTH_NULL is +* dropped from the list to avoid backward compatibility issue with +* older Linux clients, who inspect the list in reversed order. +* +* XXX: It might be more helpful to rearrange these so that flavors +* giving more access (as determined from readonly and id-squashing +* options) come first. (If we decide to do that we should probably do +* that when reading the exports rather than here.) +*/ +static void set_authflavors (struct mountres3_ok *ok,nfs_export *exp) +{ +struct sec_entry *s; +static int flavors[SECFLAVOR_COUNT ]; +int i =0 ; + +for (s = exp->m_export.e_secinfo; s->flav; s++) { +if (s->flav->fnum ==AUTH_NULL ) +continue ; + flavors[i] = s->flav->fnum; + i++; + } + ok->auth_flavors.auth_flavors_val = flavors; + ok->auth_flavors.auth_flavors_len = i; +} + +/* * NFSv3 MOUNT procedure */ bool_t mount_mnt_3_svc (struct svc_req *rqstp,dirpath *path,mountres3 *res) { -#define AUTH_GSS_KRB5 390003 -#define AUTH_GSS_KRB5I 390004 -#define AUTH_GSS_KRB5P 390005 -static int flavors[] = {AUTH_UNIX ,AUTH_GSS_KRB5 ,AUTH_GSS_KRB5I ,AUTH_GSS_KRB5P }; -/* -* We should advertise the preferred flavours first. (See RFC 2623 -* section 2.7.) AUTH_UNIX is arbitrarily ranked over the GSS's. -* AUTH_NULL is dropped from the list to avoid backward compatibility -* issue with older Linux clients, who inspect the list in reversed -* order. -*/ struct mountres3_ok *ok = &res->mountres3_u.mountinfo; +nfs_export *exp;struct nfs_fh_len *fh;xlog (D_CALL ,"MNT3(%s) called" , *path); - fh =get_rootfh (rqstp, path, &res->fhs_status,1 ); + fh =get_rootfh (rqstp, path, &exp, &res->fhs_status,1 );if (!fh)return 1 ; ok->fhandle.fhandle3_len = fh->fh_size; ok->fhandle.fhandle3_val = (char *)fh->fh_handle; - ok->auth_flavors.auth_flavors_len =sizeof (flavors)/sizeof (flavors[0 ]); - ok->auth_flavors.auth_flavors_val = flavors; +set_authflavors (ok, exp);return 1 ; }static struct nfs_fh_len * -get_rootfh (struct svc_req *rqstp,dirpath *path,mountstat3 *error,int v3) +get_rootfh (struct svc_req *rqstp,dirpath *path,nfs_export **expret, +mountstat3 *error,int v3) {struct sockaddr_in *sin = (struct sockaddr_in *)svc_getcaller (rqstp->rq_xprt);
@@ -467,6 +486,8 @@ get_rootfh(struct svc_req *rqstp, dirpath *path, mountstat3 *error, int v3) } *error =NFS_OK ;mountlist_add (inet_ntoa (sin->sin_addr), p); +if (expret) + *expret = exp;return fh; }
Signature · 1 line
1.5.5.rc1
Re: [PATCH 3/3] Determine supported pseudoflavors from export
Quoted 6 lines · Bruce Fields
From: J. Bruce Fields <[email protected]>
Instead of using a static list of supported flavors, we should be taking the list from the export.
Signed-off-by J. Bruce Fields <[email protected]>
Committed...
steved.
Re: [PATCH 2/3] Minor mountd.c cleanup
In reply to Bruce FieldsQuoted 7 lines · Bruce Fields
From: J. Bruce Fields <[email protected]>
I find it more readable to have the normal (non-error) case unindented, and to keep conditionals relatively simple, as is the usual kernel style. Fix some inconsistent indentation while we're there.
Signed-off-by J. Bruce Fields <[email protected]>
Committed...
steved.
Re: [PATCH 1/3] Remove redundant m_path field
In reply to Bruce FieldsQuoted 10 lines · Bruce Fields
From: J. Bruce Fields <[email protected]>
Contrary to the comment above its definition, the field
m_pathalways has the same value ase_path: the *only* modifications ofm_pathare all of the form:strncpy(exp->m_export.m_path, exp->m_export.e_path, sizeof (exp->m_export.m_path) - 1); exp->m_export.m_path[sizeof (exp->m_export.m_path) - 1] = '\0';
Quoted 5 lines
So
m_pathis always just a copy ofe_path. In places where we need to store a path to a submount of a CROSSMNT-exported filesystem, as incache.c, we just use a local variable.Signed-off-by J. Bruce Fields <[email protected]>
Committed....
steved.
PATCH 1/1 remove idmapd.conf
In reply to Bruce FieldsFrom: J. Bruce Fields <[email protected]>
The example idmapd.conf file is kept in libnfsidmap now, which is what's responsible for parsing it anyway.
--- utils/idmapd/idmapd.conf | 10 ---------- 1 files changed, 0 insertions(+), 10 deletions(-) delete mode 100644 utils/idmapd/idmapd.conf
Oh, also, this is unrelated, but: I just noticed I had this still lying around.
--b.
utils/idmapd/idmapd.conf deleted +0 -10
@@ -1,10 +0,0 @@ -[General] - -Verbosity = 0 -Pipefs-Directory = /var/lib/nfs/rpc_pipefs -Domain = localdomain - -[Mapping] - -Nobody-User = nobody -Nobody-Group = nobody
Signature · 1 line
1.5.5.rc1
Re: [PATCH] remove idmapd.conf
Quoted 6 lines · J. Bruce Fields
From: J. Bruce Fields <[email protected]>
The example idmapd.conf file is kept in libnfsidmap now, which is what's responsible for parsing it anyway.
Signed-off-by J. Bruce Fields <[email protected]>
Committed...
steved.