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

Email part (2) #3849

Merged
merged 13 commits into from
Feb 9, 2021
Merged

Email part (2) #3849

merged 13 commits into from
Feb 9, 2021

Conversation

bbhtt
Copy link
Contributor

@bbhtt bbhtt commented Dec 28, 2020

Continuation of #3607

Mutt/Neomutt needs some testing I may not have went through all use cases, leaving a (neo)muttrc if anyone wants to do some testing https://termbin.com/tryb

Copy link
Collaborator

@rusty-snake rusty-snake left a comment

Choose a reason for hiding this comment

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

mutt

  • whitelist 🚀
  • some nitpicks and future suggestions
  • does it break w/o quiet?

neomutt

Wouldn't it make sense to make it a mutt redirect?

etc/profile-m-z/mutt.profile Outdated Show resolved Hide resolved
etc/profile-m-z/mutt.profile Show resolved Hide resolved
etc/profile-m-z/mutt.profile Show resolved Hide resolved
etc/profile-m-z/mutt.profile Show resolved Hide resolved
Copy link
Collaborator

@rusty-snake rusty-snake left a comment

Choose a reason for hiding this comment

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

geary

I've a unfinished and untested version of this in my local profiles directory.

# Uncomment if you want to ...
#noblacklist /var/mail
#noblacklist /var/spool/mail
#whitelist /var/mail
#whitelist /var/spool/mail
#writable-var
machine-id
seccomp.block-secondary

What have other email programs?

disable-mnt

From flatpak and to permissive:

dbus-user filter
dbus-user.own org.gnome.Geary
dbus-user.talk ca.desrt.dconf
dbus-user.talk org.gnome.Contacts
dbus-user.talk org.gnome.OnlineAccounts
dbus-user.talk org.gnome.evolution.dataserver.AddressBook10
dbus-user.talk org.gnome.evolution.dataserver.Sources5
dbus-user.talk org.freedesktop.secrets
dbus-system none

@bbhtt
Copy link
Contributor Author

bbhtt commented Dec 29, 2020

neomutt
Wouldn't it make sense to make it a mutt redirect?

Yea I think so but the difference is private-etc, neomutt and mutt has some stuff unique/extra and some common.

does it break w/o quiet?

No it doesn't but they can take command line arguments too such as subject, to/from etc., I don't think leaving quiet is a good option.

I've a unfinished and untested version of this in my local profiles directory.

I don't think Geary has the option to utilize a local mailbox.

From flatpak and to permissive:

Thanks.

machine-id

Makes sense.

seccomp.block-secondary

I'll check this.

What have other email programs?

Not sure what you mean. disable-mnt was commented in all mail clients post-whitelisting.

@kmk3
Copy link
Collaborator

kmk3 commented Dec 29, 2020

diff --git a/etc/profile-m-z/mutt.profile b/etc/profile-m-z/mutt.profile
index 1ce12f54f..87e7c7f06 100644
--- a/etc/profile-m-z/mutt.profile
+++ b/etc/profile-m-z/mutt.profile
[...]
+whitelist ${HOME}/.config/nano
+whitelist ${HOME}/.elinks
+whitelist ${HOME}/.emacs
+whitelist ${HOME}/.emacs.d
+whitelist ${HOME}/.gnupg
+whitelist ${HOME}/.mail
+whitelist ${HOME}/.mailcap
+whitelist ${HOME}/.msmtprc
+whitelist ${HOME}/.mutt
+whitelist ${HOME}/.muttrc
+whitelist ${HOME}/.nanorc
+whitelist ${HOME}/.signature
+whitelist ${HOME}/.vim
+whitelist ${HOME}/.viminfo
+whitelist ${HOME}/.vimrc

MUAs like mutt are also used as development tools, especially on projects
where the reviews center around mailing lists (e.g.: the Linux kernel), so
using whitelists by default seems a bit much.

As a featureful example, aerc has a built-in terminal emulator and applying
patches, building the project and pushing the result is part of the indended
workflow
. So a complete whitelist to support that would have to include at
least git, every cli text editor and potentially every adjacent dev tool and
language/runtime if you build projects from inside aerc (e.g.: using git am +
make test).

Mutt is also not too far off since it supports macros for running arbitrary
commands, such as applying patches with git am (see 3, 4).

Besides that, there's the effort of mantaining the whitelists for every email
client x text editor combination. Examples off the top of my head:

  • Text editors: nano, vim, neovim, emacs
  • Email clients: mutt, neomutt, aerc, alpine

These alone would mean maintaining 4 sets of paths on 4 different profiles.


On a related note, there's already some text editor exceptions scattered in a
few profiles, but I've been working on a PR (WIP) to move all of that to a
single .inc. And also related, in there I've stumbled upon some whitelist
entries (e.g.: those on git.profile) that make it harder to organize things.

The main problem with whitelists is that there's no way to tell firejail to
only apply a given whitelist rule if a whitelist is already in effect (e.g.: if
there was a whitelist-enable /usr/share command). If it worked differently
in this regard, it would be possible to add whitelist command to .inc files
without worrying about breakage, just like the noblacklist commands.

@rusty-snake
Copy link
Collaborator

MUAs like mutt are also used as development tools, especially on projects
where the reviews center around mailing lists (e.g.: the Linux kernel), so
using whitelists by default seems a bit much.

whitelisting is a essential security feature, especial for programs with such a high attack vector as email-clients. Blacklisting profiles can be escaped easily in the most cases. Enabling whitelist for allusers is good IMHO. If you need arbitrary file access in $HOME, just use ignore whitelist ${HOME} .

Users who use a tui-mail-client and develop, are expected to have the knowledge to read documentation and customize there profiles. Therefore the question is IMHO not "Does this work with all workflows?" but "Does it work with the workflow of the (huge) majority of the users?". IDK how many mutt+firejail users develop on the kernel or similar, although I think that there are not that many.

The main problem with whitelists is that there's no way to tell firejail to
only apply a given whitelist rule if a whitelist is already in effect (e.g.: if
there was a whitelist-enable /usr/share command). If it worked differently
in this regard, it would be possible to add whitelist command to .inc files
without worrying about breakage, just like the noblacklist commands.

Conditions would also be a solution (e.g. ?HAS_USR_SHARE_WHITELIST: whitelist /usr/share/spam). However #3447 is a better solution I think because it solves this by removing complexity instead of adding. (Talking about the complexity of profiles, not the one of the code to implement this.)

@kmk3
Copy link
Collaborator

kmk3 commented Dec 30, 2020

MUAs like mutt are also used as development tools, especially on projects
where the reviews center around mailing lists (e.g.: the Linux kernel), so
using whitelists by default seems a bit much.

whitelisting is a essential security feature, especial for programs with
such a high attack vector as email-clients.

Agreed.

Blacklisting profiles can be escaped easily in the most cases.

Sounds interesting; do you know of any docs/articles/papers on this?

Enabling whitelist for allusers is good IMHO. If you need arbitrary file
access in $HOME, just use ignore whitelist ${HOME} .

Thanks, I didn't know that disabling the whitelist was that simple.

Users who use a tui-mail-client and develop, are expected to have the
knowledge to read documentation and customize there profiles. Therefore the
question is IMHO not "Does this work with all workflows?" but "Does it work
with the workflow of the (huge) majority of the users?".

If all it takes is ignore whitelist ${HOME} / ignore whitelist /usr/share /
etc instead of ignore whitelist ${HOME}/foo/bar/*, then sure.

But to nitpick a bit: speaking of documentation, it didn't occur to me that
this was the way to do it and it doesn't seem to be documented, so it seems a
little unfair to expect such knowledge just because one uses a tui-mail-client:

$ git show -q --pretty='%h %s' master
862f6820 manpage: /bin/bash -> user's perferred shell
$ grep -Fnr 'ignore whitelist' | grep -v '^etc/profile-'

It's not exactly clear to me which paths are treated specially by blacklist /
whitelist. For example, I know that blacklist / (I used that for testing)
does not affect ${HOME}, so blacklist ${HOME} would be required as well.

Anyways, my other point of contention is the amount of junk files that would
dumped at the doorstep of ${HOME} through mkdir / mkfile by merely
opening a program. I know that they have to exist before applying the
namespace, but there has to be a better way...

Well, if ignore whitelist ${HOME} works, so should ignore mkfile ${HOME},
right?

ignore mkdir ${HOME}
ignore mkfile ${HOME}
ignore whitelist ${HOME}

But I did some testing and unfortunately that's not the case:

$ cat /tmp/firejail/test.profile
ignore mkdir ${HOME}
ignore mkfile ${HOME}
ignore whitelist ${HOME}

mkdir ${HOME}/.spam
mkfile ${HOME}/.spamrc
whitelist ${HOME}/.spam
whitelist ${HOME}/.spamrc
$ ls .spam .spamrc | cat
ls: cannot access '.spam': No such file or directory
ls: cannot access '.spamrc': No such file or directory
$ firejail --quiet --profile=/tmp/firejail/test.profile ls .spam .spamrc | cat
.spamrc

.spam:
$ ls .spam .spamrc | cat
.spamrc

.spam:

So ignore whitelist ${HOME} is not enough to avoid the side effects of
profiles with whitelists; it seems that there would need to be an exception
for every mkdir / mkfile entry as well, which would be a hassle to maintain
locally, especially for every text editor x email client combination.


On a related side note, it would be nice to create only the xdg-base-dir paths
so that at least there's less clutter:

-mkfile ${HOME}/.muttrc
-mkfile ${HOME}/.nanorc
 [...]
 mkdir ${HOME}/.config/mutt
 mkdir ${HOME}/.config/nano

But since whitelist fails when a path doesn't exist, there would need to be a
whitelist-if-exists... Problem case: ${HOME}/.nanorc exists, but the
profile has only mkfile ${HOME}/.config/nano -> user config is not present in
the namespace -> nano uses an empty config.


IDK how many mutt+firejail users develop on the kernel or similar, although I
think that there are not that many.

I personally see both mutt and firejail as relatively medium-advanced user
tools (or at least not something that a new Linux user would go about
installing), so I think that there is above average overlap in the audiences
(potential or otherwise).

I mentioned the kernel because it's the most known example, but perhaps not a
very good one.

A more acessible instance would be SourceHut. It is
a software forge like GitHub, but every project gets a mailing list. The web
and email interfaces are linked together; whatever you do on one shows up in
the other. You can send a patchset, open issues, and reply to them by email
(all without needing an account) and everything will show up in the web ui. If
you do something in the web ui, people get emails and can reply to them and
perform actions (e.g.: close an issue) through them. Incidentally, there's an
article from today comparing the workflows:
https://blog.brixit.nl/git-email-flow-versus-github-flow/

The point being that this is not something only used by high-profile or
high-complexity projects.

(I'll reply to the rest on a new issue as that's mostly about a feature
request)

@rusty-snake
Copy link
Collaborator

So ignore whitelist ${HOME} is not enough to avoid the side effects of
profiles with whitelists; it seems that there would need to be an exception
for every mkdir / mkfile entry as well, which would be a hassle to maintain
locally, especially for every text editor x email client combination.

The other option would be to provide a commented opt-in whitelist. (Again) users of such software aren't expected to be completely unskilled, so they can look at the profile and uncomment/copy this part. ... or we have it opt-out and add a "If you need less restricted $HOME access, add 'ignore whitelist ${HOME}' to your mutt.local" note.

But to nitpick a bit: speaking of documentation, it didn't occur to me that
this was the way to do it and it doesn't seem to be documented, so it seems a
little unfair to expect such knowledge just because one uses a tui-mail-client:

Agreed. (As in the most OS-projects, there's a lower interest in writing and fine-tuning documentation and no one is paid to do that). Here you need either some playing with ignore or reading it's full code.

For example, I know that blacklist / (I used that for testing)
does not affect ${HOME}, so blacklist ${HOME} would be required as well.

/home | ${HOME} is usually treated special (and some others). Again undocumented.

Commands to / often don't work as expected (e.g. read-only / doesn't make everything ro). On the other hand the command like blacklist or noexec does make sens to be used on /. Again undocumented.

Blacklisting profiles can be escaped easily in the most cases.

Sounds interesting; do you know of any docs/articles/papers on this?

  1. Of course there's no blacklisting of unknown files. How should it work.
  2. Blacklisting is only applied to existing files. Example: ~/.config/systemd is read by systemd --user (which is used by Debian, Ubuntu, Mint, Fedora, Arch, OpenSUSE, ...) but is not present on the most systems. Or ~/.profile isn't in /etc/skel on some distros (I know Fedora, as I use it) but it is read by sh -l. Or ~/.pam_environment or ~/.config/environment.d/ and so on. So the issue are files which can be execute or change the environment (e.g. PATH=$HOME/dir_with_bad_coreutils:$PATH).

FYI: #3527. In my opinion a skilled attacker who can run any code in the sandbox, can escape any sandbox. The question is how much work it is to escape (without exploits). If the NSA wants to hack you, they can. If some ransomware uses /usr/bin/openssl for encryption of ~/Documents, read-only ${HOME} whitelist ${HOME}/foo blacklist ${PATH}/openssl (disable-common.inc) and blacklist ${DOCUMENTS} (disable-xdg.inc) all break it and why should a ransomware author do much extra work for edge cases, there are hundred system where it works.


becomes OT. If you have more question it would be the best to open an issues with quotes from here.

Copy link
Contributor Author

@bbhtt bbhtt left a comment

Choose a reason for hiding this comment

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

I added Sylpheed and Claws-mail too. (1) gpg (2) hyperlinks (3) dbus permissions

@rusty-snake I think you missed electron-mail as part of the electron refactor pull (3807). It is whitelisted already.

Only email clients left are Fetchmail, Fossamail and Thunderbird let me know if there are any others. This one ends here, rest I'll do as part of another.

@kmk3
Copy link
Collaborator

kmk3 commented Dec 31, 2020

The other option would be to provide a commented opt-in whitelist. (Again)
users of such software aren't expected to be completely unskilled, so they
can look at the profile and uncomment/copy this part. ... or we have it
opt-out and add a "If you need less restricted $HOME access, add 'ignore
whitelist ${HOME}' to your mutt.local" note.

My point isn't so much about the defaults or knowing how to write profiles, but
about UX / how complex it is profile-wise to create exceptions and maintain
them. If configuring it away is simple enough (e.g.: the 3 lines to ignore
mkdir/mkfile/whitelist in ${HOME}), then whitelisting by default is fine by
me.

Something which would work right now and partly achieve that is to put the
editor paths on e.g.: whitelist-text-editors.inc instead of hard-coding them
on these profiles. That way, it's at least possible to override the .inc with
only the editor(s) that I actually use and that would also work for every
profile that includes that.

Or rather, to avoid derailing this further, this can be merged without that and
I'll do the changes in the WIP branch that I mentioned. In that case, @bbhtt
after this is merged, would you mind waiting for a PR of that branch before
creating new ones which whitelist editors?

But to nitpick a bit: speaking of documentation, it didn't occur to me that
this was the way to do it and it doesn't seem to be documented, so it seems
a little unfair to expect such knowledge just because one uses a
tui-mail-client:

Agreed. (As in the most OS-projects, there's a lower interest in writing and
fine-tuning documentation and no one is paid to do that). Here you need
either some playing with ignore or reading it's full code.

For example, I know that blacklist / (I used that for testing) does not
affect ${HOME}, so blacklist ${HOME} would be required as well.

/home | ${HOME} is usually treated special (and some others). Again
undocumented.

Commands to / often don't work as expected (e.g. read-only / doesn't make
everything ro). On the other hand the command like blacklist or noexec
does make sens to be used on /. Again undocumented.

Yeah, maintaining documentation is not exactly the most fun activity. As long
as people aren't expected to dive deep into the source for creating profiles,
that's alright by me. Anyways, thanks for the details.

Blacklisting profiles can be escaped easily in the most cases.

Sounds interesting; do you know of any docs/articles/papers on this?

1. Of course there's no blacklisting of unknown files. How should it
   work.

2. Blacklisting is only applied to existing files. Example:
   `~/.config/systemd` is read by `systemd --user` (which is used by
   Debian, Ubuntu, Mint, Fedora, Arch, OpenSUSE, ...) but is not present
   on the most systems. Or `~/.profile` isn't in `/etc/skel` on some
   distros (I know Fedora, as I use it) but it is read by `sh -l`. Or
   `~/.pam_environment` or `~/.config/environment.d/` and so on. So the
   issue are files which can be execute or change the environment (e.g.
   `PATH=$HOME/dir_with_bad_coreutils:$PATH`).

FYI: #3527.

Thanks for the explanation/link. I thought you had meant escape in the sense
of bypassing the root:root ownership/file permissions imposed by firejail or
something. I wasn't aware that blacklisting only works if the file already
exists; that's crucial information. Now whitelisting sounds a lot more
important.

Some testing for future reference:

$ cat .emacs
cat: .emacs: No such file or directory
$ firejail --noprofile --quiet --blacklist='${HOME}/.emacs' \
  bash -c 'echo evil >~/.emacs; echo $?' 2>/dev/null
0
$ cat .emacs
evil
$ firejail --noprofile --quiet --blacklist='${HOME}/.emacs' \
  bash -c 'echo evil2 >>~/.emacs; echo $?' 2>/dev/null
1
$ cat .emacs
evil

In my opinion a skilled attacker who can run any code in the sandbox, can
escape any sandbox. The question is how much work it is to escape (without
exploits). If the NSA wants to hack you, they can. If some ransomware uses
/usr/bin/openssl for encryption of ~/Documents, read-only ${HOME}
whitelist ${HOME}/foo blacklist ${PATH}/openssl (disable-common.inc)
and blacklist ${DOCUMENTS} (disable-xdg.inc) all break it and why should
a ransomware author do much extra work for edge cases, there are hundred
system where it works.

Indeed, and that's where I believe firejail shines the most, by mitigating the
damage from untargeted userland exploits (even if it's just by being an edge
case) and by protecting against abuse from proprietary programs. IMO it's what
antivirus programs should have been. I don't really expect it to be a life
saver if e.g.: kernel exploits are involved. Either way, the overall cost
increase is a net win.

becomes OT. If you have more question it would be the best to open an issues
with quotes from here.

Agreed.

etc/profile-a-l/geary.profile Outdated Show resolved Hide resolved
etc/profile-a-l/geary.profile Outdated Show resolved Hide resolved
etc/profile-m-z/mutt.profile Outdated Show resolved Hide resolved
etc/profile-m-z/mutt.profile Outdated Show resolved Hide resolved
@bbhtt
Copy link
Contributor Author

bbhtt commented Dec 31, 2020

In that case, @bbhtt after this is merged, would you mind waiting for a PR of that branch before creating new ones which whitelist editors?

Sure.

About the sorts except dbus: IMO it is better to consider macros except Home/ as their own section. Otherwise if the whitelists get too long it'll be easy to loose them since Downloads/Documents will be in the top then Home/... then Pictures,Videos etc. people most often need to tweak these. I'm fine either way.

etc/profile-m-z/neomutt.profile Show resolved Hide resolved
etc/profile-m-z/neomutt.profile Show resolved Hide resolved
etc/profile-m-z/mutt.profile Show resolved Hide resolved
etc/profile-a-l/email-common.profile Show resolved Hide resolved
@kmk3
Copy link
Collaborator

kmk3 commented Jan 3, 2021

About the sorts except dbus: IMO it is better to consider macros except Home/
as their own section. Otherwise if the whitelists get too long it'll be easy
to loose them since Downloads/Documents will be in the top then Home/... then
Pictures,Videos etc. people most often need to tweak these. I'm fine either
way.

Ah, that's right; I forgot about those.

Considering that, putting ${HOME} entries first makes more sense now, as
${DOCUMENTS}, ${PICTURES}, etc don't need mkdir, so the sorting would
still work from the first mkdir until the last whitelist ${HOME}.

Anyway, there was one last attempted sort on email-common.profile, but GitHub
failed with:

Applying suggestions on deleted lines is not supported.

So here is the sorted version:

mkdir ${HOME}/.gnupg
mkfile ${HOME}/.config/mimeapps.list
mkfile ${HOME}/.signature
whitelist ${HOME}/.config/mimeapps.list
whitelist ${HOME}/.gnupg
whitelist ${HOME}/.mozilla/firefox/profiles.ini
whitelist ${HOME}/.signature
whitelist ${DOCUMENTS}
whitelist ${DOWNLOADS}
# when storing mail outside the default ${HOME}/Mail path, 'whitelist' the custom path in your email-common.local
whitelist ${HOME}/Mail

Other than that, LGTM.

@kmk3
Copy link
Collaborator

kmk3 commented Jan 3, 2021

$ grep -Fnr 'mkfile ${HOME}/.elinks' etc
etc/profile-m-z/neomutt.profile:68:mkfile ${HOME}/.elinks
etc/profile-m-z/mutt.profile:67:mkfile ${HOME}/.elinks

$ grep -Fnr 'mkfile ${HOME}/.w3m' etc
etc/profile-m-z/neomutt.profile:78:mkfile ${HOME}/.w3m
etc/profile-m-z/mutt.profile:76:mkfile ${HOME}/.w3m

These should be mkdir.

@kmk3
Copy link
Collaborator

kmk3 commented Jan 9, 2021

Hello, I have a pile of changes that are based on this branch (the WIP branch
mentioned here) and are almost ready for a PR, so I'm wondering what is
the current status of this PR.

Copy link
Collaborator

@rusty-snake rusty-snake left a comment

Choose a reason for hiding this comment

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

Some suggestions (mostly styling). I still think a redirect profile for neomutt would be good, but this can be done in a future PR.

etc/profile-m-z/neomutt.profile Outdated Show resolved Hide resolved
etc/profile-m-z/mutt.profile Outdated Show resolved Hide resolved
Comment on lines 41 to 43
include allow-perl.inc
include allow-python2.inc
include allow-python3.inc
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are they needed? mutt.profile has disable-interpreters.inc since 3 years.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are some optional functionality for Oauth etc. I will comment them.

etc/profile-a-l/geary.profile Outdated Show resolved Hide resolved
etc/profile-a-l/geary.profile Outdated Show resolved Hide resolved
etc/profile-a-l/email-common.profile Outdated Show resolved Hide resolved
etc/profile-a-l/email-common.profile Outdated Show resolved Hide resolved
@bbhtt
Copy link
Contributor Author

bbhtt commented Jan 9, 2021

I still think a redirect profile for neomutt would be good, but this can be done in a future PR.

Yea this one is too long I'll add it in the next one.

@netblue30 netblue30 merged commit 0b818f1 into netblue30:master Feb 9, 2021
@netblue30
Copy link
Owner

All going in, thanks!

@bbhtt bbhtt deleted the email branch February 16, 2021 10:29
kmk3 added a commit to kmk3/firejail that referenced this pull request Jan 28, 2023
To reduce the amount of spam created in the user home directory.

It's unlikely that these paths are going to be both:

* Created only after mutt is first opened through firejail and
* Created from within mutt

Also, no other profile does that:

    $ git grep -El '(mkdir|mkfile) \$\{HOME\}/\.(emacs|nano|vim)' -- etc
    etc/profile-m-z/mutt.profile

So just whitelist them if they already exist.

Added on commit a8a8e33 ("Add whitelisting to mutt; improve geary, new
profile for neomutt", 2020-12-28) / PR netblue30#3849.
kmk3 added a commit to kmk3/firejail that referenced this pull request Jan 28, 2023
Let either the respective program or the user create the file.

* ~/.bogofilter: Used by the bogofilter program
* ~/.msmtprc: Used by the msmtp program

Added on commit a8a8e33 ("Add whitelisting to mutt; improve geary, new
profile for neomutt", 2020-12-28) / PR netblue30#3849.
kmk3 added a commit to kmk3/firejail that referenced this pull request Jan 30, 2023
Move some paths from mutt.profile and neomutt.profile.

Added on commit 6b9bfad ("Fix python; add read-only to editors/cli
browsers;re-add cache directory", 2020-12-29) / PR netblue30#3849.

Misc: This is a follow-up to netblue30#5626.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants