linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* RFC: KEYS: Is this too-big a behavioural change for a system call?
@ 2014-01-30 17:14 David Howells
  2014-01-31  1:07 ` James Morris
  0 siblings, 1 reply; 4+ messages in thread
From: David Howells @ 2014-01-30 17:14 UTC (permalink / raw)
  To: torvalds
  Cc: dhowells, ghudson, simo, sgallagh, keys, linux-kernel,
	linux-security-module


[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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: RFC: KEYS: Is this too-big a behavioural change for a system call?
  2014-01-30 17:14 RFC: KEYS: Is this too-big a behavioural change for a system call? David Howells
@ 2014-01-31  1:07 ` James Morris
  0 siblings, 0 replies; 4+ messages in thread
From: James Morris @ 2014-01-31  1:07 UTC (permalink / raw)
  To: David Howells
  Cc: torvalds, ghudson, simo, sgallagh, keys, linux-kernel,
	linux-security-module

On Thu, 30 Jan 2014, David Howells wrote:

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

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

So there are existing apps which will see semantic changes?

If so, we can't accept this change.

> 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?

Getting it right is more important than the size of the change.

What about creating a new system call with the desired behavior, and 
deprecating the current one (or at least, making it a wrapper for the new 
call).

-- 
James Morris
<jmorris@namei.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: RFC: KEYS: Is this too-big a behavioural change for a system call?
  2014-01-30 17:03 David Howells
@ 2014-01-31  2:37 ` Linus Torvalds
  0 siblings, 0 replies; 4+ messages in thread
From: Linus Torvalds @ 2014-01-31  2:37 UTC (permalink / raw)
  To: David Howells
  Cc: ghudson, simo, sgallagh, keys, Linux Kernel Mailing List, LSM List

On Thu, Jan 30, 2014 at 9:03 AM, David Howells <dhowells@redhat.com> wrote:
>
> 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.

So the rule about ABI changes has always been: "If somebody notices
it, we revert it".

IOW, it's not so much that you can't make changes, it's that you
cannot make changes that break programs.

And quite frankly, I have no good way to judge. Your (5) _sounds_
fair, but who knows what odd things some distributions do. We'd have
to be very careful, in *particular* we'd need to make it very very
clear to the krb people that if the change scews over any old users,
it gets reverted with extreme prejudice. At that point, maybe they say
"never mind, we might as well just always make sure we have a session
keyring".

And no, we are *not* going to say "we'll just stop doing this in the
kernel and expect pam_keyinit to do it for us".

But James' suggestion to perhaps have a new version of add_key() with
new semantics could work - if it's worth the pain (because we *would*
have to maintain the old interface basically forever, so it would be
more of a "the new system call doesn't really deprecate the old one,
it just has more convenient semantics").

          Linus

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RFC: KEYS: Is this too-big a behavioural change for a system call?
@ 2014-01-30 17:03 David Howells
  2014-01-31  2:37 ` Linus Torvalds
  0 siblings, 1 reply; 4+ messages in thread
From: David Howells @ 2014-01-30 17:03 UTC (permalink / raw)
  To: torvalds
  Cc: dhowells, ghudson, simo, sgallagh, keys, linux-kernel,
	linux-security-module


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-01-31  2:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-30 17:14 RFC: KEYS: Is this too-big a behavioural change for a system call? David Howells
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).