[PATCH] make check with Apple's SIP enabled

Started by Jörg Westheidealmost 8 years ago3 messages
#1Jörg Westheide
joerg.westheide@sparkpost.com
1 attachment(s)

Hi!

When doing a "make check" on Mac OS X with SIP (aka rootless mode)
enabled it will fail like this:

----- >8 snip 8< -----
rm -rf ./testtablespace
mkdir ./testtablespace
PATH="/Volumes/Home/arbeit/port25/postgresql-clean/tmp_install/usr/local/pgsql/bin:$PATH"
DYLD_LIBRARY_PATH="/<path_to_postgresql_dir>/tmp_install/usr/local/pgsql/lib"
../../../src/test/regress/pg_regress --temp-instance=./tmp_check
--inputdir=. --bindir= --dlpath=. --max-concurrent-tests=20
--schedule=./parallel_schedule
============== creating temporary instance ==============
============== initializing database system ==============
============== starting postmaster ==============
sh: line 1: 81758 Trace/BPT trap: 5 "psql" -X postgres <
/dev/null 2> /dev/null
sh: line 1: 81762 Trace/BPT trap: 5 "psql" -X postgres <
/dev/null 2> /dev/null
sh: line 1: 81765 Trace/BPT trap: 5 "psql" -X postgres <
/dev/null 2> /dev/null
sh: line 1: 81768 Trace/BPT trap: 5 "psql" -X postgres <
/dev/null 2> /dev/null
sh: line 1: 81771 Trace/BPT trap: 5 "psql" -X postgres <
/dev/null 2> /dev/null
^Cmake[1]: *** [check] Interrupt: 2
make: *** [check] Interrupt: 2
----- >8 snap 8< -----

The problem is that the psql links to libpq which it cannot find (at
least not the one from the postgres you're building). The usual
approach to set an environment variable pointing to the correct
location (DYLD_LIBRARY_PATH on Mac OS X/darwin, see above) does not
work since Apple's SIP prevents it from getting passed to psql (see
https://developer.apple.com/library/content/documentation/Security/Conceptual/System_Integrity_Protection_Guide/RuntimeProtections/RuntimeProtections.html
)

What I do in the attached patch is changing the install name of libpq
in the psql binary of the temp install (used by "make check") to point
to libpq of the temp install so the (correct) lib is found.
For not duplicating the information I created a new file
"Makefile.libdefs" to where I extracted the information needed to do
the install name change

This patch has only been tested on Mac OS X El Capitan (10.11.6) since
I currently have no other OS available

Please let me know what you think of it

Jörg

Attachments:

SIP.v1.patchapplication/octet-stream; name=SIP.v1.patch; x-unix-mode=0644Download
diff --git a/src/bin/psql/Makefile b/src/bin/psql/Makefile
index c8eb2f95cc..b8c62a774f 100644
--- a/src/bin/psql/Makefile
+++ b/src/bin/psql/Makefile
@@ -15,11 +15,15 @@ PGAPPICON=win32
 subdir = src/bin/psql
 top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global
+include $(top_builddir)/src/interfaces/libpq/Makefile.libdefs
 
 REFDOCDIR= $(top_srcdir)/doc/src/sgml/ref
 
 override CPPFLAGS := -I. -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS)
 override LDFLAGS := -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) $(LDFLAGS)
+ifeq ($(PORTNAME),darwin)
+    override LDFLAGS := -headerpad_max_install_names $(LDFLAGS)
+endif
 
 OBJS=	command.o common.o conditional.o copy.o crosstabview.o \
 	describe.o help.o input.o large_obj.o mainloop.o \
@@ -47,6 +51,19 @@ distprep: sql_help.h psqlscanslash.c
 
 install: all installdirs
 	$(INSTALL_PROGRAM) psql$(X) '$(DESTDIR)$(bindir)/psql$(X)'
+    ifndef NO_TEMP_INSTALL
+        ifeq ($(PORTNAME),darwin)
+            # With Apple's SIP (aka rootless mode) enabled one cannot use
+            # DYLD_LIBRARY_PATH to point the temp install's psql to the temp
+            # install's libpq to ensure the matching library is used.
+            # Therefore modify the binary's embedded shared library
+            # install name to point to the temp install's libpq.
+			install_name_tool -change \
+               '$(libdir)/lib$(NAME).$(SO_MAJOR_VERSION)$(DLSUFFIX)' \
+               '$(DESTDIR)$(libdir)/lib$(NAME).$(SO_MAJOR_VERSION)$(DLSUFFIX)' \
+               '$(DESTDIR)$(bindir)/psql$(X)' 
+        endif
+    endif
 	$(INSTALL_DATA) $(srcdir)/psqlrc.sample '$(DESTDIR)$(datadir)/psqlrc.sample'
 
 installdirs:
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 0bf1e7ef04..597a046a39 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -15,9 +15,7 @@ include $(top_builddir)/src/Makefile.global
 
 
 # shared library parameters
-NAME= pq
-SO_MAJOR_VERSION= 5
-SO_MINOR_VERSION= $(MAJORVERSION)
+include ./Makefile.libdefs
 
 override CPPFLAGS :=  -DFRONTEND -DUNSAFE_STAT_OK -I$(srcdir) $(CPPFLAGS) -I$(top_builddir)/src/port -I$(top_srcdir)/src/port
 ifneq ($(PORTNAME), win32)
diff --git a/src/interfaces/libpq/Makefile.libdefs b/src/interfaces/libpq/Makefile.libdefs
new file mode 100644
index 0000000000..0d20387d3e
--- /dev/null
+++ b/src/interfaces/libpq/Makefile.libdefs
@@ -0,0 +1,8 @@
+# shared library parameters
+NAME= pq
+SO_MAJOR_VERSION= 5
+SO_MINOR_VERSION= $(MAJORVERSION)
+ifeq ($(PORTNAME), darwin)
+    DLSUFFIX= .dylib
+endif
+
#2Robert Haas
robertmhaas@gmail.com
In reply to: Jörg Westheide (#1)
Re: [PATCH] make check with Apple's SIP enabled

On Fri, Jan 19, 2018 at 9:54 AM, Jörg Westheide
<joerg.westheide@sparkpost.com> wrote:

When doing a "make check" on Mac OS X with SIP (aka rootless mode) enabled
it will fail like this:

I don't know whether this fix is any good, but thanks for working on
the problem. It's a very annoying problem, and I'd love to see it
solved somehow.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jörg Westheide (#1)
Re: [PATCH] make check with Apple's SIP enabled

=?iso-8859-1?Q?J=F6rg?= Westheide <joerg.westheide@sparkpost.com> writes:

When doing a "make check" on Mac OS X with SIP (aka rootless mode)
enabled it will fail like this:
...

Yeah, well-known problem. AFAIK, all PG developers who use Macs just
disable SIP immediately.

The problem is that the psql links to libpq which it cannot find (at
least not the one from the postgres you're building). The usual
approach to set an environment variable pointing to the correct
location (DYLD_LIBRARY_PATH on Mac OS X/darwin, see above) does not
work since Apple's SIP prevents it from getting passed to psql

Yup. This is incredibly brain-damaged and unnecessary on Apple's
part. Several of us have filed bugs about it, and generally not
gotten much response. Perhaps if people keep complaining, though,
eventually they'll see the light.

What I do in the attached patch is changing the install name of libpq
in the psql binary of the temp install (used by "make check") to point
to libpq of the temp install so the (correct) lib is found.

Cute idea, but AFAICS it would at most fix "make check" and not any
tests that required other executables, or libraries besides libpq.
Plus there's the objection that then you're not really testing the
binary you intend to deploy. I suppose if we got invasive enough,
we could extend this concept to every trouble spot, but I don't
much want to go there.

IIRC, the fact that SIP loses DYLD_LIBRARY_PATH is sort of accidental.
It's not that they've broken the variable altogether, it's that when
we invoke the shell to invoke psql, they clear out DYLD_LIBRARY_PATH
because of some weird decision that the shell is a security-critical
program. (As if the ability to invoke a shell with your choice of input
didn't already give you the chance to do, far more easily, anything you
might do by subverting the shell internally.) So it might be possible to
dodge this problem, at least for "make check" and other pg_regress-driven
cases, by changing pg_regress to avoid going through system(3) but
rather fork-and-exec'ing psql directly.

Nobody's pursued that idea, in part because we've generally found
that SIP breaks enough other stuff that you have to keep it turned
off anyway on development machines. But if you're interested,
you could have a look at how messy that would be.

regards, tom lane