PolarSSL patches

Openvpn-devel In review Adriaan de Jong 5 participants 11 messages Jul 5, 2011 – Jul 7, 2011

Share & tools

Adriaan de Jong

author

2011-07-05 11:37 UTC #0 lore raw

Hi everyone,

During last week's meeting we decided to temporarily create a github tree for OpenVPN. It can be found at https://github.com/andj/openvpn-ssl-refactoring . The version found there is now ready for testing, and will (hopefully) be included in the 2.3 alpha versions.

Could interested parties please have a look and provide me with feedback (through IRC/e-mail?).

Thanks!

Adriaan de Jong

PS. Note that due to limitations in PolarSSL, it is still missing a number of features:

* PKCS#12 file support
* --capath support - Loading certificate authorities from a directory
* Windows CryptoAPI support
* Management external key support
* X.509 alternative username fields (must be "CN")

Plugin/Script features:

* X.509 Serial number is in hex, not decimal as with OpenSSL
* X.509 subject line has a different format than the OpenSSL subject line
* X.509 certificate export does not work
* X.509 certificate tracking

Fox-IT ...for a more secure society
I www.fox-it.com<http://www.fox-it.com/>

Samuli Seppänen

2011-07-06 19:20 UTC #1 lore raw

Hi Adriaan,

I tried building your tree on Debian 5 (amd64) VM and got this error:

#make plugins
/usr/bin/make -C plugin/auth-pam/
make[1]: Entering directory `/home/buildslave/andj-openvpn-ssl-refactoring-4970f14/plugin/auth-pam' gcc -O2 -Wall -DDLOPEN_PAM=0 -fPIC -c -I../.. auth-pam.c
In file included from auth-pam.c:49:
../../openvpn-plugin.h:283: error: expected specifier-qualifier-list before ‘x509_cert_t’
make[1]: *** [auth-pam.o] Error 1
make[1]: Leaving directory `/home/buildslave/andj-openvpn-ssl-refactoring-4970f14/plugin/auth-pam'
make: *** [build-stamp] Error 2

The same VM has been used to successfully build the official release packages.

Samuli

Quoted 62 lines

Hi everyone,
During last week’s meeting we decided to temporarily create a github tree for OpenVPN. It can be found at
https://github.com/andj/openvpn-ssl-refactoring . The version found there is now ready for testing, and will (hopefully) be included in the 2.3 alpha versions.
Could interested parties please have a look and provide me with feedback (through IRC/e-mail?).
Thanks!
Adriaan de Jong
PS. Note that due to limitations in PolarSSL, it is still missing a number of features:
* PKCS#12 file support
* --capath support - Loading certificate authorities from a directory
* Windows CryptoAPI support
* Management external key support
* X.509 alternative username fields (must be "CN")
Plugin/Script features:
* X.509 Serial number is in hex, not decimal as with OpenSSL
* X.509 subject line has a different format than the OpenSSL subject line
* X.509 certificate export does not work
* X.509 certificate tracking
*Fox-IT* /...for a more secure society/
I www.fox-it.com <http://www.fox-it.com/>

chantra

2011-07-06 23:08 UTC #2 lore raw

Samuli,

You will need patch openvpn-plugin.h

openvpn-plugin.h +2 -6

index 9cce9d8..529a1c6 100644
@@ -25,12 +25,8 @@
 #ifndef OPENVPN_PLUGIN_H_
 #define OPENVPN_PLUGIN_H_
 
-#ifdef USE_OPENSSL
-#include "ssl_verify_openssl.h"
-#endif
-#ifdef USE_POLARSSL
-#include "ssl_verify_polarssl.h"
-#endif
+struct x509_cert;
+typedef struct x509_cert x509_cert_t;

That will get the plugin to compile, but will break when using openssl.

I believe the plugins should not need to know if openvpn uses polarssl or openssl... that might meant that openvpn will need to provide a generic x509 cert library to be used by openvpn plugins.

chantra

Quoted 101 lines · Samuli Seppänen

Hi Adriaan,
I tried building your tree on Debian 5 (amd64) VM and got this error:
#make plugins
/usr/bin/make -C plugin/auth-pam/
make[1]: Entering directory `/home/buildslave/andj-openvpn-ssl-refactoring-4970f14/plugin/auth-pam' gcc -O2 -Wall -DDLOPEN_PAM=0 -fPIC -c -I../.. auth-pam.c
In file included from auth-pam.c:49:
../../openvpn-plugin.h:283: error: expected specifier-qualifier-list before ‘x509_cert_t’
make[1]: *** [auth-pam.o] Error 1
make[1]: Leaving directory `/home/buildslave/andj-openvpn-ssl-refactoring-4970f14/plugin/auth-pam'
make: *** [build-stamp] Error 2
The same VM has been used to successfully build the official release packages.
Samuli
Quoted 54 lines
Hi everyone,
During last week’s meeting we decided to temporarily create a github tree for OpenVPN. It can be found at
https://github.com/andj/openvpn-ssl-refactoring . The version found there is now ready for testing, and will (hopefully) be included in the 2.3 alpha versions.
Could interested parties please have a look and provide me with feedback (through IRC/e-mail?).
Thanks!
Adriaan de Jong
PS. Note that due to limitations in PolarSSL, it is still missing a number of features:
* PKCS#12 file support
* --capath support - Loading certificate authorities from a
directory
* Windows CryptoAPI support
* Management external key support
* X.509 alternative username fields (must be "CN")
Plugin/Script features:
* X.509 Serial number is in hex, not decimal as with OpenSSL
* X.509 subject line has a different format than the OpenSSL
subject line
* X.509 certificate export does not work
Quoted 13 lines
* X.509 certificate tracking
Fox-IT ...for a more secure society
I www.fox-it.com
------------------------------------------------------------------------------
All of the data generated in your IT infrastructure is seriously valuable.
Why? It contains a definitive record of application performance, security
threats, fraudulent activity, and more. Splunk takes this data and makes
sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-d2d-c2
_______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Signature · 1 line

http://www.debuntu.org

Adriaan de Jong

author

2011-07-07 07:33 UTC #3 lore raw

Quoted 3 lines

From: chantra [mailto:[email protected]...]
Sent: donderdag 7 juli 2011 1:08
I believe the plugins should not need to know if openvpn uses polarssl or openssl... that might meant that openvpn will need to provide a generic x509 cert library to > be used by openvpn plugins.

The backends defined ssl_verify_backend.h contain much of that. Refactoring could help to make that clearer. If they can't use that backend, plugins need to be aware of whether OpenSSL or PolarSSL was used.

Adriaan

David Sommerseth

2011-07-07 07:35 UTC #4 lore raw

In reply to chantra

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

Quoted 21 lines · chantra

Samuli,

You will need patch openvpn-plugin.h

openvpn-plugin.h +2 -6

index 9cce9d8..529a1c6 100644
@@ -25,12 +25,8 @@
 #ifndef OPENVPN_PLUGIN_H_
 #define OPENVPN_PLUGIN_H_
 
-#ifdef USE_OPENSSL
-#include "ssl_verify_openssl.h"
-#endif
-#ifdef USE_POLARSSL
-#include "ssl_verify_polarssl.h"
-#endif
+struct x509_cert;
+typedef struct x509_cert x509_cert_t;

Quoted 1 line

That will get the plugin to compile, but will break when using openssl.

This is the wrong approach, as this will break the SSL modularity begin implemented.

Quoted 3 lines

I believe the plugins should not need to know if openvpn uses polarssl or openssl... that might meant that openvpn will need to provide a generic x509 cert library to be used by openvpn plugins.

For now, I say the plug-ins will need to be SSL implementation aware. This is especially important for the plugin-v3 API. This API provides a parsed X509 struct to the plug-ins with the certificate information. So this API needs to be adopted to support both PolarSSL and OpenSSL, and plug-ins need to use the appropriate SSL library which OpenVPN uses.

To provide a generic X509 cert library sounds good, but I believe the efforts of providing such a layer will be far more costly (in time) than to have SSL implementation aware plug-ins. And it might give far more challenges in regards to supported features.

So for now, I recommend plug-ins to support OpenSSL and/or PolarSSL natively. If someone can come up with a sensible X509 API layer which can be used instead, lets review that if someone provides it.

kind regards,

David Sommerseth

Quoted 18 lines · Samuli Seppänen

Quoted 17 lines
Hi Adriaan,
I tried building your tree on Debian 5 (amd64) VM and got this error:
#make plugins
/usr/bin/make -C plugin/auth-pam/
make[1]: Entering directory `/home/buildslave/andj-openvpn-ssl-refactoring-4970f14/plugin/auth-pam' gcc -O2 -Wall -DDLOPEN_PAM=0 -fPIC -c -I../.. auth-pam.c
In file included from auth-pam.c:49:
../../openvpn-plugin.h:283: error: expected specifier-qualifier-list before ‘x509_cert_t’
make[1]: *** [auth-pam.o] Error 1
make[1]: Leaving directory `/home/buildslave/andj-openvpn-ssl-refactoring-4970f14/plugin/auth-pam'
make: *** [build-stamp] Error 2

Quoted 6 lines

Quoted 6 lines
The same VM has been used to successfully build the official release packages.
Samuli

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

iEYEARECAAYFAk4VYa0ACgkQDC186MBRfroUDACfXevwkwEQAlFxI/3jIffz2Hbo kJYAniv9+oLrqafN1zq/1oUG59rbIgPX
=QthI
-----END PGP SIGNATURE-----

chantra

2011-07-07 08:52 UTC #5 lore raw

Quoted 25 lines · David Sommerseth

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

Quoted 21 lines · chantra

Samuli,

You will need patch openvpn-plugin.h

openvpn-plugin.h +2 -6

index 9cce9d8..529a1c6 100644
@@ -25,12 +25,8 @@
 #ifndef OPENVPN_PLUGIN_H_
 #define OPENVPN_PLUGIN_H_
 
-#ifdef USE_OPENSSL
-#include "ssl_verify_openssl.h"
-#endif
-#ifdef USE_POLARSSL
-#include "ssl_verify_polarssl.h"
-#endif
+struct x509_cert;
+typedef struct x509_cert x509_cert_t;

Quoted 4 lines

Quoted 1 line
That will get the plugin to compile, but will break when using openssl.
This is the wrong approach, as this will break the SSL modularity begin implemented.

Dont get me wrong, I did not mean to say that it was the right approach, but auth-pam plugin, like the plugins I wrote, just dont use SSL certs hence here is no need to include SSL headers.
It was rather a quick fix for Samuli to be able to compile the plugin and test the polar ssl backend which I understood was the idea here.

The dependency to SSL got introduce in OPENVPN_PLUGIN_VERSION 3 [1]

While before it was using rather common openssl headers [2], with the introduction of Adriaan changes, chances of getting the right headers might be lower.

[1]
http://openvpn.git.sourceforge.net/git/gitweb.cgi?p=openvpn/openvpn-testing.git;a=blob;f=openvpn-plugin.h;h=24aa36cbf48d792920ac79f6855ef1074929bb2d;hb=HEAD
[2]
http://openvpn.git.sourceforge.net/git/gitweb.cgi?p=openvpn/openvpn-testing.git;a=blob;f=openvpn-plugin.h;h=24aa36cbf48d792920ac79f6855ef1074929bb2d;hb=HEAD#l25

chantra

Quoted 60 lines

Quoted 4 lines
I believe the plugins should not need to know if openvpn uses polarssl or openssl... that might meant that openvpn will need to provide a generic x509 cert library to be used by openvpn plugins.
For now, I say the plug-ins will need to be SSL implementation aware. This is especially important for the plugin-v3 API. This API provides a parsed X509 struct to the plug-ins with the certificate information. So this API needs to be adopted to support both PolarSSL and OpenSSL, and plug-ins need to use the appropriate SSL library which OpenVPN uses.
To provide a generic X509 cert library sounds good, but I believe the efforts of providing such a layer will be far more costly (in time) than to have SSL implementation aware plug-ins. And it might give far more challenges in regards to supported features.
So for now, I recommend plug-ins to support OpenSSL and/or PolarSSL natively. If someone can come up with a sensible X509 API layer which can be used instead, lets review that if someone provides it.
kind regards,
David Sommerseth
Quoted 18 lines · Samuli Seppänen
Quoted 17 lines
Hi Adriaan,
I tried building your tree on Debian 5 (amd64) VM and got this error:
#make plugins
/usr/bin/make -C plugin/auth-pam/
make[1]: Entering directory `/home/buildslave/andj-openvpn-ssl-refactoring-4970f14/plugin/auth-pam' gcc -O2 -Wall -DDLOPEN_PAM=0 -fPIC -c -I../.. auth-pam.c
In file included from auth-pam.c:49:
../../openvpn-plugin.h:283: error: expected specifier-qualifier-list before ‘x509_cert_t’
make[1]: *** [auth-pam.o] Error 1
make[1]: Leaving directory `/home/buildslave/andj-openvpn-ssl-refactoring-4970f14/plugin/auth-pam'
make: *** [build-stamp] Error 2
Quoted 6 lines
Quoted 6 lines
The same VM has been used to successfully build the official release packages.
Samuli
[...snip...]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk4VYa0ACgkQDC186MBRfroUDACfXevwkwEQAlFxI/3jIffz2Hbo kJYAniv9+oLrqafN1zq/1oUG59rbIgPX
=QthI
-----END PGP SIGNATURE-----

Signature · 1 line

http://www.debuntu.org

Adriaan de Jong

author

2011-07-07 07:09 UTC #6 lore raw

In reply to Samuli Seppänen

Hi Samuli,

Since plugins have support for X.509 certificates, they need to enable either USE_OPENSSL or USE_POLARSSL, and include the appropriate X.509 backend (ssl_verify_*.h). Since neither was defined, the file was not included and therefore the build failed.

This is part of a larger problem, where if USE_SSL is not defined a dependency to OpenSSL's x509 header still exists for plugins. I've fixed this in a patch that I'll upload and mail soon.

Adriaan

From: Samuli Seppänen [mailto:[email protected]...]
Sent: woensdag 6 juli 2011 21:21
To: Adriaan de Jong
Cc: [email protected]
Subject: Re: [Openvpn-devel] PolarSSL patches

Hi Adriaan,

I tried building your tree on Debian 5 (amd64) VM and got this error:

#make plugins
/usr/bin/make -C plugin/auth-pam/
make[1]: Entering directory `/home/buildslave/andj-openvpn-ssl-refactoring-4970f14/plugin/auth-pam' gcc -O2 -Wall -DDLOPEN_PAM=0 -fPIC -c -I../.. auth-pam.c
In file included from auth-pam.c:49:
../../openvpn-plugin.h:283: error: expected specifier-qualifier-list before 'x509_cert_t' make[1]: *** [auth-pam.o] Error 1
make[1]: Leaving directory `/home/buildslave/andj-openvpn-ssl-refactoring-4970f14/plugin/auth-pam'
make: *** [build-stamp] Error 2

The same VM has been used to successfully build the official release packages.

Samuli

Hi everyone,

Could interested parties please have a look and provide me with feedback (through IRC/e-mail?).

Thanks!

Adriaan de Jong

PS. Note that due to limitations in PolarSSL, it is still missing a number of features:

Plugin/Script features:

Fox-IT ...for a more secure society
I www.fox-it.com<http://www.fox-it.com/>

Adriaan de Jong

author

2011-07-07 07:27 UTC #7 lore raw

PATCH 1/1 Fixes for the plugin system:

- Removed the dependency on an SSL library for USE_SSL when creating non-SSL plugins
- Fixed example plugin code to include USE_SSL when needed

Signed-off-by Adriaan de Jong <[email protected]...>

---
 init.c                   |    2 +-
 misc.c                   |    2 +-
 multi.c                  |    8 ++++----
 openvpn-plugin.h         |   18 +++++++++++++-----
 pf.c                     |    2 +-
 plugin.c                 |   36 ++++++++++++++++++++++++++----------
 plugin.h                 |   35 ++++++++++++++++++++++++++++-------
 plugin/examples/log_v3.c |    3 +++
 socket.c                 |    2 +-
 ssl.c                    |    2 +-
 ssl_verify.c             |    4 ++--
 11 files changed, 81 insertions(+), 33 deletions(-)

init.c +1 -1

index fd18557..1633acb 100644
@@ -1250,7 +1250,7 @@ do_route (const struct options *options,
 
   if (plugin_defined (plugins, OPENVPN_PLUGIN_ROUTE_UP))
     {
-      if (plugin_call (plugins, OPENVPN_PLUGIN_ROUTE_UP, NULL, NULL, es, -1, NULL) != OPENVPN_PLUGIN_FUNC_SUCCESS)
+      if (plugin_call (plugins, OPENVPN_PLUGIN_ROUTE_UP, NULL, NULL, es) != OPENVPN_PLUGIN_FUNC_SUCCESS)
 	msg (M_WARN, "WARNING: route-up plugin call failed");
     }

misc.c +1 -1

index 1f94c30..119ef84 100644
@@ -213,7 +213,7 @@ run_up_down (const char *command,
 		   ifconfig_local, ifconfig_remote,
 		   context);
 
-      if (plugin_call (plugins, plugin_type, &argv, NULL, es, -1, NULL) != OPENVPN_PLUGIN_FUNC_SUCCESS)
+      if (plugin_call (plugins, plugin_type, &argv, NULL, es) != OPENVPN_PLUGIN_FUNC_SUCCESS)
 	msg (M_FATAL, "ERROR: up/down plugin call failed");
 
       argv_reset (&argv);

multi.c +4 -4

index 15bea22..d4b6a9b 100644
@@ -91,7 +91,7 @@ learn_address_script (const struct multi_context *m,
 		   mroute_addr_print (addr, &gc));
       if (mi)
 	argv_printf_cat (&argv, "%s", tls_common_name (mi->context.c2.tls_multi, false));
-      if (plugin_call (plugins, OPENVPN_PLUGIN_LEARN_ADDRESS, &argv, NULL, es, -1, NULL) != OPENVPN_PLUGIN_FUNC_SUCCESS)
+      if (plugin_call (plugins, OPENVPN_PLUGIN_LEARN_ADDRESS, &argv, NULL, es) != OPENVPN_PLUGIN_FUNC_SUCCESS)
 	{
 	  msg (M_WARN, "WARNING: learn-address plugin call failed");
 	  ret = false;

@@ -468,7 +468,7 @@ multi_client_disconnect_script (struct multi_context *m,
 
       if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_DISCONNECT))
 	{
-	  if (plugin_call (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_DISCONNECT, NULL, NULL, mi->context.c2.es, -1, NULL) != OPENVPN_PLUGIN_FUNC_SUCCESS)
+	  if (plugin_call (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_DISCONNECT, NULL, NULL, mi->context.c2.es) != OPENVPN_PLUGIN_FUNC_SUCCESS)
 	    msg (M_WARN, "WARNING: client-disconnect plugin call failed");
 	}

@@ -1633,7 +1633,7 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
           }
 
 	  argv_printf (&argv, "%s", dc_file);
-	  if (plugin_call (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT, &argv, NULL, mi->context.c2.es, -1, NULL) != OPENVPN_PLUGIN_FUNC_SUCCESS)
+	  if (plugin_call (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT, &argv, NULL, mi->context.c2.es) != OPENVPN_PLUGIN_FUNC_SUCCESS)
 	    {
 	      msg (M_WARN, "WARNING: client-connect plugin call failed");
 	      cc_succeeded = false;

@@ -1654,7 +1654,7 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 
 	  plugin_return_init (&pr);
 
-	  if (plugin_call (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT_V2, NULL, &pr, mi->context.c2.es, -1, NULL) != OPENVPN_PLUGIN_FUNC_SUCCESS)
+	  if (plugin_call (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT_V2, NULL, &pr, mi->context.c2.es) != OPENVPN_PLUGIN_FUNC_SUCCESS)
 	    {
 	      msg (M_WARN, "WARNING: client-connect-v2 plugin call failed");
 	      cc_succeeded = false;

openvpn-plugin.h +13 -5

index 9cce9d8..65a3ffd 100644
@@ -25,12 +25,15 @@
 #ifndef OPENVPN_PLUGIN_H_
 #define OPENVPN_PLUGIN_H_
 
-#ifdef USE_OPENSSL
+#ifdef USE_SSL
+#if defined(USE_OPENSSL)
 #include "ssl_verify_openssl.h"
-#endif
-#ifdef USE_POLARSSL
+#elif defined(USE_POLARSSL)
 #include "ssl_verify_polarssl.h"
+#else
+#error "Either USE_OPENSSL or USE_POLARSSL should be defined"
 #endif
+#endif /*USE_SSL*/
 
 #define OPENVPN_PLUGIN_VERSION 3

@@ -267,9 +270,9 @@ struct openvpn_plugin_args_open_return
  * *per_client_context : the per-client context pointer which was returned by
  *        openvpn_plugin_client_constructor_v1, if defined.
  *
- * current_cert_depth : Certificate depth of the certificate being passed over
+ * current_cert_depth : Certificate depth of the certificate being passed over (only if compiled with USE_SSL defined)
  *
- * *current_cert : X509 Certificate object received from the client
+ * *current_cert : X509 Certificate object received from the client (only if compiled with USE_SSL defined)
  *
  */
 struct openvpn_plugin_args_func_in

@@ -279,8 +282,13 @@ struct openvpn_plugin_args_func_in
   const char ** const envp;
   openvpn_plugin_handle_t handle;
   void *per_client_context;
+#ifdef USE_SSL
   int current_cert_depth;
   x509_cert_t *current_cert;
+#else
+  int current_cert_depth; /* Unused, for compatibility purposes only */
+  void *current_cert; /* Unused, for compatibility purposes only */
+#endif
 };

pf.c +1 -1

index 8aae99c..6b4cba4 100644
@@ -563,7 +563,7 @@ pf_init_context (struct context *c)
       if( pf_file ) {
         setenv_str (c->c2.es, "pf_file", pf_file);
 
-        if (plugin_call (c->plugins, OPENVPN_PLUGIN_ENABLE_PF, NULL, NULL, c->c2.es, -1, NULL) == OPENVPN_PLUGIN_FUNC_SUCCESS)
+        if (plugin_call (c->plugins, OPENVPN_PLUGIN_ENABLE_PF, NULL, NULL, c->c2.es) == OPENVPN_PLUGIN_FUNC_SUCCESS)
           {
             event_timeout_init (&c->c2.pf.reload, 1, now);
             c->c2.pf.filename = string_alloc (pf_file, NULL);

plugin.c +26 ~~-10~~

index 4cfdcaa..331671c 100644
@@ -345,9 +345,12 @@ plugin_call_item (const struct plugin *p,
 		  const int type,
 		  const struct argv *av,
 		  struct openvpn_plugin_string_list **retlist,
-		  const char **envp,
-		  int certdepth,
-		  x509_cert_t *current_cert)
+		  const char **envp
+#ifdef USE_SSL
+		  , int certdepth,
+		  x509_cert_t *current_cert
+#endif
+		 )
 {
   int status = OPENVPN_PLUGIN_FUNC_SUCCESS;

@@ -372,8 +375,14 @@ plugin_call_item (const struct plugin *p,
                                                     .envp    = envp,
                                                     .handle  = p->plugin_handle,
 						    .per_client_context = per_client_context,
+#ifdef USE_SSL
 						    .current_cert_depth = (current_cert ? certdepth : -1),
-						    .current_cert = current_cert };
+						    .current_cert = current_cert
+#else
+						    .current_cert_depth = -1,
+						    .current_cert = NULL
+#endif
+						  };
         struct openvpn_plugin_args_func_return retargs;
 
         CLEAR(retargs);

@@ -570,13 +579,16 @@ plugin_list_open (struct plugin_list *pl,
 }
 
 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,
-	     struct env_set *es,
-             int certdepth,
-	     x509_cert_t *current_cert)
+	     struct env_set *es
+#ifdef USE_SSL
+             , int certdepth,
+	     x509_cert_t *current_cert
+#endif
+	    )
 {
   if (pr)
     plugin_return_init (pr);

@@ -601,8 +613,12 @@ plugin_call (const struct plugin_list *pl,
 					       type,
 					       av,
 					       pr ? &pr->list[i] : NULL,
-					       envp,
-					       certdepth, current_cert);
+					       envp
+#ifdef USE_SSL
+					       ,certdepth,
+					       current_cert
+#endif
+					      );
 	  switch (status)
 	    {
 	    case OPENVPN_PLUGIN_FUNC_SUCCESS:

plugin.h +28 -7

index 5518147..8fa4168 100644
@@ -116,13 +116,31 @@ void plugin_list_open (struct plugin_list *pl,
 
 struct plugin_list *plugin_list_inherit (const struct plugin_list *src);
 
-int plugin_call (const struct plugin_list *pl,
+
+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
+      );
+}
+
+int plugin_call_ssl (const struct plugin_list *pl,
 		 const int type,
 		 const struct argv *av,
 		 struct plugin_return *pr,
-		 struct env_set *es,
-		 int current_cert_depth,
-		 x509_cert_t *current_cert);
+		 struct env_set *es
+#ifdef USE_SSL
+		 , int current_cert_depth,
+		 x509_cert_t *current_cert
+#endif
+		);
 
 void plugin_list_close (struct plugin_list *pl);
 bool plugin_defined (const struct plugin_list *pl, const int type);

@@ -174,9 +192,12 @@ plugin_call (const struct plugin_list *pl,
 	     const int type,
 	     const struct argv *av,
 	     struct plugin_return *pr,
-	     struct env_set *es,
-	     int current_cert_depth,
-	     x509_cert_t *current_cert)
+	     struct env_set *es
+#ifdef USE_SSL
+	     , int current_cert_depth,
+	     x509_cert_t *current_cert
+#endif
+	    )
 {
   return 0;
 }

plugin/examples/log_v3.c +3 -0

index bf2f1dc..2dff6a6 100644
@@ -36,6 +36,9 @@
 #include <string.h>
 #include <stdlib.h>
 
+#define USE_SSL
+#define USE_OPENSSL
+
 #include "openvpn-plugin.h"
 
 /*

socket.c +1 -1

index 6b855c0..ff31684 100644
@@ -2151,7 +2151,7 @@ link_socket_connection_initiated (const struct buffer *buf,
     {
       struct argv argv = argv_new ();
       ipchange_fmt (false, &argv, info, &gc);
-      if (plugin_call (info->plugins, OPENVPN_PLUGIN_IPCHANGE, &argv, NULL, es, -1, NULL) != OPENVPN_PLUGIN_FUNC_SUCCESS)
+      if (plugin_call (info->plugins, OPENVPN_PLUGIN_IPCHANGE, &argv, NULL, es) != OPENVPN_PLUGIN_FUNC_SUCCESS)
 	msg (M_WARN, "WARNING: ipchange plugin call failed");
       argv_reset (&argv);
     }

ssl.c +1 -1

index 3645fb0..42262de 100644
@@ -1920,7 +1920,7 @@ key_method_2_read (struct buffer *buf, struct tls_multi *multi, struct tls_sessi
    */
   if (ks->authenticated && plugin_defined (session->opt->plugins, OPENVPN_PLUGIN_TLS_FINAL))
     {
-      if (plugin_call (session->opt->plugins, OPENVPN_PLUGIN_TLS_FINAL, NULL, NULL, session->opt->es, -1, NULL) != OPENVPN_PLUGIN_FUNC_SUCCESS)
+      if (plugin_call (session->opt->plugins, OPENVPN_PLUGIN_TLS_FINAL, NULL, NULL, session->opt->es) != OPENVPN_PLUGIN_FUNC_SUCCESS)
 	ks->authenticated = false;
     }

ssl_verify.c +2 -2

index 8592942..209d7f3 100644
@@ -431,7 +431,7 @@ verify_cert_call_plugin(const struct plugin_list *plugins, struct env_set *es,
 
       argv_printf (&argv, "%d %s", cert_depth, subject);
 
-      ret = plugin_call (plugins, OPENVPN_PLUGIN_TLS_VERIFY, &argv, NULL, es, cert_depth, cert);
+      ret = plugin_call_ssl (plugins, OPENVPN_PLUGIN_TLS_VERIFY, &argv, NULL, es, cert_depth, cert);
 
       argv_reset (&argv);

@@ -1012,7 +1012,7 @@ verify_user_pass_plugin (struct tls_session *session, const struct user_pass *up
 #endif
 
       /* call command */
-      retval = plugin_call (session->opt->plugins, OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY, NULL, NULL, session->opt->es, -1, NULL);
+      retval = plugin_call (session->opt->plugins, OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY, NULL, NULL, session->opt->es);
 
 #ifdef PLUGIN_DEF_AUTH
       /* purge auth control filename (and file itself) for non-deferred returns */

Signature · 1 line

1.7.4.1

Adriaan de Jong

author

2011-07-07 08:19 UTC #8 lore raw

PATCH 1/1 Further improvements to plugin support:

- Renamed struct entries to explicitly show them as disabled
- Added a warning if USE_SSL is enabled, but neither ssl_verify_openssl.h or ssl_verify_polarssl.h is included
- If neither of those files is included, disable ssl support for a plugin including openvpn-plugin.h

Signed-off-by Adriaan de Jong <[email protected]...>

---
 openvpn-plugin.h         |   19 ++++++++-----------
 plugin.c                 |    4 ++--
 plugin.h                 |    6 ++++++
 plugin/examples/log_v3.c |    1 +
 4 files changed, 17 insertions(+), 13 deletions(-)

openvpn-plugin.h +8 ~~-11~~

index 65a3ffd..474c910 100644
@@ -24,15 +24,12 @@
 
 #ifndef OPENVPN_PLUGIN_H_
 #define OPENVPN_PLUGIN_H_
-
 #ifdef USE_SSL
-#if defined(USE_OPENSSL)
-#include "ssl_verify_openssl.h"
-#elif defined(USE_POLARSSL)
-#include "ssl_verify_polarssl.h"
-#else
-#error "Either USE_OPENSSL or USE_POLARSSL should be defined"
-#endif
+#  if defined(SSL_VERIFY_OPENSSL_H_) || defined(SSL_VERIFY_POLARSSL_H_)
+#    define ENABLE_SSL_PLUGIN
+#  else
+#    warning "Neither OpenSSL or PoLarSSL headers included, disabling plugin's SSL support"
+#  endif
 #endif /*USE_SSL*/
 
 #define OPENVPN_PLUGIN_VERSION 3

@@ -282,12 +279,12 @@ struct openvpn_plugin_args_func_in
   const char ** const envp;
   openvpn_plugin_handle_t handle;
   void *per_client_context;
-#ifdef USE_SSL
+#ifdef ENABLE_SSL_PLUGIN
   int current_cert_depth;
   x509_cert_t *current_cert;
 #else
-  int current_cert_depth; /* Unused, for compatibility purposes only */
-  void *current_cert; /* Unused, for compatibility purposes only */
+  int __current_cert_depth_disabled; /* Unused, for compatibility purposes only */
+  void *__current_cert_disabled; /* Unused, for compatibility purposes only */
 #endif
 };

plugin.c +2 -2

index 331671c..dc581a4 100644
@@ -379,8 +379,8 @@ plugin_call_item (const struct plugin *p,
 						    .current_cert_depth = (current_cert ? certdepth : -1),
 						    .current_cert = current_cert
 #else
-						    .current_cert_depth = -1,
-						    .current_cert = NULL
+						    .__current_cert_depth_disabled = -1,
+						    .__current_cert_disabled = NULL
 #endif
 						  };
         struct openvpn_plugin_args_func_return retargs;

plugin.h +6 -0

index 8fa4168..8782751 100644
@@ -29,6 +29,12 @@
 #ifndef OPENVPN_PLUGIN_H
 #define OPENVPN_PLUGIN_H
 
+#ifdef USE_OPENSSL
+#include "ssl_verify_openssl.h"
+#endif
+#ifdef USE_POLARSSL
+#include "ssl_verify_polarssl.h"
+#endif
 #include "openvpn-plugin.h"
 
 #ifdef ENABLE_PLUGIN

plugin/examples/log_v3.c +1 -0

index 2dff6a6..187c592 100644
@@ -38,6 +38,7 @@
 
 #define USE_SSL
 #define USE_OPENSSL
+#include "ssl_verify_openssl.h"
 
 #include "openvpn-plugin.h"

Signature · 1 line

1.7.4.1

Matthias Andree

2011-07-07 11:34 UTC #9 lore raw

Re: [Openvpn-devel] [PATCH] Further improvements to plugin support:

Am 07.07.2011 10:19, schrieb Adriaan de Jong:

Quoted 3 lines

- Renamed struct entries to explicitly show them as disabled
- Added a warning if USE_SSL is enabled, but neither ssl_verify_openssl.h or ssl_verify_polarssl.h is included
- If neither of those files is included, disable ssl support for a plugin including openvpn-plugin.h

This arrived later, but does address my earlier concern.

Quoted 21 lines

#else

openvpn-plugin.h +2 -2

-  int current_cert_depth; /* Unused, for compatibility purposes only */
-  void *current_cert; /* Unused, for compatibility purposes only */
+  int __current_cert_depth_disabled; /* Unused, for compatibility purposes only */
+  void *__current_cert_disabled; /* Unused, for compatibility purposes only */
 #endif
 };

plugin.c +2 -2

index 331671c..dc581a4 100644
@@ -379,8 +379,8 @@ plugin_call_item (const struct plugin *p,
 						    .current_cert_depth = (current_cert ? certdepth : -1),
 						    .current_cert = current_cert
 #else
-						    .current_cert_depth = -1,
-						    .current_cert = NULL
+						    .__current_cert_depth_disabled = -1,
+						    .__current_cert_disabled = NULL
 #endif

Matthias Andree

2011-07-07 11:32 UTC #10 lore raw

Re: [Openvpn-devel] [PATCH] Fixes for the plugin system:

In reply to Adriaan de Jong

Am 07.07.2011 09:27, schrieb Adriaan de Jong:

Quoted 22 lines

- Removed the dependency on an SSL library for USE_SSL when creating non-SSL plugins
- Fixed example plugin code to include USE_SSL when needed

Signed-off-by Adriaan de Jong <[email protected]...>

openvpn-plugin.h +2 -2

index 9cce9d8..65a3ffd 100644
@@ -267,9 +270,9 @@ struct openvpn_plugin_args_open_return
  * *per_client_context : the per-client context pointer which was returned by
  *        openvpn_plugin_client_constructor_v1, if defined.
  *
- * current_cert_depth : Certificate depth of the certificate being passed over
+ * current_cert_depth : Certificate depth of the certificate being passed over (only if compiled with USE_SSL defined)
  *
- * *current_cert : X509 Certificate object received from the client
+ * *current_cert : X509 Certificate object received from the client (only if compiled with USE_SSL defined)
  *
  */
 struct openvpn_plugin_args_func_in

@@ -279,8 +282,13 @@ struct openvpn_plugin_args_func_in
   const char ** const envp;

openvpn-plugin.h struct openvpn_plugin_args_func_in · line 292

Quoted 10 lines

openvpn-plugin.h +5 -0

   openvpn_plugin_handle_t handle;
   void *per_client_context;
+#ifdef USE_SSL
   int current_cert_depth;
   x509_cert_t *current_cert;
+#else
+  int current_cert_depth; /* Unused, for compatibility purposes only */
+  void *current_cert; /* Unused, for compatibility purposes only */
+#endif
 };

I'd veto the #else part here. It is dangerous (because it provides variables that are unused) and doesn't match the comments in the earlier hunk, and IMNSHO SSL-specific and/or undefined variables should also remain undeclared so that the plugins can either #define USE_SSL whatever, or remove references to the junkyard^W^Wcurrent_cert* variables.