On 2024/12/26 10:47, William Rusnack wrote:
> >Synopsis:    The iked(8) daemon currently requires root privileges even when 
> >run with -n (configtest mode), which only validates the configuration file 
> >syntax. This prevents system administrators from validating iked 
> >configuration files from non-privileged accounts.
> >Category:    bin
> >Description:
>       It is not possible to run `iked -n -f alt/iked.conf` as a non-root user.
>       Looking at the code in iked.c, when -n is specified, iked exits 
> immediately after parsing the config file without performing any privileged 
> operations (PF_KEY socket, UDP sockets, etc.).
>       The root check happens before the -n handling but doesn't appear 
> necessary since only config file access is needed in this mode.

I think this makes sense.

>       Other OpenBSD daemons like httpd(8), bgpd(8), and sshd(8) allow 
> non-root users to validate their respective configuration files using similar 
> test modes (-n or -t flags). 

Some do, some don't. (e.g. ldapd doesn't).

> >Fix:
>       Move the root privileges check in iked.c to after the IKED_OPT_NOACTION 
> check, allowing non-root users to validate config files while still requiring 
> root for normal daemon operation.
>       
>       ```diff
>               group_init();
>               policy_init(env);
>       
>       -       /* check for root privileges */
>       -       if (geteuid())
>       -               errx(1, "need root privileges");
>       
>               if ((ps->ps_pw =  getpwnam(IKED_USER)) == NULL)
>                       errx(1, "unknown user %s", IKED_USER);
>       
>               /* Configure the control socket */
>               ps->ps_csock.cs_name = sock;
>       
>               log_init(debug, LOG_DAEMON);
>               log_setverbose(verbose);
>       
>               if (opts & IKED_OPT_NOACTION)
>                       ps->ps_noaction = 1;
>       +       else
>       +               /* check for root privileges */
>       +               if (geteuid())
>       +                       errx(1, "need root privileges");
>       ```

Diff that can be applied with patch:
(I added optional braces as it's multi line and I think clearer
like that).

ok?


Index: iked.c
===================================================================
RCS file: /cvs/src/sbin/iked/iked.c,v
diff -u -p -r1.71 iked.c
--- iked.c      13 Jul 2024 12:22:46 -0000      1.71
+++ iked.c      26 Dec 2024 16:27:08 -0000
@@ -178,10 +178,6 @@ main(int argc, char *argv[])
        group_init();
        policy_init(env);

-       /* check for root privileges */
-       if (geteuid())
-               errx(1, "need root privileges");
-
        if ((ps->ps_pw =  getpwnam(IKED_USER)) == NULL)
                errx(1, "unknown user %s", IKED_USER);

@@ -193,6 +189,11 @@ main(int argc, char *argv[])

        if (opts & IKED_OPT_NOACTION)
                ps->ps_noaction = 1;
+       else {
+               /* check for root privileges */
+               if (geteuid())
+                       errx(1, "need root privileges");
+       }

        ps->ps_instance = proc_instance;
        if (title != NULL)

Reply via email to