[1/2] dbus: add dbus-over-tcp support

Commit Message

Ram Subramanian May 13, 2024, 5:58 p.m. UTC

l_dbus_new() will now accept tcp endpoints, however it will only accept
a numeric address for 'host'; it will not resolve hostnames. This is so
we can avoid a potential long, blocking call to getaddrinfo().

The connect() call to the host is also non-blocking. This means that
auth will not happen until you start the event loop (with a call to
l_main_run() et. al.).

Co-authored-by: Ramon Ribeiro <[e-mail]>
---
 ell/dbus.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 111 insertions(+)

Comments

Denis Kenzior May 16, 2024, 7:33 p.m. UTC | #1

Hi Ram,

On 5/13/24 12:58 PM, Ram Subramanian wrote:
> l_dbus_new() will now accept tcp endpoints, however it will only accept
> a numeric address for 'host'; it will not resolve hostnames. This is so
> we can avoid a potential long, blocking call to getaddrinfo().
> 
> The connect() call to the host is also non-blocking. This means that
> auth will not happen until you start the event loop (with a call to
> l_main_run() et. al.).
> 
> Co-authored-by: Ramon Ribeiro <[e-mail]>
> ---
>   ell/dbus.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 111 insertions(+)

<snip>

> +static struct l_dbus *setup_tcp(char *params)
> +{
> +	char *host = NULL;
> +	char *port = NULL;
> +	char *family = NULL;
> +	struct addrinfo hints = { 0 };
> +	struct addrinfo *res;
> +	struct addrinfo *iter;
> +	struct l_dbus *dbus;
> +

CI caught this one:

Clang Build
Test ID: clang
Desc: Build IWD using clang compiler
Duration: 72.29 seconds
Result: FAIL

Output:

ell/dbus.c:1217:19: error: variable 'dbus' is used uninitialized whenever 'for' 
loop exits because its condition is false [-Werror,-Wsometimes-uninitialized]
         for (iter = res; iter; iter = iter->ai_next) {
                          ^~~~
ell/dbus.c:1241:9: note: uninitialized use occurs here
         return dbus;
                ^~~~
ell/dbus.c:1217:19: note: remove the condition if it is always true
         for (iter = res; iter; iter = iter->ai_next) {
                          ^~~~
ell/dbus.c:1178:21: note: initialize the variable 'dbus' to silence this warning
         struct l_dbus *dbus;
                            ^
                             = NULL
1 error generated.
make[1]: *** [Makefile:2560: ell/dbus.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1716: all] Error 2

So I changed this line to read:
struct l_dbus *dbus = NULL;

Applied, thanks.

Regards,
-Denis

Patch

diff --git a/ell/dbus.c b/ell/dbus.c
index d610532..22deaba 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -15,8 +15,10 @@ 
 #include <unistd.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sys/types.h>
 #include <sys/socket.h>
 #include <sys/un.h>
+#include <netdb.h>
 #include <errno.h>
 
 #include "util.h"
@@ -1133,6 +1135,112 @@  static struct l_dbus *setup_unix(char *params)
 	return setup_dbus1(fd, guid);
 }
 
+static bool setup_tcp_cb(struct l_io *io, void *user_data)
+{
+	static const unsigned char creds = 0x00;
+	struct l_dbus *dbus = user_data;
+	struct l_dbus_classic *classic;
+	ssize_t written;
+	int fd = l_io_get_fd(io);
+
+	/* Send special credentials-passing nul byte */
+	written = L_TFR(send(fd, &creds, 1, 0));
+	if (written < 1) {
+		l_util_debug(dbus->debug_handler, dbus->debug_handler,
+						"error writing NUL byte");
+		close(fd);
+		return false;
+	}
+
+	dbus->driver = &classic_ops;
+	dbus->negotiate_unix_fd = false;
+	dbus->support_unix_fd = false;
+
+	classic = l_container_of(dbus, struct l_dbus_classic, super);
+	classic->match_strings = l_hashmap_new();
+	classic->auth_command = l_strdup("AUTH ANONYMOUS\r\n");
+	classic->auth_state = WAITING_FOR_OK;
+
+	l_io_set_read_handler(dbus->io, auth_read_handler, dbus, NULL);
+	l_io_set_write_handler(dbus->io, auth_write_handler, dbus, NULL);
+
+	return auth_write_handler(dbus->io, dbus);
+}
+
+static struct l_dbus *setup_tcp(char *params)
+{
+	char *host = NULL;
+	char *port = NULL;
+	char *family = NULL;
+	struct addrinfo hints = { 0 };
+	struct addrinfo *res;
+	struct addrinfo *iter;
+	struct l_dbus *dbus;
+
+	while (params) {
+		char *key = strsep(&params, ",");
+		char *value;
+
+		value = strchr(key, '=');
+		if (!value)
+			continue;
+
+		*value++ = '\0';
+
+		if (!strcmp(key, "host"))
+			host = value;
+		else if (!strcmp(key, "port"))
+			port = value;
+		else if (!strcmp(key, "family"))
+			family = value;
+	}
+
+	if (!host || !port)
+		return NULL;
+
+	if (!family)
+		hints.ai_family = AF_UNSPEC;
+	else if (!strcmp(family, "ipv4"))
+		hints.ai_family = AF_INET;
+	else if (!strcmp(family, "ipv6"))
+		hints.ai_family = AF_INET6;
+	else
+		return NULL;
+
+	hints.ai_socktype = SOCK_STREAM;
+	hints.ai_flags = AI_NUMERICHOST | AI_NUMERICSERV;
+	hints.ai_protocol = IPPROTO_TCP;
+
+	if (getaddrinfo(host, port, &hints, &res) != 0)
+		return NULL;
+
+	for (iter = res; iter; iter = iter->ai_next) {
+		int fd;
+		struct l_dbus_classic *classic;
+
+		fd = socket(iter->ai_family, iter->ai_socktype | SOCK_NONBLOCK,
+					iter->ai_protocol);
+		if (fd < 0)
+			continue;
+
+		if (connect(fd, iter->ai_addr, iter->ai_addrlen) < 0) {
+			if (errno != EINPROGRESS) {
+				close(fd);
+				continue;
+			}
+		}
+
+		classic = l_new(struct l_dbus_classic, 1);
+		dbus = &classic->super;
+		dbus_init(dbus, fd);
+		l_io_set_write_handler(dbus->io, setup_tcp_cb, dbus, NULL);
+		break;
+	}
+
+	freeaddrinfo(res);
+	return dbus;
+}
+
 static struct l_dbus *setup_address(const char *address)
 {
 	struct l_dbus *dbus = NULL;
@@ -1155,6 +1263,9 @@  static struct l_dbus *setup_address(const char *address)
 			/* Function will modify params string */
 			dbus = setup_unix(params);
 			break;
+		} else if (!strcmp(transport, "tcp")) {
+			dbus = setup_tcp(params);
+			break;
 		}
 	}