On Wed, 28 Sep 2022 14:33:18 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:
conf_runas() handles several of the different
possible cases for the
--runas argument in a slightly odd order. Although it can parse both
numeric UIDs/GIDs and user/group names, it can't parse a numeric UID
combined with a group name or vice versa. That's not obviously useful, but
it's slightly surprising gap to have.
Ah, actually, I had noticed that: I usually type my UID as number, but
I can't remember GIDs... then I had half a mind of "fixing" that, and
never got to it.
[...]
+++ b/conf.c
@@ -859,46 +859,50 @@ dns6:
*
* Return: 0 on success, negative error code on failure
*/
-static int conf_runas(const char *opt, unsigned int *uid, unsigned int *gid)
+static int conf_runas(char *opt, unsigned int *uid, unsigned int *gid)
While I like your approach better than the existing one, I see one
detail that, albeit minor, might drive somebody mad: if the UID is
valid, but the GID isn't, we go back to conf_ugid():
if (ret)
err("Invalid --runas option: %s", runas);
and print the UID part, only, because we cut before the separator.
Perhaps we could simply put the separator back before
returning
-EINVAL (instead of copying the buffer) -- we just have zero or one
occurrences of ':'.
That would work, although making sure we do that on every exit path
looks messier than I'd like. I went with a different approach of
removing that message from the caller, and instead printing more
specific messages for each of the cases within conf_runas(). A little
more involved, but I think gives a marginally better UX overall.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!