Bug 1045886

Summary: ecryptfs problems with recent Tumbleweed
Product: [openSUSE] openSUSE Tumbleweed Reporter: Roger Whittaker <roger.whittaker>
Component: OtherAssignee: Thorsten Kukuk <kukuk>
Status: RESOLVED FIXED QA Contact: E-mail List <qa-bugs>
Severity: Normal    
Priority: P5 - None CC: arvidjaar, fbui, josef.moellers, martin.wilck, nwr10cst-oslnx
Version: Current   
Target Milestone: ---   
Hardware: Other   
OS: Other   
See Also: https://bugzilla.opensuse.org/show_bug.cgi?id=1081947
Whiteboard:
Found By: --- Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---
Attachments: pam-config package with pam_keyinit support
Patch to add configuration of pam_keyinit to pam-config

Description Roger Whittaker 2017-06-25 06:13:16 UTC
ecryptfs and the ecryptfs tools have stopped working correctly on recent versions of Tumbleweed.

I noticed this because an existing ecryptfs Private directory could only be seen after a local login.  ecryptfs-mount-private over ssh stopped working.

On investigation, it is no longer possible to do ecryptfs-setup-private for a new user.


$ ecryptfs-setup-private 
Enter your login passphrase [roger]: 
Enter your mount passphrase [leave blank to generate one]: 

************************************************************************
YOU SHOULD RECORD YOUR MOUNT PASSPHRASE AND STORE IT IN A SAFE LOCATION.
  ecryptfs-unwrap-passphrase ~/.ecryptfs/wrapped-passphrase
THIS WILL BE REQUIRED IF YOU NEED TO RECOVER YOUR DATA AT A LATER TIME.
************************************************************************


Done configuring.

Testing mount/write/umount/read...
Inserted auth tok with sig [83ecc1d632221288] into the user session keyring
Inserted auth tok with sig [90e42628446d371b] into the user session keyring
mount: No such file or directory
ERROR:  Could not mount private ecryptfs directory

In the system logs we see:

2017-06-25T07:04:39.396038+01:00 [localhost] kernel: [140502.266628] Could not find key with description: [90e42628446d371b]
2017-06-25T07:04:39.396052+01:00 [localhost] kernel: [140502.266630] process_request_key_err: No key
2017-06-25T07:04:39.396054+01:00 [localhost] kernel: [140502.266631] Could not find valid key in user session keyring for sig specified in mount option: [90e42628446d371b]
2017-06-25T07:04:39.396056+01:00 [localhost] kernel: [140502.266631] One or more global auth toks could not properly register; rc = [-2]
2017-06-25T07:04:39.396072+01:00 [localhost] kernel: [140502.266632] Error parsing options; rc = [-2]

$ uname -r
4.11.6-1-default

$ grep VERSION_ID /etc/os-release
VERSION_ID="20170620"
Comment 1 Roger Whittaker 2017-06-25 06:28:20 UTC
I believe the problem here is related to systemd.

Previously I found that simply downgrading the kernel had no effect on this problem.  And there had been no change in the version of ecryptfs-utils.

But after forcing a downgrade to systemd-232-10.2, now I see success:

$ ecryptfs-setup-private 
Enter your login passphrase [roger]: 
Enter your mount passphrase [leave blank to generate one]: 

************************************************************************
YOU SHOULD RECORD YOUR MOUNT PASSPHRASE AND STORE IT IN A SAFE LOCATION.
  ecryptfs-unwrap-passphrase ~/.ecryptfs/wrapped-passphrase
THIS WILL BE REQUIRED IF YOU NEED TO RECOVER YOUR DATA AT A LATER TIME.
************************************************************************


Done configuring.

Testing mount/write/umount/read...
Inserted auth tok with sig [38016f72d097f6f6] into the user session keyring
Inserted auth tok with sig [a6db1a352f8f2d14] into the user session keyring
Inserted auth tok with sig [38016f72d097f6f6] into the user session keyring
Inserted auth tok with sig [a6db1a352f8f2d14] into the user session keyring
Testing succeeded.

Logout, and log back in to begin using your encrypted directory.
Comment 2 Andrei Borzenkov 2017-06-25 07:15:10 UTC
(In reply to Roger Whittaker from comment #1)
> 
> But after forcing a downgrade to systemd-232-10.2

You forgot to say what version of systemd has problem.
Comment 3 Roger Whittaker 2017-06-25 07:37:49 UTC
I think the problem was introduced with Tumbleweed snapshot 20170620, which has systemd version 233 for the first time.

https://lists.opensuse.org/opensuse-factory/2017-06/msg00620.html

When I forced the systemd packages down to systemd-232-10.2 (on Tumbleweed 20170622), I saw the problem go away as noted in comment#1.
Comment 4 Roger Whittaker 2017-06-25 08:02:01 UTC
Failing system has systemd-233-1.1.x86_64, ecryptfs-utils-108-3.2.x86_64. Now upgraded to 20170622 - problem persists.  But goes away if systemd is forced down to a 232 version.
Comment 5 Neil Rickert 2017-06-25 15:33:49 UTC
I can confirm this problem.

If I use ssh to login to the system, the "ecryptfs-mount-private" does not work.

If I login at the GUI prompt (I use "Icewm" with as someone not using "ecryptfs"), then running: "ecryptfs-setup-private" does not work.

I have not tried going back to an older "systemd" version.  For the present, I can live with the situation until it is fixed.  At one time, I would often login remotely to my work machine.  But now that I am retired, I don't need to do that.
Comment 6 Andrei Borzenkov 2017-06-25 17:57:10 UTC
(In reply to Roger Whittaker from comment #4)
> Failing system has systemd-233-1.1.x86_64

The obvious difference is that with 232 no kernel keyrings were setup by default at all (we do not have pam_keyinit on TW in default configuration), while with 233 each session sets up its own keyring. Unfortunately, these keyrings seem to propagate to user sessions, messing up keys lookup. More details

1. systemd commit:

commit 74dd6b515fa968c5710b396a7664cac335e25ca8
Author: Lennart Poettering <lennart@poettering.net>
Date:   Fri Dec 2 01:54:41 2016 +0100

    core: run each system service with a fresh session keyring

so far, so good.

2. Now lets look at keyrings immediately after logon

bor@10:~> : Before ecryptfs-setup
bor@10:~> cat /proc/keys
023c3b10 I--Q---     1 perm 0b0b0000     0     0 user      invocation_id: 16
05088f05 I--Q---    88 perm 3f030000     0     0 keyring   _ses: 1
2344184f I--Q---    41 perm 3f030000  1000   100 keyring   _ses: 1
bor@10:~> keyctl show -x @s
Keyring
0x05088f05 --alswrv      0     0  keyring: _ses
0x023c3b10 ----s-rv      0     0   \_ user: invocation_id

Note - our session keyring is owned by user 0!!! So it is the one inherited from systemd service. (Heck, is there any way to list session keyrings for each process?)

3. Now let's see what happens after ecryptfs-setup runs

bor@10:~> : After ecryptfs-setup
bor@10:~> cat /proc/keys
023c3b10 I--Q---     1 perm 0b0b0000     0     0 user      invocation_id: 16
041560fc I--Q---     1 perm 1f3f0000  1000 65534 keyring   _uid_ses.1000: 1
05088f05 I--Q---    90 perm 3f030000     0     0 keyring   _ses: 1
2344184f I--Q---    39 perm 3f030000  1000   100 keyring   _ses: 1
2540e350 I--Q---     2 perm 1f3f0000  1000 65534 keyring   _uid.1000: 2
28543b18 I--Q---     1 perm 3f010000  1000   100 user      ae0285ccbff6e3cb: 740
2dec2d68 I--Q---     1 perm 3f010000  1000   100 user      be1ad3e0687105d5: 740
bor@10:~> keyctl show -x @s
Keyring
0x05088f05 --alswrv      0     0  keyring: _ses
0x023c3b10 ----s-rv      0     0   \_ user: invocation_id
bor@10:~> keyctl show -x @us
Keyring
0x041560fc --alswrv   1000 65534  keyring: _uid_ses.1000
0x2540e350 --alswrv   1000 65534   \_ keyring: _uid.1000
0x2dec2d68 --alswrv   1000   100       \_ user: be1ad3e0687105d5
0x28543b18 --alswrv   1000   100       \_ user: ae0285ccbff6e3cb

Oops. ecryptfs adds keys to *user* keyring. This keyring is not used during default key lookup directly - only if it is linked from session (or in general other default search path) keyring. But we already have "wrong" session keyring which does *NOT* link to our user keyring. So default user session is not used and key is not found.

Adding pam_keyinit.so to default session PAM service fixes it (disclaimer - I do not know whether it may have some other side effects). Note that systemd 233 actually includes it in recommended PAM configuration shipped with it.
Comment 7 Andrei Borzenkov 2017-06-25 17:58:06 UTC
(In reply to Andrei Borzenkov from comment #6)
> while with 233 each session sets up its own keyring.

s/session/service/

sorry.
Comment 8 Franck Bui 2017-06-26 08:25:53 UTC
(In reply to Andrei Borzenkov from comment #6)

Thanks for your analyze !

Josef, it seems that pam_keyinit.so is not used at all on TW.

Was that done on purpose, or can we start using it ?

Thanks.
Comment 9 Josef Möllers 2017-06-26 10:03:31 UTC
(In reply to Franck Bui from comment #8)
> (In reply to Andrei Borzenkov from comment #6)
> 
> Thanks for your analyze !
> 
> Josef, it seems that pam_keyinit.so is not used at all on TW.
> 
> Was that done on purpose, or can we start using it ?
> 
> Thanks.

No idea whether that was done on purpose. There is nothing in PAM that would speak against using pam_keyinit. I don't know why it was left out.
Comment 10 Franck Bui 2017-06-26 10:15:27 UTC
OK, could you then include pam_keyinit.so in the pam setup in the next update and take over this bug ?

Thanks.
Comment 11 Josef Möllers 2017-06-27 07:15:46 UTC
(In reply to Franck Bui from comment #10)
> OK, could you then include pam_keyinit.so in the pam setup in the next
> update and take over this bug ?
> 
> Thanks.

I've discussed this with the original maintainer and he pointed me at a caveat in the pam_keyinit documentation:

"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."

So, as pam installs the "common-session" script and util-linux installs /etc/pam.d/su, which includes "common-session", it would not be wise to include pam_keyinit.so there.
Comment 12 Franck Bui 2017-06-27 08:43:57 UTC
(In reply to Josef Möllers from comment #11)
> So, as pam installs the "common-session" script and util-linux installs
> /etc/pam.d/su, which includes "common-session", it would not be wise to
> include pam_keyinit.so there.

OTOH this module is supposed to be used by login processes, so I'm not sure adding it in systemd-user is a good idea.

I recall that we already discussed about su(1) and the way it creates sessions in another bug (sorry I can't retrieve it).

su(1) and sudo(1) seem really special and there're a couple of things that they shouldn't do like other "true" login processes. So maybe it's time to split "common-session" ?
Comment 13 Josef Möllers 2017-06-27 09:14:05 UTC
(In reply to Franck Bui from comment #12)
> (In reply to Josef Möllers from comment #11)
> > So, as pam installs the "common-session" script and util-linux installs
> > /etc/pam.d/su, which includes "common-session", it would not be wise to
> > include pam_keyinit.so there.
> 
> OTOH this module is supposed to be used by login processes, so I'm not sure
> adding it in systemd-user is a good idea.
> 
> I recall that we already discussed about su(1) and the way it creates
> sessions in another bug (sorry I can't retrieve it).

There was this bug about systemd DEBUG messages that looked like ERROR messages "Cannot create session": bsc#1040019

> su(1) and sudo(1) seem really special and there're a couple of things that
> they shouldn't do like other "true" login processes. So maybe it's time to
> split "common-session" ?

Or create a special one "common-session-su", which has all but the pam_keyinit.so?
Comment 14 Franck Bui 2017-06-27 10:15:53 UTC
(In reply to Josef Möllers from comment #13)
> Or create a special one "common-session-su", which has all but the
> pam_keyinit.so?

Quite frankly, that sounds strange but I'm really not an expert.

This probably needs to be discussed with broader audience.

In the meantime I think it's general issue about how pam_keyinit module should be integrated in the SUSE PAM setup so I'm reassigning this bug to you Josef, hoping that makes sense.
Comment 15 Martin Wilck 2017-06-28 12:12:04 UTC
(In reply to Andrei Borzenkov from comment #6)

> Oops. ecryptfs adds keys to *user* keyring. This keyring is not used during
> default key lookup directly - only if it is linked from session (or in
> general other default search path) keyring. But we already have "wrong"
> session keyring which does *NOT* link to our user keyring. So default user
> session is not used and key is not found.

man keyctl(1) says:

> User default session keyring: @us or -5
>              This is the default session keyring for a particular user. Login processes that change to a particular user will bind
>              to this session until another session is set.

So the problem is that systemd sets up a session key that doesn't link to the _uid_ses key, correct? Doesn't that seem to be systemd bug?

I just tested it here, the problem goes away after running  "keyctl link @us @s".
Comment 16 Andrei Borzenkov 2017-06-28 13:10:30 UTC
(In reply to Martin Wilck from comment #15)

> So the problem is that systemd sets up a session key that doesn't link to
> the _uid_ses key, correct?

systemd cannot setup session keyring for *user* session at all. In this case "session" keyring is inherited from service that caused launching user shell. I.e. display-manager.service, getty.service, sshd.service. systemd does not and cannot know which user will log in, so it cannot setup user session for this user at all.

> I just tested it here, the problem goes away after running  "keyctl link @us @s".

a) this causes default user session keyring for *every* user to be linked to *the same* "sesssion" keyring (of mentioned top level service), so every user will have access to every other user's keys. At least for display manager (switch user) and sshd (which can launch shells for multiple connected users).

b) someone still must do it during user logon. If systemd had not setup session keyring, this should happen automatically the first time user keyring was accessed, but using the default per-user unique session keyring.

I honestly think that while this upstream commit was well intended, it probably requires more discussion to understand its implications. The only justification was "have something unique to store invocation ID" and this can probably be implemented by deriving keyring names from service name and setting up unique keyring for each service. So far we never considered running service as having a "session" as opposed to user logon.

So my suggestion would be to revert it and discuss better :)
Comment 17 Roger Whittaker 2017-06-28 13:25:26 UTC
In answer to comment#15 I can confirm that the problem that led to my original report ("ecryptfs-mount-private" failing when run remotely) is solved by the workaround of running "keyctl link @us @s" first.

And running "keyctl link @us @s" before "ecryptfs-setup-private" also allows that command to work as expected.
Comment 18 Martin Wilck 2017-06-28 13:35:24 UTC
(In reply to Andrei Borzenkov from comment #16)

> > I just tested it here, the problem goes away after running  "keyctl link @us @s".
> 
> a) this causes default user session keyring for *every* user to be linked to
> *the same* "sesssion" keyring (of mentioned top level service),

So, by running that innocently-looking command, a user would inadvertently provide his personal keys to a system service?? URGH. That looks like a really nasty backdoor to me.

You say that systemd creates a session keyring for the newly created session *before* the user actually logs in (and takes control). Apparently the user's login process inherits this session keyring after login. But the Right Thing (TM) to do would be to start with the user default session key instead. That's what the keyctl man page says: "Login processes that change to a particular user will bind to this [the default user session keyring] session until another session is set". 

If the systemd session key is really required, the login/ssh/... process should setup yet a new session key (belonging to the user) and link to it both _uid_ses and systemd's session key. Is that what pam_keyinit is supposed to do?

> So my suggestion would be to revert it and discuss better :)

Ack.
Comment 19 Andrei Borzenkov 2017-06-28 13:54:20 UTC
(In reply to Martin Wilck from comment #18)
> 
> You say that systemd creates a session keyring for the newly created session
> *before* the user actually logs in (and takes control).

Correct. systemd creates session keyring for every service. Session keyring is inherited across fork/exec so in absence of any explicit action each process started by this service inherits it. If this service was responsible for user login, then all user process will inherit it as well.

Please do not be confused by adjective "session". It simply defines lifetime of keyring. It is *NOT* associated with actual login session as we understand it in any way.

> If the systemd session key is really required, the login/ssh/... process
> should setup yet a new session key (belonging to the user) and link to it
> both _uid_ses and systemd's session key.

How systemd service key comes in picture here? Again - it was added as *unique* per-service keyring; linking it to user accessible keyring will invalidate this use.

user processes after logon do not belong to systemd service that started logon manager. In any way. They are not supposed to share keys (at least, I do not see obvious use case).

> Is that what pam_keyinit is
> supposed to do?
> 

No, it is supposed to setup new unique session keyring for every logon session and link user keyring to it.
This better matches the adjective "session" IMHO :) Having each user session share the same default session keyring defeats its purpose; then we could just use user keyring itself. As implemented by pam_keyinit, every login session has unique private keyring and also access to shared user-wide keyring.
Comment 20 Andrei Borzenkov 2017-06-28 16:33:57 UTC
(In reply to Martin Wilck from comment #18)
> So, by running that innocently-looking command, a user would inadvertently
> provide his personal keys to a system service??

And to another user. To illustrate:

bor@10:~> id -a
uid=1000(bor) gid=100(users) groups=100(users)
bor@10:~> keyctl show -x
Session Keyring
0x2f8153fa --alswrv      0     0  keyring: _ses
0x144397e9 ----s-rv      0     0   \_ user: invocation_id
test@10:~> id -a 
uid=1001(test) gid=100(users) groups=100(users)
test@10:~> keyctl show -x

So both users already have access to exactly the same keyrings. Now let's try what you suggest.

bor@10:~> keyctl link @us @s
test@10:~> keyctl link @us @s

OK, let's check.

bor@10:~> keyctl show -x
Session Keyring
0x2f8153fa --alswrv      0     0  keyring: _ses
0x144397e9 ----s-rv      0     0   \_ user: invocation_id
0x095ea2d9 ---lswrv   1001 65534   \_ keyring: _uid_ses.1001
0x320d41af ---lswrv   1001 65534   |   \_ keyring: _uid.1001
0x0e9e06aa --alswrv   1000 65534   \_ keyring: _uid_ses.1000
0x18889b01 --alswrv   1000 65534       \_ keyring: _uid.1000
test@10:~> keyctl show -x
Session Keyring
0x2f8153fa --alswrv      0     0  keyring: _ses
0x144397e9 ----s-rv      0     0   \_ user: invocation_id
0x095ea2d9 --alswrv   1001 65534   \_ keyring: _uid_ses.1001
0x320d41af --alswrv   1001 65534   |   \_ keyring: _uid.1001
0x0e9e06aa ---lswrv   1000 65534   \_ keyring: _uid_ses.1000
0x18889b01 ---lswrv   1000 65534       \_ keyring: _uid.1000

So both users now have access to user keyring of each other.
Comment 21 Martin Wilck 2017-06-29 10:09:54 UTC
(In reply to Andrei Borzenkov from comment #20)

> So both users now have access to user keyring of each other.

That's a security problem by itself, even if we work around it using pam_keyinit. Activating pam_keyinit shouldn't be mandatory to keep users' keyrings separate, or we have a major security problem IMO. This "link" operation shouldn't be permitted.

Sure, the key permissions will probably inhibit one user to actually *use* the other one's keys but still this feels BAD.
Comment 22 Andrei Borzenkov 2017-06-29 10:45:56 UTC
(In reply to Martin Wilck from comment #21)
> Activating pam_keyinit shouldn't be mandatory to keep users' keyrings separate

By default each user gets default session keyring which *is* separate from other user's. The problem stems from inheriting keyring setup by systemd for service.

> 
> Sure, the key permissions will probably inhibit one user to actually *use*
> the other one's keys

No. All keys are "possessed" by each user in this case. So individual per-user permissions are not relevant.

If each user started with own unique session keyring, *then* accessing other user's keys would require explicit permissions.
Comment 23 Martin Wilck 2017-06-29 10:58:05 UTC
Coming back to "pam_keyinit and su":

I happen to have a Fedora 24 system here, too. Fedora uses the "authconfig" tool which is similar in purpose to "pam-config".
It creates common files such as "system-auth-ac", "password-auth-ac", "fingerprint-auth-ac", and usually "system-auth" is a symlink to "system-auth-ac", etc. Individual services include either "system-auth" or "password-auth", which are identical on my system.

system-auth includes "pam_keyinit.so", and various service files include it as well. Here's a list of the status on my system, where k,f,s,p stand for 
pam_keyinit, pam_keyinit force, system-auth, and password-auth, respectively, and upper case means "included" and lower case means "not included":

s p k f config-util
s p k f cvs
s p k f liveinst
s p k f other
s p k f passwd
s p k f postlogin
s p k f postlogin-ac
s p k f screen
s p k f setup
s p k f sssd-shadowutils
s p k f vlock
s p k f vmtoolsd

k f s P atd
k f s P crond
k f s P ppp
k f S p chfn
k f S p chsh
k f S p kcheckpass
k f S p kscreensaver
k f S p polkit-1
k f S p su
k f S p systemd-user

K f s p cups
K f s p fingerprint-auth
K f s p fingerprint-auth-ac
K f s p password-auth
K f s p password-auth-ac
K f s p runuser
K f s p smartcard-auth
K f s p smartcard-auth-ac
K f s p system-auth
K f s p system-auth-ac
K f S p sudo

K F s p gdm-fingerprint
K F s p gdm-smartcard
K F s p runuser-l
K F s p sudo-i
K F s p su-l
K F s p xserver
K F s P gdm-password
K F s P gdm-pin
K F s P remote
K F s P sshd
K F S p gdm-autologin
K F S p gdm-launch-environment
K F S p login

All of the listed services except for the first block call "pam_keyinit.so revoke", and those ion the last block use "pam_keyinit.so force revoke".

Note: "su" and "sudo" include pam_keyinit.so as well. Just the "login" variants "su -l" and "sudo -i" use the "force" parameter which means that the session key is replaced even if it is not the default session key.
In practice, on my F24 system, I see my user keys under "su" and "sudo" works just fine, except when I use the "login" variants.

Bottom line: 
 1. using pam_keyinit doesn't harm su / sudo, at least not on Fedora.
(F24 still has systemd-229, I should add).
 2. pam_keyinit is always used with "revoke" on Fedora.
Comment 24 Martin Wilck 2017-06-29 10:59:54 UTC
(In reply to Andrei Borzenkov from comment #22)

> > Sure, the key permissions will probably inhibit one user to actually *use*
> > the other one's keys
> 
> No. All keys are "possessed" by each user in this case. So individual
> per-user permissions are not relevant.
> 
> If each user started with own unique session keyring, *then* accessing other
> user's keys would require explicit permissions.

If that's true, and I don't doubt it if you say so, we really have a major security issue in TW.
Comment 25 Andrei Borzenkov 2017-07-05 03:35:38 UTC
Note that now upstream links user keyring to service session keyring thus exposing the exact key leak issue discussed here ...

https://github.com/systemd/systemd/commit/437a85112e02042b62751395b9e7225628c1b708

and proposed

https://github.com/systemd/systemd/pull/6286
Comment 26 Franck Bui 2017-07-06 14:20:42 UTC
(In reply to Andrei Borzenkov from comment #16)
> So my suggestion would be to revert it and discuss better :)

I agree and the session keyring stuff will be disabled in the next update.
Comment 27 Franck Bui 2017-07-07 15:33:03 UTC
(In reply to Andrei Borzenkov from comment #20)
> (In reply to Martin Wilck from comment #18)
> > So, by running that innocently-looking command, a user would inadvertently
> > provide his personal keys to a system service??
> 
> And to another user. To illustrate:
> 
> bor@10:~> id -a
> uid=1000(bor) gid=100(users) groups=100(users)
> bor@10:~> keyctl show -x
> Session Keyring
> 0x2f8153fa --alswrv      0     0  keyring: _ses
> 0x144397e9 ----s-rv      0     0   \_ user: invocation_id
> test@10:~> id -a 
> uid=1001(test) gid=100(users) groups=100(users)
> test@10:~> keyctl show -x
> 
> So both users already have access to exactly the same keyrings. Now let's
> try what you suggest.
> 

Since you don't show the result of "keyctl show -x" for "test" user, it's hard to say ;)

I've run the same test and the 2 users get a differ session keyring...

How did you log in BTW ? Through different ttys ?

> bor@10:~> keyctl link @us @s
> test@10:~> keyctl link @us @s
> 
> [...]
> 
> So both users now have access to user keyring of each other.

That's definitively weird and again I'm seeing different (and expected) results here.

User session keyring is supposed to be per UID resources, so "@us" for "bor" user should be something totally different from "@us" for "test". At least it's my slight understanding of the keyrings stuff.

Which kernel version are your running ? (I'm using 4.11.5-1)
Comment 28 Franck Bui 2017-07-07 15:37:23 UTC
(In reply to Andrei Borzenkov from comment #6)
> 2. Now lets look at keyrings immediately after logon
> 
> bor@10:~> : Before ecryptfs-setup
> bor@10:~> cat /proc/keys
> 023c3b10 I--Q---     1 perm 0b0b0000     0     0 user      invocation_id: 16
> 05088f05 I--Q---    88 perm 3f030000     0     0 keyring   _ses: 1
> 2344184f I--Q---    41 perm 3f030000  1000   100 keyring   _ses: 1
> bor@10:~> keyctl show -x @s
> Keyring
> 0x05088f05 --alswrv      0     0  keyring: _ses
> 0x023c3b10 ----s-rv      0     0   \_ user: invocation_id
> 
> Note - our session keyring is owned by user 0!!! So it is the one inherited
> from systemd service. (Heck, is there any way to list session keyrings for
> each process?)

I don't see why the session keyring is owned by root here. The ownership is supposed to be changed here:

  https://github.com/systemd/systemd/blob/master/src/core/execute.c#L2127

It looks like for some reasons KEYCTL_CHOWN doesn't work...
Comment 29 Franck Bui 2017-07-07 15:42:49 UTC
(In reply to Andrei Borzenkov from comment #25)
> Note that now upstream links user keyring to service session keyring thus
> exposing the exact key leak issue discussed here ...
> 
> https://github.com/systemd/systemd/commit/
> 437a85112e02042b62751395b9e7225628c1b708
> 
> and proposed
> 
> https://github.com/systemd/systemd/pull/6286

Those 2 commits look totally broken actually and the first one defeat the whole point of having 74dd6b515fa968c57 commit whose aim is to have a dedicated session keyring for each system services which is *not* linked with the user keyring.

It looks like that only pam_keyinit.so was missing from the PAM config.
Comment 30 Andrei Borzenkov 2017-07-07 16:44:35 UTC
(In reply to Franck Bui from comment #27)
> 
> I've run the same test and the 2 users get a differ session keyring...
> 
> How did you log in BTW ? Through different ttys ?
> 

I mentioned several times that you need to "enter system" via the same top-level service. In this case sshd. Below 22222 is TW VM port:

bor@bor-Latitude-E5450:~$ ssh localhost -p 22222 -l bor
Password: 
Last login: Fri Jul  7 19:36:49 2017 from console
Have a lot of fun...
bor@tw:~> keyctl add user foo "I am bor" @s
866499585
bor@tw:~> keyctl print 866499585
I am bor
bor@tw:~> keyctl show -x
Session Keyring
0x1356a15e --alswrv      0     0  keyring: _ses
0x3b003997 ----s-rv      0     0   \_ user: invocation_id
0x33a5bc01 --alswrv   1000   100   \_ user: foo
bor@tw:~> id -a
uid=1000(bor) gid=100(users) groups=100(users)
bor@tw:~> cat /proc/keys
1356a15e I--Q---    17 perm 3f030000     0     0 keyring   _ses: 2
33a5bc01 I--Q---     1 perm 3f010000  1000   100 user      foo: 8
3851ed26 I--Q---    41 perm 3f030000  1000   100 keyring   _ses: 1
3b003997 I--Q---     1 perm 0b0b0000     0     0 user      invocation_id: 16
bor@tw:~> 
 


bor@bor-Latitude-E5450:~$ ssh localhost -p 22222 -l test
Password: 
Last login: Fri Jul  7 19:28:08 2017 from ::1
Have a lot of fun...
test@tw:~> keyctl print 866499585
I am bor
test@tw:~> keyctl show -x
Session Keyring
0x1356a15e --alswrv      0     0  keyring: _ses
0x3b003997 ----s-rv      0     0   \_ user: invocation_id
0x33a5bc01 --alswrv   1000   100   \_ user: foo
test@tw:~> id -a
uid=1001(test) gid=100(users) groups=100(users)
test@tw:~> cat /proc/keys 
1356a15e I--Q---    24 perm 3f030000     0     0 keyring   _ses: 2
2a571e17 I--Q---     4 perm 3f030000  1001   100 keyring   _ses: 1
33a5bc01 I--Q---     1 perm 3f010000  1000   100 user      foo: 8
3b003997 I--Q---     1 perm 0b0b0000     0     0 user      invocation_id: 16
test@tw:~> 


I do not know where additional _ses keys come from (3851ed26 and 2a571e17).
Comment 31 Andrei Borzenkov 2017-07-07 16:47:45 UTC
And, BTW, those keys now PERSIST through logout!

bor@tw:~> logout
Connection to localhost closed.
bor@bor-Latitude-E5450:~$ 


test@tw:~> logout
Connection to localhost closed.
bor@bor-Latitude-E5450:~$ ssh localhost -p 22222 -l test
Password: 
Last login: Fri Jul  7 19:40:51 2017 from ::1
Have a lot of fun...
test@tw:~> keyctl show -x
Session Keyring
0x1356a15e --alswrv      0     0  keyring: _ses
0x3b003997 ----s-rv      0     0   \_ user: invocation_id
0x33a5bc01 --alswrv   1000   100   \_ user: foo
test@tw:~> keyctl print 866499585
I am bor
test@tw:~> 

EVERY user which connected via ssh will now have access to the same keys.
Comment 32 Andrei Borzenkov 2017-07-08 06:01:39 UTC
(In reply to Andrei Borzenkov from comment #30)
> In this case sshd. 

(In reply to Andrei Borzenkov from comment #31)
> And, BTW, those keys now PERSIST through logout!
> 


Same for logging in through display manager (at least, observed with lightdm here). Multiple logins via user switch "share" the same session keyring and it persists after logout so any user logged in will see all past keys.
Comment 33 Franck Bui 2017-07-10 13:41:33 UTC
To sum up my current understanding: the kernel keyring stuff is currently not integrated in the PAM config used by (open)SUSE (the user session keyring is used as the session keyring).

systemd, with 74dd6b515fa968c5710b39 commit, doesn't make any distinction between service when it creates a session keyring (which is not linked to the user keyring) for each service. Therefore services related to user login (sshd, DM, ...) will be started with a wrong session keyring and this one will be used and shared by all their process children.

For now a change which consists in disabling 74dd6b515fa968c5710b39 commit has been submitted.

But it would be better if we could integrate pam_keyinit.so in the PAM setup so we can restore the reverted feature.

Does that sound correct ?
Comment 34 Andrei Borzenkov 2017-07-10 14:15:55 UTC
(In reply to Franck Bui from comment #33)
> 
> Does that sound correct ?

Yes, except ...

> But it would be better if we could integrate pam_keyinit.so.

I hesitate to say "better". One can argue that it worked so far without any explicit integration and justification for such serious change in systemd is rather weak (are you aware of any service that actually makes use of invocation ID?). Alternative is to explicitly pass invocation IDs on those services that really need it.
Comment 35 Franck Bui 2017-07-10 15:04:32 UTC
(In reply to Andrei Borzenkov from comment #34)
> I hesitate to say "better".

Well according to the documentation, the user session keyring is used as fallback when no session keyring has been created for a given process. And this happens when the keyring stuff has not been integrated.

And in this case session key are visible by all process running with the same UID, which is not too good.

That's probably the reason why the doc says:

  Rather  than  relying  on the user session keyring, it is strongly
  recommended —especially if the process is running  as  root— that  a 
  session-keyring(7)  be  set  explicitly,  for  example  by pam_keyinit(8).

> One can argue that it worked so far without any
> explicit integration and justification for such serious change in systemd is
> rather weak (are you aware of any service that actually makes use of
> invocation ID?). Alternative is to explicitly pass invocation IDs on those
> services that really need it.

The main justification, from my POV, is rather that all system services get a dedicated session keyring which is disconnected from the root user keyring.
Comment 36 Andrei Borzenkov 2017-07-10 18:20:35 UTC
(In reply to Franck Bui from comment #35)
> 
> And in this case session key are visible by all process running with the
> same UID, which is not too good.
> 

Still it is better than what we have now.

> That's probably the reason why the doc says:
> 
>   Rather  than  relying  on the user session keyring, it is strongly
>   recommended —especially if the process is running  as  root— that  a 
>   session-keyring(7)  be  set  explicitly,  for  example  by pam_keyinit(8).
> 

You miss the point. It makes pam_keyinit mandatory without as much as giving any heads up to users (just try to search for pam_keyinit in systemd NEWS). Before this change pam_keyinit was recommended, but the whole system still worked reasonably well without it. So the actual question is whether we want to mandate pam_keyinit and risk security implications if it is missing for some reasons.
Comment 37 Franck Bui 2017-07-10 19:05:19 UTC
(In reply to Andrei Borzenkov from comment #36)
> (In reply to Franck Bui from comment #35)
> > 
> > And in this case session key are visible by all process running with the
> > same UID, which is not too good.
> > 
> 
> Still it is better than what we have now.
> 

Nobody is saying the contrary and as already said it will be temporarily reverted until we will find a better solution.

But something better than something broken doesn't necessarily mean that it's something good to keep... 

> You miss the point. It makes pam_keyinit mandatory without as much as giving
> any heads up to users (just try to search for pam_keyinit in systemd NEWS).
> Before this change pam_keyinit was recommended, but the whole system still
> worked reasonably well without it.

It's not working reasonably well see my previous comment.

pam_keyinit is not recommended but *strongly* recommended.

What do you think the emphasis implies ?

> So the actual question is whether we want
> to mandate pam_keyinit and risk security implications if it is missing for
> some reasons.

That's what this bug is all about now I guess: integrate pam_keyinit in the PAM config so the kernel keyring stuff works as it should and the risk is keep as low as possible.

This way we can improve the old setup and may reconsider restoring the keyring feature in systemd.
Comment 38 Franck Bui 2017-07-31 09:17:25 UTC
Josef, is there any plan here ?
Comment 39 Josef Möllers 2017-07-31 09:24:43 UTC
(In reply to Franck Bui from comment #38)
> Josef, is there any plan here ?

Yes.

I'm currently working on integrating pam_keyinit into pam-config:
* It will be added to individual PAM-scripts only, not to any common-* scripts!
* Each invocation will have "revoke" as an argument
* Each service may add "force" at it's discretion.

Then, an rpmlint-skript will have to be written to indicate to package maintainers that they need to add a call to "pam-config" to their postinstall-script.
Comment 40 Franck Bui 2017-07-31 09:43:54 UTC
Thanks for the update Josef. Could you let us know when your work will be finished so we could give it a try ?
Comment 41 Josef Möllers 2017-07-31 09:45:51 UTC
(In reply to Franck Bui from comment #40)
> Thanks for the update Josef. Could you let us know when your work will be
> finished so we could give it a try ?

Yes, definitely. I'll post a message here.
Comment 42 Josef Möllers 2017-08-01 15:24:25 UTC
Created attachment 734792 [details]
pam-config package with pam_keyinit support

This is a pam-config with pam_keyinit support.
run "pam-config --service <servicename> -a --keyinit" to include pam_keyinit in <servicename>'s config file.

* "revoke" is automatically appended, I cannot think of a scenario where this is not wanted.
* You need to *also* call "pam-config --service <servicename> -a --keyinit-force" if you want the "force" argument to be appended.
* Only the "session" module type is supported (by pam_keyinit).
* The entry is inserted before any other "session" entry.
Comment 43 Josef Möllers 2017-08-01 15:25:05 UTC
PS The package was built on Leap 42.3.
Comment 44 Franck Bui 2017-08-02 12:21:09 UTC
Hi Josef, thanks for the update. I'll give it a test when I'll be back from my vacation (in 3 weeks).

Cheers.
Comment 45 Josef Möllers 2017-09-19 14:11:53 UTC
Created attachment 741103 [details]
Patch to add configuration of pam_keyinit to pam-config

This patch adds configuration of pam_keyinit to pam-config.
* "revoke" is always appendes
* "force" and "debug" can be configured.
Comment 46 Josef Möllers 2017-09-19 14:14:17 UTC
Thorsten, can you please check if this patch is OK and take it?

As this is security-related, it might be better if more then just my two old eyes look over it.
Comment 47 Thorsten Kukuk 2018-01-15 16:03:21 UTC
Commited to git.
Comment 48 Swamp Workflow Management 2018-01-15 16:40:08 UTC
This is an autogenerated message for OBS integration:
This bug (1045886) was mentioned in
https://build.opensuse.org/request/show/565816 Factory / pam-config
Comment 52 Franck Bui 2018-02-21 09:23:52 UTC
(In reply to Swamp Workflow Management from comment #48)
> This is an autogenerated message for OBS integration:
> This bug (1045886) was mentioned in
> https://build.opensuse.org/request/show/565816 Factory / pam-config

I was reconsidering to re-enable the service keyring support in systemd while upgrading it to the latest version but it seems that pam_keyinit is still not integrated in the PAM stack.

I'm not sure why the above request of pam-config mentioned this bug as it does not fix this bug.

In my understanding it was fixed by the disabling of the keyring stuff in systemd, but IMHO it should be a temporary workaround until the pam_keyinit is really included in the PAM stack.

To track the integration of pam_keyinit, I opened bug #1081947.

Hope that makes sense
Comment 53 Franck Bui 2022-04-19 11:42:59 UTC
(In reply to Franck Bui from comment #52)
> In my understanding it was fixed by the disabling of the keyring stuff in
> systemd, but IMHO it should be a temporary workaround until the pam_keyinit
> is really included in the PAM stack.
> 
> To track the integration of pam_keyinit, I opened bug #1081947.

FYI I dropped the temporary workaround as bsc#1081947 has been addressed. My basic testing seems to indicate that the initial problem with ecryptfs+sshd (or login) is no more present and now user keyring is linked from session keyring.

Hence next update of systemd will set up a private keyring for each system service.

Please open a new bug report if you encounter a bug related to this change.