This repository was archived by the owner on Jan 8, 2019. It is now read-only.
Ensure unprivileged users have unique tmp and home dirs#1278
Open
cbaenziger wants to merge 2 commits into
Open
Ensure unprivileged users have unique tmp and home dirs#1278cbaenziger wants to merge 2 commits into
cbaenziger wants to merge 2 commits into
Conversation
cbaenziger
force-pushed
the
tmp_dirs
branch
3 times, most recently
from
84bf2e1 to
ab088eb
Compare
cbaenziger
force-pushed
the
tmp_dirs
branch
2 times, most recently
from
c86170c to
13b07cf
Compare
Member
Author
|
Oh, also to explain; we override |
cbaenziger
force-pushed
the
tmp_dirs
branch
2 times, most recently
from
8045dc3 to
7db2d5b
Compare
cbaenziger
commented
Nov 1, 2018
cbaenziger
commented
Nov 1, 2018
| # see if user still has any prescence on this machine | ||
| pgrep -u "$PAM_USER" && exit 0 | ||
| # if no prescense of user, remove all trace of their activity | ||
| rm -rf "<%= @polyinstantion_dir %>/home_$PAM_USER" \ |
Member
Author
There was a problem hiding this comment.
Chris asks please ensure that hardlink traversal isn't a risk here.
cbaenziger
commented
Nov 1, 2018
| source 'pam_namespace.conf.erb' | ||
| mode 500 | ||
| variables(lazy {{ real_home_dir_users: | ||
| node['bcpc']['pam_namespace']['real_home_dir_users'].join(','), |
Member
Author
There was a problem hiding this comment.
Ron asks, please .uniq on node['bcpc']['pam_namespace']['real_home_dir_users'].
cbaenziger
commented
Nov 1, 2018
cbaenziger
commented
Nov 1, 2018
vt0r
reviewed
Nov 1, 2018
vt0r
reviewed
Nov 1, 2018
vt0r
reviewed
Nov 1, 2018
| source 'pam_namespace.conf.erb' | ||
| mode 500 | ||
| variables(lazy {{ real_home_dir_users: | ||
| node['bcpc']['pam_namespace']['real_home_dir_users'].join(','), |
Contributor
There was a problem hiding this comment.
Not sure if this would work, since I haven't tested it: Working. See below comment.
Suggested change
| node['bcpc']['pam_namespace']['real_home_dir_users'].join(','), | |
| node['bcpc']['pam_namespace']['real_home_dir_users'].uniq.sort.join(','), |
Contributor
There was a problem hiding this comment.
Seems ok
[1] pry(main)> blah = %w(hdfs hdfs yarn hbase mapred hdfs hbase yarn mapred mapred hdfs hbase hadoop)
=> ["hdfs", "hdfs", "yarn", "hbase", "mapred", "hdfs", "hbase", "yarn", "mapred", "mapred", "hdfs", "hbase", "hadoop"]
[2] pry(main)> blah.uniq.sort.join(',')
=> "hadoop,hbase,hdfs,mapred,yarn"
macmaster
approved these changes
Nov 1, 2018
Member
Author
|
All requested changes made |
Also DRY-up sysctl/pam code repeated in Kafka and Hadoop; should be in BCPC
vt0r
reviewed
Nov 2, 2018
| directory '/inst-dirs' do | ||
| user 'root' | ||
| group 'root' | ||
| mode 0o000 |
Contributor
There was a problem hiding this comment.
Suggested change
| mode 0o000 | |
| mode 0o0000 |
vt0r
reviewed
Nov 2, 2018
|
|
||
| if [ -z "$PAM_TYPE" -o "$PAM_TYPE" == "open_session" ]; then | ||
| # need to setup the polyinstantation directory for login | ||
| mkdir -pm 000 "/dev/shm/<%= @shm_polyinstantion_dir %>" \ |
Contributor
There was a problem hiding this comment.
Looks like this got merged out:
Suggested change
| mkdir -pm 000 "/dev/shm/<%= @shm_polyinstantion_dir %>" \ | |
| mkdir -pm 0000 "/dev/shm/<%= @shm_polyinstantion_dir %>" \ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces pam_namespaces to ensure that each user gets its own kernel usernamespace for its potential state directories that are managed outside YARN. One further addition is automatic creation and clean-up of directories as needed using pam_exec.
Providing per-user namespaces means we can prevent accidental data sharing and we will prevent many imporoper uses of temporary directories by pro-actively cleaning them up. We have seen many users impact themselves with both anti-patterns and similarly when a user disconnects we will ensure they stop taking up space on our root partition for what should be their ephemeral home directory. We will need to notify users of this change!
The directories affected are listed in `/etc/secuirty/namespaces.conf. Users listed in that file are excluded from that line.
The directories created in
inst_dir.sh.erbare the necessary directories for pam_namespace to create directories and when the user is no longer running any processes and the user terminations its session the script is called to remove the directories. (If one usesnohupor such you can hold your directory around I think but I am not too concerned about that right now; we can write clean-up logic if need be later; similarly no quotas are introduced here but use of ZFS or possibly LVM could allow us to easily do that.)The structure under the polyinstantion directory looks like follows; and inside each directory is what the user see: