| Message ID | 1356037019-19426-1-git-send-email-laurent@vivier.eu |
|---|---|
| State | New |
| Headers | show |
Ping ! Le jeudi 20 décembre 2012 à 21:56 +0100, Laurent Vivier a écrit : > When qemu-linux-user is used in a linux container or chroot, > if it needs to load binaries with SUID/SGID bits, it needs to > have root rights to be able to change UID/GID. To do that, we > need to install it with SUID bits and root owner. > Then, if the SUID bit is not set on the binary to load, > qemu will set its UID to the saved UID (the current user ID). > > To be able to retrieve unsecure environment variables (LD_PRELOAD, > LD_LIBRARY_PATH) with SUID bit, we need to disable "unsetenv()". > Otherwise libc unsets these variables before entering in main() > > To enable this feature, add "--suid-able" to the configure parameters. > > You can check all is working fine with : > > - install qemu-<arch> in your <arch> root filesystem environment > and chown root:root ... and chmow +s ... > > - check sudo in this environment (chroot or linux container) : > > laurent@m68k $ id > uid=1000(laurent) gid=1000(laurent) groups=1000(laurent) > laurent@m68k $ sudo id > Password: > uid=0(root) gid=0(root) groups=0(root) > > - check LD_PRELOAD is available (debian fakeroot is my testcase) : > > laurent@m68k $ fakeroot id > uid=0(root) gid=0(root) groups=1000(laurent) > laurent@m68k $ rm -f toto > laurent@m68k $ fakeroot > root@m68k # touch toto > root@m68k # ls -l toto > -rw-r--r-- 1 root root 0 2012-12-18 22:50 toto > root@m68k # exit > exit > root@m68k # ls -l toto > -rw-r--r-- 1 laurent laurent 0 2012-12-18 22:50 toto > > Signed-off-by: Laurent Vivier <laurent@vivier.eu> > --- > configure | 15 +++++++++++++++ > linux-user/linuxload.c | 16 +++++++++++----- > linux-user/main.c | 20 ++++++++++++++++++++ > 3 files changed, 46 insertions(+), 5 deletions(-) > > diff --git a/configure b/configure > index b101d5c..2322387 100755 > --- a/configure > +++ b/configure > @@ -111,6 +111,7 @@ source_path=`dirname "$0"` > cpu="" > interp_prefix="/usr/gnemul/qemu-%M" > static="no" > +suidable="no" > cross_prefix="" > audio_drv_list="" > audio_card_list="ac97 es1370 sb16 hda" > @@ -624,6 +625,9 @@ for opt do > LDFLAGS="-static $LDFLAGS" > QEMU_PKG_CONFIG_FLAGS="--static $QEMU_PKG_CONFIG_FLAGS" > ;; > + --suid-able) > + suidable="yes" > + ;; > --mandir=*) mandir="$optarg" > ;; > --bindir=*) bindir="$optarg" > @@ -885,6 +889,11 @@ for opt do > esac > done > > +if test "$suidable" = "yes" -a "$static" = "no" ; then > + echo "ERROR: --suid-able needs --static" > + exit 1 > +fi > + > case "$cpu" in > sparc) > LDFLAGS="-m32 $LDFLAGS" > @@ -1014,6 +1023,7 @@ echo " --install=INSTALL use specified install [$install]" > echo " --python=PYTHON use specified python [$python]" > echo " --smbd=SMBD use specified smbd [$smbd]" > echo " --static enable static build [$static]" > +echo " --suid-able allow to use qemu with SUID bit [$suidable]" > echo " --mandir=PATH install man pages in PATH" > echo " --datadir=PATH install firmware in PATH$confsuffix" > echo " --docdir=PATH install documentation in PATH$confsuffix" > @@ -3196,6 +3206,7 @@ echo "sparse enabled $sparse" > echo "strip binaries $strip_opt" > echo "profiler $profiler" > echo "static build $static" > +echo "suid-able $suidable" > echo "-Werror enabled $werror" > if test "$darwin" = "yes" ; then > echo "Cocoa support $cocoa" > @@ -4160,6 +4171,10 @@ if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then > ;; > esac > fi > +if test "$target_linux_user" = "yes" -a "$suidable" = "yes" ; then > + ldflags="$ldflags -Wl,--wrap=__unsetenv" > + echo "CONFIG_SUIDABLE=y" >> $config_target_mak > +fi > > echo "LDFLAGS+=$ldflags" >> $config_target_mak > echo "QEMU_CFLAGS+=$cflags" >> $config_target_mak > diff --git a/linux-user/linuxload.c b/linux-user/linuxload.c > index 381ab89..783afce 100644 > --- a/linux-user/linuxload.c > +++ b/linux-user/linuxload.c > @@ -58,11 +58,6 @@ static int prepare_binprm(struct linux_binprm *bprm) > bprm->e_uid = geteuid(); > bprm->e_gid = getegid(); > > - /* Set-uid? */ > - if(mode & S_ISUID) { > - bprm->e_uid = st.st_uid; > - } > - > /* Set-gid? */ > /* > * If setgid is set but no group execute bit then this > @@ -72,6 +67,17 @@ static int prepare_binprm(struct linux_binprm *bprm) > if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) { > bprm->e_gid = st.st_gid; > } > +#if defined(CONFIG_SUIDABLE) > + setresgid(getgid(), bprm->e_gid, bprm->e_gid); > +#endif > + > + /* Set-uid? */ > + if(mode & S_ISUID) { > + bprm->e_uid = st.st_uid; > + } > +#if defined(CONFIG_SUIDABLE) > + setresuid(getuid(), bprm->e_uid, bprm->e_uid); > +#endif > > retval = read(bprm->fd, bprm->buf, BPRM_BUF_SIZE); > if (retval < 0) { > diff --git a/linux-user/main.c b/linux-user/main.c > index f6c4c8d..dd9dd24 100644 > --- a/linux-user/main.c > +++ b/linux-user/main.c > @@ -3385,6 +3385,20 @@ static int parse_args(int argc, char **argv) > return optind; > } > > +#if defined(CONFIG_SUIDABLE) > +static int allow_unsetenv = 0; > + > +int __wrap___unsetenv(const char *name); > +int __real___unsetenv(const char *name); > +int > +__wrap___unsetenv(const char *name) > +{ > + if (!allow_unsetenv) > + return 0; > + return __real___unsetenv(name); > +} > +#endif > + > int main(int argc, char **argv, char **envp) > { > const char *log_file = DEBUG_LOGFILE; > @@ -3400,6 +3414,12 @@ int main(int argc, char **argv, char **envp) > int i; > int ret; > > +#if defined(CONFIG_SUIDABLE) > + allow_unsetenv = 1; > + seteuid(getuid()); > + setegid(getgid()); > +#endif > + > module_call_init(MODULE_INIT_QOM); > > qemu_cache_utils_init(envp);
On 20 December 2012 20:56, Laurent Vivier <laurent@vivier.eu> wrote: > When qemu-linux-user is used in a linux container or chroot, > if it needs to load binaries with SUID/SGID bits, it needs to > have root rights to be able to change UID/GID. To do that, we > need to install it with SUID bits and root owner. I suspect a suid qemu binary is a big fat security hole... > Then, if the SUID bit is not set on the binary to load, > qemu will set its UID to the saved UID (the current user ID). > > To be able to retrieve unsecure environment variables (LD_PRELOAD, > LD_LIBRARY_PATH) with SUID bit, we need to disable "unsetenv()". > Otherwise libc unsets these variables before entering in main() This is basically deliberately disabling a glibc security check. Needs careful thought and review (which I don't have time for just now I'm afraid) before this can be committed. -- PMM
Le mercredi 02 janvier 2013 à 01:16 +0000, Peter Maydell a écrit : > On 20 December 2012 20:56, Laurent Vivier <laurent@vivier.eu> wrote: > > When qemu-linux-user is used in a linux container or chroot, > > if it needs to load binaries with SUID/SGID bits, it needs to > > have root rights to be able to change UID/GID. To do that, we > > need to install it with SUID bits and root owner. > > I suspect a suid qemu binary is a big fat security hole... This is why this feature is disabled by default and must be enabled with configure. Moreover this is only for qemu-linux-user and the first thing done in main() is to set euid/egid to real uid/gid. > > Then, if the SUID bit is not set on the binary to load, > > qemu will set its UID to the saved UID (the current user ID). > > > > To be able to retrieve unsecure environment variables (LD_PRELOAD, > > LD_LIBRARY_PATH) with SUID bit, we need to disable "unsetenv()". > > Otherwise libc unsets these variables before entering in main() > > This is basically deliberately disabling a glibc security check. Yes, but this security check is mainly to avoid to load unsecure library. To avoid this too, we force the "--static" mode. This is not perfect but bring to qemu-linux-user an interesting feature. > Needs careful thought and review (which I don't have time for > just now I'm afraid) before this can be committed. > > -- PMM
In fact, this patch is useless as binfmt_mist provides a flag to manage credentials and security token. A new patch follows... Regards, Laurent Le jeudi 20 décembre 2012 à 21:56 +0100, Laurent Vivier a écrit : > When qemu-linux-user is used in a linux container or chroot, > if it needs to load binaries with SUID/SGID bits, it needs to > have root rights to be able to change UID/GID. To do that, we > need to install it with SUID bits and root owner. > Then, if the SUID bit is not set on the binary to load, > qemu will set its UID to the saved UID (the current user ID). > > To be able to retrieve unsecure environment variables (LD_PRELOAD, > LD_LIBRARY_PATH) with SUID bit, we need to disable "unsetenv()". > Otherwise libc unsets these variables before entering in main() > > To enable this feature, add "--suid-able" to the configure parameters. > > You can check all is working fine with : > > - install qemu-<arch> in your <arch> root filesystem environment > and chown root:root ... and chmow +s ... > > - check sudo in this environment (chroot or linux container) : > > laurent@m68k $ id > uid=1000(laurent) gid=1000(laurent) groups=1000(laurent) > laurent@m68k $ sudo id > Password: > uid=0(root) gid=0(root) groups=0(root) > > - check LD_PRELOAD is available (debian fakeroot is my testcase) : > > laurent@m68k $ fakeroot id > uid=0(root) gid=0(root) groups=1000(laurent) > laurent@m68k $ rm -f toto > laurent@m68k $ fakeroot > root@m68k # touch toto > root@m68k # ls -l toto > -rw-r--r-- 1 root root 0 2012-12-18 22:50 toto > root@m68k # exit > exit > root@m68k # ls -l toto > -rw-r--r-- 1 laurent laurent 0 2012-12-18 22:50 toto > > Signed-off-by: Laurent Vivier <laurent@vivier.eu> > --- > configure | 15 +++++++++++++++ > linux-user/linuxload.c | 16 +++++++++++----- > linux-user/main.c | 20 ++++++++++++++++++++ > 3 files changed, 46 insertions(+), 5 deletions(-) > > diff --git a/configure b/configure > index b101d5c..2322387 100755 > --- a/configure > +++ b/configure > @@ -111,6 +111,7 @@ source_path=`dirname "$0"` > cpu="" > interp_prefix="/usr/gnemul/qemu-%M" > static="no" > +suidable="no" > cross_prefix="" > audio_drv_list="" > audio_card_list="ac97 es1370 sb16 hda" > @@ -624,6 +625,9 @@ for opt do > LDFLAGS="-static $LDFLAGS" > QEMU_PKG_CONFIG_FLAGS="--static $QEMU_PKG_CONFIG_FLAGS" > ;; > + --suid-able) > + suidable="yes" > + ;; > --mandir=*) mandir="$optarg" > ;; > --bindir=*) bindir="$optarg" > @@ -885,6 +889,11 @@ for opt do > esac > done > > +if test "$suidable" = "yes" -a "$static" = "no" ; then > + echo "ERROR: --suid-able needs --static" > + exit 1 > +fi > + > case "$cpu" in > sparc) > LDFLAGS="-m32 $LDFLAGS" > @@ -1014,6 +1023,7 @@ echo " --install=INSTALL use specified install [$install]" > echo " --python=PYTHON use specified python [$python]" > echo " --smbd=SMBD use specified smbd [$smbd]" > echo " --static enable static build [$static]" > +echo " --suid-able allow to use qemu with SUID bit [$suidable]" > echo " --mandir=PATH install man pages in PATH" > echo " --datadir=PATH install firmware in PATH$confsuffix" > echo " --docdir=PATH install documentation in PATH$confsuffix" > @@ -3196,6 +3206,7 @@ echo "sparse enabled $sparse" > echo "strip binaries $strip_opt" > echo "profiler $profiler" > echo "static build $static" > +echo "suid-able $suidable" > echo "-Werror enabled $werror" > if test "$darwin" = "yes" ; then > echo "Cocoa support $cocoa" > @@ -4160,6 +4171,10 @@ if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then > ;; > esac > fi > +if test "$target_linux_user" = "yes" -a "$suidable" = "yes" ; then > + ldflags="$ldflags -Wl,--wrap=__unsetenv" > + echo "CONFIG_SUIDABLE=y" >> $config_target_mak > +fi > > echo "LDFLAGS+=$ldflags" >> $config_target_mak > echo "QEMU_CFLAGS+=$cflags" >> $config_target_mak > diff --git a/linux-user/linuxload.c b/linux-user/linuxload.c > index 381ab89..783afce 100644 > --- a/linux-user/linuxload.c > +++ b/linux-user/linuxload.c > @@ -58,11 +58,6 @@ static int prepare_binprm(struct linux_binprm *bprm) > bprm->e_uid = geteuid(); > bprm->e_gid = getegid(); > > - /* Set-uid? */ > - if(mode & S_ISUID) { > - bprm->e_uid = st.st_uid; > - } > - > /* Set-gid? */ > /* > * If setgid is set but no group execute bit then this > @@ -72,6 +67,17 @@ static int prepare_binprm(struct linux_binprm *bprm) > if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) { > bprm->e_gid = st.st_gid; > } > +#if defined(CONFIG_SUIDABLE) > + setresgid(getgid(), bprm->e_gid, bprm->e_gid); > +#endif > + > + /* Set-uid? */ > + if(mode & S_ISUID) { > + bprm->e_uid = st.st_uid; > + } > +#if defined(CONFIG_SUIDABLE) > + setresuid(getuid(), bprm->e_uid, bprm->e_uid); > +#endif > > retval = read(bprm->fd, bprm->buf, BPRM_BUF_SIZE); > if (retval < 0) { > diff --git a/linux-user/main.c b/linux-user/main.c > index f6c4c8d..dd9dd24 100644 > --- a/linux-user/main.c > +++ b/linux-user/main.c > @@ -3385,6 +3385,20 @@ static int parse_args(int argc, char **argv) > return optind; > } > > +#if defined(CONFIG_SUIDABLE) > +static int allow_unsetenv = 0; > + > +int __wrap___unsetenv(const char *name); > +int __real___unsetenv(const char *name); > +int > +__wrap___unsetenv(const char *name) > +{ > + if (!allow_unsetenv) > + return 0; > + return __real___unsetenv(name); > +} > +#endif > + > int main(int argc, char **argv, char **envp) > { > const char *log_file = DEBUG_LOGFILE; > @@ -3400,6 +3414,12 @@ int main(int argc, char **argv, char **envp) > int i; > int ret; > > +#if defined(CONFIG_SUIDABLE) > + allow_unsetenv = 1; > + seteuid(getuid()); > + setegid(getgid()); > +#endif > + > module_call_init(MODULE_INIT_QOM); > > qemu_cache_utils_init(envp);
diff --git a/configure b/configure index b101d5c..2322387 100755 --- a/configure +++ b/configure @@ -111,6 +111,7 @@ source_path=`dirname "$0"` cpu="" interp_prefix="/usr/gnemul/qemu-%M" static="no" +suidable="no" cross_prefix="" audio_drv_list="" audio_card_list="ac97 es1370 sb16 hda" @@ -624,6 +625,9 @@ for opt do LDFLAGS="-static $LDFLAGS" QEMU_PKG_CONFIG_FLAGS="--static $QEMU_PKG_CONFIG_FLAGS" ;; + --suid-able) + suidable="yes" + ;; --mandir=*) mandir="$optarg" ;; --bindir=*) bindir="$optarg" @@ -885,6 +889,11 @@ for opt do esac done +if test "$suidable" = "yes" -a "$static" = "no" ; then + echo "ERROR: --suid-able needs --static" + exit 1 +fi + case "$cpu" in sparc) LDFLAGS="-m32 $LDFLAGS" @@ -1014,6 +1023,7 @@ echo " --install=INSTALL use specified install [$install]" echo " --python=PYTHON use specified python [$python]" echo " --smbd=SMBD use specified smbd [$smbd]" echo " --static enable static build [$static]" +echo " --suid-able allow to use qemu with SUID bit [$suidable]" echo " --mandir=PATH install man pages in PATH" echo " --datadir=PATH install firmware in PATH$confsuffix" echo " --docdir=PATH install documentation in PATH$confsuffix" @@ -3196,6 +3206,7 @@ echo "sparse enabled $sparse" echo "strip binaries $strip_opt" echo "profiler $profiler" echo "static build $static" +echo "suid-able $suidable" echo "-Werror enabled $werror" if test "$darwin" = "yes" ; then echo "Cocoa support $cocoa" @@ -4160,6 +4171,10 @@ if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then ;; esac fi +if test "$target_linux_user" = "yes" -a "$suidable" = "yes" ; then + ldflags="$ldflags -Wl,--wrap=__unsetenv" + echo "CONFIG_SUIDABLE=y" >> $config_target_mak +fi echo "LDFLAGS+=$ldflags" >> $config_target_mak echo "QEMU_CFLAGS+=$cflags" >> $config_target_mak diff --git a/linux-user/linuxload.c b/linux-user/linuxload.c index 381ab89..783afce 100644 --- a/linux-user/linuxload.c +++ b/linux-user/linuxload.c @@ -58,11 +58,6 @@ static int prepare_binprm(struct linux_binprm *bprm) bprm->e_uid = geteuid(); bprm->e_gid = getegid(); - /* Set-uid? */ - if(mode & S_ISUID) { - bprm->e_uid = st.st_uid; - } - /* Set-gid? */ /* * If setgid is set but no group execute bit then this @@ -72,6 +67,17 @@ static int prepare_binprm(struct linux_binprm *bprm) if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) { bprm->e_gid = st.st_gid; } +#if defined(CONFIG_SUIDABLE) + setresgid(getgid(), bprm->e_gid, bprm->e_gid); +#endif + + /* Set-uid? */ + if(mode & S_ISUID) { + bprm->e_uid = st.st_uid; + } +#if defined(CONFIG_SUIDABLE) + setresuid(getuid(), bprm->e_uid, bprm->e_uid); +#endif retval = read(bprm->fd, bprm->buf, BPRM_BUF_SIZE); if (retval < 0) { diff --git a/linux-user/main.c b/linux-user/main.c index f6c4c8d..dd9dd24 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -3385,6 +3385,20 @@ static int parse_args(int argc, char **argv) return optind; } +#if defined(CONFIG_SUIDABLE) +static int allow_unsetenv = 0; + +int __wrap___unsetenv(const char *name); +int __real___unsetenv(const char *name); +int +__wrap___unsetenv(const char *name) +{ + if (!allow_unsetenv) + return 0; + return __real___unsetenv(name); +} +#endif + int main(int argc, char **argv, char **envp) { const char *log_file = DEBUG_LOGFILE; @@ -3400,6 +3414,12 @@ int main(int argc, char **argv, char **envp) int i; int ret; +#if defined(CONFIG_SUIDABLE) + allow_unsetenv = 1; + seteuid(getuid()); + setegid(getgid()); +#endif + module_call_init(MODULE_INIT_QOM); qemu_cache_utils_init(envp);
When qemu-linux-user is used in a linux container or chroot, if it needs to load binaries with SUID/SGID bits, it needs to have root rights to be able to change UID/GID. To do that, we need to install it with SUID bits and root owner. Then, if the SUID bit is not set on the binary to load, qemu will set its UID to the saved UID (the current user ID). To be able to retrieve unsecure environment variables (LD_PRELOAD, LD_LIBRARY_PATH) with SUID bit, we need to disable "unsetenv()". Otherwise libc unsets these variables before entering in main() To enable this feature, add "--suid-able" to the configure parameters. You can check all is working fine with : - install qemu-<arch> in your <arch> root filesystem environment and chown root:root ... and chmow +s ... - check sudo in this environment (chroot or linux container) : laurent@m68k $ id uid=1000(laurent) gid=1000(laurent) groups=1000(laurent) laurent@m68k $ sudo id Password: uid=0(root) gid=0(root) groups=0(root) - check LD_PRELOAD is available (debian fakeroot is my testcase) : laurent@m68k $ fakeroot id uid=0(root) gid=0(root) groups=1000(laurent) laurent@m68k $ rm -f toto laurent@m68k $ fakeroot root@m68k # touch toto root@m68k # ls -l toto -rw-r--r-- 1 root root 0 2012-12-18 22:50 toto root@m68k # exit exit root@m68k # ls -l toto -rw-r--r-- 1 laurent laurent 0 2012-12-18 22:50 toto Signed-off-by: Laurent Vivier <laurent@vivier.eu> --- configure | 15 +++++++++++++++ linux-user/linuxload.c | 16 +++++++++++----- linux-user/main.c | 20 ++++++++++++++++++++ 3 files changed, 46 insertions(+), 5 deletions(-)