Re: Using tame() in userland

2015-08-30 Thread Martin Natano 
Inline comments below.

 --- tsort.c   29 Jul 2015 10:42:37 -  1.26
 +++ tsort.c   28 Aug 2015 08:03:59 -
 @@ -798,6 +799,43 @@ find_longest_cycle(struct array *h, stru
  
  #define plural(n) ((n)  1 ? s : )
  
 +static void
 +TAME(int flags, const char *wl[])
 +{
 + if (tame(flags, wl) != 0)
 + err(1, untamed program);
 +}
 +
 +static void 
 +compute_whitelist(int argc, char *argv[], char *args[])
 +{
 + int c;
 + int i = 0;
 +
 + while ((c = getopt(argc, argv, h:flqrvw)) != -1) {
 + switch(c) {
 + case 'h':
 + args[i++] = optarg;
This assignment can write past the end of 'args' when the -h option is
given multiple times.

 + /*FALLTHRU*/
 + case 'f':
 + case 'l':
 + case 'q':
 + case 'r':
 + case 'v':
 + case 'w':
 + break;
 + default:
 + usage();
 + }
 + }
 + argc -= optind;
 + argv += optind;
 + if (argc == 1)
 + args[i++] = argv[0];
 + args[i] = NULL;
 + optind = optreset = 1;
 +}
 +
  int
  main(int argc, char *argv[])
  {
 @@ -806,6 +844,10 @@ main(int argc, char *argv[])
   warn_flag, hints_flag, verbose_flag;
   unsigned intorder;
  
 + char *args[3];
How about something like this?

char **args = calloc(argc, sizeof(char *));
if (args == NULL)
err(1, calloc);

In most cases this will probably be a waste of space, but at least it
keeps the code simple. The other option would be to fiddle with
realloc() in compute_whitelist().

 +
 + compute_whitelist(argc, argv, args);
 + TAME(TAME_STDIO|TAME_RPATH, args);
   order = 0;
  
   reverse_flag = quiet_flag = long_flag =

cheers,
natano

Re: Using tame() in userland

2015-08-29 Thread Steven McDonald 
Sorry, my terminal seems to be mangling tabs into spaces. Here's a
properly copypasted diff:

Index: bin/chmod/chmod.c
===
RCS file: /cvs/src/bin/chmod/chmod.c,v
retrieving revision 1.34
diff -u -p -r1.34 chmod.c
--- bin/chmod/chmod.c   25 Jun 2015 02:04:08 -  1.34
+++ bin/chmod/chmod.c   29 Aug 2015 11:33:33 -
@@ -32,6 +32,7 @@
 
 #include sys/types.h
 #include sys/stat.h
+#include sys/tame.h
 
 #include err.h
 #include errno.h
@@ -153,6 +154,8 @@ done:
atflags = 0;
 
if (ischflags) {
+   tame(TAME_STDIO | TAME_RPATH | TAME_FATTR, NULL);
+
flags = *argv;
if (*flags = '0'  *flags = '7') {
errno = 0;
Index: usr.bin/head/head.c
===
RCS file: /cvs/src/usr.bin/head/head.c,v
retrieving revision 1.18
diff -u -p -r1.18 head.c
--- usr.bin/head/head.c 8 Oct 2014 08:31:53 -   1.18
+++ usr.bin/head/head.c 29 Aug 2015 11:33:33 -
@@ -29,6 +29,8 @@
  * SUCH DAMAGE.
  */
 
+#include sys/tame.h
+
 #include stdio.h
 #include stdlib.h
 #include ctype.h
@@ -87,6 +89,7 @@ main(int argc, char *argv[])
if (!firsttime)
exit(status);
fp = stdin;
+   tame(TAME_STDIO, NULL);
} else {
if ((fp = fopen(*argv, r)) == NULL) {
warn(%s, *argv++);

Re: Using tame() in userland

2015-08-29 Thread Steven McDonald 
Hi Theo,

I think chmod fits in the cannot be tamed category. tame(2) says of
chmod(2) and friends:

Setuid/setgid bits do not work, nor can the user or group be
changed on a file.

This breaks 'chmod u+s'. It might be possible to tame only if it looks
like a mode is being set which is allowed, but I think the complexity
would outweight the gain, looking at the way chmod handles modes. I'd be
happy to try writing a diff if you think it's a sound idea, though.

chown is in the same boat as chmod. chflags should be fine, but doesn't
need TAME_WPATH, as far as I can tell.

head requires the ability to call open(2) (via fopen(3)) when given a
filename argument, which is always restricted to specific paths with
tame. It also doesn't seem to actually use the privileges granted by
TAME_FATTR.

Revised diff for chmod and head follows.

Index: bin/chmod/chmod.c
===
RCS file: /cvs/src/bin/chmod/chmod.c,v
retrieving revision 1.34
diff -u -p -r1.34 chmod.c
--- bin/chmod/chmod.c   25 Jun 2015 02:04:08 -  1.34
+++ bin/chmod/chmod.c   29 Aug 2015 09:48:14 -
@@ -32,6 +32,7 @@
 
 #include sys/types.h
 #include sys/stat.h
+#include sys/tame.h
 
 #include err.h
 #include errno.h
@@ -153,6 +154,8 @@ done:
atflags = 0;
 
if (ischflags) {
+   tame(TAME_STDIO | TAME_RPATH | TAME_FATTR, NULL);
+
flags = *argv;
if (*flags = '0'  *flags = '7') {
errno = 0;
Index: usr.bin/head/head.c
===
RCS file: /cvs/src/usr.bin/head/head.c,v
retrieving revision 1.18
diff -u -p -r1.18 head.c
--- usr.bin/head/head.c 8 Oct 2014 08:31:53 -   1.18
+++ usr.bin/head/head.c 29 Aug 2015 09:48:14 -
@@ -29,6 +29,8 @@
  * SUCH DAMAGE.
  */
 
+#include sys/tame.h
+
 #include stdio.h
 #include stdlib.h
 #include ctype.h
@@ -87,6 +89,7 @@ main(int argc, char *argv[])
if (!firsttime)
exit(status);
fp = stdin;
+   tame(TAME_STDIO, NULL);
} else {
if ((fp = fopen(*argv, r)) == NULL) {
warn(%s, *argv++);

Re: Using tame() in userland

2015-08-29 Thread trondd 

On 2015-08-29 06:05, Steven McDonald wrote:


I think chmod fits in the cannot be tamed category. tame(2) says of
chmod(2) and friends:

Setuid/setgid bits do not work, nor can the user or group be
changed on a file.

This breaks 'chmod u+s'.


I ran into this when building Xenocara.

Also upon reboot (after building a tamed userland) mkdir is getting 
killed with syscall 15 right after clearing /tmp


Seems to be in Setup_X_sockets making a directory with a specific mode.

Tim.

Re: Using tame() in userland

2015-08-28 Thread Marc Espie 
   have a much more stringent one for tsort.

Index: tsort.c
===
RCS file: /build/data/openbsd/cvs/src/usr.bin/tsort/tsort.c,v
retrieving revision 1.26
diff -u -p -r1.26 tsort.c
--- tsort.c 29 Jul 2015 10:42:37 -  1.26
+++ tsort.c 28 Aug 2015 08:03:59 -
@@ -28,6 +28,7 @@
 #include sysexits.h
 #include unistd.h
 #include ohash.h
+#include sys/tame.h
 
 /* The complexity of topological sorting is O(e), where e is the
  * size of input.  While reading input, vertices have to be identified,
@@ -798,6 +799,43 @@ find_longest_cycle(struct array *h, stru
 
 #define plural(n) ((n)  1 ? s : )
 
+static void
+TAME(int flags, const char *wl[])
+{
+   if (tame(flags, wl) != 0)
+   err(1, untamed program);
+}
+
+static void 
+compute_whitelist(int argc, char *argv[], char *args[])
+{
+   int c;
+   int i = 0;
+
+   while ((c = getopt(argc, argv, h:flqrvw)) != -1) {
+   switch(c) {
+   case 'h':
+   args[i++] = optarg;
+   /*FALLTHRU*/
+   case 'f':
+   case 'l':
+   case 'q':
+   case 'r':
+   case 'v':
+   case 'w':
+   break;
+   default:
+   usage();
+   }
+   }
+   argc -= optind;
+   argv += optind;
+   if (argc == 1)
+   args[i++] = argv[0];
+   args[i] = NULL;
+   optind = optreset = 1;
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -806,6 +844,10 @@ main(int argc, char *argv[])
warn_flag, hints_flag, verbose_flag;
unsigned intorder;
 
+   char *args[3];
+
+   compute_whitelist(argc, argv, args);
+   TAME(TAME_STDIO|TAME_RPATH, args);
order = 0;
 
reverse_flag = quiet_flag = long_flag =
@@ -863,6 +905,7 @@ main(int argc, char *argv[])
FILE *f;
 
f = fopen(argv[0], r);
+   TAME(TAME_STDIO, NULL);
if (f == NULL)
err(EX_NOINPUT, Can't open file %s, argv[1]);
order = read_pairs(f, pairs, reverse_flag, argv[1], order,
@@ -871,6 +914,7 @@ main(int argc, char *argv[])
break;
}
case 0:
+   TAME(TAME_STDIO, NULL);
order = read_pairs(stdin, pairs, reverse_flag, stdin,
order, hints_flag == 2);
break;
@@ -878,6 +922,7 @@ main(int argc, char *argv[])
usage();
}
 
+   TAME(TAME_RW, NULL);
{
struct arrayaux;/* Unrefed nodes/cycle reporting.  */
struct arrayremaining;

Using tame() in userland

2015-08-27 Thread Theo de Raadt 
This is for those of you interested in tame, and skilled enough to
play along.

This is a set of almost 100 diffs to programs in the tree to use tame.
These have been done by myself, doug, florian, semarie, and a few
other people I forget.  I would make a rough guess these changes took
about 100 hours of developer time; so making programs use tame() is
pretty efficient.

None of these examples uses the path whitelist yet.

It is not perfect or final, but it shows the strategy for applying
them to the base.  It can make it through a 'make build'.  Feel free
to do tests, look for mistakes, or write diffs for other programs.

Be careful writing such diffs; you need to fully understand the
program and handle all cases.  Not all programs can be tamed, some
behaviours (like execve) are not compatible with features tame
can do.

Index: bin/cat/cat.c
===
RCS file: /cvs/src/bin/cat/cat.c,v
retrieving revision 1.21
diff -u -p -u -r1.21 cat.c
--- bin/cat/cat.c   16 Jan 2015 06:39:28 -  1.21
+++ bin/cat/cat.c   26 Aug 2015 22:07:37 -
@@ -35,6 +35,7 @@
 
 #include sys/types.h
 #include sys/stat.h
+#include sys/tame.h
 
 #include ctype.h
 #include err.h
@@ -65,6 +66,8 @@ main(int argc, char *argv[])
int ch;
 
setlocale(LC_ALL, );
+
+   tame(TAME_STDIO | TAME_RPATH, NULL);
 
while ((ch = getopt(argc, argv, benstuv)) != -1)
switch (ch) {
Index: bin/chmod/chmod.c
===
RCS file: /cvs/src/bin/chmod/chmod.c,v
retrieving revision 1.34
diff -u -p -u -r1.34 chmod.c
--- bin/chmod/chmod.c   25 Jun 2015 02:04:08 -  1.34
+++ bin/chmod/chmod.c   26 Aug 2015 22:45:24 -
@@ -32,6 +32,7 @@
 
 #include sys/types.h
 #include sys/stat.h
+#include sys/tame.h
 
 #include err.h
 #include errno.h
@@ -69,6 +70,8 @@ main(int argc, char *argv[])
char *ep, *mode, *cp, *flags;
 
setlocale(LC_ALL, );
+
+   tame(TAME_STDIO | TAME_RPATH | TAME_WPATH | TAME_FATTR, NULL);
 
if (strlen(__progname)  2) {
ischown = __progname[2] == 'o';
Index: bin/dd/dd.c
===
RCS file: /cvs/src/bin/dd/dd.c,v
retrieving revision 1.21
diff -u -p -u -r1.21 dd.c
--- bin/dd/dd.c 16 Jan 2015 06:39:31 -  1.21
+++ bin/dd/dd.c 26 Aug 2015 22:07:37 -
@@ -38,6 +38,7 @@
 #include sys/stat.h
 #include sys/ioctl.h
 #include sys/mtio.h
+#include sys/tame.h
 
 #include ctype.h
 #include err.h
@@ -148,6 +149,11 @@ setup(void)
pos_in();
if (out.offset)
pos_out();
+
+   if (in.fd == STDIN_FILENO  out.fd == STDOUT_FILENO)
+   tame(TAME_STDIO, NULL);
+   else
+   tame(TAME_STDIO | TAME_RW, NULL);
 
/*
 * Truncate the output file; ignore errors because it fails on some
Index: bin/df/df.c
===
RCS file: /cvs/src/bin/df/df.c,v
retrieving revision 1.52
diff -u -p -u -r1.52 df.c
--- bin/df/df.c 16 Jan 2015 06:39:31 -  1.52
+++ bin/df/df.c 27 Aug 2015 03:32:58 -
@@ -37,6 +37,7 @@
 
 #include sys/stat.h
 #include sys/mount.h
+#include sys/tame.h
 
 #include err.h
 #include errno.h
@@ -78,6 +79,9 @@ main(int argc, char *argv[])
int ch, i;
int width, maxwidth;
char *mntpt;
+
+   // but what about getfstat() ?
+   //tame(TAME_STDIO | TAME_RPATH, NULL);
 
while ((ch = getopt(argc, argv, hiklnPt:)) != -1)
switch (ch) {
Index: bin/echo/echo.c
===
RCS file: /cvs/src/bin/echo/echo.c,v
retrieving revision 1.8
diff -u -p -u -r1.8 echo.c
--- bin/echo/echo.c 14 Dec 2014 16:55:59 -  1.8
+++ bin/echo/echo.c 26 Aug 2015 22:07:37 -
@@ -30,6 +30,8 @@
  * SUCH DAMAGE.
  */
 
+#include sys/tame.h
+
 #include stdio.h
 #include string.h
 
@@ -38,6 +40,8 @@ int
 main(int argc, char *argv[])
 {
int nflag;
+
+   tame(TAME_STDIO, NULL);
 
/* This utility may NOT do getopt(3) option parsing. */
if (*++argv  !strcmp(*argv, -n)) {
Index: bin/expr/expr.c
===
RCS file: /cvs/src/bin/expr/expr.c,v
retrieving revision 1.20
diff -u -p -u -r1.20 expr.c
--- bin/expr/expr.c 11 Aug 2015 17:15:46 -  1.20
+++ bin/expr/expr.c 26 Aug 2015 22:07:37 -
@@ -6,6 +6,8 @@
  * Public domain.
  */
 
+#include sys/tame.h
+
 #include stdio.h
 #include stdlib.h
 #include string.h
@@ -499,6 +501,8 @@ main(int argc, char *argv[])
struct val *vp;
 
(void) setlocale(LC_ALL, );
+
+   tame(TAME_STDIO, NULL);
 
if (argc  1  !strcmp(argv[1], --))
argv++;
Index: bin/ls/ls.c
===
RCS file: /cvs/src/bin/ls/ls.c,v
retrieving