From fdd429cd587737840a8ca08c2eb59b63423d4c3e Mon Sep 17 00:00:00 2001 From: Mike Looijmans Date: Fri, 12 Jan 2024 09:24:48 +0100 Subject: [PATCH 1/6] x0vncserver: Add support for systemd socket activation systemd can pass in sockets as file descriptors 3 and beyond. Allows the server to use socket activation. When triggered by systemd, no other listening sockets (e.g. rfbport) will be activated. Signed-off-by: Mike Looijmans --- CMakeLists.txt | 3 ++ unix/x0vncserver/CMakeLists.txt | 6 +++ unix/x0vncserver/x0vncserver.cxx | 81 +++++++++++++++++++++----------- unix/x0vncserver/x0vncserver.man | 4 +- 4 files changed, 65 insertions(+), 29 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e8d8d85d43..6028fadfa5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -301,6 +301,9 @@ if(UNIX AND NOT APPLE) endif() endif() +# check for systemd support (socket activation) +pkg_check_modules(LIBSYSTEMD libsystemd) + # Generate config.h and make sure the source finds it configure_file(config.h.in config.h) add_definitions(-DHAVE_CONFIG_H) diff --git a/unix/x0vncserver/CMakeLists.txt b/unix/x0vncserver/CMakeLists.txt index 31da51183a..08b346f1a6 100644 --- a/unix/x0vncserver/CMakeLists.txt +++ b/unix/x0vncserver/CMakeLists.txt @@ -21,6 +21,12 @@ target_include_directories(x0vncserver PUBLIC ${CMAKE_SOURCE_DIR}/unix) target_include_directories(x0vncserver PUBLIC ${CMAKE_SOURCE_DIR}/common) target_link_libraries(x0vncserver tx rfb network rdr unixcommon) +# systemd support (socket activation) +if (LIBSYSTEMD_FOUND) + add_definitions(-DHAVE_SYSTEMD_H) + target_link_libraries(x0vncserver ${LIBSYSTEMD_LIBRARIES}) +endif() + if(X11_FOUND AND X11_XTest_LIB) add_definitions(-DHAVE_XTEST) target_link_libraries(x0vncserver ${X11_XTest_LIB}) diff --git a/unix/x0vncserver/x0vncserver.cxx b/unix/x0vncserver/x0vncserver.cxx index f87eb61e4a..5e9a7a570c 100644 --- a/unix/x0vncserver/x0vncserver.cxx +++ b/unix/x0vncserver/x0vncserver.cxx @@ -38,6 +38,9 @@ #include #include #include +#ifdef HAVE_SYSTEMD_H +# include +#endif #include #include @@ -113,6 +116,25 @@ static void CleanupSignalHandler(int /*sig*/) caughtSignal = true; } +#ifdef HAVE_SYSTEMD_H +static int createSystemdListeners(std::list *listeners) +{ + int count = sd_listen_fds(0); + + for (int i = 0; i < count; ++i) { + /* systemd sockets start at FD 3 */ + listeners->push_back(new TcpListener(3 + i)); + } + + return count; +} +#else +static int createSystemdListeners(std::list *) +{ + return 0; +} +#endif + class FileTcpFilter : public TcpFilter { @@ -300,35 +322,40 @@ int main(int argc, char** argv) VNCServerST server(desktopName, &desktop); - if (rfbunixpath.getValueStr()[0] != '\0') { - listeners.push_back(new network::UnixListener(rfbunixpath, rfbunixmode)); - vlog.info("Listening on %s (mode %04o)", (const char*)rfbunixpath, (int)rfbunixmode); - } - - if ((int)rfbport != -1) { - std::list tcp_listeners; - const char *addr = interface; - - if (strcasecmp(addr, "all") == 0) - addr = 0; - if (localhostOnly) - createLocalTcpListeners(&tcp_listeners, (int)rfbport); - else - createTcpListeners(&tcp_listeners, addr, (int)rfbport); - - if (!tcp_listeners.empty()) { - listeners.splice (listeners.end(), tcp_listeners); - vlog.info("Listening for VNC connections on %s interface(s), port %d", - localhostOnly ? "local" : (const char*)interface, - (int)rfbport); + if (createSystemdListeners(&listeners)) { + // When systemd is in charge of listeners, do not listen to anything else + vlog.info("Listening on systemd sockets"); + } else { + if (rfbunixpath.getValueStr()[0] != '\0') { + listeners.push_back(new network::UnixListener(rfbunixpath, rfbunixmode)); + vlog.info("Listening on %s (mode %04o)", (const char*)rfbunixpath, (int)rfbunixmode); } - FileTcpFilter fileTcpFilter(hostsFile); - if (strlen(hostsFile) != 0) - for (std::list::iterator i = listeners.begin(); - i != listeners.end(); - i++) - (*i)->setFilter(&fileTcpFilter); + if ((int)rfbport != -1) { + std::list tcp_listeners; + const char *addr = interface; + + if (strcasecmp(addr, "all") == 0) + addr = 0; + if (localhostOnly) + createLocalTcpListeners(&tcp_listeners, (int)rfbport); + else + createTcpListeners(&tcp_listeners, addr, (int)rfbport); + + if (!tcp_listeners.empty()) { + listeners.splice (listeners.end(), tcp_listeners); + vlog.info("Listening for VNC connections on %s interface(s), port %d", + localhostOnly ? "local" : (const char*)interface, + (int)rfbport); + } + + FileTcpFilter fileTcpFilter(hostsFile); + if (strlen(hostsFile) != 0) + for (std::list::iterator i = listeners.begin(); + i != listeners.end(); + i++) + (*i)->setFilter(&fileTcpFilter); + } } if (listeners.empty()) { diff --git a/unix/x0vncserver/x0vncserver.man b/unix/x0vncserver/x0vncserver.man index 359538656f..0ba195ad0f 100644 --- a/unix/x0vncserver/x0vncserver.man +++ b/unix/x0vncserver/x0vncserver.man @@ -61,7 +61,7 @@ DISPLAY environment variable. Specifies the TCP port on which x0vncserver listens for connections from viewers (the protocol used in VNC is called RFB - "remote framebuffer"). The default port is 5900. Specify \fB-1\fP to disable listening on a TCP -port. +port. Ignored when activated by a systemd socket. . .TP .B \-UseIPv4 @@ -74,7 +74,7 @@ Use IPv6 for incoming and outgoing connections. Default is on. .TP .B \-rfbunixpath \fIpath\fP Specifies the path of a Unix domain socket on which x0vncserver listens for -connections from viewers. +connections from viewers. Ignored when activated by a systemd socket. . .TP .B \-rfbunixmode \fImode\fP From 76ee7475c09ecb46586800251c30cdf2dec90f6b Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Wed, 24 Jan 2024 09:07:57 +0100 Subject: [PATCH 2/6] Make systemd detection more robust We don't need to look for this on Windows or macOS, and we need to be able to gracefully handle systems without pkg-config. --- CMakeLists.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6028fadfa5..f1bfd61850 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -302,7 +302,12 @@ if(UNIX AND NOT APPLE) endif() # check for systemd support (socket activation) -pkg_check_modules(LIBSYSTEMD libsystemd) +if(UNIX AND NOT APPLE) + find_package(PkgConfig) + if (PKG_CONFIG_FOUND) + pkg_check_modules(LIBSYSTEMD libsystemd) + endif() +endif() # Generate config.h and make sure the source finds it configure_file(config.h.in config.h) From 166c287a8ab0ceb37ed6f56b92076fa64b5b29eb Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Wed, 24 Jan 2024 09:25:35 +0100 Subject: [PATCH 3/6] Use SD_LISTEN_FDS_START constant Avoid magical numbers as it makes it hard to understand the code. --- unix/x0vncserver/x0vncserver.cxx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/unix/x0vncserver/x0vncserver.cxx b/unix/x0vncserver/x0vncserver.cxx index 5e9a7a570c..100c15f9fe 100644 --- a/unix/x0vncserver/x0vncserver.cxx +++ b/unix/x0vncserver/x0vncserver.cxx @@ -121,10 +121,8 @@ static int createSystemdListeners(std::list *listeners) { int count = sd_listen_fds(0); - for (int i = 0; i < count; ++i) { - /* systemd sockets start at FD 3 */ - listeners->push_back(new TcpListener(3 + i)); - } + for (int i = 0; i < count; ++i) + listeners->push_back(new TcpListener(SD_LISTEN_FDS_START + i)); return count; } From 52f438811027ed49c9aff016ac3c41c9ac3dec52 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Wed, 24 Jan 2024 09:41:32 +0100 Subject: [PATCH 4/6] Log when sd_listen_fds() fails --- unix/x0vncserver/x0vncserver.cxx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/unix/x0vncserver/x0vncserver.cxx b/unix/x0vncserver/x0vncserver.cxx index 100c15f9fe..43e68d71af 100644 --- a/unix/x0vncserver/x0vncserver.cxx +++ b/unix/x0vncserver/x0vncserver.cxx @@ -120,6 +120,11 @@ static void CleanupSignalHandler(int /*sig*/) static int createSystemdListeners(std::list *listeners) { int count = sd_listen_fds(0); + if (count < 0) { + vlog.error("Error getting listening sockets from systemd: %s", + strerror(-count)); + return count; + } for (int i = 0; i < count; ++i) listeners->push_back(new TcpListener(SD_LISTEN_FDS_START + i)); From 311ef22ffd660831e5580af065dd6aa4537e1105 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Wed, 24 Jan 2024 09:43:02 +0100 Subject: [PATCH 5/6] Allow combining socket activation and extra ports The common use case is probably to only listem to the systemd provided socket when using socket activation, but it might not be the only use case. Make sure things can be combined if explicitly requested. --- unix/x0vncserver/x0vncserver.cxx | 79 +++++++++++++++++++------------- unix/x0vncserver/x0vncserver.man | 7 +-- 2 files changed, 50 insertions(+), 36 deletions(-) diff --git a/unix/x0vncserver/x0vncserver.cxx b/unix/x0vncserver/x0vncserver.cxx index 43e68d71af..28567a6e26 100644 --- a/unix/x0vncserver/x0vncserver.cxx +++ b/unix/x0vncserver/x0vncserver.cxx @@ -116,9 +116,20 @@ static void CleanupSignalHandler(int /*sig*/) caughtSignal = true; } +static bool hasSystemdListeners() +{ #ifdef HAVE_SYSTEMD_H + // This also returns true on errors, because we then assume we were + // meant to use systemd but failed somewhere + return sd_listen_fds(0) != 0; +#else + return false; +#endif +} + static int createSystemdListeners(std::list *listeners) { +#ifdef HAVE_SYSTEMD_H int count = sd_listen_fds(0); if (count < 0) { vlog.error("Error getting listening sockets from systemd: %s", @@ -130,13 +141,11 @@ static int createSystemdListeners(std::list *listeners) listeners->push_back(new TcpListener(SD_LISTEN_FDS_START + i)); return count; -} #else -static int createSystemdListeners(std::list *) -{ + (void)listeners; return 0; -} #endif +} class FileTcpFilter : public TcpFilter @@ -277,6 +286,10 @@ int main(int argc, char** argv) Configuration::removeParam("SendCutText"); Configuration::removeParam("MaxCutText"); + // Assume different defaults when socket activated + if (hasSystemdListeners()) + rfbport.setParam(-1); + for (int i = 1; i < argc; i++) { if (Configuration::setParam(argv[i])) continue; @@ -325,40 +338,40 @@ int main(int argc, char** argv) VNCServerST server(desktopName, &desktop); - if (createSystemdListeners(&listeners)) { + if (createSystemdListeners(&listeners) > 0) { // When systemd is in charge of listeners, do not listen to anything else vlog.info("Listening on systemd sockets"); - } else { - if (rfbunixpath.getValueStr()[0] != '\0') { - listeners.push_back(new network::UnixListener(rfbunixpath, rfbunixmode)); - vlog.info("Listening on %s (mode %04o)", (const char*)rfbunixpath, (int)rfbunixmode); - } + } - if ((int)rfbport != -1) { - std::list tcp_listeners; - const char *addr = interface; - - if (strcasecmp(addr, "all") == 0) - addr = 0; - if (localhostOnly) - createLocalTcpListeners(&tcp_listeners, (int)rfbport); - else - createTcpListeners(&tcp_listeners, addr, (int)rfbport); - - if (!tcp_listeners.empty()) { - listeners.splice (listeners.end(), tcp_listeners); - vlog.info("Listening for VNC connections on %s interface(s), port %d", - localhostOnly ? "local" : (const char*)interface, - (int)rfbport); - } + if (rfbunixpath.getValueStr()[0] != '\0') { + listeners.push_back(new network::UnixListener(rfbunixpath, rfbunixmode)); + vlog.info("Listening on %s (mode %04o)", (const char*)rfbunixpath, (int)rfbunixmode); + } - FileTcpFilter fileTcpFilter(hostsFile); - if (strlen(hostsFile) != 0) - for (std::list::iterator i = listeners.begin(); - i != listeners.end(); - i++) - (*i)->setFilter(&fileTcpFilter); + if ((int)rfbport != -1) { + std::list tcp_listeners; + const char *addr = interface; + + if (strcasecmp(addr, "all") == 0) + addr = 0; + if (localhostOnly) + createLocalTcpListeners(&tcp_listeners, (int)rfbport); + else + createTcpListeners(&tcp_listeners, addr, (int)rfbport); + + if (!tcp_listeners.empty()) { + listeners.splice (listeners.end(), tcp_listeners); + vlog.info("Listening for VNC connections on %s interface(s), port %d", + localhostOnly ? "local" : (const char*)interface, + (int)rfbport); } + + FileTcpFilter fileTcpFilter(hostsFile); + if (strlen(hostsFile) != 0) + for (std::list::iterator i = listeners.begin(); + i != listeners.end(); + i++) + (*i)->setFilter(&fileTcpFilter); } if (listeners.empty()) { diff --git a/unix/x0vncserver/x0vncserver.man b/unix/x0vncserver/x0vncserver.man index 0ba195ad0f..78cf60b9a4 100644 --- a/unix/x0vncserver/x0vncserver.man +++ b/unix/x0vncserver/x0vncserver.man @@ -60,8 +60,8 @@ DISPLAY environment variable. .B \-rfbport \fIport\fP Specifies the TCP port on which x0vncserver listens for connections from viewers (the protocol used in VNC is called RFB - "remote framebuffer"). -The default port is 5900. Specify \fB-1\fP to disable listening on a TCP -port. Ignored when activated by a systemd socket. +Specify \fB-1\fP to disable listening on a TCP port. The default port is +5900 when started directly, and -1 when activated by a systemd socket. . .TP .B \-UseIPv4 @@ -74,7 +74,8 @@ Use IPv6 for incoming and outgoing connections. Default is on. .TP .B \-rfbunixpath \fIpath\fP Specifies the path of a Unix domain socket on which x0vncserver listens for -connections from viewers. Ignored when activated by a systemd socket. +connections from viewers. Default is to not listen to any Unix domain +socket. . .TP .B \-rfbunixmode \fImode\fP From baa21c364f842e609a4782dc594725e524cd409e Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Wed, 24 Jan 2024 10:21:18 +0100 Subject: [PATCH 6/6] Document optional libsystemd requirement --- BUILDING.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/BUILDING.txt b/BUILDING.txt index bfcad5b940..83a68aee17 100644 --- a/BUILDING.txt +++ b/BUILDING.txt @@ -48,6 +48,9 @@ Build Requirements (Unix) * All build requirements Xorg imposes (see its documentation) * patch +-- If building x0vncserver with socket activation support: + * libsystemd + -- Optional ffmpeg development kit support (libav) * You might have to enable additional repositories for this. E.g., on RHEL, EPEL and RPMFusion (free + nonfree) need to be enabled.