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.

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

Refactor electron.profile and electron based programs #3807

Merged
merged 9 commits into from
Dec 17, 2020

Conversation

rusty-snake
Copy link
Collaborator

see #3806

@rusty-snake rusty-snake marked this pull request as ready for review December 11, 2020 10:50
@rusty-snake rusty-snake changed the title Refactor electron.profile and electron based programs (1) Refactor electron.profile and electron based programs Dec 11, 2020
@@ -6,26 +6,22 @@ include freetube.local
# Persistent global definitions
include globals.local

# Disabled until someone reported positive feedback
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have been running freetube without issues having these in my freetube.local:

include whitelist-common.inc
include whitelist-runuser-common.inc
include whitelist-usr-share-common.inc
include whitelist-var-common.inc

nou2f
novideo

The nou2f is untested due to not having such hardware.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If I'm right freetube has no login support at all.

#include globals.local
include globals.local

# Disabled until someone reported positive feedback
Copy link
Collaborator

Choose a reason for hiding this comment

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

Tested with element-{desktop,web} from Arch community repo. Works fine with the below in riot-web.local:


# WIP 3807
include disable-devel.inc
# shell needed for tray icon
noblacklist ${PATH}/bash
noblacklist ${PATH}/sh
ignore noexec /tmp
whitelist /usr/share/chromium
whitelist /usr/share/webapps/element
include disable-exec.inc
include disable-interpreters.inc
include disable-xdg.inc
include whitelist-runuser-common.inc
include whitelist-usr-share-common.inc
include whitelist-var-common.inc
nou2f
novideo
# shell needed for tray icon
#shell none
disable-mnt
private-cache
private-dev
private-tmp

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

two questions

  • electron does not include disable-shell.inc, is it in one of your locals?
  • shell none just mean start direct and w/o a shell, but a shell is still present inside the sandbox. Does it really need to be ignored?

Copy link
Collaborator

@glitsj16 glitsj16 Dec 12, 2020

Choose a reason for hiding this comment

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

electron does not include disable-shell.inc, is it in one of your locals?

It isn't, nice catch. The relatively long include chain (element-desktop --> riot-desktop --> riot-web --> electron) threw me off here apparently.

shell none just mean start direct and w/o a shell, but a shell is still present inside the sandbox. Does it really need to be ignored?

Ditto as above. I suspect it was a leftover from testing permutations. I can now confirm adding 'shell none' works fine.

To put things straight, element-desktop is working with the below in my riot-web.local:

# WIP 3807
include disable-devel.inc
ignore noexec /tmp
whitelist /usr/share/chromium
whitelist /usr/share/webapps/element
include disable-exec.inc
include disable-interpreters.inc
include disable-xdg.inc
include whitelist-runuser-common.inc
include whitelist-usr-share-common.inc
include whitelist-var-common.inc
nou2f
novideo
shell none
disable-mnt
private-cache
private-dev
private-tmp
```

@mYnDstrEAm
Copy link

mYnDstrEAm commented Jan 9, 2021

Should it be sufficient to change /etc/firejail/electron.profile riot-web.profile and riot-desktop.profile to get element-desktop working with the latest firejail version in backports in Debian 10 (0.9.64)? I changed these 3 files as well as element-desktop.profile to match the latest versions in this repo and riot-web.local to match the comment above but now I get this error when running firejail element-desktop:

Warning: An abstract unix socket for session D-BUS might still be available. Use --net or remove unix from --protocol set.
Child process initialized in 1135.90 ms
The setuid sandbox is not running as root. Common causes:
  * An unprivileged process using ptrace on it, like a debugger.
  * A parent process set prctl(PR_SET_NO_NEW_PRIVS, ...)
Failed to move to new namespace: PID namespaces supported, Network namespace supported, but failed: errno = Operation not permitted

Parent is shutting down, bye...

@rusty-snake
Copy link
Collaborator Author

Using profiles from git master with a stable release (even the latest) is nothing what you should expect to work. Anyway ATM it works to use git master profiles in 0.9.64.

debian + element-desktop -> #3586.

kmk3 added a commit to kmk3/firejail that referenced this pull request Feb 21, 2021
Added on commit f4f6767 ("Refactor electron.profile and electron based
programs (netblue30#3807)").

This appears to be the only instance of that:

    $ grep -Fnr 'include-xdg' etc
    etc/profile-m-z/signal-desktop.profile:9:ignore include-xdg.inc

Credits go to syntax highlighting on vim.
kmk3 added a commit to kmk3/firejail that referenced this pull request Feb 22, 2021
Added on commit f4f6767 ("Refactor electron.profile and electron based
programs (netblue30#3807)").

This appears to be the only instance of that:

    $ grep -Fnr 'include-xdg' etc
    etc/profile-m-z/signal-desktop.profile:9:ignore include-xdg.inc

Simply fixing the typo would enable xdg dirs for the first time since
the aforementioned commit.  But, as talked with @rusty-snake[1], since
there has been no negative feedback, and since it's a whitelisting
profile, just remove the affected line instead.

Credits go to syntax highlighting on vim.

[1]: netblue30#4001
@matu3ba matu3ba mentioned this pull request Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants