On Thu, Jun 04, 2015 at 10:20:35PM +0100, Nicholas Marriott wrote:
> Hi
>
> I think having two different main()s is silly. Why not keep the privsep
> and everything else but just skip the systrace bits?
Well, part of the problem is to avoid the imsg interface
between parent and child. Theo's suggestion was to solve
this issue with systrace(1)-ed file(1) invocation.
Or did I miss something?
--patrick
>
> On Thu, Jun 04, 2015 at 02:09:43PM -0700, patrick keshishian wrote:
> > On Thu, Jun 04, 2015 at 12:46:14PM -0600, Theo de Raadt wrote:
> > > > Trying to get file to work under systrace(1).
> > > >
> > > > Is this a reasonable patch?
> > >
> > > Semi-reasonable.
> > >
> > > I think if the ioctl fails, it should be entirely silent.
> > >
> > > > file(1) still fails with systrace because of sendmsg(2), any
> > > > help with correcting systrace.policy for this case would be
> > > > appreciated.
> > >
> > > Perhaps this situation can be tested earlier, using an ioctl, and then
> > > the privsep model can be skipped. Trusting in the non-fragility of
> > > the code, and that it is already operating with some containment.
> >
> > Nice suggestion. I borrowed from top(1); reason it is in
> > separate file is to avoid Coypright clutter.
> >
> > Hope this is OK, or at least close to being acceptable.
> > The port build seems to be moving along now.
> >
> > Also cursory tests (e.g., file -, file *.c, file -c) seem to be
> > doing what they are supposed to do.
> >
> > --patrick
> >
> >
> > Index: Makefile
> > ===================================================================
> > RCS file: /cvs/obsd/src/usr.bin/file/Makefile,v
> > retrieving revision 1.15
> > diff -u -p -u -p -r1.15 Makefile
> > --- Makefile 27 Apr 2015 13:52:17 -0000 1.15
> > +++ Makefile 4 Jun 2015 21:01:40 -0000
> > @@ -1,8 +1,8 @@
> > # $OpenBSD: Makefile,v 1.14 2015/04/27 13:41:45 nicm Exp $
> >
> > PROG= file
> > -SRCS= file.c magic-dump.c magic-load.c magic-test.c magic-common.c
> > sandbox.c \
> > - text.c xmalloc.c
> > +SRCS= file.c magic-dump.c magic-load.c magic-test.c magic-common.c \
> > + proc.c sandbox.c text.c xmalloc.c
> > MAN= file.1 magic.5
> >
> > LDADD= -lutil
> > Index: file.c
> > ===================================================================
> > RCS file: /cvs/obsd/src/usr.bin/file/file.c,v
> > retrieving revision 1.45
> > diff -u -p -u -p -r1.45 file.c
> > --- file.c 30 May 2015 06:25:35 -0000 1.45
> > +++ file.c 4 Jun 2015 21:01:40 -0000
> > @@ -79,6 +79,7 @@ static int read_message(struct imsgbuf
> > static void read_link(struct input_msg *, const char *);
> >
> > static __dead void child(int, pid_t, int, char **);
> > +static __dead void solo(int, char **);
> >
> > static void test_file(struct input_file *, size_t);
> >
> > @@ -188,6 +189,9 @@ main(int argc, char **argv)
> > if (magicfp == NULL)
> > err(1, "%s", magicpath);
> >
> > + if (being_traced())
> > + solo(argc, argv);
> > +
> > parent = getpid();
> > if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pair) != 0)
> > err(1, "socketpair");
> > @@ -637,4 +641,60 @@ test_file(struct input_file *inf, size_t
> >
> > if (inf->mapped && inf->base != NULL)
> > munmap(inf->base, inf->size);
> > +}
> > +
> > +static __dead void
> > +solo(int argc, char *argv[])
> > +{
> > + struct stat sb;
> > + struct input_msg msg;
> > + struct input_file inf;
> > + struct magic *m;
> > + int fd, idx;
> > + size_t len, width = 0;
> > +
> > + m = magic_load(magicfp, magicpath, cflag || Wflag);
> > + if (cflag) {
> > + magic_dump(m);
> > + exit(0);
> > + }
> > +
> > + for (idx = 0; idx < argc; idx++) {
> > + len = strlen(argv[idx]) + 1;
> > + if (len > width)
> > + width = len;
> > + }
> > +
> > + for (idx = 0; idx < argc; idx++) {
> > + memset(&msg, 0, sizeof msg);
> > + msg.idx = idx;
> > +
> > + if (strcmp(argv[idx], "-") == 0) {
> > + if (fstat(STDIN_FILENO, &sb) == -1) {
> > + fd = -1;
> > + msg.error = errno;
> > + } else
> > + fd = STDIN_FILENO;
> > + } else if (lstat(argv[idx], &sb) == -1) {
> > + fd = -1;
> > + msg.error = errno;
> > + } else {
> > + fd = open(argv[idx], O_RDONLY|O_NONBLOCK);
> > + if (fd == -1 && (errno == ENFILE || errno == EMFILE))
> > + err(1, "open");
> > + if (S_ISLNK(sb.st_mode))
> > + read_link(&msg, argv[idx]);
> > + }
> > +
> > + memset(&inf, 0, sizeof inf);
> > + inf.m = m;
> > + inf.msg = &msg;
> > + inf.path = argv[idx];
> > + inf.fd = fd;
> > +
> > + test_file(&inf, width);
> > + if (STDIN_FILENO != fd)
> > + close(fd);
> > + }
> > + exit(0);
> > }
> > Index: file.h
> > ===================================================================
> > RCS file: /cvs/obsd/src/usr.bin/file/file.h,v
> > retrieving revision 1.29
> > diff -u -p -u -p -r1.29 file.h
> > --- file.h 27 Apr 2015 13:52:17 -0000 1.29
> > +++ file.h 4 Jun 2015 21:01:40 -0000
> > @@ -32,4 +32,7 @@ int sandbox_fork(const char *);
> > const char *text_get_type(const void *, size_t);
> > const char *text_try_words(const void *, size_t, int);
> >
> > +/* proc.c */
> > +int being_traced(void);
> > +
> > #endif /* FILE_H */
> > Index: proc.c
> > ===================================================================
> > RCS file: proc.c
> > diff -N proc.c
> > --- /dev/null 1 Jan 1970 00:00:00 -0000
> > +++ proc.c 4 Jun 2015 21:01:40 -0000
> > @@ -0,0 +1,64 @@
> > +/* $OpenBSD:$ */
> > +
> > +/*-
> > + * Copyright (c) 1994 Thorsten Lockert <th...@sigmasoft.com>
> > + * All rights reserved.
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions
> > + * are met:
> > + * 1. Redistributions of source code must retain the above copyright
> > + * notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above copyright
> > + * notice, this list of conditions and the following disclaimer in the
> > + * documentation and/or other materials provided with the distribution.
> > + * 3. The name of the author may not be used to endorse or promote products
> > + * derived from this software without specific prior written permission.
> > + *
> > + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> > WARRANTIES,
> > + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
> > + * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
> > + * THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
> > + * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
> > + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
> > PROFITS;
> > + * OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
> > + * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
> > + * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
> > + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> > + *
> > + * AUTHOR: Thorsten Lockert <th...@sigmasoft.com>
> > + * Adapted from BSD4.4 by Christos Zoulas
> > <chris...@ee.cornell.edu>
> > + * Patch for process wait display by Jarl F. Greipsland
> > <ja...@idt.unit.no>
> > + * Patch for -DORDER by Kenneth Stailey <kstai...@disclosure.com>
> > + * Patch for new swapctl(2) by Tobias Weingartner
> > <weing...@openbsd.org>
> > + */
> > +
> > +#include <sys/param.h> /* DEV_BSIZE MAXCOMLEN PZERO */
> > +#include <sys/types.h>
> > +#include <sys/signal.h>
> > +#include <sys/mount.h>
> > +#include <sys/proc.h>
> > +#include <sys/sched.h>
> > +#include <sys/swap.h>
> > +#include <sys/sysctl.h>
> > +
> > +#include <err.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +
> > +int
> > +being_traced(void)
> > +{
> > + size_t size;
> > + int mib[6] = {CTL_KERN, KERN_PROC, KERN_PROC_PID, 0,
> > + sizeof(struct kinfo_proc), 1};
> > + static struct kinfo_proc pbase;
> > +
> > + mib[3] = getpid();
> > + size = sizeof(pbase);
> > +
> > + if (-1 == sysctl(mib, 6, &pbase, &size, NULL, 0))
> > + err(1, "%s: sysctl()", __func__);
> > +
> > + return (pbase.p_flag & P_SYSTRACE);
> > +}
> >
>
