Skip to content

dhcpcd: bring back enablePrivSep option, nixos/release-notes: remove duplicate note#347578

Merged
rnhmjoj merged 2 commits intoNixOS:staging-nextfrom
rnhmjoj:staging-next
Oct 12, 2024
Merged

dhcpcd: bring back enablePrivSep option, nixos/release-notes: remove duplicate note#347578
rnhmjoj merged 2 commits intoNixOS:staging-nextfrom
rnhmjoj:staging-next

Conversation

@rnhmjoj
Copy link
Contributor

@rnhmjoj rnhmjoj commented Oct 9, 2024

A couple of fixes of #336988

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 via dhcpcd.tests
  • built manual as nix build -f nixos/release.nix manual.x86_64-linux
  • 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.

cc: @aanderse @Izorkin @vcunat

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog This PR adds or changes release notes labels Oct 9, 2024
@Izorkin
Copy link
Contributor

Izorkin commented Oct 9, 2024

Can do revert commit a432668 ?

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Oct 9, 2024

I just brought back the build option because it may be useful on other distros. The rest... I don't see why you would use it on NixOS, given with the service running unprivileged. Am I missing something?

@ofborg ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux. labels Oct 9, 2024
@Atemu Atemu changed the title Fixup of PR #336988 dhcpcd: bring back enablePrivSep option, nixos/release-notes: remove duplicate note Oct 10, 2024
@Izorkin
Copy link
Contributor

Izorkin commented Oct 10, 2024

I thought it was possible to make a default parameter. I use profile hardened and jemalloc memory allocator. The default profile uses the scudo memory allocator, which dhcpcd also works with.

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Oct 10, 2024

I thought it was possible to make a default parameter.

I'm not sure what you mean: enablePrivSep ? true? IIRC it can't be disabled at runtime, so dhcpcd alway tries to setuid and crashes because it's now running as an unprivilged user.

@Izorkin
Copy link
Contributor

Izorkin commented Oct 10, 2024

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Oct 10, 2024

But what for? Privsep is now unnecessary thanks to the systemd hardening, and if for some reason you want a different configuration you can just do dhcpcd.override { enablePrivSep = true; }.

@Izorkin
Copy link
Contributor

Izorkin commented Oct 10, 2024

But what for? Privsep is now unnecessary thanks to the systemd hardening, and if for some reason you want a different configuration you can just do dhcpcd.override { enablePrivSep = true; }.

Ok.

@Izorkin
Copy link
Contributor

Izorkin commented Oct 10, 2024

Have you tried comparing capabilites between the two options at the dhcpcd processes?

@Izorkin
Copy link
Contributor

Izorkin commented Oct 10, 2024

Capabilites with use enablePrivSep:

CapInh: 0000000000000000
CapPrm: 0000000000000000
CapEff: 0000000000000000
CapBnd: 00000000000434c0
CapAmb: 0000000000000000
0x00000000000434c0=cap_setgid,cap_setuid,cap_net_bind_service,cap_net_admin,cap_net_raw,cap_sys_chroot

Capabilites without use enablePrivSep and run as unprivileged user:

CapInh: 0000000000203500
CapPrm: 0000000000003400
CapEff: 0000000000003400
CapBnd: 000001f3f5fcffff
CapAmb: 0000000000003400
0x0000000000203500=cap_setpcap,cap_net_bind_service,cap_net_admin,cap_net_raw,cap_sys_admin
0x0000000000003400=cap_net_bind_service,cap_net_admin,cap_net_raw
0x000001f3f5fcffff=cap_chown,cap_dac_override,cap_dac_read_search,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_linux_immutable,cap_net_bind_service,cap_net_broadcast,cap_net_admin,cap_net_raw,cap_ipc_lock,cap_ipc_owner,cap_sys_chroot,cap_sys_ptrace,cap_sys_pacct,cap_sys_admin,cap_sys_boot,cap_sys_nice,cap_sys_resource,cap_sys_tty_config,cap_lease,cap_audit_write,cap_audit_control,cap_setfcap,cap_mac_override,cap_mac_admin,cap_block_suspend,cap_audit_read,cap_perfmon,cap_bpf,cap_checkpoint_restore

Capabilites with use enablePrivSep and run as unprivileged user:

CapInh: 0000000000043440
CapPrm: 0000000000043440
CapEff: 0000000000043440
CapBnd: 000001ffffffffff
CapAmb: 0000000000043440

0x0000000000043440=cap_setgid,cap_net_bind_service,cap_net_admin,cap_net_raw,cap_sys_chroot

Edit: Updated the test results.

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Oct 10, 2024

I'm not sure what you're getting at, I don't see anything unexpected: systemd sets the ambient capabilities to "CAP_NET_ADMIN, CAP_NET_RAW CAP_NET_BIND_SERVICE", then inheritable = permitted = effective = ambient and everything is in the bouding set.

@Izorkin
Copy link
Contributor

Izorkin commented Oct 10, 2024

I've updated the previous comment.
Judging by the tests the parameter using the enablePrivSep parameter, the service needs to be granted more privileges.
I thought it would be the other way around.

@Izorkin
Copy link
Contributor

Izorkin commented Oct 10, 2024

I assume there is no point to this parameter then?

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Oct 11, 2024

You mean building dhcpcd with enablePrivSep = true? As I said earlier, it may be useful on other distros without systemd (hardening).

@Izorkin
Copy link
Contributor

Izorkin commented Oct 11, 2024

You mean building dhcpcd with enablePrivSep = true?

Yes, also had to add a few parameters to AmbientCapabilities as well.

As I said earlier, it may be useful on other distros without systemd (hardening).

Ok.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Oct 11, 2024
@rnhmjoj rnhmjoj merged commit 3106e48 into NixOS:staging-next Oct 12, 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: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants