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

Error: too long environment variables, please use --rmenv #3678

Open
simonfxr opened this issue Oct 18, 2020 · 20 comments
Open

Error: too long environment variables, please use --rmenv #3678

simonfxr opened this issue Oct 18, 2020 · 20 comments

Comments

@simonfxr
Copy link

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 setting rmenv LS_COLORS in globals.local does also not work.
The problem is that calling unsetenv(*ptr) in main does not modify the envp passed to main(), 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.

@glitsj16
Copy link
Collaborator

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 #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.

@simonfxr
Copy link
Author

@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.

@topimiettinen
Copy link
Collaborator

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.

@rusty-snake
Copy link
Collaborator

So #3322 fixes this, right?

@topimiettinen
Copy link
Collaborator

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.

@ghost
Copy link

ghost commented Apr 4, 2021

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

git pull.strace

and git status

git status.strace

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.


My setup

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

@glitsj16
Copy link
Collaborator

glitsj16 commented Apr 5, 2021

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.

@ghost
Copy link

ghost commented Apr 5, 2021

$ 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.

@rusty-snake
Copy link
Collaborator

It's ssh which is called by git pull.

@ghost
Copy link

ghost commented Apr 5, 2021

@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.

@rusty-snake
Copy link
Collaborator

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.

@ghost
Copy link

ghost commented Apr 5, 2021

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.

@rusty-snake
Copy link
Collaborator

PS1 and _OLD_VIRTUAL_PS1 are the only envvar which are too long on your system. Why do you export them?

@ghost
Copy link

ghost commented Apr 5, 2021

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.

@haarp
Copy link
Contributor

haarp commented Apr 6, 2021

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 :(

@glitsj16
Copy link
Collaborator

glitsj16 commented Apr 6, 2021

@haarp Have you tried a bash alias yet as suggested in #3325?

@haarp
Copy link
Contributor

haarp commented Apr 6, 2021

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 ;)

@glitsj16
Copy link
Collaborator

glitsj16 commented Apr 6, 2021

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.

@haarp
Copy link
Contributor

haarp commented Apr 6, 2021

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.

@ghost
Copy link

ghost commented Apr 6, 2021

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.

hlein added a commit to hlein/firejail that referenced this issue Nov 10, 2021
This change doesn't alter any checks, but it gives more specific
errors when a sanity check of env vars or argv does not pass, which
can point to limits to raise or at least give us better detailed bug
reports.

Signed-off-by: Hank Leininger <[email protected]>
Bug: netblue30#3678
Bug: netblue30#3851
Bug: netblue30#4633
hlein added a commit to hlein/firejail that referenced this issue Nov 10, 2021
This change doesn't alter any checks, but it gives more specific
errors when a sanity check of env vars or argv does not pass, which
can point to limits to raise or at least give us better detailed bug
reports.

Signed-off-by: Hank Leininger <[email protected]>
Bug: netblue30#3678
Bug: netblue30#3851
Bug: netblue30#4633
hlein added a commit to hlein/firejail that referenced this issue Nov 10, 2021
This change doesn't alter any checks, but it gives more specific
errors when a sanity check of env vars or argv does not pass, which
can point to limits to raise or at least give us better detailed bug
reports.

Signed-off-by: Hank Leininger <[email protected]>
Bug: netblue30#3678
Bug: netblue30#3851
Bug: netblue30#4633
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

No branches or pull requests

5 participants