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

BSD: provide a fallback for user_runtime_dir #201

Merged
merged 1 commit into from
Jul 6, 2023

Conversation

RayyanAnsari
Copy link
Contributor

On *BSD platforms, the /var/run/user/{uid} directory is not always created. Provide a fallback of /tmp/runtime-{uid} if that path does not exist.

Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test and make sure CI passes.

@RayyanAnsari RayyanAnsari force-pushed the bsd-runtime-dir branch 2 times, most recently from 864eeff to 48550bd Compare July 4, 2023 18:15
@RayyanAnsari
Copy link
Contributor Author

RayyanAnsari commented Jul 4, 2023

src/platformdirs/unix.py:164:28: S108 Probable insecure usage of temporary file or directory: "/tmp/runtime-"
Why is this considered to be insecure?

src/platformdirs/unix.py Outdated Show resolved Hide resolved
@RayyanAnsari RayyanAnsari force-pushed the bsd-runtime-dir branch 2 times, most recently from 288732e to 786a8bd Compare July 5, 2023 23:02
@RayyanAnsari RayyanAnsari marked this pull request as ready for review July 5, 2023 23:02
@RayyanAnsari RayyanAnsari requested a review from gaborbernat July 6, 2023 00:02
tests/test_unix.py Outdated Show resolved Hide resolved
@gaborbernat gaborbernat marked this pull request as draft July 6, 2023 15:57
On *BSD platforms, the /var/run/user/{uid} directory is not always
created. Provide a fallback of /tmp/runtime-{uid} if that path does
not exist and update the test to reflect these changes.
Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gaborbernat gaborbernat marked this pull request as ready for review July 6, 2023 21:48
@ThomasWaldmann
Copy link
Contributor

@bket can you check?

@gaborbernat gaborbernat merged commit 5f7eb47 into tox-dev:main Jul 6, 2023
bket added a commit to bket/borg that referenced this pull request Jul 7, 2023
A borgbackup-2.0.0b6 test fails on OpenBSD with the message below.

```
=================================== FAILURES ===================================
_____________________________ test_get_runtime_dir _____________________________

path = '/run/user/55/borg', mode = 511, pretty_deadly = True

    def ensure_dir(path, mode=stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO, pretty_deadly=True):
        """
        Ensures that the dir exists with the right permissions.
        1) Make sure the directory exists in a race-free operation
        2) If mode is not None and the directory has been created, give the right
        permissions to the leaf directory. The current umask value is masked out first.
        3) If pretty_deadly is True, catch exceptions, reraise them with a pretty
        message.
        Returns if the directory has been created and has the right permissions,
        An exception otherwise. If a deadly exception happened it is reraised.
        """
        try:
>           os.makedirs(path, mode=mode, exist_ok=True)

build/lib.openbsd-7.3-amd64-cpython-310/borg/helpers/fs.py:37:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
...

If $XDG_RUNTIME_DIR is not set `platformdirs.user_runtime_dir()` returns
one of 3 different paths
(tox-dev/platformdirs#201. Proposed fix is
to check if `get_runtime_dir()` returns one of these paths.
bket added a commit to bket/borg that referenced this pull request Jul 7, 2023
A borgbackup-2.0.0b6 test fails on OpenBSD with the message below.

```
=================================== FAILURES ===================================
_____________________________ test_get_runtime_dir _____________________________

path = '/run/user/55/borg', mode = 511, pretty_deadly = True

    def ensure_dir(path, mode=stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO, pretty_deadly=True):
        """
        Ensures that the dir exists with the right permissions.
        1) Make sure the directory exists in a race-free operation
        2) If mode is not None and the directory has been created, give the right
        permissions to the leaf directory. The current umask value is masked out first.
        3) If pretty_deadly is True, catch exceptions, reraise them with a pretty
        message.
        Returns if the directory has been created and has the right permissions,
        An exception otherwise. If a deadly exception happened it is reraised.
        """
        try:
>           os.makedirs(path, mode=mode, exist_ok=True)

build/lib.openbsd-7.3-amd64-cpython-310/borg/helpers/fs.py:37:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
```

If `$XDG_RUNTIME_DIR` is not set `platformdirs.user_runtime_dir()`
returns one of 3 different paths
(tox-dev/platformdirs#201. Proposed fix is
to check if `get_runtime_dir()` returns one of these paths.
bket added a commit to bket/borg that referenced this pull request Jul 7, 2023
A borgbackup-2.0.0b6 test fails on OpenBSD with the message below.

```
=================================== FAILURES ===================================
_____________________________ test_get_runtime_dir _____________________________

path = '/run/user/55/borg', mode = 511, pretty_deadly = True

    def ensure_dir(path, mode=stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO, pretty_deadly=True):
        """
        Ensures that the dir exists with the right permissions.
        1) Make sure the directory exists in a race-free operation
        2) If mode is not None and the directory has been created, give the right
        permissions to the leaf directory. The current umask value is masked out first.
        3) If pretty_deadly is True, catch exceptions, reraise them with a pretty
        message.
        Returns if the directory has been created and has the right permissions,
        An exception otherwise. If a deadly exception happened it is reraised.
        """
        try:
>           os.makedirs(path, mode=mode, exist_ok=True)

build/lib.openbsd-7.3-amd64-cpython-310/borg/helpers/fs.py:37:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
```

If `$XDG_RUNTIME_DIR` is not set `platformdirs.user_runtime_dir()`
returns one of 3 different paths
(tox-dev/platformdirs#201. Proposed fix is
to check if `get_runtime_dir()` returns one of these paths.
bket added a commit to bket/borg that referenced this pull request Jul 7, 2023
A borgbackup-2.0.0b6 test fails on OpenBSD with the message below.

```
=================================== FAILURES ===================================
_____________________________ test_get_runtime_dir _____________________________

path = '/run/user/55/borg', mode = 511, pretty_deadly = True

    def ensure_dir(path, mode=stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO, pretty_deadly=True):
        """
        Ensures that the dir exists with the right permissions.
        1) Make sure the directory exists in a race-free operation
        2) If mode is not None and the directory has been created, give the right
        permissions to the leaf directory. The current umask value is masked out first.
        3) If pretty_deadly is True, catch exceptions, reraise them with a pretty
        message.
        Returns if the directory has been created and has the right permissions,
        An exception otherwise. If a deadly exception happened it is reraised.
        """
        try:
>           os.makedirs(path, mode=mode, exist_ok=True)

build/lib.openbsd-7.3-amd64-cpython-310/borg/helpers/fs.py:37:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
```

If `$XDG_RUNTIME_DIR` is not set `platformdirs.user_runtime_dir()`
returns one of 3 different paths
(tox-dev/platformdirs#201. Proposed fix is
to check if `get_runtime_dir()` returns one of these paths.
bket added a commit to bket/borg that referenced this pull request Jul 7, 2023
A borgbackup-2.0.0b6 test fails on OpenBSD with the message below.

```
=================================== FAILURES ===================================
_____________________________ test_get_runtime_dir _____________________________

path = '/run/user/55/borg', mode = 511, pretty_deadly = True

    def ensure_dir(path, mode=stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO, pretty_deadly=True):
        """
        Ensures that the dir exists with the right permissions.
        1) Make sure the directory exists in a race-free operation
        2) If mode is not None and the directory has been created, give the right
        permissions to the leaf directory. The current umask value is masked out first.
        3) If pretty_deadly is True, catch exceptions, reraise them with a pretty
        message.
        Returns if the directory has been created and has the right permissions,
        An exception otherwise. If a deadly exception happened it is reraised.
        """
        try:
>           os.makedirs(path, mode=mode, exist_ok=True)

build/lib.openbsd-7.3-amd64-cpython-310/borg/helpers/fs.py:37:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
```

If `$XDG_RUNTIME_DIR` is not set `platformdirs.user_runtime_dir()`
returns one of 3 different paths
(tox-dev/platformdirs#201). Proposed fix is
to check if `get_runtime_dir()` returns one of these paths.
bket added a commit to bket/borg that referenced this pull request Jul 7, 2023
A borgbackup-2.0.0b6 test fails on OpenBSD with the message below.

```
=================================== FAILURES ===================================
_____________________________ test_get_runtime_dir _____________________________

path = '/run/user/55/borg', mode = 511, pretty_deadly = True

    def ensure_dir(path, mode=stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO, pretty_deadly=True):
        """
        Ensures that the dir exists with the right permissions.
        1) Make sure the directory exists in a race-free operation
        2) If mode is not None and the directory has been created, give the right
        permissions to the leaf directory. The current umask value is masked out first.
        3) If pretty_deadly is True, catch exceptions, reraise them with a pretty
        message.
        Returns if the directory has been created and has the right permissions,
        An exception otherwise. If a deadly exception happened it is reraised.
        """
        try:
>           os.makedirs(path, mode=mode, exist_ok=True)

build/lib.openbsd-7.3-amd64-cpython-310/borg/helpers/fs.py:37:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
```

If `$XDG_RUNTIME_DIR` is not set `platformdirs.user_runtime_dir()`
returns one of 3 different paths
(tox-dev/platformdirs#201). Proposed fix is
to check if `get_runtime_dir()` returns one of these paths.
@sthen
Copy link

sthen commented Jul 16, 2023

src/platformdirs/unix.py:164:28: S108 Probable insecure usage of temporary file or directory: "/tmp/runtime-" Why is this considered to be insecure?

Because another user can easily create the directory (it has a predictable name and is in a world-writable temp dir). To be safe the dir would need to be created and checked that it has been created with correct ownership and permissions (and probably fail if not).

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 this pull request may close these issues.

5 participants