nixos/openldap: refactor the openldap module to make it safer and more versatile

[poscat0x04]

Motivation for this change

Changes

• Run OpenLDAP in foreground
• Use systemd's StateDirectory to create configuration dir (if it is the default config dir), and database dirs (if they are located under /var/lib/openldap)
• Forbid setting declarativeContents for database whos directory is not under /var/lib/openldap to avoid accidentally rm -rf of the whole directory
• Run both the prestart script and slapd as unprivileged user instead of root
• Change the default config directory to /var/lib/openldap/slapd.d

cc @kwohlfahrt

Todos

• Test on my machine
• Fix tests

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• 21.11 Release Notes (or backporting 21.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.

[kwohlfahrt]

• Run OpenLDAP in foreground

I don't think this is a good idea, for reasons mentioned inline.

• Change the default config directory to /var/lib/openldap/slapd.d

Why this change? Generally, system configuration lives in /etc, and this seems like system configuration...

• Use systemd's StateDirectory to create configuration dir (if it is the default config dir), and database dirs (if they are located under /var/lib/openldap)

Makes sense for database dirs, but I'm not sure about the configuration dir. See inline comment as well.

• Forbid setting declarativeContents for database whos directory is not under /var/lib/openldap to avoid accidentally rm -rf of the whole directory
• Run both the prestart script and slapd as unprivileged user instead of root

I like these, these are good improvements.

[kwohlfahrt]

Can we use ConfigurationDirectory for the configuration? IMO it makes sense for it to be separate from the rest of the state.

[aanderse]

It depends... when an application manages its own configuration we often call that "state" in NixOS land.

This module allows either the application or NixOS to manage the configuration (and maybe both?)

One argument for using StateDirectory is that the ownership allows you to run the application as non root entirely - ConfigurationDirectory does not allow this.

Good points on both side.

[kwohlfahrt]

Yeah, openldap is a bit weird with this. Because on the one hand, the OLC configuration can be changed at run-time, by writing to the appropriate directory via LDAP. OTOH, we nuke those settings every restart, with the declarative settings. This was a conscious choice, since it seems the most nix-y.

IMO it would be nice to deny writes to the configuration directory entirely, so that we don't surprise users by dropping settings after they are applied. I think this would require running the pre-start script as root, but the process could still be non-root (as it just needs read access)?

[kwohlfahrt]

Looks like good improvements to maintainability, especially this one:

Forbid setting declarativeContents for database whos directory is not under /var/lib/openldap to avoid accidentally rm -rf of the whole directory

That was kind of dangerous before!

Sorry for the delay in reviewing, I meant to do it last weekend but was busier than I expected.

[poscat0x04]

Why this change? Generally, system configuration lives in /etc, and this seems like system configuration...

I mean the purpose of OLC (on-line configuration) is to make the configuration more like a database than a static config file so that configuration can be changed (via LDAP) without restarting the server. I think it makes sense to put the configuration database, which is stateful, in /var instead of /etc. While it is true that currently, the default configuration directory will always be stateless, I plan to add a third option that is somewhere between a manually managed (stateful) configuration directory and a stateless one: populate the default config directory if it is empty and no nothing otherwise. So, I think it's reasonable to treat the default configuration as stateful.

[poscat0x04]

I can confirm that OpenLDAP has been working on my machine for two weeks now (running alongside postfix and dovecot).

[poscat0x04]

NixOS tests has been fixed

[kwohlfahrt]

I think OLC doesn't fit well with the general NIxOS philosophy, of having declarative configuration entirely controlled by NixOS modules, because the actual system configuration might diverge over time from what is configured through the modules. However, it is a powerful feature, and might be necessary for some people, and we don't want to block them from using NixOS.

IMO, the best solution would be to have this depend on the enableStatefulConfiguration option (of whatever it should be called). If false (the default), state is read-only (i.e. in /etc), managed entirely by NixOS. If true, state is writable by OpenLDAP, and only initialized from the Nix configuration, and therefore must live in /var. I realize this is a bit more work to implement, but this seems like it would be the best long term. What do you think?

[poscat0x04]

I still don't think it's a good idea to put the OLC database in /etc. If we really need a read-only configuration, we could just use a derivation to build it. Putting things in /etc just feels.. awkward. Afaik, the reason for most of the programs that write their configuration to /etc to do so is that they support hot reloading (which is not possible without putting the configuration file in a fixed location).
And I would argue that read-only configuration provides no obvious benefits and complicates the design of the module (thus reducing the maintainability). Putting the OLC database in /var works and is definitely simpler. And it's not like there's no precedence of putting configuration file in /var -- the PostgreSQL module does this as well.

[Mic92]

I would leave reviewing of this PR to you (@kwohlfahrt) and just do a final test on my infrastructure once it is ready.

[kwohlfahrt]

I still don't think it's a good idea to put the OLC database in /etc. If we really need a read-only configuration, we could just use a derivation to build it. Putting things in /etc just feels.. awkward. Afaik, the reason for most of the programs that write their configuration to /etc to do so is that they support hot reloading (which is not possible without putting the configuration file in a fixed location).

That's a good approach, can we move the current preStart script into a derivation? That sounds like it would fit the Nix-managed (i.e. readonly) use case. Then the OLC-enabled config can live in /var, and the fixed output in the NIx store.

[kwohlfahrt]

If we really need a read-only configuration, we could just use a derivation to build it.

That's a good approach, can we move the current preStart script into a derivation?

I tried to implement this this today, and it looks like it might not be workable. Unfortunately, file includes (like foo:< file:///path/to/value), are resolved when the configuration is built, not loaded. So it cannot be built from a derivation, since these are sandboxed. This is probably the best way we currently have of handling passwords as well for read-only config (as we don't want them copied into the store), so it probably does need to run in the preStart script, with access to the file-system.

And I would argue that read-only configuration provides no obvious benefits

I disagree on this - knowing that my configuration exactly matches what is declared in the module (and won't change when I redeploy the service) is a big benefit for me.

I can implement this and open a PR against your branch, then we can merge this?

[poscat0x04]

Sure.

[kwohlfahrt]

Just in case you missed it, I opened a PR here: poscat0x04#1 - let me know what you think!