Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC 0068] Minimal daemon #68

Closed
wants to merge 11 commits into from

Conversation

Ericson2314
Copy link
Member

rfcs/0000-minimal-daemon.md Outdated Show resolved Hide resolved
rfcs/0000-minimal-daemon.md Outdated Show resolved Hide resolved
rfcs/0000-minimal-daemon.md Outdated Show resolved Hide resolved
rfcs/0000-minimal-daemon.md Show resolved Hide resolved
- `nix-daemon` will be a separate executable that only links the nix libraries it needs.
\[At this time, those libraries are `libnixutil`, `libnixutil`, and `libnixrust`, but this is subject to change.\]

- `nix-daemon` should never need to understand the expression language and depend `libnixexpr`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 since the separate nix-daemon is also what allows things like guix

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean it allows guix?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the sense the daemon can work in other frameworks not directly related to Nix language. Guix can use the same daemon as us and they don't need to read a single line of Nix code, only Scheme (and maybe C++).

@edolstra
Copy link
Member

What is the advantage over the current situation? Logically nix-daemon doesn't know anything about the Nix language, flakes, etc. We just link everything into one executable to reduce distribution size.

Co-authored-by: Cole Helbling <cole.e.helbling@outlook.com>
@Ericson2314
Copy link
Member Author

@edolstra Besides the reasons I wrote about taking baby steps towards more modularity, in the short term, I don't think it's wise to have tons of dead code exposed to a program running as root written in a language that doesn't try very hard to prevent memory errors. (I'll add that to the proposal.)

rfcs/0000-minimal-daemon.md Outdated Show resolved Hide resolved
Co-authored-by: Cole Helbling <cole.e.helbling@outlook.com>
rfcs/0000-minimal-daemon.md Outdated Show resolved Hide resolved
@edolstra edolstra added status: open for nominations Open for shepherding team nominations and removed status: new labels Jun 11, 2020
@edolstra
Copy link
Member

This RFC is open for shepherd nominations.

I'd like to nominate myself :-)

@kloenk
Copy link
Member

kloenk commented Jun 11, 2020

I just started working on nix. But I would give it a try as a shepherd meber

@Ericson2314
Copy link
Member Author

Ericson2314 commented Jun 11, 2020

Fine with me :), I don't think this RFC doesn't require much prior knowledge/experience to reach a decision.

@kloenk
Copy link
Member

kloenk commented Jun 20, 2020

Maybe this also could be a time to redesign the protocol? Bools stored as 64 Bits sound kind of huge, and also there are many unused values and packages.

@Ericson2314
Copy link
Member Author

Yes that would be good. I wrote this RFC as a toe-hold to then propose such things, but if during the shepherding meetings there is consensus that we should, up front, commit to making the daemon more conceptually standalone to motivate this, I don't mind adding those extra steps.

@wmertens
Copy link

Just a note: in #17 (comment) I'm imagining a very minimal store daemon that communicates via staging directories. That's probably a few steps beyond what you're trying to achieve here though.

@Mic92
Copy link
Member

Mic92 commented Jun 25, 2020

I would say we need at least one more shepherd here.

@thufschmitt
Copy link
Member

I'd like to nominate myself too

rfcs/0000-minimal-daemon.md Outdated Show resolved Hide resolved

With flakes and other development, we are moving towards a more "batteries included" Nix command line.
We don't want any of those features in the daemon, however, because the daemon is a special trusted process that we should strive to keep as simple as possible.
\[This is comparable to a compilation pipeline, with a concise intermediate representation that nicer user-facing features "desugar" into.\]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would either remove the square brackets, or remove this.

Originally, each Nix command was its own executable, but then we combined them into one executable.
I think this is fine for the main user-facing commands, but not good for the daemon.

Finally, it's probably best not to give the daemon---a long lived process running with elevated privileges---access to tons of dead code.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we specify exactly what privileges the daemon is running with, here? I think it's pertinent to the motivations of this RFC.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And what is meant here by dead code?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will explain the "dead code". The elevated privileges thing I am not sure what to right: in theory the nix daemon just needs to be make sandboxes and access the store, but in practice we run it as root last I checked.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is run as Root. Currently the source doesn't allow anything else. We could check for CAPs, but we only check for Root prior to remounting the store in rw


Finally, it's probably best not to give the daemon---a long lived process running with elevated privileges---access to tons of dead code.
C++ doesn't exactly prevent memory errors, and that dead code is just more fodder to be used in some low-level attack.
There are other solutions to this in the long term, but this is the easiest solution in the short term.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to mention the other solutions in Future Work, methinks.

[examples-and-interactions]: #examples-and-interactions

I certainly hope there are no interactions!
One of the bad things we should seek to prevent with this is the daemon unintentionally growing dependencies on more of the code base.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could be moved to the Motivations section.

[drawbacks]: #drawbacks

Installation is slightly bigger as the two binaries (`nix` and `nix-daemon`) have some redundancy.
Build rules perhaps are slightly more complex as there are both separate and independent executables.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would it take to remove the perhaps and know for sure? :)

rfcs/0000-minimal-daemon.md Outdated Show resolved Hide resolved
# Future work
[future]: #future-work

There's lots of future work we could do in the vein of modularity.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, I would try to be (a bit more) specific.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I'm trying to elude to various things people have told me they'd like to see happen over the years, without poisoning the specifics. I rather have this stuff organically come back in the RFC shepherd meetings.

@spacekookie
Copy link
Member

There's enough shepherds on this RFC, right? @edolstra do you want to organise a meeting?

@edolstra
Copy link
Member

edolstra commented Sep 24, 2020

@Ericson2314 @regnat @kloenk I've made a doodle for a meeting next week: https://doodle.com/poll/6gs9h3zvacbtev43. Could you fill in your availability?

@thufschmitt
Copy link
Member

@Ericson2314 @regnat @kloenk I've made a doodle for a meeting next week: https://doodle.com/poll/6gs9h3zvacbtev43. Could you fill in your availability?

Unfortunately I'm away for the whole next week. But feel free to do a first meeting without me if you want to

@edolstra
Copy link
Member

edolstra commented Nov 5, 2020

@Ericson2314 Would you be opposed to closing this RFC and creating a new feature request issue in the Nix repo? Having a minimal daemon is not really a potentially controversial or hard change that requires a shepherd team and all that :-) What do you think?

@kloenk
Copy link
Member

kloenk commented Nov 6, 2020

My idea of how to implement this would probably need an rfc. Maybe we could/should open a new rfc when I got time to make it working.

CC @grahamc

Co-authored-by: Jonathan Ringer <jonringer@users.noreply.github.com>
@Ericson2314
Copy link
Member Author

I am hoping to bolster the motivation here with some new developments I hope will arise soon. Marking it WIP until I do so.

@spacekookie
Copy link
Member

Closing this until there's more progress. Feel free to re-open a new PR with an updated RFC

@Ericson2314
Copy link
Member Author

Ericson2314 commented Nov 26, 2021

Well, the "new developments" I was hoping for would be something like sharing a daemon / libnixstore with Guix, but they didn't seem vary interested at the time.

I might revise this to make it a more expansive call for layering Nix / separate packages, in which case it could become more RFC-worthy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.