tang: allow running standalone
This enables running tangd in environments without systemd (e.g., embedded), without requiring xinetd or other superservers.
I had issues trying to run tangd on systems like openwrt that use procd (not systemd), so let me propose this patch set for consideration. This extends tangd to be socket aware itself so it can run either standalone or in systemd, and it would make it much easier using and integrating tangd in these systems. While at it, it adds getopt and a --version option.
In retrospect, it is really sad to see how much code one needs to add to add a listening port in a server.
Codecov Report
Merging #86 (88abb28) into master (e2059ee) will decrease coverage by
0.32%. The diff coverage is54.60%.
@@ Coverage Diff @@
## master #86 +/- ##
==========================================
- Coverage 57.35% 57.03% -0.33%
==========================================
Files 3 4 +1
Lines 401 533 +132
Branches 114 141 +27
==========================================
+ Hits 230 304 +74
- Misses 88 130 +42
- Partials 83 99 +16
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/tangd.c | 50.73% <50.00%> (+1.22%) |
:arrow_up: |
| src/socket.c | 56.56% <56.56%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update e2059ee...88abb28. Read the comment docs.
It seems workflows timeout or fail during dependency installation, but it is independent of the changes proposed here. I've tried to improve a little the workflows by switching to centos-stream and ubuntu 20.04.
Thanks for the work here, also for fixing the CI. Overall it looks good. I will look at it more closely hopefully later today.
Thank you @sergio-correia , any concerns so far?
In general I like this, although it would be nice to have more granularity than all or none for listening interfaces. I do understand that would complicate the code a lot. And while I absolutely would like to eliminate socat/inetd/xinetd from the equation in smaller devices, it seems like this is going to be unnecessary code to maintain for the vast majority of use cases.
All that said, I have a specific issue in that it doesn't compile on FreeBSD. Fixing that is easy, it's just missing two necessary includes in socket.c: +#include <netinet/in.h> +#include <signal.h> I apologize that's not in the form of a clean patch, but I'm not familiar enough with meson to be sure what the right/clean way is to add those as conditional parts of the C source. Does meson pass in anything automatically in terms of say host_machine.system() as a compiler argument or would the meson.build file have to construct something that cpp could use?
I sent a pull request to nmav:tmp-tangd-standalone to address FreeBSD compatibility. Hopefully @nmav won't have any issues with those changes and can pull it into this request.
I sent a pull request to nmav:tmp-tangd-standalone to address FreeBSD compatibility. Hopefully @nmav won't have any issues with those changes and can pull it into this request.
I've included it.
I think that build failure is my fault. It looks like I failed to include the iproute in the centos:stream* install dependencies when I pushed the update to @nmav
Maybe I'm looking at it wrong, I don't have a lot of experience with github. But I'm not seeing the changes I asked for specifically changes to src/socket.c, tests.helpers, and units/tangd.rc.in don't seem to be there either in nmav:tmp-tangd-standalone or 7c65483
I submitted the changes that didn't get made with a (hopefully) much simpler and easier to manage pull request against nmav:tmp-tangd-standalone. For reference the changes are simply:
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index df9b4c3..fedb792 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -17,6 +17,8 @@ jobs:
- debian:testing
- debian:latest
- ubuntu:rolling
+ - ubuntu:jammy
+ - ubuntu:focal
- ubuntu:bionic
stable: [true]
include:
diff --git a/.github/workflows/install-dependencies b/.github/workflows/install-dependencies
index 9a8a99a..28abb63 100755
--- a/.github/workflows/install-dependencies
+++ b/.github/workflows/install-dependencies
@@ -33,7 +33,7 @@ centos:*)
dnf install -y dnf-plugins-core epel-release
dnf config-manager -y --set-enabled powertools \
|| dnf config-manager -y --set-enabled crb || :
- dnf -y install meson socat
+ dnf -y install meson socat iproute
dnf builddep -y tang
;;
esac
diff --git a/tests/helpers b/tests/helpers
index aab7f79..984434e 100755
--- a/tests/helpers
+++ b/tests/helpers
@@ -35,7 +35,11 @@ random_port() {
}
check_if_port_listening() {
+ if [ -n "${TANG_BSD}" ]; then
+ sockstat -nl|grep "[\:\.]${1}" >/dev/null 2>&1
+ else
ss -anl|grep "[\:\.]${1}"|grep LISTEN >/dev/null 2>&1
+ fi
}
wait_for_port()
diff --git a/units/tangd.rc.in b/units/tangd.rc.in
index a152a69..0027df2 100644
--- a/units/tangd.rc.in
+++ b/units/tangd.rc.in
@@ -4,44 +4,26 @@
#
# Should probably in the future allow running as non-root
-# and enable multiple interfaces in some cleaner way.
+# and enable multiple interfaces in some way in the future.
# PROVIDE: tangd
# REQUIRE: NETWORKING DAEMON
-# KEYWORD: nojail
. /etc/rc.subr
name="tangd"
desc="Network Presence Binding Daemon (tang)"
rcvar="tangd_enable"
-command="/usr/local/bin/socat"
load_rc_config $name
: ${tangd_enable:=no}
-: ${tangd_ip="127.0.0.1"}
: ${tangd_port="8888"}
: ${tangd_jwkdir="@jwkdir@"}
: ${tangd_logfile="/var/log/tang"}
-: ${tangd_executable="@libexecdir@/tangd"}
-pidfile="/var/run/${name}.pid"
-required_files="@libexecdir@/${name}"
required_dirs="${tangd_jwkdir}"
-start_postcmd="${name}_poststart"
-_tangd_listen_args="TCP-LISTEN:${tangd_port},bind=${tangd_ip},fork"
-command_args="${_tangd_listen_args} SYSTEM:\"${tangd_executable} ${tangd_jwkdir} 2>> ${tangd_logfile} \" &"
-
-# Since we may not be the only socat running we can't use the built-in process
-# management, so we'll need to use a pid file and find the pid from unique arguments.
-tangd_poststart() {
- ps_pid=`ps ax -o pid= -o command= | grep ${_tangd_listen_args} | grep -v grep | awk '{print $1}'`
- if [ -z "$ps_pid" ]; then
- err 1 "Cannot get pid for ${command} ${command_args}"
- fi
- echo $ps_pid > ${pidfile}
- return $?
-}
+command="@libexecdir@/${name}"
+command_args="-p ${tangd_port} -l ${tangd_jwkdir} 2>> ${tangd_logfile} &"
run_rc_command "$1"
I submitted the changes that didn't get made with a (hopefully) much simpler and easier to manage pull request against nmav:tmp-tangd-standalone. For reference the changes are simply:
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index df9b4c3..fedb792 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -17,6 +17,8 @@ jobs: - debian:testing - debian:latest - ubuntu:rolling + - ubuntu:jammy + - ubuntu:focal - ubuntu:bionic stable: [true] include: diff --git a/.github/workflows/install-dependencies b/.github/workflows/install-dependencies index 9a8a99a..28abb63 100755 --- a/.github/workflows/install-dependencies +++ b/.github/workflows/install-dependencies @@ -33,7 +33,7 @@ centos:*) dnf install -y dnf-plugins-core epel-release dnf config-manager -y --set-enabled powertools \ || dnf config-manager -y --set-enabled crb || : - dnf -y install meson socat + dnf -y install meson socat iproute dnf builddep -y tang ;; esac diff --git a/tests/helpers b/tests/helpers index aab7f79..984434e 100755 --- a/tests/helpers +++ b/tests/helpers @@ -35,7 +35,11 @@ random_port() { } check_if_port_listening() { + if [ -n "${TANG_BSD}" ]; then + sockstat -nl|grep "[\:\.]${1}" >/dev/null 2>&1 + else ss -anl|grep "[\:\.]${1}"|grep LISTEN >/dev/null 2>&1 + fi } wait_for_port() diff --git a/units/tangd.rc.in b/units/tangd.rc.in index a152a69..0027df2 100644 --- a/units/tangd.rc.in +++ b/units/tangd.rc.in @@ -4,44 +4,26 @@ # # Should probably in the future allow running as non-root -# and enable multiple interfaces in some cleaner way. +# and enable multiple interfaces in some way in the future. # PROVIDE: tangd # REQUIRE: NETWORKING DAEMON -# KEYWORD: nojail . /etc/rc.subr name="tangd" desc="Network Presence Binding Daemon (tang)" rcvar="tangd_enable" -command="/usr/local/bin/socat" load_rc_config $name : ${tangd_enable:=no} -: ${tangd_ip="127.0.0.1"} : ${tangd_port="8888"} : ${tangd_jwkdir="@jwkdir@"} : ${tangd_logfile="/var/log/tang"} -: ${tangd_executable="@libexecdir@/tangd"} -pidfile="/var/run/${name}.pid" -required_files="@libexecdir@/${name}" required_dirs="${tangd_jwkdir}" -start_postcmd="${name}_poststart" -_tangd_listen_args="TCP-LISTEN:${tangd_port},bind=${tangd_ip},fork" -command_args="${_tangd_listen_args} SYSTEM:\"${tangd_executable} ${tangd_jwkdir} 2>> ${tangd_logfile} \" &" - -# Since we may not be the only socat running we can't use the built-in process -# management, so we'll need to use a pid file and find the pid from unique arguments. -tangd_poststart() { - ps_pid=`ps ax -o pid= -o command= | grep ${_tangd_listen_args} | grep -v grep | awk '{print $1}'` - if [ -z "$ps_pid" ]; then - err 1 "Cannot get pid for ${command} ${command_args}" - fi - echo $ps_pid > ${pidfile} - return $? -} +command="@libexecdir@/${name}" +command_args="-p ${tangd_port} -l ${tangd_jwkdir} 2>> ${tangd_logfile} &" run_rc_command "$1"
I cannot merge this because I cannot test, and I do not like its inner workings (the environment variable). At this point my goal is to have the linux version adopted. I think this PR can be a follow up once this is merged.
@sarroutbi I think the issues you raised are now addressed. Any remaining issues to merge?