[Project Quota]file owner could change its project ID?

[Project Quota]file owner could change its project ID?

[Wang Shilong]

Steps to reproduce:
[wangsl@localhost tmp]$ mkdir project
[wangsl@localhost tmp]$ lsattr -p project -d
    0 ------------------ project
[wangsl@localhost tmp]$ chattr -p 1 project
[wangsl@localhost tmp]$ lsattr -p -d project
    1 ------------------ project
[wangsl@localhost tmp]$ chattr -p 2 project
[wangsl@localhost tmp]$ lsattr -p -d project
    2 ------------------ project
[wangsl@localhost tmp]$ df -Th .
Filesystem     Type  Size  Used Avail Use% Mounted on
/dev/sda3      xfs    36G  4.1G   32G  12% /
[wangsl@localhost tmp]$ uname -r
5.4.0-rc2+

As above you could see file owner could change project ID of file its self.
As my understanding, we could set project ID and inherit attribute to account
Directory usage, and implement a similar 'Directory Quota' based on this.

But Directories could easily break this limit by change its file to
other project ID.

And we used vfs_ioc_fssetxattr_check() to only allow init userspace to
change project quota:

        /*

         * Project Quota ID state is only allowed to change from within the init

         * namespace. Enforce that restriction only if we are trying to change

         * the quota ID state. Everything else is allowed in user namespaces.

         */

        if (current_user_ns() != &init_user_ns) {

                if (old_fa->fsx_projid != fa->fsx_projid)

                        return -EINVAL;

                if ((old_fa->fsx_xflags ^ fa->fsx_xflags) &

                                FS_XFLAG_PROJINHERIT)

                        return -EINVAL;

        }

Shall we have something like following to limit admin change for
Project state too?

diff --git a/fs/inode.c b/fs/inode.c

index fef457a42882..3e324931ee84 100644

--- a/fs/inode.c

+++ b/fs/inode.c

@@ -2273,7 +2273,7 @@ int vfs_ioc_fssetxattr_check(struct inode
*inode, const struct fsxattr *old_fa,

         * namespace. Enforce that restriction only if we are trying to change

         * the quota ID state. Everything else is allowed in user namespaces.

         */

-       if (current_user_ns() != &init_user_ns) {

+       if (current_user_ns() != &init_user_ns || !capable(CAP_SYS_ADMIN)){

                if (old_fa->fsx_projid != fa->fsx_projid)

                        return -EINVAL;

                if ((old_fa->fsx_xflags ^ fa->fsx_xflags) &

Re: [Project Quota]file owner could change its project ID?

[Dave Chinner]

On Sat, Oct 12, 2019 at 02:33:36PM +0800, Wang Shilong wrote:
> Steps to reproduce:
> [wangsl@localhost tmp]$ mkdir project
> [wangsl@localhost tmp]$ lsattr -p project -d
>     0 ------------------ project
> [wangsl@localhost tmp]$ chattr -p 1 project
> [wangsl@localhost tmp]$ lsattr -p -d project
>     1 ------------------ project
> [wangsl@localhost tmp]$ chattr -p 2 project
> [wangsl@localhost tmp]$ lsattr -p -d project
>     2 ------------------ project
> [wangsl@localhost tmp]$ df -Th .
> Filesystem     Type  Size  Used Avail Use% Mounted on
> /dev/sda3      xfs    36G  4.1G   32G  12% /
> [wangsl@localhost tmp]$ uname -r
> 5.4.0-rc2+
> 
> As above you could see file owner could change project ID of file its self.

Perfectly legal to do this. Working as designed, and intended.
Project quotas have allowed this since they were introduced in the
late 1980s on Irix...

> As my understanding, we could set project ID and inherit attribute to account
> Directory usage, and implement a similar 'Directory Quota' based on this.

Yes, that is a -use- of project quotas, but not the -only- use.
Users have always been allowed to select what project their files
belong to - it's a method of accounting space even when users cannot
access all of the project information.

> But Directories could easily break this limit by change its file to
> other project ID.

Yes. But then users are using the project quotas as traditional
project quotas, which is just fine. There are users out there that
use this mix of behaviours - users have a default directory quota to
limit unbound space usage of home directories, but for files that
belong to a specific project they simply change the project ID and
now the space in their home directory those project files take up is
accounted to the project rather than the user.

> And we used vfs_ioc_fssetxattr_check() to only allow init userspace to
> change project quota:

Right, that's so directory quotas can be enforced for container
use cases, preventing the users inside a container from modifying
the project quota and escaping the space usage limit set for the
container. This means project quotas are unavailable for use
inside user namespaces as they are used for container resource usage
limiting.

> Shall we have something like following to limit admin change for
> Project state too?

As per above, that will break many existing use cases for project
quotas, so I don't think we will be doing that.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [Project Quota]file owner could change its project ID?

[Darrick J. Wong]

On Sat, Oct 12, 2019 at 02:33:36PM +0800, Wang Shilong wrote:
> Steps to reproduce:
> [wangsl@localhost tmp]$ mkdir project
> [wangsl@localhost tmp]$ lsattr -p project -d
>     0 ------------------ project
> [wangsl@localhost tmp]$ chattr -p 1 project
> [wangsl@localhost tmp]$ lsattr -p -d project
>     1 ------------------ project
> [wangsl@localhost tmp]$ chattr -p 2 project
> [wangsl@localhost tmp]$ lsattr -p -d project
>     2 ------------------ project
> [wangsl@localhost tmp]$ df -Th .
> Filesystem     Type  Size  Used Avail Use% Mounted on
> /dev/sda3      xfs    36G  4.1G   32G  12% /
> [wangsl@localhost tmp]$ uname -r
> 5.4.0-rc2+
> 
> As above you could see file owner could change project ID of file its self.
> As my understanding, we could set project ID and inherit attribute to account
> Directory usage, and implement a similar 'Directory Quota' based on this.

So the problem here is that the admin sets up a project quota on a
directory, then non-container users change the project id and thereby
break quota enforcement?  Dave didn't sound at all enthusiastic, but I'm
still wondering what exactly you're trying to prevent.

(Which is to say, maybe we introduce a mount option to prevent changing
projid if project quota *enforcement* is enabled?)

--D

> But Directories could easily break this limit by change its file to
> other project ID.
> 
> And we used vfs_ioc_fssetxattr_check() to only allow init userspace to
> change project quota:
> 
>         /*
> 
>          * Project Quota ID state is only allowed to change from within the init
> 
>          * namespace. Enforce that restriction only if we are trying to change
> 
>          * the quota ID state. Everything else is allowed in user namespaces.
> 
>          */
> 
>         if (current_user_ns() != &init_user_ns) {
> 
>                 if (old_fa->fsx_projid != fa->fsx_projid)
> 
>                         return -EINVAL;
> 
>                 if ((old_fa->fsx_xflags ^ fa->fsx_xflags) &
> 
>                                 FS_XFLAG_PROJINHERIT)
> 
>                         return -EINVAL;
> 
>         }
> 
> Shall we have something like following to limit admin change for
> Project state too?
> 
> diff --git a/fs/inode.c b/fs/inode.c
> 
> index fef457a42882..3e324931ee84 100644
> 
> --- a/fs/inode.c
> 
> +++ b/fs/inode.c
> 
> @@ -2273,7 +2273,7 @@ int vfs_ioc_fssetxattr_check(struct inode
> *inode, const struct fsxattr *old_fa,
> 
>          * namespace. Enforce that restriction only if we are trying to change
> 
>          * the quota ID state. Everything else is allowed in user namespaces.
> 
>          */
> 
> -       if (current_user_ns() != &init_user_ns) {
> 
> +       if (current_user_ns() != &init_user_ns || !capable(CAP_SYS_ADMIN)){
> 
>                 if (old_fa->fsx_projid != fa->fsx_projid)
> 
>                         return -EINVAL;
> 
>                 if ((old_fa->fsx_xflags ^ fa->fsx_xflags) &

Re: [Project Quota]file owner could change its project ID?

[Wang Shilong]

On Mon, Oct 14, 2019 at 12:41 AM Darrick J. Wong
<darrick.wong@oracle.com> wrote:
>
> On Sat, Oct 12, 2019 at 02:33:36PM +0800, Wang Shilong wrote:
> > Steps to reproduce:
> > [wangsl@localhost tmp]$ mkdir project
> > [wangsl@localhost tmp]$ lsattr -p project -d
> >     0 ------------------ project
> > [wangsl@localhost tmp]$ chattr -p 1 project
> > [wangsl@localhost tmp]$ lsattr -p -d project
> >     1 ------------------ project
> > [wangsl@localhost tmp]$ chattr -p 2 project
> > [wangsl@localhost tmp]$ lsattr -p -d project
> >     2 ------------------ project
> > [wangsl@localhost tmp]$ df -Th .
> > Filesystem     Type  Size  Used Avail Use% Mounted on
> > /dev/sda3      xfs    36G  4.1G   32G  12% /
> > [wangsl@localhost tmp]$ uname -r
> > 5.4.0-rc2+
> >
> > As above you could see file owner could change project ID of file its self.
> > As my understanding, we could set project ID and inherit attribute to account
> > Directory usage, and implement a similar 'Directory Quota' based on this.
>
> So the problem here is that the admin sets up a project quota on a
> directory, then non-container users change the project id and thereby
> break quota enforcement?  Dave didn't sound at all enthusiastic, but I'm
> still wondering what exactly you're trying to prevent.

Yup, we are trying to prevent no-root users to change their project ID.
As we want to implement 'Directory Quota':

If non-root users could change their project ID, they could always try
to change its project ID to steal space when EDQUOT returns.

Yup, if mount option could be introduced to make this case work,
that will be nice.


>
> (Which is to say, maybe we introduce a mount option to prevent changing
> projid if project quota *enforcement* is enabled?)
>
> --D
>

Re: [Project Quota]file owner could change its project ID?

[Dave Chinner]

On Wed, Oct 16, 2019 at 07:51:15PM +0800, Wang Shilong wrote:
> On Mon, Oct 14, 2019 at 12:41 AM Darrick J. Wong
> <darrick.wong@oracle.com> wrote:
> >
> > On Sat, Oct 12, 2019 at 02:33:36PM +0800, Wang Shilong wrote:
> > > Steps to reproduce:
> > > [wangsl@localhost tmp]$ mkdir project
> > > [wangsl@localhost tmp]$ lsattr -p project -d
> > >     0 ------------------ project
> > > [wangsl@localhost tmp]$ chattr -p 1 project
> > > [wangsl@localhost tmp]$ lsattr -p -d project
> > >     1 ------------------ project
> > > [wangsl@localhost tmp]$ chattr -p 2 project
> > > [wangsl@localhost tmp]$ lsattr -p -d project
> > >     2 ------------------ project
> > > [wangsl@localhost tmp]$ df -Th .
> > > Filesystem     Type  Size  Used Avail Use% Mounted on
> > > /dev/sda3      xfs    36G  4.1G   32G  12% /
> > > [wangsl@localhost tmp]$ uname -r
> > > 5.4.0-rc2+
> > >
> > > As above you could see file owner could change project ID of file its self.
> > > As my understanding, we could set project ID and inherit attribute to account
> > > Directory usage, and implement a similar 'Directory Quota' based on this.
> >
> > So the problem here is that the admin sets up a project quota on a
> > directory, then non-container users change the project id and thereby
> > break quota enforcement?  Dave didn't sound at all enthusiastic, but I'm
> > still wondering what exactly you're trying to prevent.
> 
> Yup, we are trying to prevent no-root users to change their project ID.
> As we want to implement 'Directory Quota':

You can already implement "Directory Quota" with project quotas.

What you are asking for is a different implementation of "Directory
Quota" to what we currently support.

> If non-root users could change their project ID, they could always try
> to change its project ID to steal space when EDQUOT returns.

Audit trail will tell you if users are doing this :P

Basically,  what you want is a -strict- directory quota, where users
are unable to select the quota ID the directory is placed under.
i.e. it is entirely admin controlled, and users can't place
different directories under different quota IDs.

We can do this, but we need and explicit description of the
behaviour your require, and how it differs from the existing
"directory quota via project quotas" mechanism that people currently
use. i.e. there are two very different use cases for "directory
quotas" here, and users are going to need to know which one to
select for their intended us. 

> Yup, if mount option could be introduced to make this case work,
> that will be nice.

That's maybe ten  lines of code. I expect that the documentation of
the use cases and the differences in behaviour between the two modes
of behaviour will be a couple of hundred lines at least...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [Project Quota]file owner could change its project ID?

[Darrick J. Wong]

On Wed, Oct 16, 2019 at 07:51:15PM +0800, Wang Shilong wrote:
> On Mon, Oct 14, 2019 at 12:41 AM Darrick J. Wong
> <darrick.wong@oracle.com> wrote:
> >
> > On Sat, Oct 12, 2019 at 02:33:36PM +0800, Wang Shilong wrote:
> > > Steps to reproduce:
> > > [wangsl@localhost tmp]$ mkdir project
> > > [wangsl@localhost tmp]$ lsattr -p project -d
> > >     0 ------------------ project
> > > [wangsl@localhost tmp]$ chattr -p 1 project
> > > [wangsl@localhost tmp]$ lsattr -p -d project
> > >     1 ------------------ project
> > > [wangsl@localhost tmp]$ chattr -p 2 project
> > > [wangsl@localhost tmp]$ lsattr -p -d project
> > >     2 ------------------ project
> > > [wangsl@localhost tmp]$ df -Th .
> > > Filesystem     Type  Size  Used Avail Use% Mounted on
> > > /dev/sda3      xfs    36G  4.1G   32G  12% /
> > > [wangsl@localhost tmp]$ uname -r
> > > 5.4.0-rc2+
> > >
> > > As above you could see file owner could change project ID of file its self.
> > > As my understanding, we could set project ID and inherit attribute to account
> > > Directory usage, and implement a similar 'Directory Quota' based on this.
> >
> > So the problem here is that the admin sets up a project quota on a
> > directory, then non-container users change the project id and thereby
> > break quota enforcement?  Dave didn't sound at all enthusiastic, but I'm
> > still wondering what exactly you're trying to prevent.
> 
> Yup, we are trying to prevent no-root users to change their project ID.
> As we want to implement 'Directory Quota':
> 
> If non-root users could change their project ID, they could always try
> to change its project ID to steal space when EDQUOT returns.
> 
> Yup, if mount option could be introduced to make this case work,
> that will be nice.

Well then we had better discuss and write down the exact behaviors of
this new directory quota feature and how it differs from ye olde project
quota.  Here's the existing definition of project quotas in the
xfs_quota manpage:

       10.    XFS  supports  the notion of project quota, which can be
              used to implement a form of directory tree  quota  (i.e.
              to  restrict  a directory tree to only being able to use
              up a component of the filesystems  available  space;  or
              simply  to  keep  track  of the amount of space used, or
              number of inodes, within the tree).

First, we probably ought to add the following to that definition to
reflect a few pieces of current reality:

"Processes running inside runtime environments using mapped user or
group ids, such as container runtimes, are not allowed to change the
project id and project id inheritance flag of inodes."

What do you all think of this starting definition for directory quotas:

       11.    XFS supports the similar notion of directory quota.  The
	      key difference between project and directory quotas is the
	      additional restriction that only a system administrator
	      running outside of a mapped user or group id runtime
	      environment (such as a container runtime) can change the
	      project id and project id inheritenace flag.  This means
	      that unprivileged users are never allowed to manage their
              own directory quotas.

We'd probably enable this with a new 'dirquota' mount option that is
mutually exclusive with the old 'prjquota' option.

--D

> 
> 
> >
> > (Which is to say, maybe we introduce a mount option to prevent changing
> > projid if project quota *enforcement* is enabled?)
> >
> > --D
> >

Re: [Project Quota]file owner could change its project ID?

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 4688 bytes --]

On Oct 16, 2019, at 3:37 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> 
> On Wed, Oct 16, 2019 at 07:51:15PM +0800, Wang Shilong wrote:
>> On Mon, Oct 14, 2019 at 12:41 AM Darrick J. Wong
>> <darrick.wong@oracle.com> wrote:
>>> 
>>> On Sat, Oct 12, 2019 at 02:33:36PM +0800, Wang Shilong wrote:
>>>> Steps to reproduce:
>>>> [wangsl@localhost tmp]$ mkdir project
>>>> [wangsl@localhost tmp]$ lsattr -p project -d
>>>>    0 ------------------ project
>>>> [wangsl@localhost tmp]$ chattr -p 1 project
>>>> [wangsl@localhost tmp]$ lsattr -p -d project
>>>>    1 ------------------ project
>>>> [wangsl@localhost tmp]$ chattr -p 2 project
>>>> [wangsl@localhost tmp]$ lsattr -p -d project
>>>>    2 ------------------ project
>>>> [wangsl@localhost tmp]$ df -Th .
>>>> Filesystem     Type  Size  Used Avail Use% Mounted on
>>>> /dev/sda3      xfs    36G  4.1G   32G  12% /
>>>> [wangsl@localhost tmp]$ uname -r
>>>> 5.4.0-rc2+
>>>> 
>>>> As above you could see file owner could change project ID of file its self.
>>>> As my understanding, we could set project ID and inherit attribute to account
>>>> Directory usage, and implement a similar 'Directory Quota' based on this.
>>> 
>>> So the problem here is that the admin sets up a project quota on a
>>> directory, then non-container users change the project id and thereby
>>> break quota enforcement?  Dave didn't sound at all enthusiastic, but I'm
>>> still wondering what exactly you're trying to prevent.
>> 
>> Yup, we are trying to prevent no-root users to change their project ID.
>> As we want to implement 'Directory Quota':
>> 
>> If non-root users could change their project ID, they could always try
>> to change its project ID to steal space when EDQUOT returns.
>> 
>> Yup, if mount option could be introduced to make this case work,
>> that will be nice.
> 
> Well then we had better discuss and write down the exact behaviors of
> this new directory quota feature and how it differs from ye olde project
> quota.  Here's the existing definition of project quotas in the
> xfs_quota manpage:
> 
>       10.    XFS  supports  the notion of project quota, which can be
>              used to implement a form of directory tree  quota  (i.e.
>              to  restrict  a directory tree to only being able to use
>              up a component of the filesystems  available  space;  or
>              simply  to  keep  track  of the amount of space used, or
>              number of inodes, within the tree).
> 
> First, we probably ought to add the following to that definition to
> reflect a few pieces of current reality:
> 
> "Processes running inside runtime environments using mapped user or
> group ids, such as container runtimes, are not allowed to change the
> project id and project id inheritance flag of inodes."
> 
> What do you all think of this starting definition for directory quotas:
> 
>       11.    XFS supports the similar notion of directory quota.  The
> 	      key difference between project and directory quotas is the
> 	      additional restriction that only a system administrator
> 	      running outside of a mapped user or group id runtime
> 	      environment (such as a container runtime) can change the
> 	      project id and project id inheritenace flag.  This means
> 	      that unprivileged users are never allowed to manage their
>              own directory quotas.
> 
> We'd probably enable this with a new 'dirquota' mount option that is
> mutually exclusive with the old 'prjquota' option.

I don't think that this is really "directory quotas" in the end, since it
isn't changing the semantics that the same projid could exist in multiple
directory trees.  The real difference is the ability to enforce existing
project quota limits for regular users outside of a container.  Basically,
it is the same as regular users not being able to change the UID of their
files to dump quota to some other user.

So rather than rename this "dirquota", it would be better to have a
an option like "projid_enforce" or "projid_restrict", or maybe some
more flexibility to allow only users in specific groups to change the
projid like "projid_admin=<gid>" so that e.g. "staff" or "admin" groups
can still change it (in addition to root) but not regular users.  To
restrict it to root only, leave "projid_admin=0" and the default (to
keep the same "everyone can change projid" behavior) would be -1?

Cheers, Andreas

>>> (Which is to say, maybe we introduce a mount option to prevent changing
>>> projid if project quota *enforcement* is enabled?)
>>> 
>>> --D


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [Project Quota]file owner could change its project ID?

[Theodore Y. Ts'o]

On Wed, Oct 16, 2019 at 06:28:08PM -0600, Andreas Dilger wrote:
> I don't think that this is really "directory quotas" in the end, since it
> isn't changing the semantics that the same projid could exist in multiple
> directory trees.  The real difference is the ability to enforce existing
> project quota limits for regular users outside of a container.  Basically,
> it is the same as regular users not being able to change the UID of their
> files to dump quota to some other user.
> 
> So rather than rename this "dirquota", it would be better to have a
> an option like "projid_enforce" or "projid_restrict", or maybe some
> more flexibility to allow only users in specific groups to change the
> projid like "projid_admin=<gid>" so that e.g. "staff" or "admin" groups
> can still change it (in addition to root) but not regular users.  To
> restrict it to root only, leave "projid_admin=0" and the default (to
> keep the same "everyone can change projid" behavior) would be -1?

I'm not sure how common the need for restsrictive quota enforcement is
really going to be.  Can someone convince me this is actually going to
be a common use case?

We could also solve the problem by adding an LSM hook called when
there is an attempt to set the project ID, and for people who really
want this, they can create a stackable LSM which enforces whatever
behavior they want.

If we think this going to be an speciality request, this might be the
better way to go.

						- Ted

Re: [Project Quota]file owner could change its project ID?

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 2457 bytes --]

On Oct 17, 2019, at 6:12 AM, Theodore Y. Ts'o <tytso@mit.edu> wrote:
> 
> On Wed, Oct 16, 2019 at 06:28:08PM -0600, Andreas Dilger wrote:
>> I don't think that this is really "directory quotas" in the end, since it
>> isn't changing the semantics that the same projid could exist in multiple
>> directory trees.  The real difference is the ability to enforce existing
>> project quota limits for regular users outside of a container.  Basically,
>> it is the same as regular users not being able to change the UID of their
>> files to dump quota to some other user.
>> 
>> So rather than rename this "dirquota", it would be better to have a
>> an option like "projid_enforce" or "projid_restrict", or maybe some
>> more flexibility to allow only users in specific groups to change the
>> projid like "projid_admin=<gid>" so that e.g. "staff" or "admin" groups
>> can still change it (in addition to root) but not regular users.  To
>> restrict it to root only, leave "projid_admin=0" and the default (to
>> keep the same "everyone can change projid" behavior) would be -1?
> 
> I'm not sure how common the need for restsrictive quota enforcement is
> really going to be.  Can someone convince me this is actually going to
> be a common use case?

Project quota (i.e. quota tracking that doesn't automatically also convey
permission to access a file or directory) is one of the most requested
features from our users.  This is useful for e.g. university or industry
research groups with multiple grad students/researchers under a single
principal professor/project that controls the funding.

> We could also solve the problem by adding an LSM hook called when
> there is an attempt to set the project ID, and for people who really
> want this, they can create a stackable LSM which enforces whatever
> behavior they want.

So, rather than add a few-line change that decides whether the user
is allowed to change the projid for a file, we would instead add *more*
lines to add a hook, then have to write and load an LSM that is called
each time?  That seems backward to me.

> If we think this going to be an speciality request, this might be the
> better way to go.

I don't see how this is more "speciality" than regular quota enforcement?
Just like we impose limits on users and groups, it makes sense to impose
a limit on a project, instead of just tracking it and then having to add
extra machinery to impose the limit externally.

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [Project Quota]file owner could change its project ID?

[Theodore Y. Ts'o]

On Sun, Oct 20, 2019 at 02:19:19PM -0600, Andreas Dilger wrote:
> > We could also solve the problem by adding an LSM hook called when
> > there is an attempt to set the project ID, and for people who really
> > want this, they can create a stackable LSM which enforces whatever
> > behavior they want.
> 
> So, rather than add a few-line change that decides whether the user
> is allowed to change the projid for a file, we would instead add *more*
> lines to add a hook, then have to write and load an LSM that is called
> each time?  That seems backward to me.

I'm just not sure we've necessarily gotten the semantics right.  For
example, I could easily see someone else coming out of the woodwork
saying that The Right Model (tm) is one where users belong to a number
of projects (much like supplementary group ID's) and you should be
able to set the project of any file that you own to a project.

Or perhaps the policy is that you can only change the project ID if
the project ID has a non-zero project quota.  etc.

> > If we think this going to be an speciality request, this might be the
> > better way to go.
> 
> I don't see how this is more "speciality" than regular quota enforcement?
> Just like we impose limits on users and groups, it makes sense to impose
> a limit on a project, instead of just tracking it and then having to add
> extra machinery to impose the limit externally.

We *do* have regular quota enforcement.  The question here has nothing
to do with how quota tracking or enforcement work.  The question is
about what are the authorization checks and policy surrounding when
the project ID can modified.

Right now the policy is "the owner of the file can set the project ID
to any integer if it is issued from the initial user namespace;
otherwise, no changes to the project ID or the PROJINHERIT flag is
allowed".

Making it be "only root in the inital user namespace is allowed change
project ID or PROJINHERIT flag" is certain an alternate policy.  Are
we sure those are the only two possible policies that users might ask
for?

					- Ted

Re: [Project Quota]file owner could change its project ID?

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 5337 bytes --]

On Oct 20, 2019, at 4:25 PM, Theodore Y. Ts'o <tytso@mit.edu> wrote:
> 
> On Sun, Oct 20, 2019 at 02:19:19PM -0600, Andreas Dilger wrote:
>>> We could also solve the problem by adding an LSM hook called when
>>> there is an attempt to set the project ID, and for people who really
>>> want this, they can create a stackable LSM which enforces whatever
>>> behavior they want.
>> 
>> So, rather than add a few-line change that decides whether the user
>> is allowed to change the projid for a file, we would instead add *more*
>> lines to add a hook, then have to write and load an LSM that is called
>> each time?  That seems backward to me.
> 
> I'm just not sure we've necessarily gotten the semantics right.  For
> example, I could easily see someone else coming out of the woodwork
> saying that The Right Model (tm) is one where users belong to a number
> of projects (much like supplementary group ID's) and you should be
> able to set the project of any file that you own to a project.

Definitely I've thought of that kind of behavior, but it needs a much larger
change, and is not clear that anyone actually needs this yet.  It is not
incompatible with the "add an option for root-only (or specific GID-only)
'setprojid' permission", but rather would be an enhancement that could be
added after the fact (there is no need for it with the "free for all" today).
However, I'm uncertain whether any benefit would be had from "supplementary
projid" support as you describe or not.

If a user has write permission in a target directory with a different projid
then they can easily (with "mv" handling the EXDEV case automatically) move
files from a source projid to a target projid by copying the data through
userspace.  In this respect, I wonder why ext4 enforces the "can't rename
to a target directory with a different projid" restriction that XFS has?  It
seems possible (very beneficial even) to change the projid in ext4 when a
file is renamed to a different directory, rather than forcing a full-file
copy in userspace, which can be very slow if the file is large.

Essentially the permission check for rename() could become "if user has write
permission in the target directory with a different projid, then allow the
change" since they could just as easily bypass the permissions in userspace.

According to Dave's original post this is an "XFS implementation detail":
https://www.spinics.net/lists/linux-ext4/msg44738.html

    XFS doesn't transfer the quota from projid to projid because it's
    borderline impossible to correctly track all the metadata
    allocation/free operations that can happen in a rename operation and
    account them to the correct quota. Hence all those corner cases are
    avoided by treating it as EXDEV and forcing userspace to cp/unlink
    the files rather than rename.

    That's really an implementation detail.

If we don't have this problem in ext4 then we don't need to return -EXDEV.

However, that is digressing from the original point of "restrict permission"
rather than "allow for rename".

> Or perhaps the policy is that you can only change the project ID if
> the project ID has a non-zero project quota.  etc.
> 
>>> If we think this going to be an speciality request, this might be the
>>> better way to go.
>> 
>> I don't see how this is more "speciality" than regular quota enforcement?
>> Just like we impose limits on users and groups, it makes sense to impose
>> a limit on a project, instead of just tracking it and then having to add
>> extra machinery to impose the limit externally.
> 
> We *do* have regular quota enforcement.  The question here has nothing
> to do with how quota tracking or enforcement work.  The question is
> about what are the authorization checks and policy surrounding when
> the project ID can modified.

> 
> Right now the policy is "the owner of the file can set the project ID
> to any integer if it is issued from the initial user namespace;
> otherwise, no changes to the project ID or the PROJINHERIT flag is
> allowed".

IMHO, if any user can arbitrarily change the projid of a file, that prevents
effective project quota enforcement.

> Making it be "only root in the inital user namespace is allowed change
> project ID or PROJINHERIT flag" is certain an alternate policy.  Are
> we sure those are the only two possible policies that users might ask
> for?

No, but at the risk of bike-shedding this too much in advance of actual user
need for a more complex mechanism, preventing users from arbitrarily shedding
quota to other projects seems very useful.  The restrictive and traditional
method is to allow only root (uid==0) to do this, but modern systems try to
avoid this by using e.g. CAP_SYS_RESOURCE (which is used for other quota
changes).  A slightly more flexible method would be to allow a group (e.g.
"wheel" or "admin") to also control project IDs for files/directories.  This
would default to GID=0 to start (root only), or could default to GID=-1 (any
group can change) to match the current behavior.  Defaulting to GID=-1 would
have the benefit that the mechanism for restricting "setprojid" could be added
but keep the same behavior as today, but make it easy to change if desired.

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [Project Quota]file owner could change its project ID?

[Dave Chinner]

On Wed, Oct 16, 2019 at 06:28:08PM -0600, Andreas Dilger wrote:
> On Oct 16, 2019, at 3:37 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> > 
> > On Wed, Oct 16, 2019 at 07:51:15PM +0800, Wang Shilong wrote:
> >> On Mon, Oct 14, 2019 at 12:41 AM Darrick J. Wong
> >> <darrick.wong@oracle.com> wrote:
> >>> 
> >>> On Sat, Oct 12, 2019 at 02:33:36PM +0800, Wang Shilong wrote:
> >>>> Steps to reproduce:
> >>>> [wangsl@localhost tmp]$ mkdir project
> >>>> [wangsl@localhost tmp]$ lsattr -p project -d
> >>>>    0 ------------------ project
> >>>> [wangsl@localhost tmp]$ chattr -p 1 project
> >>>> [wangsl@localhost tmp]$ lsattr -p -d project
> >>>>    1 ------------------ project
> >>>> [wangsl@localhost tmp]$ chattr -p 2 project
> >>>> [wangsl@localhost tmp]$ lsattr -p -d project
> >>>>    2 ------------------ project
> >>>> [wangsl@localhost tmp]$ df -Th .
> >>>> Filesystem     Type  Size  Used Avail Use% Mounted on
> >>>> /dev/sda3      xfs    36G  4.1G   32G  12% /
> >>>> [wangsl@localhost tmp]$ uname -r
> >>>> 5.4.0-rc2+
> >>>> 
> >>>> As above you could see file owner could change project ID of file its self.
> >>>> As my understanding, we could set project ID and inherit attribute to account
> >>>> Directory usage, and implement a similar 'Directory Quota' based on this.
> >>> 
> >>> So the problem here is that the admin sets up a project quota on a
> >>> directory, then non-container users change the project id and thereby
> >>> break quota enforcement?  Dave didn't sound at all enthusiastic, but I'm
> >>> still wondering what exactly you're trying to prevent.
> >> 
> >> Yup, we are trying to prevent no-root users to change their project ID.
> >> As we want to implement 'Directory Quota':
> >> 
> >> If non-root users could change their project ID, they could always try
> >> to change its project ID to steal space when EDQUOT returns.
> >> 
> >> Yup, if mount option could be introduced to make this case work,
> >> that will be nice.
> > 
> > Well then we had better discuss and write down the exact behaviors of
> > this new directory quota feature and how it differs from ye olde project
> > quota.  Here's the existing definition of project quotas in the
> > xfs_quota manpage:
> > 
> >       10.    XFS  supports  the notion of project quota, which can be
> >              used to implement a form of directory tree  quota  (i.e.
> >              to  restrict  a directory tree to only being able to use
> >              up a component of the filesystems  available  space;  or
> >              simply  to  keep  track  of the amount of space used, or
> >              number of inodes, within the tree).
> > 
> > First, we probably ought to add the following to that definition to
> > reflect a few pieces of current reality:
> > 
> > "Processes running inside runtime environments using mapped user or
> > group ids, such as container runtimes, are not allowed to change the
> > project id and project id inheritance flag of inodes."
> > 
> > What do you all think of this starting definition for directory quotas:
> > 
> >       11.    XFS supports the similar notion of directory quota.  The
> > 	      key difference between project and directory quotas is the
> > 	      additional restriction that only a system administrator
> > 	      running outside of a mapped user or group id runtime
> > 	      environment (such as a container runtime) can change the
> > 	      project id and project id inheritenace flag.  This means
> > 	      that unprivileged users are never allowed to manage their
> >              own directory quotas.
> > 
> > We'd probably enable this with a new 'dirquota' mount option that is
> > mutually exclusive with the old 'prjquota' option.
> 
> I don't think that this is really "directory quotas" in the end, since it
> isn't changing the semantics that the same projid could exist in multiple
> directory trees.


The quota ID associated with a directory is an admin choice - admins
are free to have multiple directories use the same quota ID if
that's how they want to control usage across those directories.

i.e. "directory quota" does not mean "quota IDs must be unique for
different directory heirarchies", it just means quota IDs are
assigned to new directory entries based on the current directory
quota ID.

> The real difference is the ability to enforce existing
> project quota limits for regular users outside of a container.  Basically,
> it is the same as regular users not being able to change the UID of their
> files to dump quota to some other user.

No, no it isn't - project IDs are not user IDs. In fact, UIDs and
permission bits are used to determine if the user has permission to
change the project ID of the file. i.e. anyone who can write data to
the file can change the project ID and "dump quota to some other
user".

That's the way it's always worked, and there are many users out
there that rely on users setting project quotas correctly for their
data sets. i.e. the default project quota is set up as a directory
quota and they are set low to force people creating large data sets
account them to the project that the space is being used for.

IOWs, directory-based project quotas and project-based project quotas
are often used in the same filesystem, and users are expected to
manage them directly.

What is being asked for here is a strict interpretation of directory
quotas to remove the capability of mixing directory and project
based quotas in the one filesystem. That's not the same thing as
an "enforced project quota".

> So rather than rename this "dirquota", it would be better to have a
> an option like "projid_enforce" or "projid_restrict", or maybe some

If only specific admins can change project quotas, then the only way
that project quotas can be used is either:

	1. inherit project ID from parent directory at create time;
	or
	2. admin manually walks newly created files classifying them
	into the correct project after the fact.

#2 is pretty much useless to anyone - who wants to look at thousands
of files a day and classify them to this project or that one? I
haven't seen that admin model in use anywhere in the real world.

Which leaves #1 - default project IDs inherited from the parent
directory, and users can't change them. And that is the very
definition of a strict directory quota...

Quite frankly, people are going to understand what "dirquota" means
and how it behaves intuitively, as opposed to having to understand
what a project quota is, how project IDs work and what the magical
"projid_restrict" mount option does and when you'd want to use it.

They do the same things - one API is easy to understand for users,
the other is a horrible "designed by an engineer to meet a specific
requirement" interface. I know which one I'd prefer as a user...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [Project Quota]file owner could change its project ID?

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 10759 bytes --]

On Oct 21, 2019, at 5:35 PM, Dave Chinner <david@fromorbit.com> wrote:
> 
> On Wed, Oct 16, 2019 at 06:28:08PM -0600, Andreas Dilger wrote:
>> On Oct 16, 2019, at 3:37 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
>>> 
>>> On Wed, Oct 16, 2019 at 07:51:15PM +0800, Wang Shilong wrote:
>>>> On Mon, Oct 14, 2019 at 12:41 AM Darrick J. Wong
>>>> <darrick.wong@oracle.com> wrote:
>>>>> 
>>>>> On Sat, Oct 12, 2019 at 02:33:36PM +0800, Wang Shilong wrote:
>>>>>> Steps to reproduce:
>>>>>> [wangsl@localhost tmp]$ mkdir project
>>>>>> [wangsl@localhost tmp]$ lsattr -p project -d
>>>>>>   0 ------------------ project
>>>>>> [wangsl@localhost tmp]$ chattr -p 1 project
>>>>>> [wangsl@localhost tmp]$ lsattr -p -d project
>>>>>>   1 ------------------ project
>>>>>> [wangsl@localhost tmp]$ chattr -p 2 project
>>>>>> [wangsl@localhost tmp]$ lsattr -p -d project
>>>>>>   2 ------------------ project
>>>>>> [wangsl@localhost tmp]$ df -Th .
>>>>>> Filesystem     Type  Size  Used Avail Use% Mounted on
>>>>>> /dev/sda3      xfs    36G  4.1G   32G  12% /
>>>>>> [wangsl@localhost tmp]$ uname -r
>>>>>> 5.4.0-rc2+
>>>>>> 
>>>>>> As above you could see file owner could change project ID of file its self.
>>>>>> As my understanding, we could set project ID and inherit attribute to account
>>>>>> Directory usage, and implement a similar 'Directory Quota' based on this.
>>>>> 
>>>>> So the problem here is that the admin sets up a project quota on a
>>>>> directory, then non-container users change the project id and thereby
>>>>> break quota enforcement?  Dave didn't sound at all enthusiastic, but I'm
>>>>> still wondering what exactly you're trying to prevent.
>>>> 
>>>> Yup, we are trying to prevent no-root users to change their project ID.
>>>> As we want to implement 'Directory Quota':
>>>> 
>>>> If non-root users could change their project ID, they could always try
>>>> to change its project ID to steal space when EDQUOT returns.
>>>> 
>>>> Yup, if mount option could be introduced to make this case work,
>>>> that will be nice.
>>> 
>>> Well then we had better discuss and write down the exact behaviors of
>>> this new directory quota feature and how it differs from ye olde project
>>> quota.  Here's the existing definition of project quotas in the
>>> xfs_quota manpage:
>>> 
>>>      10.    XFS  supports  the notion of project quota, which can be
>>>             used to implement a form of directory tree  quota  (i.e.
>>>             to  restrict  a directory tree to only being able to use
>>>             up a component of the filesystems  available  space;  or
>>>             simply  to  keep  track  of the amount of space used, or
>>>             number of inodes, within the tree).
>>> 
>>> First, we probably ought to add the following to that definition to
>>> reflect a few pieces of current reality:
>>> 
>>> "Processes running inside runtime environments using mapped user or
>>> group ids, such as container runtimes, are not allowed to change the
>>> project id and project id inheritance flag of inodes."
>>> 
>>> What do you all think of this starting definition for directory quotas:
>>> 
>>>      11.    XFS supports the similar notion of directory quota.  The
>>> 	      key difference between project and directory quotas is the
>>> 	      additional restriction that only a system administrator
>>> 	      running outside of a mapped user or group id runtime
>>> 	      environment (such as a container runtime) can change the
>>> 	      project id and project id inheritenace flag.  This means
>>> 	      that unprivileged users are never allowed to manage their
>>>             own directory quotas.
>>> 
>>> We'd probably enable this with a new 'dirquota' mount option that is
>>> mutually exclusive with the old 'prjquota' option.
>> 
>> I don't think that this is really "directory quotas" in the end, since it
>> isn't changing the semantics that the same projid could exist in multiple
>> directory trees.
> 
> 
> The quota ID associated with a directory is an admin choice - admins
> are free to have multiple directories use the same quota ID if
> that's how they want to control usage across those directories.

Sure, I understand that, and am not suggesting anything change here.

> i.e. "directory quota" does not mean "quota IDs must be unique for
> different directory heirarchies", it just means quota IDs are
> assigned to new directory entries based on the current directory
> quota ID.

Essentially you mean "PROJID_INHERIT" is the difference between
"project quota" and "directory quota"?

>> The real difference is the ability to enforce existing
>> project quota limits for regular users outside of a container.
>> Basically, it is the same as regular users not being able to
>> change the UID of their files to dump quota to some other user.
> 
> No, no it isn't - project IDs are not user IDs. In fact, UIDs and
> permission bits are used to determine if the user has permission to
> change the project ID of the file. i.e. anyone who can write data to
> the file can change the project ID and "dump quota to some other
> user".

I fully understand that project IDs do no controlling file access.
That is IMHO one of the benefits of project quotas - that users can be
grouped together without automatically granting file access permissions.
My point is that people use quotas to track and limit space usage.  In
this regard, "project quotas" are only "project accounting" since can
be set arbitrarily, which inhibits the ability to limit project space
usage.

> That's the way it's always worked, and there are many users out
> there that rely on users setting project quotas correctly for their
> data sets. i.e. the default project quota is set up as a directory
> quota and they are set low to force people creating large data sets
> account them to the project that the space is being used for.

While this may be the way it's always worked, several new sites using
project quotas found this completely unintuitive.  Their objection is
that the benefit of project quota tracking is lost if users can easily
circumvent the tracking itself.  If there are no restrictions on which
projid values can be set on a file, how do users even know if they are
not arbitrarily using conflicting or incorrect projid values?

> IOWs, directory-based project quotas and project-based project quotas
> are often used in the same filesystem, and users are expected to
> manage them directly.
> 
> What is being asked for here is a strict interpretation of directory
> quotas to remove the capability of mixing directory and project
> based quotas in the one filesystem. That's not the same thing as
> an "enforced project quota".

Might I ask how would you implement "enforced project quota"?  Presumably
it mush prevent users from changing the project ID of a file arbitrarily,
as is allowed today?  It isn't a rhetorical question, I'm genuinely
interested what you would do as this is what users are asking for.

>> So rather than rename this "dirquota", it would be better to have a
>> an option like "projid_enforce" or "projid_restrict", or maybe some
> 
> If only specific admins can change project quotas, then the only way
> that project quotas can be used is either:
> 
> 	1. inherit project ID from parent directory at create time;
> 	or
> 	2. admin manually walks newly created files classifying them
> 	into the correct project after the fact.
> 
> #2 is pretty much useless to anyone - who wants to look at thousands
> of files a day and classify them to this project or that one? I
> haven't seen that admin model in use anywhere in the real world.

I wouldn't imagine admins doing this on a file-by-file basis either.

Rather, non-root admins could create new project directories (e.g.
when a new project is added to a system) and/or (in presumably rare
cases) change the projid of an existing tree.  This wouldn't be any
different than admins doing other quota admin, and not needing root
to do it is good security practice.

> Which leaves #1 - default project IDs inherited from the parent
> directory, and users can't change them. And that is the very
> definition of a strict directory quota...

I've previously steered away from using the term "directory quota"
(even though that's what exists in other filesystems) because it
isn't what it is called in XFS.

> Quite frankly, people are going to understand what "dirquota" means
> and how it behaves intuitively, as opposed to having to understand
> what a project quota is, how project IDs work and what the magical
> "projid_restrict" mount option does and when you'd want to use it.

Naming of the mount option aside, I don't see a tangible difference
in complexity for users to understand what a project ID is whether
they are allowed to change it or not?

Note users *could* still change the project ID of files even if project
quota is enforced by moving the files into a different directory that
has a different project ID assigned. The current XFS/ext4 implementation
returns -EXDEV for rename() between directories with different projid,
forcing userspace to do a file copy.  It isn't clear to me that this
restriction is useful (for ext4 at least, you wrote previously that
there were implementation complexities for XFS for this case), since
userspace doing "copy + unlink" is not really a win in my books.

It seems possible to change the projid directly in ext4 on rename() to
that of the target directory, assuming the user has write permission.
That shouldn't be more complex than chgrp() changing the group quota
of a file, or in case the user currently changes the projid directly
and assigns quota to a new project (ext4_ioctl_setproject() calling
__dquot_transfer(inode, <new projid>))?

> They do the same things - one API is easy to understand for users,
> the other is a horrible "designed by an engineer to meet a specific
> requirement" interface. I know which one I'd prefer as a user...

It isn't clear to me what you are unhappy about?  Is it the mount option
naming?  Feel free to suggest something better.  Some alternatives "setprojid=on/off/setprojid_gid=N", "dirquota/nodirquota.dirquota_gid=N",
or something else?  It doesn't seem to me that having a different *name*
for the mount option turns functionality from "intuitive" to "horrible"
in one shot (especially since the admin only has to set this once), so
I'm genuinely at a loss how "chattr -p <any_projid> <file>" (or equivalent
XFS command) being allowed/forbidden for a regular user is a "horrible"?

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]