From: Serge E. Hallyn <[email protected]>
fsuid should always trail euid changes. So p9auth should
set fsuid as well when it sets ruid and euid. Whether the
suid should also be set is an open question - keeping the
old uid in suid may be useful, or may just serve to trick
lazy userspace.
Note that so long as we do not also set suid, the setuid_fixup()
code will not (when we later switch to setresuid()) fully
fill/clear capability sets. So while I had previously thought
that keeping suid unchanged would be useful, I think it is
better to change all uids.
Signed-off-by: Serge E. Hallyn <[email protected]>
Cc: Greg KH <[email protected]>
cc: [email protected]
Cc: Ashwin Ganti <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/staging/p9auth/p9auth.c | 14 +++++++++-----
1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/p9auth/p9auth.c b/drivers/staging/p9auth/p9auth.c
index db79626..70ef45b 100644
--- a/drivers/staging/p9auth/p9auth.c
+++ b/drivers/staging/p9auth/p9auth.c
@@ -275,10 +275,14 @@ static ssize_t cap_write(struct file *filp, const char __user *buf,
goto out;
}
/*
- * What all id's need to be changed here? uid,
- * euid, fsid, savedids ?? Currently I am
- * changing the effective user id since most of
- * the authorisation decisions are based on it
+ * Change all uids. It might be useful to
+ * keep suid unchanged, however that will
+ * mean that changing from uid=0 to uid=!0
+ * pP is not emptied (only pE is), and when
+ * changing from uid=!0 to uid=0, sets are
+ * not filled. They will be correct after
+ * the next exec, but this is IMO not
+ * sufficient. So change all uids.
*/
new = prepare_creds();
if (!new) {
@@ -286,7 +290,7 @@ static ssize_t cap_write(struct file *filp, const char __user *buf,
goto out;
}
new->uid = (uid_t) target_int;
- new->euid = (uid_t) target_int;
+ new->suid = new->fsuid = new->euid = new->uid;
retval = commit_creds(new);
if (retval)
goto out;
--
1.6.1
From: Serge E. Hallyn <[email protected]>
Granting userid capabilities to another task is a dangerous
privilege. Don't just let file permissions authorize it.
Define CAP_GRANT_ID as a new capability needed to write to
/dev/caphash.
For one thing this lets us start a factotum server early on
in init, then have init drop CAP_GRANT_ID from its bounding
set so the rest of the system cannot regain it.
Signed-off-by: Serge E. Hallyn <[email protected]>
Cc: Greg KH <[email protected]>
cc: [email protected]
Cc: Ashwin Ganti <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/staging/p9auth/p9auth.c | 4 ++++
include/linux/capability.h | 6 +++++-
2 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/p9auth/p9auth.c b/drivers/staging/p9auth/p9auth.c
index 8f70daa..fb27459 100644
--- a/drivers/staging/p9auth/p9auth.c
+++ b/drivers/staging/p9auth/p9auth.c
@@ -201,6 +201,10 @@ static ssize_t cap_write(struct file *filp, const char __user *buf,
retval = -EINVAL;
goto out;
}
+ if (!capable(CAP_GRANT_ID)) {
+ retval = -EPERM;
+ goto out;
+ }
printk(KERN_INFO "Capability being written to /dev/caphash : \n");
hexdump(user_buf, count);
memcpy(node_ptr->data, user_buf, count);
diff --git a/include/linux/capability.h b/include/linux/capability.h
index 39e5ff5..ba2cbfe 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -355,7 +355,11 @@ struct cpu_vfs_cap_data {
#define CAP_MAC_ADMIN 33
-#define CAP_LAST_CAP CAP_MAC_ADMIN
+/* Allow granting setuid capabilities through p9auth /dev/caphash */
+
+#define CAP_GRANT_ID 34
+
+#define CAP_LAST_CAP CAP_GRANT_ID
#define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
--
1.6.1
From: Serge E. Hallyn <[email protected]>
Use setresuid to set userids through p9auth capability device, to
make sure MAC checks and setuid fixup are done. In particular,
becoming root or dropping root should tweak capability sets.
Have the cred_setresuid() helper take a 'int force' argument,
which means the setuid has been authorized, so no need to
check CAP_SETUID. (Analogous for cred_setresgid, which will
also eventually be used).
Signed-off-by: Serge E. Hallyn <[email protected]>
Cc: Greg KH <[email protected]>
cc: [email protected]
Cc: Ashwin Ganti <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/staging/p9auth/p9auth.c | 12 ++++++++----
include/linux/cred.h | 8 ++++++--
kernel/cred.c | 11 ++++++-----
kernel/sys.c | 4 ++--
4 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/p9auth/p9auth.c b/drivers/staging/p9auth/p9auth.c
index 70ef45b..8f70daa 100644
--- a/drivers/staging/p9auth/p9auth.c
+++ b/drivers/staging/p9auth/p9auth.c
@@ -289,11 +289,15 @@ static ssize_t cap_write(struct file *filp, const char __user *buf,
retval = -ENOMEM;
goto out;
}
- new->uid = (uid_t) target_int;
- new->suid = new->fsuid = new->euid = new->uid;
- retval = commit_creds(new);
- if (retval)
+ retval = cred_setresuid(new, target_int,
+ target_int, target_int,
+ CRED_SETID_FORCE);
+ if (retval == 0)
+ commit_creds(new);
+ else {
+ abort_creds(new);
goto out;
+ }
/*
* Remove the capability from the list and
diff --git a/include/linux/cred.h b/include/linux/cred.h
index e35631e..6337e18 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -399,8 +399,12 @@ do { \
*(_fsgid) = __cred->fsgid; \
} while(0)
-int cred_setresuid(struct cred *new, uid_t ruid, uid_t euid, uid_t suid);
-int cred_setresgid(struct cred *new, gid_t rgid, gid_t egid, gid_t sgid);
+#define CRED_SETID_NOFORCE 0
+#define CRED_SETID_FORCE 1
+int cred_setresuid(struct cred *new, uid_t ruid, uid_t euid, uid_t suid,
+ int force);
+int cred_setresgid(struct cred *new, gid_t rgid, gid_t egid, gid_t sgid,
+ int force);
int cred_setfsuid(struct cred *new, uid_t uid, uid_t *old_fsuid);
int cred_setfsgid(struct cred *new, gid_t gid, gid_t *old_fsgid);
diff --git a/kernel/cred.c b/kernel/cred.c
index fb73c5b..16c0a04 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -780,7 +780,8 @@ int set_create_files_as(struct cred *new, struct inode *inode)
}
EXPORT_SYMBOL(set_create_files_as);
-int cred_setresuid(struct cred *new, uid_t ruid, uid_t euid, uid_t suid)
+int cred_setresuid(struct cred *new, uid_t ruid, uid_t euid, uid_t suid,
+ int force)
{
int retval;
const struct cred *old;
@@ -790,7 +791,7 @@ int cred_setresuid(struct cred *new, uid_t ruid, uid_t euid, uid_t suid)
return retval;
old = current_cred();
- if (!capable(CAP_SETUID)) {
+ if (!force && !capable(CAP_SETUID)) {
if (ruid != (uid_t) -1 && ruid != old->uid &&
ruid != old->euid && ruid != old->suid)
return -EPERM;
@@ -820,8 +821,8 @@ int cred_setresuid(struct cred *new, uid_t ruid, uid_t euid, uid_t suid)
}
EXPORT_SYMBOL_GPL(cred_setresuid);
-int cred_setresgid(struct cred *new, gid_t rgid, gid_t egid,
- gid_t sgid)
+int cred_setresgid(struct cred *new, gid_t rgid, gid_t egid, gid_t sgid,
+ int force)
{
const struct cred *old = current_cred();
int retval;
@@ -830,7 +831,7 @@ int cred_setresgid(struct cred *new, gid_t rgid, gid_t egid,
if (retval)
return retval;
- if (!capable(CAP_SETGID)) {
+ if (!force && !capable(CAP_SETGID)) {
if (rgid != (gid_t) -1 && rgid != old->gid &&
rgid != old->egid && rgid != old->sgid)
return -EPERM;
diff --git a/kernel/sys.c b/kernel/sys.c
index 1927edf..f6c2534 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -720,7 +720,7 @@ SYSCALL_DEFINE3(setresuid, uid_t, ruid, uid_t, euid, uid_t, suid)
if (!new)
return -ENOMEM;
- retval = cred_setresuid(new, ruid, euid, suid);
+ retval = cred_setresuid(new, ruid, euid, suid, CRED_SETID_NOFORCE);
if (retval == 0)
return commit_creds(new);
@@ -752,7 +752,7 @@ SYSCALL_DEFINE3(setresgid, gid_t, rgid, gid_t, egid, gid_t, sgid)
if (!new)
return -ENOMEM;
- retval = cred_setresgid(new, rgid, egid, sgid);
+ retval = cred_setresgid(new, rgid, egid, sgid, CRED_SETID_NOFORCE);
if (retval == 0)
return commit_creds(new);
--
1.6.1
From: Serge E. Hallyn <[email protected]>
Break the core functionality of set{fs,res}{u,g}id into cred_setX
which performs the access checks based on current_cred(), but performs
the requested change on a passed-in cred.
The helpers will be further broken down into permission-checks
and permission-less id-setting helpers. The id-setting helpers
for resuid and resgid will be used by p9auth to perform the
authorized id changes. Export the helpers, since p9auth can be
compiled as a module.
Really the setfs{u,g}id helper isn't needed, but move it as
well to keep the code consistent.
Signed-off-by: Serge E. Hallyn <[email protected]>
Cc: Greg KH <[email protected]>
cc: [email protected]
Cc: Ashwin Ganti <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
include/linux/cred.h | 8 +++
kernel/cred.c | 118 +++++++++++++++++++++++++++++++++++++++++++++
kernel/sys.c | 131 ++++++++------------------------------------------
3 files changed, 146 insertions(+), 111 deletions(-)
diff --git a/include/linux/cred.h b/include/linux/cred.h
index 4e3387a..e35631e 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -22,6 +22,9 @@ struct user_struct;
struct cred;
struct inode;
+/* defined in sys.c, used in cred_setresuid */
+extern int set_user(struct cred *new);
+
/*
* COW Supplementary groups list
*/
@@ -396,4 +399,9 @@ do { \
*(_fsgid) = __cred->fsgid; \
} while(0)
+int cred_setresuid(struct cred *new, uid_t ruid, uid_t euid, uid_t suid);
+int cred_setresgid(struct cred *new, gid_t rgid, gid_t egid, gid_t sgid);
+int cred_setfsuid(struct cred *new, uid_t uid, uid_t *old_fsuid);
+int cred_setfsgid(struct cred *new, gid_t gid, gid_t *old_fsgid);
+
#endif /* _LINUX_CRED_H */
diff --git a/kernel/cred.c b/kernel/cred.c
index dd76cfe..fb73c5b 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -780,6 +780,124 @@ int set_create_files_as(struct cred *new, struct inode *inode)
}
EXPORT_SYMBOL(set_create_files_as);
+int cred_setresuid(struct cred *new, uid_t ruid, uid_t euid, uid_t suid)
+{
+ int retval;
+ const struct cred *old;
+
+ retval = security_task_setuid(ruid, euid, suid, LSM_SETID_RES);
+ if (retval)
+ return retval;
+ old = current_cred();
+
+ if (!capable(CAP_SETUID)) {
+ if (ruid != (uid_t) -1 && ruid != old->uid &&
+ ruid != old->euid && ruid != old->suid)
+ return -EPERM;
+ if (euid != (uid_t) -1 && euid != old->uid &&
+ euid != old->euid && euid != old->suid)
+ return -EPERM;
+ if (suid != (uid_t) -1 && suid != old->uid &&
+ suid != old->euid && suid != old->suid)
+ return -EPERM;
+ }
+
+ if (ruid != (uid_t) -1) {
+ new->uid = ruid;
+ if (ruid != old->uid) {
+ retval = set_user(new);
+ if (retval < 0)
+ return retval;
+ }
+ }
+ if (euid != (uid_t) -1)
+ new->euid = euid;
+ if (suid != (uid_t) -1)
+ new->suid = suid;
+ new->fsuid = new->euid;
+
+ return security_task_fix_setuid(new, old, LSM_SETID_RES);
+}
+EXPORT_SYMBOL_GPL(cred_setresuid);
+
+int cred_setresgid(struct cred *new, gid_t rgid, gid_t egid,
+ gid_t sgid)
+{
+ const struct cred *old = current_cred();
+ int retval;
+
+ retval = security_task_setgid(rgid, egid, sgid, LSM_SETID_RES);
+ if (retval)
+ return retval;
+
+ if (!capable(CAP_SETGID)) {
+ if (rgid != (gid_t) -1 && rgid != old->gid &&
+ rgid != old->egid && rgid != old->sgid)
+ return -EPERM;
+ if (egid != (gid_t) -1 && egid != old->gid &&
+ egid != old->egid && egid != old->sgid)
+ return -EPERM;
+ if (sgid != (gid_t) -1 && sgid != old->gid &&
+ sgid != old->egid && sgid != old->sgid)
+ return -EPERM;
+ }
+
+ if (rgid != (gid_t) -1)
+ new->gid = rgid;
+ if (egid != (gid_t) -1)
+ new->egid = egid;
+ if (sgid != (gid_t) -1)
+ new->sgid = sgid;
+ new->fsgid = new->egid;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(cred_setresgid);
+
+int cred_setfsuid(struct cred *new, uid_t uid, uid_t *old_fsuid)
+{
+ const struct cred *old;
+
+ old = current_cred();
+ *old_fsuid = old->fsuid;
+
+ if (security_task_setuid(uid, (uid_t)-1, (uid_t)-1, LSM_SETID_FS) < 0)
+ return -EPERM;
+
+ if (uid == old->uid || uid == old->euid ||
+ uid == old->suid || uid == old->fsuid ||
+ capable(CAP_SETUID)) {
+ if (uid != *old_fsuid) {
+ new->fsuid = uid;
+ if (security_task_fix_setuid(new, old, LSM_SETID_FS) == 0)
+ return 0;
+ }
+ }
+ return -EPERM;
+}
+EXPORT_SYMBOL_GPL(cred_setfsuid);
+
+int cred_setfsgid(struct cred *new, gid_t gid, gid_t *old_fsgid)
+{
+ const struct cred *old;
+
+ old = current_cred();
+ *old_fsgid = old->fsgid;
+
+ if (security_task_setgid(gid, (gid_t)-1, (gid_t)-1, LSM_SETID_FS))
+ return -EPERM;
+
+ if (gid == old->gid || gid == old->egid ||
+ gid == old->sgid || gid == old->fsgid ||
+ capable(CAP_SETGID)) {
+ if (gid != *old_fsgid) {
+ new->fsgid = gid;
+ return 0;
+ }
+ }
+ return -EPERM;
+}
+EXPORT_SYMBOL_GPL(cred_setfsgid);
+
#ifdef CONFIG_DEBUG_CREDENTIALS
bool creds_are_invalid(const struct cred *cred)
diff --git a/kernel/sys.c b/kernel/sys.c
index 26a6b73..1927edf 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -561,11 +561,11 @@ error:
/*
* change the user struct in a credentials set to match the new UID
*/
-static int set_user(struct cred *new)
+int set_user(struct cred *new)
{
struct user_struct *new_user;
- new_user = alloc_uid(current_user_ns(), new->uid);
+ new_user = alloc_uid(new->user->user_ns, new->uid);
if (!new_user)
return -EAGAIN;
@@ -713,7 +713,6 @@ error:
*/
SYSCALL_DEFINE3(setresuid, uid_t, ruid, uid_t, euid, uid_t, suid)
{
- const struct cred *old;
struct cred *new;
int retval;
@@ -721,45 +720,10 @@ SYSCALL_DEFINE3(setresuid, uid_t, ruid, uid_t, euid, uid_t, suid)
if (!new)
return -ENOMEM;
- retval = security_task_setuid(ruid, euid, suid, LSM_SETID_RES);
- if (retval)
- goto error;
- old = current_cred();
+ retval = cred_setresuid(new, ruid, euid, suid);
+ if (retval == 0)
+ return commit_creds(new);
- retval = -EPERM;
- if (!capable(CAP_SETUID)) {
- if (ruid != (uid_t) -1 && ruid != old->uid &&
- ruid != old->euid && ruid != old->suid)
- goto error;
- if (euid != (uid_t) -1 && euid != old->uid &&
- euid != old->euid && euid != old->suid)
- goto error;
- if (suid != (uid_t) -1 && suid != old->uid &&
- suid != old->euid && suid != old->suid)
- goto error;
- }
-
- if (ruid != (uid_t) -1) {
- new->uid = ruid;
- if (ruid != old->uid) {
- retval = set_user(new);
- if (retval < 0)
- goto error;
- }
- }
- if (euid != (uid_t) -1)
- new->euid = euid;
- if (suid != (uid_t) -1)
- new->suid = suid;
- new->fsuid = new->euid;
-
- retval = security_task_fix_setuid(new, old, LSM_SETID_RES);
- if (retval < 0)
- goto error;
-
- return commit_creds(new);
-
-error:
abort_creds(new);
return retval;
}
@@ -781,43 +745,17 @@ SYSCALL_DEFINE3(getresuid, uid_t __user *, ruid, uid_t __user *, euid, uid_t __u
*/
SYSCALL_DEFINE3(setresgid, gid_t, rgid, gid_t, egid, gid_t, sgid)
{
- const struct cred *old;
struct cred *new;
int retval;
new = prepare_creds();
if (!new)
return -ENOMEM;
- old = current_cred();
- retval = security_task_setgid(rgid, egid, sgid, LSM_SETID_RES);
- if (retval)
- goto error;
+ retval = cred_setresgid(new, rgid, egid, sgid);
+ if (retval == 0)
+ return commit_creds(new);
- retval = -EPERM;
- if (!capable(CAP_SETGID)) {
- if (rgid != (gid_t) -1 && rgid != old->gid &&
- rgid != old->egid && rgid != old->sgid)
- goto error;
- if (egid != (gid_t) -1 && egid != old->gid &&
- egid != old->egid && egid != old->sgid)
- goto error;
- if (sgid != (gid_t) -1 && sgid != old->gid &&
- sgid != old->egid && sgid != old->sgid)
- goto error;
- }
-
- if (rgid != (gid_t) -1)
- new->gid = rgid;
- if (egid != (gid_t) -1)
- new->egid = egid;
- if (sgid != (gid_t) -1)
- new->sgid = sgid;
- new->fsgid = new->egid;
-
- return commit_creds(new);
-
-error:
abort_creds(new);
return retval;
}
@@ -843,35 +781,20 @@ SYSCALL_DEFINE3(getresgid, gid_t __user *, rgid, gid_t __user *, egid, gid_t __u
*/
SYSCALL_DEFINE1(setfsuid, uid_t, uid)
{
- const struct cred *old;
struct cred *new;
uid_t old_fsuid;
+ int retval;
new = prepare_creds();
if (!new)
return current_fsuid();
- old = current_cred();
- old_fsuid = old->fsuid;
-
- if (security_task_setuid(uid, (uid_t)-1, (uid_t)-1, LSM_SETID_FS) < 0)
- goto error;
-
- if (uid == old->uid || uid == old->euid ||
- uid == old->suid || uid == old->fsuid ||
- capable(CAP_SETUID)) {
- if (uid != old_fsuid) {
- new->fsuid = uid;
- if (security_task_fix_setuid(new, old, LSM_SETID_FS) == 0)
- goto change_okay;
- }
- }
-error:
- abort_creds(new);
- return old_fsuid;
+ retval = cred_setfsuid(new, uid, &old_fsuid);
+ if (retval == 0)
+ commit_creds(new);
+ else
+ abort_creds(new);
-change_okay:
- commit_creds(new);
return old_fsuid;
}
@@ -880,34 +803,20 @@ change_okay:
*/
SYSCALL_DEFINE1(setfsgid, gid_t, gid)
{
- const struct cred *old;
struct cred *new;
gid_t old_fsgid;
+ int retval;
new = prepare_creds();
if (!new)
return current_fsgid();
- old = current_cred();
- old_fsgid = old->fsgid;
-
- if (security_task_setgid(gid, (gid_t)-1, (gid_t)-1, LSM_SETID_FS))
- goto error;
-
- if (gid == old->gid || gid == old->egid ||
- gid == old->sgid || gid == old->fsgid ||
- capable(CAP_SETGID)) {
- if (gid != old_fsgid) {
- new->fsgid = gid;
- goto change_okay;
- }
- }
-error:
- abort_creds(new);
- return old_fsgid;
+ retval = cred_setfsgid(new, gid, &old_fsgid);
+ if (retval == 0)
+ commit_creds(new);
+ else
+ abort_creds(new);
-change_okay:
- commit_creds(new);
return old_fsgid;
}
--
1.6.1
From: Serge E. Hallyn <[email protected]>
A p9auth capability used to be 'old_uid@new_uid@random_string'. Now
it is 'old_uid@new_uid@new_gid@num_groups@g_1@...@g_n@random_string'.
After using the capability, credentials will be:
suid = old_uid;
sgid = old_gid;
uid = euid = fsuid = new_uid
gid = egid = fsgid = new_gid
auxiliary groups = g_1, g_2, ..., g_n
Signed-off-by: Serge E. Hallyn <[email protected]>
Cc: Greg KH <[email protected]>
cc: [email protected]
Cc: Ashwin Ganti <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/staging/p9auth/p9auth.c | 99 +++++++++++++++++++++++++--------------
1 files changed, 64 insertions(+), 35 deletions(-)
diff --git a/drivers/staging/p9auth/p9auth.c b/drivers/staging/p9auth/p9auth.c
index 50447d4..e94c4fe 100644
--- a/drivers/staging/p9auth/p9auth.c
+++ b/drivers/staging/p9auth/p9auth.c
@@ -167,8 +167,10 @@ static int cap_release(struct inode *inode, struct file *filp)
struct id_set {
char *source_user, *target_user;
- char *randstr;
uid_t old_uid, new_uid;
+ gid_t new_gid;
+ unsigned int ngroups;
+ struct group_info *newgroups;
char *full; /* The full entry which must be freed */
};
@@ -180,7 +182,8 @@ struct id_set {
*/
static int parse_user_capability(char *s, struct id_set *set)
{
- char *tmpu;
+ char *tmp, *tmpu;
+ int i, ret;
/*
* break the supplied string into tokens with @ as the
@@ -192,18 +195,45 @@ static int parse_user_capability(char *s, struct id_set *set)
if (!tmpu)
return -ENOMEM;
+ ret = -EINVAL;
set->source_user = strsep(&tmpu, "@");
set->target_user = strsep(&tmpu, "@");
- set->randstr = tmpu;
- if (!set->source_user || !set->target_user || !set->randstr) {
- kfree(set->full);
- return -EINVAL;
- }
+ tmp = strsep(&tmpu, "@");
+ if (!set->source_user || !set->target_user || !tmp)
+ goto out;
set->new_uid = simple_strtoul(set->target_user, NULL, 0);
set->old_uid = simple_strtoul(set->source_user, NULL, 0);
+ set->new_gid = simple_strtoul(tmp, NULL, 0);
- return 0;
+ tmp = strsep(&tmpu, "@");
+ if (!tmp)
+ goto out;
+ if (sscanf(tmp, "%d", &set->ngroups) != 1 || set->ngroups < 0)
+ goto out;
+
+ ret = -ENOMEM;
+ set->newgroups = groups_alloc(set->ngroups);
+ if (!set->newgroups)
+ goto out;
+
+ ret = -EINVAL;
+ for (i = 0; i < set->ngroups; i++) {
+ gid_t g;
+
+ tmp = strsep(&tmpu, "@");
+ if (!tmp || sscanf(tmp, "%d", &g) != 1) {
+ groups_free(set->newgroups);
+ goto out;
+ }
+ GROUP_AT(set->newgroups, i) = g;
+ }
+
+ ret = 0;
+
+out:
+ kfree(set->full);
+ return ret;
}
static int grant_id(struct id_set *set)
@@ -230,8 +260,13 @@ static int grant_id(struct id_set *set)
if (!new)
return -ENOMEM;
- ret = cred_setresuid(new, set->new_uid, set->new_uid, set->new_uid,
- CRED_SETID_FORCE);
+ ret = set_groups(new, set->newgroups);
+ if (!ret)
+ ret = cred_setresgid(new, set->new_gid, set->new_gid,
+ set->new_gid, CRED_SETID_FORCE);
+ if (!ret)
+ ret = cred_setresuid(new, set->new_uid, set->new_uid,
+ set->new_uid, CRED_SETID_FORCE);
if (ret == 0)
commit_creds(new);
else
@@ -260,12 +295,12 @@ static int add_caphash_entry(struct cap_dev *dev, char *user_buf, size_t count)
return 0;
}
-static int use_caphash_entry(struct cap_dev *dev, char *user_buf)
+static int use_caphash_entry(struct cap_dev *dev, char *ubuf)
{
struct cap_node *node;
struct id_set set;
- int ret, len, found = 0;
- char *tohash, *hashed;
+ int ret, found = 0;
+ char *hashed = NULL, *sep;
struct list_head *pos;
if (!cap_devices[0].head)
@@ -273,37 +308,30 @@ static int use_caphash_entry(struct cap_dev *dev, char *user_buf)
if (list_empty(&(cap_devices[0].head->list)))
return -EINVAL;
- ret = parse_user_capability(user_buf, &set);
+ ret = parse_user_capability(ubuf, &set);
if (ret)
return ret;
- /* hash the string user1@user2 with randstr as the key */
- len = strlen(set.source_user) + strlen(set.target_user) + 1;
- /* src, @, len, \0 */
- tohash = kzalloc(len+1, GFP_KERNEL);
- if (!tohash) {
- kfree(set.full);
- return -ENOMEM;
+ /*
+ * hash the string user1@user2@ngrp@grp... with randstr as the key
+ * XXX is there any vulnerability we're opening ourselves up to by
+ * not rebuilding the string from its components?
+ */
+ sep = strrchr(ubuf, '@');
+ if (sep) {
+ char *rand = sep + 1;
+ *sep = '\0';
+ hashed = cap_hash(ubuf, strlen(ubuf), rand, strlen(rand));
+ }
+ if (NULL == hashed) {
+ ret = -EINVAL;
+ goto out;
}
- strcat(tohash, set.source_user);
- strcat(tohash, "@");
- strcat(tohash, set.target_user);
- printk(KERN_ALERT "the source user is %s \n", set.source_user);
- printk(KERN_ALERT "the target user is %s \n", set.target_user);
- hashed = cap_hash(tohash, len, set.randstr, strlen(set.randstr));
- kfree(set.full);
- kfree(tohash);
- if (NULL == hashed)
- return -EFAULT;
/* Change the process's uid if the hash is present in the
* list of hashes
*/
list_for_each(pos, &(cap_devices->head->list)) {
- /*
- * Change the user id of the process if the hashes
- * match
- */
node = list_entry(pos, struct cap_node, list);
if (0 == memcmp(hashed, node->data, CAP_NODE_SIZE)) {
ret = grant_id(&set);
@@ -323,6 +351,7 @@ static int use_caphash_entry(struct cap_dev *dev, char *user_buf)
ret = -EFAULT;
}
out:
+ put_group_info(set.newgroups);
kfree(hashed);
return ret;
}
--
1.6.1
From: Serge E. Hallyn <[email protected]>
Move the code doing the actual uid change into its own helper
function. Next it will become a bit more complicated when I
add primary and auxiliary groups handling.
Split the handling of /dev/capuse and /dev/caphash writes into
their own functions.
Changelog:
Jan 3: fix memory leak in error case
Signed-off-by: Serge E. Hallyn <[email protected]>
Cc: Greg KH <[email protected]>
cc: [email protected]
Cc: Ashwin Ganti <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/staging/p9auth/p9auth.c | 308 +++++++++++++++++++++------------------
1 files changed, 168 insertions(+), 140 deletions(-)
diff --git a/drivers/staging/p9auth/p9auth.c b/drivers/staging/p9auth/p9auth.c
index fb27459..50447d4 100644
--- a/drivers/staging/p9auth/p9auth.c
+++ b/drivers/staging/p9auth/p9auth.c
@@ -165,26 +165,180 @@ static int cap_release(struct inode *inode, struct file *filp)
return 0;
}
+struct id_set {
+ char *source_user, *target_user;
+ char *randstr;
+ uid_t old_uid, new_uid;
+ char *full; /* The full entry which must be freed */
+};
+
+/*
+ * read an entry. For now it is
+ * source_user@target_user@rand
+ * Next it will become
+ * source_user@target_user@target_group@numgroups@grp1..@grpn@rand
+ */
+static int parse_user_capability(char *s, struct id_set *set)
+{
+ char *tmpu;
+
+ /*
+ * break the supplied string into tokens with @ as the
+ * delimiter If the string is "user1@user2@randomstring" we
+ * need to split it and hash 'user1@user2' using 'randomstring'
+ * as the key.
+ */
+ tmpu = set->full = kstrdup(s, GFP_KERNEL);
+ if (!tmpu)
+ return -ENOMEM;
+
+ set->source_user = strsep(&tmpu, "@");
+ set->target_user = strsep(&tmpu, "@");
+ set->randstr = tmpu;
+ if (!set->source_user || !set->target_user || !set->randstr) {
+ kfree(set->full);
+ return -EINVAL;
+ }
+
+ set->new_uid = simple_strtoul(set->target_user, NULL, 0);
+ set->old_uid = simple_strtoul(set->source_user, NULL, 0);
+
+ return 0;
+}
+
+static int grant_id(struct id_set *set)
+{
+ struct cred *new;
+ int ret;
+
+ /*
+ * Check whether the process writing to capuse
+ * is actually owned by the source owner
+ */
+ if (set->old_uid != current_uid()) {
+ printk(KERN_ALERT
+ "Process is not owned by the source user of the capability.\n");
+ return -EFAULT;
+ }
+
+ /*
+ * Change uid, euid, and fsuid. The suid remains for
+ * flexibility - though I'm torn as to the tradeoff of
+ * usefulness vs. danger in that.
+ */
+ new = prepare_creds();
+ if (!new)
+ return -ENOMEM;
+
+ ret = cred_setresuid(new, set->new_uid, set->new_uid, set->new_uid,
+ CRED_SETID_FORCE);
+ if (ret == 0)
+ commit_creds(new);
+ else
+ abort_creds(new);
+
+ return ret;
+}
+
+static int add_caphash_entry(struct cap_dev *dev, char *user_buf, size_t count)
+{
+ struct cap_node *node_ptr;
+
+ if (count > CAP_NODE_SIZE)
+ return -EINVAL;
+ if (!capable(CAP_GRANT_ID))
+ return -EPERM;
+ node_ptr = kmalloc(sizeof(struct cap_node), GFP_KERNEL);
+ if (!node_ptr)
+ return -ENOMEM;
+
+ printk(KERN_INFO "Capability being written to /dev/caphash : \n");
+ hexdump(user_buf, count);
+ memcpy(node_ptr->data, user_buf, count);
+ list_add(&(node_ptr->list), &(dev->head->list));
+
+ return 0;
+}
+
+static int use_caphash_entry(struct cap_dev *dev, char *user_buf)
+{
+ struct cap_node *node;
+ struct id_set set;
+ int ret, len, found = 0;
+ char *tohash, *hashed;
+ struct list_head *pos;
+
+ if (!cap_devices[0].head)
+ return -EINVAL;
+ if (list_empty(&(cap_devices[0].head->list)))
+ return -EINVAL;
+
+ ret = parse_user_capability(user_buf, &set);
+ if (ret)
+ return ret;
+
+ /* hash the string user1@user2 with randstr as the key */
+ len = strlen(set.source_user) + strlen(set.target_user) + 1;
+ /* src, @, len, \0 */
+ tohash = kzalloc(len+1, GFP_KERNEL);
+ if (!tohash) {
+ kfree(set.full);
+ return -ENOMEM;
+ }
+ strcat(tohash, set.source_user);
+ strcat(tohash, "@");
+ strcat(tohash, set.target_user);
+ printk(KERN_ALERT "the source user is %s \n", set.source_user);
+ printk(KERN_ALERT "the target user is %s \n", set.target_user);
+ hashed = cap_hash(tohash, len, set.randstr, strlen(set.randstr));
+ kfree(set.full);
+ kfree(tohash);
+ if (NULL == hashed)
+ return -EFAULT;
+
+ /* Change the process's uid if the hash is present in the
+ * list of hashes
+ */
+ list_for_each(pos, &(cap_devices->head->list)) {
+ /*
+ * Change the user id of the process if the hashes
+ * match
+ */
+ node = list_entry(pos, struct cap_node, list);
+ if (0 == memcmp(hashed, node->data, CAP_NODE_SIZE)) {
+ ret = grant_id(&set);
+ if (ret < 0)
+ goto out;
+
+ /* Capability may only be used once */
+ list_del(pos);
+ kfree(node);
+ found = 1;
+ break;
+ }
+ }
+ if (!found) {
+ printk(KERN_ALERT
+ "Invalid capabiliy written to /dev/capuse\n");
+ ret = -EFAULT;
+ }
+out:
+ kfree(hashed);
+ return ret;
+}
+
static ssize_t cap_write(struct file *filp, const char __user *buf,
size_t count, loff_t *f_pos)
{
- struct cap_node *node_ptr, *tmp;
- struct list_head *pos;
struct cap_dev *dev = filp->private_data;
ssize_t retval = -ENOMEM;
- struct cred *new;
- int len, target_int, source_int, flag = 0;
- char *user_buf, *user_buf_running, *source_user, *target_user,
- *rand_str, *hash_str, *result;
+ char *user_buf;
if (down_interruptible(&dev->sem))
return -ERESTARTSYS;
- user_buf_running = NULL;
- hash_str = NULL;
- node_ptr = kmalloc(sizeof(struct cap_node), GFP_KERNEL);
user_buf = kzalloc(count+1, GFP_KERNEL);
- if (!node_ptr || !user_buf)
+ if (!user_buf)
goto out;
if (copy_from_user(user_buf, buf, count)) {
@@ -196,134 +350,11 @@ static ssize_t cap_write(struct file *filp, const char __user *buf,
* If the minor number is 0 ( /dev/caphash ) then simply add the
* hashed capability supplied by the user to the list of hashes
*/
- if (0 == iminor(filp->f_dentry->d_inode)) {
- if (count > CAP_NODE_SIZE) {
- retval = -EINVAL;
- goto out;
- }
- if (!capable(CAP_GRANT_ID)) {
- retval = -EPERM;
- goto out;
- }
- printk(KERN_INFO "Capability being written to /dev/caphash : \n");
- hexdump(user_buf, count);
- memcpy(node_ptr->data, user_buf, count);
- list_add(&(node_ptr->list), &(dev->head->list));
- node_ptr = NULL;
- } else {
- char *tmpu;
- if (!cap_devices[0].head ||
- list_empty(&(cap_devices[0].head->list))) {
- retval = -EINVAL;
- goto out;
- }
- /*
- * break the supplied string into tokens with @ as the
- * delimiter If the string is "user1@user2@randomstring" we
- * need to split it and hash 'user1@user2' using 'randomstring'
- * as the key.
- */
- tmpu = user_buf_running = kstrdup(user_buf, GFP_KERNEL);
- source_user = strsep(&tmpu, "@");
- target_user = strsep(&tmpu, "@");
- rand_str = tmpu;
- if (!source_user || !target_user || !rand_str) {
- retval = -EINVAL;
- goto out;
- }
+ if (0 == iminor(filp->f_dentry->d_inode))
+ retval = add_caphash_entry(dev, user_buf, count);
+ else
+ retval = use_caphash_entry(dev, user_buf);
- /* hash the string user1@user2 with rand_str as the key */
- len = strlen(source_user) + strlen(target_user) + 1;
- /* src, @, len, \0 */
- hash_str = kzalloc(len+1, GFP_KERNEL);
- strcat(hash_str, source_user);
- strcat(hash_str, "@");
- strcat(hash_str, target_user);
-
- printk(KERN_ALERT "the source user is %s \n", source_user);
- printk(KERN_ALERT "the target user is %s \n", target_user);
-
- result = cap_hash(hash_str, len, rand_str, strlen(rand_str));
- if (NULL == result) {
- retval = -EFAULT;
- goto out;
- }
- memcpy(node_ptr->data, result, CAP_NODE_SIZE); /* why? */
- /* Change the process's uid if the hash is present in the
- * list of hashes
- */
- list_for_each(pos, &(cap_devices->head->list)) {
- /*
- * Change the user id of the process if the hashes
- * match
- */
- if (0 ==
- memcmp(result,
- list_entry(pos, struct cap_node,
- list)->data,
- CAP_NODE_SIZE)) {
- target_int = (unsigned int)
- simple_strtol(target_user, NULL, 0);
- source_int = (unsigned int)
- simple_strtol(source_user, NULL, 0);
- flag = 1;
-
- /*
- * Check whether the process writing to capuse
- * is actually owned by the source owner
- */
- if (source_int != current_uid()) {
- printk(KERN_ALERT
- "Process is not owned by the source user of the capability.\n");
- retval = -EFAULT;
- goto out;
- }
- /*
- * Change all uids. It might be useful to
- * keep suid unchanged, however that will
- * mean that changing from uid=0 to uid=!0
- * pP is not emptied (only pE is), and when
- * changing from uid=!0 to uid=0, sets are
- * not filled. They will be correct after
- * the next exec, but this is IMO not
- * sufficient. So change all uids.
- */
- new = prepare_creds();
- if (!new) {
- retval = -ENOMEM;
- goto out;
- }
- retval = cred_setresuid(new, target_int,
- target_int, target_int,
- CRED_SETID_FORCE);
- if (retval == 0)
- commit_creds(new);
- else {
- abort_creds(new);
- goto out;
- }
-
- /*
- * Remove the capability from the list and
- * break
- */
- tmp = list_entry(pos, struct cap_node, list);
- list_del(pos);
- kfree(tmp);
- break;
- }
- }
- if (0 == flag) {
- /*
- * The capability is not present in the list of the
- * hashes stored, hence return failure
- */
- printk(KERN_ALERT
- "Invalid capabiliy written to /dev/capuse \n");
- retval = -EFAULT;
- goto out;
- }
- }
*f_pos += count;
retval = count;
/* update the size */
@@ -331,10 +362,7 @@ static ssize_t cap_write(struct file *filp, const char __user *buf,
dev->size = *f_pos;
out:
- kfree(node_ptr);
kfree(user_buf);
- kfree(user_buf_running);
- kfree(hash_str);
up(&dev->sem);
return retval;
}
--
1.6.1