pam module: enable pam_keyinit.so by default #30333

obadz · 2017-10-11T22:50:59Z

Motivation for this change

Fixes ecryptfs regression (from 17.03 to 17.09) as explained in #27574 (comment)

I'm unfortunately not entirely sure what other side-effects this might have, though this thread seems to say that enabling pam_keyinit is "recommended" anyway.

Hopefully someone more competent can opine.

From pam_keyinit's man page:

The pam_keyinit PAM module ensures that the invoking process has a session keyring other than the user default session keyring.

The session component of the module checks to see if the process's session keyring is the user default, and, if it is, creates a new anonymous session keyring with which to replace it.

If a new session keyring is created, it will install a link to the user common keyring in the session keyring so that keys common to the user will be automatically accessible through it.

The session keyring of the invoking process will thenceforth be inherited by all its children unless they override it.

This module is intended primarily for use by login processes. Be aware that after the session keyring has been replaced, the old session keyring and the keys it contains will no longer be accessible.

This module should not, generally, be invoked by programs like su, since it is usually desirable for the key set to percolate through to the alternate context. The keys have their own permissions system to manage this.

AFAIK we don't have a separate pam.nix for su and sudo so can't carve them out.

This module should be included as early as possible in a PAM configuration, so that other PAM modules can attach tokens to the keyring.

force

Causes the session keyring of the invoking process to be replaced unconditionally.

(Without force, regression wouldn't go away)

revoke

Causes the session keyring of the invoking process to be revoked when the invoking process exits if the session keyring was created for this process in the first place.

(Wasn't sure about this one but it sounded conservative so I put it in)

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

obadz · 2017-10-12T20:05:24Z

cc @edolstra @grahamc @ttuegel @globin

obadz · 2017-10-19T17:00:00Z

Ping.

ttuegel · 2017-10-20T14:15:19Z

I admit I don't totally understand what this does, but if it's upstream-recommended and it fixes ecryptfs, then I'm all for it! 😄

obadz · 2017-10-30T22:56:48Z

We're not in a great place as this is a regression in 17.09. When we get this approved I think we should keep it in master for at least 2 weeks before cherry-picking it in 17.09 to make sure there are no side-effect to the pam change. The saving grace is this probably affects very few people…

Chiiruno · 2018-01-28T23:49:01Z

Any updates?

fpletz · 2018-01-29T00:52:36Z

Debian and Ubuntu only have it in a few PAM configs:

$ grep keyinit /etc/pam.d/ -R
/etc/pam.d/runuser:session		optional	pam_keyinit.so revoke
/etc/pam.d/runuser-l:session		optional	pam_keyinit.so force revoke
/etc/pam.d/sshd:session    optional     pam_keyinit.so force revoke

This can also be done with our NixOS PAM module.

Can somebody test that this is enough to make ecryptfs work?

obadz · 2018-01-30T23:10:00Z

@fpletz, well I'm biased but I've been running with it since I opened the PR & it's worked for me..

fpletz · 2018-01-30T23:17:02Z

Okay. I'll try to give this a spin soon. We'll also have to find out why our ecryptfs test doesn't catch this.

obadz · 2018-01-31T00:28:57Z

I vaguely recall trying to extend the test to cover the failing usecases but it went nowhere (and now don't recall much of the subtleties). Mostly I think it had to do with mounting encrypted directories at other times than login.

Closes NixOS#27574

obadz · 2018-02-10T15:19:49Z

(I've rebased this on fresh master)

obadz · 2018-02-18T16:31:42Z

@fpletz, shall we merge this?

obadz · 2018-02-18T16:32:55Z

Looks like this fixes other issues: #32279 (comment)

obadz · 2018-02-18T16:34:58Z

So @eternaleye, based on #32279 (comment), you don't think we should merge it?

Chiiruno · 2018-03-02T03:09:22Z

I would like to see this merged, bcachefs with it's own encryption is very alluring.

eternaleye · 2018-03-02T20:19:03Z

@obadz Sorry I took so long to reply - that is correct, I think this should not be merged.

The issue is that this exposes shared, mutable, security-relevant state (the keyring) to all PAM sessions.

In addition, the issue with ecryptfs and bcachefs was fixed upstream; as a result, the next release of systemd (v238) will make this workaround superfluous.

Chiiruno · 2018-03-24T10:10:14Z

This should be fixed now by #37647 and 361bd59...2d2ab94 which upgraded systemd to 238.

Chiiruno · 2018-03-26T22:46:33Z

My comment on the bottom of #32279 may be relevant here.

fpletz · 2018-03-28T05:38:37Z

Could one of you please try a recent 18.03 beta release with the systemd bump #36918? It's supposed to also fix this issue.

Chiiruno · 2018-03-28T06:48:12Z

@fpletz That's what I did. See #32279
Unfortunately, for some strange reason the same problem exists. Have there been any other changes since #36918 that I should attempt a new try with?

fpletz · 2018-03-28T06:53:04Z

Thanks! There was a bump to systemd to use the stable tree but I haven't checked if fixes for this issue went in there.

Chiiruno · 2018-03-28T06:54:49Z

I'll try again with that commit to make sure it's just not user error, we'll see if this is still the case.

Chiiruno · 2018-03-28T07:20:56Z

@fpletz
I can confirm that the same problem is occurring. It is having trouble requesting the key, and then can't allocate memory. I tested on a staging-18.03 USB drive.

This fixes a bug with changed qemu network interface names and also generally should be preferred to using a release tag.

ttuegel · 2018-04-24T17:21:56Z

I am still experiencing #27574 after the systemd-238 update.

obadz requested a review from edolstra October 19, 2017 17:00

hyperfekt mentioned this pull request Jan 17, 2018

Key sharing for bcachefs and ecryptfs #32279

Open

fpletz added this to the 17.09 milestone Jan 30, 2018

fpletz added 6.topic: nixos 0.kind: regression Something that worked before working no longer labels Jan 30, 2018

pam module: enable pam_keyinit.so by default

b878adf

Closes NixOS#27574

obadz force-pushed the pam_keyinit branch from 7072eec to b878adf Compare February 10, 2018 15:15

GrahamcOfBorg added 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Feb 10, 2018

Chiiruno mentioned this pull request Mar 13, 2018

systemd is out of date. #36918

Closed

fpletz closed this Mar 28, 2018

fpletz referenced this pull request Apr 1, 2018

systemd: use stable tree

6b9771e

This fixes a bug with changed qemu network interface names and also generally should be preferred to using a release tag.

hyperfekt mentioned this pull request Oct 31, 2018

nixos/pam: add defaults option #49506

Closed

9 tasks

hyperfekt mentioned this pull request Feb 9, 2019

linux_testing_bcachefs,bcachefs-tools: 20190123 #54752

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pam module: enable pam_keyinit.so by default #30333

pam module: enable pam_keyinit.so by default #30333

obadz commented Oct 11, 2017 •

edited

obadz commented Oct 12, 2017

obadz commented Oct 19, 2017

ttuegel commented Oct 20, 2017

obadz commented Oct 30, 2017

Chiiruno commented Jan 28, 2018

fpletz commented Jan 29, 2018

obadz commented Jan 30, 2018

fpletz commented Jan 30, 2018

obadz commented Jan 31, 2018

obadz commented Feb 10, 2018

obadz commented Feb 18, 2018

obadz commented Feb 18, 2018

obadz commented Feb 18, 2018

Chiiruno commented Mar 2, 2018

eternaleye commented Mar 2, 2018

Chiiruno commented Mar 24, 2018

Chiiruno commented Mar 26, 2018

fpletz commented Mar 28, 2018

Chiiruno commented Mar 28, 2018

fpletz commented Mar 28, 2018

Chiiruno commented Mar 28, 2018

Chiiruno commented Mar 28, 2018 •

edited

ttuegel commented Apr 24, 2018

pam module: enable pam_keyinit.so by default #30333

pam module: enable pam_keyinit.so by default #30333

Conversation

obadz commented Oct 11, 2017 • edited

Motivation for this change

Things done

obadz commented Oct 12, 2017

obadz commented Oct 19, 2017

ttuegel commented Oct 20, 2017

obadz commented Oct 30, 2017

Chiiruno commented Jan 28, 2018

fpletz commented Jan 29, 2018

obadz commented Jan 30, 2018

fpletz commented Jan 30, 2018

obadz commented Jan 31, 2018

obadz commented Feb 10, 2018

obadz commented Feb 18, 2018

obadz commented Feb 18, 2018

obadz commented Feb 18, 2018

Chiiruno commented Mar 2, 2018

eternaleye commented Mar 2, 2018

Chiiruno commented Mar 24, 2018

Chiiruno commented Mar 26, 2018

fpletz commented Mar 28, 2018

Chiiruno commented Mar 28, 2018

fpletz commented Mar 28, 2018

Chiiruno commented Mar 28, 2018

Chiiruno commented Mar 28, 2018 • edited

ttuegel commented Apr 24, 2018

obadz commented Oct 11, 2017 •

edited

Chiiruno commented Mar 28, 2018 •

edited