[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH 2/4] Allow the process owner to perform all privileged operations
|
From: |
Sergey Bugaev |
|
Subject: |
[PATCH 2/4] Allow the process owner to perform all privileged operations |
|
Date: |
Mon, 26 Jun 2023 02:11:35 +0300 |
The user already has full access to our task, and the same kind of
access to the file system image (if any) as our task does, we're not
buying any additional security by disallowing them access.
In practice, this allows creating and arbitrarily modifying ext2
filesystem images owned by an otherwise unprivileged user.
Allowing this was already the intention, see isroot / _is_privileged ()
in libtrivfs, and the existing check in fshelp_iscontroller (), but it
hasn't been done consistently.
Also, when running unprivileged and trying to spawn a nested root-owned
translator, start it as our user and not as root. Trying to start it as
root fails with EPERM anyway, while running it as the user has a good
chance of working, or will print a more meaningful error than just EPERM
from dir_lookup -- perhaps the translator actually needs root access for
something, and will fail at trying to access that when run unprivileged.
---
Please review carefully and do test. I have tested, too, and it seems to
work as expected for me. This is an important security thing, we
wouldn't want to get anything wrong here.
ext2fs/storeinfo.c | 1 +
libdiskfs/file-chflags.c | 5 +++--
libdiskfs/file-chmod.c | 3 ++-
libdiskfs/file-chown.c | 3 ++-
libdiskfs/file-getfh.c | 3 ++-
libfshelp/exec-reauth.c | 12 ++++++++----
libfshelp/fetch-root.c | 15 +++++++++++++++
libfshelp/perms-access.c | 7 ++++---
libfshelp/perms-checkdirmod.c | 4 ++--
libfshelp/perms-isowner.c | 4 +++-
libiohelp/iouser-restrict.c | 4 +++-
11 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/ext2fs/storeinfo.c b/ext2fs/storeinfo.c
index 40e0f603..7664baf9 100644
--- a/ext2fs/storeinfo.c
+++ b/ext2fs/storeinfo.c
@@ -110,6 +110,7 @@ diskfs_S_file_get_storage_info (struct protid *cred,
err = store_remap (file_store, runs, num_runs, &file_store);
if (!err
&& !idvec_contains (cred->user->uids, 0)
+ && !idvec_contains (cred->user->uids, geteuid ())
&& !store_is_securely_returnable (file_store, cred->po->openstat))
{
err = store_set_flags (file_store, STORE_INACTIVE);
diff --git a/libdiskfs/file-chflags.c b/libdiskfs/file-chflags.c
index a29ff07c..03a12a0a 100644
--- a/libdiskfs/file-chflags.c
+++ b/libdiskfs/file-chflags.c
@@ -29,7 +29,8 @@ diskfs_S_file_chflags (struct protid *cred,
/* Only root is allowed to change the high 16
bits. */
if ((HI (flags) != HI (np->dn_stat.st_flags))
- && ! idvec_contains (cred->user->uids, 0))
+ && ! idvec_contains (cred->user->uids, 0)
+ && ! idvec_contains (cred->user->uids, geteuid ()))
return EPERM;
err = fshelp_isowner (&np->dn_stat, cred->user);
@@ -41,7 +42,7 @@ diskfs_S_file_chflags (struct protid *cred,
np->dn_set_ctime = 1;
}
if (!err && np->filemod_reqs)
- diskfs_notice_filechange(np, FILE_CHANGED_META,
+ diskfs_notice_filechange(np, FILE_CHANGED_META,
0, 0);
}));
#undef HI
diff --git a/libdiskfs/file-chmod.c b/libdiskfs/file-chmod.c
index df262ea3..f906affb 100644
--- a/libdiskfs/file-chmod.c
+++ b/libdiskfs/file-chmod.c
@@ -29,7 +29,8 @@ diskfs_S_file_chmod (struct protid *cred,
({
if (!(err = fshelp_isowner (&np->dn_stat, cred->user)))
{
- if (!idvec_contains (cred->user->uids, 0))
+ if (!idvec_contains (cred->user->uids, 0)
+ && !idvec_contains (cred->user->uids, geteuid
()))
{
if (!S_ISDIR (np->dn_stat.st_mode))
mode &= ~S_ISVTX;
diff --git a/libdiskfs/file-chown.c b/libdiskfs/file-chown.c
index ecb851f2..c62aac44 100644
--- a/libdiskfs/file-chown.c
+++ b/libdiskfs/file-chown.c
@@ -35,7 +35,8 @@ diskfs_S_file_chown (struct protid *cred,
&& !idvec_contains (cred->user->uids, uid))
|| (gid != (gid_t) -1
&& !idvec_contains (cred->user->gids, gid)))
- && !idvec_contains (cred->user->uids, 0)))
+ && !idvec_contains (cred->user->uids, 0)
+ && !idvec_contains (cred->user->uids, geteuid ())))
err = EPERM;
else
{
diff --git a/libdiskfs/file-getfh.c b/libdiskfs/file-getfh.c
index db120813..4edce83d 100644
--- a/libdiskfs/file-getfh.c
+++ b/libdiskfs/file-getfh.c
@@ -35,7 +35,8 @@ diskfs_S_file_getfh (struct protid *cred, data_t *fh,
if (! cred)
return EOPNOTSUPP;
- if (! idvec_contains (cred->user->uids, 0))
+ if (! idvec_contains (cred->user->uids, 0)
+ && ! idvec_contains (cred->user->uids, geteuid ()))
return EPERM;
assert_backtrace (sizeof *f == sizeof f->bytes);
diff --git a/libfshelp/exec-reauth.c b/libfshelp/exec-reauth.c
index 6b99175a..20875a9a 100644
--- a/libfshelp/exec-reauth.c
+++ b/libfshelp/exec-reauth.c
@@ -23,6 +23,7 @@
#include <hurd/process.h>
#include <hurd/auth.h>
#include <idvec.h>
+#include <unistd.h>
#include "fshelp.h"
@@ -69,8 +70,10 @@ fshelp_exec_reauth (int suid, uid_t uid, int sgid, gid_t gid,
if (err)
goto abandon_suid;
- already_root =
- idvec_contains (eff_uids, 0) || idvec_contains (avail_uids, 0);
+ already_root = (idvec_contains (eff_uids, 0)
+ || idvec_contains (avail_uids, 0)
+ || idvec_contains (eff_uids, geteuid ())
+ || idvec_contains (avail_uids, geteuid ()));
/* If the user's auth port is fraudulent, then these values will be
wrong. No matter; we will repeat these checks using secure id sets
@@ -105,7 +108,8 @@ fshelp_exec_reauth (int suid, uid_t uid, int sgid, gid_t
gid,
/* Now add some from a source we trust. */
err = (*get_file_ids)(eff_uids, eff_gids);
- already_root = idvec_contains (eff_uids, 0);
+ already_root = (idvec_contains (eff_uids, 0)
+ || idvec_contains (eff_uids, geteuid ()));
if (suid && !err)
err = idvec_setid (eff_uids, avail_uids, uid, &_secure);
if (sgid && !err)
@@ -133,7 +137,7 @@ fshelp_exec_reauth (int suid, uid_t uid, int sgid, gid_t
gid,
/* Try proc_setowner () for compatibility with older proc server. */
err = proc_setowner (ports[INIT_PORT_PROC],
- eff_uids->num > 0 ? eff_uids->ids[0] : 0,
+ eff_uids->num > 0 ? eff_uids->ids[0] : geteuid (),
!eff_uids->num);
if (err == EOPNOTSUPP)
err = 0;
diff --git a/libfshelp/fetch-root.c b/libfshelp/fetch-root.c
index f3ae0ee9..5e64be88 100644
--- a/libfshelp/fetch-root.c
+++ b/libfshelp/fetch-root.c
@@ -120,9 +120,24 @@ fshelp_fetch_root (struct transbox *box, void *cookie,
{
uid_t uidarray[2] = { uid, uid };
gid_t gidarray[2] = { gid, gid };
+ makeauth:
err = auth_makeauth (ourauth, 0, MACH_MSG_TYPE_COPY_SEND, 0,
uidarray, 1, uidarray, 2,
gidarray, 1, gidarray, 2, &newauth);
+
+ if (err == EPERM && uid == 0)
+ {
+ /* Trying to spawn a root-owned translator, but got EPERM
+ on reauthentication. Spawn it as our user instead. We only
+ do this for root-owned translators currently, since those
+ are presumably trusted enough.
+ TODO: Consider running non-root-owned translators as
+ nullauth. */
+ uid = uidarray[0] = uidarray[1] = getuid ();
+ gid = gidarray[0] = gidarray[1] = getgid ();
+ assert_backtrace (uid != 0);
+ goto makeauth;
+ }
if (err)
goto return_error;
}
diff --git a/libfshelp/perms-access.c b/libfshelp/perms-access.c
index 67e52812..9a5b9ddd 100644
--- a/libfshelp/perms-access.c
+++ b/libfshelp/perms-access.c
@@ -1,4 +1,4 @@
-/*
+/*
Copyright (C) 1999 Free Software Foundation, Inc.
Written by Thomas Bushnell, BSG.
@@ -18,7 +18,7 @@
along with this program; if not, write to the Free Software
Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111, USA. */
-
+#include <unistd.h>
#include "fshelp.h"
/* Check to see whether the user USER can operate on a file identified
@@ -29,7 +29,8 @@ error_t
fshelp_access (struct stat *st, int op, struct iouser *user)
{
int gotit;
- if (idvec_contains (user->uids, 0))
+ if (idvec_contains (user->uids, 0)
+ || idvec_contains (user->uids, geteuid ()))
gotit = (op != S_IEXEC) || !S_ISREG(st->st_mode) || (st->st_mode &
(S_IXUSR | S_IXGRP | S_IXOTH));
else if (user->uids->num == 0 && (st->st_mode & S_IUSEUNK))
gotit = st->st_mode & (op << S_IUNKSHIFT);
diff --git a/libfshelp/perms-checkdirmod.c b/libfshelp/perms-checkdirmod.c
index 823c9f63..9df78726 100644
--- a/libfshelp/perms-checkdirmod.c
+++ b/libfshelp/perms-checkdirmod.c
@@ -1,4 +1,4 @@
-/*
+/*
Copyright (C) 1999 Free Software Foundation, Inc.
Written by Thomas Bushnell, BSG.
@@ -18,7 +18,7 @@
along with this program; if not, write to the Free Software
Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111, USA. */
-
+#include <unistd.h>
#include "fshelp.h"
/* Check to see whether USER is allowed to modify DIR with respect to
diff --git a/libfshelp/perms-isowner.c b/libfshelp/perms-isowner.c
index d1975993..8785607e 100644
--- a/libfshelp/perms-isowner.c
+++ b/libfshelp/perms-isowner.c
@@ -1,4 +1,4 @@
-/*
+/*
Copyright (C) 1999 Free Software Foundation, Inc.
Written by Thomas Bushnell, BSG.
@@ -18,6 +18,7 @@
along with this program; if not, write to the Free Software
Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111, USA. */
+#include <unistd.h>
#include "fshelp.h"
/* Check to see whether USER should be considered the owner of the
@@ -31,6 +32,7 @@ fshelp_isowner (struct stat *st, struct iouser *user)
their user ID. */
if (idvec_contains (user->uids, st->st_uid)
|| idvec_contains (user->uids, 0)
+ || idvec_contains (user->uids, geteuid ())
|| (idvec_contains (user->gids, st->st_gid)
&& idvec_contains (user->uids, st->st_gid)))
return 0;
diff --git a/libiohelp/iouser-restrict.c b/libiohelp/iouser-restrict.c
index 184061e1..150d36bf 100644
--- a/libiohelp/iouser-restrict.c
+++ b/libiohelp/iouser-restrict.c
@@ -15,6 +15,7 @@
along with this program; if not, write to the Free Software
Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */
+#include <unistd.h>
#include "iohelp.h"
/* Tell if the array LIST (of size N) contains a member equal to QUERY. */
@@ -34,7 +35,8 @@ iohelp_restrict_iouser (struct iouser **new_user,
const uid_t *uids, int nuids,
const gid_t *gids, int ngids)
{
- if (idvec_contains (old_user->uids, 0))
+ if (idvec_contains (old_user->uids, 0)
+ || idvec_contains (old_user->uids, geteuid ()))
/* OLD_USER has root access, and so may use any ids. */
return iohelp_create_complex_iouser (new_user, uids, nuids, gids, ngids);
else
--
2.41.0