From: "Serge E. Hallyn" <serue@us.ibm.com>
To: David Howells <dhowells@redhat.com>
Cc: torvalds@osdl.org, akpm@linux-foundation.org, jmorris@namei.org,
linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org
Subject: Re: [PATCH 1/6] KEYS: Deal with dead-type keys appropriately
Date: Tue, 4 Aug 2009 13:16:02 -0500 [thread overview]
Message-ID: <20090804181602.GA8442@us.ibm.com> (raw)
In-Reply-To: <20090804145530.17676.24656.stgit@warthog.procyon.org.uk>
Quoting David Howells (dhowells@redhat.com):
> Allow keys for which the key type has been removed to be unlinked. Currently
> dead-type keys can only be disposed of by completely clearing the keyrings
> that point to them.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
This also makes for a nice readability improvement. Would be even nicer
to add a
#define KEY_LOOKUP_FLAGS_NONE 0
and pass that in instead of 0 at the callers doing so.
Acked-by: Serge Hallyn <serue@us.ibm.com>
thanks,
-serge
> ---
>
> security/keys/internal.h | 5 +++-
> security/keys/key.c | 6 ++---
> security/keys/keyctl.c | 50 ++++++++++++++++++++++++------------------
> security/keys/process_keys.c | 18 +++++++++++----
> 4 files changed, 48 insertions(+), 31 deletions(-)
>
>
> diff --git a/security/keys/internal.h b/security/keys/internal.h
> index 9fb679c..a7252e7 100644
> --- a/security/keys/internal.h
> +++ b/security/keys/internal.h
> @@ -124,8 +124,11 @@ extern struct key *request_key_and_link(struct key_type *type,
> struct key *dest_keyring,
> unsigned long flags);
>
> -extern key_ref_t lookup_user_key(key_serial_t id, int create, int partial,
> +extern key_ref_t lookup_user_key(key_serial_t id, unsigned long flags,
> key_perm_t perm);
> +#define KEY_LOOKUP_CREATE 0x01
> +#define KEY_LOOKUP_PARTIAL 0x02
> +#define KEY_LOOKUP_FOR_UNLINK 0x04
>
> extern long join_session_keyring(const char *name);
>
> diff --git a/security/keys/key.c b/security/keys/key.c
> index 4a1297d..3762d5b 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -642,10 +642,8 @@ struct key *key_lookup(key_serial_t id)
> goto error;
>
> found:
> - /* pretend it doesn't exist if it's dead */
> - if (atomic_read(&key->usage) == 0 ||
> - test_bit(KEY_FLAG_DEAD, &key->flags) ||
> - key->type == &key_type_dead)
> + /* pretend it doesn't exist if it is awaiting deletion */
> + if (atomic_read(&key->usage) == 0)
> goto not_found;
>
> /* this races with key_put(), but that doesn't matter since key_put()
> diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
> index 7f09fb8..b85ace2 100644
> --- a/security/keys/keyctl.c
> +++ b/security/keys/keyctl.c
> @@ -103,7 +103,7 @@ SYSCALL_DEFINE5(add_key, const char __user *, _type,
> }
>
> /* find the target keyring (which must be writable) */
> - keyring_ref = lookup_user_key(ringid, 1, 0, KEY_WRITE);
> + keyring_ref = lookup_user_key(ringid, KEY_LOOKUP_CREATE, KEY_WRITE);
> if (IS_ERR(keyring_ref)) {
> ret = PTR_ERR(keyring_ref);
> goto error3;
> @@ -185,7 +185,8 @@ SYSCALL_DEFINE4(request_key, const char __user *, _type,
> /* get the destination keyring if specified */
> dest_ref = NULL;
> if (destringid) {
> - dest_ref = lookup_user_key(destringid, 1, 0, KEY_WRITE);
> + dest_ref = lookup_user_key(destringid, KEY_LOOKUP_CREATE,
> + KEY_WRITE);
> if (IS_ERR(dest_ref)) {
> ret = PTR_ERR(dest_ref);
> goto error3;
> @@ -233,9 +234,11 @@ SYSCALL_DEFINE4(request_key, const char __user *, _type,
> long keyctl_get_keyring_ID(key_serial_t id, int create)
> {
> key_ref_t key_ref;
> + unsigned long lflags;
> long ret;
>
> - key_ref = lookup_user_key(id, create, 0, KEY_SEARCH);
> + lflags = create ? KEY_LOOKUP_CREATE : 0;
> + key_ref = lookup_user_key(id, lflags, KEY_SEARCH);
> if (IS_ERR(key_ref)) {
> ret = PTR_ERR(key_ref);
> goto error;
> @@ -309,7 +312,7 @@ long keyctl_update_key(key_serial_t id,
> }
>
> /* find the target key (which must be writable) */
> - key_ref = lookup_user_key(id, 0, 0, KEY_WRITE);
> + key_ref = lookup_user_key(id, 0, KEY_WRITE);
> if (IS_ERR(key_ref)) {
> ret = PTR_ERR(key_ref);
> goto error2;
> @@ -337,7 +340,7 @@ long keyctl_revoke_key(key_serial_t id)
> key_ref_t key_ref;
> long ret;
>
> - key_ref = lookup_user_key(id, 0, 0, KEY_WRITE);
> + key_ref = lookup_user_key(id, 0, KEY_WRITE);
> if (IS_ERR(key_ref)) {
> ret = PTR_ERR(key_ref);
> goto error;
> @@ -363,7 +366,7 @@ long keyctl_keyring_clear(key_serial_t ringid)
> key_ref_t keyring_ref;
> long ret;
>
> - keyring_ref = lookup_user_key(ringid, 1, 0, KEY_WRITE);
> + keyring_ref = lookup_user_key(ringid, KEY_LOOKUP_CREATE, KEY_WRITE);
> if (IS_ERR(keyring_ref)) {
> ret = PTR_ERR(keyring_ref);
> goto error;
> @@ -389,13 +392,13 @@ long keyctl_keyring_link(key_serial_t id, key_serial_t ringid)
> key_ref_t keyring_ref, key_ref;
> long ret;
>
> - keyring_ref = lookup_user_key(ringid, 1, 0, KEY_WRITE);
> + keyring_ref = lookup_user_key(ringid, KEY_LOOKUP_CREATE, KEY_WRITE);
> if (IS_ERR(keyring_ref)) {
> ret = PTR_ERR(keyring_ref);
> goto error;
> }
>
> - key_ref = lookup_user_key(id, 1, 0, KEY_LINK);
> + key_ref = lookup_user_key(id, KEY_LOOKUP_CREATE, KEY_LINK);
> if (IS_ERR(key_ref)) {
> ret = PTR_ERR(key_ref);
> goto error2;
> @@ -423,13 +426,13 @@ long keyctl_keyring_unlink(key_serial_t id, key_serial_t ringid)
> key_ref_t keyring_ref, key_ref;
> long ret;
>
> - keyring_ref = lookup_user_key(ringid, 0, 0, KEY_WRITE);
> + keyring_ref = lookup_user_key(ringid, 0, KEY_WRITE);
> if (IS_ERR(keyring_ref)) {
> ret = PTR_ERR(keyring_ref);
> goto error;
> }
>
> - key_ref = lookup_user_key(id, 0, 0, 0);
> + key_ref = lookup_user_key(id, KEY_LOOKUP_FOR_UNLINK, 0);
> if (IS_ERR(key_ref)) {
> ret = PTR_ERR(key_ref);
> goto error2;
> @@ -465,7 +468,7 @@ long keyctl_describe_key(key_serial_t keyid,
> char *tmpbuf;
> long ret;
>
> - key_ref = lookup_user_key(keyid, 0, 1, KEY_VIEW);
> + key_ref = lookup_user_key(keyid, KEY_LOOKUP_PARTIAL, KEY_VIEW);
> if (IS_ERR(key_ref)) {
> /* viewing a key under construction is permitted if we have the
> * authorisation token handy */
> @@ -474,7 +477,8 @@ long keyctl_describe_key(key_serial_t keyid,
> if (!IS_ERR(instkey)) {
> key_put(instkey);
> key_ref = lookup_user_key(keyid,
> - 0, 1, 0);
> + KEY_LOOKUP_PARTIAL,
> + 0);
> if (!IS_ERR(key_ref))
> goto okay;
> }
> @@ -558,7 +562,7 @@ long keyctl_keyring_search(key_serial_t ringid,
> }
>
> /* get the keyring at which to begin the search */
> - keyring_ref = lookup_user_key(ringid, 0, 0, KEY_SEARCH);
> + keyring_ref = lookup_user_key(ringid, 0, KEY_SEARCH);
> if (IS_ERR(keyring_ref)) {
> ret = PTR_ERR(keyring_ref);
> goto error2;
> @@ -567,7 +571,8 @@ long keyctl_keyring_search(key_serial_t ringid,
> /* get the destination keyring if specified */
> dest_ref = NULL;
> if (destringid) {
> - dest_ref = lookup_user_key(destringid, 1, 0, KEY_WRITE);
> + dest_ref = lookup_user_key(destringid, KEY_LOOKUP_CREATE,
> + KEY_WRITE);
> if (IS_ERR(dest_ref)) {
> ret = PTR_ERR(dest_ref);
> goto error3;
> @@ -637,7 +642,7 @@ long keyctl_read_key(key_serial_t keyid, char __user *buffer, size_t buflen)
> long ret;
>
> /* find the key first */
> - key_ref = lookup_user_key(keyid, 0, 0, 0);
> + key_ref = lookup_user_key(keyid, 0, 0);
> if (IS_ERR(key_ref)) {
> ret = -ENOKEY;
> goto error;
> @@ -700,7 +705,8 @@ long keyctl_chown_key(key_serial_t id, uid_t uid, gid_t gid)
> if (uid == (uid_t) -1 && gid == (gid_t) -1)
> goto error;
>
> - key_ref = lookup_user_key(id, 1, 1, KEY_SETATTR);
> + key_ref = lookup_user_key(id, KEY_LOOKUP_CREATE | KEY_LOOKUP_PARTIAL,
> + KEY_SETATTR);
> if (IS_ERR(key_ref)) {
> ret = PTR_ERR(key_ref);
> goto error;
> @@ -805,7 +811,8 @@ long keyctl_setperm_key(key_serial_t id, key_perm_t perm)
> if (perm & ~(KEY_POS_ALL | KEY_USR_ALL | KEY_GRP_ALL | KEY_OTH_ALL))
> goto error;
>
> - key_ref = lookup_user_key(id, 1, 1, KEY_SETATTR);
> + key_ref = lookup_user_key(id, KEY_LOOKUP_CREATE | KEY_LOOKUP_PARTIAL,
> + KEY_SETATTR);
> if (IS_ERR(key_ref)) {
> ret = PTR_ERR(key_ref);
> goto error;
> @@ -847,7 +854,7 @@ static long get_instantiation_keyring(key_serial_t ringid,
>
> /* if a specific keyring is nominated by ID, then use that */
> if (ringid > 0) {
> - dkref = lookup_user_key(ringid, 1, 0, KEY_WRITE);
> + dkref = lookup_user_key(ringid, KEY_LOOKUP_CREATE, KEY_WRITE);
> if (IS_ERR(dkref))
> return PTR_ERR(dkref);
> *_dest_keyring = key_ref_to_ptr(dkref);
> @@ -1083,7 +1090,8 @@ long keyctl_set_timeout(key_serial_t id, unsigned timeout)
> time_t expiry;
> long ret;
>
> - key_ref = lookup_user_key(id, 1, 1, KEY_SETATTR);
> + key_ref = lookup_user_key(id, KEY_LOOKUP_CREATE | KEY_LOOKUP_PARTIAL,
> + KEY_SETATTR);
> if (IS_ERR(key_ref)) {
> ret = PTR_ERR(key_ref);
> goto error;
> @@ -1170,7 +1178,7 @@ long keyctl_get_security(key_serial_t keyid,
> char *context;
> long ret;
>
> - key_ref = lookup_user_key(keyid, 0, 1, KEY_VIEW);
> + key_ref = lookup_user_key(keyid, KEY_LOOKUP_PARTIAL, KEY_VIEW);
> if (IS_ERR(key_ref)) {
> if (PTR_ERR(key_ref) != -EACCES)
> return PTR_ERR(key_ref);
> @@ -1182,7 +1190,7 @@ long keyctl_get_security(key_serial_t keyid,
> return PTR_ERR(key_ref);
> key_put(instkey);
>
> - key_ref = lookup_user_key(keyid, 0, 1, 0);
> + key_ref = lookup_user_key(keyid, KEY_LOOKUP_PARTIAL, 0);
> if (IS_ERR(key_ref))
> return PTR_ERR(key_ref);
> }
> diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
> index 276d278..349c315 100644
> --- a/security/keys/process_keys.c
> +++ b/security/keys/process_keys.c
> @@ -487,7 +487,7 @@ static int lookup_user_key_possessed(const struct key *key, const void *target)
> * - don't create special keyrings unless so requested
> * - partially constructed keys aren't found unless requested
> */
> -key_ref_t lookup_user_key(key_serial_t id, int create, int partial,
> +key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
> key_perm_t perm)
> {
> struct request_key_auth *rka;
> @@ -503,7 +503,7 @@ try_again:
> switch (id) {
> case KEY_SPEC_THREAD_KEYRING:
> if (!cred->thread_keyring) {
> - if (!create)
> + if (!(lflags & KEY_LOOKUP_CREATE))
> goto error;
>
> ret = install_thread_keyring();
> @@ -521,7 +521,7 @@ try_again:
>
> case KEY_SPEC_PROCESS_KEYRING:
> if (!cred->tgcred->process_keyring) {
> - if (!create)
> + if (!(lflags & KEY_LOOKUP_CREATE))
> goto error;
>
> ret = install_process_keyring();
> @@ -642,7 +642,14 @@ try_again:
> break;
> }
>
> - if (!partial) {
> + /* unlink does not use the nominated key in any way, so can skip all
> + * the permission checks as it is only concerned with the keyring */
> + if (lflags & KEY_LOOKUP_FOR_UNLINK) {
> + ret = 0;
> + goto error;
> + }
> +
> + if (!(lflags & KEY_LOOKUP_PARTIAL)) {
> ret = wait_for_key_construction(key, true);
> switch (ret) {
> case -ERESTARTSYS:
> @@ -660,7 +667,8 @@ try_again:
> }
>
> ret = -EIO;
> - if (!partial && !test_bit(KEY_FLAG_INSTANTIATED, &key->flags))
> + if (!(lflags & KEY_LOOKUP_PARTIAL) &&
> + !test_bit(KEY_FLAG_INSTANTIATED, &key->flags))
> goto invalid_key;
>
> /* check the permissions */
prev parent reply other threads:[~2009-08-04 18:16 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-08-04 14:55 [PATCH 1/6] KEYS: Deal with dead-type keys appropriately David Howells
2009-08-04 14:55 ` [PATCH 2/6] KEYS: Allow keyctl_revoke() on keys that have SETATTR but not WRITE perm David Howells
2009-08-04 15:17 ` Serge E. Hallyn
2009-08-04 15:38 ` David Howells
2009-08-04 15:43 ` David Howells
2009-08-04 14:55 ` [PATCH 3/6] KEYS: Flag dead keys to induce EKEYREVOKED David Howells
2009-08-04 18:22 ` Serge E. Hallyn
2009-08-04 14:55 ` [PATCH 4/6] KEYS: Add garbage collection for dead, revoked and expired keys David Howells
2009-08-04 18:43 ` Serge E. Hallyn
2009-08-04 20:30 ` David Howells
2009-08-04 21:01 ` Serge E. Hallyn
2009-08-04 22:00 ` David Howells
2009-08-04 22:33 ` Serge E. Hallyn
2009-08-04 14:55 ` [PATCH 5/6] KEYS: Make /proc/keys use keyid not numread as file position David Howells
2009-08-04 14:55 ` [PATCH 6/6] KEYS: Do some whitespace cleanups David Howells
2009-08-04 18:46 ` Serge E. Hallyn
2009-08-04 18:16 ` Serge E. Hallyn [this message]
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=20090804181602.GA8442@us.ibm.com \
--to=serue@us.ibm.com \
--cc=akpm@linux-foundation.org \
--cc=dhowells@redhat.com \
--cc=jmorris@namei.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=torvalds@osdl.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