Re: Missing 'libubus.so' and change of ABI in 19.07.8

2021-11-13 Thread Giovanni Giacobbi

On 13/11/2021 18:47, Jo-Philipp Wich wrote:

But I really can't see the breaking change, could you please point it out for 
me?


struct ubus_client changed its layout.



Ok but isn't that affecting only the binary executable 'ubusd'? I don't 
see 'libubus.so' including 'ubusd.h' even indirectly... Which makes 
sense as that struct is for managing a server-to-client, while the 
shared library only manages client-to-server connections?


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Missing 'libubus.so' and change of ABI in 19.07.8

2021-11-13 Thread Giovanni Giacobbi
Greetings. I have two questions about a recent change related to ubus 
package in 19.07.8.


The first question is related to the ABI version change from 20191227 to 
20210603. The changeset in the ubus repository the following:


git diff 041c9d1..38c7fdd

 CMakeLists.txt |  4 
 lua/ubus.c |  5 -
 ubusd.c| 25 +
 ubusd.h| 13 ++---
 ubusd_main.c   | 37 +
 ubusd_proto.c  |  1 +

But I really can't see the breaking change, could you please point it 
out for me?



The second question is the change of the library name from 'libubus.so' 
to 'libubus.so.20210603'. Was this intended? Could we at least add the 
symlink 'libubus.so'?


My take is that both of these changes, together with unsharing the ubus 
package, were really unnecessary in a stable branch nearing its end of life.


Thank you for insights and kind regards.

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[PATCH ubox] logread: fix erroneous message "Logread connected to" when using udp

2021-07-27 Thread Giovanni Giacobbi
When streaming the syslog messages via udp, the socket connection always 
succeeds by definition, but it can still fail to send. In such case,
the syslog keep repeating the following two messages:

failed to send log data to ip:port via udp
Logread connected to ip:port

With this change, only one initial message "Logread streaming to..." is logged.

Also fixed capital letter for "failed to send" message.

Signed-off-by: Giovanni Giacobbi 
---
 log/logread.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/log/logread.c b/log/logread.c
index a764742..dc00c43 100644
--- a/log/logread.c
+++ b/log/logread.c
@@ -80,41 +80,47 @@ static int check_facility_filter(int f)
 }
 
 static const char* getcodetext(int value, CODE *codetable) {
CODE *i;
 
if (value >= 0)
for (i = codetable; i->c_val != -1; i++)
if (i->c_val == value)
return (i->c_name);
return "";
 };
 
 static void log_handle_reconnect(struct uloop_timeout *timeout)
 {
sender.fd = usock((log_udp) ? (USOCK_UDP) : (USOCK_TCP), log_ip, 
log_port);
if (sender.fd < 0) {
fprintf(stderr, "failed to connect: %m\n");
uloop_timeout_set(&retry, 1000);
} else {
uloop_fd_add(&sender, ULOOP_READ);
-   syslog(LOG_INFO, "Logread connected to %s:%s\n", log_ip, 
log_port);
+   if (!log_udp) {
+   syslog(LOG_INFO, "Logread connected to %s:%s via 
tcp\n", log_ip, log_port);
+   }
+   else if (log_udp == 1) {
+   syslog(LOG_INFO, "Logread streaming to %s:%s via 
udp\n", log_ip, log_port);
+   log_udp = 2;
+   }
}
 }
 
 static void log_handle_fd(struct uloop_fd *u, unsigned int events)
 {
if (u->eof) {
uloop_fd_delete(u);
close(sender.fd);
sender.fd = -1;
uloop_timeout_set(&retry, 1000);
}
 }
 
 static int log_notify(struct blob_attr *msg)
 {
struct blob_attr *tb[__LOG_MAX];
struct stat s;
char buf[LOG_LINE_SIZE + 128];
char buf_ts[32];
uint32_t p;
@@ -175,41 +181,41 @@ static int log_notify(struct blob_attr *msg)
strncat(buf, hostname, sizeof(buf) - strlen(buf) - 1);
strncat(buf, " ", sizeof(buf) - strlen(buf) - 1);
}
if (log_prefix) {
strncat(buf, log_prefix, sizeof(buf) - strlen(buf) - 1);
strncat(buf, ": ", sizeof(buf) - strlen(buf) - 1);
}
if (blobmsg_get_u32(tb[LOG_SOURCE]) == SOURCE_KLOG)
strncat(buf, "kernel: ", sizeof(buf) - strlen(buf) - 1);
strncat(buf, m, sizeof(buf) - strlen(buf) - 1);
if (log_udp)
err = write(sender.fd, buf, strlen(buf));
else {
size_t buflen = strlen(buf);
if (!log_trailer_null)
buf[buflen] = '\n';
err = send(sender.fd, buf, buflen + 1, 0);
}
 
if (err < 0) {
-   syslog(LOG_INFO, "failed to send log data to %s:%s via 
%s\n",
+   syslog(LOG_INFO, "Failed to send log data to %s:%s via 
%s\n",
log_ip, log_port, (log_udp) ? ("udp") : 
("tcp"));
uloop_fd_delete(&sender);
close(sender.fd);
sender.fd = -1;
uloop_timeout_set(&retry, 1000);
}
} else {
snprintf(buf, sizeof(buf), "%s %s%s.%s%s %s\n",
c, log_timestamp ? buf_ts : "",
getcodetext(LOG_FAC(p) << 3, facilitynames),
getcodetext(LOG_PRI(p), prioritynames),
(blobmsg_get_u32(tb[LOG_SOURCE])) ? ("") : (" 
kernel:"), m);
ret = write(sender.fd, buf, strlen(buf));
}
 
if (log_type == LOG_FILE)
fsync(sender.fd);
 
return ret;
 }
-- 
2.17.2


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH ubox] logd: fix priviledge dropping order

2021-07-27 Thread Giovanni Giacobbi

On 27/07/2021 14:39, Giovanni Giacobbi wrote:


Fixes: 41664054b8b1 ("logd: fix ignored return values in set{gid,uid}")
Signed-off-by: Giovanni Giacobbi 
---
  log/logd.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)


Let me add some remarks about this change. Comments on each point are 
welcome.


1) The way it is now, the daemon is completely broken as it cannot 
setgid(2) after setuid(2) to a non-zero uid. This is the minimal fix to 
this situation. The bug exists already in 21.02 and master but the 
setgid is failing silently (so the egid remains zero).


2) I'm not sure that exit() is the right response in case of failure. 
Given the role of this daemon I would consider letting it run as root to 
avoid ending up without a syslog.


3) Currently the privilege de-escalation is activated by the sole 
presence of the "logd" user. If it doesn't exist, it silently keeps 
running as root, while if the user is there but setuid/setgid fails, it 
exits. I personally don't like this asymmetric behaviour, i'd rather 
have it print a warning (and maybe self-log it to itself as log 
critical?) and continue running as root. As an alternative, I would 
propose to add a command line switch to indicate the user to switch to, 
and then fail if user not found or setgid/setuid fail, so that you know 
for sure once you specify the command line switch that either privilege 
is dropped, or it doesn't start. Not using the command line switch would 
NOT attempt to switch user even if "logd" exists. Ideas? Should I submit 
this?



P.S. for the maintainer: If you commit this patch, please fix the typo 
in the subject, thx. "logd: fix privilege dropping order"




___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[PATCH ubox] logd: fix priviledge dropping order

2021-07-27 Thread Giovanni Giacobbi
Fixes: 41664054b8b1 ("logd: fix ignored return values in set{gid,uid}")
Signed-off-by: Giovanni Giacobbi 
---
 log/logd.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/log/logd.c b/log/logd.c
index 5d6c458..594b1e0 100644
--- a/log/logd.c
+++ b/log/logd.c
@@ -243,37 +243,37 @@ main(int argc, char **argv)
struct passwd *p = NULL;
 
signal(SIGPIPE, SIG_IGN);
while ((ch = getopt(argc, argv, "S:")) != -1) {
switch (ch) {
case 'S':
log_size = atoi(optarg);
if (log_size < 1)
log_size = 16;
break;
}
}
log_size *= 1024;
 
uloop_init();
log_init(log_size);
conn.cb = ubus_connect_handler;
ubus_auto_connect(&conn);
p = getpwnam("logd");
if (p) {
-   if (setuid(p->pw_uid) < 0) {
-   fprintf(stderr, "setuid() failed: %s\n", 
strerror(errno));
+   if (setgid(p->pw_gid) < 0) {
+   fprintf(stderr, "setgid() failed: %s\n", 
strerror(errno));
exit(1);
}
 
-   if (setgid(p->pw_gid) < 0) {
-   fprintf(stderr, "setgid() failed: %s\n", 
strerror(errno));
+   if (setuid(p->pw_uid) < 0) {
+   fprintf(stderr, "setuid() failed: %s\n", 
strerror(errno));
exit(1);
}
}
uloop_run();
log_shutdown();
uloop_done();
ubus_auto_shutdown(&conn);
 
return 0;
 }
-- 
2.17.2


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH 19.07 4/4] treewide: mark selected packages nonshared

2021-07-09 Thread Giovanni Giacobbi

On 04/07/2021 14:46, Adrian Schmutzler wrote:

   PKG_NAME:=json-c
   PKG_VERSION:=0.12.1
-PKG_RELEASE:=3.1
+PKG_RELEASE:=3.2

I've never seen a non integer release, is there a special reason for this?

I've also used this as standard scheme for changes in stable branches. The 
advantage is that you immediately see up to which version the changes are 
shared with master, and when it starts to deviate.


What would you do then if the upstream version is bumped, but the 
patchset/Makefile would still differ between the master and branch? In 
this case, you bump PKG_VERSION to 0.12.2, but if you restart the 
PKG_RELEASE from 1 for both the branch and the master you end up with 
two different "0.12.2-1" packages in let's say 21.02 and 19.07.




___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[PATCH urngd] Fix busy loop in case of ioctl(RNDADDENTROPY) failure

2021-01-30 Thread Giovanni Giacobbi
On systems where adding entropy to /dev/random using ioctl(RNDADDENTROPY) is 
not allowed (notably: docker containers), avoid entering a busy loop that 
consumes high cpu, as the poll loop will keep firing.

Cc: Petr Štetiar 
---
 urngd.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/urngd.c b/urngd.c
index 35ccdec..410d300 100644
--- a/urngd.c
+++ b/urngd.c
@@ -129,9 +129,14 @@ static size_t gather_entropy(struct urngd *u)
 static void low_entropy_cb(struct uloop_fd *ufd, unsigned int events)
 {
struct urngd *u = container_of(ufd, struct urngd, rnd_fd);
+   size_t res;
 
DEBUG(2, DEV_RANDOM " signals low entropy\n");
-   gather_entropy(u);
+   res = gather_entropy(u);
+   if (!res) {
+   DEBUG(2, "gather_entropy failed, delaying further attempts\n");
+   sleep(60);
+   }
 }
 
 static void urngd_done(struct urngd *u)
-- 
Giovanni Giacobbi


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel