From: David Howells <dhowells@redhat.com>
To: viro@ftp.linux.org.uk, hch@infradead.org,
Trond.Myklebust@netapp.com, sds@tycho.nsa.gov,
casey@schaufler-ca.com
Cc: linux-kernel@vger.kernel.org, selinux@tycho.nsa.gov,
linux-security-module@vger.kernel.org, dhowells@redhat.com
Subject: [PATCH 2/7] KEYS: Check starting keyring as part of search
Date: Wed, 05 Dec 2007 19:37:12 +0000 [thread overview]
Message-ID: <20071205193712.24437.15984.stgit@warthog.procyon.org.uk> (raw)
In-Reply-To: <20071205193702.24437.76418.stgit@warthog.procyon.org.uk>
Check the starting keyring as part of the search to (a) see if that is what
we're searching for, and (b) to check it is still valid for searching.
The scenario: User in process A does things that cause things to be
created in its process session keyring. The user then does an su to
another user and starts a new process, B. The two processes now
share the same process session keyring.
Process B does an NFS access which results in an upcall to gssd.
When gssd attempts to instantiate the context key (to be linked
into the process session keyring), it is denied access even though it
has an authorization key.
The order of calls is:
keyctl_instantiate_key()
lookup_user_key() (the default: case)
search_process_keyrings(current)
search_process_keyrings(rka->context) (recursive call)
keyring_search_aux()
keyring_search_aux() verifies the keys and keyrings underneath the
top-level keyring it is given, but that top-level keyring is neither
fully validated nor checked to see if it is the thing being searched for.
This patch changes keyring_search_aux() to:
1) do more validation on the top keyring it is given and
2) check whether that top-level keyring is the thing being searched for
Signed-off-by: Kevin Coffman <kwc@citi.umich.edu>
Signed-off-by: David Howells <dhowells@redhat.com>
---
security/keys/keyring.c | 35 +++++++++++++++++++++++++++++++----
1 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 88292e3..76b89b2 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -292,7 +292,7 @@ key_ref_t keyring_search_aux(key_ref_t keyring_ref,
struct keyring_list *keylist;
struct timespec now;
- unsigned long possessed;
+ unsigned long possessed, kflags;
struct key *keyring, *key;
key_ref_t key_ref;
long err;
@@ -318,6 +318,32 @@ key_ref_t keyring_search_aux(key_ref_t keyring_ref,
now = current_kernel_time();
err = -EAGAIN;
sp = 0;
+
+ /* firstly we should check to see if this top-level keyring is what we
+ * are looking for */
+ key_ref = ERR_PTR(-EAGAIN);
+ kflags = keyring->flags;
+ if (keyring->type == type && match(keyring, description)) {
+ key = keyring;
+
+ /* check it isn't negative and hasn't expired or been
+ * revoked */
+ if (kflags & (1 << KEY_FLAG_REVOKED))
+ goto error_2;
+ if (key->expiry && now.tv_sec >= key->expiry)
+ goto error_2;
+ key_ref = ERR_PTR(-ENOKEY);
+ if (kflags & (1 << KEY_FLAG_NEGATIVE))
+ goto error_2;
+ goto found;
+ }
+
+ /* otherwise, the top keyring must not be revoked, expired, or
+ * negatively instantiated if we are to search it */
+ key_ref = ERR_PTR(-EAGAIN);
+ if (kflags & ((1 << KEY_FLAG_REVOKED) | (1 << KEY_FLAG_NEGATIVE)) ||
+ (keyring->expiry && now.tv_sec >= keyring->expiry))
+ goto error_2;
/* start processing a new keyring */
descend:
@@ -331,13 +357,14 @@ descend:
/* iterate through the keys in this keyring first */
for (kix = 0; kix < keylist->nkeys; kix++) {
key = keylist->keys[kix];
+ kflags = key->flags;
/* ignore keys not of this type */
if (key->type != type)
continue;
/* skip revoked keys and expired keys */
- if (test_bit(KEY_FLAG_REVOKED, &key->flags))
+ if (kflags & (1 << KEY_FLAG_REVOKED))
continue;
if (key->expiry && now.tv_sec >= key->expiry)
@@ -352,8 +379,8 @@ descend:
context, KEY_SEARCH) < 0)
continue;
- /* we set a different error code if we find a negative key */
- if (test_bit(KEY_FLAG_NEGATIVE, &key->flags)) {
+ /* we set a different error code if we pass a negative key */
+ if (kflags & (1 << KEY_FLAG_NEGATIVE)) {
err = -ENOKEY;
continue;
}
next prev parent reply other threads:[~2007-12-05 19:37 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-12-05 19:37 [PATCH 0/7] Permit filesystem local caching David Howells
2007-12-05 19:37 ` [PATCH 1/7] KEYS: Increase the payload size when instantiating a key David Howells
2007-12-05 19:37 ` David Howells [this message]
2007-12-05 19:37 ` [PATCH 3/7] KEYS: Allow the callout data to be passed as a blob rather than a string David Howells
2007-12-05 19:37 ` [PATCH 4/7] KEYS: Add keyctl function to get a security label David Howells
2007-12-05 20:12 ` Casey Schaufler
2007-12-05 20:38 ` Stephen Smalley
2007-12-06 1:28 ` David Howells
2007-12-06 1:40 ` Casey Schaufler
2007-12-05 19:37 ` [PATCH 5/7] Security: Change current->fs[ug]id to current_fs[ug]id() David Howells
2007-12-07 17:44 ` Serge E. Hallyn
2007-12-08 0:53 ` David Howells
2007-12-05 19:37 ` [PATCH 6/7] SECURITY: Separate task security context from task_struct David Howells
2007-12-05 19:37 ` [PATCH 7/7] SECURITY: De-embed task security record from task and use refcounting David Howells
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=20071205193712.24437.15984.stgit@warthog.procyon.org.uk \
--to=dhowells@redhat.com \
--cc=Trond.Myklebust@netapp.com \
--cc=casey@schaufler-ca.com \
--cc=hch@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=sds@tycho.nsa.gov \
--cc=selinux@tycho.nsa.gov \
--cc=viro@ftp.linux.org.uk \
/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®