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

ipc: fix force-filesystem-sockets #358

Merged
merged 1 commit into from
Jun 24, 2019
Merged

ipc: fix force-filesystem-sockets #358

merged 1 commit into from
Jun 24, 2019

Conversation

chrissie-c
Copy link
Contributor

the /etc/libqb/force-filesystem-sockets option got broken for some
applications in the last security update.

@jfriesse
Copy link
Member

Seems to work just fine on Linux with both fs and non-fs sockets. Sadly doesn't work on FreeBSD when running ipc.test.

@jnpkrn
Copy link
Contributor

jnpkrn commented Jun 19, 2019

It cannot work where abstract sockets are not supported (such as FreeBSD
as stated by Honza), that's not the approach for a portable fix.

Am I right to think it's related to my recent comment
#325 (comment)
?

Note that the problem may be (with FreeBSD stock package is) actually
self-resolved with passing a specific prefix (like /usr/local) to the
build configuration whereby the respective /usr/local/var/run is
actually world-writeable, which wins it for the non-root daemons (not
sure how reliably, though).

Sure enough, end-users and downstreams may replicate this "win" with
--with-socket-dir=/var/libqb-sockets and injecting something along
the lines of install -M1777 -d /var/libqb-sockets to the package's
install recipe, but you get other problems -- in admin's role -- to
deal with, like dead sockets accrued behind (there's a fitting touch
on another concurrently solved problem starting with
corosync/corosync#429
). At least with some modern Linux distributions, /var/run is tmpfs.
And there are also various predictable filesystem hierarchy conventions
that would be suddenly violated (OK, so use /var/lib/libqb-sockets
instead, not really addressing the former concern).

What about using /tmp/ right away? Bad idea, it would mean
self-inflicted unintentional DoSes, since it's all-public namespace
where random "meaningful API identifiers" clashes (with whatever other
files/dirs stored there temporarily) are imminent.

What about using /tmp/libqb-sockets? Better, but extra precautions
needed in libqb code, but on the bright side, could at least allow for
reasonably non-conflicting probe-socket-liveness and possibly its
removal before installing the current live one with bind).

For good measure, two things can be added:

  1. have /etc/libqb/force-filesystem-sockets contain an actually
    actionable content, in this case, an absolute path to override
    the preconfigured socket dir (default would still be used if
    that file is empty or contains something not resembling an
    absolute path)

  2. have early system boot run a script for creating such a directory
    anew (clean) and owned by root, so nobody unprivileged can have
    it replaced with something else/symlink etc.

In Linux/systemd settings, this schema could then be completed with
force-filesystem-sockets containing a package-build-time constant
(likely all-time-constant, for absolute compatibility), and the same
constant would then be referenced in tmpfiles.d config resulting
in the file created on boot as mentioned.

Note that 1. would also afford a convenient multi-container sharing
if need be, without a necessity to remap mount points explicitly;
the containers that need to have an IPC conversation with each other
would posses paired /etc/libqb/force-filesystem-sockets unique
pointers to some shared (and thanks to uniqueness, conceivably
all-shared) filesystem.

Just a dump of some of my thoughts here.

@jnpkrn
Copy link
Contributor

jnpkrn commented Jun 20, 2019

.... or replace /tmp/libqb-sockets with /var/run/libqb-sockets, indeed
(depends on universality/prevailing conventions with given OS).

Another idea would be to make force-filesystem-sockets a symlink
rather than plain text "link", but that would prevent in-place comment,
which would likely be very desirable (brief explanation of what that
file is good for and how to use it properly incl. combining with
containers). Alternatively (or in addition), there could be
libqb(4) man page to deliver exhaustive documentation for that
file, and others in /etc/libqb hierarchy if there will more in
the future).

@jnpkrn
Copy link
Contributor

jnpkrn commented Jun 20, 2019

So the updated patch now effectively reverts #248. What's the point?
If it is indeed desired to get rid of a feature, it makes more sense
to do a complete revert, doesn't it? If it is indeed desired to provide
support for container use cases, the way forward is not to disable it.

I think the right solution to the problem is along the lines mentioned
above, basically externalizing some portion of administrative burden
out to direct consumers/downstream maintainers+integrators and possibly
administrators (in that order, the latter can rest on their laurels if
the former get the job well done). This will also require updating
INSTALL file with instructions about what coordination around
--with-socket-dir is needed.

Btw. the permissions issue we are talking about here was precisely
described in #248 (comment)
(I wasn't attentive to that thread back then anymore, wasn't related to
my commits).

the /etc/libqb/force-filesystem-sockets option got broken for some
applications in the last security update.
@jfriesse
Copy link
Member

Latest patch seems to work fine on Linux with or without forced fs sockets and FreeBSD. So Ack by me.

@chrissie-c chrissie-c merged commit f1bf5d9 into ClusterLabs:master Jun 24, 2019
@chrissie-c
Copy link
Contributor Author

Thanks for the review.

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.

3 participants