linux-pam: make it use SUID wrapped version of unix_ckpwd

[vcunat]

Motivation for this change

Fix regressions like #153104 (comment)

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, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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/)
• 22.05 Release Notes (or backporting 21.11 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
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[vcunat]

It's unfortunate that the rebuild is large, so it's not so easy to test. It would be nice to get confirmation that known regressions really do get fixed by this.

[winterqt]

Diff LGTM.

[mweinelt]

Rebuilding my system with this change, as I was bitten by the swaylock problem this morning.

[andrevmatos]

This seems to apply only to pam, right? kscreenlocker seems to use pam lib, and have no direct reference to unix_chkpwd, so this may work for it, but I'm not sure if this binary is used somewhere else. Maybe just reverting to the symlink hack would be more future-proof?

[vcunat]

I think this would also fix invocation via the lib, using the .so module that gets patched here, but I'm not sure. Reverting to the well tested state is certainly an option to consider, though that approach seems less clean to me. (either is the same mass rebuild)

EDIT: or more precisely, I'd hope this addresses all methods of unix_chkpwd invocation except for directly running that (wrong) executable.

[mweinelt]

Can confirm swaylock unlock works again.

[andrevmatos]

If possible, despite it being a big rebuild, I'd call for the fix for this issue (be it this PR or the revert approach) to be merged directly to master, since this is a quite urgent issue for UX and is currently affecting anyone who updates to unstable.

[cole-h]

May just be me being selfish, but I agree. Would be nice to not have to pkill -f swaylock every time I lock my computer ^^;

[c0deaddict]

Merge to master would be nice 👍 i3lock is also affected by this.

[vcunat]

I'm really sorry for the regression.

Precise rebuild amounts pulled from ofborg-eval (quite high):

  27940 aarch64-linux
  28698 i686-linux
      4 x86_64-darwin
  31411 x86_64-linux

[mweinelt]

May just be me being selfish, but I agree. Would be nice to not have to pkill -f swaylock every time I lock my computer ^^;

loginctl unlock-sessions should do it.

[mohe2015]

If it's the same issue "switch user" on kde and then login to the same user seemed to work yesterday.

[mweinelt]

We can't really expect people to work around broken lockscreens for two weeks, so this should probably go to master instead.

[vcunat]

We might at least look for other very-important (or unlikely-to-break) changes in master..staging. Since the rebuild amount of this is > 50% anyway, it can't possibly be quick, at least not without "hacks" like system.replaceRuntimeDependencies.

People can roll back; that doesn't seem terribly surprising on "-unstable" (though I really am sorry for this).

I'm also not clear about if this PR is sufficiently confirmed or we rather revert.

[c0deaddict]

We might at least look for other very-important (or unlikely-to-break) changes in master..staging. Since the rebuild amount of this is > 50% anyway, it can't possibly be quick, at least not without "hacks" like system.replaceRuntimeDependencies.

People can roll back; that doesn't seem terribly surprising on "-unstable" (though I really am sorry for this).

I'm also not clear about if this PR is sufficiently confirmed or we rather revert.

I'm rebuilding my system with this PR to see if it fixes the problem, but it's taking quite a long time.

[winterqt]

I am also very sorry for this regression.

@vcunat I'd say that if multiple people (using different lock screens) have their issues fixed, then we should consider this PR over reverting.

[c0deaddict]

We might at least look for other very-important (or unlikely-to-break) changes in master..staging. Since the rebuild amount of this is > 50% anyway, it can't possibly be quick, at least not without "hacks" like system.replaceRuntimeDependencies.
People can roll back; that doesn't seem terribly surprising on "-unstable" (though I really am sorry for this).
I'm also not clear about if this PR is sufficiently confirmed or we rather revert.

I'm rebuilding my system with this PR to see if it fixes the problem, but it's taking quite a long time.

The rebuild is taking too long, i've given up. Is there another way to test this PR with i3lock without rebuilding the entire system?

[vcunat]

Rebuild just i3lock, I think.

[vcunat]

Confirmed. Rebuilding just i3lock fixes it for me.

[vcunat]

Same with xlock from xlockmore package.

[samueldr]

I have a build running of plasma mobile, though I suspect it's going to take 10+ hours more until it's done. Given your observations, the few testers who reported successes, and the scope of the issue, I guess this can be at least undrafted?

[putchar]

Confirmed. Rebuilding just i3lock fixes it for me.

hello
i am sorry to ask
while this pr is on review , should be put an example of making an overlay for nixos / hm and for a configuration inside a flake ?

[berbiche]

@vcunat
Confirmed. Rebuilding just i3lock fixes it for me.

In the meantime, here's the fix provided as an overlay:

# some-overlay.nix
final: prev:
let
  patchedPkgs = import (builtins.fetchTarball {
    url = "https://github.com/nixos/nixpkgs/archive/ffdadd3ef9167657657d60daf3fe0f1b3176402d.tar.gz";
    sha256 = "1nrz4vzjsf3n8wlnxskgcgcvpwaymrlff690f5njm4nl0rv22hkh";
  }) {
    inherit (prev) system config;
    # inherit (prev) overlays;  # not sure
  };
  patchedPam = patchedPkgs.pam;
in {
  i3lock = prev.i3lock.override { pam = patchedPam; };
  # apply the same patch to other packages
}

(I was brought here due to a discussion in the HM matrix room)

[vcunat]

Also confirmed with xsecurelock.

[jonringer]

seems like everyone is good with this going into staging

[jonringer]

LGTM

Result of nixpkgs-review pr 156974 run on x86_64-linux 1

1 package built:

• linux-pam

[miallo]

@vcunat Don't worry - these things happen ;)

[NANASHI0X74]

I think this should be fixed on master as well soon, as previously said:

We can't really expect people to work around broken lockscreens for two weeks, so this should probably go to master instead.

Are there any plans to do so? I guess someone has to open another PR with the same changes to master? Should I just do that?

[vcunat]

No. For days we've only been waiting for the rebuilds to finish; something like 2/3 done now. You can watch the counts on https://hydra.nixos.org/jobset/nixpkgs/staging-next

[vcunat]

Naturally, you can use the staging-next branch already. Individual builds are uploaded to cache.nixos.org immediately after they finish.

[NANASHI0X74]

No. For days we've only been waiting for the rebuilds to finish; something like 2/3 done now. You can watch the counts on https://hydra.nixos.org/jobset/nixpkgs/staging-next

Do you mean to say that after those rebuilds are done another pr to master can be started or that the rebuilds are so much load that we shouldn't do another PR? 😅 was a bit confused

[collares]

@NANASHI0X74 The PR to master is #157215, and it will be merged when rebuilds finish.

[NANASHI0X74]

ahh, thanks a lot, I didn't see that one 😄