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

broken container/selinux integration #83074

Open
LeifMiddelschulte mannequin opened this issue Nov 22, 2019 · 10 comments
Open

broken container/selinux integration #83074

LeifMiddelschulte mannequin opened this issue Nov 22, 2019 · 10 comments
Assignees
Labels
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes topic-IO type-bug An unexpected behavior, bug, or error

Comments

@LeifMiddelschulte
Copy link
Mannequin

LeifMiddelschulte mannequin commented Nov 22, 2019

BPO 38893
Nosy @tiran, @hynek, @ensc
PRs
  • gh-83074: Ignore EACCES, ENOSYS in copyxattr #21430
  • gh-83074: Add preserve_security_context to shutil #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:

    assignee = 'https://github.com/tiran'
    closed_at = None
    created_at = <Date 2019-11-22.07:37:24.975>
    labels = ['3.10', 'type-bug', '3.8', '3.9', 'expert-IO']
    title = 'broken container/selinux integration'
    updated_at = <Date 2020-12-09.15:30:04.323>
    user = 'https://bugs.python.org/LeifMiddelschulte'
    

    bugs.python.org fields:

    activity = <Date 2020-12-09.15:30:04.323>
    actor = 'christian.heimes'
    assignee = 'christian.heimes'
    closed = False
    closed_date = None
    closer = None
    components = ['IO']
    creation = <Date 2019-11-22.07:37:24.975>
    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']
    

    @LeifMiddelschulte
    Copy link
    Mannequin Author

    LeifMiddelschulte mannequin commented Nov 22, 2019

    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:
    containers/container-selinux#81

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

    @LeifMiddelschulte LeifMiddelschulte mannequin added 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes topic-IO type-bug An unexpected behavior, bug, or error labels Nov 22, 2019
    @tiran
    Copy link
    Member

    tiran commented Nov 22, 2019

    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.

    @LeifMiddelschulte
    Copy link
    Mannequin Author

    LeifMiddelschulte mannequin commented Nov 25, 2019

    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)

    @LeifMiddelschulte
    Copy link
    Mannequin Author

    LeifMiddelschulte mannequin commented Nov 25, 2019

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

    @LeifMiddelschulte
    Copy link
    Mannequin Author

    LeifMiddelschulte mannequin commented Nov 29, 2019

    @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
    Copy link
    Member

    tiran commented Mar 12, 2020

    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
    Copy link
    Member

    tiran commented Jul 10, 2020

    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 0beab05 back in 2013. What's your opinion?

    @tiran tiran added 3.10 only security fixes and removed 3.7 (EOL) end of life labels Jul 10, 2020
    @tiran tiran self-assigned this Jul 10, 2020
    @ensc
    Copy link
    Mannequin

    ensc mannequin commented Oct 5, 2020

    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
    Copy link
    Member

    tiran commented Dec 9, 2020

    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.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @erlend-aasland
    Copy link
    Contributor

    cc. @sethmlarson

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes 3.9 only security fixes 3.10 only security fixes topic-IO type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants