: hard-coded values in Makefiles for down-root and auth-pam plugins prevent cross compilation

Due to the fact that I have spent the last two and a half hours trying to find a place where to submit a bug report via the Trac system (https://community.openvpn.net/openvpn) - and failing, miserably so - I am submitting it here!

In this version of OpenVPN, the Makefiles for the above plugins contain hard-coded compile/link parameters, which prevent cross compilation.

For example, during building openvpn-down-root.so the value of the c/c++ compiler is assumed to be gcc, along with various other settings (CLFAGS being set as "-O2 and -Wall" and LDFLAGS as "-shared -Wl,-soname"), disregarding the rest, therefore ruining any possibility of cross compilation as the file produced will have the same attributes as the build system!

If I build OpenVPN on x86_64-based machine to be executed on i686 the two Makefiles will produce code for x86_64, despite the openvpn executable being produced, correctly, for the i686 host system. Thus, the produced 'package' will introduce at least 2 additional - and highly undesired - dependencies, namely: glibc (x86_64) and libpam (x86_64).

To prevent that I am enclosing a patch, which although fixes this problem I am of the opinion that it could be optimised further to co-exist with the rest of the build system and be auto-generated, though I will leave that with the developers to decide what is the best way to proceed and fix this bug.

diff -NurBb openvpn-2.1.4/plugin/auth-pam/Makefile a/plugin/auth-pam/Makefile
@@ -15,16 +15,14 @@
 # This directory is where we will look for openvpn-plugin.h
 INCLUDE=-I../..
 
-CC_FLAGS=-O2 -Wall -DDLOPEN_PAM=$(DLOPEN_PAM)
-
 openvpn-auth-pam.so : auth-pam.o pamdl.o
-	gcc ${CC_FLAGS} -fPIC -shared -Wl,-soname,openvpn-auth-pam.so -o openvpn-auth-pam.so auth-pam.o pamdl.o -lc $(LIBPAM)
+	${CC} -DDLOPEN_PAM=$(DLOPEN_PAM) -fPIC -shared ${LDFLAGS} -Wl,-soname,openvpn-auth-pam.so -o openvpn-auth-pam.so auth-pam.o pamdl.o -lc $(LIBPAM)
 
 auth-pam.o : auth-pam.c pamdl.h
-	gcc ${CC_FLAGS} -fPIC -c ${INCLUDE} auth-pam.c
+	${CC} ${CFLAGS} -DDLOPEN_PAM=$(DLOPEN_PAM) -fPIC -c ${INCLUDE} auth-pam.c
 
 pamdl.o : pamdl.c pamdl.h
-	gcc ${CC_FLAGS} -fPIC -c ${INCLUDE} pamdl.c
+	${CC} ${CFLAGS} -DDLOPEN_PAM=$(DLOPEN_PAM) -fPIC -c ${INCLUDE} pamdl.c
 
 clean :
 	rm -f *.o *.so

diff -NurBb openvpn-2.1.4/plugin/down-root/Makefile a/plugin/down-root/Makefile
@@ -5,13 +5,11 @@
 # This directory is where we will look for openvpn-plugin.h
 INCLUDE=-I../..
 
-CC_FLAGS=-O2 -Wall
-
 down-root.so : down-root.o
-	gcc ${CC_FLAGS} -fPIC -shared -Wl,-soname,openvpn-down-root.so -o openvpn-down-root.so down-root.o -lc
+	${CC} -fPIC -shared ${LDFLAGS} -Wl,-soname,openvpn-down-root.so -o openvpn-down-root.so down-root.o -lc
 
 down-root.o : down-root.c
-	gcc ${CC_FLAGS} -fPIC -c ${INCLUDE} down-root.c
+	${CC} ${CFLAGS} -fPIC -c ${INCLUDE} down-root.c
 
 clean :
 	rm -f *.o *.so

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 26/12/10 18:32, Mr Dash Four wrote:

| Due to the fact that I have spent the last two and a half hours trying
| to find a place where to submit a bug report via the Trac system
| (https://community.openvpn.net/openvpn) - and failing, miserably so - I
| am submitting it here!
|
| In this version of OpenVPN, the Makefiles for the above plugins contain
| hard-coded compile/link parameters, which prevent cross compilation.
|
| For example, during building openvpn-down-root.so the value of the c/c++
| compiler is assumed to be gcc, along with various other settings (CLFAGS
| being set as "-O2 and -Wall" and LDFLAGS as "-shared -Wl,-soname"),
| disregarding the rest, therefore ruining any possibility of cross
| compilation as the file produced will have the same attributes as the
| build system!
|
| If I build OpenVPN on x86_64-based machine to be executed on i686 the
| two Makefiles will produce code for x86_64, despite the openvpn
| executable being produced, correctly, for the i686 host system. Thus,
| the produced 'package' will introduce at least 2 additional - and highly
| undesired - dependencies, namely: glibc (x86_64) and libpam (x86_64).
|
| To prevent that I am enclosing a patch, which although fixes this
| problem I am of the opinion that it could be optimised further to
| co-exist with the rest of the build system and be auto-generated, though
| I will leave that with the developers to decide what is the best way to
| proceed and fix this bug.

Thank you very much for your patch! This seems reasonable and I am almost giving it an ACK immediately. There are just a one thing I am less convinced about.

Your patch removes -Wall completely. I am really not sure that's a good practice. I see no harm of compiling with -Wall or other warnings enabled by default. In fact, that might help discover issues otherwise not seen. AFAIK,
- -Wall should not cause any cross-compilation issues.

So, I'd prefer that you don't remove CFLAGS completely, but rather append the build platform CFLAGS with -Wall. So you can then do it like this:

~ CLFAGS="$(CFLAGS) -Wall -DDLOPEN_PAM=$(DLOPEN_PAM)"

This will be a smaller and easier patch, which will provide the features you're looking for. Removing -O2 is less of an issue as that one should more be defined by the build platform, and that one can cause more issues with cross-compilation.

Unfortunately this patch is arriving too late to get accepted for the OpenVPN 2.2 release we're putting together nowadays, but we will be able to get an adjusted patch into the development tree for the next version.

kind regards,

David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk0cpyMACgkQDC186MBRfrqYLACcCO51oYXUJ4QsnsMQlwfq9At+ cp8AnAyDZEk2lDjnSLJ02B/yi2f/W1B/
=cVLV
-----END PGP SIGNATURE-----

Am 26.12.2010 18:32, schrieb Mr Dash Four:

I think it would be good to integrate this with automake if the whole setup is to be integrated and these plugins are supposed to be built more or less officially.

I can take a stab at it if desired, drop me a note with a Cc: to my personal address. If desired, I can also look at libtool although I have no relevant experience with that yet, but I do have substantial experience with autoconf/automake.

Best
Matthias

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 30/12/10 17:08, Matthias Andree wrote:
| Am 26.12.2010 18:32, schrieb Mr Dash Four:
[...snip...]

|>
|> To prevent that I am enclosing a patch, which although fixes this
|> problem I am of the opinion that it could be optimised further to
|> co-exist with the rest of the build system and be auto-generated, though
|> I will leave that with the developers to decide what is the best way to
|> proceed and fix this bug.
|
| I think it would be good to integrate this with automake if the whole setup is
| to be integrated and these plugins are supposed to be built more or less

officially.

That's a very good point! And I think this makes even more sense.

| I can take a stab at it if desired, drop me a note with a Cc: to my personal
| address.  If desired, I can also look at libtool although I have no relevant
| experience with that yet, but I do have substantial experience with
| autoconf/automake.

If you have time and are willing to do so, that would be great! I see most Linux distroes installs these plug-ins under /usr/lib{,64}/openvpn/plugin/ ... depending on 32/64 bit arch. So if we could even get some of these plug-ins installed there, that would probably ease the packaging for most distroes.

Currently only auth-pam and down-root are the ones being interesting, IMHO. The defer plug-in is yet another example (and could probably be moved into examples/), and the plug-ins in examples/ are less useful in a production environment as well.

kind regards,

David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk0cvKgACgkQDC186MBRfrobVQCeNe4F7bHmmHilW3bqSAuOoUUk x28AnjhHzSGKFJwptLNp9A8GypyPBEWA
=89bY
-----END PGP SIGNATURE-----

No, it doesn't! "-Wall" is (normally) part of CFLAGS and is included "by default", so there is no need to be specified again here. If anything needs to be added, it has to be compile/link flags specific for this module and/or objects (like DLOPEN_PAM for example) - the rest of the "generic" flags should be and will be specified with CFLAGS/LDFLAGS as any well-designed make system uses them.

As I already pointed out - -Wall is not "completely removed" - it is already part of CFLAGS (the environment variable set for compilation of the entire OpenVPN module). Same goes for -O2. I don't see why they have to be specified again here.

This, though, as I already pointed out, is not ideal. I would like to see this makefile auto generated with autogen so that the entire make process is more streamlined. Mathias's idea in his next post is a good start and this is what I was referring to in my original post. My patch only patches the problem and papers up over the cracks - more appropriate solution needs to be found so that when additional plugins/modules are added we don't face the same problem.

On a side note, could you let me know how to submit a bug with the Trac system - I spent a good couple of hours and still didn't manage to find a way to submit a bug?

Yep, that's what I meant. Better still, autogen can be utilised to do the same thing, but automake is a good start.

Am 30.12.2010 22:04, schrieb Mr Dash Four:

Automake is what's already in place, so it seems diligent to reuse that and not instigate new build fashions to a branch in maintenance mode such as 2.1. :-)

Greetings,

following up the "[OpenVPN 2.1.4 BUG]: hard-coded values in Makefiles for down-root and auth-pam plugins prevent cross compilation" bug, I am providing patches for review and test in cross-compilation environments.

I have tested these on native compiles with Ubuntu Linux 10.10 32-bit, openSUSE 11.3 32-bit, and FreeBSD 8.1 32-bit (all ix86), but the actual plugins are lightly tested (they load and don't crash), and that's about it.

This is the patch series for the bugfix2.1 branch.

I will send a separate patch pair for beta 2.2 soon.

Looking for feedback, review, and ACK or NAK :-)

Best regards
Matthias

---
 configure.ac |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index 4777108..529abd3 100644
@@ -32,6 +32,7 @@ AC_CONFIG_SRCDIR(syshead.h)
 
 dnl Guess host type.
 AC_CANONICAL_HOST
+AC_GNU_SOURCE
 AM_INIT_AUTOMAKE(openvpn, [$PACKAGE_VERSION])
 
 AC_ARG_WITH(cygwin-native,

@@ -331,7 +332,6 @@ dnl Checks for programs.
 AC_PROG_CC
 AC_PROG_INSTALL
 AC_PROG_GCC_TRADITIONAL
-AC_GNU_SOURCE
 
 if test "${WIN32}" = "yes"; then
 	AC_ARG_VAR([MAN2HTML], [man2html utility])

1.7.1

This includes autoconf checks so we only build auth-pam on systems that have security/pam_appl.h

---
 Makefile.am   |   16 ++++++++++++++++
 configure.ac  |   10 ++++++++++
 m4/README     |    3 +++
 plugin/README |   13 +++++++------
 4 files changed, 36 insertions(+), 6 deletions(-)
 create mode 100644 m4/README

index ad0f7e3..3918cf7 100644
@@ -29,6 +29,8 @@ LDADD = @LIBOBJS@
 # INSTALL targets:
 AUTOMAKE_OPTIONS = foreign
 
+ACLOCAL_AMFLAGS = -I m4
+
 MAINTAINERCLEANFILES = \
 	config.log config.status \
 	$(srcdir)/Makefile.in \

@@ -141,6 +143,20 @@ openvpn_SOURCES = \
 	win32.h win32.c \
 	cryptoapi.h cryptoapi.c
 
+# building default plugins
+pkglib_LTLIBRARIES =	down-root.la
+if HAVE_PAM
+pkglib_LTLIBRARIES +=	auth-pam.la
+endif
+libtoolmoduleflags =	-module -avoid-version
+down_root_la_SOURCES =	plugin/down-root/down-root.c
+down_root_la_LDFLAGS =	$(libtoolmoduleflags)
+
+auth_pam_la_SOURCES =	plugin/auth-pam/auth-pam.c \
+			plugin/auth-pam/pamdl.h \
+			plugin/auth-pam/pamdl.c
+auth_pam_la_LIBADD =	-lpam
+auth_pam_la_LDFLAGS =	$(libtoolmoduleflags)
 
 dist-hook:
 	cd $(distdir) && for i in $(EXTRA_DIST) $(SUBDIRS) ; do find $$i -name .svn -type d -prune -exec rm -rf '{}' ';' ; rm -f `find $$i -type f | grep -E '(^|\/)\.?\#|\~$$|\.s?o$$'` ; done

index 529abd3..f6280cd 100644
@@ -29,6 +29,7 @@ m4_include(version.m4)
 AC_INIT([OpenVPN], [PRODUCT_VERSION], [[email protected]], [openvpn])
 AM_CONFIG_HEADER(config.h)
 AC_CONFIG_SRCDIR(syshead.h)
+AC_CONFIG_MACRO_DIR([m4])
 
 dnl Guess host type.
 AC_CANONICAL_HOST

@@ -332,6 +333,11 @@ dnl Checks for programs.
 AC_PROG_CC
 AC_PROG_INSTALL
 AC_PROG_GCC_TRADITIONAL
+LT_INIT([disable-static])
+dnl we need to dlopen() things, later, we might
+dnl look into libltdl stuff that can possibly link the plugin
+dnl statically, but I suppose that takes changes to plugin.? code,
+dnl too.  -- Matthias Andree, 2011-01-06
 
 if test "${WIN32}" = "yes"; then
 	AC_ARG_VAR([MAN2HTML], [man2html utility])

@@ -373,7 +379,9 @@ if test "${WIN32}" != "yes"; then
 		 netinet/tcp.h arpa/inet.h dnl
 		 netdb.h sys/uio.h linux/if_tun.h linux/sockios.h dnl
 		 linux/types.h sys/poll.h sys/epoll.h err.h dnl
+		 security/pam_appl.h dnl
    )
+
    AC_CHECK_HEADERS(net/if.h,,,
 		 [#ifdef HAVE_SYS_TYPES_H
 		  # include <sys/types.h>

@@ -416,6 +424,8 @@ if test "${WIN32}" != "yes"; then
 	])
 fi
 
+AM_CONDITIONAL(HAVE_PAM, test "$ac_cv_header_security_pam_appl_h" = yes)
+
 AC_CACHE_SAVE
 
 dnl check that in_addr_t is defined

new file mode 100644
index 0000000..8f83679
@@ -0,0 +1,3 @@
+This directory is empty in the Git repository and gets
+populated by libtoolize (which is usually run automatically from
+autoreconf).

index 6e490c5..7f8d805 100644
@@ -2,8 +2,8 @@ OpenVPN Plugins
 ---------------
 
 Starting with OpenVPN 2.0-beta17, compiled plugin modules are
-supported on any *nix OS which includes libdl or on Windows.
-One or more modules may be loaded into OpenVPN using
+supported on any *nix OS which includes libdl, or on Windows.
+   One or more modules may be loaded into OpenVPN using
 the --plugin directive, and each plugin module is capable of
 intercepting any of the script callbacks which OpenVPN supports:
 

@@ -28,7 +28,7 @@ auth-pam -- Authenticate using PAM and a split privilege
             execution model which functions even if
             root privileges or the execution environment
             have been altered with --user/--group/--chroot.
-            Tested on Linux only.
+            Tested on Linux only.  
 
 down-root -- Enable the running of down scripts with root privileges
              even if --user/--group/--chroot have been used

@@ -42,6 +42,7 @@ examples -- A simple example that demonstrates a portable
 Building Plugins
 ----------------
 
-cd to the top-level directory of a plugin, and use the
-"make" command to build it.  The examples plugin is
-built using a build script, not a makefile.
+The auth-pam and down-root modules are built as part of the regular
+OpenVPN build.  Other than that, cd to the top-level directory of a
+plugin, and use the "make" command to build it.  The examples plugin is
+built using a build script, not a Makefile.

1.7.1

I take it that these patches could be applied to the 2.1.4 version tree, right? If so, I would be able to test this tonight (including cross-compilation) and provide you with some feedback on these.