Skip to content

Commit 96b5c8f

Browse files
committed
KEYS: Reduce initial permissions on keys
Reduce the initial permissions on new keys to grant the possessor everything, view permission only to the user (so the keys can be seen in /proc/keys) and nothing else. This gives the creator a chance to adjust the permissions mask before other processes can access the new key or create a link to it. To aid with this, keyring_alloc() now takes a permission argument rather than setting the permissions itself. The following permissions are now set: (1) The user and user-session keyrings grant the user that owns them full permissions and grant a possessor everything bar SETATTR. (2) The process and thread keyrings grant the possessor full permissions but only grant the user VIEW. This permits the user to see them in /proc/keys, but not to do anything with them. (3) Anonymous session keyrings grant the possessor full permissions, but only grant the user VIEW and READ. This means that the user can see them in /proc/keys and can list them, but nothing else. Possibly READ shouldn't be provided either. (4) Named session keyrings grant everything an anonymous session keyring does, plus they grant the user LINK permission. The whole point of named session keyrings is that others can also subscribe to them. Possibly this should be a separate permission to LINK. (5) The temporary session keyring created by call_sbin_request_key() gets the same permissions as an anonymous session keyring. (6) Keys created by add_key() get VIEW, SEARCH, LINK and SETATTR for the possessor, plus READ and/or WRITE if the key type supports them. The used only gets VIEW now. (7) Keys created by request_key() now get the same as those created by add_key(). Reported-by: Lennart Poettering <lennart@poettering.net> Reported-by: Stef Walter <stefw@redhat.com> Signed-off-by: David Howells <dhowells@redhat.com>
1 parent 3a50597 commit 96b5c8f

File tree

5 files changed

+34
-19
lines changed

5 files changed

+34
-19
lines changed

include/linux/key.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,7 @@ extern int key_unlink(struct key *keyring,
264264

265265
extern struct key *keyring_alloc(const char *description, uid_t uid, gid_t gid,
266266
const struct cred *cred,
267+
key_perm_t perm,
267268
unsigned long flags,
268269
struct key *dest);
269270

security/keys/key.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -826,13 +826,13 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
826826
/* if the client doesn't provide, decide on the permissions we want */
827827
if (perm == KEY_PERM_UNDEF) {
828828
perm = KEY_POS_VIEW | KEY_POS_SEARCH | KEY_POS_LINK | KEY_POS_SETATTR;
829-
perm |= KEY_USR_VIEW | KEY_USR_SEARCH | KEY_USR_LINK | KEY_USR_SETATTR;
829+
perm |= KEY_USR_VIEW;
830830

831831
if (ktype->read)
832-
perm |= KEY_POS_READ | KEY_USR_READ;
832+
perm |= KEY_POS_READ;
833833

834834
if (ktype == &key_type_keyring || ktype->update)
835-
perm |= KEY_USR_WRITE;
835+
perm |= KEY_POS_WRITE;
836836
}
837837

838838
/* allocate a new key */

security/keys/keyring.c

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -257,17 +257,14 @@ static long keyring_read(const struct key *keyring,
257257
* Allocate a keyring and link into the destination keyring.
258258
*/
259259
struct key *keyring_alloc(const char *description, uid_t uid, gid_t gid,
260-
const struct cred *cred, unsigned long flags,
261-
struct key *dest)
260+
const struct cred *cred, key_perm_t perm,
261+
unsigned long flags, struct key *dest)
262262
{
263263
struct key *keyring;
264264
int ret;
265265

266266
keyring = key_alloc(&key_type_keyring, description,
267-
uid, gid, cred,
268-
(KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_ALL,
269-
flags);
270-
267+
uid, gid, cred, perm, flags);
271268
if (!IS_ERR(keyring)) {
272269
ret = key_instantiate_and_link(keyring, NULL, 0, dest, NULL);
273270
if (ret < 0) {

security/keys/process_keys.c

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,11 @@ int install_user_keyrings(void)
4646
struct user_struct *user;
4747
const struct cred *cred;
4848
struct key *uid_keyring, *session_keyring;
49+
key_perm_t user_keyring_perm;
4950
char buf[20];
5051
int ret;
5152

53+
user_keyring_perm = (KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_ALL;
5254
cred = current_cred();
5355
user = cred->user;
5456

@@ -72,8 +74,8 @@ int install_user_keyrings(void)
7274
uid_keyring = find_keyring_by_name(buf, true);
7375
if (IS_ERR(uid_keyring)) {
7476
uid_keyring = keyring_alloc(buf, user->uid, (gid_t) -1,
75-
cred, KEY_ALLOC_IN_QUOTA,
76-
NULL);
77+
cred, user_keyring_perm,
78+
KEY_ALLOC_IN_QUOTA, NULL);
7779
if (IS_ERR(uid_keyring)) {
7880
ret = PTR_ERR(uid_keyring);
7981
goto error;
@@ -88,7 +90,8 @@ int install_user_keyrings(void)
8890
if (IS_ERR(session_keyring)) {
8991
session_keyring =
9092
keyring_alloc(buf, user->uid, (gid_t) -1,
91-
cred, KEY_ALLOC_IN_QUOTA, NULL);
93+
cred, user_keyring_perm,
94+
KEY_ALLOC_IN_QUOTA, NULL);
9295
if (IS_ERR(session_keyring)) {
9396
ret = PTR_ERR(session_keyring);
9497
goto error_release;
@@ -129,6 +132,7 @@ int install_thread_keyring_to_cred(struct cred *new)
129132
struct key *keyring;
130133

131134
keyring = keyring_alloc("_tid", new->uid, new->gid, new,
135+
KEY_POS_ALL | KEY_USR_VIEW,
132136
KEY_ALLOC_QUOTA_OVERRUN, NULL);
133137
if (IS_ERR(keyring))
134138
return PTR_ERR(keyring);
@@ -173,8 +177,9 @@ int install_process_keyring_to_cred(struct cred *new)
173177
if (new->process_keyring)
174178
return -EEXIST;
175179

176-
keyring = keyring_alloc("_pid", new->uid, new->gid,
177-
new, KEY_ALLOC_QUOTA_OVERRUN, NULL);
180+
keyring = keyring_alloc("_pid", new->uid, new->gid, new,
181+
KEY_POS_ALL | KEY_USR_VIEW,
182+
KEY_ALLOC_QUOTA_OVERRUN, NULL);
178183
if (IS_ERR(keyring))
179184
return PTR_ERR(keyring);
180185

@@ -223,8 +228,9 @@ int install_session_keyring_to_cred(struct cred *cred, struct key *keyring)
223228
if (cred->session_keyring)
224229
flags = KEY_ALLOC_IN_QUOTA;
225230

226-
keyring = keyring_alloc("_ses", cred->uid, cred->gid,
227-
cred, flags, NULL);
231+
keyring = keyring_alloc("_ses", cred->uid, cred->gid, cred,
232+
KEY_POS_ALL | KEY_USR_VIEW | KEY_USR_READ,
233+
flags, NULL);
228234
if (IS_ERR(keyring))
229235
return PTR_ERR(keyring);
230236
} else {
@@ -773,8 +779,10 @@ long join_session_keyring(const char *name)
773779
keyring = find_keyring_by_name(name, false);
774780
if (PTR_ERR(keyring) == -ENOKEY) {
775781
/* not found - try and create a new one */
776-
keyring = keyring_alloc(name, old->uid, old->gid, old,
777-
KEY_ALLOC_IN_QUOTA, NULL);
782+
keyring = keyring_alloc(
783+
name, old->uid, old->gid, old,
784+
KEY_POS_ALL | KEY_USR_VIEW | KEY_USR_READ | KEY_USR_LINK,
785+
KEY_ALLOC_IN_QUOTA, NULL);
778786
if (IS_ERR(keyring)) {
779787
ret = PTR_ERR(keyring);
780788
goto error2;

security/keys/request_key.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ static int call_sbin_request_key(struct key_construction *cons,
126126

127127
cred = get_current_cred();
128128
keyring = keyring_alloc(desc, cred->fsuid, cred->fsgid, cred,
129+
KEY_POS_ALL | KEY_USR_VIEW | KEY_USR_READ,
129130
KEY_ALLOC_QUOTA_OVERRUN, NULL);
130131
put_cred(cred);
131132
if (IS_ERR(keyring)) {
@@ -347,6 +348,7 @@ static int construct_alloc_key(struct key_type *type,
347348
const struct cred *cred = current_cred();
348349
unsigned long prealloc;
349350
struct key *key;
351+
key_perm_t perm;
350352
key_ref_t key_ref;
351353
int ret;
352354

@@ -355,8 +357,15 @@ static int construct_alloc_key(struct key_type *type,
355357
*_key = NULL;
356358
mutex_lock(&user->cons_lock);
357359

360+
perm = KEY_POS_VIEW | KEY_POS_SEARCH | KEY_POS_LINK | KEY_POS_SETATTR;
361+
perm |= KEY_USR_VIEW;
362+
if (type->read)
363+
perm |= KEY_POS_READ;
364+
if (type == &key_type_keyring || type->update)
365+
perm |= KEY_POS_WRITE;
366+
358367
key = key_alloc(type, description, cred->fsuid, cred->fsgid, cred,
359-
KEY_POS_ALL, flags);
368+
perm, flags);
360369
if (IS_ERR(key))
361370
goto alloc_failed;
362371

0 commit comments

Comments
 (0)