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 <giova...@giacobbi.net>
---
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
