#604 closed enhancement (fixed)
xpra_Xdummy: run Xorg directly, without copying to remove setuid
Reported by: | pyther | Owned by: | pyther |
---|---|---|---|
Priority: | major | Milestone: | 0.14 |
Component: | server | Version: | 0.13.x |
Keywords: | Cc: |
Description
This is a cleaner solution than copying /usr/bin/Xorg to $HOME/.xpra and removing the setuid bits.
The script checks to see if the file is setuid and launches Xorg with ld-linux, otherwise it start Xorg normally. If this is implemented, the code in setup.py that checks if Xorg is setuid can be removed, which should simplify things a bit.
This works because ld-linux.so.2/ld-linux-x86_64.so.2 does not have the setuid bit set. ld-linux is the one executing Xorg.
It was suggested I try the kernel's no-new-privileges feature (new in linux 3.5). Although this looked like what I wanted,it did not work as expected. Xorg started without an issue, but I was unable to attach to the X session. I tested using 'setpriv --no-new-privilages Xorg' on fedora 20.
I tested this on RHEL6/Fedora20/Arch Linux.
#!/bin/sh #@PydevCodeAnalysisIgnore function find_ld_linux { arch=$(uname -m) if [[ $arch == "x86_64" ]]; then if [[ -f /lib64/ld-linux-x86-64.so.2 ]]; then LD_LINUX='/lib64/ld-linux-x86-64.so.2' fi elif [[ $arch = "i686" ]]; then if [[ -f /lib/ld-linux.so.2 ]]; then LD_LINUX='/lib/ld-linux.so.2' fi fi if [[ ! $LD_LINUX ]]; then echo "could not determine ld_linux path, please file an xpra bug" exit 1 fi } XORG_BIN=$(which Xorg) if [[ -u $XORG_BIN ]]; then # setuid is set, we need to do magic find_ld_linux exec "${LD_LINUX}" "${XORG_BIN}" "$@" else # setuid is not set on xorg_bin exec "${XORG_BIN}" "$@" fi
Change History (15)
comment:1 Changed 7 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:3 Changed 7 years ago by
I think r6813 in unnecessary. Every Linux system should have ld-linux.so.*. From what I could tell every major distro is using so major version 2. Perhaps, we try ld-linux.so.2 if arch != x86_64.
if [[ $arch == "x86_64" ]]; then if [[ -f /lib64/ld-linux-x86-64.so.2 ]]; then LD_LINUX='/lib64/ld-linux-x86-64.so.2' fi else if [[ -f /lib/ld-linux.so.2 ]]; then LD_LINUX='/lib/ld-linux.so.2' fi fi
Copying the binary is really dirty, in my opinion. Granted, using ld-linux isn't the most ideal.
I think the code that copies the binary is bloat since it is unlikely it will ever be used.
In r6812, what shells do we expect /bin/sh to expand to? I'm no expert but I believe var=$(/path/to/command) and -u /path/to/file ? are bash specific. Perhaps #!/bin/sh sholud be changed to #!/bin/bash.
comment:4 Changed 7 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
comment:5 Changed 7 years ago by
OK, then we need to start again from the top bearing in mind some points that I had half forgotten and that you need to be aware of:
- we must support other arches and not just x86_64 / i686, even though those probably represent 99% of the users - are there any arches doing funny things with this? possibly.
- we should not rely on bashisms, in part because:
- we must also support non-Linux distros (FreeBSD, etc), hence the need for the copy fallback code and maybe more: I don't know how Linux emulation works on those distros, but they may have something like
ld-linux.so
which may not be compatible with theirbin/Xorg
, etc..
comment:6 Changed 7 years ago by
Patch to remove bashism and make the script POSIX compliant.
--- xpra_Xdummy 2014-06-16 12:00:01.722865000 -0400 +++ xpra_Xdummy_posix 2014-06-16 12:22:45.849143000 -0400 @@ -1,16 +1,16 @@ #!/bin/sh #@PydevCodeAnalysisIgnore -function find_ld_linux { +find_ld_linux() { arch=$(uname -m) - if [[ $arch == "x86_64" ]]; then + if [ $arch = "x86_64" ]; then LD_LINUX='/lib64/ld-linux-x86-64.so.2' else LD_LINUX='/lib/ld-linux.so.2' fi - if [[ ! -x $LD_LINUX ]]; then + if [ ! -x $LD_LINUX ]; then LD_LINUX='' echo "could not determine ld_linux path for $arch, please file an xpra bug" fi @@ -18,11 +18,11 @@ function find_ld_linux { XORG_BIN=$(which Xorg) -if [[ -u $XORG_BIN ]]; then +if [ -u $XORG_BIN ]; then # setuid is set, we need to do magic find_ld_linux - if [[ -n $LD_LINUX ]]; then - if [[ -n $BASH ]]; then + if [ -n $LD_LINUX ]; then + if [ -n "$BASH" ]; then #running in bash, can show a more helpful command name: exec -a "Xorg-nosuid" "${LD_LINUX}" "${XORG_BIN}" "$@" else @@ -31,7 +31,7 @@ if [[ -u $XORG_BIN ]]; then else #fallback to making a copy of the binary: DOTXPRA_DIR="${HOME}/.xpra" - if [[ ! -d "${DOTXPRA_DIR}" ]]; then + if [ ! -d "${DOTXPRA_DIR}" ]; then mkdir "${DOTXPRA_DIR}" chmod 700 "${DOTXPRA_DIR}" fi
comment:7 Changed 7 years ago by
Looking at debian's libc6 packages for various arches
armel /lib/ld-linux.so.3 armhf /lib/ld-linux-armhf.so.3 powerpc /lib/ld.so.1 mipsel /lib/ld.so.1 mips /lib/ld.so.1 s390x /lib/ld64.so.1 sparc /lib/ld-linux.so.2 i386 /lib/ld-linux.so.2 amd64 /lib64/ld-linux-x86-64.so.2
and Fedora
s390x /lib64/ld64.so.1 s390 /lib/ld.so.1 ppc64 /lib64/ld64.so.1 ppc /lib/ld.so.1 armhfp /lib/ld-linux.so.3
Perhaps we should try /lib/ld-linux-armhf.so.3 if arch == arm, otherwise try /lib/ld-linux.so.2, /lib/ld-linux.so.3, /lib/ld.so.1, andlib/ld64.so.1 (in that order)?
Perhaps that is over kill, maybe arm is the only thing we need to try to support (as I think it is the most common) and anyone using a different arch could file a bug or use the fallback method.
comment:8 Changed 7 years ago by
Owner: | changed from Antoine Martin to pyther |
---|---|
Status: | reopened → new |
Please see: r6815
What have I missed this time?
comment:9 Changed 7 years ago by
We probably want to match i386, i586, and i686.
I'm not sure if there are any modern systems running i386 or i586, I think everything is i686.
comment:10 Changed 7 years ago by
Is there a posix shell syntax for matching "i?86"? Or just ugly OR statements?
comment:11 Changed 7 years ago by
Sadly I don't think there is. We could call an external tool such as grep, but it is probably better to 'or' or 'elif' in this case.
comment:13 Changed 7 years ago by
Related (future): Xorg without root rights now available in Fedora
comment:14 Changed 7 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
Works OK, tested beta packages too. Closing.
comment:15 Changed 6 weeks ago by
this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/604
Awesome! Applied in r6810 and updated the wiki: wiki/Xdummy