Re: Add rtable capability to login.conf
On Fri, Feb 18, 2022 at 03:25:51PM -0500, Ted Unangst wrote:
> On 2022-02-06, Ted Unangst wrote:
> > On 2022-02-05, Matthew Martin wrote:
> > > On Sat, Jan 29, 2022 at 06:25:32PM -0600, Matthew Martin wrote:
> > > > On Sat, Jan 29, 2022 at 07:10:00PM -0500, Ted Unangst wrote:
> > > > > I believe it would be better to add setrtable to id pledge.
> > >
> > > ping
> > >
> > > Also are there any opinions on adding LOGIN_SETRTABLE to doas?
> >
> > I think this diff looks fine.
> >
> > For doas, we can use setall with an extra note in the man page.
>
> Final auction for oks. I think all the login.conf.d changes are in now.
>
> Plan is add setrtable to pledge first so people don't get caught, then libc.
ping?
> > Index: doas.1
> > ===
> > RCS file: /home/cvs/src/usr.bin/doas/doas.1,v
> > retrieving revision 1.25
> > diff -u -p -r1.25 doas.1
> > --- doas.1 16 Jan 2021 09:18:41 - 1.25
> > +++ doas.1 6 Feb 2022 18:41:53 -
> > @@ -54,6 +54,8 @@ and
> > and the
> > .Xr umask 2
> > are set to values appropriate for the target user.
> > +Other values may also be set as specified in
> > +.Pa /etc/login.conf .
> > .Ev DOAS_USER
> > is set to the name of the user executing
> > .Nm .
> > Index: doas.c
> > ===
> > RCS file: /home/cvs/src/usr.bin/doas/doas.c,v
> > retrieving revision 1.93
> > diff -u -p -r1.93 doas.c
> > --- doas.c 30 Nov 2021 20:08:15 - 1.93
> > +++ doas.c 6 Feb 2022 18:39:38 -
> > @@ -450,10 +450,7 @@ main(int argc, char **argv)
> > if (targpw == NULL)
> > errx(1, "no passwd entry for target");
> >
> > - if (setusercontext(NULL, targpw, target, LOGIN_SETGROUP |
> > - LOGIN_SETPATH |
> > - LOGIN_SETPRIORITY | LOGIN_SETRESOURCES | LOGIN_SETUMASK |
> > - LOGIN_SETUSER) != 0)
> > + if (setusercontext(NULL, targpw, target, LOGIN_SETALL) == -1)
> > errx(1, "failed to set user context for target");
> >
> > if (pledge("stdio rpath exec", NULL) == -1)
> >
> >
Re: Add rtable capability to login.conf
On 2022-02-06, Ted Unangst wrote:
> On 2022-02-05, Matthew Martin wrote:
> > On Sat, Jan 29, 2022 at 06:25:32PM -0600, Matthew Martin wrote:
> > > On Sat, Jan 29, 2022 at 07:10:00PM -0500, Ted Unangst wrote:
> > > > I believe it would be better to add setrtable to id pledge.
> >
> > ping
> >
> > Also are there any opinions on adding LOGIN_SETRTABLE to doas?
>
> I think this diff looks fine.
>
> For doas, we can use setall with an extra note in the man page.
Final auction for oks. I think all the login.conf.d changes are in now.
Plan is add setrtable to pledge first so people don't get caught, then libc.
>
>
> Index: doas.1
> ===
> RCS file: /home/cvs/src/usr.bin/doas/doas.1,v
> retrieving revision 1.25
> diff -u -p -r1.25 doas.1
> --- doas.116 Jan 2021 09:18:41 - 1.25
> +++ doas.16 Feb 2022 18:41:53 -
> @@ -54,6 +54,8 @@ and
> and the
> .Xr umask 2
> are set to values appropriate for the target user.
> +Other values may also be set as specified in
> +.Pa /etc/login.conf .
> .Ev DOAS_USER
> is set to the name of the user executing
> .Nm .
> Index: doas.c
> ===
> RCS file: /home/cvs/src/usr.bin/doas/doas.c,v
> retrieving revision 1.93
> diff -u -p -r1.93 doas.c
> --- doas.c30 Nov 2021 20:08:15 - 1.93
> +++ doas.c6 Feb 2022 18:39:38 -
> @@ -450,10 +450,7 @@ main(int argc, char **argv)
> if (targpw == NULL)
> errx(1, "no passwd entry for target");
>
> - if (setusercontext(NULL, targpw, target, LOGIN_SETGROUP |
> - LOGIN_SETPATH |
> - LOGIN_SETPRIORITY | LOGIN_SETRESOURCES | LOGIN_SETUMASK |
> - LOGIN_SETUSER) != 0)
> + if (setusercontext(NULL, targpw, target, LOGIN_SETALL) == -1)
> errx(1, "failed to set user context for target");
>
> if (pledge("stdio rpath exec", NULL) == -1)
>
>
Re: Add rtable capability to login.conf
On 2022-02-05, Matthew Martin wrote:
> On Sat, Jan 29, 2022 at 06:25:32PM -0600, Matthew Martin wrote:
> > On Sat, Jan 29, 2022 at 07:10:00PM -0500, Ted Unangst wrote:
> > > I believe it would be better to add setrtable to id pledge.
>
> ping
>
> Also are there any opinions on adding LOGIN_SETRTABLE to doas?
I think this diff looks fine.
For doas, we can use setall with an extra note in the man page.
Index: doas.1
===
RCS file: /home/cvs/src/usr.bin/doas/doas.1,v
retrieving revision 1.25
diff -u -p -r1.25 doas.1
--- doas.1 16 Jan 2021 09:18:41 - 1.25
+++ doas.1 6 Feb 2022 18:41:53 -
@@ -54,6 +54,8 @@ and
and the
.Xr umask 2
are set to values appropriate for the target user.
+Other values may also be set as specified in
+.Pa /etc/login.conf .
.Ev DOAS_USER
is set to the name of the user executing
.Nm .
Index: doas.c
===
RCS file: /home/cvs/src/usr.bin/doas/doas.c,v
retrieving revision 1.93
diff -u -p -r1.93 doas.c
--- doas.c 30 Nov 2021 20:08:15 - 1.93
+++ doas.c 6 Feb 2022 18:39:38 -
@@ -450,10 +450,7 @@ main(int argc, char **argv)
if (targpw == NULL)
errx(1, "no passwd entry for target");
- if (setusercontext(NULL, targpw, target, LOGIN_SETGROUP |
- LOGIN_SETPATH |
- LOGIN_SETPRIORITY | LOGIN_SETRESOURCES | LOGIN_SETUMASK |
- LOGIN_SETUSER) != 0)
+ if (setusercontext(NULL, targpw, target, LOGIN_SETALL) == -1)
errx(1, "failed to set user context for target");
if (pledge("stdio rpath exec", NULL) == -1)
Re: Add rtable capability to login.conf
On Sat, Jan 29, 2022 at 06:25:32PM -0600, Matthew Martin wrote:
> On Sat, Jan 29, 2022 at 07:10:00PM -0500, Ted Unangst wrote:
> > I believe it would be better to add setrtable to id pledge.
ping
Also are there any opinions on adding LOGIN_SETRTABLE to doas?
diff --git include/login_cap.h include/login_cap.h
index d9a4c2c349c..1e831b6471a 100644
--- include/login_cap.h
+++ include/login_cap.h
@@ -53,7 +53,8 @@
#defineLOGIN_SETUMASK 0x0020 /* Set umask */
#defineLOGIN_SETUSER 0x0040 /* Set user */
#defineLOGIN_SETENV0x0080 /* Set environment */
-#defineLOGIN_SETALL0x00ff /* Set all. */
+#defineLOGIN_SETRTABLE 0x0100 /* Set rtable */
+#defineLOGIN_SETALL0x01ff /* Set all. */
#defineBI_AUTH "authorize" /* Accepted
authentication */
#defineBI_REJECT "reject"/* Rejected
authentication */
diff --git lib/libc/gen/login_cap.c lib/libc/gen/login_cap.c
index 862f33b2065..65848f09ef6 100644
--- lib/libc/gen/login_cap.c
+++ lib/libc/gen/login_cap.c
@@ -52,6 +52,7 @@
#include
#include
#include
+#include
#include
#include
@@ -574,7 +575,7 @@ int
setusercontext(login_cap_t *lc, struct passwd *pwd, uid_t uid, u_int flags)
{
login_cap_t *flc;
- quad_t p;
+ quad_t p, rtable;
int i;
flc = NULL;
@@ -625,6 +626,14 @@ setusercontext(login_cap_t *lc, struct passwd *pwd, uid_t
uid, u_int flags)
umask((mode_t)p);
}
+ if (flags & LOGIN_SETRTABLE) {
+ rtable = login_getcapnum(lc, "rtable", 0, 0);
+
+ if (setrtable((int)rtable) == -1) {
+ syslog(LOG_ERR, "%s: setrtable: %m", lc->lc_class);
+ }
+ }
+
if (flags & LOGIN_SETGROUP) {
if (setresgid(pwd->pw_gid, pwd->pw_gid, pwd->pw_gid) == -1) {
syslog(LOG_ERR, "setresgid(%u,%u,%u): %m",
diff --git lib/libc/sys/pledge.2 lib/libc/sys/pledge.2
index 581d274822c..ff3aa59f9e3 100644
--- lib/libc/sys/pledge.2
+++ lib/libc/sys/pledge.2
@@ -514,7 +514,8 @@ process:
.Xr setlogin 2 ,
.Xr setrlimit 2 ,
.Xr getpriority 2 ,
-.Xr setpriority 2
+.Xr setpriority 2 ,
+.Xr setrtable 2
.It Cm pf
Allows a subset of
.Xr ioctl 2
diff --git share/man/man5/login.conf.5 share/man/man5/login.conf.5
index da935fa223e..ffeafc3531c 100644
--- share/man/man5/login.conf.5
+++ share/man/man5/login.conf.5
@@ -276,6 +276,10 @@ Initial priority (nice) level.
Require home directory to login.
.\"
.Pp
+.It rtable Ta number Ta Dv 0 Ta
+Rtable to be set for the class.
+.\"
+.Pp
.It setenv Ta envlist Ta "" Ta
A list of environment variables and associated values to be set for the class.
.\"
diff --git sys/kern/kern_pledge.c sys/kern/kern_pledge.c
index b876b91a18a..26b5e1fab96 100644
--- sys/kern/kern_pledge.c
+++ sys/kern/kern_pledge.c
@@ -300,6 +300,7 @@ const uint64_t pledge_syscalls[SYS_MAXSYSCALL] = {
[SYS_setresgid] = PLEDGE_ID,
[SYS_setgroups] = PLEDGE_ID,
[SYS_setlogin] = PLEDGE_ID,
+ [SYS_setrtable] = PLEDGE_ID,
[SYS_unveil] = PLEDGE_UNVEIL,
Re: Add rtable capability to login.conf
On Sat, Jan 29, 2022 at 07:10:00PM -0500, Ted Unangst wrote:
> I believe it would be better to add setrtable to id pledge.
Makes the diff quite a bit shorter too.
diff --git include/login_cap.h include/login_cap.h
index d9a4c2c349c..1e831b6471a 100644
--- include/login_cap.h
+++ include/login_cap.h
@@ -53,7 +53,8 @@
#defineLOGIN_SETUMASK 0x0020 /* Set umask */
#defineLOGIN_SETUSER 0x0040 /* Set user */
#defineLOGIN_SETENV0x0080 /* Set environment */
-#defineLOGIN_SETALL0x00ff /* Set all. */
+#defineLOGIN_SETRTABLE 0x0100 /* Set rtable */
+#defineLOGIN_SETALL0x01ff /* Set all. */
#defineBI_AUTH "authorize" /* Accepted
authentication */
#defineBI_REJECT "reject"/* Rejected
authentication */
diff --git lib/libc/gen/login_cap.c lib/libc/gen/login_cap.c
index 862f33b2065..65848f09ef6 100644
--- lib/libc/gen/login_cap.c
+++ lib/libc/gen/login_cap.c
@@ -52,6 +52,7 @@
#include
#include
#include
+#include
#include
#include
@@ -574,7 +575,7 @@ int
setusercontext(login_cap_t *lc, struct passwd *pwd, uid_t uid, u_int flags)
{
login_cap_t *flc;
- quad_t p;
+ quad_t p, rtable;
int i;
flc = NULL;
@@ -625,6 +626,14 @@ setusercontext(login_cap_t *lc, struct passwd *pwd, uid_t
uid, u_int flags)
umask((mode_t)p);
}
+ if (flags & LOGIN_SETRTABLE) {
+ rtable = login_getcapnum(lc, "rtable", 0, 0);
+
+ if (setrtable((int)rtable) == -1) {
+ syslog(LOG_ERR, "%s: setrtable: %m", lc->lc_class);
+ }
+ }
+
if (flags & LOGIN_SETGROUP) {
if (setresgid(pwd->pw_gid, pwd->pw_gid, pwd->pw_gid) == -1) {
syslog(LOG_ERR, "setresgid(%u,%u,%u): %m",
diff --git lib/libc/sys/pledge.2 lib/libc/sys/pledge.2
index 581d274822c..ff3aa59f9e3 100644
--- lib/libc/sys/pledge.2
+++ lib/libc/sys/pledge.2
@@ -514,7 +514,8 @@ process:
.Xr setlogin 2 ,
.Xr setrlimit 2 ,
.Xr getpriority 2 ,
-.Xr setpriority 2
+.Xr setpriority 2 ,
+.Xr setrtable 2
.It Cm pf
Allows a subset of
.Xr ioctl 2
diff --git share/man/man5/login.conf.5 share/man/man5/login.conf.5
index da935fa223e..ffeafc3531c 100644
--- share/man/man5/login.conf.5
+++ share/man/man5/login.conf.5
@@ -276,6 +276,10 @@ Initial priority (nice) level.
Require home directory to login.
.\"
.Pp
+.It rtable Ta number Ta Dv 0 Ta
+Rtable to be set for the class.
+.\"
+.Pp
.It setenv Ta envlist Ta "" Ta
A list of environment variables and associated values to be set for the class.
.\"
diff --git sys/kern/kern_pledge.c sys/kern/kern_pledge.c
index 6687bf91f09..ead40b2df6f 100644
--- sys/kern/kern_pledge.c
+++ sys/kern/kern_pledge.c
@@ -300,6 +300,7 @@ const uint64_t pledge_syscalls[SYS_MAXSYSCALL] = {
[SYS_setresgid] = PLEDGE_ID,
[SYS_setgroups] = PLEDGE_ID,
[SYS_setlogin] = PLEDGE_ID,
+ [SYS_setrtable] = PLEDGE_ID,
[SYS_unveil] = PLEDGE_UNVEIL,
Re: Add rtable capability to login.conf
> I believe it would be better to add setrtable to id pledge. That's right.
Re: Add rtable capability to login.conf
I believe it would be better to add setrtable to id pledge.
On 2022-01-29, Matthew Martin wrote:
> It would be nice to have the ability to set a user's rtable upon login.
> This would be useful both for road warrior VPN setups (put both the VPN
> interface and user in an rdomain other than 0) and to differentiate
> users in firewall rules on the gateway or unbound views on a resolver.
> The below patch adds an rtable capability to login.conf which sets the
> user's default rtable on login.
>
> Since setusercontext will now call setrtable, that syscall needs to be
> covered under some pledge promise. Since the wroute promise already
> allows programs to set a different rtable on a per socket basis, I don't
> think adding the setrtable syscall materially changes the scope or
> attack surface. This impacts dhcpleased, iked, rad, and slaacd which
> already use the wroute promise. The wroute promise is added to
> calendar, skeyaudit, su, cron, and inetd.
>
> chroot, login, ssh, nsd, and unbound are impacted, but don't show up in
> the diff because they don't use pledge. ssh continues to work as
> expected: the user's session has the login specified default rtable;
> however, the socket connecting the user to the session remains in the
> original rtable so the connection doesn't break. nsd and unbound open
> their listening sockets prior to privdrop, so those sockets are in the
> original rtable (which should be set via rcctl). Notifications and XFRs
> for nsd and lookups for unbound happen in the login specified rtable.
> While this is somewhat surprising, I think most people would set the
> rtable for a daemon via rcctl.
>
> doas is a bit of a special case: it explicitly lists the LOGIN_ flags
> rather than using LOGIN_SETALL and removing specific flags. I am
> inclined to say doas should also set the default rtable, but it's not
> a strong stance. I've left it out of this version of the patch pending
> feedback on how others feel.
>
> In addition xenodm would need to add wroute to the pledge in dm.c
> StartDisplay and to the first pledge in session.c StartClient.
>
> From a Debian code search the ports which use LOGIN_SETALL appear to be
> mail/alpine, security/sudo, and x11/gnome/gdm. None of these ports have
> pledge markers in their Makefile.
>
> Finally it may be nicer to drop -a from calendar and skeyaudit. Each
> user that desires the service could then create their own cron job.
> While it would allow a fair bit of code to be deleted and a tighter
> pledge, it would break existing workflows and could be considered as
> in a separate thread if desired.
>
> - Matthew Martin
>
>
>
> diff --git include/login_cap.h include/login_cap.h
> index d9a4c2c349c..1e831b6471a 100644
> --- include/login_cap.h
> +++ include/login_cap.h
> @@ -53,7 +53,8 @@
> #define LOGIN_SETUMASK 0x0020 /* Set umask */
> #define LOGIN_SETUSER 0x0040 /* Set user */
> #define LOGIN_SETENV0x0080 /* Set environment */
> -#define LOGIN_SETALL0x00ff /* Set all. */
> +#define LOGIN_SETRTABLE 0x0100 /* Set rtable */
> +#define LOGIN_SETALL0x01ff /* Set all. */
>
> #define BI_AUTH "authorize" /* Accepted
> authentication */
> #define BI_REJECT "reject"/* Rejected
> authentication */
> diff --git lib/libc/gen/login_cap.c lib/libc/gen/login_cap.c
> index 862f33b2065..65848f09ef6 100644
> --- lib/libc/gen/login_cap.c
> +++ lib/libc/gen/login_cap.c
> @@ -52,6 +52,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -574,7 +575,7 @@ int
> setusercontext(login_cap_t *lc, struct passwd *pwd, uid_t uid, u_int flags)
> {
> login_cap_t *flc;
> - quad_t p;
> + quad_t p, rtable;
> int i;
>
> flc = NULL;
> @@ -625,6 +626,14 @@ setusercontext(login_cap_t *lc, struct passwd *pwd,
> uid_t uid, u_int flags)
> umask((mode_t)p);
> }
>
> + if (flags & LOGIN_SETRTABLE) {
> + rtable = login_getcapnum(lc, "rtable", 0, 0);
> +
> + if (setrtable((int)rtable) == -1) {
> + syslog(LOG_ERR, "%s: setrtable: %m", lc->lc_class);
> + }
> + }
> +
> if (flags & LOGIN_SETGROUP) {
> if (setresgid(pwd->pw_gid, pwd->pw_gid, pwd->pw_gid) == -1) {
> syslog(LOG_ERR, "setresgid(%u,%u,%u): %m",
> diff --git share/man/man5/login.conf.5 share/man/man5/login.conf.5
> index da935fa223e..ffeafc3531c 100644
> --- share/man/man5/login.conf.5
> +++ share/man/man5/login.conf.5
> @@ -276,6 +276,10 @@ Initial priority (nice) level.
> Require home directory to login.
> .\"
> .Pp
> +.It rtable Ta number Ta Dv 0 Ta
> +Rtable to be set for the class.
> +.\"
> +.Pp
> .It setenv Ta envlist Ta "" Ta
> A list of environment variables and associated values to be set for the
> class.
> .\"
> diff --git sys/kern/kern_pledge.c
Add rtable capability to login.conf
It would be nice to have the ability to set a user's rtable upon login.
This would be useful both for road warrior VPN setups (put both the VPN
interface and user in an rdomain other than 0) and to differentiate
users in firewall rules on the gateway or unbound views on a resolver.
The below patch adds an rtable capability to login.conf which sets the
user's default rtable on login.
Since setusercontext will now call setrtable, that syscall needs to be
covered under some pledge promise. Since the wroute promise already
allows programs to set a different rtable on a per socket basis, I don't
think adding the setrtable syscall materially changes the scope or
attack surface. This impacts dhcpleased, iked, rad, and slaacd which
already use the wroute promise. The wroute promise is added to
calendar, skeyaudit, su, cron, and inetd.
chroot, login, ssh, nsd, and unbound are impacted, but don't show up in
the diff because they don't use pledge. ssh continues to work as
expected: the user's session has the login specified default rtable;
however, the socket connecting the user to the session remains in the
original rtable so the connection doesn't break. nsd and unbound open
their listening sockets prior to privdrop, so those sockets are in the
original rtable (which should be set via rcctl). Notifications and XFRs
for nsd and lookups for unbound happen in the login specified rtable.
While this is somewhat surprising, I think most people would set the
rtable for a daemon via rcctl.
doas is a bit of a special case: it explicitly lists the LOGIN_ flags
rather than using LOGIN_SETALL and removing specific flags. I am
inclined to say doas should also set the default rtable, but it's not
a strong stance. I've left it out of this version of the patch pending
feedback on how others feel.
In addition xenodm would need to add wroute to the pledge in dm.c
StartDisplay and to the first pledge in session.c StartClient.
>From a Debian code search the ports which use LOGIN_SETALL appear to be
mail/alpine, security/sudo, and x11/gnome/gdm. None of these ports have
pledge markers in their Makefile.
Finally it may be nicer to drop -a from calendar and skeyaudit. Each
user that desires the service could then create their own cron job.
While it would allow a fair bit of code to be deleted and a tighter
pledge, it would break existing workflows and could be considered as
in a separate thread if desired.
- Matthew Martin
diff --git include/login_cap.h include/login_cap.h
index d9a4c2c349c..1e831b6471a 100644
--- include/login_cap.h
+++ include/login_cap.h
@@ -53,7 +53,8 @@
#defineLOGIN_SETUMASK 0x0020 /* Set umask */
#defineLOGIN_SETUSER 0x0040 /* Set user */
#defineLOGIN_SETENV0x0080 /* Set environment */
-#defineLOGIN_SETALL0x00ff /* Set all. */
+#defineLOGIN_SETRTABLE 0x0100 /* Set rtable */
+#defineLOGIN_SETALL0x01ff /* Set all. */
#defineBI_AUTH "authorize" /* Accepted
authentication */
#defineBI_REJECT "reject"/* Rejected
authentication */
diff --git lib/libc/gen/login_cap.c lib/libc/gen/login_cap.c
index 862f33b2065..65848f09ef6 100644
--- lib/libc/gen/login_cap.c
+++ lib/libc/gen/login_cap.c
@@ -52,6 +52,7 @@
#include
#include
#include
+#include
#include
#include
@@ -574,7 +575,7 @@ int
setusercontext(login_cap_t *lc, struct passwd *pwd, uid_t uid, u_int flags)
{
login_cap_t *flc;
- quad_t p;
+ quad_t p, rtable;
int i;
flc = NULL;
@@ -625,6 +626,14 @@ setusercontext(login_cap_t *lc, struct passwd *pwd, uid_t
uid, u_int flags)
umask((mode_t)p);
}
+ if (flags & LOGIN_SETRTABLE) {
+ rtable = login_getcapnum(lc, "rtable", 0, 0);
+
+ if (setrtable((int)rtable) == -1) {
+ syslog(LOG_ERR, "%s: setrtable: %m", lc->lc_class);
+ }
+ }
+
if (flags & LOGIN_SETGROUP) {
if (setresgid(pwd->pw_gid, pwd->pw_gid, pwd->pw_gid) == -1) {
syslog(LOG_ERR, "setresgid(%u,%u,%u): %m",
diff --git share/man/man5/login.conf.5 share/man/man5/login.conf.5
index da935fa223e..ffeafc3531c 100644
--- share/man/man5/login.conf.5
+++ share/man/man5/login.conf.5
@@ -276,6 +276,10 @@ Initial priority (nice) level.
Require home directory to login.
.\"
.Pp
+.It rtable Ta number Ta Dv 0 Ta
+Rtable to be set for the class.
+.\"
+.Pp
.It setenv Ta envlist Ta "" Ta
A list of environment variables and associated values to be set for the class.
.\"
diff --git sys/kern/kern_pledge.c sys/kern/kern_pledge.c
index 6687bf91f09..901d41cefb6 100644
--- sys/kern/kern_pledge.c
+++ sys/kern/kern_pledge.c
@@ -373,6 +373,8 @@ const uint64_t pledge_syscalls[SYS_MAXSYSCALL] = {
[SYS_flock] = PLEDGE_FLOCK | PLEDGE_YPACTIVE,
[SYS_swapctl] = PLEDGE_VMINFO, /* XXX should
