KEYS: If install_session_keyring() is given a keyring, it should install it
David Howells [Mon, 22 Aug 2011 13:08:33 +0000 (14:08 +0100)]
If install_session_keyring() is given a keyring, it should install it rather
than just creating a new one anyway.  This was accidentally broken in:

commit d84f4f992cbd76e8f39c488cf0c5d123843923b1
Author: David Howells <dhowells@redhat.com>
Date:   Fri Nov 14 10:39:23 2008 +1100
Subject: CRED: Inaugurate COW credentials

The impact of that commit is that pam_keyinit no longer works correctly if
'force' isn't specified against a login process. This is because:

keyctl_get_keyring_ID(KEY_SPEC_SESSION_KEYRING, 0)

now always creates a new session keyring and thus the check whether the session
keyring and the user-session keyring are the same is always false.  This leads
pam_keyinit to conclude that a session keyring is installed and it shouldn't be
revoked by pam_keyinit here if 'revoke' is specified.

Any system that specifies 'force' against pam_keyinit in the PAM configuration
files for login methods (login, ssh, su -l, kdm, etc.) is not affected since
that bypasses the broken check and forces the creation of a new session keyring
anyway (for which the revoke flag is not cleared) - and any subsequent call to
pam_keyinit really does have a session keyring already installed, and so the
check works correctly there.

Reverting to the previous behaviour will cause the kernel to subscribe the
process to the user-session keyring as its session keyring if it doesn't have a
session keyring of its own.  pam_keyinit will detect this and install a new
session keyring anyway (and won't clear the revert flag).

This can be tested by commenting out pam_keyinit in the /etc/pam.d files and
running the following program a couple of times in a row:

#include <stdio.h>
#include <stdlib.h>
#include <keyutils.h>
int main(int argc, char *argv[])
{
key_serial_t uk, usk, sk;
uk = keyctl_get_keyring_ID(KEY_SPEC_USER_KEYRING, 0);
usk = keyctl_get_keyring_ID(KEY_SPEC_USER_SESSION_KEYRING, 0);
sk = keyctl_get_keyring_ID(KEY_SPEC_SESSION_KEYRING, 0);
printf("keys: %08x %08x %08x\n", uk, usk, sk);
return 0;
}

Without the patch, I see:

keys: 3884e281 24c4dfcf 22825f8e
keys: 3884e281 24c4dfcf 068772be

With the patch, I see:

keys: 26be9c83 0e755ce0 0e755ce0
keys: 26be9c83 0e755ce0 0e755ce0

As can be seen, with the patch, the session keyring is the same as the
user-session keyring each time; without the patch a new session keyring is
generated each time.

Reported-by: Greg Wettstein <greg@enjellic.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: Greg Wettstein <greg@enjellic.com>
Signed-off-by: James Morris <jmorris@namei.org>

security/keys/process_keys.c

index a3063eb..3bc6071 100644 (file)
@@ -270,7 +270,7 @@ static int install_session_keyring(struct key *keyring)
        if (!new)
                return -ENOMEM;
 
-       ret = install_session_keyring_to_cred(new, NULL);
+       ret = install_session_keyring_to_cred(new, keyring);
        if (ret < 0) {
                abort_creds(new);
                return ret;