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

Remove writable group file due to issues with being able to give su access when shouldn't be allowed. #653

Conversation

GrahamDumpleton
Copy link
Contributor

This change is to revert making the /etc/group file writable as part solution to avoid issues as discussed in #560.

Although it means in environment where container run as group ID not in /etc/groups you will get a warning:

groups: cannot find name for group ID 100000

in certain situations when creating an interactive shell in the container, there is no known Python packages or applications which will fail if running with a group ID not in /etc/group.

With the /etc/group file once again not writable, as part of #560 can then work out whether restricting su to members of wheel group can be done and lock down su for normal case.

@GrahamDumpleton
Copy link
Contributor Author

Actually, thinking about it some more, not sure removing this makes any difference.

For someone to be able to add an entry to the /etc/group file they would already need to be root or running with group root. If they are already running as group root, since there is no wheel group, it will fallback to allowing you to run su anyway, even if pam_wheel is enabled. So remove write access to /etc/group gains you nothing.

The only thing one can do is document that if running with a uid not in the /etc/passwd file, and thus inherit group root, you should always drop capabilities from running su.

@GrahamDumpleton
Copy link
Contributor Author

Withdrawing this as indeed makes no difference and how is currently done can't be improved. Can only document that should drop capabilities as necessary.

@GrahamDumpleton
Copy link
Contributor Author

May yet reopen this. Although there is no wheel group on Debian/Ubuntu, you can add one and su will then honour it and will not fallback to allowing anyone in group root to run su. So, if use:

RUN groupadd wheel && \
    echo "auth required pam_wheel.so use_uid" >> /etc/pam.d/su

even if when run with uid not in /etc/passwd and so run as gid of root, su will fail.

$ docker run -it --rm -u 1000000 -p 8888:8888 base-notebook bash
I have no name!@b41018af87fc:~$ id
uid=1000000 gid=0(root) groups=0(root)
I have no name!@b41018af87fc:~$ start.sh bash
/usr/local/bin/start.sh: ignoring /usr/local/bin/start-notebook.d/*

Adding passwd file entry for 1000000
Container must be run with group "users" to update files
Executing the command: bash
jovyan@b41018af87fc:~$ cp /etc/passwd /tmp/passwd
jovyan@b41018af87fc:~$ rootpass=`openssl passwd -1 root`
jovyan@b41018af87fc:~$ cat /tmp/passwd | sed "s%root:x%root:${rootpass}%" > /etc/passwd
jovyan@b41018af87fc:~$ su root
Password:
su: Permission denied
jovyan@b41018af87fc:~$ id
uid=1000000(jovyan) gid=0(root) groups=0(root)
jovyan@b41018af87fc:~$ exit
exit
I have no name!@b41018af87fc:~$ exit
exit

@GrahamDumpleton
Copy link
Contributor Author

Note that this PR was superseded by #654 which contains the addition of wheel group and enabling of pam_wheel as explained.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant