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

noexec /tmp causes pip install to miss chmod +x on scripts included in wheel file #6364

Open
vikingsloth opened this issue Mar 28, 2019 · 8 comments
Labels
C: error messages Improving error messages C: wheel The wheel format and 'pip wheel' command UX User experience related

Comments

@vikingsloth
Copy link

vikingsloth commented Mar 28, 2019

Environment

pip version: pip 19.0.3
Python version: python 3.6
OS: Oracle Linux 7.6

tmpfs on /tmp type tmpfs (rw,nosuid,nodev,noexec,relatime)

Description
Pip won't install a script as executable if /tmp is mounted as noexec. I have a wheel file that was created with setuptools. Pip unpacks it into /tmp and chmod the file to 755 which is correct and executable. However, when pip goes to copy the file from the tmp staging dir to the destination it uses access(file, X_OK) to check if the file is executable rather than looking at the mode flags. the access() returns "EACCES (permission denied)" and pip skips the chmod step.

Broken:
chmod("/tmp/pip-install-lzeaps2y/YYY/YYY-0.3.4.data/scripts/export", 0755) = 0 ... stat("/tmp/pip-install-lzeaps2y/YYY/YYY-0.3.4.data/scripts/export", {st_mode=S_IFREG|0755, st_size=3593, ...}) = 0 access("/tmp/pip-install-lzeaps2y/YYY/YYY-0.3.4.data/scripts/export", X_OK) = -1 EACCES (Permission denied) stat("/home/XXX/pt/bin/export", {st_mode=S_IFREG|0644, st_size=3593, ...}) = 0

Working:
chmod("/tmp/pip-install-bfuur148/YYY/YYY-0.3.4.data/scripts/export", 0775) = 0 ... stat("/tmp/pip-install-bfuur148/YYY/YYY-0.3.4.data/scripts/export", {st_mode=S_IFREG|0775, st_size=3593, ...}) = 0 access("/tmp/pip-install-bfuur148/YYY/YYY-0.3.4.data/scripts/export", X_OK) = 0 stat("/tmp/pip-install-bfuur148/YYY/YYY-0.3.4.data/scripts/export", {st_mode=S_IFREG|0775, st_size=3593, ...}) = 0 chmod("/home/XXX/pt/bin/export", 0100775) = 0 stat("/home/XXX/pt/bin/export", {st_mode=S_IFREG|0775, st_size=3593, ...}) = 0

Expected behavior

How to Reproduce
Enable tmpfs /tmp with noexec
mount -t tmpfs -o rw,nosuid,nodev,noexec,relatime tmpfs /mnt

Create a python wheel file with a script that just echos test

cat << EOF > testbin
#!/bin/bash
echo test
EOF

cat << EOF > setup.py
import setuptools
setuptools.setup(
    name="example-pkg",
    version="0.0.1",
    scripts=["testbin"]
)
EOF

virtualenv --python python36 pt
source pt/bin/activate

python setup.py bdist_wheel
pip install dist/example_pkg-0.0.1-py3-none-any.whl

ls -l pt/bin

Output

Broken output shows the testbin is not +x

(pt) [XXX@YYY]$ cat << EOF > testbin
> #!/bin/bash
> echo test
> EOF
(pt) [XXX@YYY]$
(pt) [XXX@YYY]$ cat << EOF > setup.py
> import setuptools
> setuptools.setup(
>     name="example-pkg",
>     version="0.0.1",
>     scripts=["testbin"]
> )
> EOF
(pt) [XXX@YYY]$
(pt) [XXX@YYY]$ virtualenv --python python36 pt
Running virtualenv with interpreter /usr/bin/python36
Using base prefix '/usr'
New python executable in /backups/tmp/pt/bin/python36
Also creating executable in /backups/tmp/pt/bin/python
Installing setuptools, pip, wheel...done.
(pt) [XXX@YYY]$ source pt/bin/activate
(pt) [XXX@YYY]$
(pt) [XXX@YYY]$ python setup.py bdist_wheel
running bdist_wheel
running build
running build_scripts
copying testbin -> build/scripts-3.6
changing mode of build/scripts-3.6/testbin from 644 to 755
installing to build/bdist.linux-x86_64/wheel
running install
running install_egg_info
running egg_info
writing example_pkg.egg-info/PKG-INFO
writing dependency_links to example_pkg.egg-info/dependency_links.txt
writing top-level names to example_pkg.egg-info/top_level.txt
reading manifest file 'example_pkg.egg-info/SOURCES.txt'
writing manifest file 'example_pkg.egg-info/SOURCES.txt'
Copying example_pkg.egg-info to build/bdist.linux-x86_64/wheel/example_pkg-0.0.1-py3.6.egg-info
running install_scripts
creating build/bdist.linux-x86_64/wheel/example_pkg-0.0.1.data
creating build/bdist.linux-x86_64/wheel/example_pkg-0.0.1.data/scripts
copying build/scripts-3.6/testbin -> build/bdist.linux-x86_64/wheel/example_pkg-0.0.1.data/scripts
changing mode of build/bdist.linux-x86_64/wheel/example_pkg-0.0.1.data/scripts/testbin to 755
creating build/bdist.linux-x86_64/wheel/example_pkg-0.0.1.dist-info/WHEEL
(pt) [XXX@YYY]$ pip install dist/example_pkg-0.0.1-py3-none-any.whl
Processing ./dist/example_pkg-0.0.1-py3-none-any.whl
Installing collected packages: example-pkg
Successfully installed example-pkg-0.0.1
(pt) [XXX@YYY]$ ls -l pt/bin/testbin
-rw-r--r-- 1 XXX access-login 22 Mar 28 19:51 pt/bin/testbin
@cjerdonek cjerdonek added C: wheel The wheel format and 'pip wheel' command S: needs triage Issues/PRs that need to be triaged labels Apr 7, 2019
@chrahunt
Copy link
Member

I was able to reproduce.

docker-compose.yml

---
version: "3"
services:
  test:
    image: python:3.7
    tmpfs:
    - /tmp:noexec
    command: /test/run_test
    volumes:
    - .:/test

./run_test

#!/bin/sh

cd /root
mkdir example
cd example

cat << EOF > testbin
#!/bin/bash
echo test
EOF
chmod +x testbin

cat << EOF > setup.py
import setuptools
setuptools.setup(
    name="example-pkg",
    version="0.0.1",
    scripts=["testbin"]
)
EOF

python -m venv .venv
. .venv/bin/activate
pip install --upgrade pip wheel

python setup.py bdist_wheel
pip install dist/example_pkg-0.0.1-py3-none-any.whl

ls -l .venv/bin

Executing docker-compose run --rm test shows that testbin does not have execute permission.

Output:

$ docker-compose run --rm test
Collecting pip
  Downloading https://files.pythonhosted.org/packages/5c/e0/be401c003291b56efc55aeba6a80ab790d3d4cece2778288d65323009420/pip-19.1.1-py2.py3-none-any.whl (1.4MB)
    100% |████████████████████████████████| 1.4MB 6.5MB/s
Collecting wheel
  Downloading https://files.pythonhosted.org/packages/bb/10/44230dd6bf3563b8f227dbf344c908d412ad2ff48066476672f3a72e174e/wheel-0.33.4-py2.py3-none-any.whl
Installing collected packages: pip, wheel
  Found existing installation: pip 19.0.3
    Uninstalling pip-19.0.3:
      Successfully uninstalled pip-19.0.3
Successfully installed pip-19.1.1 wheel-0.33.4
running bdist_wheel
running build
running build_scripts
creating build
creating build/scripts-3.7
copying testbin -> build/scripts-3.7
installing to build/bdist.linux-x86_64/wheel
running install
running install_egg_info
running egg_info
creating example_pkg.egg-info
writing example_pkg.egg-info/PKG-INFO
writing dependency_links to example_pkg.egg-info/dependency_links.txt
writing top-level names to example_pkg.egg-info/top_level.txt
writing manifest file 'example_pkg.egg-info/SOURCES.txt'
reading manifest file 'example_pkg.egg-info/SOURCES.txt'
writing manifest file 'example_pkg.egg-info/SOURCES.txt'
Copying example_pkg.egg-info to build/bdist.linux-x86_64/wheel/example_pkg-0.0.1-py3.7.egg-info
running install_scripts
creating build/bdist.linux-x86_64/wheel/example_pkg-0.0.1.data
creating build/bdist.linux-x86_64/wheel/example_pkg-0.0.1.data/scripts
copying build/scripts-3.7/testbin -> build/bdist.linux-x86_64/wheel/example_pkg-0.0.1.data/scripts
changing mode of build/bdist.linux-x86_64/wheel/example_pkg-0.0.1.data/scripts/testbin to 755
creating build/bdist.linux-x86_64/wheel/example_pkg-0.0.1.dist-info/WHEEL
creating 'dist/example_pkg-0.0.1-py3-none-any.whl' and adding 'build/bdist.linux-x86_64/wheel' to it
adding 'example_pkg-0.0.1.data/scripts/testbin'
adding 'example_pkg-0.0.1.dist-info/METADATA'
adding 'example_pkg-0.0.1.dist-info/WHEEL'
adding 'example_pkg-0.0.1.dist-info/top_level.txt'
adding 'example_pkg-0.0.1.dist-info/RECORD'
removing build/bdist.linux-x86_64/wheel
Processing ./dist/example_pkg-0.0.1-py3-none-any.whl
Installing collected packages: example-pkg
Successfully installed example-pkg-0.0.1
total 40
-rw-r--r-- 1 root root 2197 Jun 30 22:49 activate
-rw-r--r-- 1 root root 1253 Jun 30 22:49 activate.csh
-rw-r--r-- 1 root root 2405 Jun 30 22:49 activate.fish
-rwxr-xr-x 1 root root  247 Jun 30 22:49 easy_install
-rwxr-xr-x 1 root root  247 Jun 30 22:49 easy_install-3.7
-rwxr-xr-x 1 root root  229 Jun 30 22:49 pip
-rwxr-xr-x 1 root root  229 Jun 30 22:49 pip3
-rwxr-xr-x 1 root root  229 Jun 30 22:49 pip3.7
lrwxrwxrwx 1 root root   21 Jun 30 22:49 python -> /usr/local/bin/python
lrwxrwxrwx 1 root root    6 Jun 30 22:49 python3 -> python
-rw-r--r-- 1 root root   22 Jun 30 22:49 testbin
-rwxr-xr-x 1 root root  225 Jun 30 22:49 wheel

Editing docker-compose.yml to have /tmp:exec and re-running the same command shows testbin with execute permission, as expected.

Output:

Collecting pip
  Downloading https://files.pythonhosted.org/packages/5c/e0/be401c003291b56efc55aeba6a80ab790d3d4cece2778288d65323009420/pip-19.1.1-py2.py3-none-any.whl (1.4MB)
    100% |████████████████████████████████| 1.4MB 8.1MB/s
Collecting wheel
  Downloading https://files.pythonhosted.org/packages/bb/10/44230dd6bf3563b8f227dbf344c908d412ad2ff48066476672f3a72e174e/wheel-0.33.4-py2.py3-none-any.whl
Installing collected packages: pip, wheel
  Found existing installation: pip 19.0.3
    Uninstalling pip-19.0.3:
      Successfully uninstalled pip-19.0.3
Successfully installed pip-19.1.1 wheel-0.33.4
running bdist_wheel
running build
running build_scripts
creating build
creating build/scripts-3.7
copying testbin -> build/scripts-3.7
installing to build/bdist.linux-x86_64/wheel
running install
running install_egg_info
running egg_info
creating example_pkg.egg-info
writing example_pkg.egg-info/PKG-INFO
writing dependency_links to example_pkg.egg-info/dependency_links.txt
writing top-level names to example_pkg.egg-info/top_level.txt
writing manifest file 'example_pkg.egg-info/SOURCES.txt'
reading manifest file 'example_pkg.egg-info/SOURCES.txt'
writing manifest file 'example_pkg.egg-info/SOURCES.txt'
Copying example_pkg.egg-info to build/bdist.linux-x86_64/wheel/example_pkg-0.0.1-py3.7.egg-info
running install_scripts
creating build/bdist.linux-x86_64/wheel/example_pkg-0.0.1.data
creating build/bdist.linux-x86_64/wheel/example_pkg-0.0.1.data/scripts
copying build/scripts-3.7/testbin -> build/bdist.linux-x86_64/wheel/example_pkg-0.0.1.data/scripts
changing mode of build/bdist.linux-x86_64/wheel/example_pkg-0.0.1.data/scripts/testbin to 755
creating build/bdist.linux-x86_64/wheel/example_pkg-0.0.1.dist-info/WHEEL
creating 'dist/example_pkg-0.0.1-py3-none-any.whl' and adding 'build/bdist.linux-x86_64/wheel' to it
adding 'example_pkg-0.0.1.data/scripts/testbin'
adding 'example_pkg-0.0.1.dist-info/METADATA'
adding 'example_pkg-0.0.1.dist-info/WHEEL'
adding 'example_pkg-0.0.1.dist-info/top_level.txt'
adding 'example_pkg-0.0.1.dist-info/RECORD'
removing build/bdist.linux-x86_64/wheel
Processing ./dist/example_pkg-0.0.1-py3-none-any.whl
Installing collected packages: example-pkg
Successfully installed example-pkg-0.0.1
total 40
-rw-r--r-- 1 root root 2197 Jun 30 22:51 activate
-rw-r--r-- 1 root root 1253 Jun 30 22:51 activate.csh
-rw-r--r-- 1 root root 2405 Jun 30 22:51 activate.fish
-rwxr-xr-x 1 root root  247 Jun 30 22:51 easy_install
-rwxr-xr-x 1 root root  247 Jun 30 22:51 easy_install-3.7
-rwxr-xr-x 1 root root  229 Jun 30 22:51 pip
-rwxr-xr-x 1 root root  229 Jun 30 22:51 pip3
-rwxr-xr-x 1 root root  229 Jun 30 22:51 pip3.7
lrwxrwxrwx 1 root root   21 Jun 30 22:51 python -> /usr/local/bin/python
lrwxrwxrwx 1 root root    6 Jun 30 22:51 python3 -> python
-rwxr-xr-x 1 root root   22 Jun 30 22:51 testbin
-rwxr-xr-x 1 root root  225 Jun 30 22:51 wheel

The Linux agents in Azure pipelines have docker available, so we should be able to use the above + a helper like this to cover this case in our automated tests.

@pradyunsg
Copy link
Member

Thanks for filing this issue!

I don't see any good way to work around this from pip's end. We need to use a temporary build directory to help ensure some amount of reproduciblilty in builds.

I'm okay to add a check and a warning for this situation though.

@pradyunsg
Copy link
Member

Thanks @chrahunt for the easy reproduction example! :D

@pradyunsg pradyunsg removed the S: needs triage Issues/PRs that need to be triaged label Jul 1, 2019
@pradyunsg pradyunsg added this to the Print Better Error Messages milestone Jul 1, 2019
@bsolomon1124
Copy link

bsolomon1124 commented Mar 2, 2020

I'm interested in adding a better error message for this case, at the very least.

My understanding of the original post is that the os.X_OK check occurs here:

if os.access(srcfile, os.X_OK):

If True, permissions are carried over to destfile. However, with /tmp mounted noexec, even if a file has its executable permission bits set, the os.access() call will return False.

So it seems like there are two possible routes here:

  1. Improve the error message only; this relies on determining that the temporary directory is mounted noexec (or maybe, check if the resulting permissions of destfile are "right")
  2. Rather than just os.access(), test the file permissions 'on paper' with the stat_result.st_mode that results from os.stat().

Reference for st_mode is at http://man7.org/linux/man-pages/man7/inode.7.html.

@chrahunt
Copy link
Member

chrahunt commented Mar 2, 2020

2. Rather than just os.access(), test the file permissions 'on paper' with the stat_result.st_mode that results from os.stat().

This would be my suggestion.

@bsolomon1124
Copy link

bsolomon1124 commented Mar 3, 2020

Outline of thought process:

My initial thinking was "just test the flags." That would look something like:

def has_any_x_flag(path) -> bool:
    mode = os.stat(path).st_mode
    return bool((mode & 1) or ((mode >> 3) & 1) or ((mode >> 6) & 1))

def has_owner_x_flag(path) -> bool:
    return bool((os.stat(path).st_mode >> 6) & 1)

However, that will fall far short of what os.access() does. For instance, it does not account for the user or group.

In effect what should be tested for is if this directory were not noexec, what would os.access() return?

To that end, I think this implementation most closely mimics os.access(file, os.X_OK) but by looking directly at the stat_result attributes and ignoring mount:

import os

def real_x_flag(path) -> bool:
    st = os.stat(path)
    mode = st.st_mode
    user_x = (mode >> 6) & 1
    group_x = (mode >> 3) & 1
    any_x = mode & 1
    any_x = any((user_x, group_x, any_x))
    if not any_x:
        # Case 1: 0 of 3 -x permissions set, never executable
        return False
    uid = os.getuid()
    if uid == 0:
        # Case 2: user is root; True if any of 3 -x permissions set
        return any_x
    elif (uid == st.st_uid) and user_x:
        # Case 3: user is owner of file and user -x permission is set
        return True
    elif st.st_gid in os.getgroups() and group_x and (uid != st.st_uid):
        # Case 4: group matches, owner does not match
        return True
    elif any_x and (uid != st.st_uid) and (st.st_gid not in os.getgroups()):
        # Edge case: the 'any other' -x is effective if both
        # user and group do not match
        return True
    return False

To compare, something like this could be used:

import itertools as it
import subprocess

def test_x_flag(path):
    perms = 'rwxrwxrwx'
    print(os.stat(path))
    all_modes = map(''.join, it.product('01', repeat=9))
    for m in all_modes:
        octmode = ''.join(str(int(i, 2)) for i in (m[0:3], m[3:6], m[6:9]))
        subprocess.check_output(["sudo", "chmod", octmode, path])
        print(
            m,
            oct(int(m, 2)),
            ''.join(p if i == '1' else '-' for p, i in zip(perms, m)),
            real_x_flag(path),
            os.access(path, os.X_OK),
            sep="\t"
        )

(Tested on paths of differing owner/group combinations.)

Sample output assuming I am brad and:

$ ls -la foo.sh
----------  1 testuser  staff  28 Mar  2 19:46 foo.sh

Then:

$ ./tflag.py foo.sh
os.stat_result(st_mode=32768, st_ino=8627062717, st_dev=16777220, st_nlink=1, st_uid=502, st_gid=20, st_size=28, st_atime=1583196374, st_mtime=1583196372, st_ctime=1583199149)
000000000	0o0	---------	False	False
000000001	0o1	--------x	False	False
000000010	0o2	-------w-	False	False
000000011	0o3	-------wx	False	False
000000100	0o4	------r--	False	False
000000101	0o5	------r-x	False	False
000000110	0o6	------rw-	False	False
000000111	0o7	------rwx	False	False
000001000	0o10	-----x---	True	True
000001001	0o11	-----x--x	True	True

... and so on.

Last but not least, this Python impl. does not handle the setuid or setgid bits, unlike os.access() with its effective_ids param.

Please let me know if you have thoughts on that @chrahunt .

@chrahunt
Copy link
Member

chrahunt commented Mar 3, 2020

We don't need to feel like we have to do what os.access does. Setting all x bits when any is set seems reasonable to me.

We could replace this condition with something like:

if st.st_mode & (stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH):
    ...

This has nice properties:

  1. likely to be the intended outcome - we're just trying to make sure included scripts are executable if they are set as such
  2. easy to reason about
  3. minimal code
  4. easy to test

@bsolomon1124
Copy link

OK, I am more than happy to keep it simple 😉 @chrahunt.

bsolomon1124 added a commit to bsolomon1124/pip that referenced this issue Mar 3, 2020
Closes Issue pypa#6364: use os.stat() rather than os.access()
to correctly determine if srcfile has any executable bits
set.  os.access() will always return False in cases where
the directory is mounted noexec, which can lead to a false
negative and the resulting script not being executable.
@nlhkabu nlhkabu added C: error messages Improving error messages UX User experience related labels Jul 28, 2020
@nlhkabu nlhkabu removed this from the Print Better Error Messages milestone Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: error messages Improving error messages C: wheel The wheel format and 'pip wheel' command UX User experience related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants