On Thu, Dec 26, 2024 at 04:29:30PM +0000, Stuart Henderson wrote:
> 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?

ok tobhe@

> 
> 
> 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