uid/gidmapping #145

ezrizhu · 2024-02-17T02:37:48Z

try
tests
better sync mechanism future: sync #140
c gidmapper rewrite gidwrapper #143
docs (mention that docker needs spawned privileged and is root in the ct) (also mention below)
test apt Running apt #6
~~test/toplevel-perms.sh permissions test change (blocked by keep toplevel dir modes and symlinks #138)~~ [1]

[1] due to regular files not being tracked in try (tangent #150), we're also not setting the permissions correctly. e.g., if /test is owned by another user, this will not be reflected in try. This is likely outside the scope of this PR. However we can probably track this in another issue, #90 could also fix it.

Four tiers of uid/gid mapping

no gid mapping: Files are only accessible if the ownership is set to both current user's uid and gid.
unprivileged gid mapping: groups will look exactly the same inside and outside the container, the try will have access to things owned by things that one of user's group has access to aswell
privileged gid mapping: both a file's user and group ownership will also look and act exactly the same inside and outside try. (needs sudo, and try will have access to everything)
privileged uid mapping with -u set: id result will look the same.

# no mapping
ezri@pashsn3:~/try$ ./try ls -l try
-rwxrwxr-x 1 root root 21884 Feb 17 10:39 try

# unprivileged mapping
ezri@pashsn3:~/try$ ./try ls -l try
-rwxrwxr-x 1 root ezri 21884 Feb 17 10:39 try

# privileged mapping
ezri@pashsn3:~/try$ sudo ./try ls -l try
-rwxrwxr-x 1 ezri ezri 21884 Feb 17 10:39 try

# shell
ezri@pashsn3:~/try$ ls -l try
-rwxrwxr-x 1 ezri ezri 20263 Feb 17 10:42 try

# no mapping
ezri@pashsn3:~/try$ ./try id
uid=0(root) gid=0(root) groups=0(root),65534(nogroup)

# unprivileged mapping
ezri@pashsn3:~/try$ ./try id
uid=0(root) gid=1001(ezri) groups=1001(ezri),27(sudo)

# privileged mapping without user set
ezri@pashsn3:~/try$ sudo ./try id
uid=0(root) gid=0(root) groups=0(root)

# privileged mapping with user set
ezri@pashsn3:~/try$ sudo ./try -u ezri id
uid=1001(ezri) gid=1001(ezri) groups=1001(ezri),27(sudo)

# shell
ezri@pashsn3:~/try$ id
uid=1001(ezri) gid=1001(ezri) groups=1001(ezri),27(sudo)

closes #140
closes #143
closes #6

…clude /usr/loca/bin

mgree

I understand... most of this. A few more questions. I think moving the mapper into this repo is important from a supply-chain security perspective.

setup.sh

mgree · 2024-02-22T18:29:15Z

try

@@ -159,6 +166,12 @@ autodetect_union_helper() {
    fi
 }

+# notify the mapper that we're up
+echo "a" > "$SOCKET"


Let's use different sentinels when sending to and receiving from the mapper---we should check that the sentinels are what we expect, too.

mgree · 2024-02-22T18:30:48Z

try

+              then
+                  error "need root for -u" 2
+              fi
+              EUSER="$OPTARG"


We need to unconditionally clear EUSER at startup, so that it can't be invoked as EUSER=foo try .... I suggest using a more specific name, like TRYUSER.

mgree · 2024-02-22T18:33:08Z

try

-. "$script_to_execute"
+if [ "$EUSER" ]
+then
+    su - $EUSER -c "cd "$START_DIR" && sh $script_to_execute"


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 do cd "$START_DIR" && exec su - ....

Let's add a test to ensure that:

(a) normal (non-sudo) use of try doesn't let you read files you should not have
(b) sudo within the normal try container doesn't leak privileges
(c) we need to carefully articulate how we lose privileges after having run --map-root-user

mgree · 2024-02-22T18:33:34Z

try

+# Change uid/gid mapping
+################################################################################
+
+mapper() {


Maybe a more informative name, like uid_gid_mapper()?

mgree · 2024-02-22T18:33:59Z

try

+mapper() {
+    cat "$SOCKET" > /dev/null
+    # Get the pid of the unshare process with current pid as parent
+    pid=$(pgrep -P $$ -f unshare)


Is this a new external dependency?

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.

Let's write the process id on the socket, save ourselves the trouble.

mgree

notes from meeting

we need to understand the security implications before moving forward

mgree · 2024-02-27T18:33:42Z

try

+mapper() {
+    cat "$SOCKET" > /dev/null
+    # Get the pid of the unshare process with current pid as parent
+    pid=$(pgrep -P $$ -f unshare)


Let's write the process id on the socket, save ourselves the trouble.

mgree · 2024-02-27T19:10:08Z

try

+        gidmapper "$pid" 0 0 65535 0 0 65535
+    else
+        # If not running as root, we can only mount the caller user
+        gidmapper "$pid" 0 "$(id -u)" 1 0 0 65535


investigate gidmapper "$pid" 0 "$(id -u)" 1 0 "$(id -g)" 1? why map all groups?

mgree · 2024-02-27T19:15:07Z

try

-. "$script_to_execute"
+if [ "$EUSER" ]
+then
+    su - $EUSER -c "cd "$START_DIR" && sh $script_to_execute"


Let's add a test to ensure that:

(a) normal (non-sudo) use of try doesn't let you read files you should not have
(b) sudo within the normal try container doesn't leak privileges
(c) we need to carefully articulate how we lose privileges after having run --map-root-user

try

mgree · 2024-02-27T19:54:53Z

One other test to run: can a valid sudoer use sudo inside the container? (Feels good for running scripts that need to install things, but mostly are unprivileged.)

ezrizhu added 4 commits February 16, 2024 18:21

docker support

70ae8dd

add setup.sh from og future

b58a42d

gidmapper support, ability to select executing user

bb7be24

set tests to use setup.sh

1a87872

ezrizhu self-assigned this Feb 17, 2024

ezrizhu added 2 commits February 17, 2024 03:27

install libcap2-bin on debian vagrants

f3704d4

fix(test/nomerger): add path for gidmapper

0ed1827

ezrizhu force-pushed the future2 branch from 92d64d9 to 0ed1827 Compare February 17, 2024 09:46

Merge branch 'docker' into future2

4325803

ezrizhu changed the title ~~[WIP] gidmapping support~~ [WIP] gidmapping + docker support Feb 17, 2024

ezrizhu added 4 commits February 17, 2024 10:15

test: gidmapping

eb9c436

test: fileowner test, requires root

8bcf1e1

test/fileowner: clarify redirection for shellcheck

8bb213f

test: uidmapping, needs sudo

ba6a906

ezrizhu changed the title ~~[WIP] gidmapping + docker support~~ [WIP] uid/gidmapping + docker support Feb 17, 2024

ezrizhu mentioned this pull request Feb 17, 2024

try release #141

Open

3 tasks

ezrizhu marked this pull request as ready for review February 17, 2024 10:49

use /usr/bin for gidmapper, (default secure_path for sudo does not in…

d6c6269

…clude /usr/loca/bin

ezrizhu force-pushed the future2 branch from 2111d33 to d06c285 Compare February 20, 2024 00:59

fix id output to not print security context

0c87461

ezrizhu force-pushed the future2 branch from d06c285 to 0c87461 Compare February 20, 2024 01:01

ezrizhu changed the title ~~[WIP] uid/gidmapping + docker support~~ uid/gidmapping + docker support Feb 20, 2024

mgree requested changes Feb 22, 2024

View reviewed changes

mgree requested changes Feb 27, 2024

View reviewed changes

mgree reviewed Feb 27, 2024

View reviewed changes

try Outdated Show resolved Hide resolved

mgree reviewed Feb 27, 2024

View reviewed changes

try Outdated Show resolved Hide resolved

ezrizhu added 2 commits February 29, 2024 04:07

add mapper

6defb9b

remove docker support

3495a17

ezrizhu changed the title ~~uid/gidmapping + docker support~~ uid/gidmapping Feb 29, 2024

ezrizhu added 7 commits March 1, 2024 15:30

merge in main

0493613

lint fix

b98f7af

Merge branch 'main' into future2

088001e

Merge branch 'main' into future2

411ca92

edit toplevel-perms test to also check for user/group ownership

9165206

shellcheck ignore

bf0dfae

test(toplevel-perms): excl. regular files #150

3ad69ce

ezrizhu force-pushed the future2 branch from c957ce4 to 3ad69ce Compare March 9, 2024 04:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

uid/gidmapping #145

uid/gidmapping #145

ezrizhu commented Feb 17, 2024 •

edited

mgree left a comment

mgree Feb 22, 2024

mgree Feb 22, 2024

mgree Feb 22, 2024

mgree Feb 27, 2024

mgree Feb 22, 2024

mgree Feb 22, 2024

ezrizhu Feb 22, 2024

mgree Feb 27, 2024

mgree left a comment

mgree Feb 27, 2024

mgree Feb 27, 2024

mgree Feb 27, 2024

mgree commented Feb 27, 2024

uid/gidmapping #145

Are you sure you want to change the base?

uid/gidmapping #145

Conversation

ezrizhu commented Feb 17, 2024 • edited

mgree left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgree left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgree commented Feb 27, 2024

ezrizhu commented Feb 17, 2024 •

edited