Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

profiles: telegram: allow opening links (xdg-open) #4783

Merged
merged 1 commit into from
Dec 19, 2021
Merged

profiles: telegram: allow opening links (xdg-open) #4783

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion etc/profile-m-z/telegram.profile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ include globals.local
noblacklist ${HOME}/.TelegramDesktop
noblacklist ${HOME}/.local/share/TelegramDesktop

# Allow opening hyperlinks
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: use our inc file specifically designed for situations like this (instead of the two noblacklists)

include allow-bin-sh.inc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: use our inc file specifically designed for situations like this (instead of the two noblacklists)

include allow-bin-sh.inc

Nice suggestion, thank you @glitsj16.

include allow-bin-sh.inc

include disable-common.inc
include disable-devel.inc
include disable-exec.inc
Expand Down Expand Up @@ -41,7 +44,7 @@ seccomp.block-secondary
shell none

disable-mnt
private-bin telegram,Telegram,telegram-desktop
private-bin bash,sh,telegram,Telegram,telegram-desktop,xdg-open
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why bash?

xdg-open is a shell-script, consider sed,xdg-mime,which,mimeopen,grep,egrep,printf,cut,uname,dbus-send,xprop,dirname,cat,…, exo-open,gio,gvfs-open,mate-open,enlightenment_open,gnome-open,dde-open,kde-open,kde-open*,kfmclient,cygstart,kde-config,gnome-default-applications-properties,… or just drop private-bin.

Copy link
Contributor Author

@YorkZ YorkZ Dec 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why bash?

I believe enabling bash is necessary because in Arch Linux, /usr/bin/sh has been symlink'ed to /usr/bin/bash (/usr/bin/sh is provided by package bash). Some customized systems also use bash as sh. I personally frequently use bash as sh in production systems too because bash is so much more featureful than sh. In fact, bash has a sh compatible mode which is probably why I've never had any issue when replacing sh with bash. Finally, in this case, I tested only enabling sh, and confirmed that it didn't work.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe enabling bash is necessary because in Arch Linux, /usr/bin/sh has been symlink'ed to /usr/bin/bash

Do not believe, test 😉.

private-bin follows symlinks IIRC and even if it don't there are other shells commonly used as /bin/sh (namely dash by Debian and Ubuntu(?)).

Finally, in this case, I tested only enabling sh, and confirmed that it didn't work.

(Fedora Linux 35; sh->bash):

$ firejail --noprofile --private-bin=sh,ls ls -l /usr/bin
bash
ls
sh -> /usr/bin/bash

I guess you had tested with only sh in private-bin and only noblacklist ${PATH}/sh. This does not work blacklist ${PATH}/bash will blacklist the binary used by /usr/bin/sh. FWIW, blacklist follows symlinks too.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xdg-open is a shell-script, consider …

On my system for xdg-open to work at all: sh,xdg-open,grep,egrep,<A installed well known browser, e.g. firefox>.
On my system for xdg-open to work correct: sh,xdg-open,grep,egrep,xdg-mime,sed,tr,awk,cut,head,basename,which,readlink,<My browser, e.g. firefox>
For the firefox start script: ...

or just drop private-bin.

yeah, I think this is the way to go.

Copy link
Contributor Author

@YorkZ YorkZ Dec 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you had tested with only sh in private-bin and only noblacklist ${PATH}/sh

That's correct, I think I tested with only sh in private-bin and I only noblacklist sh.

there are other shells commonly used as /bin/sh (namely dash by Debian and Ubuntu(?))

I think dash is indeed an implementation of POSIX sh which is actually sh.

Copy link
Collaborator

@rusty-snake rusty-snake Dec 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC this is because firejail hardcodes bash as the default shell.

It don't, read the issues title in your comment:

  • Default shell is guessed from $SHELL, despite manpage specifying /bin/bash

On Artix it fails, but I'm not sure why:

If I get you right (sh->dash; getent passwd $USER | cut -d: -f7: bash), you try to start a program (bash) which isn't inside the sandbox.

IMHO --shell=none should be the default (if you specify a program).

So in my case it does require private-bin bash, even though /bin/sh is
/bin/dash.

Because you still run bash inside the sandbox and it isn't present (as a side-effect).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rusty-snake on Dec 19:

This just lists the files;

@kmk3 it clearly shows that the file /bin/sh points to is
copied/bind-mounted (what does it do?) if you only list sh but not bash
(assuming /bin/sh points to somewhere in {/usr,}/bin).

I don't understand what you mean. A symlink points to a (text) path; it
doesn't actually point to an inode. For example, if /bin/sh points to just
"dash" and you copy the symlink with to /tmp, the copy will not work:

$ readlink /bin/sh
dash
$ /bin/sh -c 'echo yes'
yes
$ cd /tmp
$ cp -P /bin/sh .
$ readlink ./sh
dash
$ ./sh -c 'echo yes'
bash: ./sh: No such file or directory
$ echo 'echo hello world' >dash
$ chmod +x dash
$ ./dash
hello world
$ ./sh
hello world

Do you mean that sh points to just "bash" outside of firejail but inside of it
it points to "/usr/bin/bash"? That might mean that firejail hardcodes its own
/bin/sh symlink inside the sandbox, but whether /bin/bash is bind-mounted or
not is unclear from your example.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I want to tell is that private-bin=sh will copy/bind-mount sh and the program it points to (e.g. bash or dash). No matter if you list bash/dash or not.

I don't care about /bin/sh pointing to somewhere else (e.g. /mnt/extra-program/mysh).

Copy link
Contributor Author

@YorkZ YorkZ Dec 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried removing bash from private-bin, and it worked (the link was opened from Telegram). @kmk3 do you still have problem if removing bash from private-bin?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Continued on #4790)

private-cache
private-dev
private-etc alsa,alternatives,ca-certificates,crypto-policies,fonts,group,ld.so.cache,ld.so.preload,localtime,machine-id,os-release,passwd,pki,pulse,resolv.conf,ssl,xdg
Expand Down