search

ClusterLabs / libqb

libqb is a library providing high performance logging, tracing, ipc, and poll.
http://clusterlabs.github.io/libqb/
GNU Lesser General Public License v2.1
165 stars 99 forks source link

Default socketdir (`/var/run`) is restricted area #294

Open wferi opened 6 years ago

wferi commented 6 years ago

Hi,

The default directory for file system sockets (/var/run) is generally writable by root only, making file system sockets usable by privileged applications only. On the other hand abstract sockets (used by default on Linux) have no access control whatsoever. Looks like the default socket file directory should be changed to a world-writable one like /tmp to achieve similar behavior on Linux and BSDs for example. This have already come up a couple of times: https://github.com/ClusterLabs/libqb/pull/248#issuecomment-348992701, https://github.com/ClusterLabs/libqb/issues/222#issuecomment-355989054. Is there a good reason to stay with the restricted default? Or not to introduce some means of changing the socket directory from the application (for example a new API call)?

Thanks, Feri.

jirib commented 6 years ago

What? You can create /var/run/corosync* in advance before running the daemons? I see only issue with corosync.pid in /var/run which I'm trying to change with sed -i 's!corosync.pid!corosync/&!' exec/main.c. /tmp is really silly idea.

wferi commented 6 years ago

@jirib, I don't get your point. This issue is about libqb in general. How can you write under /var/run without having any privilege?

chrissie-c commented 6 years ago

You can configure libqb with --socketdir though that's obviously not a good general solution. I think this is partly down to libqb being spun-off from corosync where the daemons it was connected to were all run as root.

Also there is quite a lot of other code in the IPC that still requires root anyway (eg authentication) - of course it could be bypassed in the event of a non-root server socket but it's all work that needs to be done to make it work properly. And, TBH, we've never had the need so far :)

wferi commented 6 years ago

You can configure libqb with --socketdir though that's obviously not a good general solution.

Yes, that's what I currently do. Why isn't that a good general solution? I opened this issue because it wasn't obvious to me. If configuring with --socketdir=/tmp was a good general solution, the default wouldn't be /var/run, I guess. Symlink attacks come to mind, that's why I mentioned setting the socket directory via an API call. Creating a temporary directory would help as well.

I think this is partly down to libqb being spun-off from corosync where the daemons it was connected to were all run as root.

Understandable. But now it begs the question whether running corosync as root is really necessary. It could inherit elevated scheduling priority and does not run plugins anymore.

Also there is quite a lot of other code in the IPC that still requires root anyway (eg authentication)

Could you please provide a more concrete hint? Which IPC operations require privileges?

  • of course it could be bypassed in the event of a non-root server socket but it's all work that needs to be done to make it work properly. And, TBH, we've never had the need so far :)

Now that doxyxml also uses libqb things can get out of hand rather quickly. :)

chrissie-c commented 6 years ago

Why isn't that a good general solution? I opened this issue because it wasn't obvious to me. If configuring with --socketdir=/tmp was a good general solution, the default wouldn't be /var/run, I guess. Symlink attacks come to mind, that's why I mentioned setting the socket directory via an API call.

Standards mainly (which is a fairly feeble excuse I'll grant) - keeping things tidy means utilities like things in familiar places - certainly for system programs.

One of the problems with an API to change the location of the socket is that there is the problem of clients not knowing where servers have put sockets. If a server calls an API to put a socket in /tmp rather than /var/run how does the client know to look in /tmp? If they're from the same package then that's easy to manage but it's not always the case.

But now it begs the question whether running corosync as root is really necessary. It could inherit elevated scheduling priority and does not run plugins any more.

You've seen the code in knet - I'm not about to de-root that any time soon :) Yes, I know you've de-rooted the tests but that's not feasible for a performance-sensitive thing like corosync. And now we can add/remove links at will in corosync we need to be able to configure those sockets on the fly.

Could you please provide a more concrete hint? Which IPC operations require privileges? The one that comes to mind immediately is the authentication. There are maybe others, but I'm not sure TBH. IFF that's all there is then it's not too hard to bypass I suppose. It would just need a flag in the connection struct saying it's unprivileged

Now that doxyxml also uses libqb things can get out of hand rather quickly. :)

Yeah, sorry about that - I didn't fancy writing yet another hash table (which is literally all it uses from libqb) :/

kgaillot commented 6 years ago

Why isn't that a good general solution? I opened this issue because it wasn't obvious to me. If configuring with --socketdir=/tmp was a good general solution, the default wouldn't be /var/run, I guess. Symlink attacks come to mind, that's why I mentioned setting the socket directory via an API call.

Standards mainly (which is a fairly feeble excuse I'll grant) - keeping things tidy means utilities like things in familiar places - certainly for system programs.

Some daemons (e.g. postfix) use /var/spool, so /var/spool/qb (or libqb) could be created at package install time. I would make that directory root-only, for well-known daemons to use, and create a subdirectory (./public?) that would have /tmp permissions, for non-daemon programs.

For backward compatibility, maybe libqb could create a symlink from /var/run when the caller is root.

The only downside I see is leaving more files behind in an unclean exit; /var/run will at least be cleaned at boot, but /var/spool/qb wouldn't.

wferi commented 6 years ago

Aren't we comparing apples to oranges here? Since libqb was split out of corosync, it isn't an application anymore (like postfix). It became a library , and as such it shouldn't have (much less force its) policy on applications using it. This doesn't look easy the least, with all those privileges having been taken granted in the past, but might be something to aim for.

chrissie-c commented 6 years ago

I agree totally, and sometimes it's hard for us (well, me perhaps as I mainly work on knet & corosync) to get out of the 'libqb supports the cluster; mindset. Sometimes we need help with this :)