Re: [devel] [PATCH hasher-priv v1 3/3] Add cgroup support

[Arseny Maslennikov <arseny@altlinux.org>]

[-- Attachment #1: Type: text/plain, Size: 6153 bytes --]

On Fri, Oct 02, 2020 at 02:42:55AM +0200, Alexey Gladkov wrote:
> On Thu, Oct 01, 2020 at 11:23:53PM +0300, Arseny Maslennikov wrote:
> > > > Could you please explain what you're trying to do with this patch?
> > > > Even if it's obvious from the source itself, we still must have an
> > > > opportunity to discuss, and a decent explanation should stay in the
> > > > project history.
> > > 
> > > I think this patch is simple enough.
> > 
> > There's a misunderstanding here. I'm not asking to explain the
> > semantics (what this patch does) — I repeat, it's rather obvious from
> > the source itself, the patch is indeed simple. I'm trying to get how the
> > patch's author would describe the pragmatic value of this patch. IOW:
> > we see this patch does XXX. What, in Alexey's view, are we trying to
> > achieve by implementing XXX?
> 
> I remember that this patch was the result of a discussion with ldv.

That discussion then likely was not public; in part, that's why I'm asking.

> I didn't want to add complex support for different versions of cgroups.

I also believe that complexity would be unnecessary today.

> The
> idea was that the admin would prepare the system for use of cgroups by the
> hasher-privd daemon.

If I understood correctly ^U
To put it another way, we're doing this because the machine admin might
want hasher-privd to put the processes it spawns in cgroups
_at_an_arbitrary_path_, at the administrator's discretion.

Ok, this is a valid explanation and a valid feature. Thank you.

Might not be fully implementable on systemd-based installations,
though[1], but we'll look into it in time and work out something that
fits everyone's varying needs and circumstances. I'm not yet sure from
systemd's documentation if that program is OK with us making arbitrary
cgroup trees _in_the_root_cgroup_. But we definitely can get a cgroup
subtree to work in; this works.

[1] https://systemd.io/CGROUP_DELEGATION/

> 
> I'm not considering the hasher-privd as an end user server. This is a
> low-level server on which you can build different solutions. I don't mean
> just hasher.

Subject: the future of hasher-privd

I'm not particularly opposed to the expansion of hasher-privd's utility
scope; there are quite a lot of potential use cases: hasher-privd as a
general-purpose cgroup manager, hasher-privd as a daemon-based
NO_NEW_PRIVS-ready policy-enforcing "su -", ...

While this sounds interesting, I believe there are currently some
obstacles. Would those solutions on top of hasher-privd be
co-installable and co-existing on a single machine? E.g. two hasher-privd
init scripts with different configuration files for different things,
spawning different processes.
Or they wouldn't? Or a single hasher-privd instance — aka node, aka main
process if you will — would do both services? I don't yet have a picture
of this in my head; this will have to be thought out.

Will the decoupled, generic hasher-privd have to expand its IPC API?

If we decouple hasher-privd from hasher, this would also mean we support
arbitrary clients, so we'll have to formally define the IPC interface,
see my concerns on it in a previous mail.

As it stands now, the hasher project currently sees hasher-privd as its
vital component, a specialized tool for a special purpose, configured at
/etc/hasher-priv/. You're proposing something different.

In short, it's gonna be a long road.


> With this in mind, I don't think that this server should do
> everything out of the box without configuration.

I believe the hasher project _would_ want some sane out-of-the-box
configuration. The generic privd you describe above might not, much like
runc/crun do not, but the hasher project definitely would. Furthermore, in
my personal (but shared by many) opinion, this hasher OOTB experience
would have to be catered to the common case of an ALT Team developer,
not to public builder services (which are already expected to take care
to tune and harden their non-trivial configuration, and we can even ship
recommendations for their use case in /usr/share/doc).

The approach you suggest here could work, if e. g. the decoupled privd
is shipped with no defaults, and the hasher project ships its own
defaults for the desired operation of privd.

> 
> Does this make sense to you?

Barring the questions raised above, it does, thank you.
I'm just being thorough =) We still need to support every use case, and
not forget anything.

> 
> > Descriptive commit messages are done (and are enforced in successful
> > communities, e. g. LKML) for a reason.
> > 
> > The above essentially is my previous comment here, reworded and clarified.
> > 
> > If for some reason you believe it's shameful or rude to the community to
> > "waste time" on textual explanations, fair enough — I'll maybe write a commit
> > message myself (with my take on why this might be useful) and then most
> > likely ACK the same patch, with authorship reattributed to you via From:
> > in the patch body and the new commit message. Or else NAK this
> > particular revision with an empty commit message and leave it up to
> > ldv@.
> > If it were up to me, I would not approve of empty commit messages in a
> > lasting, crucial project like hasher-privd. People are forgetful, and
> > commit messages exist to help.
> 
> Ok.
> 
> > > > Do we only support cgroup2 and ignore cgroup1? If yes, great, but
> > > > perhaps then we might want to have a setting to not fiddle with cgroup
> > > > trees, to support the unfortunate users that have to run Docker and
> > > > other garbage.
> > > 
> > > Yeah, I didn't plan on supporting legacy version of cgroups. Docker
> > > already can work with cgroupsv2.
> > 
> > Oh, I heard they were just recently working on cgroup2 support.
> 
> https://github.com/opencontainers/runc/blob/master/docs/cgroup-v2.md
> 
> -- 
> Rgrds, legion
> 



> _______________________________________________
> Devel mailing list
> Devel@lists.altlinux.org
> https://lists.altlinux.org/mailman/listinfo/devel


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]