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

missing conventions for (un)installation, interference from firejail/sandboxing on copying installation files #560

Open
matu3ba opened this issue May 31, 2021 · 4 comments

Comments

@matu3ba
Copy link

matu3ba commented May 31, 2021

Describe The Bug

zellij uses cargo-make to build multiple crates and provides also release builds. However adding a path like cargo make install PATH can only set permissions 0700. This makes binaries usable, but they can not be overwritten.
Unfortunately cargo make is invoked by the restricted cargo process, so subprocesses can only write binaries as read-only for cargo (and later on without write-permissions).

(The behavior is also not very good either, because ideally one wants to forbid access to globally installed programs/programs in PATH for subprocesses.)

This means that cargo make install PATH will never work as expected and install to PATH with read-write permissions.

  1. Can you clarify this in your conventions?

  2. The conventions are also missing that one should separate the build step from installation step (which is less ideal when people enable this by adding a PATH after cargo make install).

  3. How to uninstall is missing.

  4. Ideally cargo-make would be explicit about the supported installation and uninstall methods. Ideally it would give advice how to not interfere with the package manager if anything with root is intended to be supported.

TO REPRODUCE:

  1. install firejail ie with
    git clone https://github.com/netblue30/firejail ; cd firejail; ./configure && make && sudo make install-strip
    sudo firecfg

  2. install zellij + requirements with cargo-make (cargo-make must be installed of course)
    git clone https://github.com/zellij-org/zellij; cd zellij; cargo make install ~/.cargo/bin/zellij
    stat ~/.cargo/bin/zellij
    permissions should show 0700
    If this is not the case, git checkout another revision/hash and retry installation.
    Now permissions should show 0700
    (If the file is non-existing writing it apparently is fine, but followup writes do make it 0700)

POTENTIAL WORKAROUND:
chmod +rwx FILE could work, but are an ugly hack.

WHY FIREJAIL AND THE MESS:
Restricting cargo to limit simple abuse forms is a valid use case and this requires to disallow executing created binaries or silently overwriting binaries without explicit permission to do so. Its very unfortunate that this breaks cargo-make installing stuff.

@sagiegurari
Copy link
Owner

@matu3ba thanks for the issue, i'll try to help but i feel some things are related to zellij, no?

  1. cargo make install - this task is actually defined in zellij and not part of cargo-make
    its defined here:
    https://github.com/zellij-org/zellij/blob/9cbe4107403e94b3545257c7b13acd7fdd2cf04c/Makefile.toml#L72
    basically its doing cp for files to some folder. the task can do chmod after the cp if needed (and go permissions to do so...).
  2. how to uninstall - feels also part of zellij, as it has the install task, it can provide an uninstall task as well. but basically it would just do rm to the file from what i am seeing.
  3. separating build and install steps - again this is part of the zellij makefile, not cargo-make but as i see it the install depends on the build. you can run the build standalone. but you can't run the install without building. it makes sense to me.
  4. "ideally one wants to forbid access to globally installed programs/programs in PATH for subprocesses" - right, but then it means you need to run the whole thing with sudo maybe.

@matu3ba
Copy link
Author

matu3ba commented May 31, 2021

@sagiegurari No, see this cargo profile inside firejail which is causing it. On disabling firejail with firejail --noprofile cargo make install ~/.cargo/bin the problem does not persist.

Yes, 1. is affected by this. I am not entirely sure how exactly. However the cp from zellij does not retain the same permissions: here.

I am only asking you to document this behavior and provide a convention how to work around this so others dont trap into the same bugfeature.

The idea is to first fix the documentation upstream, so I can then add it to zellij.

It should probably go into the first simple example to hint that the cargo make install (cp with potential chmod to another filepath) must be a separate step and does not work with sandboxing as expected. So it must be executed separatedly in the CI shell script.

@sagiegurari
Copy link
Owner

wait, the cp doesn't copy the permissions? because of the cargo.profile?

the duckscript cp is using std::fs::copy which in the docs state that it copies permissions as well. strange

@matu3ba
Copy link
Author

matu3ba commented May 31, 2021

yes. This is kinda intended behavior of firejail, but still has abit poor behavior. The original motivation was to contain cargo to make secrets not easily extractable with macros. For a proper solution also the rustc paths must be sandboxed.

However using cargo-make as standalone program works, since it does not get called with a sandbox.

You can try it yourself with the commands I provided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants