[PATCH 1/2] dbus: Call destroy function in l_dbus_remove_watch
by Andrew Zaborowski
l_dbus_remove_watch (_dbus_name_cache_remove_watch) marks name watches
as removed and schedules a service_watch_remove_all call. Do the
watch's destroy callback when marking the watch as removed instead of
waiting until the service_watch_remove_all idle callback to do this.
Semantically it may be ok to do the destroy callback later but I think
it's easier on the user if they can be sure they'll get no more
callbacks after using l_dbus_remove_watch.
---
ell/dbus-name-cache.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/ell/dbus-name-cache.c b/ell/dbus-name-cache.c
index cb6a516..b09cc97 100644
--- a/ell/dbus-name-cache.c
+++ b/ell/dbus-name-cache.c
@@ -283,6 +283,12 @@ static void service_watch_mark(const void *key, void *value, void *user_data)
watch->removed = true;
watch->connect_func = NULL;
watch->disconnect_func = NULL;
+
+ if (watch->destroy) {
+ watch->destroy(watch->user_data);
+ watch->destroy = NULL;
+ }
+
*id = 0;
break;
}
--
2.25.1
4 months, 3 weeks
RFC: DBus send_with_reply wrapper doesn't time out
by Gix, Brian
In Bluetooth Mesh, we have a particular interaction between the mesh daemon, and Applications where we need to
ensure that the application has received a critical piece of data (the node token), and if it doesn't, we need
to destroy the newly created node.
Specifically we have the JoinComplete() method, that the Application is required to implement, and it is
invoked by the daemon with l_dbus_send_with_reply().
This *mostly* works, in that the daemon gets any responses that the App sends, and is alerted if the App
crashes in such a way the it's dbus socket gets closed.
However: If the Application has a bug in that it *never* responds (for instance, it returns NULL in the
JoinComplete() message handler, and never responds asyncronously either), then the mesh daemon never receives a
call on it's callback function. I had always assumed that the dbus-daemon would timeout on responses, but this
does not appear to be the case.
Is timing out reply required methods the dbus daemon responsibility? Is it the l_dbus_send_with_reply()
responsibility? The mesh daemon responsibility? Ideally, the App just responds, but we have no control over
how Apps respond to method calls.
5 months, 2 weeks
[PATCH v1] ell/rtnl: fix compile with older toolchains
by Peter Seiderer
Older toolchains need to include sys/types.h and sys/socket.h before
linux/if.h, RTA_PREF was introduces with linux-4.1.x.
Fixes:
In file included from ell/rtnl.c:28:0:
.../host/arm-buildroot-linux-gnueabi/sysroot/usr/include/linux/if.h:185:19: error: field 'ifru_addr' has incomplete type
.../host/arm-buildroot-linux-gnueabi/sysroot/usr/include/linux/if.h:186:19: error: field 'ifru_dstaddr' has incomplete type
.../host/arm-buildroot-linux-gnueabi/sysroot/usr/include/linux/if.h:187:19: error: field 'ifru_broadaddr' has incomplete type
.../host/arm-buildroot-linux-gnueabi/sysroot/usr/include/linux/if.h:188:19: error: field 'ifru_netmask' has incomplete type
.../arm-buildroot-linux-gnueabi/sysroot/usr/include/linux/if.h:189:20: error: field 'ifru_hwaddr' has incomplete type
ell/rtnl.c: In function 'l_rtnl_route_extract':
ell/rtnl.c:120:8: error: 'RTA_PREF' undeclared (first use in this function)
---
configure.ac | 2 ++
ell/rtnl.c | 7 ++++++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac
index 0351f89..3be35a4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -113,6 +113,8 @@ AC_CHECK_LIB(dl, dlopen, dummy=yes,
AC_CHECK_HEADERS(linux/types.h linux/if_alg.h)
+AC_CHECK_DECLS([RTA_PREF], [], [], [[#include <linux/rtnetlink.h>]])
+
AC_ARG_ENABLE(glib, AC_HELP_STRING([--enable-glib],
[enable ell/glib main loop example]),
[enable_glib=${enableval}])
diff --git a/ell/rtnl.c b/ell/rtnl.c
index dc83937..3493d34 100644
--- a/ell/rtnl.c
+++ b/ell/rtnl.c
@@ -25,8 +25,9 @@
#endif
#define _GNU_SOURCE
-#include <linux/if.h>
+#include <sys/types.h>
#include <sys/socket.h>
+#include <linux/if.h>
#include <arpa/inet.h>
#include "util.h"
@@ -35,6 +36,10 @@
#include "rtnl.h"
#include "private.h"
+#if defined HAVE_DECL_RTA_PREF && !HAVE_DECL_RTA_PREF
+#define RTA_PREF 20
+#endif
+
static size_t rta_add_u8(void *rta_buf, unsigned short type, uint8_t value)
{
struct rtattr *rta = rta_buf;
--
2.26.0
5 months, 2 weeks
[PATCH 1/5] cert: Fix verification of the 2nd certificate in chain
by Andrew Zaborowski
In the case that the trusted CA certificates are provided to
l_certchain_verify() and one of them binary-matches the top CA
certificate in the chain being verified, we'd skip linking it to the
keyring and we'd link the next certificate directly. As a result
that second certificate would be linked to an empty unrestricted keyring
and would not be verified at all.
Instead add the top CA to the keyring and restrict it the same way we
proceed when no trusted CAs are provided.
---
ell/cert.c | 33 +++++++++++++++++++--------------
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/ell/cert.c b/ell/cert.c
index d1f58d7..eb04765 100644
--- a/ell/cert.c
+++ b/ell/cert.c
@@ -454,6 +454,7 @@ LIB_EXPORT bool l_certchain_verify(struct l_certchain *chain,
struct l_cert *cert;
struct l_key *prev_key = NULL;
int verified = 0;
+ bool ca_match;
static char error_buf[200];
if (unlikely(!chain || !chain->leaf))
@@ -464,6 +465,7 @@ LIB_EXPORT bool l_certchain_verify(struct l_certchain *chain,
RETURN_ERROR("Can't create verify keyring");
cert = chain->ca;
+ ca_match = cert_is_in_set(cert, ca_certs);
/*
* For TLS compatibility the trusted root CA certificate is
@@ -477,19 +479,22 @@ LIB_EXPORT bool l_certchain_verify(struct l_certchain *chain,
* already possess it in order to validate it in any case."
*
* The following is an optimization to skip verifying the root
- * cert in the chain if it is identical to one of the trusted CA
- * certificates. It also happens to work around a kernel issue
- * preventing self-signed certificates missing the AKID
- * extension from being linked to a keyring.
+ * cert in the chain if it is bitwise-identical to one of the
+ * trusted CA certificates. In that case we don't have to load
+ * all of the trusted certificates into the kernel, link them
+ * to @ca_ring or link @ca_ring to @verify_ring, instead we
+ * load the first certificate into @verify_ring before we set
+ * the restric mode on it, same as when no trusted CAs are
+ * provided.
+ *
+ * Note this happens to work around a kernel issue preventing
+ * self-signed certificates missing the optional AKID extension
+ * from being linked to a restricted keyring. That issue would
+ * have affected us if the trusted CA set included such
+ * certificate and the same certificate was at the root of
+ * the chain.
*/
- if (cert_is_in_set(cert, ca_certs)) {
- verified++;
- cert = cert->issued;
- if (!cert)
- return true;
-
- prev_key = cert_try_link(cert, verify_ring);
- } else if (ca_certs) {
+ if (ca_certs && !ca_match) {
ca_ring = cert_set_to_keyring(ca_certs, error_buf);
if (!ca_ring) {
if (error)
@@ -550,8 +555,8 @@ LIB_EXPORT bool l_certchain_verify(struct l_certchain *chain,
"verified against trusted CA(s) and the "
"following %i top certificates verified ok",
verified + 1, total,
- ca_certs && verified ? "" : "not ",
- verified ? verified - 1 : 0);
+ ca_certs && (ca_match || verified) ? "" :
+ "not ", verified ? verified - 1 : 0);
}
l_key_free(prev_key);
--
2.20.1
5 months, 2 weeks
[PATCH 1/3] cert: Fix verification of the 2nd certificate in chain
by Andrew Zaborowski
In the case that the trusted CA certificates are provided to
l_certchain_verify() and one of them binary-matches the top CA
certificate in the chain being verified, we'd skip linking it to the
keyring and we'd link the next certificate directly. As a result
that second certificate would be linked to an empty unrestricted keyring
and would not be verified at all.
Instead add the top CA to the keyring and restrict it the same way we
proceed when no trusted CAs are provided.
---
ell/cert.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/ell/cert.c b/ell/cert.c
index d1f58d7..dc6b845 100644
--- a/ell/cert.c
+++ b/ell/cert.c
@@ -454,6 +454,7 @@ LIB_EXPORT bool l_certchain_verify(struct l_certchain *chain,
struct l_cert *cert;
struct l_key *prev_key = NULL;
int verified = 0;
+ bool ca_match;
static char error_buf[200];
if (unlikely(!chain || !chain->leaf))
@@ -464,6 +465,7 @@ LIB_EXPORT bool l_certchain_verify(struct l_certchain *chain,
RETURN_ERROR("Can't create verify keyring");
cert = chain->ca;
+ ca_match = cert_is_in_set(cert, ca_certs);
/*
* For TLS compatibility the trusted root CA certificate is
@@ -480,16 +482,9 @@ LIB_EXPORT bool l_certchain_verify(struct l_certchain *chain,
* cert in the chain if it is identical to one of the trusted CA
* certificates. It also happens to work around a kernel issue
* preventing self-signed certificates missing the AKID
- * extension from being linked to a keyring.
+ * extension from being linked to a restricted keyring.
*/
- if (cert_is_in_set(cert, ca_certs)) {
- verified++;
- cert = cert->issued;
- if (!cert)
- return true;
-
- prev_key = cert_try_link(cert, verify_ring);
- } else if (ca_certs) {
+ if (ca_certs && !ca_match) {
ca_ring = cert_set_to_keyring(ca_certs, error_buf);
if (!ca_ring) {
if (error)
@@ -550,8 +545,8 @@ LIB_EXPORT bool l_certchain_verify(struct l_certchain *chain,
"verified against trusted CA(s) and the "
"following %i top certificates verified ok",
verified + 1, total,
- ca_certs && verified ? "" : "not ",
- verified ? verified - 1 : 0);
+ ca_certs && (ca_match || verified) ? "" :
+ "not ", verified ? verified - 1 : 0);
}
l_key_free(prev_key);
--
2.20.1
5 months, 2 weeks
[PATCH 1/4] ell: add missing void to function declarations
by Rosen Penev
Found with -Wstrict-prototypes.
---
ell/checksum.h | 2 +-
ell/dbus-private.h | 2 +-
ell/main.h | 2 +-
ell/random.h | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/ell/checksum.h b/ell/checksum.h
index 08d74b7..531fcb0 100644
--- a/ell/checksum.h
+++ b/ell/checksum.h
@@ -64,7 +64,7 @@ ssize_t l_checksum_get_digest(struct l_checksum *checksum,
char *l_checksum_get_string(struct l_checksum *checksum);
bool l_checksum_is_supported(enum l_checksum_type type, bool check_hmac);
-bool l_checksum_cmac_aes_supported();
+bool l_checksum_cmac_aes_supported(void);
ssize_t l_checksum_digest_length(enum l_checksum_type type);
diff --git a/ell/dbus-private.h b/ell/dbus-private.h
index be62691..d34a298 100644
--- a/ell/dbus-private.h
+++ b/ell/dbus-private.h
@@ -189,7 +189,7 @@ struct _dbus_signal *_dbus_interface_find_signal(struct l_dbus_interface *i,
struct _dbus_property *_dbus_interface_find_property(struct l_dbus_interface *i,
const char *property);
-struct _dbus_object_tree *_dbus_object_tree_new();
+struct _dbus_object_tree *_dbus_object_tree_new(void);
void _dbus_object_tree_free(struct _dbus_object_tree *tree);
struct object_node *_dbus_object_tree_makepath(struct _dbus_object_tree *tree,
diff --git a/ell/main.h b/ell/main.h
index 99b34ad..51e051a 100644
--- a/ell/main.h
+++ b/ell/main.h
@@ -42,7 +42,7 @@ typedef void (*l_main_signal_cb_t) (uint32_t signo, void *user_data);
int l_main_run_with_signal(l_main_signal_cb_t callback, void *user_data);
-int l_main_get_epoll_fd();
+int l_main_get_epoll_fd(void);
#ifdef __cplusplus
}
diff --git a/ell/random.h b/ell/random.h
index 49a4637..acc278b 100644
--- a/ell/random.h
+++ b/ell/random.h
@@ -32,7 +32,7 @@ extern "C" {
#include <stdint.h>
bool l_getrandom(void *buf, size_t len);
-bool l_getrandom_is_supported();
+bool l_getrandom_is_supported(void);
uint32_t l_getrandom_uint32(void);
--
2.25.1
5 months, 2 weeks
[PATCH] ell/ecc.h: fix build with uclibc
by Fabrice Fontaine
ssize_t is defined in sys/types.h
Fixes:
- http://autobuild.buildroot.org/results/444c9deb728fb041e560d940145f96cc4f...
Signed-off-by: Fabrice Fontaine <fontaine.fabrice(a)gmail.com>
---
ell/ecc.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ell/ecc.h b/ell/ecc.h
index 26a5889..65c49e8 100644
--- a/ell/ecc.h
+++ b/ell/ecc.h
@@ -27,6 +27,8 @@
extern "C" {
#endif
+#include <sys/types.h> // for ssize_t
+
#define L_ECC_MAX_DIGITS 6
#define L_ECC_SCALAR_MAX_BYTES L_ECC_MAX_DIGITS * 8
#define L_ECC_POINT_MAX_BYTES L_ECC_SCALAR_MAX_BYTES * 2
--
2.20.1
5 months, 2 weeks
[PATCH] settings: clear copies of values in set_value
by Andrew Zaborowski
We bzero the setting values before they're free elsewhere in the file
but not in set_value, and we do use it in iwd to set a PIN.
---
ell/settings.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/ell/settings.c b/ell/settings.c
index f4326d6..945dff7 100644
--- a/ell/settings.c
+++ b/ell/settings.c
@@ -885,17 +885,13 @@ static bool set_value(struct l_settings *settings, const char *group_name,
if (!validate_group_name(group_name)) {
l_util_debug(settings->debug_handler, settings->debug_data,
"Invalid group name %s", group_name);
- l_free(value);
-
- return false;
+ goto error;
}
if (!validate_key(key)) {
l_util_debug(settings->debug_handler, settings->debug_data,
"Invalid key %s", key);
- l_free(value);
-
- return false;
+ goto error;
}
group = l_queue_find(settings->groups, group_match, group_name);
@@ -919,10 +915,16 @@ add_pair:
return true;
}
+ explicit_bzero(pair->value, strlen(pair->value));
l_free(pair->value);
pair->value = value;
return true;
+
+error:
+ explicit_bzero(value, strlen(value));
+ l_free(value);
+ return false;
}
LIB_EXPORT bool l_settings_set_value(struct l_settings *settings,
--
2.20.1
5 months, 2 weeks
