openvpn-devel Mailing List for OpenVPN (Page 20)

Hi, On Wed, Feb 08, 2012 at 11:27:10AM -0800, James Ring wrote: > > Exactly.  The first three things are sort of "nearly done", the > > "receive file descriptor to use for tun/tap" would need to be > > implemented (tun.c, open_tun(), #ifdef ANDROID_MAGIC_VPN :-) ) > > I was thinking about this a little more. Presumably openvpn will be > forked and exec'd before the file descriptor is available. Presumably > openvpn could connect to a UNIX domain socket inside open_tun() if > ANDROID_MAGIC_VPN is specified. > > Does other code within openvpn care whether the fd is a UNIX socket or > a tun/tap device? I'm guessing there may be some ioctls it wants to > perform on the device. There aren't any ioctl()s (I'm aware of) for tun/tap devices, but blocking/non-blocking behaviour might be an interesting problem, and performance / battery usage won't be helped by copying the packet around another time... > Other than that, openvpn would be reading and > writing IP packets with an encrypted payload and the Java wrapper > would be responsible for forwarding the bytes between the UNIX domain > socket and the actual tun device. Don't show idea that to Jan-Just, he's already complaining that OpenVPN wastes too many CPU cycles... gert -- USENET is *not* the non-clickable part of WWW! //www.muc.de/~gert/ Gert Doering - Munich, Germany ge...@gr... fax: +49-89-35655025 ge...@ne... 

Hi James, 2012/2/8 James Ring <sj...@jd...>: > On Wed, Feb 8, 2012 at 10:24 AM, Gert Doering <ge...@gr...> wrote: >> Exactly.  The first three things are sort of "nearly done", the >> "receive file descriptor to use for tun/tap" would need to be >> implemented (tun.c, open_tun(), #ifdef ANDROID_MAGIC_VPN :-) ) > > I was thinking about this a little more. Presumably openvpn will be > forked and exec'd before the file descriptor is available. Presumably > openvpn could connect to a UNIX domain socket inside open_tun() if > ANDROID_MAGIC_VPN is specified. > > Does other code within openvpn care whether the fd is a UNIX socket or > a tun/tap device? I'm guessing there may be some ioctls it wants to > perform on the device. Other than that, openvpn would be reading and > writing IP packets with an encrypted payload and the Java wrapper > would be responsible for forwarding the bytes between the UNIX domain > socket and the actual tun device. Unless Android's Linux is stripped down in this respect, you can pass file descriptors over UNIX domain sockets. (The first google hit is [0]. The interface isn't beautiful, but it works nicely.) This would allow you to take the java wrapper out of the loop as far as the raw data shuffling is concerned. Cheers Fabian 0: http://www.lst.de/~okir/blackhats/node121.html 

Hey Gert, On Wed, Feb 8, 2012 at 10:24 AM, Gert Doering <ge...@gr...> wrote: > Hi, > > On Wed, Feb 08, 2012 at 04:47:35AM -0800, James Ring wrote: >> Perhaps I wrote this too hastily. I wasn't thinking of how OpenVPN >> actually works. OpenVPN would: >> >> * open the connection (tcp or udp) to the remote end >> * negotiate session parameters >> * provide the Android Java wrapper with the session parameters via the >> service pipe >> * receive the file descriptor to use as the tun/tap device from the >> Android Java wrapper via the service pipe > > Exactly.  The first three things are sort of "nearly done", the > "receive file descriptor to use for tun/tap" would need to be > implemented (tun.c, open_tun(), #ifdef ANDROID_MAGIC_VPN :-) ) I was thinking about this a little more. Presumably openvpn will be forked and exec'd before the file descriptor is available. Presumably openvpn could connect to a UNIX domain socket inside open_tun() if ANDROID_MAGIC_VPN is specified. Does other code within openvpn care whether the fd is a UNIX socket or a tun/tap device? I'm guessing there may be some ioctls it wants to perform on the device. Other than that, openvpn would be reading and writing IP packets with an encrypted payload and the Java wrapper would be responsible for forwarding the bytes between the UNIX domain socket and the actual tun device. Regards, James 

Hi, On Wed, Feb 08, 2012 at 04:47:35AM -0800, James Ring wrote: > Perhaps I wrote this too hastily. I wasn't thinking of how OpenVPN > actually works. OpenVPN would: > > * open the connection (tcp or udp) to the remote end > * negotiate session parameters > * provide the Android Java wrapper with the session parameters via the > service pipe > * receive the file descriptor to use as the tun/tap device from the > Android Java wrapper via the service pipe Exactly. The first three things are sort of "nearly done", the "receive file descriptor to use for tun/tap" would need to be implemented (tun.c, open_tun(), #ifdef ANDROID_MAGIC_VPN :-) ) > Another thing to think about would be whether the tunnel could be > reestablished after the device wakes up from sleep. Depending on the OpenVPN --ping parameter settings, it would reconnect to the server automatically - and then re-initialize the interface. gert -- USENET is *not* the non-clickable part of WWW! //www.muc.de/~gert/ Gert Doering - Munich, Germany ge...@gr... fax: +49-89-35655025 ge...@ne... 

Hi, On Wed, Feb 08, 2012 at 04:16:20AM -0800, James Ring wrote: > Looks like you need to pass a native fd. OpenVPN would not be able to > open the device itself. There looks to be a chicken and egg problem > here though: the fd is returned by the VpnService.Builder.establish() > method > > http://developer.android.com/reference/android/net/VpnService.Builder.html#establish() > > This needs to happen after a bunch of parameters are already known. > Since OpenVPN would normally take care of negotiating these with the > other end, it would seem that the tunnel fd is not available to > OpenVPN in time to do this. Perhaps the user would have to configure > the route, search domain, IP address and other parameters in advance > of starting the VPN connection. That would suck. You only need the *tunnel* FD to forward packets to the android networking stack. To connect to the OpenVPN server, you use a normal socket, which would have to be opened by OpenVPN - but that's a standard network operation which doesn't need special privileges. Right now, passing in the tun device file handle from an external source isn't something directly supported, but the open_tun() method is very platform specific anyway, so it's not unsolvable. [..] > > ... and for the rest, well, we'd need a volunteer that wants to *work* on > > this, not just ask for it...  I don't have an Android device (and no > > time) so it wouldn't be me. > > I'm raising my hand. This path sounds better than what I thought would > be necessary (writing OpenVPN client implementation in Java). I'm also > asking around here at Google to see if somebody with more experience > with Android is interested in helping out. I'm sure there is > sufficient interest on both sides (Android and OpenVPN) to get > something working. This would make a niche segment of Android users > very happy I'm sure. This would be extremely cool! gert -- USENET is *not* the non-clickable part of WWW! //www.muc.de/~gert/ Gert Doering - Munich, Germany ge...@gr... fax: +49-89-35655025 ge...@ne... 

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 05/02/12 16:48, Gert Doering wrote: > Hi, > > On Sun, Feb 05, 2012 at 03:02:40PM +0100, David Sommerseth wrote: >>> as promised :-) - it now passes all my t_client.rc tests, and >>> smells a lot less funny. We now might even throw out all these >>> weird #ifdefs from tun.h... >> >> Maybe an attachment would be nice? ;-) > > And I thought you could just fetch this from the git repo on my laptop > (suspended to disk and turned off, of course!). Well, that's what you > get when debugging assert() crashes instead concentrating on your > e-mail :-) ACK. Applied to master and pushed to -stable and -testing trees. commit 82d4e12068774b0a6ca787ef1345b8a16c460466 Author: Gert Doering <ge...@gr...> Date: Sun Feb 5 13:35:03 2012 +0100 Platform cleanup for OpenBSD Turn around initialization order (IFCONFIG_AFTER_TUN_OPEN) to make it "the same as all other platforms besides Windows" (tun.h). Remove "ifconfig destroy / ifconfig create" from open_tun() and change to generic "create tun device by opening /dev/tunN" approach, thus cleaning up the IFCONFIG_BEFORE_TUN_OPEN bit. Add "-link0" for ifconfig calls in tun mode, to make sure that even if we happen to re-use a not-cleaned-up tun interface in tap mode, it will then be setup correctly (-link0 -> tun, link0 -> tap). Add correct ifconfig calls for "topology subnet". On tunnel close, only call "ifconfig destroy" if it was a tap interface (tun + link0), because those do not auto-disappear (OpenBSD bug?) Get rid of READV/WRITEV #ifdef's - as per the man page, these calls have been added to 4.2BSD, and there never was an OpenBSD version without. Tested on OpenBSD 4.9 with tun+tap, ipv4+ipv6, topology net30+subnet Signed-off-by: Gert Doering <ge...@gr...> Acked-by: David Sommerseth <da...@re...> Signed-off-by: David Sommerseth <da...@re...> kind regards, David Sommerseth -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk8yos0ACgkQDC186MBRfrrVqgCffQQV0Dbpv9r3aKPVE/Vc9So/ wxIAnR37yPY1qGC7hzZQcyRReRLOjCRw =TDBp -----END PGP SIGNATURE----- 

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 07/02/12 14:31, David Sommerseth wrote: > In commit bee92b479414d12035b0422f81ac5fcfe14fa645 the gc_malloc() > was hardened to always require a gc_arena object for garbage > collection. Some places in the code expected the old behaviour of a > normal malloc() in these cases, that is a memory allocation without > garbage collection. > > This old behaviour is partly restored by allowing string_alloc() to > do a non-gc based allocation if no gc_arena object is available. In > addition some other places string_alloc() will now be called with a > gc_arena pointer where such an object is available. > > The alloc_buf() function has also been refactored to not use > gc_malloc() at all. > > v2: - removes a memleak when --ifconfig-ipv6 is used several times - > makes string_alloc() behave properly if DMALLOC is enabled > > Signed-off-by: David Sommerseth <da...@re...> --- buffer.c > | 32 ++++++++++++++++++++++++++++---- init.c | 2 +- > openvpn.c | 2 +- options.c | 7 ++++++- pf.c | 2 > +- ssl_verify.c | 2 ++ 6 files changed, 39 insertions(+), 8 > deletions(-) > Acked by Gert on #openvpn-devel. Applied to master and pushed out to -stable and -testing. commit dc7be6d078ba106f9b0de12f3e879c3561c3c537 Author: David Sommerseth <da...@re...> Date: Mon Feb 6 00:30:47 2012 +0100 Fix assert() situations where gc_malloc() is called without a gc_arena object Signed-off-by: David Sommerseth <da...@re...> Acked-by: Gert Doering <ge...@gr...> kind regards, David Sommerseth -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk8yoqkACgkQDC186MBRfrpwUQCgqMy+nI3jvoIzxJF6j8tOgNRo gdcAnAn9q5OOK4p8z/OJDvRkOkxxRQJs =kYaj -----END PGP SIGNATURE----- 

I give this one a feature-ACK as it seems to fix the buffer.c line 313 issue: <http://thread.gmane.org/gmane.network.openvpn.devel/5346> All the best, -- Samuli Seppänen Community Manager OpenVPN Technologies, Inc irc freenode net: mattock > In commit bee92b479414d12035b0422f81ac5fcfe14fa645 the gc_malloc() was hardened > to always require a gc_arena object for garbage collection. Some places in the > code expected the old behaviour of a normal malloc() in these cases, that is a > memory allocation without garbage collection. > > This old behaviour is partly restored by allowing string_alloc() to do a non-gc > based allocation if no gc_arena object is available. In addition some other > places string_alloc() will now be called with a gc_arena pointer where such an > object is available. > > The alloc_buf() function has also been refactored to not use gc_malloc() at > all. > > v2: - removes a memleak when --ifconfig-ipv6 is used several times > - makes string_alloc() behave properly if DMALLOC is enabled > > Signed-off-by: David Sommerseth <da...@re...> > --- > buffer.c | 32 ++++++++++++++++++++++++++++---- > init.c | 2 +- > openvpn.c | 2 +- > options.c | 7 ++++++- > pf.c | 2 +- > ssl_verify.c | 2 ++ > 6 files changed, 39 insertions(+), 8 deletions(-) > > diff --git a/buffer.c b/buffer.c > index c39bbcb..fca6a90 100644 > --- a/buffer.c > +++ b/buffer.c > @@ -54,11 +54,21 @@ alloc_buf_debug (size_t size, const char *file, int line) > alloc_buf (size_t size) > #endif > { > + struct buffer buf; > + > + if (!buf_size_valid (size)) > + buf_size_error (size); > + buf.capacity = (int)size; > + buf.offset = 0; > + buf.len = 0; > #ifdef DMALLOC > - return alloc_buf_gc_debug (size, NULL, file, line); > + buf.data = openvpn_dmalloc (file, line, size); > #else > - return alloc_buf_gc (size, NULL); > + buf.data = calloc (1, size); > #endif > + check_malloc_return(buf.data); > + > + return buf; > } > > struct buffer > @@ -515,11 +525,25 @@ string_alloc (const char *str, struct gc_arena *gc) > const int n = strlen (str) + 1; > char *ret; > > + if (gc) { > +#ifdef DMALLOC > + ret = (char *) gc_malloc_debug (n, false, gc, file, line); > +#else > + ret = (char *) gc_malloc (n, false, gc); > +#endif > + } else { > + /* If there are no garbage collector available, it's expected > + * that the caller cleans up afterwards. This is coherent with the > + * earlier behaviour when gc_malloc() would be called with gc == NULL > + */ > #ifdef DMALLOC > - ret = (char *) gc_malloc_debug (n, false, gc, file, line); > + ret = openvpn_dmalloc (file, line, n); > + memset(ret, 0, n); > #else > - ret = (char *) gc_malloc (n, false, gc); > + ret = calloc(1, n); > #endif > + check_malloc_return(ret); > + } > memcpy (ret, str, n); > return ret; > } > diff --git a/init.c b/init.c > index 525f441..f0c3693 100644 > --- a/init.c > +++ b/init.c > @@ -3012,7 +3012,7 @@ do_close_ifconfig_pool_persist (struct context *c) > static void > do_inherit_env (struct context *c, const struct env_set *src) > { > - c->c2.es = env_set_create (NULL); > + c->c2.es = env_set_create (&c->c2.gc); > c->c2.es_owned = true; > env_set_inherit (c->c2.es, src); > } > diff --git a/openvpn.c b/openvpn.c > index f5f2bce..84289d2 100644 > --- a/openvpn.c > +++ b/openvpn.c > @@ -164,7 +164,7 @@ main (int argc, char *argv[]) > gc_init (&c.gc); > > /* initialize environmental variable store */ > - c.es = env_set_create (NULL); > + c.es = env_set_create (&c.gc); > #ifdef WIN32 > set_win_sys_path_via_env (c.es); > #endif > diff --git a/options.c b/options.c > index 6b8ae22..a0b3431 100644 > --- a/options.c > +++ b/options.c > @@ -4291,7 +4291,7 @@ add_option (struct options *options, > { > unsigned int netbits; > char * ipv6_local; > - > + > VERIFY_PERMISSION (OPT_P_UP); > if ( get_ipv6_addr( p[1], NULL, &netbits, &ipv6_local, msglevel ) && > ipv6_addr_safe( p[2] ) ) > @@ -4301,6 +4301,11 @@ add_option (struct options *options, > msg( msglevel, "ifconfig-ipv6: /netbits must be between 64 and 124, not '/%d'", netbits ); > goto err; > } > + > + if (options->ifconfig_ipv6_local) > + /* explicitly ignoring this is a const char */ > + free ((char *) options->ifconfig_ipv6_local); > + > options->ifconfig_ipv6_local = ipv6_local; > options->ifconfig_ipv6_netbits = netbits; > options->ifconfig_ipv6_remote = p[2]; > diff --git a/pf.c b/pf.c > index 6b4cba4..79915fa 100644 > --- a/pf.c > +++ b/pf.c > @@ -566,7 +566,7 @@ pf_init_context (struct context *c) > 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); > + c->c2.pf.filename = string_alloc (pf_file, &c->c2.gc); > c->c2.pf.enabled = true; > #ifdef ENABLE_DEBUG > if (check_debug_level (D_PF_DEBUG)) > diff --git a/ssl_verify.c b/ssl_verify.c > index e45f149..37d4982 100644 > --- a/ssl_verify.c > +++ b/ssl_verify.c > @@ -83,6 +83,7 @@ set_common_name (struct tls_session *session, const char *common_name) > } > if (common_name) > { > + /* FIXME: Last alloc will never be freed */ > session->common_name = string_alloc (common_name, NULL); > #ifdef ENABLE_PF > { > @@ -703,6 +704,7 @@ man_def_auth_set_client_reason (struct tls_multi *multi, const char *client_reas > multi->client_reason = NULL; > } > if (client_reason && strlen (client_reason)) > + /* FIXME: Last alloc will never be freed */ > multi->client_reason = string_alloc (client_reason, NULL); > } > 

Hey, On Wed, Feb 8, 2012 at 4:16 AM, James Ring <sj...@jd...> wrote: > Looks like you need to pass a native fd. OpenVPN would not be able to > open the device itself. There looks to be a chicken and egg problem > here though: the fd is returned by the VpnService.Builder.establish() > method > > http://developer.android.com/reference/android/net/VpnService.Builder.html#establish() > > This needs to happen after a bunch of parameters are already known. > Since OpenVPN would normally take care of negotiating these with the > other end, it would seem that the tunnel fd is not available to > OpenVPN in time to do this. Perhaps the user would have to configure > the route, search domain, IP address and other parameters in advance > of starting the VPN connection. That would suck. Perhaps I wrote this too hastily. I wasn't thinking of how OpenVPN actually works. OpenVPN would: * open the connection (tcp or udp) to the remote end * negotiate session parameters * provide the Android Java wrapper with the session parameters via the service pipe * receive the file descriptor to use as the tun/tap device from the Android Java wrapper via the service pipe Another thing to think about would be whether the tunnel could be reestablished after the device wakes up from sleep. Regards, James 

Hi Gert, On Wed, Feb 8, 2012 at 12:09 AM, Gert Doering <ge...@gr...> wrote: > Hi, > > On Wed, Feb 08, 2012 at 08:39:32AM +0100, Adriaan de Jong wrote: >>  - The ABI expects a two-stage setup process: set up a control >> channel first for negotiation, then a call VpnService.Builder with >> the proper routing, DNS, addresses, etc. As far as I've been told >> by other developers, this is not how OpenVPN currently works. > > Unless I misunderstand something, this is *exactly* how OpenVPN works > on the client side :-) - connect to server, figure out what we need to > configure, then call init_tun(), add_route() etc. to configure interface > and routing. > > Now, right now there is not a single "setup_stuff()" function but it's > spread across interface init, interface ifconfig, route addition, etc. - > but it shouldn't be too hard to build a set of "#ifdef ANDROID" functions > that just gather this information and then pass it in one go to the API. > >> - We would need the management interface to export the routing >> and network parameters that need to be set, so that a surrounding >> android app can call the appropriate Java methods. > > Which very much sounds like the --service-pipe stuff that Heiko is > building for Windows.  openvpn.exe runs as unprivileged process, and > when it wants to setup interfaces or routing, messages get sent via > the service pipe to the (new) openvpn service that does the privileged > operations. This sounds like a good way to go. I assume the service pipe can handle tearing down the tunnel as well. >>  - We would need a call in the management interface allowing the tun interface to be passed back into OpenVPN. > > Can't you just pass "--dev tun4" to OpenVPN?  Or do you need to pass a > file descriptor, handed to you by VpnService, instead of opening the tun > inside OpenVPN? > > Maybe I don't fully understand how that API should be working. Looks like you need to pass a native fd. OpenVPN would not be able to open the device itself. There looks to be a chicken and egg problem here though: the fd is returned by the VpnService.Builder.establish() method http://developer.android.com/reference/android/net/VpnService.Builder.html#establish() This needs to happen after a bunch of parameters are already known. Since OpenVPN would normally take care of negotiating these with the other end, it would seem that the tunnel fd is not available to OpenVPN in time to do this. Perhaps the user would have to configure the route, search domain, IP address and other parameters in advance of starting the VPN connection. That would suck. >> None of these problems are impossible to solve, but they would >> take some refactoring of the OpenVPN main and connection code. Any >> input would of course be appreciated! > > Heiko's new service stuff already solves large parts of this :-) > > ... and for the rest, well, we'd need a volunteer that wants to *work* on > this, not just ask for it...  I don't have an Android device (and no > time) so it wouldn't be me. I'm raising my hand. This path sounds better than what I thought would be necessary (writing OpenVPN client implementation in Java). I'm also asking around here at Google to see if somebody with more experience with Android is interested in helping out. I'm sure there is sufficient interest on both sides (Android and OpenVPN) to get something working. This would make a niche segment of Android users very happy I'm sure. > gert Regards, James 

Alon Bar-Lev wrote: > This is nice! > > Some questions: > > 1. Why not enable this automatically if openssl is sufficient version? > There is no point in not using this if available. > some distro's (notably RedHat) disable EC support by default; so it's not possible to check the OpenSSL version number. There is a '#define OPENSSL_NO_EC' that could be used, so the configure.ac script should check for that. As other SSL implementations might use a different #define to state that ECs are supported (or are NOT supported), I figure the decision whether to support ECs should be made by the configure.ac script, not by the openvpn code itself. > 2. I would have liked to see this in negotiation as well, so server > will use EC if supported by the client and fallback if not... This way > migration path can take place. > The client won't even enter the negotiation phase without this patch: if the client and server are configured to use ECDSA+SHA512 certs and the 'ecdh' parameters are NOT set on the server then the initial TLS handshake fails. cheers, JJK > On Tue, Feb 7, 2012 at 5:13 PM, Jan Just Keijser <ja...@ni... > <mailto:ja...@ni...>> wrote: > > Added support for Elliptic curves (ECDSA) + SHA2 family signed > certificates. > --- > init.c | 7 ++++ > options.c | 15 ++++++++++ > options.h | 6 ++++ > ssl.c | 3 ++ > ssl_backend.h | 10 ++++++ > ssl_openssl.c | 84 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > ssl_polarssl.c | 9 ++++++ > 7 files changed, 134 insertions(+), 0 deletions(-) > > diff --git a/init.c b/init.c > index 525f441..51b0d64 100644 > --- a/init.c > +++ b/init.c > @@ -895,6 +895,9 @@ print_openssl_info (const struct options *options) > if (options->show_ciphers || options->show_digests || > options->show_engines > #ifdef USE_SSL > || options->show_tls_ciphers > +#ifdef USE_SSL_EC > +|| options->show_curves > +#endif > #endif > ) > { > @@ -907,6 +910,10 @@ print_openssl_info (const struct options > *options) > #ifdef USE_SSL > if (options->show_tls_ciphers) > show_available_tls_ciphers (); > +#ifdef USE_SSL_EC > + if (options->show_curves) > + show_available_curves (); > +#endif > #endif > return true; > } > diff --git a/options.c b/options.c > index 6b8ae22..ce23dbc 100644 > --- a/options.c > +++ b/options.c > @@ -836,6 +836,9 @@ init_options (struct options *o, const bool > init_gc) > #ifdef ENABLE_X509ALTUSERNAME > o->x509_username_field = X509_USERNAME_FIELD_DEFAULT; > #endif > +#ifdef USE_SSL_EC > + o->curve_name = NULL; > +#endif > #endif /* USE_SSL */ > #endif /* USE_CRYPTO */ > #ifdef ENABLE_PKCS11 > @@ -6368,6 +6371,18 @@ add_option (struct options *options, > VERIFY_PERMISSION (OPT_P_GENERAL); > options->show_tls_ciphers = true; > } > +#ifdef USE_SSL_EC > + else if (streq (p[0], "show-curves")) > + { > + VERIFY_PERMISSION (OPT_P_GENERAL); > + options->show_curves = true; > + } > + else if (streq (p[0], "ecdh") && p[1]) > + { > + VERIFY_PERMISSION (OPT_P_CRYPTO); > + options->curve_name= p[1]; > + } > +#endif > else if (streq (p[0], "tls-server")) > { > VERIFY_PERMISSION (OPT_P_GENERAL); > diff --git a/options.h b/options.h > index 831d4f6..81e0757 100644 > --- a/options.h > +++ b/options.h > @@ -200,6 +200,9 @@ struct options > bool show_engines; > #ifdef USE_SSL > bool show_tls_ciphers; > +#ifdef USE_SSL_EC > + bool show_curves; > +#endif > #endif > bool genkey; > #endif > @@ -533,6 +536,9 @@ struct options > const char *priv_key_file; > const char *pkcs12_file; > const char *cipher_list; > +#ifdef USE_SSL_EC > + const char *curve_name; > +#endif > const char *tls_verify; > const char *tls_export_cert; > const char *tls_remote; > diff --git a/ssl.c b/ssl.c > index c26756e..54efe2f 100644 > --- a/ssl.c > +++ b/ssl.c > @@ -308,6 +308,9 @@ init_ssl (const struct options *options, > struct tls_root_ctx *new_ctx) > { > tls_ctx_server_new(new_ctx); > tls_ctx_load_dh_params(new_ctx, options->dh_file, > options->dh_file_inline); > +#ifdef USE_SSL_EC > + tls_ctx_load_ecdh_params(new_ctx, options->curve_name); > +#endif > } > else /* if client */ > { > diff --git a/ssl_backend.h b/ssl_backend.h > index 243c9e3..ebf9f36 100644 > --- a/ssl_backend.h > +++ b/ssl_backend.h > @@ -145,6 +145,16 @@ void tls_ctx_load_dh_params(struct > tls_root_ctx *ctx, const char *dh_file > ); > > /** > + * Load Elliptic Curve Parameters, and load them into the > library-specific > + * TLS context. > + * > + * @param ctx TLS context to use > + * @param curve_name The name of the elliptic curve to load. > + */ > +void tls_ctx_load_ecdh_params(struct tls_root_ctx *ctx, const > char *curve_name > + ); > + > +/** > * Load PKCS #12 file for key, cert and (optionally) CA certs, and > add to > * library-specific TLS context. > * > diff --git a/ssl_openssl.c b/ssl_openssl.c > index b95944c..912dd8f 100644 > --- a/ssl_openssl.c > +++ b/ssl_openssl.c > @@ -50,6 +50,9 @@ > #include <openssl/pkcs12.h> > #include <openssl/x509.h> > #include <openssl/crypto.h> > +#ifdef USE_SSL_EC > +#include <openssl/ec.h> > +#endif > > /* > * Allocate space in SSL objects in which to store a struct > tls_session > @@ -238,6 +241,46 @@ tls_ctx_load_dh_params (struct tls_root_ctx > *ctx, const char *dh_file > DH_free (dh); > } > > +void > +tls_ctx_load_ecdh_params (struct tls_root_ctx *ctx, const char > *curve_name > + ) > +{ > +#ifdef USE_SSL_EC > + if (curve_name != NULL) > + { > + int nid; > + EC_KEY *ecdh = NULL; > + > + nid = OBJ_sn2nid(curve_name); > + > + if (nid == 0) > + msg(M_SSLERR, "unknown curve name (%s)", curve_name); > + else > + { > + ecdh = EC_KEY_new_by_curve_name(nid); > + if (ecdh == NULL) > + msg (M_SSLERR, "Unable to create curve (%s)", curve_name); > + else > + { > + const char *sname; > + > + if (!SSL_CTX_set_tmp_ecdh(ctx->ctx, ecdh)) > + msg (M_SSLERR, "SSL_CTX_set_tmp_ecdh: cannot add curve"); > + > + /* Translate NID back to name , just for kicks */ > + sname = OBJ_nid2sn(nid); > + if (sname == NULL) sname = "(Unknown)"; > + msg (D_TLS_DEBUG_LOW, "ECDH curve %s added", sname); > + > + EC_KEY_free(ecdh); > + } > + } > + } > +#else > + msg(M_SSLERR, "Elliptic Curves not supported by this version of > OpenSSL"); > +#endif > +} > + > int > tls_ctx_load_pkcs12(struct tls_root_ctx *ctx, const char > *pkcs12_file, > #if ENABLE_INLINE_FILES > @@ -1273,6 +1316,47 @@ show_available_tls_ciphers () > SSL_CTX_free (ctx); > } > > +/* > + * * Show the Elliptic curves that are available for us to use > + * * in the OpenSSL library. > + * */ > +#ifdef USE_SSL_EC > +void > +show_available_curves() > +{ > + EC_builtin_curve *curves = NULL; > + size_t crv_len = 0; > + size_t n = 0; > + > + crv_len = EC_get_builtin_curves(NULL, 0); > + > + curves = OPENSSL_malloc((int)(sizeof(EC_builtin_curve) * crv_len)); > + > + if (curves == NULL) > + msg (M_SSLERR, "Cannot create EC_builtin_curve object"); > + else > + { > + if (EC_get_builtin_curves(curves, crv_len)) > + { > + printf ("Available Elliptic curves:\n"); > + for (n = 0; n < crv_len; n++) > + { > + const char *sname; > + sname = OBJ_nid2sn(curves[n].nid); > + if (sname == NULL) sname = ""; > + > + printf("%s\n", sname); > + } > + } > + else > + { > + msg (M_SSLERR, "Cannot get list of builtin curves"); > + } > + OPENSSL_free(curves); > + } > +} > +#endif > + > void > get_highest_preference_tls_cipher (char *buf, int size) > { > diff --git a/ssl_polarssl.c b/ssl_polarssl.c > index c50cf0a..a7a6d61 100644 > --- a/ssl_polarssl.c > +++ b/ssl_polarssl.c > @@ -218,6 +218,15 @@ else > (counter_type) 8 * mpi_size(&ctx->dhm_ctx->P)); > } > > +#ifdef USE_SSL_EC > +void > +tls_ctx_load_ecdh_params (struct tls_root_ctx *ctx, const char > *curve_name > + ) > +{ > + msg(M_WARN, "Elliptic Curves not yet supported by PolarSSL"); > +} > +#endif > + > int > tls_ctx_load_pkcs12(struct tls_root_ctx *ctx, const char > *pkcs12_file, > #if ENABLE_INLINE_FILES > -- > 1.7.4.4 > 

Hi, On Wed, Feb 08, 2012 at 08:39:32AM +0100, Adriaan de Jong wrote: > - The ABI expects a two-stage setup process: set up a control > channel first for negotiation, then a call VpnService.Builder with > the proper routing, DNS, addresses, etc. As far as I've been told > by other developers, this is not how OpenVPN currently works. Unless I misunderstand something, this is *exactly* how OpenVPN works on the client side :-) - connect to server, figure out what we need to configure, then call init_tun(), add_route() etc. to configure interface and routing. Now, right now there is not a single "setup_stuff()" function but it's spread across interface init, interface ifconfig, route addition, etc. - but it shouldn't be too hard to build a set of "#ifdef ANDROID" functions that just gather this information and then pass it in one go to the API. > - We would need the management interface to export the routing > and network parameters that need to be set, so that a surrounding > android app can call the appropriate Java methods. Which very much sounds like the --service-pipe stuff that Heiko is building for Windows. openvpn.exe runs as unprivileged process, and when it wants to setup interfaces or routing, messages get sent via the service pipe to the (new) openvpn service that does the privileged operations. > - We would need a call in the management interface allowing the tun interface to be passed back into OpenVPN. Can't you just pass "--dev tun4" to OpenVPN? Or do you need to pass a file descriptor, handed to you by VpnService, instead of opening the tun inside OpenVPN? Maybe I don't fully understand how that API should be working. > None of these problems are impossible to solve, but they would > take some refactoring of the OpenVPN main and connection code. Any > input would of course be appreciated! Heiko's new service stuff already solves large parts of this :-) ... and for the rest, well, we'd need a volunteer that wants to *work* on this, not just ask for it... I don't have an Android device (and no time) so it wouldn't be me. gert -- USENET is *not* the non-clickable part of WWW! //www.muc.de/~gert/ Gert Doering - Munich, Germany ge...@gr... fax: +49-89-35655025 ge...@ne... 

> -----Original Message----- > From: James Ring [mailto:sj...@jd...] > Sent: dinsdag 7 februari 2012 23:33 > To: ope...@li... > Subject: [Openvpn-devel] OpenVPN and Android 4.0 VPN API > > Hi there, > > I was just wondering if anybody has seen the new Android 4.0 VPN API. > It looks like a promising way to have a native Java OpenVPN solution on > Android. > > http://developer.android.com/reference/android/net/VpnService.html > > There is a lot of interest in having an Android implementation of > OpenVPN that does not require rooting the phone. > > Thanks, > James Hi James, I had a look at it a few months ago, and it looks really promising. It uses a tun device at its base. Network settings can be passed to There are a few tricky bits though: - The ABI expects a two-stage setup process: set up a control channel first for negotiation, then a call VpnService.Builder with the proper routing, DNS, addresses, etc. As far as I've been told by other developers, this is not how OpenVPN currently works. - We would need the management interface to export the routing and network parameters that need to be set, so that a surrounding android app can call the appropriate Java methods. - We would need a call in the management interface allowing the tun interface to be passed back into OpenVPN. None of these problems are impossible to solve, but they would take some refactoring of the OpenVPN main and connection code. Any input would of course be appreciated! Adriaan 

Hi there, I was just wondering if anybody has seen the new Android 4.0 VPN API. It looks like a promising way to have a native Java OpenVPN solution on Android. http://developer.android.com/reference/android/net/VpnService.html There is a lot of interest in having an Android implementation of OpenVPN that does not require rooting the phone. Thanks, James 

This is nice! Some questions: 1. Why not enable this automatically if openssl is sufficient version? There is no point in not using this if available. 2. I would have liked to see this in negotiation as well, so server will use EC if supported by the client and fallback if not... This way migration path can take place. Alon. On Tue, Feb 7, 2012 at 5:13 PM, Jan Just Keijser <ja...@ni...> wrote: > Added support for Elliptic curves (ECDSA) + SHA2 family signed > certificates. > --- > init.c | 7 ++++ > options.c | 15 ++++++++++ > options.h | 6 ++++ > ssl.c | 3 ++ > ssl_backend.h | 10 ++++++ > ssl_openssl.c | 84 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > ssl_polarssl.c | 9 ++++++ > 7 files changed, 134 insertions(+), 0 deletions(-) > > diff --git a/init.c b/init.c > index 525f441..51b0d64 100644 > --- a/init.c > +++ b/init.c > @@ -895,6 +895,9 @@ print_openssl_info (const struct options *options) > if (options->show_ciphers || options->show_digests || > options->show_engines > #ifdef USE_SSL > || options->show_tls_ciphers > +#ifdef USE_SSL_EC > +|| options->show_curves > +#endif > #endif > ) > { > @@ -907,6 +910,10 @@ print_openssl_info (const struct options *options) > #ifdef USE_SSL > if (options->show_tls_ciphers) > show_available_tls_ciphers (); > +#ifdef USE_SSL_EC > + if (options->show_curves) > + show_available_curves (); > +#endif > #endif > return true; > } > diff --git a/options.c b/options.c > index 6b8ae22..ce23dbc 100644 > --- a/options.c > +++ b/options.c > @@ -836,6 +836,9 @@ init_options (struct options *o, const bool init_gc) > #ifdef ENABLE_X509ALTUSERNAME > o->x509_username_field = X509_USERNAME_FIELD_DEFAULT; > #endif > +#ifdef USE_SSL_EC > + o->curve_name = NULL; > +#endif > #endif /* USE_SSL */ > #endif /* USE_CRYPTO */ > #ifdef ENABLE_PKCS11 > @@ -6368,6 +6371,18 @@ add_option (struct options *options, > VERIFY_PERMISSION (OPT_P_GENERAL); > options->show_tls_ciphers = true; > } > +#ifdef USE_SSL_EC > + else if (streq (p[0], "show-curves")) > + { > + VERIFY_PERMISSION (OPT_P_GENERAL); > + options->show_curves = true; > + } > + else if (streq (p[0], "ecdh") && p[1]) > + { > + VERIFY_PERMISSION (OPT_P_CRYPTO); > + options->curve_name= p[1]; > + } > +#endif > else if (streq (p[0], "tls-server")) > { > VERIFY_PERMISSION (OPT_P_GENERAL); > diff --git a/options.h b/options.h > index 831d4f6..81e0757 100644 > --- a/options.h > +++ b/options.h > @@ -200,6 +200,9 @@ struct options > bool show_engines; > #ifdef USE_SSL > bool show_tls_ciphers; > +#ifdef USE_SSL_EC > + bool show_curves; > +#endif > #endif > bool genkey; > #endif > @@ -533,6 +536,9 @@ struct options > const char *priv_key_file; > const char *pkcs12_file; > const char *cipher_list; > +#ifdef USE_SSL_EC > + const char *curve_name; > +#endif > const char *tls_verify; > const char *tls_export_cert; > const char *tls_remote; > diff --git a/ssl.c b/ssl.c > index c26756e..54efe2f 100644 > --- a/ssl.c > +++ b/ssl.c > @@ -308,6 +308,9 @@ init_ssl (const struct options *options, struct > tls_root_ctx *new_ctx) > { > tls_ctx_server_new(new_ctx); > tls_ctx_load_dh_params(new_ctx, options->dh_file, > options->dh_file_inline); > +#ifdef USE_SSL_EC > + tls_ctx_load_ecdh_params(new_ctx, options->curve_name); > +#endif > } > else /* if client */ > { > diff --git a/ssl_backend.h b/ssl_backend.h > index 243c9e3..ebf9f36 100644 > --- a/ssl_backend.h > +++ b/ssl_backend.h > @@ -145,6 +145,16 @@ void tls_ctx_load_dh_params(struct tls_root_ctx *ctx, > const char *dh_file > ); > > /** > + * Load Elliptic Curve Parameters, and load them into the library-specific > + * TLS context. > + * > + * @param ctx TLS context to use > + * @param curve_name The name of the elliptic curve to load. > + */ > +void tls_ctx_load_ecdh_params(struct tls_root_ctx *ctx, const char > *curve_name > + ); > + > +/** > * Load PKCS #12 file for key, cert and (optionally) CA certs, and add to > * library-specific TLS context. > * > diff --git a/ssl_openssl.c b/ssl_openssl.c > index b95944c..912dd8f 100644 > --- a/ssl_openssl.c > +++ b/ssl_openssl.c > @@ -50,6 +50,9 @@ > #include <openssl/pkcs12.h> > #include <openssl/x509.h> > #include <openssl/crypto.h> > +#ifdef USE_SSL_EC > +#include <openssl/ec.h> > +#endif > > /* > * Allocate space in SSL objects in which to store a struct tls_session > @@ -238,6 +241,46 @@ tls_ctx_load_dh_params (struct tls_root_ctx *ctx, > const char *dh_file > DH_free (dh); > } > > +void > +tls_ctx_load_ecdh_params (struct tls_root_ctx *ctx, const char *curve_name > + ) > +{ > +#ifdef USE_SSL_EC > + if (curve_name != NULL) > + { > + int nid; > + EC_KEY *ecdh = NULL; > + > + nid = OBJ_sn2nid(curve_name); > + > + if (nid == 0) > + msg(M_SSLERR, "unknown curve name (%s)", curve_name); > + else > + { > + ecdh = EC_KEY_new_by_curve_name(nid); > + if (ecdh == NULL) > + msg (M_SSLERR, "Unable to create curve (%s)", curve_name); > + else > + { > + const char *sname; > + > + if (!SSL_CTX_set_tmp_ecdh(ctx->ctx, ecdh)) > + msg (M_SSLERR, "SSL_CTX_set_tmp_ecdh: cannot add curve"); > + > + /* Translate NID back to name , just for kicks */ > + sname = OBJ_nid2sn(nid); > + if (sname == NULL) sname = "(Unknown)"; > + msg (D_TLS_DEBUG_LOW, "ECDH curve %s added", sname); > + > + EC_KEY_free(ecdh); > + } > + } > + } > +#else > + msg(M_SSLERR, "Elliptic Curves not supported by this version of > OpenSSL"); > +#endif > +} > + > int > tls_ctx_load_pkcs12(struct tls_root_ctx *ctx, const char *pkcs12_file, > #if ENABLE_INLINE_FILES > @@ -1273,6 +1316,47 @@ show_available_tls_ciphers () > SSL_CTX_free (ctx); > } > > +/* > + * * Show the Elliptic curves that are available for us to use > + * * in the OpenSSL library. > + * */ > +#ifdef USE_SSL_EC > +void > +show_available_curves() > +{ > + EC_builtin_curve *curves = NULL; > + size_t crv_len = 0; > + size_t n = 0; > + > + crv_len = EC_get_builtin_curves(NULL, 0); > + > + curves = OPENSSL_malloc((int)(sizeof(EC_builtin_curve) * crv_len)); > + > + if (curves == NULL) > + msg (M_SSLERR, "Cannot create EC_builtin_curve object"); > + else > + { > + if (EC_get_builtin_curves(curves, crv_len)) > + { > + printf ("Available Elliptic curves:\n"); > + for (n = 0; n < crv_len; n++) > + { > + const char *sname; > + sname = OBJ_nid2sn(curves[n].nid); > + if (sname == NULL) sname = ""; > + > + printf("%s\n", sname); > + } > + } > + else > + { > + msg (M_SSLERR, "Cannot get list of builtin curves"); > + } > + OPENSSL_free(curves); > + } > +} > +#endif > + > void > get_highest_preference_tls_cipher (char *buf, int size) > { > diff --git a/ssl_polarssl.c b/ssl_polarssl.c > index c50cf0a..a7a6d61 100644 > --- a/ssl_polarssl.c > +++ b/ssl_polarssl.c > @@ -218,6 +218,15 @@ else > (counter_type) 8 * mpi_size(&ctx->dhm_ctx->P)); > } > > +#ifdef USE_SSL_EC > +void > +tls_ctx_load_ecdh_params (struct tls_root_ctx *ctx, const char *curve_name > + ) > +{ > + msg(M_WARN, "Elliptic Curves not yet supported by PolarSSL"); > +} > +#endif > + > int > tls_ctx_load_pkcs12(struct tls_root_ctx *ctx, const char *pkcs12_file, > #if ENABLE_INLINE_FILES > -- > 1.7.4.4 > > > > ------------------------------------------------------------------------------ > Keep Your Developer Skills Current with LearnDevNow! > The most comprehensive online learning library for Microsoft developers > is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, > Metro Style Apps, more. Free future releases when you subscribe now! > http://p.sf.net/sfu/learndevnow-d2d > _______________________________________________ > Openvpn-devel mailing list > Ope...@li... > https://lists.sourceforge.net/lists/listinfo/openvpn-devel > 

sorry about the noise, folks; this was my second git patch attempt :) cheers, JJK Jan Just Keijser wrote: > Made some options connection-entry specific: > fragment > mssfix > tun-mtu > tun-mtu-extra > link-mtu > mtu_discover_type > explicit-exit-notification > in order to support stuff like > <connection> > remote host > proto udp > fragment > explicit-exit-notification 3 > </connection> > <connection> > remote host > proto tcp > </connection> > > Signed-off-by: Jan Just Keijser <ja...@ni...> > --- > forward.c | 2 +- > init.c | 38 ++++++++++--------- > occ.c | 2 +- > options.c | 125 +++++++++++++++++++++++++++++++------------------------------ > options.h | 36 +++++++++--------- > sig.c | 6 +- > 6 files changed, 107 insertions(+), 102 deletions(-) > > diff --git a/forward.c b/forward.c > index dfef4ef..96c6b9a 100644 > --- a/forward.c > +++ b/forward.c > @@ -1005,7 +1005,7 @@ process_incoming_tun (struct context *c) > void > process_ipv4_header (struct context *c, unsigned int flags, struct buffer *buf) > { > - if (!c->options.mssfix) > + if (!c->options.ce.mssfix) > flags &= ~PIPV4_MSSFIX; > #if PASSTOS_CAPABILITY > if (!c->options.passtos) > diff --git a/init.c b/init.c > index 51b0d64..8f42120 100644 > --- a/init.c > +++ b/init.c > @@ -1786,10 +1786,10 @@ do_deferred_options (struct context *c, const unsigned int found) > #ifdef ENABLE_OCC > if (found & OPT_P_EXPLICIT_NOTIFY) > { > - if (!proto_is_udp(c->options.ce.proto) && c->options.explicit_exit_notification) > + if (!proto_is_udp(c->options.ce.proto) && c->options.ce.explicit_exit_notification) >	{ > msg (D_PUSH, "OPTIONS IMPORT: --explicit-exit-notify can only be used with --proto udp"); > - c->options.explicit_exit_notification = 0; > + c->options.ce.explicit_exit_notification = 0; >	} > else >	msg (D_PUSH, "OPTIONS IMPORT: explicit notify parm(s) modified"); > @@ -1962,10 +1962,10 @@ frame_finalize_options (struct context *c, const struct options *o) > } > > frame_finalize (&c->c2.frame, > - o->link_mtu_defined, > - o->link_mtu, > - o->tun_mtu_defined, > - o->tun_mtu); > + o->ce.link_mtu_defined, > + o->ce.link_mtu, > + o->ce.tun_mtu_defined, > + o->ce.tun_mtu); > } > > /* > @@ -2411,8 +2411,8 @@ do_init_frame (struct context *c) > /* > * Adjust frame size based on the --tun-mtu-extra parameter. > */ > - if (c->options.tun_mtu_extra_defined) > - tun_adjust_frame_parameters (&c->c2.frame, c->options.tun_mtu_extra); > + if (c->options.ce.tun_mtu_extra_defined) > + tun_adjust_frame_parameters (&c->c2.frame, c->options.ce.tun_mtu_extra); > > /* > * Adjust frame size based on link socket parameters. > @@ -2441,13 +2441,13 @@ do_init_frame (struct context *c) > /* > * MTU advisories > */ > - if (c->options.fragment && c->options.mtu_test) > + if (c->options.ce.fragment && c->options.mtu_test) > msg (M_WARN, > "WARNING: using --fragment and --mtu-test together may produce an inaccurate MTU test result"); > #endif > > #ifdef ENABLE_FRAGMENT > - if ((c->options.mssfix || c->options.fragment) > + if ((c->options.ce.mssfix || c->options.ce.fragment) > && TUN_MTU_SIZE (&c->c2.frame_fragment) != ETHERNET_MTU) > msg (M_WARN, > "WARNING: normally if you use --mssfix and/or --fragment, you should also set --tun-mtu %d (currently it is %d)", > @@ -2619,9 +2619,9 @@ do_init_buffers (struct context *c) > static void > do_init_fragment (struct context *c) > { > - ASSERT (c->options.fragment); > + ASSERT (c->options.ce.fragment); > frame_set_mtu_dynamic (&c->c2.frame_fragment, > - c->options.fragment, SET_MTU_UPPER_BOUND); > + c->options.ce.fragment, SET_MTU_UPPER_BOUND); > fragment_frame_init (c->c2.fragment, &c->c2.frame_fragment); > } > #endif > @@ -2632,10 +2632,10 @@ do_init_fragment (struct context *c) > static void > do_init_mssfix (struct context *c) > { > - if (c->options.mssfix) > + if (c->options.ce.mssfix) > { > frame_set_mtu_dynamic (&c->c2.frame, > - c->options.mssfix, SET_MTU_UPPER_BOUND); > + c->options.ce.mssfix, SET_MTU_UPPER_BOUND); > } > } > > @@ -2691,7 +2691,7 @@ do_init_socket_1 (struct context *c, const int mode) > c->options.ce.connect_retry_seconds, > c->options.ce.connect_timeout, > c->options.ce.connect_retry_max, > - c->options.mtu_discover_type, > + c->options.ce.mtu_discover_type, > c->options.rcvbuf, > c->options.sndbuf, > c->options.mark, > @@ -3406,7 +3408,7 @@ init_instance (struct context *c, const struct env_set *env, const unsigned int > > #ifdef ENABLE_FRAGMENT > /* initialize internal fragmentation object */ > - if (options->fragment && (c->mode == CM_P2P || child)) > + if (options->ce.fragment && (c->mode == CM_P2P || child)) > c->c2.fragment = fragment_init (&c->c2.frame); > #endif > > @@ -3442,7 +3444,7 @@ init_instance (struct context *c, const struct env_set *env, const unsigned int > > #ifdef ENABLE_FRAGMENT > /* initialize internal fragmentation capability with known frame size */ > - if (options->fragment && (c->mode == CM_P2P || child)) > + if (options->ce.fragment && (c->mode == CM_P2P || child)) > do_init_fragment (c); > #endif > > diff --git a/occ.c b/occ.c > index bcf91cc..2fdbff0 100644 > --- a/occ.c > +++ b/occ.c > @@ -368,7 +368,7 @@ process_received_occ_msg (struct context *c) > c->c2.max_recv_size_remote, > c->c2.max_send_size_remote, > c->c2.max_recv_size_local); > - if (!c->options.fragment > + if (!c->options.ce.fragment > && (proto_is_dgram(c->options.ce.proto)) > && c->c2.max_send_size_local > TUN_MTU_MIN > && (c->c2.max_recv_size_remote < c->c2.max_send_size_local > diff --git a/options.c b/options.c > index ce23dbc..df8dc91 100644 > --- a/options.c > +++ b/options.c > @@ -765,10 +765,10 @@ init_options (struct options *o, const bool init_gc) > o->status_file_update_freq = 60; > o->status_file_version = 1; > o->ce.bind_local = true; > - o->tun_mtu = TUN_MTU_DEFAULT; > - o->link_mtu = LINK_MTU_DEFAULT; > - o->mtu_discover_type = -1; > - o->mssfix = MSSFIX_DEFAULT; > + o->ce.tun_mtu = TUN_MTU_DEFAULT; > + o->ce.link_mtu = LINK_MTU_DEFAULT; > + o->ce.mtu_discover_type = -1; > + o->ce.mssfix = MSSFIX_DEFAULT; > o->route_delay_window = 30; > o->max_routes = MAX_ROUTES_DEFAULT; > o->resolve_retry_seconds = RESOLV_RETRY_INFINITE; > @@ -1361,8 +1361,26 @@ show_connection_entry (const struct connection_entry *o) > SHOW_INT (socks_proxy_port); > SHOW_BOOL (socks_proxy_retry); > #endif > + SHOW_INT (tun_mtu); > + SHOW_BOOL (tun_mtu_defined); > + SHOW_INT (link_mtu); > + SHOW_BOOL (link_mtu_defined); > + SHOW_INT (tun_mtu_extra); > + SHOW_BOOL (tun_mtu_extra_defined); > + > + SHOW_INT (mtu_discover_type); > + > +#ifdef ENABLE_FRAGMENT > + SHOW_INT (fragment); > +#endif > + SHOW_INT (mssfix); > + > +#ifdef ENABLE_OCC > + SHOW_INT (explicit_exit_notification); > +#endif > } > > + > static void > show_connection_entries (const struct options *o) > { > @@ -1433,19 +1451,6 @@ show_settings (const struct options *o) > #ifdef HAVE_GETTIMEOFDAY > SHOW_INT (shaper); > #endif > - SHOW_INT (tun_mtu); > - SHOW_BOOL (tun_mtu_defined); > - SHOW_INT (link_mtu); > - SHOW_BOOL (link_mtu_defined); > - SHOW_INT (tun_mtu_extra); > - SHOW_BOOL (tun_mtu_extra_defined); > - > -#ifdef ENABLE_FRAGMENT > - SHOW_INT (fragment); > -#endif > - > - SHOW_INT (mtu_discover_type); > - > #ifdef ENABLE_OCC > SHOW_INT (mtu_test); > #endif > @@ -1460,16 +1465,11 @@ show_settings (const struct options *o) > SHOW_INT (ping_rec_timeout_action); > SHOW_BOOL (ping_timer_remote); > SHOW_INT (remap_sigusr1); > -#ifdef ENABLE_OCC > - SHOW_INT (explicit_exit_notification); > -#endif > SHOW_BOOL (persist_tun); > SHOW_BOOL (persist_local_ip); > SHOW_BOOL (persist_remote_ip); > SHOW_BOOL (persist_key); > > - SHOW_INT (mssfix); > - > #if PASSTOS_CAPABILITY > SHOW_BOOL (passtos); > #endif > @@ -1959,7 +1959,7 @@ options_postprocess_verify_ce (const struct options *options, const struct conne > /* > * Sanity check on MTU parameters > */ > - if (options->tun_mtu_defined && options->link_mtu_defined) > + if (options->ce.tun_mtu_defined && options->ce.link_mtu_defined) > msg (M_USAGE, "only one of --tun-mtu or --link-mtu may be defined (note that --ifconfig implies --link-mtu %d)", LINK_MTU_DEFAULT); > > #ifdef ENABLE_OCC > @@ -2042,12 +2042,12 @@ options_postprocess_verify_ce (const struct options *options, const struct conne > */ > > #ifdef ENABLE_FRAGMENT > - if (!proto_is_udp(ce->proto) && options->fragment) > + if (!proto_is_udp(ce->proto) && ce->fragment) > msg (M_USAGE, "--fragment can only be used with --proto udp"); > #endif > > #ifdef ENABLE_OCC > - if (!proto_is_udp(ce->proto) && options->explicit_exit_notification) > + if (!proto_is_udp(ce->proto) && ce->explicit_exit_notification) > msg (M_USAGE, "--explicit-exit-notify can only be used with --proto udp"); > #endif > > @@ -2132,7 +2132,7 @@ options_postprocess_verify_ce (const struct options *options, const struct conne > if (!(dev == DEV_TYPE_TAP || (dev == DEV_TYPE_TUN && options->topology == TOP_SUBNET)) && options->ifconfig_pool_netmask) >	msg (M_USAGE, "The third parameter to --ifconfig-pool (netmask) is only valid in --dev tap mode"); > #ifdef ENABLE_OCC > - if (options->explicit_exit_notification) > + if (ce->explicit_exit_notification) >	msg (M_USAGE, "--explicit-exit-notify cannot be used with --mode server"); > #endif > if (options->routes && (options->routes->flags & RG_ENABLE)) > @@ -2427,27 +2427,28 @@ options_postprocess_mutate_ce (struct options *o, struct connection_entry *ce) > /* if protocol forcing is enabled, disable all protocols except for the forced one */ > if (o->proto_force >= 0 && proto_is_tcp(o->proto_force) != proto_is_tcp(ce->proto)) > ce->flags |= CE_DISABLED; > -} > - > -static void > -options_postprocess_mutate_invariant (struct options *options) > -{ > - const int dev = dev_type_enum (options->dev, options->dev_type); > > /* > * If --mssfix is supplied without a parameter, default > * it to --fragment value, if --fragment is specified. > */ > - if (options->mssfix_default) > + if (o->ce.mssfix_default) > { > #ifdef ENABLE_FRAGMENT > - if (options->fragment) > -	options->mssfix = options->fragment; > + if (ce->fragment) > +	o->ce.mssfix = ce->fragment; > #else > msg (M_USAGE, "--mssfix must specify a parameter"); > #endif > } > > +} > + > +static void > +options_postprocess_mutate_invariant (struct options *options) > +{ > + const int dev = dev_type_enum (options->dev, options->dev_type); > + > /* > * In forking TCP server mode, you don't need to ifconfig > * the tap device (the assumption is that it will be bridged). > @@ -2459,14 +2460,14 @@ options_postprocess_mutate_invariant (struct options *options) > * Set MTU defaults > */ > { > - if (!options->tun_mtu_defined && !options->link_mtu_defined) > + if (!options->ce.tun_mtu_defined && !options->ce.link_mtu_defined) > { > -	options->tun_mtu_defined = true; > +	options->ce.tun_mtu_defined = true; > } > - if ((dev == DEV_TYPE_TAP) && !options->tun_mtu_extra_defined) > + if ((dev == DEV_TYPE_TAP) && !options->ce.tun_mtu_extra_defined) > { > -	options->tun_mtu_extra_defined = true; > -	options->tun_mtu_extra = TAP_MTU_EXTRA_DEFAULT; > +	options->ce.tun_mtu_extra_defined = true; > +	options->ce.tun_mtu_extra = TAP_MTU_EXTRA_DEFAULT; > } > } > > @@ -2952,7 +2953,7 @@ options_string (const struct options *o, > #endif > > #ifdef ENABLE_FRAGMENT > - if (o->fragment) > + if (o->ce.fragment) > buf_printf (&out, ",mtu-dynamic"); > #endif > > @@ -4696,39 +4697,40 @@ add_option (struct options *options, > } > else if ((streq (p[0], "link-mtu") || streq (p[0], "udp-mtu")) && p[1]) > { > - VERIFY_PERMISSION (OPT_P_MTU); > - options->link_mtu = positive_atoi (p[1]); > - options->link_mtu_defined = true; > + VERIFY_PERMISSION (OPT_P_MTU|OPT_P_CONNECTION); > + options->ce.link_mtu = positive_atoi (p[1]); > + options->ce.link_mtu_defined = true; > } > else if (streq (p[0], "tun-mtu") && p[1]) > { > - VERIFY_PERMISSION (OPT_P_MTU); > - options->tun_mtu = positive_atoi (p[1]); > - options->tun_mtu_defined = true; > + VERIFY_PERMISSION (OPT_P_MTU|OPT_P_CONNECTION); > + options->ce.tun_mtu = positive_atoi (p[1]); > + options->ce.tun_mtu_defined = true; > } > else if (streq (p[0], "tun-mtu-extra") && p[1]) > { > - VERIFY_PERMISSION (OPT_P_MTU); > - options->tun_mtu_extra = positive_atoi (p[1]); > - options->tun_mtu_extra_defined = true; > + VERIFY_PERMISSION (OPT_P_MTU|OPT_P_CONNECTION); > + options->ce.tun_mtu_extra = positive_atoi (p[1]); > + options->ce.tun_mtu_extra_defined = true; > } > #ifdef ENABLE_FRAGMENT > else if (streq (p[0], "mtu-dynamic")) > { > - VERIFY_PERMISSION (OPT_P_GENERAL); > + VERIFY_PERMISSION (OPT_P_MTU|OPT_P_CONNECTION); > msg (msglevel, "--mtu-dynamic has been replaced by --fragment"); > goto err; > } > else if (streq (p[0], "fragment") && p[1]) > { > - VERIFY_PERMISSION (OPT_P_MTU); > - options->fragment = positive_atoi (p[1]); > +// VERIFY_PERMISSION (OPT_P_MTU); > + VERIFY_PERMISSION (OPT_P_MTU|OPT_P_CONNECTION); > + options->ce.fragment = positive_atoi (p[1]); > } > #endif > else if (streq (p[0], "mtu-disc") && p[1]) > { > - VERIFY_PERMISSION (OPT_P_MTU); > - options->mtu_discover_type = translate_mtu_discover_type_name (p[1]); > + VERIFY_PERMISSION (OPT_P_MTU|OPT_P_CONNECTION); > + options->ce.mtu_discover_type = translate_mtu_discover_type_name (p[1]); > } > #ifdef ENABLE_OCC > else if (streq (p[0], "mtu-test")) > @@ -5073,14 +5075,15 @@ add_option (struct options *options, > #ifdef ENABLE_OCC > else if (streq (p[0], "explicit-exit-notify")) > { > - VERIFY_PERMISSION (OPT_P_EXPLICIT_NOTIFY); > + VERIFY_PERMISSION (OPT_P_GENERAL|OPT_P_CONNECTION); > +// VERIFY_PERMISSION (OPT_P_EXPLICIT_NOTIFY); > if (p[1]) >	{ > - options->explicit_exit_notification = positive_atoi (p[1]); > + options->ce.explicit_exit_notification = positive_atoi (p[1]); >	} > else >	{ > - options->explicit_exit_notification = 1; > + options->ce.explicit_exit_notification = 1; >	} > } > #endif > @@ -5336,13 +5339,13 @@ add_option (struct options *options, > } > else if (streq (p[0], "mssfix")) > { > - VERIFY_PERMISSION (OPT_P_GENERAL); > + VERIFY_PERMISSION (OPT_P_GENERAL|OPT_P_CONNECTION); > if (p[1]) >	{ > - options->mssfix = positive_atoi (p[1]); > + options->ce.mssfix = positive_atoi (p[1]); >	} > else > -	options->mssfix_default = true; > +	options->ce.mssfix_default = true; > > } > #ifdef ENABLE_OCC > diff --git a/options.h b/options.h > index 81e0757..0f9d260 100644 > --- a/options.h > +++ b/options.h > @@ -111,6 +111,24 @@ struct connection_entry > bool socks_proxy_retry; > #endif > > + int tun_mtu; /* MTU of tun device */ > + bool tun_mtu_defined; /* true if user overriding parm with command line option */ > + int tun_mtu_extra; > + bool tun_mtu_extra_defined; > + int link_mtu; /* MTU of device over which tunnel packets pass via TCP/UDP */ > + bool link_mtu_defined; /* true if user overriding parm with command line option */ > + > + /* Advanced MTU negotiation and datagram fragmentation options */ > + int mtu_discover_type; /* used if OS supports setting Path MTU discovery options on socket */ > + > + int fragment; /* internal fragmentation size */ > + int mssfix; /* Upper bound on TCP MSS */ > + bool mssfix_default; /* true if --mssfix was supplied without a parameter */ > + > +#ifdef ENABLE_OCC > + int explicit_exit_notification; /* Explicitly tell peer when we are exiting via OCC_EXIT message */ > +#endif > + > # define CE_DISABLED (1<<0) > #if HTTP_PROXY_FALLBACK > # define CE_HTTP_PROXY_FALLBACK (1<<1) > @@ -248,24 +266,13 @@ struct options > #ifdef HAVE_GETTIMEOFDAY > int shaper; > #endif > - int tun_mtu; /* MTU of tun device */ > - int tun_mtu_extra; > - bool tun_mtu_extra_defined; > - int link_mtu; /* MTU of device over which tunnel packets pass via TCP/UDP */ > - bool tun_mtu_defined; /* true if user overriding parm with command line option */ > - bool link_mtu_defined; /* true if user overriding parm with command line option */ > > int proto_force; > > - /* Advanced MTU negotiation and datagram fragmentation options */ > - int mtu_discover_type; /* used if OS supports setting Path MTU discovery options on socket */ > - > #ifdef ENABLE_OCC > bool mtu_test; > #endif > > - int fragment; /* internal fragmentation size */ > - > #ifdef ENABLE_MEMSTATS > char *memstats_fn; > #endif > @@ -288,18 +295,11 @@ struct options > # define PING_RESTART 2 > int ping_rec_timeout_action; /* What action to take on ping_rec_timeout (exit or restart)? */ > > -#ifdef ENABLE_OCC > - int explicit_exit_notification; /* Explicitly tell peer when we are exiting via OCC_EXIT message */ > -#endif > - > bool persist_tun; /* Don't close/reopen TUN/TAP dev on SIGUSR1 or PING_RESTART */ > bool persist_local_ip; /* Don't re-resolve local address on SIGUSR1 or PING_RESTART */ > bool persist_remote_ip; /* Don't re-resolve remote address on SIGUSR1 or PING_RESTART */ > bool persist_key; /* Don't re-read key files on SIGUSR1 or PING_RESTART */ > > - int mssfix; /* Upper bound on TCP MSS */ > - bool mssfix_default; /* true if --mssfix was supplied without a parameter */ > - > #if PASSTOS_CAPABILITY > bool passtos; > #endif > diff --git a/sig.c b/sig.c > index d73525f..a5703f0 100644 > --- a/sig.c > +++ b/sig.c > @@ -300,8 +300,8 @@ process_explicit_exit_notification_timer_wakeup (struct context *c) > &c->c2.timeval, > ETT_DEFAULT)) > { > - ASSERT (c->c2.explicit_exit_notification_time_wait && c->options.explicit_exit_notification); > - if (now >= c->c2.explicit_exit_notification_time_wait + c->options.explicit_exit_notification) > + ASSERT (c->c2.explicit_exit_notification_time_wait && c->options.ce.explicit_exit_notification); > + if (now >= c->c2.explicit_exit_notification_time_wait + c->options.ce.explicit_exit_notification) >	{ > event_timeout_clear (&c->c2.explicit_exit_notification_interval); > c->sig->signal_received = SIGTERM; > @@ -340,7 +340,7 @@ process_sigterm (struct context *c) > { > bool ret = true; > #ifdef ENABLE_OCC > - if (c->options.explicit_exit_notification > + if (c->options.ce.explicit_exit_notification > && !c->c2.explicit_exit_notification_time_wait) > { > process_explicit_exit_notification_init (c); > 

Made some options connection-entry specific: fragment mssfix tun-mtu tun-mtu-extra link-mtu mtu_discover_type explicit-exit-notification in order to support stuff like <connection> remote host proto udp fragment explicit-exit-notification 3 </connection> <connection> remote host proto tcp </connection> Signed-off-by: Jan Just Keijser <ja...@ni...> --- forward.c | 2 +- init.c | 38 ++++++++++--------- occ.c | 2 +- options.c | 125 +++++++++++++++++++++++++++++++------------------------------ options.h | 36 +++++++++--------- sig.c | 6 +- 6 files changed, 107 insertions(+), 102 deletions(-) diff --git a/forward.c b/forward.c index dfef4ef..96c6b9a 100644 --- a/forward.c +++ b/forward.c @@ -1005,7 +1005,7 @@ process_incoming_tun (struct context *c) void process_ipv4_header (struct context *c, unsigned int flags, struct buffer *buf) { - if (!c->options.mssfix) + if (!c->options.ce.mssfix) flags &= ~PIPV4_MSSFIX; #if PASSTOS_CAPABILITY if (!c->options.passtos) diff --git a/init.c b/init.c index 51b0d64..8f42120 100644 --- a/init.c +++ b/init.c @@ -1786,10 +1786,10 @@ do_deferred_options (struct context *c, const unsigned int found) #ifdef ENABLE_OCC if (found & OPT_P_EXPLICIT_NOTIFY) { - if (!proto_is_udp(c->options.ce.proto) && c->options.explicit_exit_notification) + if (!proto_is_udp(c->options.ce.proto) && c->options.ce.explicit_exit_notification)	{ msg (D_PUSH, "OPTIONS IMPORT: --explicit-exit-notify can only be used with --proto udp"); - c->options.explicit_exit_notification = 0; + c->options.ce.explicit_exit_notification = 0;	} else	msg (D_PUSH, "OPTIONS IMPORT: explicit notify parm(s) modified"); @@ -1962,10 +1962,10 @@ frame_finalize_options (struct context *c, const struct options *o) } frame_finalize (&c->c2.frame, - o->link_mtu_defined, - o->link_mtu, - o->tun_mtu_defined, - o->tun_mtu); + o->ce.link_mtu_defined, + o->ce.link_mtu, + o->ce.tun_mtu_defined, + o->ce.tun_mtu); } /* @@ -2411,8 +2411,8 @@ do_init_frame (struct context *c) /* * Adjust frame size based on the --tun-mtu-extra parameter. */ - if (c->options.tun_mtu_extra_defined) - tun_adjust_frame_parameters (&c->c2.frame, c->options.tun_mtu_extra); + if (c->options.ce.tun_mtu_extra_defined) + tun_adjust_frame_parameters (&c->c2.frame, c->options.ce.tun_mtu_extra); /* * Adjust frame size based on link socket parameters. @@ -2441,13 +2441,13 @@ do_init_frame (struct context *c) /* * MTU advisories */ - if (c->options.fragment && c->options.mtu_test) + if (c->options.ce.fragment && c->options.mtu_test) msg (M_WARN, "WARNING: using --fragment and --mtu-test together may produce an inaccurate MTU test result"); #endif #ifdef ENABLE_FRAGMENT - if ((c->options.mssfix || c->options.fragment) + if ((c->options.ce.mssfix || c->options.ce.fragment) && TUN_MTU_SIZE (&c->c2.frame_fragment) != ETHERNET_MTU) msg (M_WARN, "WARNING: normally if you use --mssfix and/or --fragment, you should also set --tun-mtu %d (currently it is %d)", @@ -2619,9 +2619,9 @@ do_init_buffers (struct context *c) static void do_init_fragment (struct context *c) { - ASSERT (c->options.fragment); + ASSERT (c->options.ce.fragment); frame_set_mtu_dynamic (&c->c2.frame_fragment, - c->options.fragment, SET_MTU_UPPER_BOUND); + c->options.ce.fragment, SET_MTU_UPPER_BOUND); fragment_frame_init (c->c2.fragment, &c->c2.frame_fragment); } #endif @@ -2632,10 +2632,10 @@ do_init_fragment (struct context *c) static void do_init_mssfix (struct context *c) { - if (c->options.mssfix) + if (c->options.ce.mssfix) { frame_set_mtu_dynamic (&c->c2.frame, - c->options.mssfix, SET_MTU_UPPER_BOUND); + c->options.ce.mssfix, SET_MTU_UPPER_BOUND); } } @@ -2691,7 +2691,7 @@ do_init_socket_1 (struct context *c, const int mode) c->options.ce.connect_retry_seconds, c->options.ce.connect_timeout, c->options.ce.connect_retry_max, - c->options.mtu_discover_type, + c->options.ce.mtu_discover_type, c->options.rcvbuf, c->options.sndbuf, c->options.mark, @@ -3406,7 +3408,7 @@ init_instance (struct context *c, const struct env_set *env, const unsigned int #ifdef ENABLE_FRAGMENT /* initialize internal fragmentation object */ - if (options->fragment && (c->mode == CM_P2P || child)) + if (options->ce.fragment && (c->mode == CM_P2P || child)) c->c2.fragment = fragment_init (&c->c2.frame); #endif @@ -3442,7 +3444,7 @@ init_instance (struct context *c, const struct env_set *env, const unsigned int #ifdef ENABLE_FRAGMENT /* initialize internal fragmentation capability with known frame size */ - if (options->fragment && (c->mode == CM_P2P || child)) + if (options->ce.fragment && (c->mode == CM_P2P || child)) do_init_fragment (c); #endif diff --git a/occ.c b/occ.c index bcf91cc..2fdbff0 100644 --- a/occ.c +++ b/occ.c @@ -368,7 +368,7 @@ process_received_occ_msg (struct context *c) c->c2.max_recv_size_remote, c->c2.max_send_size_remote, c->c2.max_recv_size_local); - if (!c->options.fragment + if (!c->options.ce.fragment && (proto_is_dgram(c->options.ce.proto)) && c->c2.max_send_size_local > TUN_MTU_MIN && (c->c2.max_recv_size_remote < c->c2.max_send_size_local diff --git a/options.c b/options.c index ce23dbc..df8dc91 100644 --- a/options.c +++ b/options.c @@ -765,10 +765,10 @@ init_options (struct options *o, const bool init_gc) o->status_file_update_freq = 60; o->status_file_version = 1; o->ce.bind_local = true; - o->tun_mtu = TUN_MTU_DEFAULT; - o->link_mtu = LINK_MTU_DEFAULT; - o->mtu_discover_type = -1; - o->mssfix = MSSFIX_DEFAULT; + o->ce.tun_mtu = TUN_MTU_DEFAULT; + o->ce.link_mtu = LINK_MTU_DEFAULT; + o->ce.mtu_discover_type = -1; + o->ce.mssfix = MSSFIX_DEFAULT; o->route_delay_window = 30; o->max_routes = MAX_ROUTES_DEFAULT; o->resolve_retry_seconds = RESOLV_RETRY_INFINITE; @@ -1361,8 +1361,26 @@ show_connection_entry (const struct connection_entry *o) SHOW_INT (socks_proxy_port); SHOW_BOOL (socks_proxy_retry); #endif + SHOW_INT (tun_mtu); + SHOW_BOOL (tun_mtu_defined); + SHOW_INT (link_mtu); + SHOW_BOOL (link_mtu_defined); + SHOW_INT (tun_mtu_extra); + SHOW_BOOL (tun_mtu_extra_defined); + + SHOW_INT (mtu_discover_type); + +#ifdef ENABLE_FRAGMENT + SHOW_INT (fragment); +#endif + SHOW_INT (mssfix); + +#ifdef ENABLE_OCC + SHOW_INT (explicit_exit_notification); +#endif } + static void show_connection_entries (const struct options *o) { @@ -1433,19 +1451,6 @@ show_settings (const struct options *o) #ifdef HAVE_GETTIMEOFDAY SHOW_INT (shaper); #endif - SHOW_INT (tun_mtu); - SHOW_BOOL (tun_mtu_defined); - SHOW_INT (link_mtu); - SHOW_BOOL (link_mtu_defined); - SHOW_INT (tun_mtu_extra); - SHOW_BOOL (tun_mtu_extra_defined); - -#ifdef ENABLE_FRAGMENT - SHOW_INT (fragment); -#endif - - SHOW_INT (mtu_discover_type); - #ifdef ENABLE_OCC SHOW_INT (mtu_test); #endif @@ -1460,16 +1465,11 @@ show_settings (const struct options *o) SHOW_INT (ping_rec_timeout_action); SHOW_BOOL (ping_timer_remote); SHOW_INT (remap_sigusr1); -#ifdef ENABLE_OCC - SHOW_INT (explicit_exit_notification); -#endif SHOW_BOOL (persist_tun); SHOW_BOOL (persist_local_ip); SHOW_BOOL (persist_remote_ip); SHOW_BOOL (persist_key); - SHOW_INT (mssfix); - #if PASSTOS_CAPABILITY SHOW_BOOL (passtos); #endif @@ -1959,7 +1959,7 @@ options_postprocess_verify_ce (const struct options *options, const struct conne /* * Sanity check on MTU parameters */ - if (options->tun_mtu_defined && options->link_mtu_defined) + if (options->ce.tun_mtu_defined && options->ce.link_mtu_defined) msg (M_USAGE, "only one of --tun-mtu or --link-mtu may be defined (note that --ifconfig implies --link-mtu %d)", LINK_MTU_DEFAULT); #ifdef ENABLE_OCC @@ -2042,12 +2042,12 @@ options_postprocess_verify_ce (const struct options *options, const struct conne */ #ifdef ENABLE_FRAGMENT - if (!proto_is_udp(ce->proto) && options->fragment) + if (!proto_is_udp(ce->proto) && ce->fragment) msg (M_USAGE, "--fragment can only be used with --proto udp"); #endif #ifdef ENABLE_OCC - if (!proto_is_udp(ce->proto) && options->explicit_exit_notification) + if (!proto_is_udp(ce->proto) && ce->explicit_exit_notification) msg (M_USAGE, "--explicit-exit-notify can only be used with --proto udp"); #endif @@ -2132,7 +2132,7 @@ options_postprocess_verify_ce (const struct options *options, const struct conne if (!(dev == DEV_TYPE_TAP || (dev == DEV_TYPE_TUN && options->topology == TOP_SUBNET)) && options->ifconfig_pool_netmask)	msg (M_USAGE, "The third parameter to --ifconfig-pool (netmask) is only valid in --dev tap mode"); #ifdef ENABLE_OCC - if (options->explicit_exit_notification) + if (ce->explicit_exit_notification)	msg (M_USAGE, "--explicit-exit-notify cannot be used with --mode server"); #endif if (options->routes && (options->routes->flags & RG_ENABLE)) @@ -2427,27 +2427,28 @@ options_postprocess_mutate_ce (struct options *o, struct connection_entry *ce) /* if protocol forcing is enabled, disable all protocols except for the forced one */ if (o->proto_force >= 0 && proto_is_tcp(o->proto_force) != proto_is_tcp(ce->proto)) ce->flags |= CE_DISABLED; -} - -static void -options_postprocess_mutate_invariant (struct options *options) -{ - const int dev = dev_type_enum (options->dev, options->dev_type); /* * If --mssfix is supplied without a parameter, default * it to --fragment value, if --fragment is specified. */ - if (options->mssfix_default) + if (o->ce.mssfix_default) { #ifdef ENABLE_FRAGMENT - if (options->fragment) -	options->mssfix = options->fragment; + if (ce->fragment) +	o->ce.mssfix = ce->fragment; #else msg (M_USAGE, "--mssfix must specify a parameter"); #endif } +} + +static void +options_postprocess_mutate_invariant (struct options *options) +{ + const int dev = dev_type_enum (options->dev, options->dev_type); + /* * In forking TCP server mode, you don't need to ifconfig * the tap device (the assumption is that it will be bridged). @@ -2459,14 +2460,14 @@ options_postprocess_mutate_invariant (struct options *options) * Set MTU defaults */ { - if (!options->tun_mtu_defined && !options->link_mtu_defined) + if (!options->ce.tun_mtu_defined && !options->ce.link_mtu_defined) { -	options->tun_mtu_defined = true; +	options->ce.tun_mtu_defined = true; } - if ((dev == DEV_TYPE_TAP) && !options->tun_mtu_extra_defined) + if ((dev == DEV_TYPE_TAP) && !options->ce.tun_mtu_extra_defined) { -	options->tun_mtu_extra_defined = true; -	options->tun_mtu_extra = TAP_MTU_EXTRA_DEFAULT; +	options->ce.tun_mtu_extra_defined = true; +	options->ce.tun_mtu_extra = TAP_MTU_EXTRA_DEFAULT; } } @@ -2952,7 +2953,7 @@ options_string (const struct options *o, #endif #ifdef ENABLE_FRAGMENT - if (o->fragment) + if (o->ce.fragment) buf_printf (&out, ",mtu-dynamic"); #endif @@ -4696,39 +4697,40 @@ add_option (struct options *options, } else if ((streq (p[0], "link-mtu") || streq (p[0], "udp-mtu")) && p[1]) { - VERIFY_PERMISSION (OPT_P_MTU); - options->link_mtu = positive_atoi (p[1]); - options->link_mtu_defined = true; + VERIFY_PERMISSION (OPT_P_MTU|OPT_P_CONNECTION); + options->ce.link_mtu = positive_atoi (p[1]); + options->ce.link_mtu_defined = true; } else if (streq (p[0], "tun-mtu") && p[1]) { - VERIFY_PERMISSION (OPT_P_MTU); - options->tun_mtu = positive_atoi (p[1]); - options->tun_mtu_defined = true; + VERIFY_PERMISSION (OPT_P_MTU|OPT_P_CONNECTION); + options->ce.tun_mtu = positive_atoi (p[1]); + options->ce.tun_mtu_defined = true; } else if (streq (p[0], "tun-mtu-extra") && p[1]) { - VERIFY_PERMISSION (OPT_P_MTU); - options->tun_mtu_extra = positive_atoi (p[1]); - options->tun_mtu_extra_defined = true; + VERIFY_PERMISSION (OPT_P_MTU|OPT_P_CONNECTION); + options->ce.tun_mtu_extra = positive_atoi (p[1]); + options->ce.tun_mtu_extra_defined = true; } #ifdef ENABLE_FRAGMENT else if (streq (p[0], "mtu-dynamic")) { - VERIFY_PERMISSION (OPT_P_GENERAL); + VERIFY_PERMISSION (OPT_P_MTU|OPT_P_CONNECTION); msg (msglevel, "--mtu-dynamic has been replaced by --fragment"); goto err; } else if (streq (p[0], "fragment") && p[1]) { - VERIFY_PERMISSION (OPT_P_MTU); - options->fragment = positive_atoi (p[1]); +// VERIFY_PERMISSION (OPT_P_MTU); + VERIFY_PERMISSION (OPT_P_MTU|OPT_P_CONNECTION); + options->ce.fragment = positive_atoi (p[1]); } #endif else if (streq (p[0], "mtu-disc") && p[1]) { - VERIFY_PERMISSION (OPT_P_MTU); - options->mtu_discover_type = translate_mtu_discover_type_name (p[1]); + VERIFY_PERMISSION (OPT_P_MTU|OPT_P_CONNECTION); + options->ce.mtu_discover_type = translate_mtu_discover_type_name (p[1]); } #ifdef ENABLE_OCC else if (streq (p[0], "mtu-test")) @@ -5073,14 +5075,15 @@ add_option (struct options *options, #ifdef ENABLE_OCC else if (streq (p[0], "explicit-exit-notify")) { - VERIFY_PERMISSION (OPT_P_EXPLICIT_NOTIFY); + VERIFY_PERMISSION (OPT_P_GENERAL|OPT_P_CONNECTION); +// VERIFY_PERMISSION (OPT_P_EXPLICIT_NOTIFY); if (p[1])	{ - options->explicit_exit_notification = positive_atoi (p[1]); + options->ce.explicit_exit_notification = positive_atoi (p[1]);	} else	{ - options->explicit_exit_notification = 1; + options->ce.explicit_exit_notification = 1;	} } #endif @@ -5336,13 +5339,13 @@ add_option (struct options *options, } else if (streq (p[0], "mssfix")) { - VERIFY_PERMISSION (OPT_P_GENERAL); + VERIFY_PERMISSION (OPT_P_GENERAL|OPT_P_CONNECTION); if (p[1])	{ - options->mssfix = positive_atoi (p[1]); + options->ce.mssfix = positive_atoi (p[1]);	} else -	options->mssfix_default = true; +	options->ce.mssfix_default = true; } #ifdef ENABLE_OCC diff --git a/options.h b/options.h index 81e0757..0f9d260 100644 --- a/options.h +++ b/options.h @@ -111,6 +111,24 @@ struct connection_entry bool socks_proxy_retry; #endif + int tun_mtu; /* MTU of tun device */ + bool tun_mtu_defined; /* true if user overriding parm with command line option */ + int tun_mtu_extra; + bool tun_mtu_extra_defined; + int link_mtu; /* MTU of device over which tunnel packets pass via TCP/UDP */ + bool link_mtu_defined; /* true if user overriding parm with command line option */ + + /* Advanced MTU negotiation and datagram fragmentation options */ + int mtu_discover_type; /* used if OS supports setting Path MTU discovery options on socket */ + + int fragment; /* internal fragmentation size */ + int mssfix; /* Upper bound on TCP MSS */ + bool mssfix_default; /* true if --mssfix was supplied without a parameter */ + +#ifdef ENABLE_OCC + int explicit_exit_notification; /* Explicitly tell peer when we are exiting via OCC_EXIT message */ +#endif + # define CE_DISABLED (1<<0) #if HTTP_PROXY_FALLBACK # define CE_HTTP_PROXY_FALLBACK (1<<1) @@ -248,24 +266,13 @@ struct options #ifdef HAVE_GETTIMEOFDAY int shaper; #endif - int tun_mtu; /* MTU of tun device */ - int tun_mtu_extra; - bool tun_mtu_extra_defined; - int link_mtu; /* MTU of device over which tunnel packets pass via TCP/UDP */ - bool tun_mtu_defined; /* true if user overriding parm with command line option */ - bool link_mtu_defined; /* true if user overriding parm with command line option */ int proto_force; - /* Advanced MTU negotiation and datagram fragmentation options */ - int mtu_discover_type; /* used if OS supports setting Path MTU discovery options on socket */ - #ifdef ENABLE_OCC bool mtu_test; #endif - int fragment; /* internal fragmentation size */ - #ifdef ENABLE_MEMSTATS char *memstats_fn; #endif @@ -288,18 +295,11 @@ struct options # define PING_RESTART 2 int ping_rec_timeout_action; /* What action to take on ping_rec_timeout (exit or restart)? */ -#ifdef ENABLE_OCC - int explicit_exit_notification; /* Explicitly tell peer when we are exiting via OCC_EXIT message */ -#endif - bool persist_tun; /* Don't close/reopen TUN/TAP dev on SIGUSR1 or PING_RESTART */ bool persist_local_ip; /* Don't re-resolve local address on SIGUSR1 or PING_RESTART */ bool persist_remote_ip; /* Don't re-resolve remote address on SIGUSR1 or PING_RESTART */ bool persist_key; /* Don't re-read key files on SIGUSR1 or PING_RESTART */ - int mssfix; /* Upper bound on TCP MSS */ - bool mssfix_default; /* true if --mssfix was supplied without a parameter */ - #if PASSTOS_CAPABILITY bool passtos; #endif diff --git a/sig.c b/sig.c index d73525f..a5703f0 100644 --- a/sig.c +++ b/sig.c @@ -300,8 +300,8 @@ process_explicit_exit_notification_timer_wakeup (struct context *c) &c->c2.timeval, ETT_DEFAULT)) { - ASSERT (c->c2.explicit_exit_notification_time_wait && c->options.explicit_exit_notification); - if (now >= c->c2.explicit_exit_notification_time_wait + c->options.explicit_exit_notification) + ASSERT (c->c2.explicit_exit_notification_time_wait && c->options.ce.explicit_exit_notification); + if (now >= c->c2.explicit_exit_notification_time_wait + c->options.ce.explicit_exit_notification)	{ event_timeout_clear (&c->c2.explicit_exit_notification_interval); c->sig->signal_received = SIGTERM; @@ -340,7 +340,7 @@ process_sigterm (struct context *c) { bool ret = true; #ifdef ENABLE_OCC - if (c->options.explicit_exit_notification + if (c->options.ce.explicit_exit_notification && !c->c2.explicit_exit_notification_time_wait) { process_explicit_exit_notification_init (c); -- 1.7.4.4 

 Made some options connection-entry specific: fragment mssfix tun-mtu tun-mtu-extra link-mtu mtu_discover_type explicit-exit-notification in order to support stuff like <connection> remote host proto udp fragment explicit-exit-notification 3 </connection> <connection> remote host proto tcp </connection> 

Added support for Elliptic curves (ECDSA) + SHA2 family signed certificates. --- init.c | 7 ++++ options.c | 15 ++++++++++ options.h | 6 ++++ ssl.c | 3 ++ ssl_backend.h | 10 ++++++ ssl_openssl.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ ssl_polarssl.c | 9 ++++++ 7 files changed, 134 insertions(+), 0 deletions(-) diff --git a/init.c b/init.c index 525f441..51b0d64 100644 --- a/init.c +++ b/init.c @@ -895,6 +895,9 @@ print_openssl_info (const struct options *options) if (options->show_ciphers || options->show_digests || options->show_engines #ifdef USE_SSL || options->show_tls_ciphers +#ifdef USE_SSL_EC +|| options->show_curves +#endif #endif ) { @@ -907,6 +910,10 @@ print_openssl_info (const struct options *options) #ifdef USE_SSL if (options->show_tls_ciphers)	show_available_tls_ciphers (); +#ifdef USE_SSL_EC + if (options->show_curves) + show_available_curves (); +#endif #endif return true; } diff --git a/options.c b/options.c index 6b8ae22..ce23dbc 100644 --- a/options.c +++ b/options.c @@ -836,6 +836,9 @@ init_options (struct options *o, const bool init_gc) #ifdef ENABLE_X509ALTUSERNAME o->x509_username_field = X509_USERNAME_FIELD_DEFAULT; #endif +#ifdef USE_SSL_EC + o->curve_name = NULL; +#endif #endif /* USE_SSL */ #endif /* USE_CRYPTO */ #ifdef ENABLE_PKCS11 @@ -6368,6 +6371,18 @@ add_option (struct options *options, VERIFY_PERMISSION (OPT_P_GENERAL); options->show_tls_ciphers = true; } +#ifdef USE_SSL_EC + else if (streq (p[0], "show-curves")) + { + VERIFY_PERMISSION (OPT_P_GENERAL); + options->show_curves = true; + } + else if (streq (p[0], "ecdh") && p[1]) + { + VERIFY_PERMISSION (OPT_P_CRYPTO); + options->curve_name= p[1]; + } +#endif else if (streq (p[0], "tls-server")) { VERIFY_PERMISSION (OPT_P_GENERAL); diff --git a/options.h b/options.h index 831d4f6..81e0757 100644 --- a/options.h +++ b/options.h @@ -200,6 +200,9 @@ struct options bool show_engines; #ifdef USE_SSL bool show_tls_ciphers; +#ifdef USE_SSL_EC + bool show_curves; +#endif #endif bool genkey; #endif @@ -533,6 +536,9 @@ struct options const char *priv_key_file; const char *pkcs12_file; const char *cipher_list; +#ifdef USE_SSL_EC + const char *curve_name; +#endif const char *tls_verify; const char *tls_export_cert; const char *tls_remote; diff --git a/ssl.c b/ssl.c index c26756e..54efe2f 100644 --- a/ssl.c +++ b/ssl.c @@ -308,6 +308,9 @@ init_ssl (const struct options *options, struct tls_root_ctx *new_ctx) { tls_ctx_server_new(new_ctx); tls_ctx_load_dh_params(new_ctx, options->dh_file, options->dh_file_inline); +#ifdef USE_SSL_EC + tls_ctx_load_ecdh_params(new_ctx, options->curve_name); +#endif } else	/* if client */ { diff --git a/ssl_backend.h b/ssl_backend.h index 243c9e3..ebf9f36 100644 --- a/ssl_backend.h +++ b/ssl_backend.h @@ -145,6 +145,16 @@ void tls_ctx_load_dh_params(struct tls_root_ctx *ctx, const char *dh_file ); /** + * Load Elliptic Curve Parameters, and load them into the library-specific + * TLS context. + * + * @param ctx	TLS context to use + * @param curve_name	The name of the elliptic curve to load. + */ +void tls_ctx_load_ecdh_params(struct tls_root_ctx *ctx, const char *curve_name + ); + +/** * Load PKCS #12 file for key, cert and (optionally) CA certs, and add to * library-specific TLS context. * diff --git a/ssl_openssl.c b/ssl_openssl.c index b95944c..912dd8f 100644 --- a/ssl_openssl.c +++ b/ssl_openssl.c @@ -50,6 +50,9 @@ #include <openssl/pkcs12.h> #include <openssl/x509.h> #include <openssl/crypto.h> +#ifdef USE_SSL_EC +#include <openssl/ec.h> +#endif /* * Allocate space in SSL objects in which to store a struct tls_session @@ -238,6 +241,46 @@ tls_ctx_load_dh_params (struct tls_root_ctx *ctx, const char *dh_file DH_free (dh); } +void +tls_ctx_load_ecdh_params (struct tls_root_ctx *ctx, const char *curve_name + ) +{ +#ifdef USE_SSL_EC + if (curve_name != NULL) + { + int nid; + EC_KEY *ecdh = NULL; + + nid = OBJ_sn2nid(curve_name); + + if (nid == 0) + msg(M_SSLERR, "unknown curve name (%s)", curve_name); + else + { + ecdh = EC_KEY_new_by_curve_name(nid); + if (ecdh == NULL) + msg (M_SSLERR, "Unable to create curve (%s)", curve_name); + else + { + const char *sname; + + if (!SSL_CTX_set_tmp_ecdh(ctx->ctx, ecdh)) + msg (M_SSLERR, "SSL_CTX_set_tmp_ecdh: cannot add curve"); + + /* Translate NID back to name , just for kicks */ + sname = OBJ_nid2sn(nid); + if (sname == NULL) sname = "(Unknown)"; + msg (D_TLS_DEBUG_LOW, "ECDH curve %s added", sname); + + EC_KEY_free(ecdh); + } + } + } +#else + msg(M_SSLERR, "Elliptic Curves not supported by this version of OpenSSL"); +#endif +} + int tls_ctx_load_pkcs12(struct tls_root_ctx *ctx, const char *pkcs12_file, #if ENABLE_INLINE_FILES @@ -1273,6 +1316,47 @@ show_available_tls_ciphers () SSL_CTX_free (ctx); } +/* + * * Show the Elliptic curves that are available for us to use + * * in the OpenSSL library. + * */ +#ifdef USE_SSL_EC +void +show_available_curves() +{ + EC_builtin_curve *curves = NULL; + size_t crv_len = 0; + size_t n = 0; + + crv_len = EC_get_builtin_curves(NULL, 0); + + curves = OPENSSL_malloc((int)(sizeof(EC_builtin_curve) * crv_len)); + + if (curves == NULL) + msg (M_SSLERR, "Cannot create EC_builtin_curve object"); + else + { + if (EC_get_builtin_curves(curves, crv_len)) + { + printf ("Available Elliptic curves:\n"); + for (n = 0; n < crv_len; n++) + { + const char *sname; + sname = OBJ_nid2sn(curves[n].nid); + if (sname == NULL) sname = ""; + + printf("%s\n", sname); + } + } + else + { + msg (M_SSLERR, "Cannot get list of builtin curves"); + } + OPENSSL_free(curves); + } +} +#endif + void get_highest_preference_tls_cipher (char *buf, int size) { diff --git a/ssl_polarssl.c b/ssl_polarssl.c index c50cf0a..a7a6d61 100644 --- a/ssl_polarssl.c +++ b/ssl_polarssl.c @@ -218,6 +218,15 @@ else (counter_type) 8 * mpi_size(&ctx->dhm_ctx->P)); } +#ifdef USE_SSL_EC +void +tls_ctx_load_ecdh_params (struct tls_root_ctx *ctx, const char *curve_name + ) +{ + msg(M_WARN, "Elliptic Curves not yet supported by PolarSSL"); +} +#endif + int tls_ctx_load_pkcs12(struct tls_root_ctx *ctx, const char *pkcs12_file, #if ENABLE_INLINE_FILES -- 1.7.4.4 

In commit bee92b479414d12035b0422f81ac5fcfe14fa645 the gc_malloc() was hardened to always require a gc_arena object for garbage collection. Some places in the code expected the old behaviour of a normal malloc() in these cases, that is a memory allocation without garbage collection. This old behaviour is partly restored by allowing string_alloc() to do a non-gc based allocation if no gc_arena object is available. In addition some other places string_alloc() will now be called with a gc_arena pointer where such an object is available. The alloc_buf() function has also been refactored to not use gc_malloc() at all. v2: - removes a memleak when --ifconfig-ipv6 is used several times - makes string_alloc() behave properly if DMALLOC is enabled Signed-off-by: David Sommerseth <da...@re...> --- buffer.c | 32 ++++++++++++++++++++++++++++---- init.c | 2 +- openvpn.c | 2 +- options.c | 7 ++++++- pf.c | 2 +- ssl_verify.c | 2 ++ 6 files changed, 39 insertions(+), 8 deletions(-) diff --git a/buffer.c b/buffer.c index c39bbcb..fca6a90 100644 --- a/buffer.c +++ b/buffer.c @@ -54,11 +54,21 @@ alloc_buf_debug (size_t size, const char *file, int line) alloc_buf (size_t size) #endif { + struct buffer buf; + + if (!buf_size_valid (size)) + buf_size_error (size); + buf.capacity = (int)size; + buf.offset = 0; + buf.len = 0; #ifdef DMALLOC - return alloc_buf_gc_debug (size, NULL, file, line); + buf.data = openvpn_dmalloc (file, line, size); #else - return alloc_buf_gc (size, NULL); + buf.data = calloc (1, size); #endif + check_malloc_return(buf.data); + + return buf; } struct buffer @@ -515,11 +525,25 @@ string_alloc (const char *str, struct gc_arena *gc) const int n = strlen (str) + 1; char *ret; + if (gc) { +#ifdef DMALLOC + ret = (char *) gc_malloc_debug (n, false, gc, file, line); +#else + ret = (char *) gc_malloc (n, false, gc); +#endif + } else { + /* If there are no garbage collector available, it's expected + * that the caller cleans up afterwards. This is coherent with the + * earlier behaviour when gc_malloc() would be called with gc == NULL + */ #ifdef DMALLOC - ret = (char *) gc_malloc_debug (n, false, gc, file, line); + ret = openvpn_dmalloc (file, line, n); + memset(ret, 0, n); #else - ret = (char *) gc_malloc (n, false, gc); + ret = calloc(1, n); #endif + check_malloc_return(ret); + } memcpy (ret, str, n); return ret; } diff --git a/init.c b/init.c index 525f441..f0c3693 100644 --- a/init.c +++ b/init.c @@ -3012,7 +3012,7 @@ do_close_ifconfig_pool_persist (struct context *c) static void do_inherit_env (struct context *c, const struct env_set *src) { - c->c2.es = env_set_create (NULL); + c->c2.es = env_set_create (&c->c2.gc); c->c2.es_owned = true; env_set_inherit (c->c2.es, src); } diff --git a/openvpn.c b/openvpn.c index f5f2bce..84289d2 100644 --- a/openvpn.c +++ b/openvpn.c @@ -164,7 +164,7 @@ main (int argc, char *argv[]) gc_init (&c.gc); /* initialize environmental variable store */ - c.es = env_set_create (NULL); + c.es = env_set_create (&c.gc); #ifdef WIN32 set_win_sys_path_via_env (c.es); #endif diff --git a/options.c b/options.c index 6b8ae22..a0b3431 100644 --- a/options.c +++ b/options.c @@ -4291,7 +4291,7 @@ add_option (struct options *options, { unsigned int netbits; char * ipv6_local; - + VERIFY_PERMISSION (OPT_P_UP); if ( get_ipv6_addr( p[1], NULL, &netbits, &ipv6_local, msglevel ) && ipv6_addr_safe( p[2] ) ) @@ -4301,6 +4301,11 @@ add_option (struct options *options, msg( msglevel, "ifconfig-ipv6: /netbits must be between 64 and 124, not '/%d'", netbits ); goto err; } + + if (options->ifconfig_ipv6_local) + /* explicitly ignoring this is a const char */ + free ((char *) options->ifconfig_ipv6_local); + options->ifconfig_ipv6_local = ipv6_local; options->ifconfig_ipv6_netbits = netbits; options->ifconfig_ipv6_remote = p[2]; diff --git a/pf.c b/pf.c index 6b4cba4..79915fa 100644 --- a/pf.c +++ b/pf.c @@ -566,7 +566,7 @@ pf_init_context (struct context *c) 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); + c->c2.pf.filename = string_alloc (pf_file, &c->c2.gc); c->c2.pf.enabled = true; #ifdef ENABLE_DEBUG if (check_debug_level (D_PF_DEBUG)) diff --git a/ssl_verify.c b/ssl_verify.c index e45f149..37d4982 100644 --- a/ssl_verify.c +++ b/ssl_verify.c @@ -83,6 +83,7 @@ set_common_name (struct tls_session *session, const char *common_name) } if (common_name) { + /* FIXME: Last alloc will never be freed */ session->common_name = string_alloc (common_name, NULL); #ifdef ENABLE_PF { @@ -703,6 +704,7 @@ man_def_auth_set_client_reason (struct tls_multi *multi, const char *client_reas multi->client_reason = NULL; } if (client_reason && strlen (client_reason)) + /* FIXME: Last alloc will never be freed */ multi->client_reason = string_alloc (client_reason, NULL); } -- 1.7.4.4 

 ----- Mail original ----- > De: "Gert Doering" <ge...@gr...> > À: ope...@li... > Envoyé: Mardi 7 Février 2012 11:24:13 > Objet: [Openvpn-devel] configure patch for MacOS 10.7 > > Hi, > > I'm forwarding this "as-is", as I do not have enough understanding of > autoconf to say whether this is necessary, or "the right fix" - but > anyway, I've been told that this is needed to make our configure > behave on MacOS 10.7. > > gert > > -- Adding AC_LANG_SOURCE where it is required is the right fix. I made that fix on other packages. This is not specific to MacOS, that just depend on the autoconf version installed on the system. With autoconf-2.68 installed and running autoreconf on a package produce warning where AC_LANG_SOURCE is missing. That look to be done properly (I haven't tested the patch) Gilles 

In commit bee92b479414d12035b0422f81ac5fcfe14fa645 the gc_malloc() was hardened to always require a gc_arena object for garbage collection. Some places in the code expected the old behaviour of a normal malloc() in these cases, that is a memory allocation without garbage collection. This old behaviour is partly restored by allowing string_alloc() to do a non-gc based allocation if no gc_arena object is available. In addition some other places string_alloc() will now be called with a gc_arena pointer where such an object is available. The alloc_buf() function has also been refactored to not use gc_malloc() at all. Signed-off-by: David Sommerseth <da...@re...> --- buffer.c | 25 ++++++++++++++++++++++--- init.c | 2 +- openvpn.c | 2 +- options.c | 2 ++ pf.c | 2 +- ssl_verify.c | 2 ++ 6 files changed, 29 insertions(+), 6 deletions(-) diff --git a/buffer.c b/buffer.c index c39bbcb..f9335f0 100644 --- a/buffer.c +++ b/buffer.c @@ -54,11 +54,21 @@ alloc_buf_debug (size_t size, const char *file, int line) alloc_buf (size_t size) #endif { + struct buffer buf; + + if (!buf_size_valid (size)) + buf_size_error (size); + buf.capacity = (int)size; + buf.offset = 0; + buf.len = 0; #ifdef DMALLOC - return alloc_buf_gc_debug (size, NULL, file, line); + buf.data = openvpn_dmalloc (file, line, size); #else - return alloc_buf_gc (size, NULL); + buf.data = calloc (1, size); #endif + check_malloc_return(buf.data); + + return buf; } struct buffer @@ -518,7 +528,16 @@ string_alloc (const char *str, struct gc_arena *gc) #ifdef DMALLOC ret = (char *) gc_malloc_debug (n, false, gc, file, line); #else - ret = (char *) gc_malloc (n, false, gc); + if (gc) { + ret = (char *) gc_malloc (n, false, gc); + } else { + /* If there are no garbage collector available, it's expected + * that the caller cleans up afterwards. This is coherent with the + * earlier behaviour when gc_malloc() would be called with gc == NULL + */ + ret = calloc(1, n); + check_malloc_return(ret); + } #endif memcpy (ret, str, n); return ret; diff --git a/init.c b/init.c index 525f441..f0c3693 100644 --- a/init.c +++ b/init.c @@ -3012,7 +3012,7 @@ do_close_ifconfig_pool_persist (struct context *c) static void do_inherit_env (struct context *c, const struct env_set *src) { - c->c2.es = env_set_create (NULL); + c->c2.es = env_set_create (&c->c2.gc); c->c2.es_owned = true; env_set_inherit (c->c2.es, src); } diff --git a/openvpn.c b/openvpn.c index f5f2bce..84289d2 100644 --- a/openvpn.c +++ b/openvpn.c @@ -164,7 +164,7 @@ main (int argc, char *argv[]) gc_init (&c.gc); /* initialize environmental variable store */ - c.es = env_set_create (NULL); + c.es = env_set_create (&c.gc); #ifdef WIN32 set_win_sys_path_via_env (c.es); #endif diff --git a/options.c b/options.c index 6b8ae22..bec11e5 100644 --- a/options.c +++ b/options.c @@ -4301,6 +4301,8 @@ add_option (struct options *options, msg( msglevel, "ifconfig-ipv6: /netbits must be between 64 and 124, not '/%d'", netbits ); goto err; } + /* FIXME: Check if ipv6_local might miss a free(). It might leak + * via get_ipv6_addr() -> string_alloc() ? */ options->ifconfig_ipv6_local = ipv6_local; options->ifconfig_ipv6_netbits = netbits; options->ifconfig_ipv6_remote = p[2]; diff --git a/pf.c b/pf.c index 6b4cba4..79915fa 100644 --- a/pf.c +++ b/pf.c @@ -566,7 +566,7 @@ pf_init_context (struct context *c) 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); + c->c2.pf.filename = string_alloc (pf_file, &c->c2.gc); c->c2.pf.enabled = true; #ifdef ENABLE_DEBUG if (check_debug_level (D_PF_DEBUG)) diff --git a/ssl_verify.c b/ssl_verify.c index e45f149..37d4982 100644 --- a/ssl_verify.c +++ b/ssl_verify.c @@ -83,6 +83,7 @@ set_common_name (struct tls_session *session, const char *common_name) } if (common_name) { + /* FIXME: Last alloc will never be freed */ session->common_name = string_alloc (common_name, NULL); #ifdef ENABLE_PF { @@ -703,6 +704,7 @@ man_def_auth_set_client_reason (struct tls_multi *multi, const char *client_reas multi->client_reason = NULL; } if (client_reason && strlen (client_reason)) + /* FIXME: Last alloc will never be freed */ multi->client_reason = string_alloc (client_reason, NULL); } -- 1.7.4.4 

Hi, I'm forwarding this "as-is", as I do not have enough understanding of autoconf to say whether this is necessary, or "the right fix" - but anyway, I've been told that this is needed to make our configure behave on MacOS 10.7. gert -- USENET is *not* the non-clickable part of WWW! //www.muc.de/~gert/ Gert Doering - Munich, Germany ge...@gr... fax: +49-89-35655025 ge...@ne... 

Thank you, that patch had more impact than I thought it would. I'm sorry about the inconvenience, it's a bug I'm looking into at the moment. David, could you revert the patch please, so I have a little more time to refactor there? I'm looking into the best way to refactor that function with a minimal risk of causing new memory leaks. Adriaan > -----Original Message----- > From: Michal Ludvig [mailto:ml...@lo...] > Sent: dinsdag 7 februari 2012 2:13 > To: ope...@li... > Subject: [Openvpn-devel] Assertion failed at buffer.c:313 > > Hi guys > > I experience "Assertion failed at buffer.c:313" on my RHEL5/x64 caused > by: > > commit bee92b479414d12035b0422f81ac5fcfe14fa645 > Author: Adriaan de Jong <de...@fo...> > Date: Sun Feb 5 12:51:25 2012 +0100 > > Removed support for calling gc_malloc with a NULL gc_arena struct > > Once this patch is reverted OpenVPN compiles and runs smoothly. > > Michal > > ----------------------------------------------------------------------- > ------- > Keep Your Developer Skills Current with LearnDevNow! > The most comprehensive online learning library for Microsoft developers > is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, > MVC3, Metro Style Apps, more. Free future releases when you subscribe > now! > http://p.sf.net/sfu/learndevnow-d2d > _______________________________________________ > Openvpn-devel mailing list > Ope...@li... > https://lists.sourceforge.net/lists/listinfo/openvpn-devel 

Hi guys I experience "Assertion failed at buffer.c:313" on my RHEL5/x64 caused by: commit bee92b479414d12035b0422f81ac5fcfe14fa645 Author: Adriaan de Jong <de...@fo...> Date: Sun Feb 5 12:51:25 2012 +0100 Removed support for calling gc_malloc with a NULL gc_arena struct Once this patch is reverted OpenVPN compiles and runs smoothly. Michal