python / cpython

The Python programming language
https://www.python.org
Other
63.37k stars 30.33k forks source link

broken container/selinux integration #83074

Open 4b7252b2-f989-4cb3-941c-9173a4c5549d opened 4 years ago

4b7252b2-f989-4cb3-941c-9173a4c5549d commented 4 years ago
BPO 38893
Nosy @tiran, @hynek, @ensc
PRs
  • python/cpython#21430
  • python/cpython#23720
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = 'https://github.com/tiran' closed_at = None created_at = labels = ['3.10', 'type-bug', '3.8', '3.9', 'expert-IO'] title = 'broken container/selinux integration' updated_at = user = 'https://bugs.python.org/LeifMiddelschulte' ``` bugs.python.org fields: ```python activity = actor = 'christian.heimes' assignee = 'christian.heimes' closed = False closed_date = None closer = None components = ['IO'] creation = creator = 'Leif Middelschulte' dependencies = [] files = [] hgrepos = [] issue_num = 38893 keywords = ['patch'] message_count = 9.0 messages = ['357248', '357250', '357433', '357434', '357636', '364011', '373451', '378031', '382794'] nosy_count = 4.0 nosy_names = ['christian.heimes', 'hynek', 'Leif Middelschulte', 'ensc2'] pr_nums = ['21430', '23720'] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue38893' versions = ['Python 3.8', 'Python 3.9', 'Python 3.10'] ```

    4b7252b2-f989-4cb3-941c-9173a4c5549d commented 4 years ago

    It seems Python does not necessarily determine that it is running inside a container correctly.

    This leads to broken/unexpected behavior when trying to copy files across filesytems using copy2. This directly affects Python3 inside the official fedora:latest image.

    Steps to reproduce the issue can be found here: https://github.com/containers/container-selinux/issues/81

    bpo-26328 (gh-70516) *might* be related too.

    tiran commented 4 years ago

    From the Github bug:

    copy2() fails while copying extended attributes.

    # python3
    Python 3.7.4 (default, Aug 12 2019, 14:45:07) 
    [GCC 9.1.1 20190605 (Red Hat 9.1.1-2)] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import shutil
    >>> shutil.copy2('/tmp/some_file', '/relabel_bug/failure')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib64/python3.7/shutil.py", line 267, in copy2
        copystat(src, dst, follow_symlinks=follow_symlinks)
      File "/usr/lib64/python3.7/shutil.py", line 209, in copystat
        _copyxattr(src, dst, follow_symlinks=follow)
      File "/usr/lib64/python3.7/shutil.py", line 165, in _copyxattr
        os.setxattr(dst, name, value, follow_symlinks=follow_symlinks)
    PermissionError: [Errno 13] Permission denied: '/relabel_bug/failure'

    The setxattr() fail is blocked SELinux: type=AVC msg=audit(1573815617.682:1332): avc: denied { relabelto } for pid=3157530 comm="python3" name="failure" dev="loop1" ino=12 scontext=system_u:system_r:container_t:s0:c552,c859 tcontext=system_u:object_r:fusefs_t:s0 tclass=file permissive=0

    Could you please provide name and value of the setxattr() call? I bet it's trying to setxattr 'security.selinux' extended file attribute.

    4b7252b2-f989-4cb3-941c-9173a4c5549d commented 4 years ago

    Could you please provide name and value of the setxattr() call? I bet it's trying to setxattr 'security.selinux' extended file attribute.

    (Pdb) bt full /usr/lib64/python3.7/pdb.py(1701)main() -> pdb._runscript(mainpyfile) /usr/lib64/python3.7/pdb.py(1570)_runscript() -> self.run(statement) /usr/lib64/python3.7/bdb.py(585)run() -> exec(cmd, globals, locals) \<string>(1)\<module>()->None /tmp/test.py(6)\<module>()->None -> copy2('/tmp/some_file', '/relabel_bug/failure') /usr/lib64/python3.7/shutil.py(267)copy2() -> copystat(src, dst, follow_symlinks=follow_symlinks) /usr/lib64/python3.7/shutil.py(209)copystat() -> _copyxattr(src, dst, follow_symlinks=follow)

    /usr/lib64/python3.7/shutil.py(165)_copyxattr() -> os.setxattr(dst, name, value, follow_symlinks=follow_symlinks) (Pdb) p dst '/relabel_bug/failure' (Pdb) p name 'security.selinux' (Pdb) p value b'system_u:object_r:fusefs_t:s0\x00' (Pdb)

    4b7252b2-f989-4cb3-941c-9173a4c5549d commented 4 years ago

    For the sake of completeness, the content of /tmp/test.py:

    #!/usr/bin/env python3
    
    from shutil import copy2
    
    copy2('/tmp/some_file', '/relabel_bug/failure')
    4b7252b2-f989-4cb3-941c-9173a4c5549d commented 4 years ago

    @Christian Heimes: is there anything else you need from me? Is this the wrong forum?

    As discussed in the referenced GitHub issue, some SELinux people suggest it might be a fault in how Python determines (?) it's running within a container environment and how to act upon it.

    Does it determine it at all? Does it use libselinux[0]?

    Background: I came across this issue by building a Linux distribution using Yocto in a Fedora:30 podman managed container with host volumes bound in. I guess that it is a fairly common scenario in the near future.

    [0] https://danwalsh.livejournal.com/73099.html

    tiran commented 4 years ago

    No, CPython's stdlib doesn't use libselinux.

    I talked to an engineer from Red Hat's SELinux team today. SELinux returns EACCES for policy violations like in this case. The _copyxattr() helper function ignores EPERM but not EACCES. You are seeing a PermissionError exception because Python maps both EPERM and EACCES to PermissionError.

    As first fix the _copyxattr() helper could ignore all permission errors for "security.*" namespace and just continue. This will get rid of the error but may still cause lots of AVC audit events.

    A better but backwards incompatible approach is to handle the xattr namespaces differently. Linux defines four xattr namespaces: security, system, trusted, and user. The security namespace is used by security policies like Smack or SELinux. IMHO _copyxattr() should only copy user xattrs by default. The security namespace should only be copied when the caller opts-in. The cp tool has separate preserve settings for context (SELinux security context) and xattr (other extended attributes).

    tiran commented 4 years ago

    The issue came up at $WORK now. Core utils like copy command ignore "security.selinux" xattr unless the user explicitly asks to preserve the security context, see

    https://github.com/coreutils/coreutils/blob/6a3d2883fed853ee01079477020091068074e12d/src/copy.c#L867-L891 https://github.com/philips/attr/blob/1cc88bd4c17ef99ace22c8be362d513f155b1387/libattr/attr_copy_fd.c#L109-L111

    _copyxattr() ignores most errnos that are listed in the man page of setxattr(2) but not EACCES. The man page of setxattr(2) also points to stat(2) which lists EACCES as possible errno.

    I see three simple and two more complicated solutions:

    1) ignore EACCES completely 2) ignore EACCES for "security.selinux" 3) ignore EACCES for "security.*" 4) provide a callback similar to the check() callback in libattr's attr_copy_fd(). Only copy an xattr when the callback is not set or returns True. 5) provide an extra option to skip security context

    Related: bpo-24564#msg351555 also suggests that copyxattr should ignore ENOSYS in listxattr. Some file systems (NFS?) seem to lack xattr.

    Hynek, you implemented most of copyxattr in 0beab058dd4 back in 2013. What's your opinion?

    3630ca78-e46f-499c-bcf0-c82d059da519 commented 4 years ago

    IMO the SELinux security attributes must not be copied (except when requested explicitly). Doing so will create badly labeled systems else. It would be better to use default transition rules and call optionally selinux_restorecon() then.

    E.g. when copying selinux.* attributes, after "cp /tmp/foo /bin/" the resulting "/bin/foo" would have a "tmp_t" label (which is wrong).

    Without copying attributes, it would be labeled as "bin_t" (which is more realistic).

    When there are SELinux rules for "/bin/foo", it might be relabeled e.g. to "bin_foo_t" by the manual selinux_restorecon().

    Ignoring errors silently will make operations very unpredictable.

    tiran commented 3 years ago

    I have created a new PR that introduces preserve_security_context argument and changes the default behavior of copy operations. All copy operations behave now similar to "cp -p --preserve=xattr" by default. copy2(src, dst, preserve_security_context=True) restores the old, problematic behavior that is similar to "cp -p --preserve=xattr,context".

    It's not completely equivalent because I decided to omit all attributes in the restricted "security" xattr namespace. coreutils only handles "security.selinux" on an SELinux enabled system differently.

    erlend-aasland commented 10 months ago

    cc. @sethmlarson