Check for ENABLE_MANAGEMENT for ENABLE_CLIENT_CR

When building a very minimal OpenVPN for OpenWRT with --disable-management among others, the compilaton fails due to ENABLE_CLIENT_CR being defined, although the management interface, which makes use of it, has been disabled.

The attached simple patch checks for ENABLE_MANAGEMENT before defining ENABLE_CLIENT_CR.

@@ -713,7 +713,9 @@
 /*
  * Do we support challenge/response authentication as client?
  */
+#if defined(ENABLE_MANAGEMENT)
 #define ENABLE_CLIENT_CR
+#endif
 
 /*
  * Do we support pushing peer info?

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

Which version are you compiling? I tried a couple of compile with the latest version in git (master branch) in combination with
- --disable-management and --enable-small ... And I could not manage to trigger this one. Our buildbot (even though, not testing all combinations) have also not triggered this one.

Could you provide more version information and the configure arguments you use?

kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk85AuIACgkQDC186MBRfroHEACfbJfdxCqMM0rp+wqlWx6yxc6F T+UAnAkQ1T7n0vb4F1cOqxaLTmQyQ4Zh
=Gwvf
-----END PGP SIGNATURE-----

I'm building latest GIT with:

./configure \
--disable-debug \
--disable-plugins \
--disable-management \
--disable-socks \
--disable-password-save \
--disable-multi \
--disable-server \
--disable-pkcs11 \
--disable-http \
--disable-port-share \
--disable-def-auth \
--disable-pf \
--disable-lzo \
--disable-selinux \
--disable-iproute2 \
--enable-small

(effectively, the version that supports only static keys and no fancy stuff)

Build fails with:
...
gcc -DHAVE_CONFIG_H -I. -I. -g -O2 -MT init.o -MD -MP -MF .deps/init.Tpo -c -o init.o init.c init.c: In function ‘do_route’:
init.c:1364:7: error: too few arguments to function ‘plugin_call’ plugin.h:196:1: note: declared here
init.c: In function ‘do_init_crypto_tls’:
init.c:2286:20: error: ‘const struct options’ has no member named ‘sc_info’ make[2]: *** [init.o] Error 1
...

Here are even two errors, one of which (plugin_call) function is addressed in OpenWRT patchset:
https://dev.openwrt.org/browser/packages/net/openvpn-polarssl/patches/300-fix-plugin_call-with-ssl.patch

And another is that i'm talking about.

I would love the OpenWRT guys o give us a heads up directly when something like this is noticed. Such things we really want to fix ASAP.

However, I didn't particular like the approach in that patch, so I've attached another patch for review. If this is acked, please consider using this one instead (patch 0001).

I'm giving your patch an ACK, so that will go into the tree. But even one more fix is needed, which is in the second patch I attached.

kind regards,

David Sommerseth

From a0cc57dc54aefd0507a83174fb6c0e7a2b47b90f Mon Sep 17 00:00:00 2001
From: David Sommerseth <[email protected]...>
Date: Mon, 13 Feb 2012 15:52:00 +0100
Subject: [PATCH 1/2] Fix compile issues when plug-ins are disabled.

Commit 1876ccd012e9e2ca6f8e1cd9e7e9bb4bf24ccecb modified plugin_call() and introduced plugin_call_ssl(). But the similar approach was missing for situations without plug-ins.

Solution: Rename plugin_call() in the #else !ENABLE_PLUGIN section to plugin_call_ssl(). Then move the plugin_ssl() function inside the #ifdef ENABLE_PLUGIN section outside the #ifdef, making it available for builds with and without plug-ins enabled.

---
 plugin.h |   31 +++++++++++++++----------------
 1 files changed, 15 insertions(+), 16 deletions(-)

index 948ab88..6f75e27 100644
@@ -133,20 +133,6 @@ int plugin_call_ssl (const struct plugin_list *pl,
 #endif
 		);
 
-static inline int
-plugin_call(const struct plugin_list *pl,
-	const int type,
-	const struct argv *av,
-	struct plugin_return *pr,
-	struct env_set *es)
-{
-  return plugin_call_ssl(pl, type, av, pr, es
-#ifdef USE_SSL
-      , -1, NULL
-#endif
-      );
-}
-
 void plugin_list_close (struct plugin_list *pl);
 bool plugin_defined (const struct plugin_list *pl, const int type);
 

@@ -182,7 +168,6 @@ plugin_return_init (struct plugin_return *pr)
 }
 
 #else
-
 struct plugin_list { int dummy; };
 struct plugin_return { int dummy; };
 

@@ -193,7 +178,7 @@ plugin_defined (const struct plugin_list *pl, const int type)
 }
 
 static inline int
-plugin_call (const struct plugin_list *pl,
+plugin_call_ssl (const struct plugin_list *pl,
 	     const int type,
 	     const struct argv *av,
 	     struct plugin_return *pr,

@@ -209,4 +194,18 @@ plugin_call (const struct plugin_list *pl,
 
 #endif /* ENABLE_PLUGIN */
 
+static inline int
+plugin_call(const struct plugin_list *pl,
+	const int type,
+	const struct argv *av,
+	struct plugin_return *pr,
+	struct env_set *es)
+{
+  return plugin_call_ssl(pl, type, av, pr, es
+#ifdef USE_SSL
+      , -1, NULL
+#endif
+      );
+}
+
 #endif /* OPENVPN_PLUGIN_H */

1.7.4.4

From e3ed7ec5c5a61141807d7028b6a88b24c2798d17 Mon Sep 17 00:00:00 2001
From: David Sommerseth <[email protected]...>
Date: Mon, 13 Feb 2012 16:03:46 +0100
Subject: [PATCH 2/2] Remove --show-gateway if debug info is not enabled
(--disable-debug)

The --show-gateway feature depends on functions only being enabled when --disable-debug is _not_ used. As this I consider --show-gateway more a handy function for debugging, removing this feature when --disable-debug is used seems like the proper approach.

---
 options.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

index a0b3431..2edec4e 100644
@@ -735,7 +735,9 @@ static const char usage_message[] =
 #endif				/* ENABLE_PKCS11 */
   "\n"
   "General Standalone Options:\n"
+#ifdef ENABLE_DEBUG
   "--show-gateway : Show info about default gateway.\n"
+#endif
  ;
 
 #endif /* !ENABLE_SMALL */

@@ -4031,6 +4033,7 @@ add_option (struct options *options,
 
       read_config_file (options, p[1], level, file, line, msglevel, permission_mask, option_types_found, es);
     }
+#ifdef ENABLE_DEBUG
   else if (streq (p[0], "show-gateway"))
     {
       struct route_gateway_info rgi;

@@ -4039,6 +4042,7 @@ add_option (struct options *options,
       print_default_gateway(M_INFO, &rgi);
       openvpn_exit (OPENVPN_EXIT_STATUS_GOOD); /* exit point */
     }
+#endif
 #if 0
   else if (streq (p[0], "foreign-option") && p[1])
     {

1.7.4.4

Hi,

Seconded :-)

ACK.

gert

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

Thanks!

Well, i'm no OpenWRT developer, just customizing some packages for my needs, but i thought that they are sending patches upstream :)

Anyway, if i happen to find anything, i'll post here.

What's for the second patch (gateway/debug stuff), it has been patched in OpenWRT too, i just forgot to mention it:
https://dev.openwrt.org/browser/packages/net/openvpn-polarssl/patches/400-fix-undefined-print_default.gateway.patch

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

Thanks a lot!!! That's wonderful! We'll do our best to get these things into the source tree as soon as possible too :)

Ahh,

yeah, I didn't think about checking the other patches. Even though, the change in init.c gave me a wtf!? moment ... but it turns out wrapping that one isn't needed either. As TEST_GET_DEFAULT_GATEWAY isn't defined anywhere. I'm guessing it's James Yonan testing out some code paths here. And it's been latent in the code tree for so long I'm wondering if we need it at all.

And it seems like that there were no more patches in the openwrt tree even. :)

kind regards,

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

iEYEARECAAYFAk85YmsACgkQDC186MBRfrrVRACfWwJ5OTa9GZ2GP3We0lKYyTNx MHsAnRVgLjLuenIfDW7whLjWChdMddw/
=QkuB
-----END PGP SIGNATURE-----

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

I've already ACKed this patch in another response in this mail thread. But here is the complete overview of all patches pushed out on the master branch for the -testing and -stable source trees.

commit 2ee0dc2bd72ec318fcc227af54e5ca7e1384a6cc

Author: Igor Novgorodov <[email protected]...>
Date:   Sun Feb 12 22:40:02 2012 +0400

The code blocks enabled by ENABLE_CLIENT_CR depends on management

If the management interface is not enabled, it makes no sense in including the ENABLE_CLIENT_CR #ifdef blocks. This will also in some configurations cause build issues if these blocks are enabled.

commit ecede953d6366e9fbfecea62cc1f61fd2347dab7

Author: David Sommerseth <[email protected]...>
Date:   Mon Feb 13 16:03:46 2012 +0100

Remove --show-gateway if debug info is not enabled (--disable-debug)

The --show-gateway feature depends on functions only being enabled when --disable-debug is _not_ used. As this I consider --show-gateway more a handy function for debugging, removing this feature when
- --disable-debug

    is used seems like the proper approach.

    Signed-off-by: David Sommerseth <[email protected]...>
    Acked-by: Gert Doering <[email protected]...>

commit 22277ec675847f73203bf908144f9903d13e2869

Author: David Sommerseth <[email protected]...>
Date:   Mon Feb 13 15:52:00 2012 +0100

Fix compile issues when plug-ins are disabled.

Commit 1876ccd012e9e2ca6f8e1cd9e7e9bb4bf24ccecb modified plugin_call() and introduced plugin_call_ssl(). But the similar approach was missing for situations without plug-ins.

Solution: Rename plugin_call() in the #else !ENABLE_PLUGIN section to plugin_call_ssl(). Then move the plugin_ssl() function inside the #ifdef ENABLE_PLUGIN section outside the #ifdef, making it available for builds with and without plug-ins enabled.

kind regards,

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

iEYEARECAAYFAk85Y4EACgkQDC186MBRfrpXCQCfbm+HLw3OwXQb3lcXXs1QWE3q g5AAn0NvBrUZbEMryUQgVQaX+yNrUuk3
=TeUL
-----END PGP SIGNATURE-----

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

I've already ACKed this patch in another response in this mail thread. But here is the complete overview of all patches pushed out on the master branch for the -testing and -stable source trees.

commit 2ee0dc2bd72ec318fcc227af54e5ca7e1384a6cc

Author: Igor Novgorodov <[email protected]...>
Date:   Sun Feb 12 22:40:02 2012 +0400

The code blocks enabled by ENABLE_CLIENT_CR depends on management

If the management interface is not enabled, it makes no sense in including the ENABLE_CLIENT_CR #ifdef blocks. This will also in some configurations cause build issues if these blocks are enabled.

commit ecede953d6366e9fbfecea62cc1f61fd2347dab7

Author: David Sommerseth <[email protected]...>
Date:   Mon Feb 13 16:03:46 2012 +0100

Remove --show-gateway if debug info is not enabled (--disable-debug)

The --show-gateway feature depends on functions only being enabled when --disable-debug is _not_ used. As this I consider --show-gateway more a handy function for debugging, removing this feature when
- --disable-debug

    is used seems like the proper approach.

    Signed-off-by: David Sommerseth <[email protected]...>
    Acked-by: Gert Doering <[email protected]...>

commit 22277ec675847f73203bf908144f9903d13e2869

Author: David Sommerseth <[email protected]...>
Date:   Mon Feb 13 15:52:00 2012 +0100

Fix compile issues when plug-ins are disabled.

Commit 1876ccd012e9e2ca6f8e1cd9e7e9bb4bf24ccecb modified plugin_call() and introduced plugin_call_ssl(). But the similar approach was missing for situations without plug-ins.

Solution: Rename plugin_call() in the #else !ENABLE_PLUGIN section to plugin_call_ssl(). Then move the plugin_ssl() function inside the #ifdef ENABLE_PLUGIN section outside the #ifdef, making it available for builds with and without plug-ins enabled.

kind regards,

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

iEYEARECAAYFAk85aukACgkQDC186MBRfrq0tACgojflVPWaBkrac+epuuk5Je2n 5NwAn3wyj5omLMZ1CtsGM3XpJBIyV/MU
=oXDO
-----END PGP SIGNATURE-----