Hi
I think having two different main()s is silly. Why not keep the privsep
and everything else but just skip the systrace bits?
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);
> +}
>
