[PATCH Shepherd] Factor out a public CALL-IN-FORK.

This enables service implementations to easily inject code that is run before

their service is started. One such example is calling setrlimit from a start

action to set NOFILE (the open files limit), before the service is exec'ed and

thus inherits this value from the parent process, i.e. from Shepherd.

(fork+exec-command): Use the above.

---

modules/shepherd/service.scm | 51 ++++++++++++++++++++----------------

1 file changed, 29 insertions(+), 22 deletions(-)

This enables service implementations to easily inject code that is run before

their service is started. One such example is calling setrlimit from a start

action to set NOFILE (the open files limit), before the service is exec'ed and

inherits this value from the parent process, i.e. from Shepherd.

(fork+exec-command): Use the above.

---

v2: fixes the commit message.

modules/shepherd/service.scm | 51 ++++++++++++++++++++----------------

1 file changed, 29 insertions(+), 22 deletions(-)

Am Dienstag, dem 01.03.2022 um 08:29 +0100 schrieb Attila Lendvai:

In general, I think such capabilities should be added to exec-command,

rather than resorting to a lambda. It takes a little while to realize

that call-in-fork, fork-and-call or whatever you want to name it is in

fact not pure evil; mainly because shepherd could in its stead already

invoke any lambda you throw at it. That being said, one should always

be aware that this child process runs with the full permissions of

shepherd, which you normally don't want to do for a service.

This docstring, as well as the procedure name only describe what is

done with thunk in the crudest terms. What's more, I don't think it

makes too much sense to restrict ourselves to thunks if we already run

arbitrary code anyway.

In my opinion, it ought to be

WDYT?

 

This is just copypasta from a previous mistake, but

s/executed/executes/.

Cheers

Attila Lendvai schreef op di 01-03-2022 om 08:06 [+0100]:

'fork+exec-command' already accepts a 'environment-variables' and

'file-creation-mask', how about adding an 'open-file-limit' argument?

To me, that seems more declarative and less fragile than having

to call 'call-in-fork' manually in a 'start' procedure (*).

Support for other rlimits can be added on an as-needed basis.

Alternatively, the argument could be generalised to a more general

'rlimit' argument:

#:rlimits

`((,RLIMIT_AS ,SOFT ,HARD)

(,RLIMIT_NPROC ,SOFT ,HARD)

(,RLIMIT_NOFILE ,SOFT ,HARD))

WDYT?

Greetings,

Maxime.

(*) E.g., one of the ideas for making shepherd faster, was using some

kind of multi-threading. Forking when multi-threading is ill-defined

(see POSIX) though, so some kind of zygote process + IPC might be

necessary

(http://neugierig.org/software/chromium/notes/2011/08/zygote.htmlhas a

nice explanation on zygote processes, the bits about software updates

can be ignored here).

does the above mean that you're concerned about the security implications? if

so, then i don't understand, because Guile already allows calling/accessing

private functions/symbols, and thus this change doesn't really increase the

(already enormous) attack surface in the guile codebase.

it does increase the shoot-oneself-in-the-foot-surface a little bit, though.

it's worth pointing out, though, that trusting a channel, and adding a shepherd

service defined by it to the machine's config, is essentially giving root access

to the channel author. and this is already the case, prior to my change.

BTW, can i not already simply pass 0, or "root" as #:user to EXEC-COMMAND?

makes sense, i'll update the patch... but given the feedback from the two of

you, should i?

i think i'll abandon this, and implement Maxime's #:rlimits suggestion.

i'm not sure how much better that will be, but at least it won't make future

threading harder, and allows me to make progress with my project.

if anyone prefers the FORK+APPLY version, then do speak up!

--

• attila lendvai

• PGP: 963F 5D5F 45C7 DFCD 0A39

--

“An atheist doesn't have to be someone who thinks he has a proof that there can't be a god. He only has to be someone who believes that the evidence on the God question is at a similar level to the evidence on the werewolf question.”

— John McCarthy (1927–2011), father of Lisp

Am Dienstag, dem 01.03.2022 um 13:04 +0000 schrieb Attila Lendvai:

This attack surface is less enormous if you consider the average case

of a shepherd service in which the arguments to fork+exec-command are

already evaluated by the time the procedure is call and thus both

"sane" within and without the fork. Most of the time people are not

too conscious about the fact that shepherd can already run arbitrary

Guile code as part of actions and you typically only use that to its

fullest extent when you're trying to do something real clever.

Only if you're already root, i.e. this won't work for user shepherds,

which can't become root (easily). On the other hand, I did get my user

shepherd to launch pkexec commands, so that's that.

FWIW Maxime's complaint would also hold w.r.t. fork+exec-command, which

would then be implemented in terms of fork+apply, so assuming that

fork+exec-command still exists after the switch to multiple threads,

we'd have to patch at least one location either way. fork+apply could

make it so that less hacks are required overall to make all forking

behaviour inside shepherd services as intended, but that's so far only

a theoretical claim with no evidence to back it up.

I think the real question is what you are trying to achieve here. If

you only want to add rlimits, that's an exec-command thing. If you

instead wanted to spawn a Guile function within a sandbox (rather than

a completely new command), that would require something along the lines

of fork+apply at least under the hood. With the things you've

described, I don't think it makes sense (yet) to export fork+apply, but

it might still make sense to refactor fork+exec-command under the hood.

Cheers

Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:

In general this would be improved if we move Guix in general, and the

Shepherd services in particular, to an object capability based security

model.

It's on my TODO to lay out a sketch for how this could happen, assuming

there's support for it in the community (which I don't expect to go one

way or another until a plan is laid out to talk about).

Hi,

Maxime Devos <maximedevos@telenet.be> skribis:

Seconded.

This interface brings more flexibility, I’m all for it.

I think what we need is concurrency, not POSIX threads. IOW, we can

achieve the concurrency we need without resorting to POSIX threads, for

example using Fibers on a single POSIX thread.

Thanks,

Ludo’.

Ludovic Courtès schreef op wo 02-03-2022 om 17:05 [+0100]:

guile-fibers uses threads internally, e.g. in (fibers interrupts).

Interrupts can theoretically be avoided, but that has a downside

that if a start procedure goes into infinite loop (while forgetting to

sleep), the whole shepherd would hang.

I'm not saying that we need POSIX threads per-se -- I find

'choice-operation', 'perform-operation', the channel operations and

Fibers conditions much more convenient than the (lack of) POSIX

equivalents, but I'd prefer avoiding the assumption of single-threading

where feasible, to make it ourselves not harder than necessary in the

future, in case it turns out we need POSIX threading somewhere (even if

only as an implementation detail).

Greetings,

Maxime.

FTR, i've filed this as a patch (with a test!).

--

• attila lendvai

• PGP: 963F 5D5F 45C7 DFCD 0A39

--

“It is a miracle that curiosity survives formal education. It is a very grave mistake to think that the enjoyment of seeing and searching can be promoted by means of coercion and a sense of duty.”

— Albert Einstein (1879–1955), 'Autobiographical Notes' (1949), slightly paraphrased

tags 54205 + wontfix

quit

Attila Lendvai <attila@lendvai.name> skribis:

Awesome, closing this one!

Ludo’.