xpra icon
Bug tracker and wiki

Opened 5 years ago

Closed 5 years ago

#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 (14)

comment:1 Changed 5 years ago by Antoine Martin

Resolution: fixed
Status: newclosed

Awesome! Applied in r6810 and updated the wiki: wiki/Xdummy

comment:2 Changed 5 years ago by Antoine Martin

Further improvements in r6812 + r6813.

comment:3 Changed 5 years ago by pyther

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 5 years ago by pyther

Resolution: fixed
Status: closedreopened

comment:5 Changed 5 years ago by Antoine Martin

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 their bin/Xorg, etc..

comment:6 Changed 5 years ago by pyther

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 5 years ago by pyther

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 5 years ago by Antoine Martin

Owner: changed from Antoine Martin to pyther
Status: reopenednew

Please see: r6815

What have I missed this time?

comment:9 Changed 5 years ago by pyther

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 5 years ago by Antoine Martin

Is there a posix shell syntax for matching "i?86"? Or just ugly OR statements?

comment:11 Changed 5 years ago by pyther

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:12 Changed 5 years ago by Antoine Martin

Done in r6817.

comment:13 Changed 5 years ago by Antoine Martin

comment:14 Changed 5 years ago by Antoine Martin

Resolution: fixed
Status: newclosed

Works OK, tested beta packages too. Closing.

Note: See TracTickets for help on using tickets.