> > > + main(int argc, char *argv[])
> > > + {
> > > ++        if (pledge("stdio rpath unix dns proc exec prot_exec", NULL) == 
> > > -1)
> > > ++                die("pledge\n");
> > 
> > If you look in base, we only ever pledge after setlocale(3) at the
> > earliest. But in this case, after setlocale(3) would still be too early
> > in my opinion.
> What's the reason for picking setlocale(3) in general?

I believe there are two reasons:

First, before Ingo's rewrite last year, the code had serious tentacles
no-one had ever audited wrt pledge, so it was impossible to deal with it
correctly. In the early days, a number of issues arose from non-standard
locale settings, so the decision was to take setlocale out of pledge's
scope.

The second concern, I imagine, is portability. If other systems should
ever decide to implement pledge, it is likely that their setlocale
implementation has similar issues as our old implementation.

> > Also, I would expect this pledge set to break starting a remote X
> > session with dwm.
> You are right, missing at least "inet"...

Right.

> > > +         if (argc == 2 && !strcmp("-v", argv[1]))
> > > +                 die("dwm-"VERSION "\n");
> > > +         else if (argc != 1)
> > > +@@ -2132,7 +2134,11 @@ main(int argc, char *argv[])
> > > +         if (!(dpy = XOpenDisplay(NULL)))
> > > +                 die("dwm: cannot open display\n");
> > > +         checkotherwm();
> > > ++        if (pledge("stdio rpath proc exec prot_exec", NULL) == -1)
> > > ++                die("pledge\n");
> When testing this with Xnest(1)/Xephyr(1), I was sloppy and didn't
> actually use TCP. Without this, remote X works as expected:
> 
>       $ Xnest -listen tcp :1 &
>       [1] 89077
>       $ DISPLAY=[::1]:1 dwm &
>       [2] 65131
>       $ xterm -display [::1]:1
> 
> > Apparently, here it's still too early, dwm is still not entirely set up.
> > 
> > What in setup() requires prot_exec? Why? Have you audited the X* and
> > fontconfig libraries to make sure that no code path reachable from here
> > (e.g. via config files, environment, what have you) can commit a pledge
> > violation?
> `prot_exec' is needed until the /* init appearance */ begins, but no,
> not all possible codepaths have been audited.
> 
> > > +         setup();
> > > ++        if (pledge("stdio proc exec", NULL) == -1)
> > > ++                die("pledge\n");
> > 
> > This is the pledge that really protects you long term, and that one is
> > fine.
> Thanks a lot for your feedback! Updated diff below with this call only.

Ok cool, the pledge and taking over MAINTAINER parts are ok tb

I'll leave the rest of the ports changes to jca and/or sthen's scrutiny.
They do look good to me, though.

> 
> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/x11/dwm/Makefile,v
> retrieving revision 1.28
> diff -u -p -r1.28 Makefile
> --- Makefile  24 Oct 2016 22:46:54 -0000      1.28
> +++ Makefile  20 May 2018 12:20:32 -0000
> @@ -2,28 +2,36 @@
>  
>  COMMENT=             dynamic window manager
>  
> -DISTNAME=            dwm-6.1
> -REVISION=            0
> +V=                   6.1
> +DISTNAME=            dwm-${V}
> +REVISION=            1
>  
>  CATEGORIES=          x11
>  
> -HOMEPAGE=            http://dwm.suckless.org/
> +HOMEPAGE=            https://dwm.suckless.org/
>  
> -MAINTAINER=          Jim Razmus II <j...@openbsd.org>
> +MAINTAINER=          Klemens Nanni <k...@openbsd.org>
>  
>  # MIT/X
>  PERMIT_PACKAGE_CDROM=        Yes
>  
> -WANTLIB=             X11 Xinerama c Xft fontconfig
> +# uses pledge()
> +WANTLIB=             X11 Xinerama Xft c fontconfig
>  
> -MASTER_SITES=                http://dl.suckless.org/dwm/
> +MASTER_SITES=                https://dl.suckless.org/dwm/
>  
>  RUN_DEPENDS=         x11/dmenu>=4.6 \
>                       fonts/terminus-font
>  
> -MAKE_ENV=            LDFLAGS="${LDFLAGS}"
> -FAKE_FLAGS=          DESTDIR=""
> +MAKE_ENV=            LDFLAGS="${LDFLAGS}" \
> +                     X11INC=${X11BASE}/include \
> +                     X11LIB=${X11BASE}/lib
>  
>  NO_TEST=             Yes
> +
> +do-install:
> +     ${INSTALL_PROGRAM} ${WRKBUILD}/dwm ${PREFIX}/bin/
> +     ${INSTALL_MAN} ${WRKSRC}/dwm.1 ${PREFIX}/man/man1/
> +     sed -i s/VERSION/${V}/g ${PREFIX}/man/man1/dwm.1
>  
>  .include <bsd.port.mk>
> Index: patches/patch-Makefile
> ===================================================================
> RCS file: patches/patch-Makefile
> diff -N patches/patch-Makefile
> --- patches/patch-Makefile    16 Jul 2009 15:12:39 -0000      1.4
> +++ /dev/null 1 Jan 1970 00:00:00 -0000
> @@ -1,44 +0,0 @@
> -$OpenBSD: patch-Makefile,v 1.4 2009/07/16 15:12:39 jim Exp $
> ---- Makefile.orig    Thu Apr  3 22:57:01 2008
> -+++ Makefile Wed Apr  9 21:49:02 2008
> -@@ -15,8 +15,7 @@ options:
> -     @echo "CC       = ${CC}"
> - 
> - .c.o:
> --    @echo CC $<
> --    @${CC} -c ${CFLAGS} $<
> -+    ${CC} -c ${CFLAGS} $<
> - 
> - ${OBJ}: config.h config.mk
> - 
> -@@ -25,8 +24,7 @@ config.h:
> -     @cp config.def.h $@
> - 
> - dwm: ${OBJ}
> --    @echo CC -o $@
> --    @${CC} -o $@ ${OBJ} ${LDFLAGS}
> -+    ${CC} -o $@ ${OBJ} ${LDFLAGS}
> - 
> - clean:
> -     @echo cleaning
> -@@ -42,14 +40,12 @@ dist: clean
> -     @rm -rf dwm-${VERSION}
> - 
> - install: all
> --    @echo installing executable file to ${DESTDIR}${PREFIX}/bin
> --    @mkdir -p ${DESTDIR}${PREFIX}/bin
> --    @cp -f dwm ${DESTDIR}${PREFIX}/bin
> --    @chmod 755 ${DESTDIR}${PREFIX}/bin/dwm
> --    @echo installing manual page to ${DESTDIR}${MANPREFIX}/man1
> --    @mkdir -p ${DESTDIR}${MANPREFIX}/man1
> --    @sed "s/VERSION/${VERSION}/g" < dwm.1 > 
> ${DESTDIR}${MANPREFIX}/man1/dwm.1
> --    @chmod 644 ${DESTDIR}${MANPREFIX}/man1/dwm.1
> -+    ${BSD_INSTALL_PROGRAM_DIR} ${DESTDIR}${PREFIX}/bin
> -+    ${BSD_INSTALL_PROGRAM} dwm ${DESTDIR}${PREFIX}/bin
> -+    ${BSD_INSTALL_MAN_DIR} ${DESTDIR}${MANPREFIX}/man1
> -+    sed "s/VERSION/${VERSION}/g" < dwm.1 > dwm.1.tmp
> -+    mv dwm.1.tmp dwm.1
> -+    ${BSD_INSTALL_MAN} dwm.1 ${DESTDIR}${MANPREFIX}/man1
> - 
> - uninstall:
> -     @echo removing executable file from ${DESTDIR}${PREFIX}/bin
> Index: patches/patch-config_mk
> ===================================================================
> RCS file: /cvs/ports/x11/dwm/patches/patch-config_mk,v
> retrieving revision 1.10
> diff -u -p -r1.10 patch-config_mk
> --- patches/patch-config_mk   11 Nov 2015 09:27:13 -0000      1.10
> +++ patches/patch-config_mk   20 May 2018 10:57:36 -0000
> @@ -1,32 +1,8 @@
>  $OpenBSD: patch-config_mk,v 1.10 2015/11/11 09:27:13 jung Exp $
> ---- config.mk.orig   Sun Nov  8 23:39:37 2015
> -+++ config.mk        Mon Nov  9 21:14:22 2015
> -@@ -4,11 +4,11 @@ VERSION = 6.1
> - # Customize below to fit your system
> - 
> - # paths
> --PREFIX = /usr/local
> --MANPREFIX = ${PREFIX}/share/man
> -+PREFIX ?= /usr/local
> -+MANPREFIX = ${PREFIX}/man
> - 
> --X11INC = /usr/X11R6/include
> --X11LIB = /usr/X11R6/lib
> -+X11INC = ${X11BASE}/include
> -+X11LIB = ${X11BASE}/lib
> - 
> - # Xinerama, comment if you don't want it
> - XINERAMALIBS  = -lXinerama
> -@@ -16,7 +16,7 @@ XINERAMAFLAGS = -DXINERAMA
> - 
> - # freetype
> - FREETYPELIBS = -lfontconfig -lXft
> --FREETYPEINC = /usr/include/freetype2
> -+#FREETYPEINC = /usr/include/freetype2
> - # OpenBSD (uncomment)
> - FREETYPEINC = ${X11INC}/freetype2
> - 
> -@@ -25,14 +25,14 @@ INCS = -I${X11INC} -I${FREETYPEINC}
> +Index: config.mk
> +--- config.mk.orig
> ++++ config.mk
> +@@ -25,10 +25,10 @@ INCS = -I${X11INC} -I${FREETYPEINC}
>   LIBS = -L${X11LIB} -lX11 ${XINERAMALIBS} ${FREETYPELIBS}
>   
>   # flags
> @@ -35,13 +11,8 @@ $OpenBSD: patch-config_mk,v 1.10 2015/11
>   #CFLAGS   = -g -std=c99 -pedantic -Wall -O0 ${INCS} ${CPPFLAGS}
>  -CFLAGS   = -std=c99 -pedantic -Wall -Wno-deprecated-declarations -Os 
> ${INCS} ${CPPFLAGS}
>  -LDFLAGS  = -s ${LIBS}
> -+CFLAGS   += -std=c99 -pedantic -Wall -Wno-deprecated-declarations -Os 
> ${INCS} ${CPPFLAGS}
> ++CFLAGS   += -std=c99 -pedantic -Wall -Wno-deprecated-declarations ${INCS} 
> ${CPPFLAGS}
>  +LDFLAGS  += -s ${LIBS}
>   
>   # Solaris
>   #CFLAGS = -fast ${INCS} -DVERSION=\"${VERSION}\"
> - #LDFLAGS = ${LIBS}
> - 
> - # compiler and linker
> --CC = cc
> -+CC ?= cc
> Index: patches/patch-dwm_c
> ===================================================================
> RCS file: patches/patch-dwm_c
> diff -N patches/patch-dwm_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-dwm_c       21 May 2018 10:53:10 -0000
> @@ -0,0 +1,14 @@
> +$OpenBSD$
> +
> +Index: dwm.c
> +--- dwm.c.orig
> ++++ dwm.c
> +@@ -2133,6 +2133,8 @@ main(int argc, char *argv[])
> +             die("dwm: cannot open display\n");
> +     checkotherwm();
> +     setup();
> ++    if (pledge("stdio proc exec", NULL) == -1)
> ++            die("pledge\n");
> +     scan();
> +     run();
> +     cleanup();

Reply via email to