[GRASS-dev] mapset permissions: only owner should have write permissions

From within GRASS, only the owner of a mapset is allowed to start a

GRASS session in this mapset, i.e. only the owner of a mapset has
write permissions to this mapset. But a new mapset being a folder in
the file system is created with mode 0777, thus granting write
permissions to all. I suggest to change mode from 0777 to 0755 in
G_mkdir() and add mode = 0755 in gis_set.py. Any objections?

Markus M

Hi,

Markus M:

From within GRASS, only the owner of a mapset is allowed to start a
GRASS session in this mapset, i.e. only the owner of a mapset has
write permissions to this mapset. But a new mapset being a folder in
the file system is created with mode 0777, thus granting write
permissions to all. I suggest to change mode from 0777 to 0755 in
G_mkdir() and add mode = 0755 in gis_set.py. Any objections?

It's not created as 777 (I would have noticed that :), that's before
the umask. So for me it gets created as drwxr-xr-x, as expected.

$ man 2 mkdir:

"""
The argument mode specifies the permissions to use. It is modified by
the process's umask in the usual way: the permissions of the created
directory are (mode & ~umask & 0777). Other mode bits of the created
directory depend on the operating system.
[...]
NOTES
Under Linux apart from the permission bits, only the S_ISVTX mode bit
is honored. That is, under Linux the created directory actually gets
mode (mode & ~umask & 01777). See also stat(2).
"""

Hamish

On Sun, Jul 14, 2013 at 10:42 PM, Hamish <hamish_b@yahoo.com> wrote:

Hi,

Markus M:

From within GRASS, only the owner of a mapset is allowed to start a
GRASS session in this mapset, i.e. only the owner of a mapset has
write permissions to this mapset. But a new mapset being a folder in
the file system is created with mode 0777, thus granting write
permissions to all. I suggest to change mode from 0777 to 0755 in
G_mkdir() and add mode = 0755 in gis_set.py. Any objections?

It's not created as 777 (I would have noticed that :), that's before
the umask. So for me it gets created as drwxr-xr-x, as expected.

For me, it gets created as drwxr-xr-x too, but this depends on umask.
G_mkdir() calls (on anything but windows) mkdir(path, 0777) [0]

$ man 2 mkdir:

"""
The argument mode specifies the permissions to use. It is modified by
the process's umask in the usual way: the permissions of the created
directory are (mode & ~umask & 0777). Other mode bits of the created
directory depend on the operating system.

man 2 umask:

"The typical default value for the process umask is S_IWGRP | S_IWOTH
(octal 022)"

which results in the desired drwxr-xr-x permissions. I could not find
information on "typical", and this "typical" might be different on
different systems. I suggest to make sure that the permissions are
really drwxr-xr-x by using "mkdir(path, 0755);".

Markus M

[0] https://trac.osgeo.org/grass/browser/grass/trunk/lib/gis/paths.c#L32

[...]
NOTES
Under Linux apart from the permission bits, only the S_ISVTX mode bit
is honored. That is, under Linux the created directory actually gets
mode (mode & ~umask & 01777). See also stat(2).
"""

Hamish

Markus Metz wrote:

>From within GRASS, only the owner of a mapset is allowed to start a
GRASS session in this mapset, i.e. only the owner of a mapset has
write permissions to this mapset. But a new mapset being a folder in
the file system is created with mode 0777, thus granting write
permissions to all. I suggest to change mode from 0777 to 0755 in
G_mkdir() and add mode = 0755 in gis_set.py. Any objections?

I don't see why GRASS should be special in this regard.

The convention is that programs should allow the user to control read
and write permissions via the umask, while execute permission is
determined by the program.

But the umask can only remove permissions, not add them. So in order
for the permissions to be fully under the control of the user,
programs must use 0777 for directories and executable files, and 0666
for non-executable files.

Programs creating files containing particularly-sensitive information
(e.g. encryption keys) may reasonably impose more restrictive
permissions. Complex programs may allow permissions to be configured
via options and/or configuration files if the umask is too blunt an
instrument (i.e. the program creates different categories of file or
directory, and the desired permissions are likely to differ by
category).

GRASS already includes its own ownership check to prevent users from
shooting each other in the foot with shared directories (by creating
subdirectories which the owner cannot remove). So I don't really see
any reason to enforce the policy a second time through filesystem
permissions.

--
Glynn Clements <glynn@gclements.plus.com>

On Mon, Jul 15, 2013 at 5:55 PM, Glynn Clements
<glynn@gclements.plus.com> wrote:

Markus Metz wrote:

>From within GRASS, only the owner of a mapset is allowed to start a
GRASS session in this mapset, i.e. only the owner of a mapset has
write permissions to this mapset. But a new mapset being a folder in
the file system is created with mode 0777, thus granting write
permissions to all. I suggest to change mode from 0777 to 0755 in
G_mkdir() and add mode = 0755 in gis_set.py. Any objections?

I don't see why GRASS should be special in this regard.

The convention is that programs should allow the user to control read
and write permissions via the umask, while execute permission is
determined by the program.

In this case, would it be ok to enforce umask to 0022 in the start up script?

Programs creating files containing particularly-sensitive information
(e.g. encryption keys) may reasonably impose more restrictive
permissions.

With grass data on a network drive with multi-user access, I would
regard e.g. the contents of the PERMANENT mapset as
particularly-sensitive information.

GRASS already includes its own ownership check to prevent users from
shooting each other in the foot with shared directories (by creating
subdirectories which the owner cannot remove). So I don't really see
any reason to enforce the policy a second time through filesystem
permissions.

An inexperienced user trying to make a backup of a grass dataset,
syncing the wrong way...

Anyway, I withdraw my suggestion to use 0755 as default mode for
mkdir(path, mode). It's probably safer to enforce the system's default
mask directly.

Markus M

Markus Metz wrote:

> The convention is that programs should allow the user to control read
> and write permissions via the umask, while execute permission is
> determined by the program.

In this case, would it be ok to enforce umask to 0022 in the start up script?

Not literally. Those two bits can be *added*, i.e. it can be increased
from e.g. 002 to 022, but if it starts out at e.g. 077, it shouldn't
be lowered to 022.

Changing the umask in the startup script is an improvement over
forcing the mode in the code, as the user can always just change it
back again.

> Programs creating files containing particularly-sensitive information
> (e.g. encryption keys) may reasonably impose more restrictive
> permissions.

With grass data on a network drive with multi-user access, I would
regard e.g. the contents of the PERMANENT mapset as
particularly-sensitive information.

Plenty of sites will treat PERMANENT as public data, to be shared by
all users.

The cases where forced 0600 is reasonable are those where the data is
invariably private, i.e. passwords and encryption keys.

--
Glynn Clements <glynn@gclements.plus.com>

Markus Metz wrote:

In this case, would it be ok to enforce umask to 0022 in the start up
script?

two rules to thumb:

we shouldn't restrict others to the limits of our own imagination.
(if someone wants to set their umask to allow 777 for whatever reason
or need, why not let them?)

it's not broken == don't fix it

... my suggestion is just leave the thing alone. why make work for people?

regards,
Hamish

On Tue, Jul 16, 2013 at 10:38 PM, Hamish <hamish_b@yahoo.com> wrote:

Markus Metz wrote:

In this case, would it be ok to enforce umask to 0022 in the start up
script?

two rules to thumb:

we shouldn't restrict others to the limits of our own imagination.
(if someone wants to set their umask to allow 777 for whatever reason
  or need, why not let them?)

it's not broken == don't fix it

... my suggestion is just leave the thing alone. why make work for people?

Just to clarify, I no longer want to enforce permissions inside GRASS,
but will instead enforce permissions on system level, outside GRASS.
Maybe our shared network GRASS data are a special case.

Markus M

On Tue, Jul 16, 2013 at 8:44 PM, Glynn Clements
<glynn@gclements.plus.com> wrote:

Markus Metz wrote:

...

With grass data on a network drive with multi-user access, I would
regard e.g. the contents of the PERMANENT mapset as
particularly-sensitive information.

Plenty of sites will treat PERMANENT as public data, to be shared by
all users.

Shared yes.
Destroyed no.

The point here is (as experienced on our local shared network
grassdata/ recently):
- GRASS allows users to enter their own mapset(s)
- GRASS allows users to read all mapsets and write into the current (own) one
- GRASS does not allow to modify the mapset of a different user

So far so nice.

Assume that several users belong to the same group. If now the group
write flag is enabled for mapsets, users can delete them even if they
are not their own. This is fine since someone (admin) must have
allowed for this.

Now back to GRASS: A user runs a session in his/her mapset with
group-write enabled. This is against the GRASS internal policy where
others cannot write into your own mapsets with GRASS commands.

Wish for improvement:
When starting a session in a mapset with group/other-write enabled,
issue a warning to inform the user about this in the startup script.
This would follow the "least-surprise" paradigm.
Feasible?

Markus

Markus Neteler wrote:

The point here is (as experienced on our local shared network
grassdata/ recently):
- GRASS allows users to enter their own mapset(s)
- GRASS allows users to read all mapsets and write into the current (own) one
- GRASS does not allow to modify the mapset of a different user

In 7.0, the last one can be suppressed by setting the environment
variable GRASS_SKIP_MAPSET_OWNER_CHECK to any non-empty string.

Of course, you still need write permission on the underlying files and
directories.

So far so nice.

Assume that several users belong to the same group. If now the group
write flag is enabled for mapsets, users can delete them even if they
are not their own. This is fine since someone (admin) must have
allowed for this.

Now back to GRASS: A user runs a session in his/her mapset with
group-write enabled. This is against the GRASS internal policy where
others cannot write into your own mapsets with GRASS commands.

Wish for improvement:
When starting a session in a mapset with group/other-write enabled,
issue a warning to inform the user about this in the startup script.
This would follow the "least-surprise" paradigm.
Feasible?

Yes. Just don't do anything too invasive.

Bear in mind that paying too much attention to filesystem permissions
has a similar problem to the ownership check, namely that most Unix
systems are capable of accessing non-Unix filesystems (e.g. FAT, NTFS,
CIFS). This is one reason why I added the ability to suppress the
check.

--
Glynn Clements <glynn@gclements.plus.com>

Hi,

when putting the PERMANENT permissions the GRASS way:
GRASS 7.0.svn (latlong):~ > ls -la /grassdata/latlong | grep PERMA
drwxr-xr-x 14 neteler gis 4096 Mar 12 11:43 PERMANENT

in order to avoid that other group "gis" member be able to delete
PERMANENT through command line, a problem in g.extension appears:

GRASS 7.0.svn (latlong):~ > g.extension v.surf.mass
D1/1: grass.script.core.start_command(): g.version -rge
D1/1: grass.script.core.start_command(): g.message message=Fetching
<v.surf.mass> from GRASS-Addons SVN (be patient)...
Fetching <v.surf.mass> from GRASS-Addons SVN (be patient)...
D1/1: grass.script.core.start_command(): g.message message=Compiling...
Compiling...
ERROR: MAPSET PERMANENT - permission denied
make: *** [v.surf.mass.tmp.html] Error 1
D1/1: grass.script.core.start_command(): g.message -e
message=Compilation failed, sorry. Please check above error messages.
ERROR: Compilation failed, sorry. Please check above error messages.

GRASS 7.0.svn (latlong):~ > g.gisenv
MAPSET=matteo
GISDBASE=/grassdata
LOCATION_NAME=latlong
GUI=text
DEBUG=1

How come that it tries to write in PERMANENT? This needs to be fixed for sure.

Markus

Markus Neteler wrote:

ERROR: MAPSET PERMANENT - permission denied

This error is generated by G__gisinit() if G__mapset_permissions()
returns 0 (mapset directory exists but not owned by the current user).

How come that it tries to write in PERMANENT? This needs to be fixed for sure.

It isn't trying to write in PERMANENT. It's trying to execute the
command (to generate the HTML) with $GISBASE/demolocation/PERMANENT as
the current mapset. If the person running g.extension isn't the owner
of $GISBASE/demolocation/PERMANENT, that will fail.

--
Glynn Clements <glynn@gclements.plus.com>