Re: [Openvpn-devel] [PATCH v2 9/9] client-connect: Add deferred support to the client-connect plugin v1 handler

2016-11-30 Thread Heikki Hannikainen

Fabian,

Have you by any chance ported this patch set forward to the current
OpenVPN master / 2.4. beta?

We have used a variation of it, and gained a noticeable performance boost,
for a long time. Thanks to the deferred client-connect handling the
openvpn server does not stop packet processing while a client-connect
handler plugin is doing what it needs to do, reducing jitter and packet 
loss for other clients.

It'd be great to have this merged in the master tree at some point.


On Sat, 17 Jan 2015, Fabian Knittel wrote:

> Uses the infrastructure provided and used in the previous patch to provide
> deferral support to the v1 client-connect plugin handler as well.
>
> Signed-off-by: Fabian Knittel 
> ---
> src/openvpn/multi.c | 32 
> 1 file changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
> index 68a7248..ab73034 100644
> --- a/src/openvpn/multi.c
> +++ b/src/openvpn/multi.c
> @@ -1814,6 +1814,7 @@ multi_client_connect_call_plugin_v1 (struct 
> multi_context *m,
> {
>   enum client_connect_return ret = CC_RET_SKIPPED;
> #ifdef ENABLE_PLUGIN
> +  struct client_connect_state *ccs = mi->client_connect_state;
>   ASSERT (m);
>   ASSERT (mi);
>   ASSERT (option_types_found);
> @@ -1821,34 +1822,41 @@ multi_client_connect_call_plugin_v1 (struct 
> multi_context *m,
>   if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT))
> {
>   int plug_ret;
> -  struct gc_arena gc = gc_new ();
>   struct argv argv = argv_new ();
> -  const char *dc_file = create_temp_file (mi->context.options.tmp_dir, 
> "cc", &gc);
>
> -  if (!dc_file)
> +  if (!ccs_gen_config_file (mi) ||
> +   !ccs_gen_deferred_ret_file (mi))
>   {
> ret = CC_RET_FAILED;
> -   goto script_depr_failed;
> +   goto cleanup;
>   }
>
> -  argv_printf (&argv, "%s", dc_file);
> +  argv_printf (&argv, "%s", ccs->config_file);
>
>   plug_ret = plugin_call (mi->context.plugins,
> OPENVPN_PLUGIN_CLIENT_CONNECT,
> &argv, NULL, mi->context.c2.es);
>   argv_reset (&argv);
> -  if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
> +  if (plug_ret == OPENVPN_PLUGIN_FUNC_SUCCESS)
> + {
> +   multi_client_connect_post (m, mi, ccs->config_file,
> +  option_types_found);
> +   ret = CC_RET_SUCCEEDED;
> + }
> +  else if (plug_ret == OPENVPN_PLUGIN_FUNC_DEFERRED)
> + ret = CC_RET_DEFERRED;
> +  else
>   {
> msg (M_WARN, "WARNING: client-connect plugin call failed");
> ret = CC_RET_FAILED;
>   }
> -  else
> +
> +cleanup:
> +  if (ret != CC_RET_DEFERRED)
>   {
> -   multi_client_connect_post (m, mi, dc_file, option_types_found);
> -   ret = CC_RET_SUCCEEDED;
> +   ccs_delete_config_file (mi);
> +   ccs_delete_deferred_ret_file (mi);
>   }
> -script_depr_failed:
> -  gc_free (&gc);
> }
> #endif
>   return ret;
> @@ -2141,7 +2149,7 @@ static const struct client_connect_handlers 
> client_connect_handlers[] = {
>   },
>   {
> main: multi_client_connect_call_plugin_v1,
> -deferred: multi_client_connect_fail
> +deferred: multi_client_handle_deferred
>   },
>   {
> main: multi_client_connect_call_plugin_v2,
> -- 
> 2.1.4
>
>
> --
> New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
> GigeNET is offering a free month of service with a new server in Ashburn.
> Choose from 2 high performing configs, both with 100TB of bandwidth.
> Higher redundancy.Lower latency.Increased capacity.Completely compliant.
> http://p.sf.net/sfu/gigenet
> ___
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>

   - Hessu


--
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH] pkcs12-additional-cas option to load CA+intermediate certs from both PKCS#12 and a --ca PEM file

2013-05-18 Thread Heikki Hannikainen
Hi,

I've set up a VPN service which authenticates users using certificates
provided by a 3rd party (which has manually authenticated the users
from paper documents and given out some 50k certs). Their tools allow
the end users to nicely export a PKCS#12 certificate which OpenVPN can
currently use out of the box. Grand!

The 3rd party does not give out server certificates, so my VPN server
uses a certificate signed by my own CA. Thus I need to pass my CA to
the VPN client using the --ca option.

Currently OpenVPN ignores CA and intermediate certificates inside the
PKCS#12 file if --ca is set. Without --ca it loads them. The problem
is that the 3rd party CA uses intermediate certificates, and rotates
them ~yearly without warning, so I need the client to load the
intermediates from PKCS#12. To get both client and server validation
working I need to load certs from both the PKCS#12 file and a PEM file
provided using --ca.

So, I added a client option 'pkcs12-additional-cas' to make the --ca
and pkcs12 CA certs additive, not exclusive either-or. Default
functionality is like before. Manual page updated, too.

Patch attached, feedback welcome. I'm not quite sure if the name of
the option (--pkcs12-additional-cas) is good.


---
 doc/openvpn.8 |   16 
 src/openvpn/options.c |   16 
 src/openvpn/options.h |1 +
 src/openvpn/ssl.c |2 +-
 src/openvpn/ssl_openssl.c |2 +-
 5 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/doc/openvpn.8 b/doc/openvpn.8
index cbfc107..c758776 100644
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -4225,6 +4225,22 @@ and
 Not available with PolarSSL.
 .\"*
 .TP
+.B \-\-pkcs12-additional-cas
+Load trusted CA and intermediate certificates from both the PKCS #12
+file specified using
+.B \-\-pkcs12
+and an additional PEM file specified using
+.B \-\-ca.
+The default is to not load CA or intermediate certificates from a
+PKCS #12 file if
+.B \-\-ca
+is set. This option can be used when the PKCS #12 client certificate
+is provided by a different CA than the server certificate, and
+contains intermediate certificates required for successful client
+authentication, but does not contain the CA certificate used for signing
+the server's certificate.
+.\"*
+.TP
 .B \-\-verify-hash hash
 Specify SHA1 fingerprint for level-1 cert.  The level-1 cert is the
 CA (or intermediate cert) that signs the leaf certificate, and is
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index c5ed0d6..5175607 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -578,6 +578,10 @@ static const char usage_message[] =
 #ifndef ENABLE_CRYPTO_POLARSSL
   "--pkcs12 file   : PKCS#12 file containing local private key, local
certificate\n"
   "  and optionally the root CA certificate.\n"
+  "--pkcs12-additional-cas : Load CA certificates from both the
PKCS#12 file and a\n"
+  "  PEM file specified using --ca. Default
is to ignore\n"
+  "  CAs and intermediate certificates
contained in the\n"
+  "  PKCS#12 file if --ca is set.\n"
 #endif
 #ifdef ENABLE_X509ALTUSERNAME
   "--x509-username-field : Field used in x509 certificate to be username.\n"
@@ -1594,6 +1598,7 @@ show_settings (const struct options *o)
   SHOW_STR (priv_key_file);
 #ifndef ENABLE_CRYPTO_POLARSSL
   SHOW_STR (pkcs12_file);
+  SHOW_BOOL (pkcs12_additional_cas);
 #endif
 #ifdef ENABLE_CRYPTOAPI
   SHOW_STR (cryptoapi_cert);
@@ -2286,6 +2291,12 @@ options_postprocess_verify_ce (const struct
options *options, const struct conne
   notnull (options->priv_key_file, "private key file (--key) or
PKCS#12 file (--pkcs12)");
 }
  }
+
+  if (options->pkcs12_additional_cas && !options->ca_file &&
!options->pkcs12_file)
+{
+  msg (M_USAGE, "When --pkcs12-additional-cas is set, CA
certificates are loaded from both a PEM file (--ca) and a PKCS#12 file
(--pkcs12). Both options must be specified.");
+}
+
 }
   else
 {
@@ -6458,6 +6469,11 @@ add_option (struct options *options,
   options->pkcs12_file_inline = p[2];
  }
 }
+  else if (streq (p[0], "pkcs12-additional-cas"))
+{
+  VERIFY_PERMISSION (OPT_P_GENERAL);
+  options->pkcs12_additional_cas = true;
+}
 #endif /* ENABLE_CRYPTO_POLARSSL */
   else if (streq (p[0], "askpass"))
 {
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index d2ad94c..2f2fe8b 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -504,6 +504,7 @@ struct options
   const char *extra_certs_file;
   const char *priv_key_file;
   const char *pkcs12_file;
+  bool pkcs12_additional_cas;
   const char *cipher_list;
   const char *tls_verify;
   int verify_x509_type;
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 09cf300..e4457fd 100644
--- a/src/openvpn/ssl.c
++

[Openvpn-devel] [PATCH] Always load intermediate certificates from a PKCS#12 file

2013-06-20 Thread Heikki Hannikainen


Hi,

This one supersedes my previous patch 
(http://thread.gmane.org/gmane.network.openvpn.devel/7599) which added an 
extra option to load trusted CA certs from both PKCS#12 and the --ca PEM 
file. This new patch enables loading of extra intermediate certs from 
PKCS#12 even when --ca is set, while not making those certs trusted. Does 
not add any extra options.




From cec65ff199443c7f95101a7bf4a75644516d7810 Mon Sep 17 00:00:00 2001

From: Heikki Hannikainen 
List-Post: openvpn-devel@lists.sourceforge.net
Date: Thu, 20 Jun 2013 13:49:44 +0300
Subject: [PATCH] Load intermediate certificates from a PKCS#12 file and place
 them in the extra certs chain, when trusted CA certs are
 loaded from an external PEM file with the --ca option, and
 the CA certs in PKCS#12 are not to be trusted.

Required when client PKCS#12 file is provided by a different CA
than the server CA, the PKCS#12 file contains intermediate certificates
required for client auth, but the server CA is not in the PKCS#12 file.

When --ca is set, the PKCS#12 provided CA certs are not trusted. Without this
patch, they were ignored completely - with this patch, they're loaded
in the extra certs chain which makes them available for chain verification
but still does not make them trusted if --ca is set. Unless when, of course,
a trusted root is found from the --ca file.
---
 src/openvpn/ssl_openssl.c |   20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 2e5933d..dff4897 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -380,16 +380,34 @@ tls_ctx_load_pkcs12(struct tls_root_ctx *ctx, const char 
*pkcs12_file,
   /* Set Certificate Verification chain */
   if (load_ca_file)
{
+ /* Add CAs from PKCS12 to the cert store and mark them as trusted. 
+  * They're also used to fill in the chain of intermediate certs as

+  * necessary.
+  */
  if (ca && sk_X509_num(ca))
   {
for (i = 0; i < sk_X509_num(ca); i++)
  {
- if (!X509_STORE_add_cert(ctx->ctx->cert_store,sk_X509_value(ca, 
i)))
+   if (!X509_STORE_add_cert(ctx->ctx->cert_store,sk_X509_value(ca, i)))
  msg (M_SSLERR, "Cannot add certificate to certificate chain 
(X509_STORE_add_cert)");
if (!SSL_CTX_add_client_CA(ctx->ctx, sk_X509_value(ca, i)))
  msg (M_SSLERR, "Cannot add certificate to client CA list 
(SSL_CTX_add_client_CA)");
  }
   }
+   } else {
+ /* If trusted CA certs were loaded from a PEM file, and we ignore the
+  * ones in PKCS12, do load PKCS12-provided certs to the client extra
+  * certs chain just in case they include intermediate CAs needed to
+  * prove my identity to the other end. This does not make them trusted.
+  */
+ if (ca && sk_X509_num(ca))
+  {
+   for (i = 0; i < sk_X509_num(ca); i++)
+ {
+   if (!SSL_CTX_add_extra_chain_cert(ctx->ctx,sk_X509_value(ca, i)))
+ msg (M_SSLERR, "Cannot add extra certificate to chain 
(SSL_CTX_add_extra_chain_cert)");
+ }
+  }
}
   return 0;
 }
--
1.7.10.2 (Apple Git-33)




Re: [Openvpn-devel] [PATCH] Floating: Add support for floating in TLS mode (upd.)

2013-11-15 Thread Heikki Hannikainen
On Wed, Oct 30, 2013 at 10:12 PM, André Valentin  wrote:
> Add support for floating in tls mode using the HMAC of a packet. It costs
> a roundtrip through the clients. Its security comes from a secret key, both
> peers have. This key and the data form the signature used, which is then
> checked againts existing peer connections. Therefore a good auth algo is
> recommended.
>
> URL: https://community.openvpn.net/openvpn/ticket/49

I took a look at this patch, since I am frustrated by the same
problem, so +1 for the conceptual issue. It seems to work:

Nov 15 15:18:01 gw ovpn-udp[9964]: hessu/85.188.36.24.36:58639 MULTI:
Detected floating by hmac test, new client address:
[AF_INET]85.188.36.24.36:58640
Nov 15 15:18:01 gw ovpn-udp[9964]: hessu/85.188.36.24.36:58640 MULTI:
Floated with HMAC authentication to a new client address:
[AF_INET]85.188.36.24.36:58640

In this case the client's source port changed, because the client is
behind a NAT device, and the client didn't have traffic within 60
seconds (the NAT device's UDP session timeout). Without this patch
--float does not have any effect on the multi-client tls-server setup,
and after a 60-second break in traffic the session is dead (until
keepalive ping timeout and reconnection). I'd like to use a large
keepalive timer, since on mobile devices a 29-second keepalive tends
to drain the battery.

I think, however, there might be a problem with the patch: When a
packet is received from a srcip-srcport which does not belong to any
current client, it goes and scans through the whole client list,
calculating a HMAC for the packet contents using the HMAC key of each
and every client, effectively looking for a match in HMAC keys. An
attacker could transmit large packets with random source IP addresses
to a server which has a large number of clients (think 2000 or
something? how large client loads are people having, anyway?), and
cause extra CPU load, maybe enough to DOS the server with a smallish
packet rate.

On the other hand, it is commonly quite easy to DOS servers by sheer
packet/data volume... but we shouldn't make it any easier with new
developments.

The more elegant way would be to have a session ID in each data
packet, which would used to look up the client, and then a single HMAC
calculation would tell if the packet is from that client. No need for
scanning. That session ID could be used even for every packet instead
of looking at the client IP address+port at all initially (well, for
sending any following packets you'd use the peer's latest source
address).

Putting a session ID in the data packet would require changing the
on-the-wire data packet format, and make the packets a bit larger. The
ID doesn't have to be very large, though, since it's not secret or
used for keying, just an integer large enough to index the maximum
number of VPN clients on a single server. This would require a new
protocol version, backward compatibility and all that work.

- Hessu



Re: [Openvpn-devel] [PATCH v3] Floating: Add support for floating in TLS mode

2013-12-30 Thread Heikki Hannikainen
On Mon, Dec 23, 2013 at 4:43 PM, André Valentin  wrote:
> On 23.12.2013 12:32, Gert Doering wrote:
>> We've discussed the patch on the Munich Hackathon, and we intend to do
>> it in a different way.  James wants to change the packet format for
>> data packets anyway (due to bad alignment in the current packet format),
>> and the new format would have the option to send the session ID in the
>> data packet as well, like "normally it's not sent, but if we have't heard
>> from the server longer than  time units, send a session ID so that
>> hmac-float will work".
>
> Of course, this is a better idea and should have less problems with scaling.
> But there should be an option for the client to always send the session id,
> so that there are no timeouts if the user decides to do so.

+1 to that - it'd be really neat if floating would work seamlessly
without timeouts + retransmissions. It'd be very useful on mobile
devices, which often get hit by NAT timeouts on UDP (around 30 or 60
seconds typically), switches between WIFI and 3G, and keepalive pings
with tight timers (sub-30-second range) consume a lot of battery.

The regular SSL session ID is maybe a bit long to be sent with each
and every packet, though.

Which other enhancements to the packet format are necessary? I
gathered that the encrypted data needs to be aligned for AES-NI to
work, which probably means that either the packet headers need to be
padded so that the data starts on a nice word boundary, or that the
data packet needs to begin with the encrypted data and all the headers
need to be placed in the end of the packet. Right?

And then some backwards compatibility in the handshake - it'd be good
if old clients could optionally be allowed on new servers using the
new packet format.

>> This code has not been written yet, though, and we have no specific
>> timelines when it will happen.
> So what does this mean for my work and the current format? Is there still
> a possibility that it will be integrated for the 'old packet format'?

We could also start working on the new packet format, do a prototype
if you'd like. Just need to figure out what the exact requirements
would be.

 - Hessu



Re: [Openvpn-devel] session-id implementation

2014-08-12 Thread Heikki Hannikainen

On Wed, 9 Jul 2014, Arne Schwabe wrote:


Am 29.06.14 18:13, schrieb Arne Schwabe:

Am 27.03.14 09:57, schrieb Lev Stipakov:

Hi,

Same patch with added NULL check in push.c:308. Turns out that
peer_info might be NULL.


I looked at the patched, a few minor nitpicks:


One more little nitpick:

  uint32_t sess;

   ...

  sess = ((P_DATA_V2 << P_OPCODE_SHIFT) | ks->key_id) | (multi->vpn_session_id 
<< 8);
  ASSERT (buf_write_prepend (buf, &sess, 4));

I think this will cause byte sex issues when talking between big and 
little endian machines.


A strategically placed htonl() + ntohl() pair might help.  Have to make 
sure that the opcode really goes in as the first byte on the wire, and 
that the next 3 bytes are the 3 least significant bytes of the session ID.


  uint32_t sess = htonl(
(((P_DATA_V2 << P_OPCODE_SHIFT) | ks->key_id) << 24)
| (multi->vpn_session_id & 0xFF)
  );
  ASSERT (buf_write_prepend (buf, &sess, 4));

Right? Maybe? :)

I suppose this patch will improve things quite a bit for mobile clients 
which switch from network to another.


  - Hessu