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

UNIX socket permission bits #4155

Open
andreymal opened this issue Oct 7, 2019 · 13 comments
Open

UNIX socket permission bits #4155

andreymal opened this issue Oct 7, 2019 · 13 comments

Comments

@andreymal
Copy link

I want to use a unix socket, but it's impossible when umask is 0022 or more strict, because the nginx www-data user can't connect and prints "Permission denied" error. I want to change the permission bits for the unix socket without touching umask (a strict umask is needed for other things of my project).

There was a PR #4002, but it's closed.

I tried to chmod after on_startup signal:

class MyApp:
    def __init__(self, config):
        self.config = config
        self.webapp = web.Application()
        self.webapp.on_startup.append(self._on_startup)
        # ...

    def run_app(self):
        web.run_app(
            self.webapp,
            path=self.config.unix_socket,
            # ...
        )

    async def _on_startup(self, webapp):
        os.chmod(self.config.unix_socket, self.config.unix_socket_mode)

But the on_startup signal is called before creating the socket, and os.chmod raises "No such file or directory: 'webapp.sock'" error.

Please implement an option for this or explain how to do it with the current version of aiohttp

Or maybe add on_listen signal? :)

@asvetlov
Copy link
Member

asvetlov commented Oct 8, 2019

Changing the default unix socket permission is a bad idea because it opens a security hole.

Please note: you can create UNIX socket file before starting listening on it:

>>> import socket                                                                                                                             
>>> s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)                                                                                     
>>> s.bind('socket.sock')                                                                                                                     
>>> s.close()                                                                                                                                 
>>> import os                                                                                                                                 
>>>  os.chmod('socket.sock', 0o777)
>>> aiohttp.run(...)

@webknjaz
Copy link
Member

webknjaz commented Oct 8, 2019

Normally, you'd want to make the privileges stricter, not more open...

@asvetlov should we implement support for the socket privilege adjustment on the framework level?

@andreymal can't you just create a shared group for this purpose?

@andreymal
Copy link
Author

@webknjaz

can't you just create a shared group for this purpose?

It's exactly what I want! But the shared group will not work without correct permission bits for the socket (at least chmod g+w is required), and that's why this issue exists.

@andreymal
Copy link
Author

andreymal commented Oct 8, 2019

@asvetlov

you can create UNIX socket file before starting listening on it:

Unfortunately it does not work, because asyncio create_unix_server does os.remove automatically:

$ ls -l aiohttp.sock 
srwxrwx--- 1 andreymal http 0 окт  8 21:03 aiohttp.sock

$ python -c 'from aiohttp import web; web.run_app(web.Application(), path="aiohttp.sock")'
======== Running on http://unix:aiohttp.sock: ========
(Press CTRL+C to quit)
^C

$ ls -l aiohttp.sock 
srwx------ 1 andreymal andreymal 0 окт  8 21:10 aiohttp.sock

Actually this code works (based on aiohttp create_unix_server):

path = 'aiohttp.sock'
sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)

try:
    if stat.S_ISSOCK(os.stat(path).st_mode):
        os.remove(path)
except FileNotFoundError:
    pass
except OSError as err:
    logger.error('Unable to check or remove stale UNIX socket '
                 '%r: %r', path, err)

try:
    sock.bind(path)
except OSError as exc:
    sock.close()
    if exc.errno == errno.EADDRINUSE:
        msg = f'Address {path!r} is already in use'
        raise OSError(errno.EADDRINUSE, msg) from None
    else:
        raise
except:
    sock.close()
    raise

os.chmod(path, 0o770)
web.run_app(app, sock=sock)

But I think this is too complicated :)

@asvetlov
Copy link
Member

asvetlov commented Oct 8, 2019

@andreymal sorry, I didn't check my version before posting.

sock snippet looks overcomplicated, agree.
I'm open for proposals, especially if they don't blow up a list of accepted arguments too much.

on_listen is also an interesting option. What is the full signature?
on_listen(app) is not enough.
on_listen(app, site) maybe, called several times for every listened web.BaseSite?

gwy15 added a commit to gwy15/wechat-push that referenced this issue Nov 2, 2019
@vitto32
Copy link

vitto32 commented Apr 16, 2020

I'm trying to make aiohttp work in a Plesk environment but I'm facing this exact same issue.

What I'm doing right to make it work is:

s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
os.unlink(SOCKET_NAME)
s.bind(SOCKET_NAME)
os.chmod(SOCKET_NAME, 0o777)    
web.run_app(app, sock=s)

is it the right way?

@webknjaz
Copy link
Member

@vitto32 that should work, I guess. But the privileges seem too broad. Though it's up to you and depends on your needs.

@vitto32
Copy link

vitto32 commented Apr 20, 2020

@webknjaz yep that works.
I know the privileges are broad, but nginx and vhost users have different groups (in plesk).

@louisabraham
Copy link

louisabraham commented May 17, 2020

666 works on my machine with nginx!
I think it would be great if run_app did this automatically.

@guusbertens
Copy link
Contributor

I stumbled into this too. I don't think it's safe if aiohttp would always set the permissions to 666 - we don't know how people are already using aiohttp.
I do think it makes sense for run_app to have a way to set Unix socket permissions. Ideally something like this:

run_app(app, path="/tmp/app.socket", perms=0o666)

I have prepared code that does this, and I'd be happy to share it, but there are many caveats that need discussing - not only pertaining to my code, but also to Unix sockets in general. Should they be discussed here, or should I open a PR for this?

@guusbertens
Copy link
Contributor

Hmm, the longer I look at this, the more difficult it gets. asyncio's loop.create_unix_socket doesn't support permissions, but this is the only place where permissions can be set without creating races, and without code duplication.

@samthesuperhero
Copy link

chmod(args.socket, 0o777)

after

    aiorunner = web.AppRunner(app)
    loop.run_until_complete(aiorunner.setup())
    aiosite = web.UnixSite(aiorunner, args.socket)
    loop.run_until_complete(aiosite.start())

solved the issue for me.

But I would prefer the better solution of course.

Kyle-Verhoog added a commit to Kyle-Verhoog/dd-apm-test-agent that referenced this issue Aug 2, 2022
The permissions of the socket created by aiohttp aren't as permissive as
the ones set by the real agent. (see
aio-libs/aiohttp#4155)

from @devinsba:

test agent:

```
cnb@my-app:/var/run/datadog$ ls -la
total 4
drwxr-xr-x 2 root root   60 Aug  1 21:24 .
drwxr-xr-x 1 root root 4096 Aug  1 21:24 ..
srwxr-xr-x 1 root root    0 Aug  1 21:24 apm.socket
```

real agent:

```
cnb@my-app:/var/run/datadog$ ls -la
total 4
drwxr-xr-x 2 root root   80 Aug  1 21:30 .
drwxr-xr-x 1 root root 4096 Aug  1 21:31 ..
srwx-w--w- 1 root root    0 Aug  1 21:30 apm.socket
```
Kyle-Verhoog added a commit to Kyle-Verhoog/dd-apm-test-agent that referenced this issue Aug 2, 2022
The permissions of the socket created by aiohttp aren't as permissive as
the ones set by the real agent. (see
aio-libs/aiohttp#4155)

from @devinsba:

test agent:

```
cnb@my-app:/var/run/datadog$ ls -la
total 4
drwxr-xr-x 2 root root   60 Aug  1 21:24 .
drwxr-xr-x 1 root root 4096 Aug  1 21:24 ..
srwxr-xr-x 1 root root    0 Aug  1 21:24 apm.socket
```

real agent:

```
cnb@my-app:/var/run/datadog$ ls -la
total 4
drwxr-xr-x 2 root root   80 Aug  1 21:30 .
drwxr-xr-x 1 root root 4096 Aug  1 21:31 ..
srwx-w--w- 1 root root    0 Aug  1 21:30 apm.socket
```
Kyle-Verhoog added a commit to Kyle-Verhoog/dd-apm-test-agent that referenced this issue Aug 2, 2022
The permissions of the socket created by aiohttp aren't as permissive as
the ones set by the real agent. (see
aio-libs/aiohttp#4155)

from @devinsba:

test agent:

```
cnb@my-app:/var/run/datadog$ ls -la
total 4
drwxr-xr-x 2 root root   60 Aug  1 21:24 .
drwxr-xr-x 1 root root 4096 Aug  1 21:24 ..
srwxr-xr-x 1 root root    0 Aug  1 21:24 apm.socket
```

real agent:

```
cnb@my-app:/var/run/datadog$ ls -la
total 4
drwxr-xr-x 2 root root   80 Aug  1 21:30 .
drwxr-xr-x 1 root root 4096 Aug  1 21:31 ..
srwx-w--w- 1 root root    0 Aug  1 21:30 apm.socket
```

aiohttp doesn't seem to have a configurable way
to set these permissions but it does let us create and
manage a socket ourselves, so let's try that.
Kyle-Verhoog added a commit to Kyle-Verhoog/dd-apm-test-agent that referenced this issue Aug 2, 2022
The permissions of the socket created by aiohttp aren't as permissive as
the ones set by the real agent. (see
aio-libs/aiohttp#4155)

from @devinsba:

test agent:

```
cnb@my-app:/var/run/datadog$ ls -la
total 4
drwxr-xr-x 2 root root   60 Aug  1 21:24 .
drwxr-xr-x 1 root root 4096 Aug  1 21:24 ..
srwxr-xr-x 1 root root    0 Aug  1 21:24 apm.socket
```

real agent:

```
cnb@my-app:/var/run/datadog$ ls -la
total 4
drwxr-xr-x 2 root root   80 Aug  1 21:30 .
drwxr-xr-x 1 root root 4096 Aug  1 21:31 ..
srwx-w--w- 1 root root    0 Aug  1 21:30 apm.socket
```

aiohttp doesn't seem to have a configurable way
to set these permissions but it does let us create and
manage a socket ourselves, so let's try that.
Kyle-Verhoog added a commit to Kyle-Verhoog/dd-apm-test-agent that referenced this issue Aug 2, 2022
The permissions of the socket created by aiohttp aren't as permissive as
the ones set by the real agent. (see
aio-libs/aiohttp#4155)

from @devinsba:

test agent:

```
cnb@my-app:/var/run/datadog$ ls -la
total 4
drwxr-xr-x 2 root root   60 Aug  1 21:24 .
drwxr-xr-x 1 root root 4096 Aug  1 21:24 ..
srwxr-xr-x 1 root root    0 Aug  1 21:24 apm.socket
```

real agent:

```
cnb@my-app:/var/run/datadog$ ls -la
total 4
drwxr-xr-x 2 root root   80 Aug  1 21:30 .
drwxr-xr-x 1 root root 4096 Aug  1 21:31 ..
srwx-w--w- 1 root root    0 Aug  1 21:30 apm.socket
```

aiohttp doesn't seem to have a configurable way
to set these permissions but it does let us create and
manage a socket ourselves, so let's try that.
Kyle-Verhoog added a commit to DataDog/dd-apm-test-agent that referenced this issue Aug 2, 2022
The permissions of the socket created by aiohttp aren't as permissive as
the ones set by the real agent. (see
aio-libs/aiohttp#4155)

from @devinsba:

test agent:

```
cnb@my-app:/var/run/datadog$ ls -la
total 4
drwxr-xr-x 2 root root   60 Aug  1 21:24 .
drwxr-xr-x 1 root root 4096 Aug  1 21:24 ..
srwxr-xr-x 1 root root    0 Aug  1 21:24 apm.socket
```

real agent:

```
cnb@my-app:/var/run/datadog$ ls -la
total 4
drwxr-xr-x 2 root root   80 Aug  1 21:30 .
drwxr-xr-x 1 root root 4096 Aug  1 21:31 ..
srwx-w--w- 1 root root    0 Aug  1 21:30 apm.socket
```

aiohttp doesn't seem to have a configurable way
to set these permissions but it does let us create and
manage a socket ourselves, so let's try that.
arcan1s added a commit to arcan1s/ahriman that referenced this issue Jan 4, 2023
Some commands were made unsafe in old versions, but nowadays they can be
run without having special privileges.

There was also a bug in which status commands were not available if you
are not ahriman user and unix socket is used. It has been fixed by
switching to manual socket creation (see also
aio-libs/aiohttp#4155)
arcan1s added a commit to arcan1s/ahriman that referenced this issue Jan 4, 2023
Some commands were made unsafe in old versions, but nowadays they can be
run without having special privileges.

There was also a bug in which status commands were not available if you
are not ahriman user and unix socket is used. It has been fixed by
switching to manual socket creation (see also
aio-libs/aiohttp#4155)
arcan1s added a commit to arcan1s/ahriman that referenced this issue Jan 4, 2023
Some commands were made unsafe in old versions, but nowadays they can be
run without having special privileges.

There was also a bug in which status commands were not available if you
are not ahriman user and unix socket is used. It has been fixed by
switching to manual socket creation (see also
aio-libs/aiohttp#4155)
@Dreamsorcerer
Copy link
Member

I'll have to take a look at this some time. Recently, I've just been using a systemd hack:

ExecStartPost=sleep 0.8
ExecStartPost=!chown app-user:www-data /run/app-user/app.sock
ExecStartPost=!chmod 770 /run/app-user/app.sock

arcan1s added a commit to arcan1s/ahriman that referenced this issue Sep 30, 2024
Some commands were made unsafe in old versions, but nowadays they can be
run without having special privileges.

There was also a bug in which status commands were not available if you
are not ahriman user and unix socket is used. It has been fixed by
switching to manual socket creation (see also
aio-libs/aiohttp#4155)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants