Automatically elevate to root if necessary about appimageupdate HOT 36 OPEN

@abhay

I manually compiled with vala
check this : http://tinypic.com/view.php?pic=240ycz5&s=9#.WMBQQlfhU8o

Do you have appimageupdate (the script) on your $PATH? Just compiling the GUI is not enough, you need to do all the steps in the script section of https://github.com/probonopd/AppImageUpdate/blob/master/.travis.yml.

from appimageupdate.

@probonopd I tried to update a appimage named Darktable. No errors occured for me and the appimage is updated to latest version(2.2.3).Please see this link.Please note that I updated with root authentication only.

from appimageupdate.

Did you change something in the source code? It does not work for me.

What do you mean by

Please note that I updated with root authentication only.

The point is that if a non-root user tries to update such an AppImage, then AppImageUpdate should try to become root and run the appimageupdate with root permissions, similar to what Etcher does.

from appimageupdate.

I did not change anything in the source code.Just executed as is.pkexec will by run on opening appimage and you give the root password and then the update starts.It is similar to what Etcher does.

from appimageupdate.

So here is the more detailed explanation from @jviotti:

We got that question a couple of times already 😀 It’s definitely not very straightforward, but its language/framework independent, so it should work for Qt:

• Our application can behave in CLI mode or GUI mode depending on an environment variable, which is read on the entry point of the app
• Initially, the AppImage is mounted as a normal user
• We then prompt the user for elevation (we use https://github.com/jorangreef/sudo-prompt, but with Qt you can probably interact with polkit directly)
• We then re-execute the same AppImage with root permissions, but this time passing the environment variable that causes the app to behave in CLI mode
• The AppImage is remounted again as root
• The CLI communicates with the GUI using IPC

Check https://github.com/resin-io/etcher/blob/master/lib/child-writer/README.md for the more low level details

So in summary: we mount the AppImage twice: once as the user, and once as root

Regarding the Ubuntu Live CD, sudo-prompt, the module we’re using, checks if it can get sudo access before prompting the user. It can do so on the live CD, so it proceeds straight away

This is exactly what we should implement for AppImageUpdate.

What is their "Child Writer" is most likely the appimageupdate CLI tool for us.

from appimageupdate.

@probonopd We're very interested in having AppImage updates for Etcher, and glad to hear our current approach matches what you think AppImages should be doing. Is there anything I can contribute with? I'm a bit busy these weeks, but I can allocate time on Fridays to help.

from appimageupdate.

JvanKatwijk/qt-dab#34 (comment) would also profit from this.

from appimageupdate.

Please see JvanKatwijk/qt-dab#34 (comment) for a possible solution, more testing on that solution is needed.

from appimageupdate.

Hi @jviotti it would be great if you could have a look at JvanKatwijk/qt-dab#34 (comment)

from appimageupdate.

For reference (and searchability), @jviotti at JvanKatwijk/qt-dab#34 (comment):

The way we handle this at Etcher is that first we try to run the command (e.g: /tmp/udev-rules-helper) using sudo -n. The -n means that sudo will behave non-interactively, and thus will not prompt for a password. After the program exits, we check stderr for /sudo: /i. If this matches, then it means that we need to prompt the user for elevation using something like pkexec, if not, then it means that the command actually did run with elevation already, so that's all there is to do. Check https://github.com/jorangreef/sudo-prompt/blob/master/index.js#L11 for details.

from appimageupdate.

For Arduino, I am using this as part of a custom AppRun/start script which appears to be working for me, at least on Live ISOs (more testing on installed systems needed):

cat > /tmp/roothelper <<\EOoF                                                                                                                                                           
#!/bin/bash                                                                                                                                                                             
cat > /etc/udev/rules.d/50-arduino.rules <<\EOF                                                                                                                                         
SUBSYSTEM=="tty", KERNEL=="ttyUSB[0-9]*|ttyACM[0-9]*", GROUP="users", MODE="0666"                                                                                                       
EOF                                                                                                                                                                                     
udevadm trigger
EOoF
chmod a+x /tmp/roothelper

sudo true && sudo /tmp/roothelper
sudo true || pkexec --disable-internal-agent /tmp/roothelper || true

rm /tmp/roothelper

Intended behavior:

• On Live ISOs, it just becomes root, does its thing, without asking the user
• On installed systems, it asks the user for the root password

from appimageupdate.

Legacy issue in the old code base. Please report if it should occur in the rewrite as well.

from appimageupdate.

In rewrite we currently have this behavior, which is slightly better but not yet perfect:

Assume that /isodevice/Applications/AppImageUpdate-2f9a55e-x86_64.AppImage is stored at a location that is only accessible by the root user, which is common on Live ISOs (and hence my main use case for AppImage).

Expected result: As described above, it should "just work" since on Live systems, sudo works without needing a password (on systems that do require a password, the user should be asked for it)

Actual result: Te update succeeds, but the resulting file is located in $HOME instead of the location of the original file /isodevice/Applications/, and it is not executable.

me@host:~$ /isodevice/Applications/AppImageUpdate-2f9a55e-x86_64.AppImage /isodevice/Applications/AppImageUpdate-2f9a55e-x86_64.AppImage 
AppImageUpdate version 1-alpha (commit 2f9a55e), build 140 built on 2017-11-14 14:37:02 UTC
Checking for updates...
Fetching release information for tag "continuous" from GitHub API.
... done
Starting update...
Fetching release information for tag "continuous" from GitHub API.
Updating from GitHub Releases via ZSync
zsync2: Target file: /home/me/AppImageUpdate-8199a82-x86_64.AppImage
zsync2: Reading seed file: /isodevice/Applications/AppImageUpdate-2f9a55e-x86_64.AppImage
zsync2: Usable data from seed files: 42.249589%
zsync2: Renaming temp file
zsync2: Fetching remaining blocks
zsync2: Downloading from https://github-production-release-asset-2e65be.s3.amazonaws.com/84325774/bc496bd6-c9b4-11e7-9096-f40efba1ac9a?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAIWNJYAX4CSVEH53A%2F20171115%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20171115T063037Z&X-Amz-Expires=300&X-Amz-Signature=be067695697b25735f48dc832feb64ba99984136da620b811decdbab52b56599&X-Amz-SignedHeaders=host&actor_id=0&response-content-disposition=attachment%3B%20filename%3DAppImageUpdate-8199a82-x86_64.AppImage&response-content-type=application%2Foctet-stream
zsync2: Verifying downloaded file
zsync2: checksum matches OK
zsync2: used 5269504 local, fetched 7202552
Update successful.
Updated file: /home/me/AppImageUpdate-8199a82-x86_64.AppImage

from appimageupdate.

Try the latest release. 8199a82 implements #37. Your version is just outdated, it puts the new file in the current working directory. Then, please describe the new behavior.

from appimageupdate.

@probonopd waiting for feedback here. I'd like to resolve this issue this week.

from appimageupdate.

As long as we are not trying to elevate permissions if we encounter permission denied and are not root (like Etcher does) this is not entirely solved.

from appimageupdate.

@probonopd I meant, how does it behave now, as it now puts (or, tries to put) the binary in the original directory. Initially, you stated it put the binary into $HOME, but that was intended behavior which has been changed eventually. Hence the request to try this with an up to date version.

from appimageupdate.

Think of a regular user trying to update some AppImage stored in a system-wide location where normal users have no write access, e.g., /opt. In my test below, this system-wide location where normal users have no write access is /isodevice/OldApplications/.

Current behavior:

me@host:~$ /home/me/Downloads/AppImageUpdate-164-6e435c2-x86_64.AppImage /isodevice/OldApplications/AppImageUpdate-0d169e4-x86_64.AppImage 
AppImageUpdate version 1-alpha (commit 6e435c2), build 164 built on 2017-11-21 11:07:44 UTC
Checking for updates...
Fetching release information for tag "continuous" from GitHub API.
... done
Starting update...
Fetching release information for tag "continuous" from GitHub API.
Updating from GitHub Releases via ZSync
zsync2: Target file: /isodevice/OldApplications/AppImageUpdate-164-6e435c2-x86_64.AppImage
zsync2: Reading seed file: /isodevice/OldApplications/AppImageUpdate-0d169e4-x86_64.AppImage
rename: Invalid cross-device link
zsync2: Usable data from seed files: 10.986382%
zsync2: Renaming temp file
Update failed

Intended behavior:

Before anything else, AppImageUpdate checks whether the current user has write permissions in the directory in which the old AppImage resides. If no,

1. it tries to become root without interfering the user; if that fails
2. it asks the user for the root password (or whatever the proper PolicyKit mechanism is; if that fails
3. it shows a warning, saying root is required (or; at your choice, offers to download somewhere else)

My script above is able to do 1 and 2.

Etcher also handles this elegantly.

As you can see from the history, this bothered me already in my own first implementation, so I am hoping we can solve it with the rewrite.

Thanks!

from appimageupdate.

@probonopd side note: apart from performing root operations without explicit user consent or any warning about it (a CLI message would help already), your script leaves a udev rule in place that should definitely be removed after execution.

Not being a fan of implicit root actions, I'd always state I'm trying to get root permissions in the UI, maybe with a message. I'll think about the situation, and come up with a "polished" solution for this soon. First of all have to sort out #50.

from appimageupdate.

The udev thing was just an example for an action that needs to be performed as root. Of course here we don't need udev!

from appimageupdate.

I know, just a general notice. I wouldn't have done it like that, but you should at least have your Arduino AppImage clean up its udev config file.

from appimageupdate.

Well, that /dev/tty is not accessible to all users is just plain annoying to me. I like it like it is now.

from appimageupdate.

@probonopd all I'm saying is, if you create the file, you should probably rm it after the main app terminated. It will be recreated anyway.

from appimageupdate.

True...

from appimageupdate.

After quite some experimenting, I think I have a satisfying solution:

We have two main topics to consider here: security and user friendliness. The latter has been described in detail by @probonopd before, who's made some suggestions how to implement a user friendly "let's get root" feature: test whether higher privileges are required, test whether those privileges can be gathered without prompting for a password (otherwise show dialog), then continue with the update.

Being opposed to applications elevating privileges without notifying user, I suggested to add some functionality notifying the user about having become root. Also, the solutions @probonopd linked to didn't seem to be too secure to me, hence I've tried to find an already existing software providing the requested functionality.

There's two programs which are used nowadays to run applications sudo-ish without using the CLI tool sudo. These are pkexec (part of Polkit, available on many modern desktops and even server systems), and as a fallback (if pkexec is unavailable), gksudo (part of gksu package in Debian systems, which it superseded). It doesn't seem to be a good idea to bundle pkexec, but it seems like gksudo could be built and bundled within AppImages without much hassle (although, if a system version is available, it should always be used in favor of a bundled one). Both show a prompt similar to the UAC in Windows systems, unless the user is in sudo mode (or has root status already).

I suggest to do the following: check whether pkexec is on PATH, fall back to system gksudo, fall back to bundled gksudo. The last remaining question is: shall we re-run the whole application (by execing something like gksudo $0 $PARAMS_CONSTRUCTED_FROM_ARG_PARSER, less secure solution, as most parts don't require root permissions), or shall we attempt to limit sudo mode only to operations which really require higher privileges?

I am strongly in favor with the second option, which could be implemented by putting all temporary files in the /tmp dir, for example (possible by setting the cwd to that path), and calling $SUDO_APP mv /tmp/new.AppImage /path/to/old/appimage/. The nice part about both options is that they don't require any changes within zsync2, as the new functionality can be implemented with the existing API in AppImageUpdate.

That said, one has to consider that the mv call would be the only required as of today, but there might be more operations necessary in the future. Also, there might be slight overhead when the target directory for the updated AppImage is on another disk (mv would automatically copy the file to the new disk, and unlink it on the old drive, this avoids problems as in #50). However, considering it is by far more safe to go with option 2, I'd say the overhead file I/O is an acceptable trade-off (although @probonopd might disagree, as he's the one taking most advantage of this issue being closed.

Whatever way we take, that's the sanest options I could've come up with. And considering the lack of a working low-dependency gksudo for bundling within AppImages (existing one in Debian has a lot of dependencies, including the infamous libharfbuzz), the time spent on building this package might produce a package (on OBS, like with curl-httponly, the low-dependency cURL library for AppImage builds) that could become extremely useful to third parties as well.

from appimageupdate.

I don't think gksudo or any of those tools can work from inside an AppImage because they would be lacking the suid bit, and we can't extract them from the AppImage and give them to suid root bit because we don't have the permissions. Hence we need to use pkexec and friends which, at this point, we need to take granted as a part of the target systems (if they are not there, we can always fall back to showing a message telling the user to launch AppImageUpdate as root).

If we follow the scheme suggested by @TheAssassin, what would happen in the following (my daily) scenario:

• User keeps AppImages on a separate partition for which the user has no write permissions (e.g, system-wide /opt mounted as a separate partition)
• User updates a large AppImage

Would it the updated Version be constructed "directly" at the target directory (the /opt partition in this example), or would it be constructed in /tmp and only after the update succeeded, be moved to the target partition? The latter would not be an acceptable solution for me because a) my system's partition is almost always close to full (all running in tmpfs in RAM as a matter of fact) whereas my AppImage storage area has plenty of space (again, think large AppImages - 12 GB and such), b) the mv would take additional time, destroying the instant experience, and c) if the process is interrupted for whatever reason (e.g., network failure) the half-complete file would not be in the target directory, from where it could be re-used.

So, in summary, I am fine with your proposal as long as a) it works and b) no copying of the AppImage to be updated nor the updated AppImage between different directories is involved. I am fine with selectively copying stuff out of AppImageUpdate.AppImage to /tmp as long as we are talking about small portions, e.g., shell scripts.

from appimageupdate.

Okay, so basically we'd go with the first option, checking the permissions, and re-executing AppImageUpdate entirely with gksudo/pkexec (in case the filename parameter is lacking, I'd show the file chooser first, and then ask for the password, looks more consistent). If none of the tools is available, I'll just show a warning (after which the app exits), and tell the <5% (rough estimation) of people who'd ever see it to call AppImageUpdate with sudo from a terminal.

from appimageupdate.

Sounds like a plan.

from appimageupdate.

So, after a lot of testing and debugging (and some bugs with btrfs, at least I'd think so...), I have a working solution that even makes sure elevating to root would make sense before asking for a password. Currently forced to use gksudo (pkexec says something about the suid bit not being said although it is set, further investigation needed here). Needs some more love to ensure best UX, e.g., better error message, and some further error handling.

One last remaining issue is the re-execution of the AppImage after successful updates, which should not be done with the root permissions. gksudo at least sets $SUDO_USER and $SUDO_UID, which could be used to re-execute as the original user. Looks like pkexec sets some $PKEXEC_UID variable, but, as mentioned previously, pkexec is not functional at the moment.
We should check for the existence of these variables, and if they are missing, show a warning, I guess.

from appimageupdate.

More information on the btrfs bug: the bug only occurs when trying to patch an existing AppImage like dd if=/dev/zero of=my.AppImage seek=1M bs=100k count=1 conv=notrunc. After execution, ls shows a file size of 107374284800. Also, AppImageUpdate (and most likely other applications) thinks it has this many bytes to read, causing update checks to read all that data and hash it. This is a real issue which I should probably try to report to btrfs.

from appimageupdate.

I pushed a first version which works well from my perspective, but probably lacks a lot of testing (there's so many use cases which I can't cover because I'd just not think of them).

The code first checks whether write access to both the directory and the file are available to the current user, then checks the same for root to "predict" whether changing to root would help at all (taking into account that if the file is owned by root and read-only, it should probably not be updated at all). Then elevates to root using gksudo (see TODOs about that), updates the file, drops permissions to the original UID, and executes the AppImage (might yield a "you have no permission to execute this file" error at the moment however, as it just copies the permissions of the old file, meaning that if the current user had no permission to execute the file before, they won't have it now (actually that improves UX as in the future there might be more AppImages friendly desktops which might want to preserve some AppImages for root execution only)). While being rather complex, this is like the only way to implement the behavior you suggested (the Etcher method ignores quite a few scenarios in my opinion, at least from what I understand from the comment).

The implementation requires an extensive review and discussion, which should be done in a PR as soon as I consider it "ready to merge" (we're far from that point at the moment). The feature adds almost 300 new lines (beware: currently, the file has ~450 lines), highly increases the complexity of the tool, and I'm still not convinced that this is really necessary or useful to a larger user base, while bloating the code base and making the tool more error prone and potentially less secure, especially taking into account that it might soon be superseded by a Qt/GTK UI (although I am opposed to that as well, given the amount of code and hours invested in the well working FLTK GUI whose only real counter argument is that it doesn't look native at the moment).

It took > 1 day including extensive testing, and still I'm pretty sure I overlooked at least half of the issues in there. I'll trigger a build on Travis to generate an artifact which you can use for testing.

from appimageupdate.

Artifact: https://transfer.sh/yMU8E/AppImageUpdate-175-b927f61-x86_64.AppImage
Log: https://travis-ci.org/AppImage/AppImageUpdate/builds/309827425

from appimageupdate.

For me, I don't need to be asked whether it is OK to elevate to root - in fact, if I can elevate to root without needing a password then it should just do the right thing without even bothering me because obviously I do have the permissions. However, I should not be asked again which file to update after AppImageKit relaunched with root rights.

from appimageupdate.

@probonopd yeah, that's a bug, and I know the cause. Basically, I copy the existing argv, prepend the path to AppImageUpdate, and call the result with sudoTool. If the path is not contained in the existing argv, it will have to be inserted there. Pretty easy to fix.

from appimageupdate.

It is still not working and it doesn't give any hints in the GUI that this is a permissions problem:

# The file is on a partition on which only root can write, but my user can't

$ ls -lh /isodevice/Applications/Firefox-latest.glibc2.3.4-x86_64.AppImage
-rwxr-xr-x 1 root root 57M Nov 14 22:37 /isodevice/Applications/Firefox-latest.glibc2.3.4-x86_64.AppImage

$ strings /isodevice/Applications/Firefox-latest.glibc2.3.4-x86_64.AppImage | grep ^bintray
bintray-zsync|probono|AppImages|Firefox|Firefox-_latestVersion-x86_64.AppImage.zsync

$ /home/me/Downloads/AppImageUpdate-297-d04fe37-x86_64.AppImage /isodevice/Applications/Firefox-latest.glibc2.3.4-x86_64.AppImage
AppImageUpdate version 1-alpha (commit d04fe37), build 297 built on 2018-04-09 02:23:49 UTC
QApplication: invalid style override passed, ignoring it.
Updating from Bintray via ZSync
open: Permission denied
zsync2: Failed to parse .zsync file!
zsync2: Reading and/or parsing .zsync file failed!

# This error is clearly wrong, because if I copy the file to a location where my user can write
# or if I run this with sudo, then it starts updating...

After all the time, this is still the #1 issue for me.

The intented behavior is described in #1 (comment).

from appimageupdate.

Is this still the situation?

from appimageupdate.