Skip to content

Replace simple activationScripts #263203

Merged
lheckemann merged 22 commits intoNixOS:masterfrom
nikstur:replace-activation
Oct 28, 2023
Merged

Replace simple activationScripts #263203
lheckemann merged 22 commits intoNixOS:masterfrom
nikstur:replace-activation

Conversation

@nikstur
Copy link
Contributor

@nikstur nikstur commented Oct 24, 2023

This is one part of a series of PRs towards activation without Perl. See more about this larger project here: https://pad.lassul.us/nixos-perlless-activation#

This PR is part of step 1 of the larger project.

In this PR, I replace many of the simple activationScripts. I employed this strategy to remove activationScripts:

  1. removing them without replacement because they are not needed anymore
  2. replacing them via ExecPreStart=
  3. replacing them via a separate service ordered correctly before the other services that need it
  4. replacing them via a separate service that runs very early, e.g. with
    wantedBy = [ "sysinit.target" ];
    before = [ "sysinit.target" ];
    unitConfig.DefaultDependencies = false;

One of the immediate benefits of this work is that these activationScripts now actually run after the initrd when you use the systemd initrd. The systemd initrd calls stage-2-init.sh in the initrd as ./prepare-root

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/)
  • 23.11 Release Notes (or backporting 23.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.

The activationScript does not seem to be necessary anymore as the paths
are created anyways.
@nikstur nikstur requested a review from dasJ as a code owner October 24, 2023 18:19
@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 Oct 24, 2023
@nikstur nikstur force-pushed the replace-activation branch from 0b039b6 to 0c4ee3d Compare October 24, 2023 18:24
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Oct 24, 2023
Create the wrappers via a separate systemd service.
@nikstur nikstur force-pushed the replace-activation branch from 0c4ee3d to c3018b4 Compare October 24, 2023 22:47
blitz and others added 3 commits October 25, 2023 00:48
@nikstur nikstur force-pushed the replace-activation branch from c3018b4 to 6b07cc2 Compare October 24, 2023 22:57
Copy link
Contributor

@l0b0 l0b0 left a comment

Choose a reason for hiding this comment

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

I've only reviewed the shell script parts of this PR.

@nikstur
Copy link
Contributor Author

nikstur commented Oct 25, 2023

I've only reviewed the shell script parts of this PR.

I will not change the shell scripts themselves. I just move them into a systemd service or preStart.

Improving the shell scripts is out of scope.

Edit: Thank you for leaving these improvements here. I hope someone else can pick them up in a separate PR!

@nikstur nikstur force-pushed the replace-activation branch from 138fe17 to d300940 Compare October 26, 2023 00:14
@nikstur
Copy link
Contributor Author

nikstur commented Oct 26, 2023

@ofborg test wrappers mysql.mysql80 iscsi-root strongswan-swanctl mattermost systemd-binfmt systemd-timesyncd opensearch.opensearch stunnel grafana.provision activcation-nix-channel activation-var

The stargazer test seems to be broken on master

@nikstur
Copy link
Contributor Author

nikstur commented Oct 27, 2023

@ElvishJerricco is this good to go now?

Copy link
Contributor

@ElvishJerricco ElvishJerricco left a comment

Choose a reason for hiding this comment

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

I think it looks good yea

@delroth delroth added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Oct 27, 2023
@lheckemann lheckemann merged commit 8670794 into NixOS:master Oct 28, 2023
@K900
Copy link
Contributor

K900 commented Oct 29, 2023

a8f50f9 broke the installer tests.

@K900
Copy link
Contributor

K900 commented Oct 29, 2023

Reverting in #264200

@nikstur
Copy link
Contributor Author

nikstur commented Oct 29, 2023

Reverting in #264200

Thank you for taking the time to only revert the offending commit!

@NetaliDev
Copy link
Member

NetaliDev commented Dec 1, 2023

b5617e0 broke the MySQL auth module since it now depends on a local mysql database. Additionally, the script needs to run as root for the chown calls.

system.activationScripts.hostname = let
effectiveHostname = config.boot.kernel.sysctl."kernel.hostname" or cfg.hostName;
in optionalString (effectiveHostname != "") ''
hostname "${effectiveHostname}"
Copy link
Member

@arianvp arianvp Dec 15, 2023

Choose a reason for hiding this comment

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

This is a slight regression in behaviour. Systemd refuses to set the transient hostname altogether if /etc/hostname exists. But I think that is harmless. gethostname will still return the correct thing I think

nikstur pushed a commit to arianvp/nixpkgs that referenced this pull request Jul 20, 2024
We want to get rid of specialFileSystems / earlyMountScript eventually and
there is no need to run this before systemd anymore now that
the wrappers themselves are set up in a systemd unit since NixOS#263203

Also this is needed to make soft-reboot work. We want to make sure
that we remount /run/wrappers with the nosuid bit removed on soft-reboot
but because @earlyMountScript@ happens in initrd, this wouldn't happen
aszlig added a commit that referenced this pull request Feb 11, 2025
Version 257.1 of systemd changed[1] the PrivateTmp setting for the
systemd-timesyncd service from "yes" to "disconnected", which broke our
systemd-timesyncd test.

The reason for this is because the systemd-tmpfiles-setup.service is
*only*[2] added as a dependency of systemd-timesyncd.service if
PrivateTmp is set to "yes" but not when it is set to "disconnected"
(which would make sense given that the tmpfiles.d mechanism was
originally designed for temporary files).

Commit 339a866 switched the activation
script to using systemd-tmpfiles, but the commit in question doesn't
provide an explanation why this was necessary in this particular case.

However the pull request[3] lists an ongoing effort to get rid of Perl
and in the future get also rid of BASH for activation. The reasons for
doing this are outlined in the document[4]:

> The simple presence of interpreters on a system pose a security risk.
> An attacker that gains access to a system can abuse them to execute
> arbitrary commands. Mitre lists this as technique T1059. The most
> radical yet simple solution to mitigate this exploit is to remove all
> interpreters from a system (Mitre M1042). This radical solution is
> only really feasible and/or interesting for appliances (i.e.
> non-interactive) systems. Especially for high-security solutions this
> mitigtation is interesting.

I personally don't think this is a very compelling reason, at least for
our activation scripts, since an attacker could simply drop an
executable binary. Nevertheless, getting rid of additional dependencies
on eg. Perl or BASH is something worth pursuing to trim down moving
parts.

To address this, I decided to implement this as a normal systemd service
unit, since we need to guarantee that it's started before
systemd-timesyncd.service and with a dedicated unit we can ensure
explicit ordering. This has the advantage that we don't interfere with
the effort of getting rid of Perl/BASH for activation/boot and also
don't risk running into race conditions (again) because it's very
unlikely that systemd will change/deprecate explicit unit ordering in
the near future.

[1]: systemd/systemd@1f6e192
[2]: https://github.com/systemd/systemd/blob/30675a6ee98540a02bd1d6afcf80f0c0aa8c0910/src/core/unit.c#L1274
[3]: #263203
[4]: https://pad.lassul.us/nixos-perlless-activation

Signed-off-by: aszlig <[email protected]>
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 (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.