Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unclear fix-permissions setuid, setgid #638

Open
TiemenSch opened this issue May 22, 2018 · 3 comments
Open

unclear fix-permissions setuid, setgid #638

TiemenSch opened this issue May 22, 2018 · 3 comments
Labels
tag:Documentation Related to user, developer, and maintainer documentation type:Question A question about the use of the docker stack images

Comments

@TiemenSch
Copy link

For me, the last part of fix-permissions is a tad unclear. The how and why isn't documented.

for d in $@; do
  find "$d" \
    ! \( \
      -group $NB_GID \
      -a -perm -g+rwX  \
    \) \
    -exec chgrp $NB_GID {} \; \
    -exec chmod g+rwX {} \;
  # setuid,setgid *on directories only*
  find "$d" \
    \( \
        -type d \
        -a ! -perm -6000  \
    \) \
    -exec chmod +6000 {} \;
done

Also, the commit in which it was added is a bit unclear for me.
2df9c49#diff-9916251299bdb9a776a7153e7caad86c

As the first block is using the non-numeric approach, would it be possible to change the second one to do so, too?

Would that be something along the lines of:

for d in $@; do
  find "$d" \
    ! \( \
      -group $NB_GID \
      -a -perm -g+rwX  \
    \) \
    -exec chgrp $NB_GID {} \; \
    -exec chmod g+rwX {} \;
  # setuid,setgid *on directories only*
  find "$d" \
    \( \
        -type d \
        -a ! -perm -g+s  \
    \) \
    -exec chmod +g+s {} \;
done

?

This may just be me being too new to the permissions management in Linux.

@TiemenSch TiemenSch changed the title fix-permissions setuid, setgid unclear fix-permissions setuid, setgid May 22, 2018
@parente parente added type:Question A question about the use of the docker stack images tag:Documentation Related to user, developer, and maintainer documentation labels May 23, 2018
@minrk
Copy link
Member

minrk commented Jun 14, 2018

would it be possible to change the second one to do so, too?

Yes, I think this should be fine

I tried to describe what it's doing and why in the comments at the top of the script.

The goal of fix-permissions is to ensure that a given directory is writable by the user, even if the container is run as a uid that's not the default $NB_USER. This is accomplished by making sure everything is readable and writable by the $NB_GID group (aka users).

The first pass sets these permissions explicitly on all files and directories. Up to here, I think the documentation in the script covers it pretty well (feel free to propose clarifications if you think anything is missing / can be better put!). One thing that's missing is why we set setuid/setgid on directories. Essentially, the goal is to make it slightly less likely for files to have the wrong ownership. setgid on a directory sets the default group of new files created in that directory

For example, running this as root:

mkdir testdir
chown jovyan:users testdir
touch testdir/before
chmod g+s testdir
touch testdir/after
ls -la testdir
drwxr-sr-x 2 jovyan users 4096 Jun 14 11:09 .
drwxrwxrwt 1 root   root  4096 Jun 14 11:09 ..
-rw-r--r-- 1 root   users    0 Jun 14 11:09 after
-rw-r--r-- 1 root   root     0 Jun 14 11:09 before

You can see that the file before is owned by the user and group I'm running (root:root), but the file after is owned by the users group.

Now, this doesn't accomplish a whole lot and what we really want is to use setfacl and/or umask to ensure that all files have the right ownership and permissions without having to call fix-permissions after the initial creating of a directory. But Docker filesystems (AUFS) don't support ACLs yet.

@alshabib
Copy link

Could you explain why should any directory other than the users home directory be writable by that user? Is this something fundamental to the operation of a jupyter notebook?

It seems to me that running fix-permissions $CONDA_DIR is both risky and causes container image bloat. Could you explain why this is necessary?

@ghost
Copy link

ghost commented Feb 20, 2020

I second @alshabib's comment.

In our case having $CONDA_DIR writable is annoying because it allows users to install packages globally. Then when they restart their container they don't understand why their packages are gone! In fact in our own image the first thing we do is to undo what fix-permissions has done to $CONDA_DIR. This forces users to install with pip install --user or to use virtual environments so that dependencies get installed in their home directory which is a persisted bind mount.

What's the rational behind making $CONDA_DIR writable by users? Maybe it's just us who have weird use case?

@mathbunnyru mathbunnyru self-assigned this Jun 6, 2023
@mathbunnyru mathbunnyru removed their assignment Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tag:Documentation Related to user, developer, and maintainer documentation type:Question A question about the use of the docker stack images
Projects
None yet
Development

No branches or pull requests

5 participants