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

[system] Permission denied /proc/sys/fs/binfmt_misc inside LXD container #1662

Closed
slashdd opened this issue Apr 25, 2019 · 9 comments
Closed

Comments

@slashdd
Copy link

slashdd commented Apr 25, 2019

Running sosreport inside LXD, does not break anything besides showing a 'permission denied' message on sosreport when trying to list /proc/sys/fs/binfmt_misc inside an LXD container from system plugin.

We noticed a behaviour change from LXD 3.0.3 to 3.12.

The denial is cause on purpose by a apparmor lxd policy:

./lxd/apparmor.go:  # Handle binfmt
./lxd/apparmor.go:  mount fstype=binfmt_misc -> /proc/sys/fs/binfmt_misc/,
./lxd/apparmor.go:  deny /proc/sys/fs/binfmt_misc/{,**} rwklx,

3.0.3 was working okay due to a bug, but new version 3.12 fix it for good, and now prevent sosreport to collect binfmt_misc as it used to only when LXD 3.12 and late is in used.

Adding "/proc/sys/fs/binfmt_misc/*" in the actual add_forbidden_path would work but will also penalize all other situation where collecting would have work just fine. Since so far this is isolated to LXD only, maybe system plugin might need its own separate add_forbidden_path for LXD related stuff ?

LXD bug reference:
https://github.com/lxc/lxd/issues/5688

@slashdd
Copy link
Author

slashdd commented Apr 25, 2019

or we leave it as-is because it's not breaking anything, it's just screaming for the permission denied but doesn't cause any harm so far.

@slashdd
Copy link
Author

slashdd commented Apr 25, 2019

Looking https://github.com/lxc/lxd/blob/master/lxd/apparmor.go

There might be more thing to make sure sosreport doesn't collect when inside a LXD container.

@dnegreira
Copy link
Contributor

Maybe we can add a verification that we are running inside an LXD container and append /proc/sys/fs/binfmt_misc/* to a list based on current add_forbidden_path list

@slashdd
Copy link
Author

slashdd commented Sep 17, 2019

Should add_copy_spec not copy a path if a permission denied occurred? instead of letting a given plugin errors out ? and simply leave a WARNING|ERROR in sos.log:
WARNING or ERROR: [plugin:system] permission denied on '/proc/sys/fs/binfmt_misc/*'

@BryanQuigley thought ?

@BryanQuigley
Copy link
Contributor

I'd want these kinds of Access Denied errors to be shown in the output in a general sense. We don't want to hide from the end user that we can't collect some files as root.

The other #1299 permission denied errors (the rest of the forbidden paths in system) are a case that makes more sense to just exclude.

I don't understand upstream's choice here. Why mount it and then prevent any access to it?

@slashdd
Copy link
Author

slashdd commented Sep 18, 2019

Talking with Bryan this morning about this bug.

Here's my most current chain of thought.

Is there any real benefit to collect the entire /proc/sys ? I think that's the first question to ask ourselves. If yes, fine, if no, let's try to identify what we really care of.

Once we answer the above, maybe we should consider stopping to instruct system.py to collect the entire content of /proc/sys/* and start isolating the /proc/sys trees per plugins ?

Exactly like we do with log collection, letting each plugin collect their own respective log files, instead of collecting the whole /var/log/* like we used to.

kernel.py -> /proc/sys/kernel 
networking.py -> /proc/sys/net
filesys.py -> /proc/sys/fs
and so on

Current trees:

ls /proc/sys
abi  debug  dev  fs  kernel  net  user  vm

If we go that route, does each plugin absolutely need everything under their subset ?
and more precisely do we need /proc/sys/fs/binfmt_misc/* ?

If binfmt_misc/* not needed nor useful, then maybe we can simply stop collecting instead of workaround'ing it for lxc ? I'm just brainstorming here. Feel free to add your 2 cents.

@slashdd
Copy link
Author

slashdd commented Nov 19, 2019

What about this ?
@BryanQuigley

sos/plugins/init.py

def _copy_dir(self, srcpath):
        try:
            for name in os.listdir(srcpath):
                self._log_debug("recursively adding '%s' from '%s'"
                                % (name, srcpath))
                path = os.path.join(srcpath, name)
                self._do_copy_path(path)
        except OSError as e:
+           if e.errno == errno.EPERM or errno.EACCES:
+               msg = "PermissionError no access right"
+               self._log_info("_copy_dir: %s '%s'" % (msg, srcpath))
+               return
            if e.errno == errno.ELOOP:
                msg = "Too many levels of symbolic links copying"
                self._log_error("_copy_dir: %s '%s'" % (msg, srcpath))
                return
            raise

@slashdd
Copy link
Author

slashdd commented Nov 19, 2019

Here's an example inside a LXD container with no access to /proc/sys/fs/binfmt_misc

sos.log:2019-11-19 21:43:30,657 ERROR: [plugin:system] _copy_dir: PermissionError: Likely due to Linux Security Modules '/proc/sys/fs/binfmt_misc'

@slashdd
Copy link
Author

slashdd commented Nov 19, 2019

That way the error is flagged but doesn't generate a traceback in the form of sos_logs/system-plugin-error..... throwing a raise exception when facing a permission denied situation.

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 a pull request may close this issue.

3 participants