On Thu, Jun 04, 2015 at 02:29:50PM -0700, patrick keshishian wrote:
> 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.
Why do you want to avoid the imsg interface?
Is it just to avoid adding sendmsg to the ports systrace policy? Why not
add it - maybe not globally but just for file?
>
> 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 <[email protected]>
> > > + * 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 <[email protected]>
> > > + * Adapted from BSD4.4 by Christos Zoulas
> > > <[email protected]>
> > > + * Patch for process wait display by Jarl F. Greipsland
> > > <[email protected]>
> > > + * Patch for -DORDER by Kenneth Stailey
> > > <[email protected]>
> > > + * Patch for new swapctl(2) by Tobias Weingartner
> > > <[email protected]>
> > > + */
> > > +
> > > +#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);
> > > +}
> > >
> >
