| Message ID | 129e8d298556f6a159fcb704ed3df4bf0709ddd3.1689465600.git.me@tobias.gr |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#61462,v2,01/10] system: Disallow file-like setuid-programs. | expand |
Thanks for the refreshed v2 patches! I gave them a quick spin... As noted on IRC, apparently it lacks actual calls to setcap, so that part still needs another patch at least! Otherwise, it did seem to more-or-less work... There are compatibility symlinks from /run/setuid-programs to /run/privledged/bin and it sets setuid on requested files. I was a little curious about why /run/privlidged/bin as opposed to without /bin ... keeping the door open for other privlidged things? What about things that come from /gnu/store/*/sbin ? are those handled any differently? My only concern is... wow is it hard, even for a native speaker, to spell privileged! live well, vagrant
On 2023-07-21, Vagrant Cascadian wrote: > Thanks for the refreshed v2 patches! I gave them a quick spin... > > As noted on IRC, apparently it lacks actual calls to setcap, so that > part still needs another patch at least! > > Otherwise, it did seem to more-or-less work... > > There are compatibility symlinks from /run/setuid-programs to > /run/privledged/bin and it sets setuid on requested files. Oh, I noticed on reconfiguring back to a system without the patches to support /run/privileged configurations ... the /run/privileged directory is still present, with all those files sitting there in their previous state. This is why I think at least by default, many other distros implement /run as a tmpfs or similar, so that it at least gets thrown out at reboot. Though this is obviously a deeper problem than just this patch series... I will file a separate bug about that. live well, vagrant
Hey! Vagrant Cascadian <vagrant@debian.org> skribis: > Oh, I noticed on reconfiguring back to a system without the patches to > support /run/privileged configurations ... the /run/privileged directory > is still present, with all those files sitting there in their previous > state. > > This is why I think at least by default, many other distros implement > /run as a tmpfs or similar, so that it at least gets thrown out at > reboot. Though this is obviously a deeper problem than just this patch > series... I will file a separate bug about that. We could try to make that change: /run as tmpfs, or wiped by ‘cleanup-service-type’. Ludo’.
On 2023-08-08, Ludovic Courtès wrote: > Vagrant Cascadian <vagrant@debian.org> skribis: >> Oh, I noticed on reconfiguring back to a system without the patches to >> support /run/privileged configurations ... the /run/privileged directory >> is still present, with all those files sitting there in their previous >> state. >> >> This is why I think at least by default, many other distros implement >> /run as a tmpfs or similar, so that it at least gets thrown out at >> reboot. Though this is obviously a deeper problem than just this patch >> series... I will file a separate bug about that. > > We could try to make that change: /run as tmpfs, or wiped by > ‘cleanup-service-type’. Or both, really! Filed: https://issues.guix.gnu.org/64775 live well, vagrant
Vagrant Cascadian <vagrant@debian.org> writes: > On 2023-08-08, Ludovic Courtès wrote: >> We could try to make that change: /run as tmpfs, or wiped by >> ‘cleanup-service-type’. > > Or both, really! > > Filed: > > https://issues.guix.gnu.org/64775 I tried this a while ago, and the trivial case of mounting /run as tmpfs in the operating-system definition causes errors during activation. It turns out that the /run/current-system symlink is activated before all non-root mounts, so mounting /run afterwards causes everything to break. I don't have a solution, and haven't even looked at it past this, but maybe this report will help. -bjc
On 2023-07-21, Vagrant Cascadian wrote: > Thanks for the refreshed v2 patches! I gave them a quick spin... > > As noted on IRC, apparently it lacks actual calls to setcap, so that > part still needs another patch at least! > > Otherwise, it did seem to more-or-less work... I did eventually get some updated patches that even followed through on the promise of calling out to setcap, and from what I recall they even worked! I liked them a lot. > There are compatibility symlinks from /run/setuid-programs to > /run/privledged/bin and it sets setuid on requested files. > > I was a little curious about why /run/privlidged/bin as opposed to > without /bin ... keeping the door open for other privlidged things? What > about things that come from /gnu/store/*/sbin ? are those handled any > differently? Working patches aside, that is my only outstanding question, and I would hate to see that be a blocker. :) In short, "ping" :) live well, vagrant
On 2023-11-15, Vagrant Cascadian wrote: > On 2023-07-21, Vagrant Cascadian wrote: >> Thanks for the refreshed v2 patches! I gave them a quick spin... >> >> As noted on IRC, apparently it lacks actual calls to setcap, so that >> part still needs another patch at least! >> >> Otherwise, it did seem to more-or-less work... > > I did eventually get some updated patches that even followed through on > the promise of calling out to setcap, and from what I recall they even > worked! I liked them a lot. > > >> There are compatibility symlinks from /run/setuid-programs to >> /run/privledged/bin and it sets setuid on requested files. >> >> I was a little curious about why /run/privlidged/bin as opposed to >> without /bin ... keeping the door open for other privlidged things? What >> about things that come from /gnu/store/*/sbin ? are those handled any >> differently? > > Working patches aside, that is my only outstanding question, and I would > hate to see that be a blocker. :) I just noticed I pushed a branch with the working patches to a public branch last month: https://salsa.debian.org/debian/guix/-/tree/capabilities-61462-20231115?ref_type=heads They are even still cherry-pickable from current master! Yay! These patches were started over a year ago(well, probably before that, even), and had a working implementation about 6 months ago... My guess is the main blocker is nervousness about renaming setuid-programs to privilidged-programs (I know I am a bit nervous to do so!)? This would make it possible to properly fix several bugs: https://issues.guix.gnu.org/27415 https://issues.guix.gnu.org/39136 https://issues.guix.gnu.org/39136 https://issues.guix.gnu.org/55683 And have been mentioned indirectly in several others over the years: https://issues.guix.gnu.org/search?query=setcap live well, vagrant
Hello! Vagrant Cascadian <vagrant@debian.org> skribis: > I just noticed I pushed a branch with the working patches to a public > branch last month: > > https://salsa.debian.org/debian/guix/-/tree/capabilities-61462-20231115?ref_type=heads > > They are even still cherry-pickable from current master! Yay! Wo0t! > These patches were started over a year ago(well, probably before that, > even), and had a working implementation about 6 months ago... > > My guess is the main blocker is nervousness about renaming > setuid-programs to privilidged-programs (I know I am a bit nervous to do > so!)? It shouldn’t be an issue as /run/setuid-programs is populated with symlinks for backward compatibility. AIUI, we can still use good’ol setuid programs on the Hurd until a better solution is found, so we should be fine (meaning “make check-system TESTS=childhurd” should pass). We could emit a deprecation warning when someone uses the ‘setuid-programs’ field of <operating-system>. Not a blocker though. Tobias, ready to push? :-) Cheers, Ludo’.
diff --git a/gnu/system.scm b/gnu/system.scm index 23addf41e9..e32879b240 100644 --- a/gnu/system.scm +++ b/gnu/system.scm @@ -296,8 +296,7 @@ (define-record-type* <operating-system> operating-system (pam-services operating-system-pam-services ; list of PAM services (default (base-pam-services))) (setuid-programs operating-system-setuid-programs - (default %setuid-programs) ; list of <setuid-program> - (sanitize ensure-setuid-program-list)) + (default %setuid-programs)) ; list of <setuid-program> (sudoers-file operating-system-sudoers-file ; file-like (default %sudoers-specification)) @@ -1203,31 +1202,6 @@ (define (operating-system-environment-variables os) ;; when /etc/machine-id is missing. Make sure these warnings are non-fatal. ("DBUS_FATAL_WARNINGS" . "0"))) -;; Ensure LST is a list of <setuid-program> records and warn otherwise. -(define-with-syntax-properties (ensure-setuid-program-list (lst properties)) - (%ensure-setuid-program-list lst properties)) - -;; We want to be able to use defines, so define a procedure. -(define (%ensure-setuid-program-list lst properties) - (define warned? #f) - - (define (warn-once) - (unless warned? - (warning (source-properties->location properties) - (G_ "representing setuid programs with file-like objects is \ -deprecated; use 'setuid-program' instead~%")) - (set! warned? #t))) - - (map (match-lambda - ((? setuid-program? program) - program) - (program - ;; PROGRAM is a file-like or a gexp like #~(string-append #$foo - ;; "/bin/bar"). - (warn-once) - (setuid-program (program program)))) - lst)) - (define %setuid-programs ;; Default set of setuid-root programs. (let ((shadow (@ (gnu packages admin) shadow)))