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.
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
uid/gidmapping #145
base: main
Are you sure you want to change the base?
uid/gidmapping #145
Changes from all commits
70ae8dd
b58a42d
bb7be24
1a87872
f3704d4
0ed1827
4325803
eb9c436
8bcf1e1
8bb213f
ba6a906
d6c6269
0c87461
6defb9b
3495a17
0493613
b98f7af
088001e
411ca92
9165206
bf0dfae
3ad69ce
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use different sentinels when sending to and receiving from the mapper---we should check that the sentinels are what we expect, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we confident that all of the other variables we care about are marked as export?
This should probably be
exec su ...
, since we have no other work afterwards. In fact, we could probably docd "$START_DIR" && exec su - ...
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a test to ensure that:
(a) normal (non-
sudo
) use oftry
doesn't let you read files you should not have(b)
sudo
within the normaltry
container doesn't leak privileges(c) we need to carefully articulate how we lose privileges after having run
--map-root-user
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a more informative name, like
uid_gid_mapper()
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a new external dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, we're adding libcap2-bin and procps/procps-ng. readme will be updated appropriately.
libcap2-bin for setup to do addcap to the gidmapper.
procps for pgrep to look for unshare thread id, was hoping for a non-external-dependency needed option, perhaps we can have this be built into the c gidmapper impl.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's write the process id on the socket, save ourselves the trouble.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
investigate
gidmapper "$pid" 0 "$(id -u)" 1 0 "$(id -g)" 1
? why map all groups?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to unconditionally clear
EUSER
at startup, so that it can't be invoked asEUSER=foo try ...
. I suggest using a more specific name, likeTRYUSER
.