On 2018/05/21 15:16, Theo Buehler wrote:
> > > > + 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>
that's alright
> > 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}
it's useful to have compiler command lines in build logs, please leave
these patches removing the @.
rest is OK with me (though I haven't checked pledges).
> > - 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();
>
