[PATCH 0/2] Allow connection to a tcp dbus address

[PATCH 0/2] Allow connection to a tcp dbus address

[Ramon Henrique]

[-- Attachment #1: Type: text/plain, Size: 672 bytes --]

This patch came from a demand in a project with industrial IoT for
monitoring a PLC (creating supervisor apps). The infrastructure was
built from a DBus service that can be accessed by a front-end in the
same local network.

Ramon Ribeiro (2):
  dbus: Add support to tcp address
  unit: dbus: Add unit test to dbus tcp address

 .gitignore           |   1 +
 Makefile.am          |   3 +
 ell/dbus.c           |  95 ++++++++++++++++++-
 unit/dbus.conf       |   2 +
 unit/test-dbus-tcp.c | 261 +++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 358 insertions(+), 4 deletions(-)
 create mode 100644 unit/test-dbus-tcp.c

-- 
2.11.0

[PATCH 1/2] dbus: Add support to tcp address

[Ramon Henrique]

[-- Attachment #1: Type: text/plain, Size: 4427 bytes --]

From: Ramon Ribeiro <rhpr@cesar.org.br>

This patch allows a dbus client to connect to a dbus-daemon server
address listenning a TCP transport protocol
---
 ell/dbus.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 91 insertions(+), 4 deletions(-)

diff --git a/ell/dbus.c b/ell/dbus.c
index 92be396..256a70c 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -31,8 +31,10 @@
 #include <stdarg.h>
 #include <stdlib.h>
 #include <string.h>
+#include <netdb.h>
 #include <sys/socket.h>
 #include <sys/un.h>
+#include <netinet/in.h>
 #include <errno.h>
 
 #include "util.h"
@@ -75,6 +77,7 @@ struct l_dbus_ops {
 struct l_dbus {
 	struct l_io *io;
 	char *guid;
+	char *transport;
 	bool negotiate_unix_fd;
 	bool support_unix_fd;
 	bool is_ready;
@@ -511,7 +514,10 @@ static bool auth_read_handler(struct l_io *io, void *user_data)
 		} else if (!strncmp(ptr, "REJECTED ", 9)) {
 			static const char *command = "AUTH ANONYMOUS\r\n";
 
-			dbus->negotiate_unix_fd = true;
+			if (!strcmp(dbus->transport, "unix"))
+				dbus->negotiate_unix_fd = true;
+			else
+				dbus->negotiate_unix_fd = false;
 
 			classic->auth_command = l_strdup(command);
 			classic->auth_state = WAITING_FOR_OK;
@@ -1033,7 +1039,8 @@ static const struct l_dbus_ops classic_ops = {
 	.name_acquire = classic_name_acquire,
 };
 
-static struct l_dbus *setup_dbus1(int fd, const char *guid)
+static struct l_dbus *setup_dbus1(int fd, const char *guid,
+				  const char *transport)
 {
 	static const unsigned char creds = 0x00;
 	char uid[6], hexuid[12], *ptr = hexuid;
@@ -1065,11 +1072,13 @@ static struct l_dbus *setup_dbus1(int fd, const char *guid)
 
 	dbus_init(dbus, fd);
 	dbus->guid = l_strdup(guid);
+	dbus->transport = l_strdup(transport);
 
 	classic->auth_command = l_strdup_printf("AUTH EXTERNAL %s\r\n", hexuid);
 	classic->auth_state = WAITING_FOR_OK;
 
-	dbus->negotiate_unix_fd = true;
+	if (!strcmp(transport, "unix"))
+		dbus->negotiate_unix_fd = true;
 	dbus->support_unix_fd = false;
 
 	l_io_set_read_handler(dbus->io, auth_read_handler, dbus, NULL);
@@ -1145,7 +1154,81 @@ static struct l_dbus *setup_unix(char *params)
 		return NULL;
 	}
 
-	return setup_dbus1(fd, guid);
+	return setup_dbus1(fd, guid, "unix");
+}
+
+static struct l_dbus *setup_tcp(char *params)
+{
+	struct addrinfo hints;
+	struct addrinfo *results, *rp;
+	char *host = NULL, *port = NULL, *family = NULL, *guid = NULL;
+	int port_number = -1;
+	int fd;
+
+	while (params) {
+		char *key = strsep(&params, ",");
+		char *value;
+
+		if (!key)
+			break;
+
+		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;
+		else if (!strcmp(key, "guid"))
+			guid = value;
+	}
+
+	if (!host || !port)
+		return NULL;
+
+	port_number = atoi(port);
+
+	if (port_number < 0 || port_number >= 65536)
+		return NULL;
+
+	memset(&hints, 0, sizeof(hints));
+
+	if (family != NULL && !strcmp(family, "ipv4"))
+		hints.ai_family = AF_INET;
+	else if (family != NULL && !strcmp(family, "ipv6"))
+		hints.ai_family = AF_INET6;
+	else
+		hints.ai_family = AF_UNSPEC;
+
+	hints.ai_socktype = SOCK_STREAM;
+	hints.ai_flags = 0;
+	hints.ai_protocol = 0;
+
+	if (getaddrinfo(host, port, &hints, &results) != 0)
+		return NULL;
+
+	for (rp = results; rp != NULL; rp = rp->ai_next) {
+		fd = socket(rp->ai_family, rp->ai_socktype, rp->ai_protocol);
+		if (fd < 0)
+			continue;
+
+		if (connect(fd, rp->ai_addr, rp->ai_addrlen) != -1)
+			break;
+
+		close(fd);
+	}
+
+	if (rp == NULL)
+		return NULL;
+
+	freeaddrinfo(results);
+
+	return setup_dbus1(fd, guid, "tcp");
 }
 
 static struct l_dbus *setup_address(const char *address)
@@ -1170,6 +1253,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;
 		}
 	}
 
@@ -1232,6 +1318,7 @@ LIB_EXPORT void l_dbus_destroy(struct l_dbus *dbus)
 
 	l_free(dbus->guid);
 	l_free(dbus->unique_name);
+	l_free(dbus->transport);
 
 	_dbus_object_tree_free(dbus->tree);
 
-- 
2.11.0

[PATCH 2/2] unit: dbus: Add unit test to dbus tcp address

[Ramon Henrique]

[-- Attachment #1: Type: text/plain, Size: 7946 bytes --]

From: Ramon Ribeiro <rhpr@cesar.org.br>

---
 .gitignore           |   1 +
 Makefile.am          |   3 +
 unit/dbus.conf       |   2 +
 unit/test-dbus-tcp.c | 261 +++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 267 insertions(+)
 create mode 100644 unit/test-dbus-tcp.c

diff --git a/.gitignore b/.gitignore
index 520c509..7d938f2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -34,6 +34,7 @@ unit/test-netlink
 unit/test-genl
 unit/test-genl-msg
 unit/test-dbus
+unit/test-dbus-tcp
 unit/test-dbus-message
 unit/test-dbus-message-fds
 unit/test-dbus-util
diff --git a/Makefile.am b/Makefile.am
index 5bc13bd..28b8452 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -175,6 +175,7 @@ unit_tests = unit/test-unit \
 
 dbus_tests = unit/test-hwdb \
 			unit/test-dbus \
+			unit/test-dbus-tcp \
 			unit/test-dbus-util \
 			unit/test-dbus-message \
 			unit/test-dbus-message-fds \
@@ -243,6 +244,8 @@ unit_test_genl_msg_LDADD = ell/libell-private.la
 
 unit_test_dbus_LDADD = ell/libell-private.la
 
+unit_test_dbus_tcp_LDADD = ell/libell-private.la
+
 unit_test_dbus_message_LDADD = ell/libell-private.la
 
 unit_test_dbus_message_fds_LDADD = ell/libell-private.la
diff --git a/unit/dbus.conf b/unit/dbus.conf
index 97e1bb5..6b1fccd 100644
--- a/unit/dbus.conf
+++ b/unit/dbus.conf
@@ -7,6 +7,8 @@
   <allow_anonymous />
 
   <listen>unix:path=/tmp/ell-test-bus</listen>
+  <listen>tcp:host=localhost,bind=*,port=1234,family=ipv4</listen>
+  <apparmor mode="disabled" />
 
   <policy context="default">
     <!-- Allow everything to be sent -->
diff --git a/unit/test-dbus-tcp.c b/unit/test-dbus-tcp.c
new file mode 100644
index 0000000..4e11ca9
--- /dev/null
+++ b/unit/test-dbus-tcp.c
@@ -0,0 +1,261 @@
+/*
+ *
+ *  Embedded Linux library
+ *
+ *  Copyright (C) 2011-2014  Intel Corporation. All rights reserved.
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define _GNU_SOURCE
+#include <unistd.h>
+#include <stdlib.h>
+#include <sys/wait.h>
+
+#include <ell/ell.h>
+
+#ifndef WAIT_ANY
+#define WAIT_ANY (-1) /* Any process */
+#endif
+
+#define TEST_BUS_ADDRESS "tcp:host=localhost,port=1234"
+
+static pid_t dbus_daemon_pid = -1;
+
+static void start_dbus_daemon(void)
+{
+	char *prg_argv[6];
+	char *prg_envp[1];
+	pid_t pid;
+
+	prg_argv[0] = "/usr/bin/dbus-daemon";
+	prg_argv[1] = "--nopidfile";
+	prg_argv[2] = "--nofork";
+	prg_argv[3] = "--config-file=" UNITDIR "dbus.conf";
+	prg_argv[4] = "--print-address";
+	prg_argv[5] = NULL;
+
+	prg_envp[0] = NULL;
+
+	l_info("launching dbus-daemon");
+
+	pid = fork();
+	if (pid < 0) {
+		l_error("failed to fork new process");
+		return;
+	}
+
+	if (pid == 0) {
+		execve(prg_argv[0], prg_argv, prg_envp);
+		exit(EXIT_SUCCESS);
+	}
+
+	l_info("dbus-daemon process %d created", pid);
+
+	dbus_daemon_pid = pid;
+}
+
+static void signal_handler(uint32_t signo, void *user_data)
+{
+	switch (signo) {
+	case SIGINT:
+	case SIGTERM:
+		l_info("Terminate");
+		l_main_quit();
+		break;
+	}
+}
+
+static void sigchld_handler(void *user_data)
+{
+	while (1) {
+		pid_t pid;
+		int status;
+
+		pid = waitpid(WAIT_ANY, &status, WNOHANG);
+		if (pid < 0 || pid == 0)
+			break;
+
+		l_info("process %d terminated with status=%d\n", pid, status);
+
+		if (pid == dbus_daemon_pid) {
+			dbus_daemon_pid = -1;
+			l_main_quit();
+		}
+	}
+}
+
+static void do_debug(const char *str, void *user_data)
+{
+	const char *prefix = user_data;
+
+	l_info("%s%s", prefix, str);
+}
+
+static void signal_message(struct l_dbus_message *message, void *user_data)
+{
+	const char *path, *interface, *member, *destination, *sender;
+
+	path = l_dbus_message_get_path(message);
+	destination = l_dbus_message_get_destination(message);
+
+	l_info("path=%s destination=%s", path, destination);
+
+	interface = l_dbus_message_get_interface(message);
+	member = l_dbus_message_get_member(message);
+
+	l_info("interface=%s member=%s", interface, member);
+
+	sender = l_dbus_message_get_sender(message);
+
+	l_info("sender=%s", sender);
+
+	if (!strcmp(member, "NameOwnerChanged")) {
+		const char *name, *old_owner, *new_owner;
+
+		if (!l_dbus_message_get_arguments(message, "sss",
+					&name, &old_owner, &new_owner))
+			return;
+
+		l_info("name=%s old=%s new=%s", name, old_owner, new_owner);
+	}
+}
+
+static void request_name_setup(struct l_dbus_message *message, void *user_data)
+{
+	const char *name = "org.test";
+
+	l_dbus_message_set_arguments(message, "su", name, 0);
+}
+
+static void request_name_callback(struct l_dbus_message *message,
+							void *user_data)
+{
+	const char *error, *text;
+	uint32_t result;
+
+	if (l_dbus_message_get_error(message, &error, &text)) {
+		l_error("error=%s", error);
+		l_error("message=%s", text);
+		goto done;
+	}
+
+	if (!l_dbus_message_get_arguments(message, "u", &result))
+		goto done;
+
+	l_info("request name result=%d", result);
+
+done:
+	l_main_quit();
+}
+
+static const char *match_rule = "type=signal,sender=org.freedesktop.DBus";
+
+static void add_match_setup(struct l_dbus_message *message, void *user_data)
+{
+	l_dbus_message_set_arguments(message, "s", match_rule);
+}
+
+static void add_match_callback(struct l_dbus_message *message, void *user_data)
+{
+	const char *error, *text;
+
+	if (l_dbus_message_get_error(message, &error, &text)) {
+		l_error("error=%s", error);
+		l_error("message=%s", text);
+		return;
+	}
+
+	if (!l_dbus_message_get_arguments(message, ""))
+		return;
+
+	l_info("add match");
+}
+
+static void ready_callback(void *user_data)
+{
+	l_info("ready");
+}
+
+static void disconnect_callback(void *user_data)
+{
+	l_main_quit();
+}
+
+int main(int argc, char *argv[])
+{
+	struct l_dbus *dbus;
+	struct l_signal *sigchld;
+	int i;
+
+	if (!l_main_init())
+		return -1;
+
+	l_log_set_stderr();
+
+	start_dbus_daemon();
+
+	for (i = 0; i < 10; i++) {
+		usleep(200 * 1000);
+
+		dbus = l_dbus_new(TEST_BUS_ADDRESS);
+		if (dbus)
+			break;
+
+	}
+
+	sigchld = l_signal_create(SIGCHLD, sigchld_handler, NULL, NULL);
+
+	if (!dbus)
+		goto done;
+
+	l_dbus_set_debug(dbus, do_debug, "[DBUS] ", NULL);
+
+	l_dbus_set_ready_handler(dbus, ready_callback, dbus, NULL);
+	l_dbus_set_disconnect_handler(dbus, disconnect_callback, NULL, NULL);
+
+	l_dbus_register(dbus, signal_message, NULL, NULL);
+
+	l_dbus_method_call(dbus, "org.freedesktop.DBus",
+				"/org/freedesktop/DBus",
+				"org.freedesktop.DBus", "AddMatch",
+				add_match_setup,
+				add_match_callback, NULL, NULL);
+
+	l_dbus_method_call(dbus, "org.freedesktop.DBus",
+				"/org/freedesktop/DBus",
+				"org.freedesktop.DBus", "RequestName",
+				request_name_setup,
+				request_name_callback, NULL, NULL);
+
+	l_main_run_with_signal(signal_handler, NULL);
+
+	l_dbus_destroy(dbus);
+
+done:
+	if (dbus_daemon_pid > 0)
+		kill(dbus_daemon_pid, SIGKILL);
+
+	l_signal_remove(sigchld);
+
+	l_main_exit();
+
+	return 0;
+}
-- 
2.11.0

Re: [PATCH 1/2] dbus: Add support to tcp address

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 5706 bytes --]

Hi Ramon,

On 03/31/2019 09:00 AM, Ramon Henrique wrote:
> From: Ramon Ribeiro <rhpr@cesar.org.br>
> 
> This patch allows a dbus client to connect to a dbus-daemon server
> address listenning a TCP transport protocol
> ---
>   ell/dbus.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 91 insertions(+), 4 deletions(-)
> 
> diff --git a/ell/dbus.c b/ell/dbus.c
> index 92be396..256a70c 100644
> --- a/ell/dbus.c
> +++ b/ell/dbus.c
> @@ -31,8 +31,10 @@
>   #include <stdarg.h>
>   #include <stdlib.h>
>   #include <string.h>
> +#include <netdb.h>
>   #include <sys/socket.h>
>   #include <sys/un.h>
> +#include <netinet/in.h>
>   #include <errno.h>
>   
>   #include "util.h"
> @@ -75,6 +77,7 @@ struct l_dbus_ops {
>   struct l_dbus {
>   	struct l_io *io;
>   	char *guid;
> +	char *transport;
>   	bool negotiate_unix_fd;
>   	bool support_unix_fd;
>   	bool is_ready;
> @@ -511,7 +514,10 @@ static bool auth_read_handler(struct l_io *io, void *user_data)
>   		} else if (!strncmp(ptr, "REJECTED ", 9)) {
>   			static const char *command = "AUTH ANONYMOUS\r\n";
>   
> -			dbus->negotiate_unix_fd = true;
> +			if (!strcmp(dbus->transport, "unix"))
> +				dbus->negotiate_unix_fd = true;
> +			else
> +				dbus->negotiate_unix_fd = false;
>   
>   			classic->auth_command = l_strdup(command);
>   			classic->auth_state = WAITING_FOR_OK;
> @@ -1033,7 +1039,8 @@ static const struct l_dbus_ops classic_ops = {
>   	.name_acquire = classic_name_acquire,
>   };
>   
> -static struct l_dbus *setup_dbus1(int fd, const char *guid)
> +static struct l_dbus *setup_dbus1(int fd, const char *guid,
> +				  const char *transport)
>   {
>   	static const unsigned char creds = 0x00;
>   	char uid[6], hexuid[12], *ptr = hexuid;
> @@ -1065,11 +1072,13 @@ static struct l_dbus *setup_dbus1(int fd, const char *guid)
>   
>   	dbus_init(dbus, fd);
>   	dbus->guid = l_strdup(guid);
> +	dbus->transport = l_strdup(transport);
>   
>   	classic->auth_command = l_strdup_printf("AUTH EXTERNAL %s\r\n", hexuid);
>   	classic->auth_state = WAITING_FOR_OK;
>   
> -	dbus->negotiate_unix_fd = true;
> +	if (!strcmp(transport, "unix"))
> +		dbus->negotiate_unix_fd = true;
>   	dbus->support_unix_fd = false;
>   
>   	l_io_set_read_handler(dbus->io, auth_read_handler, dbus, NULL);
> @@ -1145,7 +1154,81 @@ static struct l_dbus *setup_unix(char *params)
>   		return NULL;
>   	}
>   
> -	return setup_dbus1(fd, guid);
> +	return setup_dbus1(fd, guid, "unix");
> +}
> +
> +static struct l_dbus *setup_tcp(char *params)
> +{
> +	struct addrinfo hints;
> +	struct addrinfo *results, *rp;
> +	char *host = NULL, *port = NULL, *family = NULL, *guid = NULL;
> +	int port_number = -1;
> +	int fd;
> +
> +	while (params) {
> +		char *key = strsep(&params, ",");
> +		char *value;
> +
> +		if (!key)
> +			break;
> +
> +		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;
> +		else if (!strcmp(key, "guid"))
> +			guid = value;
> +	}
> +
> +	if (!host || !port)
> +		return NULL;
> +
> +	port_number = atoi(port);

So in general, the use of atoi is discouraged.  Use strtoul or similar 
instead.  You have much finer understanding / control of the error 
conditions with strtoul.

> +
> +	if (port_number < 0 || port_number >= 65536)
> +		return NULL;

Hmm, according to D-Bus specification, port of zero is invalid (unless 
you're the server):

port	(number)	The tcp port the server will open. A zero value let the 
server choose a free port provided from the underlaying operating 
system. libdbus is able to retrieve the real used port from the server.

> +
> +	memset(&hints, 0, sizeof(hints));
> +
> +	if (family != NULL && !strcmp(family, "ipv4"))
> +		hints.ai_family = AF_INET;
> +	else if (family != NULL && !strcmp(family, "ipv6"))
> +		hints.ai_family = AF_INET6;
> +	else
> +		hints.ai_family = AF_UNSPEC;
> +
> +	hints.ai_socktype = SOCK_STREAM;
> +	hints.ai_flags = 0;
> +	hints.ai_protocol = 0;
> +
> +	if (getaddrinfo(host, port, &hints, &results) != 0)
> +		return NULL;
> +
> +	for (rp = results; rp != NULL; rp = rp->ai_next) {
> +		fd = socket(rp->ai_family, rp->ai_socktype, rp->ai_protocol);
> +		if (fd < 0)
> +			continue;
> +
> +		if (connect(fd, rp->ai_addr, rp->ai_addrlen) != -1)
> +			break;
> +

So you're using a blocking connect here that might take some time and 
are thus blocking the main event loop.  You can get away with this with 
AF_UNIX since the connection is local and probably nearly instantaneous. 
  With TCP I'm not sure this is a good idea...  Can we run the connect 
loop in a non-blocking fashion?

> +		close(fd);
> +	}
> +
> +	if (rp == NULL)
> +		return NULL;
> +
> +	freeaddrinfo(results);
> +
> +	return setup_dbus1(fd, guid, "tcp");
>   }
>   
>   static struct l_dbus *setup_address(const char *address)
> @@ -1170,6 +1253,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;
>   		}
>   	}
>   
> @@ -1232,6 +1318,7 @@ LIB_EXPORT void l_dbus_destroy(struct l_dbus *dbus)
>   
>   	l_free(dbus->guid);
>   	l_free(dbus->unique_name);
> +	l_free(dbus->transport);
>   
>   	_dbus_object_tree_free(dbus->tree);
>   
> 

Regards,
-Denis

[PATCH v1 0/2] Allow connection to a tcp dbus address

[Ramon Ribeiro]

[-- Attachment #1: Type: text/plain, Size: 981 bytes --]

This patch came from a demand in a project with industrial IoT for
monitoring a PLC (creating supervisor apps). The infrastructure was
built from a DBus service that can be accessed by a front-end in the
same local network.

Changes from v0 to v1:
 - Changes from atoi to strtoul
 - Handle port of zero as invalid
 - Use syscall `select` to avoid a blocking connect

Note: Maybe there is another approach using struct l_io and the
callback l_io_write but I'm not sure if that is enough, due TCP
connection timeout.

Ramon Ribeiro (2):
  dbus: Add support to tcp address
  unit: dbus: Add unit test to dbus tcp address

 .gitignore           |   1 +
 Makefile.am          |   3 +
 ell/dbus.c           | 151 ++++++++++++++++++++++++++++-
 unit/dbus.conf       |   2 +
 unit/test-dbus-tcp.c | 261 +++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 413 insertions(+), 5 deletions(-)
 create mode 100644 unit/test-dbus-tcp.c

-- 
2.7.4

[PATCH v1 1/2] dbus: Add support to tcp address

[Ramon Ribeiro]

[-- Attachment #1: Type: text/plain, Size: 6120 bytes --]

This patch allows a dbus client to connect to a dbus-daemon server
address listenning a TCP transport protocol.
A non-blocking TCP socket is created to avoid blocking the main event
loop.
---
 ell/dbus.c | 151 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 146 insertions(+), 5 deletions(-)

diff --git a/ell/dbus.c b/ell/dbus.c
index 3a84c90..3a495d0 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -31,8 +31,11 @@
 #include <stdarg.h>
 #include <stdlib.h>
 #include <string.h>
+#include <netdb.h>
 #include <sys/socket.h>
 #include <sys/un.h>
+#include <sys/select.h>
+#include <netinet/in.h>
 #include <errno.h>
 
 #include "util.h"
@@ -75,6 +78,7 @@ struct l_dbus_ops {
 struct l_dbus {
 	struct l_io *io;
 	char *guid;
+	char *transport;
 	bool negotiate_unix_fd;
 	bool support_unix_fd;
 	bool is_ready;
@@ -466,7 +470,8 @@ static bool auth_read_handler(struct l_io *io, void *user_data)
 				return false;
 
 			break;
-		}
+		} else if (len == 0)
+			close(fd);
 
 		offset += len;
 	}
@@ -511,7 +516,10 @@ static bool auth_read_handler(struct l_io *io, void *user_data)
 		} else if (!strncmp(ptr, "REJECTED ", 9)) {
 			static const char *command = "AUTH ANONYMOUS\r\n";
 
-			dbus->negotiate_unix_fd = true;
+			if (!strcmp(dbus->transport, "unix"))
+				dbus->negotiate_unix_fd = true;
+			else
+				dbus->negotiate_unix_fd = false;
 
 			classic->auth_command = l_strdup(command);
 			classic->auth_state = WAITING_FOR_OK;
@@ -680,6 +688,8 @@ static struct l_dbus_message *classic_recv_message(struct l_dbus *dbus)
 	len = recv(fd, &hdr, DBUS_HEADER_SIZE, MSG_PEEK | MSG_DONTWAIT);
 	if (len != DBUS_HEADER_SIZE)
 		return NULL;
+	else if (len == 0)
+		close(fd);
 
 	header_size = align_len(DBUS_HEADER_SIZE + hdr.dbus1.field_length, 8);
 	header = l_malloc(header_size);
@@ -1033,7 +1043,8 @@ static const struct l_dbus_ops classic_ops = {
 	.name_acquire = classic_name_acquire,
 };
 
-static struct l_dbus *setup_dbus1(int fd, const char *guid)
+static struct l_dbus *setup_dbus1(int fd, const char *guid,
+				  const char *transport)
 {
 	static const unsigned char creds = 0x00;
 	char uid[6], hexuid[12], *ptr = hexuid;
@@ -1065,11 +1076,13 @@ static struct l_dbus *setup_dbus1(int fd, const char *guid)
 
 	dbus_init(dbus, fd);
 	dbus->guid = l_strdup(guid);
+	dbus->transport = l_strdup(transport);
 
 	classic->auth_command = l_strdup_printf("AUTH EXTERNAL %s\r\n", hexuid);
 	classic->auth_state = WAITING_FOR_OK;
 
-	dbus->negotiate_unix_fd = true;
+	if (!strcmp(transport, "unix"))
+		dbus->negotiate_unix_fd = true;
 	dbus->support_unix_fd = false;
 
 	l_io_set_read_handler(dbus->io, auth_read_handler, dbus, NULL);
@@ -1145,7 +1158,131 @@ static struct l_dbus *setup_unix(char *params)
 		return NULL;
 	}
 
-	return setup_dbus1(fd, guid);
+	return setup_dbus1(fd, guid, "unix");
+}
+
+static struct l_dbus *setup_tcp(char *params)
+{
+	struct addrinfo hints;
+	struct addrinfo *results, *rp;
+	fd_set rfds, wfds;
+	socklen_t optlen;
+	struct timeval tv;
+	char *host = NULL, *port = NULL, *family = NULL, *guid = NULL;
+	unsigned long port_number;
+	int i, err;
+	int fd = -1, socket_fd = -1, optval = -1;
+
+	while (params) {
+		char *key = strsep(&params, ",");
+		char *value;
+
+		if (!key)
+			break;
+
+		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;
+		else if (!strcmp(key, "guid"))
+			guid = value;
+	}
+
+	if (!host || !port)
+		return NULL;
+
+	port_number = strtoul(port, NULL, 10);
+
+	if (port_number == 0 || port_number >= 65536)
+		return NULL;
+
+	memset(&hints, 0, sizeof(hints));
+
+	hints.ai_family = AF_UNSPEC;
+	if (family) {
+		if (!strcmp(family, "ipv4"))
+			hints.ai_family = AF_INET;
+		else if (!strcmp(family, "ipv6"))
+			hints.ai_family = AF_INET6;
+	}
+
+	hints.ai_socktype = SOCK_STREAM;
+	hints.ai_flags = 0;
+	hints.ai_protocol = 0;
+
+	if (getaddrinfo(host, port, &hints, &results) != 0)
+		return NULL;
+
+	FD_ZERO(&rfds);
+	FD_ZERO(&wfds);
+	for (rp = results; rp != NULL; rp = rp->ai_next) {
+		fd = socket(rp->ai_family, rp->ai_socktype | SOCK_NONBLOCK, rp->ai_protocol);
+		if (fd < 0)
+			continue;
+
+		errno = 0;
+		if (connect(fd, rp->ai_addr, rp->ai_addrlen) == -1) {
+			err = errno;
+			if (err == EINPROGRESS) {
+				FD_SET(fd, &rfds);
+				FD_SET(fd, &wfds);
+			} else if (err == ECONNREFUSED){
+				close(fd);
+			}
+		}
+	}
+
+	freeaddrinfo(results);
+
+	tv.tv_sec = 1;
+	tv.tv_usec = 0;
+
+	// Wait until sockets be ready, otherwise a timeout is thrown
+	if (select(fd + 1, &rfds, &wfds, NULL, &tv) == 0) {
+		close(fd);
+		return NULL;
+	}
+
+	for (i = 1; i < fd + 1; i++) {
+		if (!FD_ISSET(i, &wfds) && !FD_ISSET(i, &rfds))
+			continue;
+
+		optlen = sizeof(optval);
+
+		if (getsockopt(i,
+				SOL_SOCKET, SO_ERROR, &optval,
+				&optlen) == -1)
+			return NULL;
+
+		/**
+			* getsockopt() puts the errno value
+			* for connect into optval so 0 means
+			* no-error.
+			*/
+		if (optval == 0) {
+			socket_fd = i;
+		} else {
+			close(i);
+			return NULL;
+		}
+
+		/* Remove this socket from fd_set */
+		FD_CLR(i, &wfds);
+		FD_CLR(i, &rfds);
+	}
+
+	if (socket_fd < 0)
+		return NULL;
+
+	return setup_dbus1(socket_fd, guid, "tcp");
 }
 
 static struct l_dbus *setup_address(const char *address)
@@ -1170,6 +1307,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;
 		}
 	}
 
@@ -1232,6 +1372,7 @@ LIB_EXPORT void l_dbus_destroy(struct l_dbus *dbus)
 
 	l_free(dbus->guid);
 	l_free(dbus->unique_name);
+	l_free(dbus->transport);
 
 	_dbus_object_tree_free(dbus->tree);
 
-- 
2.7.4

[PATCH v1 2/2] unit: dbus: Add unit test to dbus tcp address

[Ramon Ribeiro]

[-- Attachment #1: Type: text/plain, Size: 7902 bytes --]

---
 .gitignore           |   1 +
 Makefile.am          |   3 +
 unit/dbus.conf       |   2 +
 unit/test-dbus-tcp.c | 261 +++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 267 insertions(+)
 create mode 100644 unit/test-dbus-tcp.c

diff --git a/.gitignore b/.gitignore
index 520c509..7d938f2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -34,6 +34,7 @@ unit/test-netlink
 unit/test-genl
 unit/test-genl-msg
 unit/test-dbus
+unit/test-dbus-tcp
 unit/test-dbus-message
 unit/test-dbus-message-fds
 unit/test-dbus-util
diff --git a/Makefile.am b/Makefile.am
index 5bc13bd..28b8452 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -175,6 +175,7 @@ unit_tests = unit/test-unit \
 
 dbus_tests = unit/test-hwdb \
 			unit/test-dbus \
+			unit/test-dbus-tcp \
 			unit/test-dbus-util \
 			unit/test-dbus-message \
 			unit/test-dbus-message-fds \
@@ -243,6 +244,8 @@ unit_test_genl_msg_LDADD = ell/libell-private.la
 
 unit_test_dbus_LDADD = ell/libell-private.la
 
+unit_test_dbus_tcp_LDADD = ell/libell-private.la
+
 unit_test_dbus_message_LDADD = ell/libell-private.la
 
 unit_test_dbus_message_fds_LDADD = ell/libell-private.la
diff --git a/unit/dbus.conf b/unit/dbus.conf
index 97e1bb5..6b1fccd 100644
--- a/unit/dbus.conf
+++ b/unit/dbus.conf
@@ -7,6 +7,8 @@
   <allow_anonymous />
 
   <listen>unix:path=/tmp/ell-test-bus</listen>
+  <listen>tcp:host=localhost,bind=*,port=1234,family=ipv4</listen>
+  <apparmor mode="disabled" />
 
   <policy context="default">
     <!-- Allow everything to be sent -->
diff --git a/unit/test-dbus-tcp.c b/unit/test-dbus-tcp.c
new file mode 100644
index 0000000..4e11ca9
--- /dev/null
+++ b/unit/test-dbus-tcp.c
@@ -0,0 +1,261 @@
+/*
+ *
+ *  Embedded Linux library
+ *
+ *  Copyright (C) 2011-2014  Intel Corporation. All rights reserved.
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define _GNU_SOURCE
+#include <unistd.h>
+#include <stdlib.h>
+#include <sys/wait.h>
+
+#include <ell/ell.h>
+
+#ifndef WAIT_ANY
+#define WAIT_ANY (-1) /* Any process */
+#endif
+
+#define TEST_BUS_ADDRESS "tcp:host=localhost,port=1234"
+
+static pid_t dbus_daemon_pid = -1;
+
+static void start_dbus_daemon(void)
+{
+	char *prg_argv[6];
+	char *prg_envp[1];
+	pid_t pid;
+
+	prg_argv[0] = "/usr/bin/dbus-daemon";
+	prg_argv[1] = "--nopidfile";
+	prg_argv[2] = "--nofork";
+	prg_argv[3] = "--config-file=" UNITDIR "dbus.conf";
+	prg_argv[4] = "--print-address";
+	prg_argv[5] = NULL;
+
+	prg_envp[0] = NULL;
+
+	l_info("launching dbus-daemon");
+
+	pid = fork();
+	if (pid < 0) {
+		l_error("failed to fork new process");
+		return;
+	}
+
+	if (pid == 0) {
+		execve(prg_argv[0], prg_argv, prg_envp);
+		exit(EXIT_SUCCESS);
+	}
+
+	l_info("dbus-daemon process %d created", pid);
+
+	dbus_daemon_pid = pid;
+}
+
+static void signal_handler(uint32_t signo, void *user_data)
+{
+	switch (signo) {
+	case SIGINT:
+	case SIGTERM:
+		l_info("Terminate");
+		l_main_quit();
+		break;
+	}
+}
+
+static void sigchld_handler(void *user_data)
+{
+	while (1) {
+		pid_t pid;
+		int status;
+
+		pid = waitpid(WAIT_ANY, &status, WNOHANG);
+		if (pid < 0 || pid == 0)
+			break;
+
+		l_info("process %d terminated with status=%d\n", pid, status);
+
+		if (pid == dbus_daemon_pid) {
+			dbus_daemon_pid = -1;
+			l_main_quit();
+		}
+	}
+}
+
+static void do_debug(const char *str, void *user_data)
+{
+	const char *prefix = user_data;
+
+	l_info("%s%s", prefix, str);
+}
+
+static void signal_message(struct l_dbus_message *message, void *user_data)
+{
+	const char *path, *interface, *member, *destination, *sender;
+
+	path = l_dbus_message_get_path(message);
+	destination = l_dbus_message_get_destination(message);
+
+	l_info("path=%s destination=%s", path, destination);
+
+	interface = l_dbus_message_get_interface(message);
+	member = l_dbus_message_get_member(message);
+
+	l_info("interface=%s member=%s", interface, member);
+
+	sender = l_dbus_message_get_sender(message);
+
+	l_info("sender=%s", sender);
+
+	if (!strcmp(member, "NameOwnerChanged")) {
+		const char *name, *old_owner, *new_owner;
+
+		if (!l_dbus_message_get_arguments(message, "sss",
+					&name, &old_owner, &new_owner))
+			return;
+
+		l_info("name=%s old=%s new=%s", name, old_owner, new_owner);
+	}
+}
+
+static void request_name_setup(struct l_dbus_message *message, void *user_data)
+{
+	const char *name = "org.test";
+
+	l_dbus_message_set_arguments(message, "su", name, 0);
+}
+
+static void request_name_callback(struct l_dbus_message *message,
+							void *user_data)
+{
+	const char *error, *text;
+	uint32_t result;
+
+	if (l_dbus_message_get_error(message, &error, &text)) {
+		l_error("error=%s", error);
+		l_error("message=%s", text);
+		goto done;
+	}
+
+	if (!l_dbus_message_get_arguments(message, "u", &result))
+		goto done;
+
+	l_info("request name result=%d", result);
+
+done:
+	l_main_quit();
+}
+
+static const char *match_rule = "type=signal,sender=org.freedesktop.DBus";
+
+static void add_match_setup(struct l_dbus_message *message, void *user_data)
+{
+	l_dbus_message_set_arguments(message, "s", match_rule);
+}
+
+static void add_match_callback(struct l_dbus_message *message, void *user_data)
+{
+	const char *error, *text;
+
+	if (l_dbus_message_get_error(message, &error, &text)) {
+		l_error("error=%s", error);
+		l_error("message=%s", text);
+		return;
+	}
+
+	if (!l_dbus_message_get_arguments(message, ""))
+		return;
+
+	l_info("add match");
+}
+
+static void ready_callback(void *user_data)
+{
+	l_info("ready");
+}
+
+static void disconnect_callback(void *user_data)
+{
+	l_main_quit();
+}
+
+int main(int argc, char *argv[])
+{
+	struct l_dbus *dbus;
+	struct l_signal *sigchld;
+	int i;
+
+	if (!l_main_init())
+		return -1;
+
+	l_log_set_stderr();
+
+	start_dbus_daemon();
+
+	for (i = 0; i < 10; i++) {
+		usleep(200 * 1000);
+
+		dbus = l_dbus_new(TEST_BUS_ADDRESS);
+		if (dbus)
+			break;
+
+	}
+
+	sigchld = l_signal_create(SIGCHLD, sigchld_handler, NULL, NULL);
+
+	if (!dbus)
+		goto done;
+
+	l_dbus_set_debug(dbus, do_debug, "[DBUS] ", NULL);
+
+	l_dbus_set_ready_handler(dbus, ready_callback, dbus, NULL);
+	l_dbus_set_disconnect_handler(dbus, disconnect_callback, NULL, NULL);
+
+	l_dbus_register(dbus, signal_message, NULL, NULL);
+
+	l_dbus_method_call(dbus, "org.freedesktop.DBus",
+				"/org/freedesktop/DBus",
+				"org.freedesktop.DBus", "AddMatch",
+				add_match_setup,
+				add_match_callback, NULL, NULL);
+
+	l_dbus_method_call(dbus, "org.freedesktop.DBus",
+				"/org/freedesktop/DBus",
+				"org.freedesktop.DBus", "RequestName",
+				request_name_setup,
+				request_name_callback, NULL, NULL);
+
+	l_main_run_with_signal(signal_handler, NULL);
+
+	l_dbus_destroy(dbus);
+
+done:
+	if (dbus_daemon_pid > 0)
+		kill(dbus_daemon_pid, SIGKILL);
+
+	l_signal_remove(sigchld);
+
+	l_main_exit();
+
+	return 0;
+}
-- 
2.7.4

Re: [PATCH v1 1/2] dbus: Add support to tcp address

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 937 bytes --]

Hi Ramon,

On 04/15/2019 07:23 AM, Ramon Ribeiro wrote:
> This patch allows a dbus client to connect to a dbus-daemon server
> address listenning a TCP transport protocol.
> A non-blocking TCP socket is created to avoid blocking the main event
> loop.
> ---
>   ell/dbus.c | 151 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 146 insertions(+), 5 deletions(-)
> 

<snip>

> +
> +	tv.tv_sec = 1;
> +	tv.tv_usec = 0;
> +
> +	// Wait until sockets be ready, otherwise a timeout is thrown
> +	if (select(fd + 1, &rfds, &wfds, NULL, &tv) == 0) {
> +		close(fd);
> +		return NULL;
> +	}

How does this help?  You're still sleeping in select and not allowing 
the main event loop to run.  You would need to setup an l_io for these 
fds (so that they're part of the main-event loop epoll_wait call) and 
wait for the connect to succeed or go on to next address.

Regards,
-Denis

[PATCH v2 0/2] Allow connection to a tcp dbus address

[Ramon Ribeiro]

[-- Attachment #1: Type: text/plain, Size: 613 bytes --]

Changes from v1 to v2:
 - Use l_io instead of select, to avoid blocking main loop.

Indeed this approach of using select is not the best. Thanks for the review.

Ramon Ribeiro (2):
  dbus: Add support to tcp address
  unit: dbus: Add unit test to dbus tcp address

 .gitignore           |   1 +
 Makefile.am          |   3 +
 ell/dbus.c           | 125 +++++++++++++++++++++++-
 unit/dbus.conf       |   2 +
 unit/test-dbus-tcp.c | 261 +++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 387 insertions(+), 5 deletions(-)
 create mode 100644 unit/test-dbus-tcp.c

-- 
2.7.4

[PATCH v2 1/2] dbus: Add support to tcp address

[Ramon Ribeiro]

[-- Attachment #1: Type: text/plain, Size: 5752 bytes --]

This patch allows a dbus client to connect to a dbus-daemon server
address listenning a TCP transport protocol.
A non-blocking TCP socket is created to avoid blocking the main event
loop.
---
 ell/dbus.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 120 insertions(+), 5 deletions(-)

diff --git a/ell/dbus.c b/ell/dbus.c
index 3a84c90..897e4b8 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -31,8 +31,10 @@
 #include <stdarg.h>
 #include <stdlib.h>
 #include <string.h>
+#include <netdb.h>
 #include <sys/socket.h>
 #include <sys/un.h>
+#include <netinet/in.h>
 #include <errno.h>
 
 #include "util.h"
@@ -43,6 +45,7 @@
 #include "dbus.h"
 #include "private.h"
 #include "dbus-private.h"
+#include "main.h"
 
 #define DEFAULT_SYSTEM_BUS_ADDRESS "unix:path=/var/run/dbus/system_bus_socket"
 
@@ -75,6 +78,7 @@ struct l_dbus_ops {
 struct l_dbus {
 	struct l_io *io;
 	char *guid;
+	char *transport;
 	bool negotiate_unix_fd;
 	bool support_unix_fd;
 	bool is_ready;
@@ -466,7 +470,8 @@ static bool auth_read_handler(struct l_io *io, void *user_data)
 				return false;
 
 			break;
-		}
+		} else if (len == 0)
+			close(fd);
 
 		offset += len;
 	}
@@ -511,7 +516,10 @@ static bool auth_read_handler(struct l_io *io, void *user_data)
 		} else if (!strncmp(ptr, "REJECTED ", 9)) {
 			static const char *command = "AUTH ANONYMOUS\r\n";
 
-			dbus->negotiate_unix_fd = true;
+			if (!strcmp(dbus->transport, "unix"))
+				dbus->negotiate_unix_fd = true;
+			else
+				dbus->negotiate_unix_fd = false;
 
 			classic->auth_command = l_strdup(command);
 			classic->auth_state = WAITING_FOR_OK;
@@ -680,6 +688,8 @@ static struct l_dbus_message *classic_recv_message(struct l_dbus *dbus)
 	len = recv(fd, &hdr, DBUS_HEADER_SIZE, MSG_PEEK | MSG_DONTWAIT);
 	if (len != DBUS_HEADER_SIZE)
 		return NULL;
+	else if (len == 0)
+		close(fd);
 
 	header_size = align_len(DBUS_HEADER_SIZE + hdr.dbus1.field_length, 8);
 	header = l_malloc(header_size);
@@ -1033,7 +1043,8 @@ static const struct l_dbus_ops classic_ops = {
 	.name_acquire = classic_name_acquire,
 };
 
-static struct l_dbus *setup_dbus1(int fd, const char *guid)
+static struct l_dbus *setup_dbus1(int fd, const char *guid,
+				  const char *transport)
 {
 	static const unsigned char creds = 0x00;
 	char uid[6], hexuid[12], *ptr = hexuid;
@@ -1065,11 +1076,13 @@ static struct l_dbus *setup_dbus1(int fd, const char *guid)
 
 	dbus_init(dbus, fd);
 	dbus->guid = l_strdup(guid);
+	dbus->transport = l_strdup(transport);
 
 	classic->auth_command = l_strdup_printf("AUTH EXTERNAL %s\r\n", hexuid);
 	classic->auth_state = WAITING_FOR_OK;
 
-	dbus->negotiate_unix_fd = true;
+	if (!strcmp(transport, "unix"))
+		dbus->negotiate_unix_fd = true;
 	dbus->support_unix_fd = false;
 
 	l_io_set_read_handler(dbus->io, auth_read_handler, dbus, NULL);
@@ -1145,7 +1158,105 @@ static struct l_dbus *setup_unix(char *params)
 		return NULL;
 	}
 
-	return setup_dbus1(fd, guid);
+	return setup_dbus1(fd, guid, "unix");
+}
+
+static bool on_tcp_connected(struct l_io *io, void *user_data)
+{
+	bool *is_connected = user_data;
+
+	*is_connected = true;
+
+	return true;
+}
+
+static struct l_dbus *setup_tcp(char *params)
+{
+	struct addrinfo hints;
+	struct addrinfo *results, *rp;
+	struct l_io *io_connect;
+	char *host = NULL, *port = NULL, *family = NULL, *guid = NULL;
+	unsigned long port_number;
+	bool is_connected = false;
+	int err;
+	int fd = -1;
+
+	while (params) {
+		char *key = strsep(&params, ",");
+		char *value;
+
+		if (!key)
+			break;
+
+		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;
+		else if (!strcmp(key, "guid"))
+			guid = value;
+	}
+
+	if (!host || !port)
+		return NULL;
+
+	port_number = strtoul(port, NULL, 10);
+
+	if (port_number == 0 || port_number >= 65536)
+		return NULL;
+
+	memset(&hints, 0, sizeof(hints));
+
+	hints.ai_family = AF_UNSPEC;
+	if (family) {
+		if (!strcmp(family, "ipv4"))
+			hints.ai_family = AF_INET;
+		else if (!strcmp(family, "ipv6"))
+			hints.ai_family = AF_INET6;
+	}
+
+	hints.ai_socktype = SOCK_STREAM;
+	hints.ai_flags = 0;
+	hints.ai_protocol = 0;
+
+	if (getaddrinfo(host, port, &hints, &results) != 0)
+		return NULL;
+
+	for (rp = results, is_connected = false;
+	     rp != NULL && !is_connected; rp = rp->ai_next) {
+		fd = socket(rp->ai_family, rp->ai_socktype | SOCK_NONBLOCK,
+			    rp->ai_protocol);
+		if (fd < 0)
+			continue;
+
+		if (!connect(fd, rp->ai_addr, rp->ai_addrlen))
+			break;
+
+		err = errno;
+
+		if (err != EINPROGRESS) {
+			close(fd);
+			continue;
+		}
+
+		io_connect = l_io_new(fd);
+		l_io_set_write_handler(io_connect,
+				       on_tcp_connected, &is_connected, NULL);
+		l_main_iterate(1000);
+		if (is_connected)
+			l_io_destroy(io_connect);
+	}
+
+	freeaddrinfo(results);
+
+	return setup_dbus1(fd, guid, "tcp");
 }
 
 static struct l_dbus *setup_address(const char *address)
@@ -1170,6 +1281,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;
 		}
 	}
 
@@ -1232,6 +1346,7 @@ LIB_EXPORT void l_dbus_destroy(struct l_dbus *dbus)
 
 	l_free(dbus->guid);
 	l_free(dbus->unique_name);
+	l_free(dbus->transport);
 
 	_dbus_object_tree_free(dbus->tree);
 
-- 
2.7.4

[PATCH v2 2/2] unit: dbus: Add unit test to dbus tcp address

[Ramon Ribeiro]

[-- Attachment #1: Type: text/plain, Size: 7902 bytes --]

---
 .gitignore           |   1 +
 Makefile.am          |   3 +
 unit/dbus.conf       |   2 +
 unit/test-dbus-tcp.c | 261 +++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 267 insertions(+)
 create mode 100644 unit/test-dbus-tcp.c

diff --git a/.gitignore b/.gitignore
index 520c509..7d938f2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -34,6 +34,7 @@ unit/test-netlink
 unit/test-genl
 unit/test-genl-msg
 unit/test-dbus
+unit/test-dbus-tcp
 unit/test-dbus-message
 unit/test-dbus-message-fds
 unit/test-dbus-util
diff --git a/Makefile.am b/Makefile.am
index 5bc13bd..28b8452 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -175,6 +175,7 @@ unit_tests = unit/test-unit \
 
 dbus_tests = unit/test-hwdb \
 			unit/test-dbus \
+			unit/test-dbus-tcp \
 			unit/test-dbus-util \
 			unit/test-dbus-message \
 			unit/test-dbus-message-fds \
@@ -243,6 +244,8 @@ unit_test_genl_msg_LDADD = ell/libell-private.la
 
 unit_test_dbus_LDADD = ell/libell-private.la
 
+unit_test_dbus_tcp_LDADD = ell/libell-private.la
+
 unit_test_dbus_message_LDADD = ell/libell-private.la
 
 unit_test_dbus_message_fds_LDADD = ell/libell-private.la
diff --git a/unit/dbus.conf b/unit/dbus.conf
index 97e1bb5..6b1fccd 100644
--- a/unit/dbus.conf
+++ b/unit/dbus.conf
@@ -7,6 +7,8 @@
   <allow_anonymous />
 
   <listen>unix:path=/tmp/ell-test-bus</listen>
+  <listen>tcp:host=localhost,bind=*,port=1234,family=ipv4</listen>
+  <apparmor mode="disabled" />
 
   <policy context="default">
     <!-- Allow everything to be sent -->
diff --git a/unit/test-dbus-tcp.c b/unit/test-dbus-tcp.c
new file mode 100644
index 0000000..4e11ca9
--- /dev/null
+++ b/unit/test-dbus-tcp.c
@@ -0,0 +1,261 @@
+/*
+ *
+ *  Embedded Linux library
+ *
+ *  Copyright (C) 2011-2014  Intel Corporation. All rights reserved.
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define _GNU_SOURCE
+#include <unistd.h>
+#include <stdlib.h>
+#include <sys/wait.h>
+
+#include <ell/ell.h>
+
+#ifndef WAIT_ANY
+#define WAIT_ANY (-1) /* Any process */
+#endif
+
+#define TEST_BUS_ADDRESS "tcp:host=localhost,port=1234"
+
+static pid_t dbus_daemon_pid = -1;
+
+static void start_dbus_daemon(void)
+{
+	char *prg_argv[6];
+	char *prg_envp[1];
+	pid_t pid;
+
+	prg_argv[0] = "/usr/bin/dbus-daemon";
+	prg_argv[1] = "--nopidfile";
+	prg_argv[2] = "--nofork";
+	prg_argv[3] = "--config-file=" UNITDIR "dbus.conf";
+	prg_argv[4] = "--print-address";
+	prg_argv[5] = NULL;
+
+	prg_envp[0] = NULL;
+
+	l_info("launching dbus-daemon");
+
+	pid = fork();
+	if (pid < 0) {
+		l_error("failed to fork new process");
+		return;
+	}
+
+	if (pid == 0) {
+		execve(prg_argv[0], prg_argv, prg_envp);
+		exit(EXIT_SUCCESS);
+	}
+
+	l_info("dbus-daemon process %d created", pid);
+
+	dbus_daemon_pid = pid;
+}
+
+static void signal_handler(uint32_t signo, void *user_data)
+{
+	switch (signo) {
+	case SIGINT:
+	case SIGTERM:
+		l_info("Terminate");
+		l_main_quit();
+		break;
+	}
+}
+
+static void sigchld_handler(void *user_data)
+{
+	while (1) {
+		pid_t pid;
+		int status;
+
+		pid = waitpid(WAIT_ANY, &status, WNOHANG);
+		if (pid < 0 || pid == 0)
+			break;
+
+		l_info("process %d terminated with status=%d\n", pid, status);
+
+		if (pid == dbus_daemon_pid) {
+			dbus_daemon_pid = -1;
+			l_main_quit();
+		}
+	}
+}
+
+static void do_debug(const char *str, void *user_data)
+{
+	const char *prefix = user_data;
+
+	l_info("%s%s", prefix, str);
+}
+
+static void signal_message(struct l_dbus_message *message, void *user_data)
+{
+	const char *path, *interface, *member, *destination, *sender;
+
+	path = l_dbus_message_get_path(message);
+	destination = l_dbus_message_get_destination(message);
+
+	l_info("path=%s destination=%s", path, destination);
+
+	interface = l_dbus_message_get_interface(message);
+	member = l_dbus_message_get_member(message);
+
+	l_info("interface=%s member=%s", interface, member);
+
+	sender = l_dbus_message_get_sender(message);
+
+	l_info("sender=%s", sender);
+
+	if (!strcmp(member, "NameOwnerChanged")) {
+		const char *name, *old_owner, *new_owner;
+
+		if (!l_dbus_message_get_arguments(message, "sss",
+					&name, &old_owner, &new_owner))
+			return;
+
+		l_info("name=%s old=%s new=%s", name, old_owner, new_owner);
+	}
+}
+
+static void request_name_setup(struct l_dbus_message *message, void *user_data)
+{
+	const char *name = "org.test";
+
+	l_dbus_message_set_arguments(message, "su", name, 0);
+}
+
+static void request_name_callback(struct l_dbus_message *message,
+							void *user_data)
+{
+	const char *error, *text;
+	uint32_t result;
+
+	if (l_dbus_message_get_error(message, &error, &text)) {
+		l_error("error=%s", error);
+		l_error("message=%s", text);
+		goto done;
+	}
+
+	if (!l_dbus_message_get_arguments(message, "u", &result))
+		goto done;
+
+	l_info("request name result=%d", result);
+
+done:
+	l_main_quit();
+}
+
+static const char *match_rule = "type=signal,sender=org.freedesktop.DBus";
+
+static void add_match_setup(struct l_dbus_message *message, void *user_data)
+{
+	l_dbus_message_set_arguments(message, "s", match_rule);
+}
+
+static void add_match_callback(struct l_dbus_message *message, void *user_data)
+{
+	const char *error, *text;
+
+	if (l_dbus_message_get_error(message, &error, &text)) {
+		l_error("error=%s", error);
+		l_error("message=%s", text);
+		return;
+	}
+
+	if (!l_dbus_message_get_arguments(message, ""))
+		return;
+
+	l_info("add match");
+}
+
+static void ready_callback(void *user_data)
+{
+	l_info("ready");
+}
+
+static void disconnect_callback(void *user_data)
+{
+	l_main_quit();
+}
+
+int main(int argc, char *argv[])
+{
+	struct l_dbus *dbus;
+	struct l_signal *sigchld;
+	int i;
+
+	if (!l_main_init())
+		return -1;
+
+	l_log_set_stderr();
+
+	start_dbus_daemon();
+
+	for (i = 0; i < 10; i++) {
+		usleep(200 * 1000);
+
+		dbus = l_dbus_new(TEST_BUS_ADDRESS);
+		if (dbus)
+			break;
+
+	}
+
+	sigchld = l_signal_create(SIGCHLD, sigchld_handler, NULL, NULL);
+
+	if (!dbus)
+		goto done;
+
+	l_dbus_set_debug(dbus, do_debug, "[DBUS] ", NULL);
+
+	l_dbus_set_ready_handler(dbus, ready_callback, dbus, NULL);
+	l_dbus_set_disconnect_handler(dbus, disconnect_callback, NULL, NULL);
+
+	l_dbus_register(dbus, signal_message, NULL, NULL);
+
+	l_dbus_method_call(dbus, "org.freedesktop.DBus",
+				"/org/freedesktop/DBus",
+				"org.freedesktop.DBus", "AddMatch",
+				add_match_setup,
+				add_match_callback, NULL, NULL);
+
+	l_dbus_method_call(dbus, "org.freedesktop.DBus",
+				"/org/freedesktop/DBus",
+				"org.freedesktop.DBus", "RequestName",
+				request_name_setup,
+				request_name_callback, NULL, NULL);
+
+	l_main_run_with_signal(signal_handler, NULL);
+
+	l_dbus_destroy(dbus);
+
+done:
+	if (dbus_daemon_pid > 0)
+		kill(dbus_daemon_pid, SIGKILL);
+
+	l_signal_remove(sigchld);
+
+	l_main_exit();
+
+	return 0;
+}
-- 
2.7.4

Re: [PATCH v2 1/2] dbus: Add support to tcp address

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 7706 bytes --]

Hi Ramon,

On 04/16/2019 08:05 AM, Ramon Ribeiro wrote:
> This patch allows a dbus client to connect to a dbus-daemon server
> address listenning a TCP transport protocol.
> A non-blocking TCP socket is created to avoid blocking the main event
> loop.
> ---
>   ell/dbus.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 120 insertions(+), 5 deletions(-)
> 
> diff --git a/ell/dbus.c b/ell/dbus.c
> index 3a84c90..897e4b8 100644
> --- a/ell/dbus.c
> +++ b/ell/dbus.c
> @@ -31,8 +31,10 @@
>   #include <stdarg.h>
>   #include <stdlib.h>
>   #include <string.h>
> +#include <netdb.h>
>   #include <sys/socket.h>
>   #include <sys/un.h>
> +#include <netinet/in.h>
>   #include <errno.h>
>   
>   #include "util.h"
> @@ -43,6 +45,7 @@
>   #include "dbus.h"
>   #include "private.h"
>   #include "dbus-private.h"
> +#include "main.h"
>   
>   #define DEFAULT_SYSTEM_BUS_ADDRESS "unix:path=/var/run/dbus/system_bus_socket"
>   
> @@ -75,6 +78,7 @@ struct l_dbus_ops {
>   struct l_dbus {
>   	struct l_io *io;
>   	char *guid;
> +	char *transport;
>   	bool negotiate_unix_fd;
>   	bool support_unix_fd;
>   	bool is_ready;
> @@ -466,7 +470,8 @@ static bool auth_read_handler(struct l_io *io, void *user_data)
>   				return false;
>   
>   			break;
> -		}
> +		} else if (len == 0)
> +			close(fd);
>   

You still need to return false from the handler or tell l_io about the 
IO being closed some other way.

>   		offset += len;
>   	}
> @@ -511,7 +516,10 @@ static bool auth_read_handler(struct l_io *io, void *user_data)
>   		} else if (!strncmp(ptr, "REJECTED ", 9)) {
>   			static const char *command = "AUTH ANONYMOUS\r\n";
>   
> -			dbus->negotiate_unix_fd = true;
> +			if (!strcmp(dbus->transport, "unix"))
> +				dbus->negotiate_unix_fd = true;
> +			else
> +				dbus->negotiate_unix_fd = false;
>   
>   			classic->auth_command = l_strdup(command);
>   			classic->auth_state = WAITING_FOR_OK;
> @@ -680,6 +688,8 @@ static struct l_dbus_message *classic_recv_message(struct l_dbus *dbus)
>   	len = recv(fd, &hdr, DBUS_HEADER_SIZE, MSG_PEEK | MSG_DONTWAIT);
>   	if (len != DBUS_HEADER_SIZE)
>   		return NULL;
> +	else if (len == 0)
> +		close(fd);

Aren't we returning NULL above if len != DBUS_HEADER_SIZE?  So how is 
this check ever true?

Also, just closing the socket isn't really enough, you need to tell the 
io object that the channel is closed.  E.g. by closing the fd and 
returning false from the read handler or by calling l_io_destroy.

>   
>   	header_size = align_len(DBUS_HEADER_SIZE + hdr.dbus1.field_length, 8);
>   	header = l_malloc(header_size);
> @@ -1033,7 +1043,8 @@ static const struct l_dbus_ops classic_ops = {
>   	.name_acquire = classic_name_acquire,
>   };
>   
> -static struct l_dbus *setup_dbus1(int fd, const char *guid)
> +static struct l_dbus *setup_dbus1(int fd, const char *guid,
> +				  const char *transport)
>   {
>   	static const unsigned char creds = 0x00;
>   	char uid[6], hexuid[12], *ptr = hexuid;
> @@ -1065,11 +1076,13 @@ static struct l_dbus *setup_dbus1(int fd, const char *guid)
>   
>   	dbus_init(dbus, fd);
>   	dbus->guid = l_strdup(guid);
> +	dbus->transport = l_strdup(transport);

Would a flag or an enum be enough?

>   
>   	classic->auth_command = l_strdup_printf("AUTH EXTERNAL %s\r\n", hexuid);
>   	classic->auth_state = WAITING_FOR_OK;
>   
> -	dbus->negotiate_unix_fd = true;
> +	if (!strcmp(transport, "unix"))
> +		dbus->negotiate_unix_fd = true;
>   	dbus->support_unix_fd = false;
>   
>   	l_io_set_read_handler(dbus->io, auth_read_handler, dbus, NULL);
> @@ -1145,7 +1158,105 @@ static struct l_dbus *setup_unix(char *params)
>   		return NULL;
>   	}
>   
> -	return setup_dbus1(fd, guid);
> +	return setup_dbus1(fd, guid, "unix");
> +}
> +
> +static bool on_tcp_connected(struct l_io *io, void *user_data)
> +{
> +	bool *is_connected = user_data;
> +
> +	*is_connected = true;
> +
> +	return true;
> +}
> +
> +static struct l_dbus *setup_tcp(char *params)
> +{
> +	struct addrinfo hints;
> +	struct addrinfo *results, *rp;
> +	struct l_io *io_connect;
> +	char *host = NULL, *port = NULL, *family = NULL, *guid = NULL;

So generally the Linux kernel coding style discourages such constructs. 
Each variable should be declared on a separate line.

> +	unsigned long port_number;
> +	bool is_connected = false;
> +	int err;
> +	int fd = -1;
> +
> +	while (params) {
> +		char *key = strsep(&params, ",");
> +		char *value;
> +
> +		if (!key)
> +			break;
> +
> +		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;
> +		else if (!strcmp(key, "guid"))
> +			guid = value;
> +	}
> +
> +	if (!host || !port)
> +		return NULL;
> +
> +	port_number = strtoul(port, NULL, 10);
> +
> +	if (port_number == 0 || port_number >= 65536)
> +		return NULL;
> +
> +	memset(&hints, 0, sizeof(hints));
> +
> +	hints.ai_family = AF_UNSPEC;
> +	if (family) {
> +		if (!strcmp(family, "ipv4"))
> +			hints.ai_family = AF_INET;
> +		else if (!strcmp(family, "ipv6"))
> +			hints.ai_family = AF_INET6;
> +	}
> +
> +	hints.ai_socktype = SOCK_STREAM;
> +	hints.ai_flags = 0;
> +	hints.ai_protocol = 0;
> +
> +	if (getaddrinfo(host, port, &hints, &results) != 0)
> +		return NULL;

So are you intending to support hostname resolution here?  Since you're 
not passing AI_NUMERICHOST or friends, I assume that you do.  Again, 
this means that you're blocking the library.

Non-blocking address resolution is actually a huge pain.  You can try to 
do threads, but since the rest of ell isn't thread safe, this is tricky. 
  The real solution is to port something like g_resolv from ConnMan to ell.

If you can live with only accepting IP addresses here, then that's what 
I would do first and live with the limitation until a non-blocking 
address resolution is added to ell.

> +
> +	for (rp = results, is_connected = false;
> +	     rp != NULL && !is_connected; rp = rp->ai_next) {
> +		fd = socket(rp->ai_family, rp->ai_socktype | SOCK_NONBLOCK,
> +			    rp->ai_protocol);
> +		if (fd < 0)
> +			continue;
> +
> +		if (!connect(fd, rp->ai_addr, rp->ai_addrlen))
> +			break;
> +
> +		err = errno;
> +
> +		if (err != EINPROGRESS) {
> +			close(fd);
> +			continue;
> +		}
> +
> +		io_connect = l_io_new(fd);
> +		l_io_set_write_handler(io_connect,
> +				       on_tcp_connected, &is_connected, NULL);
> +		l_main_iterate(1000);

Yikes.  Please don't do this, we don't do nested or re-entrant event 
loops and l_main_iterate is only meant to be used by integration with 
3rd library event loops.

> +		if (is_connected)
> +			l_io_destroy(io_connect);
> +	}
> +
> +	freeaddrinfo(results);
> +
> +	return setup_dbus1(fd, guid, "tcp");
>   }
>   
>   static struct l_dbus *setup_address(const char *address)
> @@ -1170,6 +1281,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;
>   		}
>   	}
>   
> @@ -1232,6 +1346,7 @@ LIB_EXPORT void l_dbus_destroy(struct l_dbus *dbus)
>   
>   	l_free(dbus->guid);
>   	l_free(dbus->unique_name);
> +	l_free(dbus->transport);
>   
>   	_dbus_object_tree_free(dbus->tree);
>   
> 

Regards,
-Denis