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

nixos/conduwuit: init #353651

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

niklaskorz
Copy link
Contributor

@niklaskorz niklaskorz commented Nov 4, 2024

As conduwuit deviates more and more from its conduit origins, it makes sense to give conduwuit its own module.
A custom module has been wished by upstream maintainers (@girlbossceo) as well as fellow nixpkgs maintainers (@nyabinary).

This allows us to cover breaking changes such as:

  • conduwuit does not support sqlite as database backend (unlike conduit)
  • conduwuit supports listening to unix sockets (unlike conduit)

Also includes the conduwuit-specific(?) fix from #353634.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Nov 4, 2024
@h7x4 h7x4 added 8.has: module (new) This PR adds a module in `nixos/` 8.has: tests This PR has tests labels Nov 4, 2024
};
global.address = lib.mkOption {
type = lib.types.str;
default = "::1";

Choose a reason for hiding this comment

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

Is it possible to have no default in the NixOS module? One of the issues that break UNIX sockets with conduwuit using the Conduit module is because the module forcefully listens on [::1]:6167 if you don't specify address, but we can't listen on both TCP and UNIX at the same time when listening on a UNIX socket requires address to not be filled out / populated.

Basically, a working UNIX socket example in conduwuit would be:

#address = "127.0.0.1"
#port = 6167
unix_socket_path = "/run/conduwuit/conduwuit.sock"

The Conduit module forces this config which does not work:

address = "::1"
port = 6167
unix_socket_path = "/run/conduwuit/conduwuit.sock"

PrivateUsers = true;
RestrictAddressFamilies = [
"AF_INET"
"AF_INET6"

Choose a reason for hiding this comment

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

Add AF_UNIX to this list

"@system-service"
"~@privileged"
];
StateDirectory = "conduwuit";

Choose a reason for hiding this comment

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

Could we also add RuntimeDirectory=conduwuit and RuntimeDirectoryMode=0750 to allow users to place the UNIX socket in /run/conduwuit/conduwuit.sock and give any systemd units or users permission to read/write the socket with the conduwuit group? This is how I use UNIX sockets with conduwuit for slightly increased host security.

Obviously nothing is stopping a user from putting the socket in /tmp or something.

SystemCallArchitectures = "native";
SystemCallFilter = [
"@system-service"
"~@privileged"
Copy link

@girlbossceo girlbossceo Nov 4, 2024

Choose a reason for hiding this comment

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

I recommend this syscall filter which is more secure, and I've identified @resources to be required as well. (from https://github.com/girlbossceo/conduwuit/blob/main/debian/conduwuit.service#L45-L47)

SystemCallArchitectures=native
SystemCallFilter=@system-service @resources
SystemCallFilter=~@clock @debug @module @mount @reboot @swap @cpu-emulation @obsolete @timer @chown @setuid @privileged @keyring @ipc

and is set to be read only.
'';
};
global.database_backend = lib.mkOption {

Choose a reason for hiding this comment

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

I might actually prefer if this is just removed all together. Because we only support RocksDB, and have no intentions or plans to support any other databases for quite a long time, this config option is a no-op and does not do anything in conduwuit, so it's just unnecessary.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10 labels Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (new) This PR adds a module in `nixos/` 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: tests This PR has tests 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants