KEYS: Make /proc/keys check to see if a key is possessed before security check

Make /proc/keys check to see if the calling process possesses each key before performing the security check. The possession check can be skipped if the key doesn't have the possessor-view permission bit set. This causes the keys a process possesses to show up in /proc/keys, even if they don't have matching user/group/other view permissions. Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: James Morris <jmorris@namei.org>

KEYS: Make /proc/keys check to see if a key is possessed before security check
Make /proc/keys check to see if the calling process possesses each key before performing the security check. The possession check can be skipped if the key doesn't have the possessor-view permission bit set. This causes the keys a process possesses to show up in /proc/keys, even if they don't have matching user/group/other view permissions. Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: James Morris <jmorris@namei.org>
David Howells · James Morris
1 parent 9156235b34
Showing 3 changed files with 66 additions and 23 deletions Side-by-side Diff
security/keys/internal.h
security/keys/proc.c
security/keys/process_keys.c
@@ -114,6 +114,10 @@
 				    const void *description,
 				    key_match_func_t match);
  
+extern key_ref_t search_my_process_keyrings(struct key_type *type,
+					    const void *description,
+					    key_match_func_t match,
+					    const struct cred *cred);
 extern key_ref_t search_process_keyrings(struct key_type *type,
 					 const void *description,
 					 key_match_func_t match,
@@ -134,6 +138,7 @@
 					struct key *dest_keyring,
 					unsigned long flags);
  
+extern int lookup_user_key_possessed(const struct key *key, const void *target);
 extern key_ref_t lookup_user_key(key_serial_t id, unsigned long flags,
 				 key_perm_t perm);
 #define KEY_LOOKUP_CREATE	0x01
@@ -184,20 +184,36 @@
  
 static int proc_keys_show(struct seq_file *m, void *v)
 {
+	const struct cred *cred = current_cred();
 	struct rb_node *_p = v;
 	struct key *key = rb_entry(_p, struct key, serial_node);
 	struct timespec now;
 	unsigned long timo;
+	key_ref_t key_ref, skey_ref;
 	char xbuf[12];
 	int rc;
  
+	key_ref = make_key_ref(key, 0);
+
+	/* determine if the key is possessed by this process (a test we can
+	 * skip if the key does not indicate the possessor can view it
+	 */
+	if (key->perm & KEY_POS_VIEW) {
+		skey_ref = search_my_process_keyrings(key->type, key,
+						      lookup_user_key_possessed,
+						      cred);
+		if (!IS_ERR(skey_ref)) {
+			key_ref_put(skey_ref);
+			key_ref = make_key_ref(key, 1);
+		}
+	}
+
 	/* check whether the current task is allowed to view the key (assuming
 	 * non-possession)
 	 * - the caller holds a spinlock, and thus the RCU read lock, making our
 	 *   access to __current_cred() safe
 	 */
-	rc = key_task_permission(make_key_ref(key, 0), current_cred(),
-				 KEY_VIEW);
+	rc = key_task_permission(key_ref, cred, KEY_VIEW);
 	if (rc < 0)
 		return 0;
  
@@ -309,22 +309,19 @@
  
 /*****************************************************************************/
 /*
- * search the process keyrings for the first matching key
+ * search only my process keyrings for the first matching key
  * - we use the supplied match function to see if the description (or other
  *   feature of interest) matches
  * - we return -EAGAIN if we didn't find any matching key
  * - we return -ENOKEY if we found only negative matching keys
  */
-key_ref_t search_process_keyrings(struct key_type *type,
-				  const void *description,
-				  key_match_func_t match,
-				  const struct cred *cred)
+key_ref_t search_my_process_keyrings(struct key_type *type,
+				     const void *description,
+				     key_match_func_t match,
+				     const struct cred *cred)
 {
-	struct request_key_auth *rka;
 	key_ref_t key_ref, ret, err;
  
-	might_sleep();
-
 	/* we want to return -EAGAIN or -ENOKEY if any of the keyrings were
 	 * searchable, but we failed to find a key or we found a negative key;
 	 * otherwise we want to return a sample error (probably -EACCES) if
@@ -424,6 +421,36 @@
 		}
 	}
  
+	/* no key - decide on the error we're going to go for */
+	key_ref = ret ? ret : err;
+
+found:
+	return key_ref;
+}
+
+/*****************************************************************************/
+/*
+ * search the process keyrings for the first matching key
+ * - we use the supplied match function to see if the description (or other
+ *   feature of interest) matches
+ * - we return -EAGAIN if we didn't find any matching key
+ * - we return -ENOKEY if we found only negative matching keys
+ */
+key_ref_t search_process_keyrings(struct key_type *type,
+				  const void *description,
+				  key_match_func_t match,
+				  const struct cred *cred)
+{
+	struct request_key_auth *rka;
+	key_ref_t key_ref, ret = ERR_PTR(-EACCES), err;
+
+	might_sleep();
+
+	key_ref = search_my_process_keyrings(type, description, match, cred);
+	if (!IS_ERR(key_ref))
+		goto found;
+	err = key_ref;
+
 	/* if this process has an instantiation authorisation key, then we also
 	 * search the keyrings of the process mentioned there
 	 * - we don't permit access to request_key auth keys via this method
  
@@ -446,24 +473,19 @@
 			if (!IS_ERR(key_ref))
 				goto found;
  
-			switch (PTR_ERR(key_ref)) {
-			case -EAGAIN: /* no key */
-				if (ret)
-					break;
-			case -ENOKEY: /* negative key */
-				ret = key_ref;
-				break;
-			default:
-				err = key_ref;
-				break;
-			}
+			ret = key_ref;
 		} else {
 			up_read(&cred->request_key_auth->sem);
 		}
 	}
  
 	/* no key - decide on the error we're going to go for */
-	key_ref = ret ? ret : err;
+	if (err == ERR_PTR(-ENOKEY) || ret == ERR_PTR(-ENOKEY))
+		key_ref = ERR_PTR(-ENOKEY);
+	else if (err == ERR_PTR(-EACCES))
+		key_ref = ret;
+	else
+		key_ref = err;
  
 found:
 	return key_ref;
@@ -474,7 +496,7 @@
 /*
  * see if the key we're looking at is the target key
  */
-static int lookup_user_key_possessed(const struct key *key, const void *target)
+int lookup_user_key_possessed(const struct key *key, const void *target)
 {
 	return key == target;
...	...	@@ -114,6 +114,10 @@
114	114	const void *description,
115	115	key_match_func_t match);
116	116
	117	+extern key_ref_t search_my_process_keyrings(struct key_type *type,
	118	+ const void *description,
	119	+ key_match_func_t match,
	120	+ const struct cred *cred);
117	121	extern key_ref_t search_process_keyrings(struct key_type *type,
118	122	const void *description,
119	123	key_match_func_t match,
...	...	@@ -134,6 +138,7 @@
134	138	struct key *dest_keyring,
135	139	unsigned long flags);
136	140
	141	+extern int lookup_user_key_possessed(const struct key key, const void target);
137	142	extern key_ref_t lookup_user_key(key_serial_t id, unsigned long flags,
138	143	key_perm_t perm);
139	144	#define KEY_LOOKUP_CREATE 0x01
...	...	@@ -184,20 +184,36 @@
184	184
185	185	static int proc_keys_show(struct seq_file m, void v)
186	186	{
	187	+ const struct cred *cred = current_cred();
187	188	struct rb_node *_p = v;
188	189	struct key *key = rb_entry(_p, struct key, serial_node);
189	190	struct timespec now;
190	191	unsigned long timo;
	192	+ key_ref_t key_ref, skey_ref;
191	193	char xbuf[12];
192	194	int rc;
193	195
	196	+ key_ref = make_key_ref(key, 0);
	197	+
	198	+ /* determine if the key is possessed by this process (a test we can
	199	+ * skip if the key does not indicate the possessor can view it
	200	+ */
	201	+ if (key->perm & KEY_POS_VIEW) {
	202	+ skey_ref = search_my_process_keyrings(key->type, key,
	203	+ lookup_user_key_possessed,
	204	+ cred);
	205	+ if (!IS_ERR(skey_ref)) {
	206	+ key_ref_put(skey_ref);
	207	+ key_ref = make_key_ref(key, 1);
	208	+ }
	209	+ }
	210	+
194	211	/* check whether the current task is allowed to view the key (assuming
195	212	* non-possession)
196	213	* - the caller holds a spinlock, and thus the RCU read lock, making our
197	214	* access to __current_cred() safe
198	215	*/
199		- rc = key_task_permission(make_key_ref(key, 0), current_cred(),
200		- KEY_VIEW);
	216	+ rc = key_task_permission(key_ref, cred, KEY_VIEW);
201	217	if (rc < 0)
202	218	return 0;
203	219
...	...	@@ -309,22 +309,19 @@
309	309
310	310	/*****************************************************************************/
311	311	/*
312		- * search the process keyrings for the first matching key
	312	+ * search only my process keyrings for the first matching key
313	313	* - we use the supplied match function to see if the description (or other
314	314	* feature of interest) matches
315	315	* - we return -EAGAIN if we didn't find any matching key
316	316	* - we return -ENOKEY if we found only negative matching keys
317	317	*/
318		-key_ref_t search_process_keyrings(struct key_type *type,
319		- const void *description,
320		- key_match_func_t match,
321		- const struct cred *cred)
	318	+key_ref_t search_my_process_keyrings(struct key_type *type,
	319	+ const void *description,
	320	+ key_match_func_t match,
	321	+ const struct cred *cred)
322	322	{
323		- struct request_key_auth *rka;
324	323	key_ref_t key_ref, ret, err;
325	324
326		- might_sleep();
327		-
328	325	/* we want to return -EAGAIN or -ENOKEY if any of the keyrings were
329	326	* searchable, but we failed to find a key or we found a negative key;
330	327	* otherwise we want to return a sample error (probably -EACCES) if
...	...	@@ -424,6 +421,36 @@
424	421	}
425	422	}
426	423
	424	+ /* no key - decide on the error we're going to go for */
	425	+ key_ref = ret ? ret : err;
	426	+
	427	+found:
	428	+ return key_ref;
	429	+}
	430	+
	431	+/*****************************************************************************/
	432	+/*
	433	+ * search the process keyrings for the first matching key
	434	+ * - we use the supplied match function to see if the description (or other
	435	+ * feature of interest) matches
	436	+ * - we return -EAGAIN if we didn't find any matching key
	437	+ * - we return -ENOKEY if we found only negative matching keys
	438	+ */
	439	+key_ref_t search_process_keyrings(struct key_type *type,
	440	+ const void *description,
	441	+ key_match_func_t match,
	442	+ const struct cred *cred)
	443	+{
	444	+ struct request_key_auth *rka;
	445	+ key_ref_t key_ref, ret = ERR_PTR(-EACCES), err;
	446	+
	447	+ might_sleep();
	448	+
	449	+ key_ref = search_my_process_keyrings(type, description, match, cred);
	450	+ if (!IS_ERR(key_ref))
	451	+ goto found;
	452	+ err = key_ref;
	453	+
427	454	/* if this process has an instantiation authorisation key, then we also
428	455	* search the keyrings of the process mentioned there
429	456	* - we don't permit access to request_key auth keys via this method
430	457
...	...	@@ -446,24 +473,19 @@
446	473	if (!IS_ERR(key_ref))
447	474	goto found;
448	475
449		- switch (PTR_ERR(key_ref)) {
450		- case -EAGAIN: /* no key */
451		- if (ret)
452		- break;
453		- case -ENOKEY: /* negative key */
454		- ret = key_ref;
455		- break;
456		- default:
457		- err = key_ref;
458		- break;
459		- }
	476	+ ret = key_ref;
460	477	} else {
461	478	up_read(&cred->request_key_auth->sem);
462	479	}
463	480	}
464	481
465	482	/* no key - decide on the error we're going to go for */
466		- key_ref = ret ? ret : err;
	483	+ if (err == ERR_PTR(-ENOKEY) \|\| ret == ERR_PTR(-ENOKEY))
	484	+ key_ref = ERR_PTR(-ENOKEY);
	485	+ else if (err == ERR_PTR(-EACCES))
	486	+ key_ref = ret;
	487	+ else
	488	+ key_ref = err;
467	489
468	490	found:
469	491	return key_ref;
...	...	@@ -474,7 +496,7 @@
474	496	/*
475	497	* see if the key we're looking at is the target key
476	498	*/
477		-static int lookup_user_key_possessed(const struct key key, const void target)
	499	+int lookup_user_key_possessed(const struct key key, const void target)
478	500	{
479	501	return key == target;
480	502