mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: torvalds@linux-foundation.org
Cc: dhowells@redhat.com, ghudson@mit.edu, simo@redhat.com,
	sgallagh@redhat.com, keys@linux-nfs.org,
	linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org
Subject: RFC: KEYS: Is this too-big a behavioural change for a system call?
Date: Thu, 30 Jan 2014 17:14:00 +0000	[thread overview]
Message-ID: <4268.1391102040@warthog.procyon.org.uk> (raw)


[Sent again, this time with Linus's address correct]

Hi Linus,

I've been asked by Kerberos developers to slightly change the behaviour of the
add_key() and request_key() system calls and a couple of the keyctl() functions
- and I'm wondering if you'd be okay with it.

The current behaviour can be illustrated thusly:

 (*) The add_key() syscall, for example, takes a destination keyring into which
     the newly created key will be placed.

 (*) There's a special value that can be passed as the destination keyring ID
     to indicate a process's session keyring without actually needing to know
     the ID of that keyring.

 (*) A process can have no session keyring.  If it didn't inherit one from its
     parent or if it didn't explicitly or implicitly create one, then it won't
     have one.

 (*) A process can have its session keyring pointer pointing to the default
     user-session keyring for its owner UID.

     [! A process can also have some other session keyring, but that's
     	irrelevant to this problem]

 (*) When the kernel looks up a key ID to turn it into a keyring, it calls
     lookup_user_key() and can pass a flag (KEY_LOOKUP_CREATE) to request that
     the keyring be created if it doesn't exist yet.

     Typically, KEY_LOOKUP_CREATE is set if we're going to modify the keyring
     in some way (eg. it's the destination for add_key()).  It's also possible
     for userspace to explicitly set this with:

	keyctl(KEYCTL_GET_KEYRING_ID, <id>, 1).

 (*) If a process has no session keyring or is using the user-session _and_ it
     makes a system call that requests use of the session keyring then one of
     two things happens:

     (a) If KEY_LOOKUP_CREATE set then an empty keyring will be created and
     	 assigned as this process's session keyring.

	 This will forcibly displace the user-session keyring from the session
	 pointer, even if the user explicitly joined that keyring as their
	 session.

     (b) If KEY_LOOKUP_CREATE was not set then the user-session keyring will be
     	 installed as the session keyring and then will be used as the session
     	 keyring until (a) applies.

 (*) If a process with no session keyring creates a key and attaches it to its
     session keyring, a session keyring will be created, the key will be added
     to it - and then the keyring and the key added to it will be deleted when
     the process exits.


The problem the Kerberos developers have is that they would like libkrb5 to
fall back to using the session keyring if keyctl(KEYCTL_GET_PERSISTENT) fails
with EOPNOTSUPP (say if CONFIG_PERSISTENT_KEYRINGS=n).  However, this means
that if you don't have a session keyring, kinit has one forcibly created for it
by the kernel and then your credentials cache is deleted when kinit exits.

If you have pam_keyinit properly set in your PAM configuration then this isn't
a problem for processes derived from a login shell of some sort (ssh, login, X,
etc.).

However, processes that aren't started from a PAM aware process - such as the
Kerberos developers' testfarm - don't get a session keyring unless they
explicitly create one.


Now, there are several solutions:

 (1) Don't use the session keyring as a fallback in libkrb5.

 (2) Explicitly use the user-session keyring as a fall back in libkrb5 instead
     of the session keyring.

 (3) Manually create a session keyring somewhere before it is needed.  The
     keyutils testsuite does this by running its tests from "keyctl session"

 (4) Don't implicitly create a new anonymous keyring, but always set the
     user-session keyring as a process's session keyring if the latter is
     unset.

 (5) Don't implicitly create a new anonymous keyring and don't implicitly set
     the session keyring to the user-session keyring, but rather just fall back
     to using the user-session keyring if there isn't a session keyring.

 (6) Don't implicitly create a new anonymous keyring and never use the
     user-session keyring instead, but rather reject requests with ENOKEY.

The first three don't require kernel changes.

In (5) and (6) a session keyring should still be created if userspace
explicitly asks for one with KEYCTL_GET_KEYRING_ID.


I think the best thing course would be (3).  I have reservations about using
the user and user-session keyrings:

 (*) They depend on what UID your process currently is and are thus subject to
     setuid() and SUID-exec.

 (*) I'm not keen on system daemons sharing automatically keys by the user and
     user-session keyrings - and sharing them with other root processes.

 (*) How do these interact with the SELinux?

 (*) Do you want the output of test programs dumping where everyone else can
     make use of it?


That said, I do think that the Kerberos people have a valid point.  The current
behaviour is poor.  I'm inclined to implement (5) or (6), probably (5).

This won't make any difference to most processes, ie.:

 (*) Those run from pam_keyinit-managed login shells.

 (*) Those that don't make use of libkrb5 or keyrings.


In many ways, I'd like to just get rid of the user and user-session keyrings
from the kernel entirely and have them created and maintained by pam_keyinit.
The special keyring IDs:

	KEY_SPEC_USER_KEYRING
	KEY_SPEC_USER_SESSION_KEYRING

and:

	KEY_REQKEY_DEFL_USER_KEYRING
	KEY_REQKEY_DEFL_USER_SESSION_KEYRING

would then search your session keyring for keyrings called "_uid" and
"_uid_ses" and return those.  Unfortunately, I think this is probably a
much-too-big change at this point.

Any thoughts?

Thanks,
David


             reply	other threads:[~2014-01-30 17:14 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-30 17:14 David Howells [this message]
2014-01-31  1:07 ` James Morris
  -- strict thread matches above, loose matches on Subject: below --
2014-01-30 17:03 David Howells
2014-01-31  2:37 ` Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4268.1391102040@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=ghudson@mit.edu \
    --cc=keys@linux-nfs.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=sgallagh@redhat.com \
    --cc=simo@redhat.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®