| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 24 Sep 2015 16:01:46 -0400
From: bfields@...ldses.org (J. Bruce Fields)
To: Andreas Gruenbacher <agruenba@...hat.com>
Cc: linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
linux-nfs@...r.kernel.org, linux-api@...r.kernel.org,
linux-cifs@...r.kernel.org, linux-security-module@...r.kernel.org
Subject: Re: [RFC v7 32/41] nfsd: Add support for the MAY_CREATE_{FILE,DIR}
permissions
On Sat, Sep 05, 2015 at 12:27:27PM +0200, Andreas Gruenbacher wrote:
> For local file systems, the vfs performs the necessary permission checks
> for operations like creating files and directories. NFSd duplicates
> several of those checks. The vfs checks have been extended to check for
> additional permissions like MAY_CREATE_FILE and MY_CREATE_DIR; the nfsd
> checks currently lack those extensions.
>
> Ideally, all duplicate checks should be removed; for now, just fix the
> duplicate checks instead though.
ACK to the patch, but yes this probably worth some more investigation at
some point.
--b.
>
> Signed-off-by: Andreas Gruenbacher <agruenba@...hat.com>
> ---
> fs/nfsd/nfs4proc.c | 5 +++--
> fs/nfsd/nfsfh.c | 8 ++++----
> fs/nfsd/vfs.c | 28 ++++++++++++++++++++--------
> fs/nfsd/vfs.h | 17 +++++++++--------
> 4 files changed, 36 insertions(+), 22 deletions(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index ef9e6cd..5213945 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -601,14 +601,15 @@ static __be32
> nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> struct nfsd4_create *create)
> {
> + int access = create->cr_type == NF4DIR ?
> + NFSD_MAY_CREATE_DIR : NFSD_MAY_CREATE_FILE;
> struct svc_fh resfh;
> __be32 status;
> dev_t rdev;
>
> fh_init(&resfh, NFS4_FHSIZE);
>
> - status = fh_verify(rqstp, &cstate->current_fh, S_IFDIR,
> - NFSD_MAY_CREATE);
> + status = fh_verify(rqstp, &cstate->current_fh, S_IFDIR, access);
> if (status)
> return status;
>
> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> index 350041a..7159316 100644
> --- a/fs/nfsd/nfsfh.c
> +++ b/fs/nfsd/nfsfh.c
> @@ -319,10 +319,10 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> /*
> * We still have to do all these permission checks, even when
> * fh_dentry is already set:
> - * - fh_verify may be called multiple times with different
> - * "access" arguments (e.g. nfsd_proc_create calls
> - * fh_verify(...,NFSD_MAY_EXEC) first, then later (in
> - * nfsd_create) calls fh_verify(...,NFSD_MAY_CREATE).
> + * - fh_verify may be called multiple times with different
> + * "access" arguments (e.g. nfsd_proc_create calls
> + * fh_verify(...,NFSD_MAY_EXEC) first, then later (in
> + * nfsd_create) calls fh_verify(...,NFSD_MAY_CREATE_FILE).
> * - in the NFSv4 case, the filehandle may have been filled
> * in by fh_compose, and given a dentry, but further
> * compound operations performed with that filehandle
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index b5e077a..bd19096 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1128,6 +1128,8 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
> __be32 err;
> __be32 err2;
> int host_err;
> + int access = (type == S_IFDIR) ?
> + NFSD_MAY_CREATE_DIR : NFSD_MAY_CREATE_FILE;
>
> err = nfserr_perm;
> if (!flen)
> @@ -1136,7 +1138,7 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
> if (isdotent(fname, flen))
> goto out;
>
> - err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_CREATE);
> + err = fh_verify(rqstp, fhp, S_IFDIR, access);
> if (err)
> goto out;
>
> @@ -1307,7 +1309,7 @@ do_nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
>
> /* If file doesn't exist, check for permissions to create one */
> if (d_really_is_negative(dchild)) {
> - err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_CREATE);
> + err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_CREATE_FILE);
> if (err)
> goto out;
> }
> @@ -1491,7 +1493,7 @@ nfsd_symlink(struct svc_rqst *rqstp, struct svc_fh *fhp,
> if (isdotent(fname, flen))
> goto out;
>
> - err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_CREATE);
> + err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_CREATE_FILE);
> if (err)
> goto out;
>
> @@ -1538,7 +1540,7 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp,
> __be32 err;
> int host_err;
>
> - err = fh_verify(rqstp, ffhp, S_IFDIR, NFSD_MAY_CREATE);
> + err = fh_verify(rqstp, ffhp, S_IFDIR, NFSD_MAY_CREATE_FILE);
> if (err)
> goto out;
> err = fh_verify(rqstp, tfhp, 0, NFSD_MAY_NOP);
> @@ -1610,11 +1612,12 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> struct inode *fdir, *tdir;
> __be32 err;
> int host_err;
> + int access;
>
> err = fh_verify(rqstp, ffhp, S_IFDIR, NFSD_MAY_REMOVE);
> if (err)
> goto out;
> - err = fh_verify(rqstp, tfhp, S_IFDIR, NFSD_MAY_CREATE);
> + err = fh_verify(rqstp, tfhp, S_IFDIR, NFSD_MAY_NOP);
> if (err)
> goto out;
>
> @@ -1653,6 +1656,13 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> if (odentry == trap)
> goto out_dput_old;
>
> + host_err = 0;
> + access = S_ISDIR(d_inode(odentry)->i_mode) ?
> + NFSD_MAY_CREATE_DIR : NFSD_MAY_CREATE_FILE;
> + err = fh_verify(rqstp, tfhp, S_IFDIR, access);
> + if (err)
> + goto out_dput_old;
> +
> ndentry = lookup_one_len(tname, tdentry, tlen);
> host_err = PTR_ERR(ndentry);
> if (IS_ERR(ndentry))
> @@ -1678,7 +1688,8 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> out_dput_old:
> dput(odentry);
> out_nfserr:
> - err = nfserrno(host_err);
> + if (host_err)
> + err = nfserrno(host_err);
> /*
> * We cannot rely on fh_unlock on the two filehandles,
> * as that would do the wrong thing if the two directories
> @@ -2011,8 +2022,9 @@ nfsd_permission(struct svc_rqst *rqstp, struct svc_export *exp,
> uid_eq(inode->i_uid, current_fsuid()))
> return 0;
>
> - /* This assumes NFSD_MAY_{READ,WRITE,EXEC} == MAY_{READ,WRITE,EXEC} */
> - err = inode_permission(inode, acc & (MAY_READ|MAY_WRITE|MAY_EXEC));
> + /* This assumes NFSD_MAY_{READ,WRITE,EXEC} == MAY_{READ,WRITE,EXEC}. */
> + err = inode_permission(inode, acc & (MAY_READ|MAY_WRITE|MAY_EXEC|
> + MAY_CREATE_DIR|MAY_CREATE_FILE));
>
> /* Allow read access to binaries even when mode 111 */
> if (err == -EACCES && S_ISREG(inode->i_mode) &&
> diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> index 5be875e..384fda4 100644
> --- a/fs/nfsd/vfs.h
> +++ b/fs/nfsd/vfs.h
> @@ -19,18 +19,19 @@
> #define NFSD_MAY_TRUNC 0x010
> #define NFSD_MAY_LOCK 0x020
> #define NFSD_MAY_MASK 0x03f
> +#define NFSD_MAY_CREATE_FILE 0x103 /* == MAY_{EXEC|WRITE|CREATE_FILE} */
> +#define NFSD_MAY_CREATE_DIR 0x203 /* == MAY_{EXEC|WRITE|CREATE_DIR} */
>
> /* extra hints to permission and open routines: */
> -#define NFSD_MAY_OWNER_OVERRIDE 0x040
> -#define NFSD_MAY_LOCAL_ACCESS 0x080 /* for device special files */
> -#define NFSD_MAY_BYPASS_GSS_ON_ROOT 0x100
> -#define NFSD_MAY_NOT_BREAK_LEASE 0x200
> -#define NFSD_MAY_BYPASS_GSS 0x400
> -#define NFSD_MAY_READ_IF_EXEC 0x800
> +#define NFSD_MAY_OWNER_OVERRIDE 0x04000
> +#define NFSD_MAY_LOCAL_ACCESS 0x08000 /* for device special files */
> +#define NFSD_MAY_BYPASS_GSS_ON_ROOT 0x10000
> +#define NFSD_MAY_NOT_BREAK_LEASE 0x20000
> +#define NFSD_MAY_BYPASS_GSS 0x40000
> +#define NFSD_MAY_READ_IF_EXEC 0x80000
>
> -#define NFSD_MAY_64BIT_COOKIE 0x1000 /* 64 bit readdir cookies for >= NFSv3 */
> +#define NFSD_MAY_64BIT_COOKIE 0x100000 /* 64 bit readdir cookies for >= NFSv3 */
>
> -#define NFSD_MAY_CREATE (NFSD_MAY_EXEC|NFSD_MAY_WRITE)
> #define NFSD_MAY_REMOVE (NFSD_MAY_EXEC|NFSD_MAY_WRITE|NFSD_MAY_TRUNC)
>
> /*
> --
> 2.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists