Open simonfxr opened 3 years ago
Also I don't understand what this check is trying to accomplish, any explanation would be appreciated.
There is more detailed info on protecting against stack smashing attack
in https://github.com/netblue30/firejail/pull/3325, including a few other things you might try.
For now I can just increase the limit by patching and recompiling myself.
While patching and recompiling might 'fix' your issue, please be aware of the potential pitfalls of doing so in this context. The thread referenced above should provide a safer way out for your LS_COLORS issue.
@glitsj16 Thx for the link, that makes sense. However that still does not address the problem with the --rmenv
flag logic, that patch in #3674 does not work, since the loop over the env variable uses the envp
argument, afaict the contents of envp
are not modified by unsetenv()
(see OP). Maybe it would be a better idea, to guard against the total environment size and not the size of single env entries.
There's also some info on #3350. I think the proper fix to finalize #3322. Then the environment for Firejail itself is controlled and only a very small number of whitelisted, length checked variables are allowed inside the set-uid program. The variables are restored for the application itself and then the length checks could be relaxed. But even if #3322 was ready, I don't think it would be a good idea to push it just now when we're close to release.
Checking total environment size is a nice idea. Now we allow 256*(4096B+32B) = ~1MB of variables, when in reality maybe 64kB could be plenty while still allowing more or larger variables. Though this would be obsolete with #3322.
So #3322 fixes this, right?
I prefer #3322, because it protects Firejail fully but it also allows applications to get less restricted environment. To finalize it, I still have to check all paths where Firejail executes something to ensure that the set of env variables is correct for each case.
I also ran into the same error message using the git
command what is pretty strange since there is no profile for git in /etc/firejail/
nor anywhere else in my home. Next thing is git status
works as usual
$ git status
On branch master
Your branch and 'origin/master' have diverged,
and have 1 and 1 different commits each, respectively.
(use "git pull" to merge the remote branch into yours)
nothing to commit, working tree clean
but git pull
returns
$ git pull
Error: too long environment variables, please use --rmenv
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
so there must be firejail working in the background when I use git pull
. I ran git pull
with strace
and git status
The main difference I've found so far is the last call before the error happens in the git pull.strace
file: The file /usr/lib/git-core/git
is called which is a link to /usr/bin/git on my system
$ ll /usr/lib/git-core/git
lrwxrwxrwx 1 root root 13 Mar 27 09:27 /usr/lib/git-core/git -> ../../bin/git
I don't know why this error happens, at all. Ok my PS1 and _OLD_VIRTUAL_PS1 variables are pretty long which produce a beautiful prompt I really want to keep. Running
$ firejail --rmenv=PS1 --rmenv=_OLD_VIRTUAL_PS1 /usr/bin/git pull
Already up to date.
works as expected. But this doesn't explain why firejail is involved in the plain git pull
command but not in git status
. I don't see why firejail is involved at all running git
. firecfg --list | grep git
also doesn't return anything.
apparmor (3.0.1)
$ firejail --rmenv=PS1 --rmenv=_OLD_VIRTUAL_PS1 --version
firejail version 0.9.64.4
Compile time support:
- AppArmor support is enabled
- AppImage support is enabled
- chroot support is enabled
- D-BUS proxy support is enabled
- file and directory whitelisting support is enabled
- file transfer support is enabled
- firetunnel support is enabled
- networking support is enabled
- overlayfs support is disabled
- private-home support is enabled
- private-cache and tmpfs as user enabled
- SELinux support is disabled
- user namespace support is enabled
- X11 sandboxing support is enabled
yes running firejail --version
also complains about the Error: too long environment variables, please use --rmenv
OS=Archlinux with kernel 5.10.27
$ which git
/usr/bin/git
$ git --version
git version 2.31.1
git --version
works fine
firejail version 0.9.64.4 no profile for git in /etc/firejail/ nor anywhere else in my home
@MaelStor Odd indeed. Firejail has had a git.profile for a while now, and it definately is in the 0.9.64.4 package on Arch. Can you confirm there is no /etc/firejail/git.profile on your system? Also, what does which -a git
show? Try setting quiet-by-default yes
in /etc/firejail/firejail.config temporarily to make output from a firejailed git stand out more prominently.
$ which -a git
/bin/git
/usr/bin/git
Sorry, yes indeed there is a profile for git
$ ls -la /etc/firejail | grep git
-rw-r--r-- 1 root root 1390 Feb 10 20:44 com.github.bleakgrey.tootle.profile
-rw-r--r-- 1 root root 1460 Feb 10 20:44 com.github.dahenson.agenda.profile
-rw-r--r-- 1 root root 1578 Feb 10 20:44 com.github.johnfactotum.Foliate.profile
-rw-r--r-- 1 root root 201 Feb 10 20:44 com.gitlab.newsflash.profile
-rw-r--r-- 1 root root 2383 Feb 10 20:44 git-cola.profile
-rw-r--r-- 1 root root 1366 Feb 10 20:44 gitg.profile
-rw-r--r-- 1 root root 1195 Feb 10 20:44 github-desktop.profile
-rw-r--r-- 1 root root 1185 Feb 10 20:44 git.profile
-rw-r--r-- 1 root root 907 Feb 10 20:44 gitter.profile
Seems I mixed things up with firecfg --list
what doesn't show git and there is no link in /usr/local/bin
as shown above, so it shouldn't be active or am I wrong?
I tried running the same git
commands in bash instead of zsh with another prompt etc and added a variable quiet as long as my zsh PS1 (~7000 bytes) to the environment with export SOMEVAR=$(python -c 'print("A"*7000)')
with exactly the same result.
Setting quiet-by-default
had no effect:
$ git pull
Error: too long environment variables, please use --rmenv
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
It's ssh
which is called by git pull
.
@glitsj16 @rusty-snake Thanks. Now it makes sense. However putting rmenv into a local profile for ssh
$ cat $HOME/.config/firejail/ssh.local
rmenv PS1
rmenv _OLD_VIRTUAL_PS1
didnt' help. I still cannot run git commands if ssh is involved because I cannot pass the --rmenv
flag to firejail /usr/bin/ssh
. I would love to keep ssh sandboxed but I don't see how to get rid of the error other than removing the /usr/local/bin/ssh
link. What actually was the only option I've found that helped. Do you know any other possibilities?
In addition I always have to run firejail
with firejail --rmenv=PS1 --rmenv=_OLD_VIRTUAL_PS1
no matter which subcommand (--list
, --tree
etc.) follows. Is there a way to remove environment variables in the top level like the /etc/firejail/firejail.config
? I'm currently doing it with an alias but an option would be great.
IMHO It would help if I would be noticed about running ssh in a sandbox when I execute git, like it happens when running a program directly with firejail not in quiet mode. Also in the debug output of firejail --debug /usr/bin/git pull
there is no indication that ssh profiles are loaded.
I would be noticed about running ssh in a sandbox when I execute git
Use firemon (e.g. sudo firemon
)
Also in the debug output of firejail --debug /usr/bin/git pull there is no indication that ssh profiles are loaded.
There is no ssh.profile loaded if you firejail git.
Using firemon
in my usual environment didn't work. I guess firejail fails because of too long environment variables at an early stage and therefore nothing is recorded. And it doesn't solve the problem to get ssh
working when it is invoked by git
and I've got long environment variables.
However if I switch to bash with a shorter PS1 I've got firemon
working and it shows me that ssh
is invoked. It's not so obvious like the firejail messages about loaded profiles when starting a program with firejail $program
but I guess I get my stuff working with firemon
, which is great by the way.
PS1
and _OLD_VIRTUAL_PS1
are the only envvar which are too long on your system. Why do you export them?
Yes these are the only ones. It's not me who's exporting them, at least in zsh. I've tried unsetting themwith export PS1=
in my .zshrc at the very end but they are still in my environment.
EDIT: Got it working. I've overseen a command at the end of the file UPDATE: Removing the PS1 from the environment works only as long as I'm not entering a git repository. After that the PS1 and _OLD_VIRTUAL_PS1 variables pop up again in my environment.
It would be great if I could get rid of the firejail='firejail --rmenv=LS_COLORS'
alias on my system. alas, rmenv
in the config doesn't work :(
@haarp Have you tried a bash alias yet as suggested in #3325?
Any alias is just a workaround tho. A real fix would either allow rmenv in the config, or simply not choke on long env variables ;)
Any alias is just a workaround tho. A real fix would either allow rmenv in the config, or simply not choke on long env variables ;)
We accept PR's. Feel free to provide one that offers at least the same level of protection against stack smashing attacks as currently implemented. You can always drop LS_COLORS if security > shiny setups.
Wait, this is intentional? My impression was it was just a bug. Tho I admit it makes sense to bail out before config parsing begins.
In that case, the bail-out message could probably clarify that only --rmenv
works, not rmenv
in the config, so smartasses like me don't attempt it ;)
btw, it's just not shiny. I have other long env variables for varaious reasons, that my alias needs to exclude.
How about using an extra environment.global file in /etc/firejail
which blacklists environment varaibles, so I would have more control over the environment at an early stage. A simple list of environment variables one per line would be enough for me but perhaps you see better possibilities.
I just ran into this error.
My LS_COLORS env var is rather big (~5kb), which lead to:
Error: too long environment variables, please use --rmenv
I also can not work around the problem by passing
--rmenv=LS_COLORS
settingrmenv LS_COLORS
in globals.local does also not work. The problem is that callingunsetenv(*ptr)
in main does not modify theenvp
passed tomain()
,extern char ** environ;
should be used instead. Also I guess, that the profile is loaded after the check, so setting it in a profile won't work. Maybe this should be supported?Also I don't understand what this check is trying to accomplish, any explanation would be appreciated.
For now I can just increase the limit by patching and recompiling myself.