PCSC icon indicating copy to clipboard operation
PCSC copied to clipboard

RFC: Stop depending on libsystemd for socket activation

Open EliteTK opened this issue 1 year ago • 12 comments

I've been wanting to implement this for over a year and haven't really finished my original patching attempt. But given recent events (the xz and libsystemd + sshd fiasco) I have decided to give it another go.

I am under no impression that the changes should be included as is, as I've made little to no effort to familiarise myself with the project's coding style and other aspects. But before I commit any more time to this, I wanted to ask for comments to see if this change would even be appealing.

For some background: I am a fan of non-conventional service management and have been working on my own system management suite for some time. I think that socket activation as a concept is quite cool and is not unique to systemd. Unfortunately, a bunch of services which support it, do so by depending on libsystemd which is not often shipped by distributions which do not utilise systemd.

As such, I propose that the dependency on libsystemd is dropped in favour of a simple implementation of the systemd socket activation protocol.

The patch, as it currently stands, tries to match exactly the features which were previously offered by sd_listen_fds and sd_is_socket. Although I've taken some liberties to increase the verbosity of the logging in the latter case.

While inevitably in my time dealing with service management I've perused the systemd sources, I didn't re-implement any of this from memory and instead relied solely on the libsystemd man pages and my own knowledge of how to implement these features. So I don't believe there's any copyright issue here.

EliteTK avatar Mar 31 '24 21:03 EliteTK

If you do not like systemd you can disable it using --disable-libsystemd.

In your case what/who is creating the socket and waiting for an connection?

LudovicRousseau avatar Apr 01 '24 14:04 LudovicRousseau

The issue with --disable-libsystemd is specifically that it disables socket activation support. I want to be able to make use of socket activation without needing to have libsystemd. I don't normally control the build flags of my distributions and even if I did, having to write a stub libsystemd AND handle building pcscd is a lot less convenient than just getting pcscd to support socket activation regardless of the presence of libsystemd.

In my case, I have my own code which does socket activation. Here is an example snippet of code which implements socket activation in a systemd compatible way:

#include <stdio.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <unistd.h>
#include <stdlib.h>
#include <inttypes.h>

int main(int argc, char **argv)
{
	int s = socket(AF_UNIX, SOCK_STREAM, 0);
	if (s != 3) return 1;
	struct sockaddr_un sa = {
		.sun_family = AF_UNIX,
		.sun_path = "/tmp/pcscd.socket",
	};
	unlink(sa.sun_path);
	bind(3, (void *)&sa, sizeof sa);
	listen(3, 128);
	setenv("LISTEN_FDS", "1", 1);
	pid_t pid = getpid();
	char pidbuf[100];
	snprintf(pidbuf, sizeof pidbuf, "%jd", (intmax_t)pid);
	setenv("LISTEN_PID", pidbuf, 1);
	unsetenv("LISTEN_FDNAMES");
	execv(argv[1], argv + 1);
}

This is just example code (and has no error handling) but a fuller example would open the listen socket, notify the supervision suite that the service is ready, and then exec pcscd. Or potentially wait for connections before doing the exec.

In my specific use-case, the point of the socket activation would be to minimise resource usage when pcscd is not needed. But recently I've been experimenting with notification driven service startup which requires either a service readiness notification mechanism or socket activation to work correctly.

For the current userbase of people using pcscd on systems with systemd, the proposed patch would have no impact and would not remove/add any features (aside from reducing runtime dependencies). But, for people on systems without libsystemd, it would open up the possibility to use socket activation with pcscd.

EliteTK avatar Apr 01 '24 15:04 EliteTK

Just to add to the answer. If you search github for code which is implementing the socket activation protocol, it seems I am not alone in implementing it (outside of systemd):

https://github.com/search?q=%22setenv%28%5C%22LISTEN_FDS%5C%22%22+AND+NOT+unsetenv&type=code

EliteTK avatar Apr 01 '24 15:04 EliteTK

What are the distributions you use?

LudovicRousseau avatar Apr 01 '24 15:04 LudovicRousseau

The main distro I personally care about is voidlinux ( https://github.com/void-linux/void-packages/blob/master/srcpkgs/pcsclite/template ) but I would also like to get this working on devuan.

That being said, I don't see why this shouldn't also end up working on all the BSD systems. There are also many other non-systemd distros which either use a no-op libsystemd stub or compile with --disable-libsystemd.

Just as a note to self: I need to make sure that --disable-libsystemd doesn't cause problems when passed to configure if this change is implemented.

EliteTK avatar Apr 01 '24 15:04 EliteTK

It's worth adding. When I initially looked into this feature, I considered adding an additional feature to pcscd where you can pass --socket-activated (or some equivalent command line argument) to make pcscd ignore the result of sd_listen_fds (or lack-thereof) and assume that fd 3 is an appropriate socket. But I discarded the idea.

I am willing to consider this path again if you feel that you would rather accept a new feature than a re-write of an existing one.

EliteTK avatar Apr 01 '24 15:04 EliteTK

I don't think I will integrate your changes. I use libsystemd to reuse existing code/lib.

Feel free to use your own patch on your systems. That is why Free Software is great: you can adapt it.

LudovicRousseau avatar Apr 01 '24 15:04 LudovicRousseau

Lennart Poettering recommends re-implementing such minor functionality instead of depending on all of libsystemd ( https://news.ycombinator.com/item?id=39867126 ). But I respect your decision, although I disagree that maintaining your own patches is a convenient alternative.

What about adding generic socket activation support in the way I suggested (alongside libsystemd)? With a new --socket-activated option which would be able to handle any socket activation scheme? Would you consider that? It would not be code duplication as it would not need to handle all the details of systemd socket activation.

EliteTK avatar Apr 01 '24 15:04 EliteTK

And there is one more compromise I would like to offer, if the one above is not acceptable:

What if the existing code is kept but, when --disable-libsystemd is passed, the new code is added as a fallback?

EliteTK avatar Apr 01 '24 16:04 EliteTK

That would a revert of patch 30e10951f81b9480e788965f89d0d4d0aee909c0

I will need to think about it.

LudovicRousseau avatar Apr 01 '24 16:04 LudovicRousseau

I totally see @EliteTK's point. It doesn't make sense to depend on systemd if we can achieve the same result without it, and with that making the project more flexible.

I use PorteuX, a Slackware based distro, therefore it has no systemd, and to initialize this project during boot we need to implement some ugly workarounds that are anything but reliable.

If @LudovicRousseau doesn't want to implement a systemd-free solution, I believe falling back to @EliteTK's code when the flag --disable-libsystemd is passed would be the reasonable approach.

By the way, thanks for keeping this project alive! :)

fulalas avatar Sep 04 '24 20:09 fulalas

One solution could be that:

  • you implement the functions sd_listen_fds() and sd_is_socket() in a library
  • you provide a pkg-config file named libsystemd.pc that indicates how to link with the above library
  • you configure PCSC as if you were using the real libsystemd
  • no need to modify pcsc-lite

Would that work for you?

LudovicRousseau avatar Sep 06 '24 19:09 LudovicRousseau

No answer since 2 months. I guess my proposal is OK. Closing.

LudovicRousseau avatar Nov 02 '24 17:11 LudovicRousseau

There's a number of outstanding problems with the approach:

  • It still depends on libsystemd, increasing the dependency graph size which puts the project at greater risk of the kind of libxz style security issue that my PR was initially triggered by.
  • While I think in practice it may be possible that this is solved on at least some distributions, such as those which ship elogind, it still requires getting those distributions on board to have pcscd depend on the elogind's libsystemd. It also means that if someone decides to replace libsystemd as part of a wholesale systemd compatible system supervision suite replacement, you would now need some kind of lisbsystemd shim (I think debian does something like this) which is able to dispatch calls to the right libsystemd depending on what systemd replacement you want to use.
  • Finally, (somewhat similar to my first point above but from a non-security perspective) it's still an entire libsystemd dependency to implement 33 lines of code.

But it's fine, I don't think I can convince you so you can keep this closed. I disagree that it's resolved but obviously it's your call on whether you want to take the changes or not.

EliteTK avatar Nov 07 '24 09:11 EliteTK