run apps as non-root user (diagexec setresuid) - #805
Conversation
|
Warning about the functional test in this PR: gexpect has a bug with some timing (ThomasRooney/gexpect#5) and I work around the gexpect bug with a sleep. |
There was a problem hiding this comment.
What if the user we get is in text format?
There was a problem hiding this comment.
I don't handle that. I'm not sure what we should do in that case...
See also appc/spec#231
There was a problem hiding this comment.
The fix ThomasRooney/gexpect#6 is merged.
I will update this...
61a4ffd to
a66145e
Compare
We need the new version of gexpect for rkt#805
|
Branch updated. It contains the fix for gexpect and the |
|
Does it look like this will all be obsoleted by using a version of systemd newer than 215? |
a66145e to
11704cd
Compare
We need the new version of gexpect for rkt#805
0565e3c to
0a373bc
Compare
|
github hid some comments after I rebased this branch. @vcaputo @iaguis : I didn't want to support for all forms of user/group but only numeric uid/gid for now. I added a check with error reporting. The only non-numeric user/group I accept is the string "root" because it was working before so I don't want to introduce the regression. (sytemd has special-case code for "root") It is a stop-gap and should be revisited either when we get a better idea from appc/spec#231 or when we let systemd do the setuid instead of diagexec. This stop-gap would stay even when using systemd v219 because support for I reduced the verbosity in diagexec by using the first code snippet from your review. |
0a373bc to
76e6ea6
Compare
We need the new version of gexpect for rkt#805
|
I'm not sure why Travis failed to test this. Semaphore passed the tests successfully on this branch (commit 76e6ea6). I don't know why I don't see the link to the Semaphore results. |
76e6ea6 to
d5f3dec
Compare
We need the new version of gexpect for rkt#805
|
I also had some bogus failure in #799. I suppose that travis was simply overloaded. |
There was a problem hiding this comment.
Is it expected that either cwd or root would be "" and should be silently ignored if so?
It seems incorrect for diagexec to have a successful code path which potentially doesn't attempt to chroot or chdir. I'd much rather we error out on unexpected/unsupported inputs than simply skip the associated actions.
We need the new version of gexpect for rkt#805
d5f3dec to
2ad1473
Compare
|
Thanks for the review @vcaputo. I updated the branch. |
|
ok cool, lgtm |
run apps as non-root user (diagexec setresuid)
|
awwwwwwwwwwwwwwww yeah |
|
@alban what is the latest on ExecFail? |
|
@jonboulle I have not tried to implement ExecFail in systemd yet... |
|
@alban something that came up in OOB discussion, it's unclear how ExecFail would actually work w.r.t. the chroot - is it going to inject a diagnostic exec into the chroot itself? or expected do the chroot itself? which would make it inconsistent with the other Exec* parameters in that case... |
5 participants
Fix #539 by adding
setresuidandsetresgidindiagexec.I preferred a solution based on RootDirectory (#679) but this was not ready yet.