public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] KEYS: Allow clients to set key perms in key_create_or_update()
@ 2008-03-13 19:14 David Howells
  2008-03-13 19:14 ` [PATCH 2/3] KEYS: Don't generate user and user session keyrings unless they're accessed David Howells
  2008-03-13 19:14 ` [PATCH 3/3] KEYS: Make the keyring quotas controllable through /proc/sys David Howells
  0 siblings, 2 replies; 12+ messages in thread
From: David Howells @ 2008-03-13 19:14 UTC (permalink / raw)
  To: torvalds, akpm, kwc
  Cc: arunsr, dwalsh, linux-security-module, dhowells, linux-kernel

The key_create_or_update() function provided by the keyring code has a default
set of permissions that are always applied to the key when created. This might
not be desirable to all clients.

Here's a patch that adds a "perm" parameter to the function to address this,
which can be set to KEY_PERM_UNDEF to revert to the current behaviour.

Signed-off-by: Arun Raghavan <arunsr@cse.iitk.ac.in>
Acked-by: David Howells <dhowells@redhat.com>
---

 include/linux/key.h    |    3 +++
 security/keys/key.c    |   18 ++++++++++--------
 security/keys/keyctl.c |    3 ++-
 3 files changed, 15 insertions(+), 9 deletions(-)


diff --git a/include/linux/key.h b/include/linux/key.h
index 163f864..8b0bd33 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -67,6 +67,8 @@ struct key;
 #define KEY_OTH_SETATTR	0x00000020
 #define KEY_OTH_ALL	0x0000003f
 
+#define KEY_PERM_UNDEF	0xffffffff
+
 struct seq_file;
 struct user_struct;
 struct signal_struct;
@@ -232,6 +234,7 @@ extern key_ref_t key_create_or_update(key_ref_t keyring,
 				      const char *description,
 				      const void *payload,
 				      size_t plen,
+				      key_perm_t perm,
 				      unsigned long flags);
 
 extern int key_update(key_ref_t key,
diff --git a/security/keys/key.c b/security/keys/key.c
index 654d23b..d98c619 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -757,11 +757,11 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
 			       const char *description,
 			       const void *payload,
 			       size_t plen,
+			       key_perm_t perm,
 			       unsigned long flags)
 {
 	struct key_type *ktype;
 	struct key *keyring, *key = NULL;
-	key_perm_t perm;
 	key_ref_t key_ref;
 	int ret;
 
@@ -806,15 +806,17 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
 			goto found_matching_key;
 	}
 
-	/* decide on the permissions we want */
-	perm = KEY_POS_VIEW | KEY_POS_SEARCH | KEY_POS_LINK | KEY_POS_SETATTR;
-	perm |= KEY_USR_VIEW | KEY_USR_SEARCH | KEY_USR_LINK | KEY_USR_SETATTR;
+	/* if the client doesn't provide, decide on the permissions we want */
+	if (perm == KEY_PERM_UNDEF) {
+		perm = KEY_POS_VIEW | KEY_POS_SEARCH | KEY_POS_LINK | KEY_POS_SETATTR;
+		perm |= KEY_USR_VIEW | KEY_USR_SEARCH | KEY_USR_LINK | KEY_USR_SETATTR;
 
-	if (ktype->read)
-		perm |= KEY_POS_READ | KEY_USR_READ;
+		if (ktype->read)
+			perm |= KEY_POS_READ | KEY_USR_READ;
 
-	if (ktype == &key_type_keyring || ktype->update)
-		perm |= KEY_USR_WRITE;
+		if (ktype == &key_type_keyring || ktype->update)
+			perm |= KEY_USR_WRITE;
+	}
 
 	/* allocate a new key */
 	key = key_alloc(ktype, description, current->fsuid, current->fsgid,
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 56e963b..993be63 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -112,7 +112,8 @@ asmlinkage long sys_add_key(const char __user *_type,
 	/* create or update the requested key and add it to the target
 	 * keyring */
 	key_ref = key_create_or_update(keyring_ref, type, description,
-				       payload, plen, KEY_ALLOC_IN_QUOTA);
+				       payload, plen, KEY_PERM_UNDEF,
+				       KEY_ALLOC_IN_QUOTA);
 	if (!IS_ERR(key_ref)) {
 		ret = key_ref_to_ptr(key_ref)->serial;
 		key_ref_put(key_ref);


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

* [PATCH 2/3] KEYS: Don't generate user and user session keyrings unless they're accessed
  2008-03-13 19:14 [PATCH 1/3] KEYS: Allow clients to set key perms in key_create_or_update() David Howells
@ 2008-03-13 19:14 ` David Howells
  2008-03-13 22:20   ` Andrew Morton
  2008-03-13 19:14 ` [PATCH 3/3] KEYS: Make the keyring quotas controllable through /proc/sys David Howells
  1 sibling, 1 reply; 12+ messages in thread
From: David Howells @ 2008-03-13 19:14 UTC (permalink / raw)
  To: torvalds, akpm, kwc
  Cc: arunsr, dwalsh, linux-security-module, dhowells, linux-kernel

Don't generate the per-UID user and user session keyrings unless they're
explicitly accessed.  This solves a problem during a login process whereby
set*uid() is called before the SELinux PAM module, resulting in the per-UID
keyrings having the wrong security labels.

This also cures the problem of multiple per-UID keyrings sometimes appearing
due to PAM modules (including pam_keyinit) setuiding and causing user_structs
to come into and go out of existence whilst the session keyring pins the user
keyring.  This is achieved by first searching for extant per-UID keyrings before
inventing new ones.

The serial bound argument is also dropped from find_keyring_by_name() as it's
not currently made use of (setting it to 0 disables the feature).

Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/linux/key.h          |    8 --
 kernel/user.c                |   15 +---
 security/keys/internal.h     |    4 -
 security/keys/key.c          |   45 -------------
 security/keys/keyring.c      |   19 ++----
 security/keys/process_keys.c |  142 +++++++++++++++++++++++++-----------------
 security/selinux/hooks.c     |    8 --
 7 files changed, 96 insertions(+), 145 deletions(-)


diff --git a/include/linux/key.h b/include/linux/key.h
index 8b0bd33..2effd03 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -268,9 +268,6 @@ extern struct key *key_lookup(key_serial_t id);
 /*
  * the userspace interface
  */
-extern struct key root_user_keyring, root_session_keyring;
-extern int alloc_uid_keyring(struct user_struct *user,
-			     struct task_struct *ctx);
 extern void switch_uid_keyring(struct user_struct *new_user);
 extern int copy_keys(unsigned long clone_flags, struct task_struct *tsk);
 extern int copy_thread_group_keys(struct task_struct *tsk);
@@ -299,7 +296,6 @@ extern void key_init(void);
 #define make_key_ref(k, p)			({ NULL; })
 #define key_ref_to_ptr(k)		({ NULL; })
 #define is_key_possessed(k)		0
-#define alloc_uid_keyring(u,c)		0
 #define switch_uid_keyring(u)		do { } while(0)
 #define __install_session_keyring(t, k)	({ NULL; })
 #define copy_keys(f,t)			0
@@ -312,10 +308,6 @@ extern void key_init(void);
 #define key_fsgid_changed(t)		do { } while(0)
 #define key_init()			do { } while(0)
 
-/* Initial keyrings */
-extern struct key root_user_keyring;
-extern struct key root_session_keyring;
-
 #endif /* CONFIG_KEYS */
 #endif /* __KERNEL__ */
 #endif /* _LINUX_KEY_H */
diff --git a/kernel/user.c b/kernel/user.c
index 7132022..2df1211 100644
--- a/kernel/user.c
+++ b/kernel/user.c
@@ -53,10 +53,6 @@ struct user_struct root_user = {
 	.files		= ATOMIC_INIT(0),
 	.sigpending	= ATOMIC_INIT(0),
 	.locked_shm     = 0,
-#ifdef CONFIG_KEYS
-	.uid_keyring	= &root_user_keyring,
-	.session_keyring = &root_session_keyring,
-#endif
 #ifdef CONFIG_USER_SCHED
 	.tg		= &init_task_group,
 #endif
@@ -392,12 +388,12 @@ struct user_struct * alloc_uid(struct user_namespace *ns, uid_t uid)
 		new->mq_bytes = 0;
 #endif
 		new->locked_shm = 0;
-
-		if (alloc_uid_keyring(new, current) < 0)
-			goto out_free_user;
+#ifdef CONFIG_KEYS
+		new->uid_keyring = new->session_keyring = NULL;
+#endif
 
 		if (sched_create_user(new) < 0)
-			goto out_put_keys;
+			goto out_free_user;
 
 		if (uids_user_create(new))
 			goto out_destoy_sched;
@@ -431,9 +427,6 @@ struct user_struct * alloc_uid(struct user_namespace *ns, uid_t uid)
 
 out_destoy_sched:
 	sched_destroy_user(new);
-out_put_keys:
-	key_put(new->uid_keyring);
-	key_put(new->session_keyring);
 out_free_user:
 	kmem_cache_free(uid_cachep, new);
 out_unlock:
diff --git a/security/keys/internal.h b/security/keys/internal.h
index f004835..b84f04a 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -77,8 +77,6 @@ extern struct mutex key_construction_mutex;
 extern wait_queue_head_t request_key_conswq;
 
 
-extern void keyring_publish_name(struct key *keyring);
-
 extern int __key_link(struct key *keyring, struct key *key);
 
 extern key_ref_t __keyring_search_one(key_ref_t keyring_ref,
@@ -102,7 +100,7 @@ extern key_ref_t search_process_keyrings(struct key_type *type,
 					 key_match_func_t match,
 					 struct task_struct *tsk);
 
-extern struct key *find_keyring_by_name(const char *name, key_serial_t bound);
+extern struct key *find_keyring_by_name(const char *name, bool skip_perm_check);
 
 extern int install_thread_keyring(struct task_struct *tsk);
 extern int install_process_keyring(struct task_struct *tsk);
diff --git a/security/keys/key.c b/security/keys/key.c
index d98c619..46f125a 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -1,6 +1,6 @@
 /* Basic authentication token and access key management
  *
- * Copyright (C) 2004-2007 Red Hat, Inc. All Rights Reserved.
+ * Copyright (C) 2004-2008 Red Hat, Inc. All Rights Reserved.
  * Written by David Howells (dhowells@redhat.com)
  *
  * This program is free software; you can redistribute it and/or
@@ -139,36 +139,6 @@ void key_user_put(struct key_user *user)
 
 /*****************************************************************************/
 /*
- * insert a key with a fixed serial number
- */
-static void __init __key_insert_serial(struct key *key)
-{
-	struct rb_node *parent, **p;
-	struct key *xkey;
-
-	parent = NULL;
-	p = &key_serial_tree.rb_node;
-
-	while (*p) {
-		parent = *p;
-		xkey = rb_entry(parent, struct key, serial_node);
-
-		if (key->serial < xkey->serial)
-			p = &(*p)->rb_left;
-		else if (key->serial > xkey->serial)
-			p = &(*p)->rb_right;
-		else
-			BUG();
-	}
-
-	/* we've found a suitable hole - arrange for this key to occupy it */
-	rb_link_node(&key->serial_node, parent, p);
-	rb_insert_color(&key->serial_node, &key_serial_tree);
-
-} /* end __key_insert_serial() */
-
-/*****************************************************************************/
-/*
  * assign a key the next unique serial number
  * - these are assigned randomly to avoid security issues through covert
  *   channel problems
@@ -1020,17 +990,4 @@ void __init key_init(void)
 	rb_insert_color(&root_key_user.node,
 			&key_user_tree);
 
-	/* record root's user standard keyrings */
-	key_check(&root_user_keyring);
-	key_check(&root_session_keyring);
-
-	__key_insert_serial(&root_user_keyring);
-	__key_insert_serial(&root_session_keyring);
-
-	keyring_publish_name(&root_user_keyring);
-	keyring_publish_name(&root_session_keyring);
-
-	/* link the two root keyrings together */
-	key_link(&root_session_keyring, &root_user_keyring);
-
 } /* end key_init() */
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 76b89b2..8c1373b 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -1,6 +1,6 @@
-/* keyring.c: keyring handling
+/* Keyring handling
  *
- * Copyright (C) 2004-5 Red Hat, Inc. All Rights Reserved.
+ * Copyright (C) 2004-2005, 2008 Red Hat, Inc. All Rights Reserved.
  * Written by David Howells (dhowells@redhat.com)
  *
  * This program is free software; you can redistribute it and/or
@@ -79,7 +79,7 @@ static DECLARE_RWSEM(keyring_serialise_link_sem);
  * publish the name of a keyring so that it can be found by name (if it has
  * one)
  */
-void keyring_publish_name(struct key *keyring)
+static void keyring_publish_name(struct key *keyring)
 {
 	int bucket;
 
@@ -516,10 +516,9 @@ key_ref_t __keyring_search_one(key_ref_t keyring_ref,
 /*
  * find a keyring with the specified name
  * - all named keyrings are searched
- * - only find keyrings with search permission for the process
- * - only find keyrings with a serial number greater than the one specified
+ * - normally only finds keyrings with search permission for the current process
  */
-struct key *find_keyring_by_name(const char *name, key_serial_t bound)
+struct key *find_keyring_by_name(const char *name, bool skip_perm_check)
 {
 	struct key *keyring;
 	int bucket;
@@ -545,15 +544,11 @@ struct key *find_keyring_by_name(const char *name, key_serial_t bound)
 			if (strcmp(keyring->description, name) != 0)
 				continue;
 
-			if (key_permission(make_key_ref(keyring, 0),
+			if (!skip_perm_check &&
+			    key_permission(make_key_ref(keyring, 0),
 					   KEY_SEARCH) < 0)
 				continue;
 
-			/* found a potential candidate, but we still need to
-			 * check the serial number */
-			if (keyring->serial <= bound)
-				continue;
-
 			/* we've got a match */
 			atomic_inc(&keyring->usage);
 			read_unlock(&keyring_name_lock);
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index c886a2b..5be6d01 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -1,6 +1,6 @@
-/* process_keys.c: management of a process's keyrings
+/* Management of a process's keyrings
  *
- * Copyright (C) 2004-5 Red Hat, Inc. All Rights Reserved.
+ * Copyright (C) 2004-2005, 2008 Red Hat, Inc. All Rights Reserved.
  * Written by David Howells (dhowells@redhat.com)
  *
  * This program is free software; you can redistribute it and/or
@@ -23,6 +23,9 @@
 /* session keyring create vs join semaphore */
 static DEFINE_MUTEX(key_session_mutex);
 
+/* user keyring creation semaphore */
+static DEFINE_MUTEX(key_user_keyring_mutex);
+
 /* the root user's tracking struct */
 struct key_user root_key_user = {
 	.usage		= ATOMIC_INIT(3),
@@ -33,78 +36,84 @@ struct key_user root_key_user = {
 	.uid		= 0,
 };
 
-/* the root user's UID keyring */
-struct key root_user_keyring = {
-	.usage		= ATOMIC_INIT(1),
-	.serial		= 2,
-	.type		= &key_type_keyring,
-	.user		= &root_key_user,
-	.sem		= __RWSEM_INITIALIZER(root_user_keyring.sem),
-	.perm		= (KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_ALL,
-	.flags		= 1 << KEY_FLAG_INSTANTIATED,
-	.description	= "_uid.0",
-#ifdef KEY_DEBUGGING
-	.magic		= KEY_DEBUG_MAGIC,
-#endif
-};
-
-/* the root user's default session keyring */
-struct key root_session_keyring = {
-	.usage		= ATOMIC_INIT(1),
-	.serial		= 1,
-	.type		= &key_type_keyring,
-	.user		= &root_key_user,
-	.sem		= __RWSEM_INITIALIZER(root_session_keyring.sem),
-	.perm		= (KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_ALL,
-	.flags		= 1 << KEY_FLAG_INSTANTIATED,
-	.description	= "_uid_ses.0",
-#ifdef KEY_DEBUGGING
-	.magic		= KEY_DEBUG_MAGIC,
-#endif
-};
-
 /*****************************************************************************/
 /*
- * allocate the keyrings to be associated with a UID
+ * install user and user session keyrings for a particular UID
  */
-int alloc_uid_keyring(struct user_struct *user,
-		      struct task_struct *ctx)
+static int install_user_keyrings(struct task_struct *tsk)
 {
+	struct user_struct *user = tsk->user;
 	struct key *uid_keyring, *session_keyring;
 	char buf[20];
 	int ret;
 
-	/* concoct a default session keyring */
-	sprintf(buf, "_uid_ses.%u", user->uid);
+	kenter("%p{%u}", user, user->uid);
 
-	session_keyring = keyring_alloc(buf, user->uid, (gid_t) -1, ctx,
-					KEY_ALLOC_IN_QUOTA, NULL);
-	if (IS_ERR(session_keyring)) {
-		ret = PTR_ERR(session_keyring);
-		goto error;
+	if (user->uid_keyring) {
+		kleave(" = 0 [exist]");
+		return 0;
 	}
 
-	/* and a UID specific keyring, pointed to by the default session
-	 * keyring */
-	sprintf(buf, "_uid.%u", user->uid);
+	mutex_lock(&key_user_keyring_mutex);
+	ret = 0;
 
-	uid_keyring = keyring_alloc(buf, user->uid, (gid_t) -1, ctx,
-				    KEY_ALLOC_IN_QUOTA, session_keyring);
-	if (IS_ERR(uid_keyring)) {
-		key_put(session_keyring);
-		ret = PTR_ERR(uid_keyring);
-		goto error;
+	if (!user->uid_keyring) {
+		/* get the UID-specific keyring
+		 * - there may be one in existence already as it may have been
+		 *   pinned by a session, but the user_struct pointing to it
+		 *   may have been destroyed by setuid */
+		sprintf(buf, "_uid.%u", user->uid);
+
+		uid_keyring = find_keyring_by_name(buf, true);
+		if (IS_ERR(uid_keyring)) {
+			uid_keyring = keyring_alloc(buf, user->uid, (gid_t) -1,
+						    tsk, KEY_ALLOC_IN_QUOTA,
+						    NULL);
+			if (IS_ERR(uid_keyring)) {
+				ret = PTR_ERR(uid_keyring);
+				goto error;
+			}
+		}
+
+		/* get a default session keyring (which might also exist
+		 * already) */
+		sprintf(buf, "_uid_ses.%u", user->uid);
+
+		session_keyring = find_keyring_by_name(buf, true);
+		if (IS_ERR(session_keyring)) {
+			session_keyring =
+				keyring_alloc(buf, user->uid, (gid_t) -1,
+					      tsk, KEY_ALLOC_IN_QUOTA, NULL);
+			if (IS_ERR(session_keyring)) {
+				ret = PTR_ERR(session_keyring);
+				goto error_release;
+			}
+
+			/* we install a link from the user session keyring to
+			 * the user keyring */
+			ret = key_link(session_keyring, uid_keyring);
+			if (ret < 0)
+				goto error_release_both;
+		}
+
+		/* install the keyrings */
+		user->uid_keyring = uid_keyring;
+		user->session_keyring = session_keyring;
 	}
 
-	/* install the keyrings */
-	user->uid_keyring = uid_keyring;
-	user->session_keyring = session_keyring;
-	ret = 0;
+	mutex_unlock(&key_user_keyring_mutex);
+	kleave(" = 0");
+	return 0;
 
+error_release_both:
+	key_put(session_keyring);
+error_release:
+	key_put(uid_keyring);
 error:
+	mutex_unlock(&key_user_keyring_mutex);
+	kleave(" = %d", ret);
 	return ret;
-
-} /* end alloc_uid_keyring() */
+}
 
 /*****************************************************************************/
 /*
@@ -481,7 +490,7 @@ key_ref_t search_process_keyrings(struct key_type *type,
 		}
 	}
 	/* or search the user-session keyring */
-	else {
+	else if (context->user->session_keyring) {
 		key_ref = keyring_search_aux(
 			make_key_ref(context->user->session_keyring, 1),
 			context, type, description, match);
@@ -614,6 +623,9 @@ key_ref_t lookup_user_key(struct task_struct *context, key_serial_t id,
 		if (!context->signal->session_keyring) {
 			/* always install a session keyring upon access if one
 			 * doesn't exist yet */
+			ret = install_user_keyrings(context);
+			if (ret < 0)
+				goto error;
 			ret = install_session_keyring(
 				context, context->user->session_keyring);
 			if (ret < 0)
@@ -628,12 +640,24 @@ key_ref_t lookup_user_key(struct task_struct *context, key_serial_t id,
 		break;
 
 	case KEY_SPEC_USER_KEYRING:
+		if (!context->user->uid_keyring) {
+			ret = install_user_keyrings(context);
+			if (ret < 0)
+				goto error;
+		}
+
 		key = context->user->uid_keyring;
 		atomic_inc(&key->usage);
 		key_ref = make_key_ref(key, 1);
 		break;
 
 	case KEY_SPEC_USER_SESSION_KEYRING:
+		if (!context->user->session_keyring) {
+			ret = install_user_keyrings(context);
+			if (ret < 0)
+				goto error;
+		}
+
 		key = context->user->session_keyring;
 		atomic_inc(&key->usage);
 		key_ref = make_key_ref(key, 1);
@@ -744,7 +768,7 @@ long join_session_keyring(const char *name)
 	mutex_lock(&key_session_mutex);
 
 	/* look for an existing keyring of this name */
-	keyring = find_keyring_by_name(name, 0);
+	keyring = find_keyring_by_name(name, false);
 	if (PTR_ERR(keyring) == -ENOKEY) {
 		/* not found - try and create a new one */
 		keyring = keyring_alloc(name, tsk->uid, tsk->gid, tsk,
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 05b8e58..d43a91c 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5463,14 +5463,6 @@ static __init int selinux_init(void)
 		printk(KERN_DEBUG "SELinux:  Starting in permissive mode\n");
 	}
 
-#ifdef CONFIG_KEYS
-	/* Add security information to initial keyrings */
-	selinux_key_alloc(&root_user_keyring, current,
-			  KEY_ALLOC_NOT_IN_QUOTA);
-	selinux_key_alloc(&root_session_keyring, current,
-			  KEY_ALLOC_NOT_IN_QUOTA);
-#endif
-
 	return 0;
 }
 


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

* [PATCH 3/3] KEYS: Make the keyring quotas controllable through /proc/sys
  2008-03-13 19:14 [PATCH 1/3] KEYS: Allow clients to set key perms in key_create_or_update() David Howells
  2008-03-13 19:14 ` [PATCH 2/3] KEYS: Don't generate user and user session keyrings unless they're accessed David Howells
@ 2008-03-13 19:14 ` David Howells
  2008-03-13 22:28   ` Andrew Morton
                     ` (2 more replies)
  1 sibling, 3 replies; 12+ messages in thread
From: David Howells @ 2008-03-13 19:14 UTC (permalink / raw)
  To: torvalds, akpm, kwc
  Cc: arunsr, dwalsh, linux-security-module, dhowells, linux-kernel

Make the keyring quotas controllable through /proc/sys files:

 (*) /proc/sys/kernel/keys/root_maxkeys
     /proc/sys/kernel/keys/root_maxbytes

     Maximum number of keys that root may have and the maximum total number of
     bytes of data that root may have stored in those keys.

 (*) /proc/sys/kernel/keys/maxkeys
     /proc/sys/kernel/keys/maxbytes

     Maximum number of keys that each non-root user may have and the maximum
     total number of bytes of data that each of those users may have stored in
     their keys.

Also increase the quotas as a number of people have been complaining that it's
not big enough.  I'm not sure that it's big enough now either, but on the
other hand, it can now be set in /etc/sysctl.conf.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/linux/key.h      |    5 +++++
 kernel/sysctl.c          |    9 +++++++++
 security/keys/Makefile   |    1 +
 security/keys/internal.h |   14 ++++++++++----
 security/keys/key.c      |   23 ++++++++++++++++++-----
 security/keys/keyctl.c   |   12 +++++++++---
 security/keys/proc.c     |    9 ++++++---
 7 files changed, 58 insertions(+), 15 deletions(-)


diff --git a/include/linux/key.h b/include/linux/key.h
index 2effd03..ad02d9c 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -19,6 +19,7 @@
 #include <linux/list.h>
 #include <linux/rbtree.h>
 #include <linux/rcupdate.h>
+#include <linux/sysctl.h>
 #include <asm/atomic.h>
 
 #ifdef __KERNEL__
@@ -265,6 +266,10 @@ extern struct key *key_lookup(key_serial_t id);
 
 #define key_serial(key) ((key) ? (key)->serial : 0)
 
+#ifdef CONFIG_SYSCTL
+extern ctl_table key_sysctls[];
+#endif
+
 /*
  * the userspace interface
  */
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index b2a2d68..1e89295 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -45,6 +45,7 @@
 #include <linux/nfs_fs.h>
 #include <linux/acpi.h>
 #include <linux/reboot.h>
+#include <linux/key.h>
 
 #include <asm/uaccess.h>
 #include <asm/processor.h>
@@ -820,6 +821,14 @@ static struct ctl_table kern_table[] = {
 		.proc_handler	= &proc_dostring,
 		.strategy	= &sysctl_string,
 	},
+#ifdef CONFIG_KEYS
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "keys",
+		.mode		= 0555,
+		.child		= key_sysctls,
+	},
+#endif
 /*
  * NOTE: do not add new entries to this table unless you have read
  * Documentation/sysctl/ctl_unnumbered.txt
diff --git a/security/keys/Makefile b/security/keys/Makefile
index 5145adf..747a464 100644
--- a/security/keys/Makefile
+++ b/security/keys/Makefile
@@ -14,3 +14,4 @@ obj-y := \
 
 obj-$(CONFIG_KEYS_COMPAT) += compat.o
 obj-$(CONFIG_PROC_FS) += proc.o
+obj-$(CONFIG_SYSCTL) += sysctl.o
diff --git a/security/keys/internal.h b/security/keys/internal.h
index b84f04a..e385066 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -57,10 +57,6 @@ struct key_user {
 	int			qnbytes;	/* number of bytes allocated to this user */
 };
 
-#define KEYQUOTA_MAX_KEYS	100
-#define KEYQUOTA_MAX_BYTES	10000
-#define KEYQUOTA_LINK_BYTES	4		/* a link in a keyring is worth 4 bytes */
-
 extern struct rb_root	key_user_tree;
 extern spinlock_t	key_user_lock;
 extern struct key_user	root_key_user;
@@ -68,6 +64,16 @@ extern struct key_user	root_key_user;
 extern struct key_user *key_user_lookup(uid_t uid);
 extern void key_user_put(struct key_user *user);
 
+/*
+ * key quota limits
+ * - root has its own separate limits to everyone else
+ */
+extern unsigned key_quota_root_maxkeys;
+extern unsigned key_quota_root_maxbytes;
+extern unsigned key_quota_maxkeys;
+extern unsigned key_quota_maxbytes;
+
+#define KEYQUOTA_LINK_BYTES	4		/* a link in a keyring is worth 4 bytes */
 
 
 extern struct rb_root key_serial_tree;
diff --git a/security/keys/key.c b/security/keys/key.c
index 46f125a..14948cf 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -27,6 +27,11 @@ DEFINE_SPINLOCK(key_serial_lock);
 struct rb_root	key_user_tree; /* tree of quota records indexed by UID */
 DEFINE_SPINLOCK(key_user_lock);
 
+unsigned int key_quota_root_maxkeys = 200;	/* root's key count quota */
+unsigned int key_quota_root_maxbytes = 20000;	/* root's key space quota */
+unsigned int key_quota_maxkeys = 200;		/* general key count quota */
+unsigned int key_quota_maxbytes = 20000;	/* general key space quota */
+
 static LIST_HEAD(key_types_list);
 static DECLARE_RWSEM(key_types_sem);
 
@@ -236,11 +241,16 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 	/* check that the user's quota permits allocation of another key and
 	 * its description */
 	if (!(flags & KEY_ALLOC_NOT_IN_QUOTA)) {
+		unsigned maxkeys = (uid == 0) ?
+			key_quota_root_maxkeys : key_quota_maxkeys;
+		unsigned maxbytes = (uid == 0) ?
+			key_quota_root_maxbytes : key_quota_maxbytes;
+
 		spin_lock(&user->lock);
 		if (!(flags & KEY_ALLOC_QUOTA_OVERRUN)) {
-			if (user->qnkeys + 1 >= KEYQUOTA_MAX_KEYS ||
-			    user->qnbytes + quotalen >= KEYQUOTA_MAX_BYTES
-			    )
+			if (user->qnkeys + 1 >= maxkeys ||
+			    user->qnbytes + quotalen >= maxbytes ||
+			    user->qnbytes + quotalen < user->qnbytes)
 				goto no_quota;
 		}
 
@@ -345,11 +355,14 @@ int key_payload_reserve(struct key *key, size_t datalen)
 
 	/* contemplate the quota adjustment */
 	if (delta != 0 && test_bit(KEY_FLAG_IN_QUOTA, &key->flags)) {
+		unsigned maxbytes = (key->user->uid == 0) ?
+			key_quota_root_maxbytes : key_quota_maxbytes;
+
 		spin_lock(&key->user->lock);
 
 		if (delta > 0 &&
-		    key->user->qnbytes + delta > KEYQUOTA_MAX_BYTES
-		    ) {
+		    (key->user->qnbytes + delta >= maxbytes ||
+		     key->user->qnbytes + delta < key->user->qnbytes)) {
 			ret = -EDQUOT;
 		}
 		else {
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 993be63..acc9c89 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -731,10 +731,16 @@ long keyctl_chown_key(key_serial_t id, uid_t uid, gid_t gid)
 
 		/* transfer the quota burden to the new user */
 		if (test_bit(KEY_FLAG_IN_QUOTA, &key->flags)) {
+			unsigned maxkeys = (uid == 0) ?
+				key_quota_root_maxkeys : key_quota_maxkeys;
+			unsigned maxbytes = (uid == 0) ?
+				key_quota_root_maxbytes : key_quota_maxbytes;
+
 			spin_lock(&newowner->lock);
-			if (newowner->qnkeys + 1 >= KEYQUOTA_MAX_KEYS ||
-			    newowner->qnbytes + key->quotalen >=
-			    KEYQUOTA_MAX_BYTES)
+			if (newowner->qnkeys + 1 >= maxkeys ||
+			    newowner->qnbytes + key->quotalen >= maxbytes ||
+			    newowner->qnbytes + key->quotalen <
+			    newowner->qnbytes)
 				goto quota_overrun;
 
 			newowner->qnkeys++;
diff --git a/security/keys/proc.c b/security/keys/proc.c
index 6941260..b49ad38 100644
--- a/security/keys/proc.c
+++ b/security/keys/proc.c
@@ -246,6 +246,10 @@ static int proc_key_users_show(struct seq_file *m, void *v)
 {
 	struct rb_node *_p = v;
 	struct key_user *user = rb_entry(_p, struct key_user, node);
+	unsigned maxkeys = (user->uid == 0) ?
+		key_quota_root_maxkeys : key_quota_maxkeys;
+	unsigned maxbytes = (user->uid == 0) ?
+		key_quota_root_maxbytes : key_quota_maxbytes;
 
 	seq_printf(m, "%5u: %5d %d/%d %d/%d %d/%d\n",
 		   user->uid,
@@ -253,10 +257,9 @@ static int proc_key_users_show(struct seq_file *m, void *v)
 		   atomic_read(&user->nkeys),
 		   atomic_read(&user->nikeys),
 		   user->qnkeys,
-		   KEYQUOTA_MAX_KEYS,
+		   maxkeys,
 		   user->qnbytes,
-		   KEYQUOTA_MAX_BYTES
-		   );
+		   maxbytes);
 
 	return 0;
 


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

* Re: [PATCH 2/3] KEYS: Don't generate user and user session keyrings unless they're accessed
  2008-03-13 19:14 ` [PATCH 2/3] KEYS: Don't generate user and user session keyrings unless they're accessed David Howells
@ 2008-03-13 22:20   ` Andrew Morton
  2008-03-14  2:30     ` David Howells
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2008-03-13 22:20 UTC (permalink / raw)
  To: David Howells
  Cc: torvalds, kwc, arunsr, dwalsh, linux-security-module, dhowells,
	linux-kernel

On Thu, 13 Mar 2008 19:14:37 +0000
David Howells <dhowells@redhat.com> wrote:

> Don't generate the per-UID user and user session keyrings unless they're
> explicitly accessed.  This solves a problem during a login process whereby
> set*uid() is called before the SELinux PAM module, resulting in the per-UID
> keyrings having the wrong security labels.
> 
> This also cures the problem of multiple per-UID keyrings sometimes appearing
> due to PAM modules (including pam_keyinit) setuiding and causing user_structs
> to come into and go out of existence whilst the session keyring pins the user
> keyring.  This is achieved by first searching for extant per-UID keyrings before
> inventing new ones.
> 
> The serial bound argument is also dropped from find_keyring_by_name() as it's
> not currently made use of (setting it to 0 disables the feature).
> 
> ..
>
> -/* Initial keyrings */
> -extern struct key root_user_keyring;
> -extern struct key root_session_keyring;

hm, I didn't realise that the keys code had special knowlege of "root". 
How does that play alongside the containers stuff?

> --- a/kernel/user.c
> +++ b/kernel/user.c
> ...
> +#ifdef CONFIG_KEYS
> +		new->uid_keyring = new->session_keyring = NULL;
> +#endif

		new->uid_keyring = NULL;
		new->session_keyring = NULL;

would be more conventional.

But better would be to teach alloc_uid() about kmem_cache_zalloc() then
take a chainsaw to it.

It's sorely tempting to say that initialising an atomic_t with memset(0) is
OK.  Heck, if it ever becomes not OK then we're screwed anwyay, because
vast tracts of code assumes that atomic_set(uninitalised_atomic, 0) works
OK.


I'll queue this up:


From: Andrew Morton <akpm@linux-foundation.org>

Use kmem_cache_zalloc(), remove large amounts of initialsiation code and
ifdeffery.

Note: this assumes that memset(*atomic_t, 0) correctly initialises the
atomic_t.  This is true for all present archtiectures and if it becomes false
for a future architecture then we'll need to make large changes all over the
place anyway.


Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 kernel/user.c |   18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff -puN kernel/user.c~alloc_uid-cleanup kernel/user.c
--- a/kernel/user.c~alloc_uid-cleanup
+++ a/kernel/user.c
@@ -356,7 +356,7 @@ void free_uid(struct user_struct *up)
 		local_irq_restore(flags);
 }
 
-struct user_struct * alloc_uid(struct user_namespace *ns, uid_t uid)
+struct user_struct *alloc_uid(struct user_namespace *ns, uid_t uid)
 {
 	struct hlist_head *hashent = uidhashentry(ns, uid);
 	struct user_struct *up, *new;
@@ -371,26 +371,12 @@ struct user_struct * alloc_uid(struct us
 	spin_unlock_irq(&uidhash_lock);
 
 	if (!up) {
-		new = kmem_cache_alloc(uid_cachep, GFP_KERNEL);
+		new = kmem_cache_zalloc(uid_cachep, GFP_KERNEL);
 		if (!new)
 			goto out_unlock;
 
 		new->uid = uid;
 		atomic_set(&new->__count, 1);
-		atomic_set(&new->processes, 0);
-		atomic_set(&new->files, 0);
-		atomic_set(&new->sigpending, 0);
-#ifdef CONFIG_INOTIFY_USER
-		atomic_set(&new->inotify_watches, 0);
-		atomic_set(&new->inotify_devs, 0);
-#endif
-#ifdef CONFIG_POSIX_MQUEUE
-		new->mq_bytes = 0;
-#endif
-		new->locked_shm = 0;
-#ifdef CONFIG_KEYS
-		new->uid_keyring = new->session_keyring = NULL;
-#endif
 
 		if (sched_create_user(new) < 0)
 			goto out_free_user;
_


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

* Re: [PATCH 3/3] KEYS: Make the keyring quotas controllable through /proc/sys
  2008-03-13 19:14 ` [PATCH 3/3] KEYS: Make the keyring quotas controllable through /proc/sys David Howells
@ 2008-03-13 22:28   ` Andrew Morton
  2008-03-14  2:39     ` David Howells
  2008-03-14 11:46     ` David Howells
  2008-03-13 22:47   ` Andrew Morton
  2008-03-19  0:04   ` Andrew Morton
  2 siblings, 2 replies; 12+ messages in thread
From: Andrew Morton @ 2008-03-13 22:28 UTC (permalink / raw)
  To: David Howells
  Cc: torvalds, kwc, arunsr, dwalsh, linux-security-module, dhowells,
	linux-kernel

On Thu, 13 Mar 2008 19:14:42 +0000
David Howells <dhowells@redhat.com> wrote:

> Make the keyring quotas controllable through /proc/sys files:
> 
>  (*) /proc/sys/kernel/keys/root_maxkeys
>      /proc/sys/kernel/keys/root_maxbytes
> 
>      Maximum number of keys that root may have and the maximum total number of
>      bytes of data that root may have stored in those keys.
> 
>  (*) /proc/sys/kernel/keys/maxkeys
>      /proc/sys/kernel/keys/maxbytes
> 
>      Maximum number of keys that each non-root user may have and the maximum
>      total number of bytes of data that each of those users may have stored in
>      their keys.
> 
> Also increase the quotas as a number of people have been complaining that it's
> not big enough.  I'm not sure that it's big enough now either, but on the
> other hand, it can now be set in /etc/sysctl.conf.
> 
> ...
>
> --- a/include/linux/key.h
> +++ b/include/linux/key.h
> @@ -19,6 +19,7 @@
>  #include <linux/list.h>
>  #include <linux/rbtree.h>
>  #include <linux/rcupdate.h>
> +#include <linux/sysctl.h>
>  #include <asm/atomic.h>
>  
>  #ifdef __KERNEL__
> @@ -265,6 +266,10 @@ extern struct key *key_lookup(key_serial_t id);
>  
>  #define key_serial(key) ((key) ? (key)->serial : 0)

err, why was that a macro?  It could have been implemented as an inline. 
One which doesn't evaluate its argument twice (or once if it was -1).
 
> +#ifdef CONFIG_SYSCTL
> +extern ctl_table key_sysctls[];
> +#endif

I've been going around telling people to not bother with the ifdefs here. 
Upside: looks nicer.  Downside: defers the build error from compile-time to
link-time.

>  
> +/*
> + * key quota limits
> + * - root has its own separate limits to everyone else
> + */
> +extern unsigned key_quota_root_maxkeys;
> +extern unsigned key_quota_root_maxbytes;
> +extern unsigned key_quota_maxkeys;
> +extern unsigned key_quota_maxbytes;
> +
> +#define KEYQUOTA_LINK_BYTES	4		/* a link in a keyring is worth 4 bytes */
>  
>  
>  extern struct rb_root key_serial_tree;
> diff --git a/security/keys/key.c b/security/keys/key.c
> index 46f125a..14948cf 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -27,6 +27,11 @@ DEFINE_SPINLOCK(key_serial_lock);
>  struct rb_root	key_user_tree; /* tree of quota records indexed by UID */
>  DEFINE_SPINLOCK(key_user_lock);
>  
> +unsigned int key_quota_root_maxkeys = 200;	/* root's key count quota */
> +unsigned int key_quota_root_maxbytes = 20000;	/* root's key space quota */
> +unsigned int key_quota_maxkeys = 200;		/* general key count quota */
> +unsigned int key_quota_maxbytes = 20000;	/* general key space quota */
> +
>  static LIST_HEAD(key_types_list);
>  static DECLARE_RWSEM(key_types_sem);
>  
> @@ -236,11 +241,16 @@ struct key *key_alloc(struct key_type *type, const char *desc,
>  	/* check that the user's quota permits allocation of another key and
>  	 * its description */
>  	if (!(flags & KEY_ALLOC_NOT_IN_QUOTA)) {
> +		unsigned maxkeys = (uid == 0) ?
> +			key_quota_root_maxkeys : key_quota_maxkeys;
> +		unsigned maxbytes = (uid == 0) ?
> +			key_quota_root_maxbytes : key_quota_maxbytes;

open-coded knowledge of uid==0 might be problematic for containerisation. 
Maybe it's on the containers team's radar, maybe it's actually not a
problem, don't know.  (adds cc).



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

* Re: [PATCH 3/3] KEYS: Make the keyring quotas controllable through /proc/sys
  2008-03-13 19:14 ` [PATCH 3/3] KEYS: Make the keyring quotas controllable through /proc/sys David Howells
  2008-03-13 22:28   ` Andrew Morton
@ 2008-03-13 22:47   ` Andrew Morton
  2008-03-14  2:30     ` David Howells
  2008-03-19  0:04   ` Andrew Morton
  2 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2008-03-13 22:47 UTC (permalink / raw)
  To: David Howells
  Cc: torvalds, kwc, arunsr, dwalsh, linux-security-module, dhowells,
	linux-kernel

On Thu, 13 Mar 2008 19:14:42 +0000
David Howells <dhowells@redhat.com> wrote:

> Make the keyring quotas controllable through /proc/sys files:
> 
>  (*) /proc/sys/kernel/keys/root_maxkeys
>      /proc/sys/kernel/keys/root_maxbytes
> 
>      Maximum number of keys that root may have and the maximum total number of
>      bytes of data that root may have stored in those keys.
> 
>  (*) /proc/sys/kernel/keys/maxkeys
>      /proc/sys/kernel/keys/maxbytes
> 
>      Maximum number of keys that each non-root user may have and the maximum
>      total number of bytes of data that each of those users may have stored in
>      their keys.
> 
> Also increase the quotas as a number of people have been complaining that it's
> not big enough.  I'm not sure that it's big enough now either, but on the
> other hand, it can now be set in /etc/sysctl.conf.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
> 
>  include/linux/key.h      |    5 +++++
>  kernel/sysctl.c          |    9 +++++++++
>  security/keys/Makefile   |    1 +
>  security/keys/internal.h |   14 ++++++++++----
>  security/keys/key.c      |   23 ++++++++++++++++++-----
>  security/keys/keyctl.c   |   12 +++++++++---
>  security/keys/proc.c     |    9 ++++++---
>  7 files changed, 58 insertions(+), 15 deletions(-)

Documentation/keys.txt needs an update?

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

* Re: [PATCH 2/3] KEYS: Don't generate user and user session keyrings unless they're accessed
  2008-03-13 22:20   ` Andrew Morton
@ 2008-03-14  2:30     ` David Howells
  0 siblings, 0 replies; 12+ messages in thread
From: David Howells @ 2008-03-14  2:30 UTC (permalink / raw)
  To: Andrew Morton
  Cc: dhowells, torvalds, kwc, arunsr, dwalsh, linux-security-module,
	linux-kernel

Andrew Morton <akpm@linux-foundation.org> wrote:

> hm, I didn't realise that the keys code had special knowlege of "root". 
> How does that play alongside the containers stuff?

The containers stuff lacks a keys container.  I'll have to attend to that at
some point.

The key code didn't so much have special knowledge of root, as UID 0 is
compiled into the kernel in various ways.

> would be more conventional.
> 
> But better would be to teach alloc_uid() about kmem_cache_zalloc() then
> take a chainsaw to it.

Yeah, I was thinking that.  That'd allow a slew of initialisations-to-zero to
be removed from that function, as indeed you have done in your attached patch.

> From: Andrew Morton <akpm@linux-foundation.org>
> 
> Use kmem_cache_zalloc(), remove large amounts of initialsiation code and
> ifdeffery.

"initialisation" perchance? :-)

> Note: this assumes that memset(*atomic_t, 0) correctly initialises the
> atomic_t.  This is true for all present archtiectures and if it becomes false
> for a future architecture then we'll need to make large changes all over the
> place anyway.
> 
> 
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

Acked-by: David Howells <dhowells@redhat.com>

David

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

* Re: [PATCH 3/3] KEYS: Make the keyring quotas controllable through /proc/sys
  2008-03-13 22:47   ` Andrew Morton
@ 2008-03-14  2:30     ` David Howells
  0 siblings, 0 replies; 12+ messages in thread
From: David Howells @ 2008-03-14  2:30 UTC (permalink / raw)
  To: Andrew Morton
  Cc: dhowells, torvalds, kwc, arunsr, dwalsh, linux-security-module,
	linux-kernel

Andrew Morton <akpm@linux-foundation.org> wrote:

> Documentation/keys.txt needs an update?

Good point.  The keyutils package too.

David

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

* Re: [PATCH 3/3] KEYS: Make the keyring quotas controllable through /proc/sys
  2008-03-13 22:28   ` Andrew Morton
@ 2008-03-14  2:39     ` David Howells
  2008-03-14 11:46     ` David Howells
  1 sibling, 0 replies; 12+ messages in thread
From: David Howells @ 2008-03-14  2:39 UTC (permalink / raw)
  To: Andrew Morton
  Cc: dhowells, torvalds, kwc, arunsr, dwalsh, linux-security-module,
	linux-kernel

Andrew Morton <akpm@linux-foundation.org> wrote:

> >  #define key_serial(key) ((key) ? (key)->serial : 0)
> 
> err, why was that a macro?  It could have been implemented as an inline. 
> One which doesn't evaluate its argument twice (or once if it was -1).

Probably because the CONFIG_KEYS=n version of key_serial() has to be a macro
(so as to discard key without any attempt at evaluation).

However, that doesn't apply to the CONFIG_KEYS=y case, so I'll whip up a patch
to alter that for you.

> > +#ifdef CONFIG_SYSCTL
> > +extern ctl_table key_sysctls[];
> > +#endif
> 
> I've been going around telling people to not bother with the ifdefs here. 
> Upside: looks nicer.  Downside: defers the build error from compile-time to
> link-time.

Downside #2: the #ifdef is documentation of a sort.  Personally, I prefer the
extra #ifdef here as it makes it clearer to someone looking at the .h file why
their code doesn't compile/link rather than them having to know to go fishing
in Makefiles.

> open-coded knowledge of uid==0 might be problematic for containerisation. 
> Maybe it's on the containers team's radar, maybe it's actually not a
> problem, don't know.  (adds cc).

As I understand it, the same applies to any UID.  UID 0/root is typically
special, but I don't know how this is normally handled in alternate containers
for other kernel objects.  I can't just go and look at a capability on current
because the quota belongs to the user, not the process.

I have become aware in the last couple of days that the container people
missed or left out keys.  Possibly key IDs as well as key quotas should be
separated.

David

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

* Re: [PATCH 3/3] KEYS: Make the keyring quotas controllable through /proc/sys
  2008-03-13 22:28   ` Andrew Morton
  2008-03-14  2:39     ` David Howells
@ 2008-03-14 11:46     ` David Howells
  1 sibling, 0 replies; 12+ messages in thread
From: David Howells @ 2008-03-14 11:46 UTC (permalink / raw)
  To: Andrew Morton
  Cc: dhowells, torvalds, kwc, arunsr, dwalsh, linux-security-module,
	linux-kernel

Andrew Morton <akpm@linux-foundation.org> wrote:

> > +#ifdef CONFIG_SYSCTL
> > +extern ctl_table key_sysctls[];
> > +#endif
> 
> I've been going around telling people to not bother with the ifdefs here. 
> Upside: looks nicer.  Downside: defers the build error from compile-time to
> link-time.

Should this extern actually be in include/linux/sysctl.h?  I'd rather not put
it in include/linux/key.h as that means that key.h must include sysctl.h just
to get a definition of ctl_table.  Conversely, however, sysctl.h doesn't need
to include key.h to include the above extern.

David

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

* Re: [PATCH 3/3] KEYS: Make the keyring quotas controllable through /proc/sys
  2008-03-13 19:14 ` [PATCH 3/3] KEYS: Make the keyring quotas controllable through /proc/sys David Howells
  2008-03-13 22:28   ` Andrew Morton
  2008-03-13 22:47   ` Andrew Morton
@ 2008-03-19  0:04   ` Andrew Morton
  2008-03-19 11:19     ` David Howells
  2 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2008-03-19  0:04 UTC (permalink / raw)
  To: David Howells
  Cc: torvalds, kwc, arunsr, dwalsh, linux-security-module, dhowells,
	linux-kernel

On Thu, 13 Mar 2008 19:14:42 +0000
David Howells <dhowells@redhat.com> wrote:

> Make the keyring quotas controllable through /proc/sys files:
> 
>  (*) /proc/sys/kernel/keys/root_maxkeys
>      /proc/sys/kernel/keys/root_maxbytes
> 
>      Maximum number of keys that root may have and the maximum total number of
>      bytes of data that root may have stored in those keys.
> 
>  (*) /proc/sys/kernel/keys/maxkeys
>      /proc/sys/kernel/keys/maxbytes
> 
>      Maximum number of keys that each non-root user may have and the maximum
>      total number of bytes of data that each of those users may have stored in
>      their keys.
> 
> Also increase the quotas as a number of people have been complaining that it's
> not big enough.  I'm not sure that it's big enough now either, but on the
> other hand, it can now be set in /etc/sysctl.conf.
> 
> ...
>
>  include/linux/key.h      |    5 +++++
>  kernel/sysctl.c          |    9 +++++++++
>  security/keys/Makefile   |    1 +
>  security/keys/internal.h |   14 ++++++++++----
>  security/keys/key.c      |   23 ++++++++++++++++++-----
>  security/keys/keyctl.c   |   12 +++++++++---
>  security/keys/proc.c     |    9 ++++++---
>  7 files changed, 58 insertions(+), 15 deletions(-)
>
> ...
>
> --- a/security/keys/Makefile
> +++ b/security/keys/Makefile
> @@ -14,3 +14,4 @@ obj-y := \
>  
>  obj-$(CONFIG_KEYS_COMPAT) += compat.o
>  obj-$(CONFIG_PROC_FS) += proc.o
> +obj-$(CONFIG_SYSCTL) += sysctl.o

Could we please have a copy of sysctl.c?  The boring old build system seems
to think it's important.


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

* Re: [PATCH 3/3] KEYS: Make the keyring quotas controllable through /proc/sys
  2008-03-19  0:04   ` Andrew Morton
@ 2008-03-19 11:19     ` David Howells
  0 siblings, 0 replies; 12+ messages in thread
From: David Howells @ 2008-03-19 11:19 UTC (permalink / raw)
  To: Andrew Morton
  Cc: dhowells, torvalds, kwc, arunsr, dwalsh, linux-security-module,
	linux-kernel

Andrew Morton <akpm@linux-foundation.org> wrote:

> Could we please have a copy of sysctl.c?  The boring old build system seems
> to think it's important.

Oops.  Here's a copy of the patch *with* sysctl.c.  Note that I've folded the
doc updates patch into this one.

David
---
KEYS: Make the keyring quotas controllable through /proc/sys

From: David Howells <dhowells@redhat.com>

Make the keyring quotas controllable through /proc/sys files:

 (*) /proc/sys/kernel/keys/root_maxkeys
     /proc/sys/kernel/keys/root_maxbytes

     Maximum number of keys that root may have and the maximum total number of
     bytes of data that root may have stored in those keys.

 (*) /proc/sys/kernel/keys/maxkeys
     /proc/sys/kernel/keys/maxbytes

     Maximum number of keys that each non-root user may have and the maximum
     total number of bytes of data that each of those users may have stored in
     their keys.

Also increase the quotas as a number of people have been complaining that it's
not big enough.  I'm not sure that it's big enough now either, but on the
other hand, it can now be set in /etc/sysctl.conf.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 Documentation/keys.txt   |   24 +++++++++++++++++++++-
 include/linux/key.h      |    5 +++++
 kernel/sysctl.c          |    9 ++++++++
 security/keys/Makefile   |    1 +
 security/keys/internal.h |   14 +++++++++----
 security/keys/key.c      |   23 +++++++++++++++++----
 security/keys/keyctl.c   |   12 ++++++++---
 security/keys/proc.c     |    9 ++++++--
 security/keys/sysctl.c   |   50 ++++++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 131 insertions(+), 16 deletions(-)
 create mode 100644 security/keys/sysctl.c


diff --git a/Documentation/keys.txt b/Documentation/keys.txt
index b82d38d..38a90d9 100644
--- a/Documentation/keys.txt
+++ b/Documentation/keys.txt
@@ -170,7 +170,8 @@ The key service provides a number of features besides keys:
      amount of description and payload space that can be consumed.
 
      The user can view information on this and other statistics through procfs
-     files.
+     files.  The root user may also alter the quota limits through sysctl files
+     (see the section "New procfs files").
 
      Process-specific and thread-specific keyrings are not counted towards a
      user's quota.
@@ -329,6 +330,27 @@ about the status of the key service:
 	<bytes>/<max>		Key size quota
 
 
+Four new sysctl files have been added also for the purpose of controlling the
+quota limits on keys:
+
+ (*) /proc/sys/kernel/keys/root_maxkeys
+     /proc/sys/kernel/keys/root_maxbytes
+
+     These files hold the maximum number of keys that root may have and the
+     maximum total number of bytes of data that root may have stored in those
+     keys.
+
+ (*) /proc/sys/kernel/keys/maxkeys
+     /proc/sys/kernel/keys/maxbytes
+
+     These files hold the maximum number of keys that each non-root user may
+     have and the maximum total number of bytes of data that each of those
+     users may have stored in their keys.
+
+Root may alter these by writing each new limit as a decimal number string to
+the appropriate file.
+
+
 ===============================
 USERSPACE SYSTEM CALL INTERFACE
 ===============================
diff --git a/include/linux/key.h b/include/linux/key.h
index 2effd03..ad02d9c 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -19,6 +19,7 @@
 #include <linux/list.h>
 #include <linux/rbtree.h>
 #include <linux/rcupdate.h>
+#include <linux/sysctl.h>
 #include <asm/atomic.h>
 
 #ifdef __KERNEL__
@@ -265,6 +266,10 @@ extern struct key *key_lookup(key_serial_t id);
 
 #define key_serial(key) ((key) ? (key)->serial : 0)
 
+#ifdef CONFIG_SYSCTL
+extern ctl_table key_sysctls[];
+#endif
+
 /*
  * the userspace interface
  */
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index b2a2d68..1e89295 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -45,6 +45,7 @@
 #include <linux/nfs_fs.h>
 #include <linux/acpi.h>
 #include <linux/reboot.h>
+#include <linux/key.h>
 
 #include <asm/uaccess.h>
 #include <asm/processor.h>
@@ -820,6 +821,14 @@ static struct ctl_table kern_table[] = {
 		.proc_handler	= &proc_dostring,
 		.strategy	= &sysctl_string,
 	},
+#ifdef CONFIG_KEYS
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "keys",
+		.mode		= 0555,
+		.child		= key_sysctls,
+	},
+#endif
 /*
  * NOTE: do not add new entries to this table unless you have read
  * Documentation/sysctl/ctl_unnumbered.txt
diff --git a/security/keys/Makefile b/security/keys/Makefile
index 5145adf..747a464 100644
--- a/security/keys/Makefile
+++ b/security/keys/Makefile
@@ -14,3 +14,4 @@ obj-y := \
 
 obj-$(CONFIG_KEYS_COMPAT) += compat.o
 obj-$(CONFIG_PROC_FS) += proc.o
+obj-$(CONFIG_SYSCTL) += sysctl.o
diff --git a/security/keys/internal.h b/security/keys/internal.h
index b84f04a..e385066 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -57,10 +57,6 @@ struct key_user {
 	int			qnbytes;	/* number of bytes allocated to this user */
 };
 
-#define KEYQUOTA_MAX_KEYS	100
-#define KEYQUOTA_MAX_BYTES	10000
-#define KEYQUOTA_LINK_BYTES	4		/* a link in a keyring is worth 4 bytes */
-
 extern struct rb_root	key_user_tree;
 extern spinlock_t	key_user_lock;
 extern struct key_user	root_key_user;
@@ -68,6 +64,16 @@ extern struct key_user	root_key_user;
 extern struct key_user *key_user_lookup(uid_t uid);
 extern void key_user_put(struct key_user *user);
 
+/*
+ * key quota limits
+ * - root has its own separate limits to everyone else
+ */
+extern unsigned key_quota_root_maxkeys;
+extern unsigned key_quota_root_maxbytes;
+extern unsigned key_quota_maxkeys;
+extern unsigned key_quota_maxbytes;
+
+#define KEYQUOTA_LINK_BYTES	4		/* a link in a keyring is worth 4 bytes */
 
 
 extern struct rb_root key_serial_tree;
diff --git a/security/keys/key.c b/security/keys/key.c
index 46f125a..14948cf 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -27,6 +27,11 @@ DEFINE_SPINLOCK(key_serial_lock);
 struct rb_root	key_user_tree; /* tree of quota records indexed by UID */
 DEFINE_SPINLOCK(key_user_lock);
 
+unsigned int key_quota_root_maxkeys = 200;	/* root's key count quota */
+unsigned int key_quota_root_maxbytes = 20000;	/* root's key space quota */
+unsigned int key_quota_maxkeys = 200;		/* general key count quota */
+unsigned int key_quota_maxbytes = 20000;	/* general key space quota */
+
 static LIST_HEAD(key_types_list);
 static DECLARE_RWSEM(key_types_sem);
 
@@ -236,11 +241,16 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 	/* check that the user's quota permits allocation of another key and
 	 * its description */
 	if (!(flags & KEY_ALLOC_NOT_IN_QUOTA)) {
+		unsigned maxkeys = (uid == 0) ?
+			key_quota_root_maxkeys : key_quota_maxkeys;
+		unsigned maxbytes = (uid == 0) ?
+			key_quota_root_maxbytes : key_quota_maxbytes;
+
 		spin_lock(&user->lock);
 		if (!(flags & KEY_ALLOC_QUOTA_OVERRUN)) {
-			if (user->qnkeys + 1 >= KEYQUOTA_MAX_KEYS ||
-			    user->qnbytes + quotalen >= KEYQUOTA_MAX_BYTES
-			    )
+			if (user->qnkeys + 1 >= maxkeys ||
+			    user->qnbytes + quotalen >= maxbytes ||
+			    user->qnbytes + quotalen < user->qnbytes)
 				goto no_quota;
 		}
 
@@ -345,11 +355,14 @@ int key_payload_reserve(struct key *key, size_t datalen)
 
 	/* contemplate the quota adjustment */
 	if (delta != 0 && test_bit(KEY_FLAG_IN_QUOTA, &key->flags)) {
+		unsigned maxbytes = (key->user->uid == 0) ?
+			key_quota_root_maxbytes : key_quota_maxbytes;
+
 		spin_lock(&key->user->lock);
 
 		if (delta > 0 &&
-		    key->user->qnbytes + delta > KEYQUOTA_MAX_BYTES
-		    ) {
+		    (key->user->qnbytes + delta >= maxbytes ||
+		     key->user->qnbytes + delta < key->user->qnbytes)) {
 			ret = -EDQUOT;
 		}
 		else {
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 5f0d85b..c32f5c6 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -730,10 +730,16 @@ long keyctl_chown_key(key_serial_t id, uid_t uid, gid_t gid)
 
 		/* transfer the quota burden to the new user */
 		if (test_bit(KEY_FLAG_IN_QUOTA, &key->flags)) {
+			unsigned maxkeys = (uid == 0) ?
+				key_quota_root_maxkeys : key_quota_maxkeys;
+			unsigned maxbytes = (uid == 0) ?
+				key_quota_root_maxbytes : key_quota_maxbytes;
+
 			spin_lock(&newowner->lock);
-			if (newowner->qnkeys + 1 >= KEYQUOTA_MAX_KEYS ||
-			    newowner->qnbytes + key->quotalen >=
-			    KEYQUOTA_MAX_BYTES)
+			if (newowner->qnkeys + 1 >= maxkeys ||
+			    newowner->qnbytes + key->quotalen >= maxbytes ||
+			    newowner->qnbytes + key->quotalen <
+			    newowner->qnbytes)
 				goto quota_overrun;
 
 			newowner->qnkeys++;
diff --git a/security/keys/proc.c b/security/keys/proc.c
index 6941260..b49ad38 100644
--- a/security/keys/proc.c
+++ b/security/keys/proc.c
@@ -246,6 +246,10 @@ static int proc_key_users_show(struct seq_file *m, void *v)
 {
 	struct rb_node *_p = v;
 	struct key_user *user = rb_entry(_p, struct key_user, node);
+	unsigned maxkeys = (user->uid == 0) ?
+		key_quota_root_maxkeys : key_quota_maxkeys;
+	unsigned maxbytes = (user->uid == 0) ?
+		key_quota_root_maxbytes : key_quota_maxbytes;
 
 	seq_printf(m, "%5u: %5d %d/%d %d/%d %d/%d\n",
 		   user->uid,
@@ -253,10 +257,9 @@ static int proc_key_users_show(struct seq_file *m, void *v)
 		   atomic_read(&user->nkeys),
 		   atomic_read(&user->nikeys),
 		   user->qnkeys,
-		   KEYQUOTA_MAX_KEYS,
+		   maxkeys,
 		   user->qnbytes,
-		   KEYQUOTA_MAX_BYTES
-		   );
+		   maxbytes);
 
 	return 0;
 
diff --git a/security/keys/sysctl.c b/security/keys/sysctl.c
new file mode 100644
index 0000000..b611d49
--- /dev/null
+++ b/security/keys/sysctl.c
@@ -0,0 +1,50 @@
+/* Key management controls
+ *
+ * Copyright (C) 2008 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <linux/key.h>
+#include <linux/sysctl.h>
+#include "internal.h"
+
+ctl_table key_sysctls[] = {
+	{
+		.ctl_name = CTL_UNNUMBERED,
+		.procname = "maxkeys",
+		.data = &key_quota_maxkeys,
+		.maxlen = sizeof(unsigned),
+		.mode = 0644,
+		.proc_handler = &proc_dointvec,
+	},
+	{
+		.ctl_name = CTL_UNNUMBERED,
+		.procname = "maxbytes",
+		.data = &key_quota_maxbytes,
+		.maxlen = sizeof(unsigned),
+		.mode = 0644,
+		.proc_handler = &proc_dointvec,
+	},
+	{
+		.ctl_name = CTL_UNNUMBERED,
+		.procname = "root_maxkeys",
+		.data = &key_quota_root_maxkeys,
+		.maxlen = sizeof(unsigned),
+		.mode = 0644,
+		.proc_handler = &proc_dointvec,
+	},
+	{
+		.ctl_name = CTL_UNNUMBERED,
+		.procname = "root_maxbytes",
+		.data = &key_quota_root_maxbytes,
+		.maxlen = sizeof(unsigned),
+		.mode = 0644,
+		.proc_handler = &proc_dointvec,
+	},
+	{ .ctl_name = 0 }
+};

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

end of thread, other threads:[~2008-03-19 21:00 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-13 19:14 [PATCH 1/3] KEYS: Allow clients to set key perms in key_create_or_update() David Howells
2008-03-13 19:14 ` [PATCH 2/3] KEYS: Don't generate user and user session keyrings unless they're accessed David Howells
2008-03-13 22:20   ` Andrew Morton
2008-03-14  2:30     ` David Howells
2008-03-13 19:14 ` [PATCH 3/3] KEYS: Make the keyring quotas controllable through /proc/sys David Howells
2008-03-13 22:28   ` Andrew Morton
2008-03-14  2:39     ` David Howells
2008-03-14 11:46     ` David Howells
2008-03-13 22:47   ` Andrew Morton
2008-03-14  2:30     ` David Howells
2008-03-19  0:04   ` Andrew Morton
2008-03-19 11:19     ` David Howells

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox