Re: [Openvpn-devel] GUI repo

2016-02-07 Thread Gert Doering
Hi,

On Sun, Feb 07, 2016 at 03:25:51PM -0500, Selva Nair wrote:
> So the idea is to keep the current manifest  in the GUI distributed with
> 2.3 and remove the HigestAvailable from that to be distributed with 2.4.
> This change is also required to let admin users (with UAC) use interactive
> service, as the plan is to make the GUI not connect to the service if run
> with higher privileges (see my pending pull request).

+1

(just for the record) :-)

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature


Re: [Openvpn-devel] GUI repo

2016-02-07 Thread Gert Doering
Hi,

On Mon, Feb 08, 2016 at 01:12:37AM +0500,  ?? wrote:
> there's still "Start OpenVPN directly"
> 
> https://github.com/OpenVPN/openvpn-gui/blob/master/openvpn.c#L724
> 
> in such case admin rights are still required for routes manipulation.

In this case, it would just not work.

There's basically four cases:

 - you have no admin rights and use the iservice -> works
 - you have admin rights and do not use the iservice -> works
 - you have no admin rights and use *tap* with no routes and no v6 -> works
 - it does not work

I do not see a real problem here.  If you do not want to use the iservice,
you need to either have admin rights or stick to tap, done.

> maybe we should release two installers (or make a checkbox in installer?)
> 
> 1) regular mode (with highest priv manifest)
> 2) paranoya mode (without highest priv)
> 
> 
> those who really care will choose whatever they want to

Where would the benefit in refusing to use the iservice and sticking to
"highest priv manifest"?  All you get by doing this is "larger exposed
surface to exploitable bugs, as more code has to run with elevated privs"

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature


Re: [Openvpn-devel] GUI repo

2016-02-07 Thread Илья Шипицин
as far as I understand it is up to user whether to install openvpn as a
service or not.
if openvpn is not installed as a service, highest priv is required, right?

2016-02-08 1:29 GMT+05:00 Selva Nair :

> Hi,
>
> On Sun, Feb 7, 2016 at 3:12 PM, Илья Шипицин  wrote:
>
>> there's still "Start OpenVPN directly"
>>
>> https://github.com/OpenVPN/openvpn-gui/b, lob/master/openvpn.c#L724
>> 
>>
>> in such case admin rights are still required for routes manipulation.
>>
>
> Once interactive service is available in the distribution, such usage
> would require the user to explicitly start the GUI with admin rights.
>
>
>> maybe we should release two installers (or make a checkbox in installer?)
>>
>> 1) regular mode (with highest priv manifest)
>> 2) paranoya mode (without highest priv)
>>
>
> For 2.4 just one installer with privilege request in the manifest would be
> enough. No need to force UAC prompt for privilege escalation when not
> required.
>
> Selva
>
>
>


Re: [Openvpn-devel] GUI repo

2016-02-07 Thread Selva Nair
Hi,

On Sun, Feb 7, 2016 at 3:12 PM, Илья Шипицин  wrote:

> there's still "Start OpenVPN directly"
>
> https://github.com/OpenVPN/openvpn-gui/b, lob/master/openvpn.c#L724
> 
>
> in such case admin rights are still required for routes manipulation.
>

Once interactive service is available in the distribution, such usage would
require the user to explicitly start the GUI with admin rights.


> maybe we should release two installers (or make a checkbox in installer?)
>
> 1) regular mode (with highest priv manifest)
> 2) paranoya mode (without highest priv)
>

For 2.4 just one installer with privilege request in the manifest would be
enough. No need to force UAC prompt for privilege escalation when not
required.

Selva


Re: [Openvpn-devel] GUI repo

2016-02-07 Thread Selva Nair
Hi,

On Sun, Feb 7, 2016 at 11:00 AM, Илья Шипицин  wrote:

> if you mean interactive service, keep in mind that people sometimes start
> openvpn as a child of openvpn-gui, not as a service


Requiring HighestAvailable was an interim solution until interactive
service becomes available.  Users who do not want to use the interactive
service can use Run As Administrator.

So the idea is to keep the current manifest  in the GUI distributed with
2.3 and remove the HigestAvailable from that to be distributed with 2.4.
This change is also required to let admin users (with UAC) use interactive
service, as the plan is to make the GUI not connect to the service if run
with higher privileges (see my pending pull request).

Selva


[Openvpn-devel] [PATCH] Report Windows bitness

2016-02-07 Thread Lev Stipakov
Trac #599

Signed-off-by: Lev Stipakov 
---
 src/openvpn/win32.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c
index 6c6ac4c..5702304 100644
--- a/src/openvpn/win32.c
+++ b/src/openvpn/win32.c
@@ -1323,6 +1323,20 @@ win32_version_info()
 }
 }

+bool
+win32_is_64bit()
+{
+#if defined(_WIN64)
+return true;  // 64-bit programs run only on Win64
+#elif defined(_WIN32)
+// 32-bit programs run on both 32-bit and 64-bit Windows
+BOOL f64 = FALSE;
+return IsWow64Process(GetCurrentProcess(), ) && f64;
+#else
+return false; // Win64 does not support Win16
+#endif
+}
+
 const char *
 win32_version_string(struct gc_arena *gc, bool add_name)
 {
@@ -1349,6 +1363,8 @@ win32_version_string(struct gc_arena *gc, bool add_name)
 break;
 }

+buf_printf (, win32_is_64bit() ? " 64bit" : " 32bit");
+
 return (const char *)out.data;
 }

-- 
1.9.1




Re: [Openvpn-devel] [PATCH] Fix misleading socket error code on Windows

2016-02-07 Thread Leonardo
Hi. I've made some mistakes on my previous message. I should have been
more patient. After further investigation, I realized that ETIMEDOUT
is defined in several header files:

#define ETIMEDOUT WSAETIMEDOUT - WinSock2.h
#define ETIMEDOUT 138 - winsock.h
#define ETIMEDOUT WSAETIMEDOUT - winerror.h

WSAETIMEDOUT has the right value:

#define WSAETIMEDOUT (WSABASEERR+60) - WinSock2.h
#define WSAETIMEDOUT (WSABASEERR+60) - winsock.h
#define WSAETIMEDOUT 10060L - winerror.h

So it seems like socket.c is getting its value from winsock.h. Here's
a better patch:

Signed-off-by: Leonardo Basilio 
---
 src/openvpn/socket.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 396fa54..37fa129 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -1177,7 +1177,11 @@ openvpn_connect (socket_descriptor_t sd,
 {
   if (--connect_timeout < 0)
 {
+#ifdef WIN32
+  status = WSAETIMEDOUT;
+#else
   status = ETIMEDOUT;
+#endif
   break;
 }
   openvpn_sleep (1);
-- 
2.7.0.windows.1


2016-02-07 16:43 GMT-02:00 Leonardo :
> Hi. First of all, this is my first contribution ever to an open source
> project. I hope I'm doing this right.
>
> After installing OpenVPN 2.3.10 on my Windows computer and trying to
> connect to a VPN server, I was getting this error message:
>
> TCP: connect to [AF_INET]x.x.x.x:80 failed, will try again in 5
> seconds: The system tried to join a drive to a directory on a joined
> drive.
>
> Hours of googling and no real solution later, I've decided to check
> the source code and see if I could find the real cause of this error.
> It was a timeout error. Indeed there's a mismapping going on, as
> suggested here:
>
> http://sourceforge.net/p/openvpn/mailman/message/33101265/
>
> I have both MinGW-w64 and Visual Studio 2015, and in both headers
> WSAETIMEDOUT is defined as ETIMEDOUT which is defined as 138, despite
> the online documentation saying that WSAETIMEDOUT should be 10060.
>
> https://msdn.microsoft.com/en-us/library/windows/desktop/ms740668%28v=vs.85%29.aspx
>
> I couldn't find any explanation about this, but based on other
> threads, this issue seems to be around for quite some time now. After
> the patch below, the right error message comes out:
>
> TCP: connect to [AF_INET]x.x.x.x:80 failed: Connection timed out 
> (WSAETIMEDOUT)
>
> I understand that's not the most elegant solution, but given the above
> said, I don't see an alternative. There are many forum threads out
> there just because of this misleading error message.
>
>
> Signed-off-by: Leonardo Basilio 
> ---
>  src/openvpn/socket.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
> index 396fa54..c01ef3d 100644
> --- a/src/openvpn/socket.c
> +++ b/src/openvpn/socket.c
> @@ -1177,7 +1177,11 @@ openvpn_connect (socket_descriptor_t sd,
>  {
>if (--connect_timeout < 0)
>  {
> +#ifdef WIN32
> +  status = 10060;
> +#else
>status = ETIMEDOUT;
> +#endif
>break;
>  }
>openvpn_sleep (1);
> --
> 2.7.0.windows.1



Re: [Openvpn-devel] GUI repo

2016-02-07 Thread Илья Шипицин
there's still "Start OpenVPN directly"

https://github.com/OpenVPN/openvpn-gui/blob/master/openvpn.c#L724

in such case admin rights are still required for routes manipulation.


maybe we should release two installers (or make a checkbox in installer?)

1) regular mode (with highest priv manifest)
2) paranoya mode (without highest priv)


those who really care will choose whatever they want to


2016-02-08 0:27 GMT+05:00 Gert Doering :

> Hi,
>
> On Sun, Feb 07, 2016 at 09:00:28PM +0500,  ?? wrote:
> > if you mean interactive service, keep in mind that people sometimes start
> > openvpn as a child of openvpn-gui, not as a service
>
> Interactive Service is running openvpn.exe under control of openvpn-gui,
> and you won't notice anything different - except that it magically works
> even if the user has no permissions to set routes, and the "want admin!"
> flag is not set on the GUI.
>
> And, of course, that bugs in openvpn-gui or openvpn will not lead to a
> privilege breach anymore.
>
> gert
> --
> USENET is *not* the non-clickable part of WWW!
>//
> www.muc.de/~gert/
> Gert Doering - Munich, Germany
> g...@greenie.muc.de
> fax: +49-89-35655025
> g...@net.informatik.tu-muenchen.de
>


[Openvpn-devel] [PATCH 08/10] Add AEAD cipher support (GCM)

2016-02-07 Thread Steffan Karger
Add Authenticated Encryption with Additional Data (AEAD) support for
ciphers, which obviates the need for a separate HMAC step.  The MAC is
integrated into the cipher and the MAC tag is prepended to the payload.

This patch is inspired by the patch originally submitted by Kenny Root
on the openvpn-devel mailinglist, but does a number things differently:
 * Don't support XTS (makes no sense for VPN)
 * Don't support CCM (needs extra code to make it actually work)
 * Don't force the user to specify "auth none" (that would break
   tls-auth)
 * Add support for PolarSSL (and change internal API for this)
 * Update openvpn frame size ('link mtu') calculation for AEAD modes
 * Use the HMAC key as an implicit part of the IV to save 8 bytes per
   data channel network packet.
 * Also authenticate the opcode/peer-id as AD in P_DATA_V2 packets.

By using the negotiated HMAC key as an implicit part of the IV for
AEAD-mode ciphers in TLS mode, we can save (at least) 8 bytes on each
packet sent.  This is particularly interesting for connections which
transfer many small packets, such as remote desktop or voip connections.

The current AEAD-mode ciphers (for now GCM) are based on CTR-mode cipher
operation, which requires the IV to be unique (but does not require
unpredictability).

IV uniqueness is guaranteed by using a combination of at least 64-bits
of the HMAC key (unique per TLS session), and a 32-bit packet counter.
The last 32-bit word of the 128-bit cipher block is not part of the IV,
but is used as a block counter.

AEAD cipher mode is not available for static key mode, since IV
uniqueness is harder the guarantee over sessions, and I believe
supporting AEAD in static key mode too is not worth the extra
complexity.  Modern setups should simply use TLS mode.

Signed-off-by: Steffan Karger 
---
 configure.ac  |  30 +++-
 src/openvpn/crypto.c  | 353 +++---
 src/openvpn/crypto.h  |  65 ++--
 src/openvpn/crypto_backend.h  |  69 +
 src/openvpn/crypto_openssl.c  |  85 +-
 src/openvpn/crypto_openssl.h  |   7 +
 src/openvpn/crypto_polarssl.c | 105 +++--
 src/openvpn/crypto_polarssl.h |   3 +
 src/openvpn/forward.c |  49 +++---
 src/openvpn/ssl.c | 102 +---
 src/openvpn/ssl.h |  42 -
 11 files changed, 810 insertions(+), 100 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7ff2435..3609207 100644
--- a/configure.ac
+++ b/configure.ac
@@ -93,6 +93,13 @@ AC_ARG_ENABLE(
 )

 AC_ARG_ENABLE(
+   [aead-modes],
+   [AS_HELP_STRING([--disable-aead-modes], [disable AEAD crypto modes 
@<:@default=yes@:>@])],
+   ,
+   [enable_aead_modes="yes"]
+)
+
+AC_ARG_ENABLE(
[x509-alt-username],
[AS_HELP_STRING([--enable-x509-alt-username], [enable the 
--x509-username-field feature @<:@default=no@:>@])],
,
@@ -821,6 +828,13 @@ if test "${with_crypto_library}" = "openssl"; then
AC_DEFINE([HAVE_OPENSSL_ENGINE], [1], [OpenSSL engine support 
available])
fi

+   have_crypto_aead_modes="yes"
+   AC_CHECK_FUNCS(
+   [EVP_aes_256_gcm],
+   ,
+   [have_crypto_aead_modes="no"; break]
+   )
+
CFLAGS="${saved_CFLAGS}"
LIBS="${saved_LIBS}"

@@ -897,9 +911,19 @@ elif test "${with_crypto_library}" = "polarssl"; then
AC_MSG_ERROR([PolarSSL compiled with PKCS11, while 
OpenVPN is not])
fi
fi
+
+   have_crypto_aead_modes="yes"
+   AC_CHECK_FUNCS(
+   [ \
+   cipher_write_tag \
+   cipher_check_tag \
+   ],
+   ,
+   [have_crypto_aead_modes="no"; break]
+   )
+
CFLAGS="${saved_CFLAGS}"
LIBS="${saved_LIBS}"
-
have_crypto="yes"
AC_DEFINE([ENABLE_CRYPTO_POLARSSL], [1], [Use PolarSSL library])
CRYPTO_CFLAGS="${POLARSSL_CFLAGS}"
@@ -1054,6 +1078,10 @@ test "${enable_strict_options}" = "yes" && 
AC_DEFINE([ENABLE_STRICT_OPTIONS_CHEC
 if test "${enable_crypto}" = "yes"; then
test "${have_crypto}" != "yes" && AC_MSG_ERROR([${with_crypto_library} 
crypto is required but missing])
test "${enable_crypto_ofb_cfb}" = "yes" && 
AC_DEFINE([ENABLE_OFB_CFB_MODE], [1], [Enable OFB and CFB cipher modes])
+   if test "${enable_aead_modes}" = "yes"; then
+   test "${have_crypto_aead_modes}" = "yes" && 
AC_DEFINE([HAVE_AEAD_CIPHER_MODES], [1], [Use crypto library])
+   test "${have_crypto_aead_modes}" != "yes" && AC_MSG_ERROR([AEAD 
modes required but missing])
+   fi
OPTIONAL_CRYPTO_CFLAGS="${OPTIONAL_CRYPTO_CFLAGS} ${CRYPTO_CFLAGS}"
OPTIONAL_CRYPTO_LIBS="${OPTIONAL_CRYPTO_LIBS} ${CRYPTO_LIBS}"
AC_DEFINE([ENABLE_CRYPTO], [1], [Enable crypto library])
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 9c0c353..21790a2 

[Openvpn-devel] [PATCH 10/10] Add preliminary server-side support for negotiable crypto parameters

2016-02-07 Thread Steffan Karger
Add preliminary support for Negotiable Crypto Parameters 'level 2'
(IV_NCP=2), as proposed by James Yonan on the openvpn-devel mailinglist:
http://comments.gmane.org/gmane.network.openvpn.devel/9385

This patch:
 * Makes the server advertise "IV_NCP=2", if --push-peer-info 2 is enabled.
 * Pushes a 'cipher XXX' directive to the client, if the client advertises
   "IV_NCP=2", where XXX is the cipher set in the server config file.

This enables clients that have support for IV_NCP to connect to a server,
even when the client does not have the correct cipher specified in it's
config file.

Since pushing the cipher directive is quite similar to pushing peer-id, I
moved peer-id pushing to same prepare_push_reply() function I created for
pushing cipher.  Adding these directives as regular push options allows us
to use the existing 'push-continuation' infrastructure.  Note that we
should not reduce safe_cap in send_push_reply, because it was never
increased to account for peer-id.

This is a preliminary patch, which will be followed by more patches to add
client support, and configurability.

Signed-off-by: Steffan Karger 
---
 src/openvpn/push.c | 97 +-
 src/openvpn/push.h |  2 --
 src/openvpn/ssl.c  |  4 +++
 3 files changed, 86 insertions(+), 17 deletions(-)

diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index d4f3cb6..051aef6 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -40,6 +40,30 @@

 #if P2MP

+/**
+ * Add an option to the push list by providing a format string.
+ *
+ * The string added to the push options is allocated in o->gc, so fmt may be
+ * free'd by the caller before the option is sent to the peer.
+ *
+ * @param oThe current connection's options
+ * @param msglevel The message level to use when printing errors
+ * @param fmt  Format string for the option
+ * @param ...  Format string arguments
+ *
+ * @return true on success, false on failure.
+ */
+static bool push_option_fmt(struct options *o, int msglevel,
+const char *fmt, ...)
+#ifdef __GNUC__
+#if __USE_MINGW_ANSI_STDIO
+__attribute__ ((format (gnu_printf, 3, 4)))
+#else
+__attribute__ ((format (__printf__, 3, 4)))
+#endif
+#endif
+;
+
 /*
  * Auth username/password
  *
@@ -239,7 +263,47 @@ send_push_request (struct context *c)

 #if P2MP_SERVER

-bool
+/**
+ * Prepare push options, based on local options and available peer info.
+ *
+ * @param options  Connection options
+ * @param tls_multiTLS state structure for the current tunnel
+ *
+ * @return true on success, false on failure.
+ */
+static bool
+prepare_push_reply (struct options *o, struct tls_multi *tls_multi)
+{
+  const char *optstr = NULL;
+  const char * const peer_info = tls_multi->peer_info;
+
+  /* Send peer-id if client supports it */
+  optstr = peer_info ? strstr(peer_info, "IV_PROTO=") : NULL;
+  if (optstr)
+{
+  int proto = 0;
+  int r = sscanf(optstr, "IV_PROTO=%d", );
+  if ((r == 1) && (proto >= 2))
+   {
+ push_option_fmt(o, M_USAGE, "peer-id %d", tls_multi->peer_id);
+   }
+}
+
+  /* Push cipher if client supports Negotiable Crypto Parameters */
+  optstr = peer_info ? strstr(peer_info, "IV_NCP=") : NULL;
+  if (optstr)
+{
+  int ncp = 0;
+  int r = sscanf(optstr, "IV_NCP=%d", );
+  if ((r == 1) && (ncp == 2))
+   {
+ push_option_fmt(o, M_USAGE, "cipher %s", o->ciphername);
+   }
+}
+  return true;
+}
+
+static bool
 send_push_reply (struct context *c)
 {
   struct gc_arena gc = gc_new ();
@@ -309,19 +373,6 @@ send_push_reply (struct context *c)
   if (multi_push)
 buf_printf (, ",push-continuation 1");

-  /* Send peer-id if client supports it */
-  if (c->c2.tls_multi->peer_info)
-{
-  const char* proto_str = strstr(c->c2.tls_multi->peer_info, "IV_PROTO=");
-  if (proto_str)
-   {
- int proto = 0;
- int r = sscanf(proto_str, "IV_PROTO=%d", );
- if ((r == 1) && (proto >= 2))
-   buf_printf(, ",peer-id %d", c->c2.tls_multi->peer_id);
-   }
-  }
-
   if (BLEN () > sizeof(cmd)-1)
 {
   const bool status = send_control_channel_string (c, BSTR (), D_PUSH);
@@ -409,6 +460,21 @@ push_options (struct options *o, char **p, int msglevel, 
struct gc_arena *gc)
   push_option (o, opt, msglevel);
 }

+static bool push_option_fmt(struct options *o, int msglevel,
+const char *format, ...)
+{
+  va_list arglist;
+  char tmp[256] = {0};
+  int len = -1;
+  va_start (arglist, format);
+  len = vsnprintf (tmp, sizeof(tmp), format, arglist);
+  va_end (arglist);
+  if (len > sizeof(tmp)-1)
+return false;
+  push_option (o, string_alloc (tmp, >gc), msglevel);
+  return true;
+}
+
 void
 push_reset (struct options *o)
 {
@@ -442,7 +508,8 @@ process_incoming_push_request (struct context *c)
}
   else
{
- if (send_push_reply (c))
+ if (prepare_push_reply(>options, 

[Openvpn-devel] [PATCH 09/10] Add cipher name translation for OpenSSL.

2016-02-07 Thread Steffan Karger
This keeps naming consistent. For example, instead of id-aes128-GCM use
AES-128-GCM, which is more like AES-128-CBC.

Signed-off-by: Steffan Karger 
---
 src/openvpn/crypto.c  | 40 +++-
 src/openvpn/crypto_backend.h  | 30 ++
 src/openvpn/crypto_openssl.c  | 23 ++-
 src/openvpn/crypto_polarssl.c | 43 +++
 src/openvpn/options.c |  3 ++-
 5 files changed, 84 insertions(+), 55 deletions(-)

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 21790a2..9620f72 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -797,7 +797,7 @@ init_key_ctx (struct key_ctx *ctx, struct key *key,

   msg (D_HANDSHAKE, "%s: Cipher '%s' initialized with %d bit key",
   prefix,
-  cipher_kt_name(kt->cipher),
+  translate_cipher_name_to_openvpn(cipher_kt_name(kt->cipher)),
   kt->cipher_length *8);

   dmsg (D_SHOW_KEYS, "%s: CIPHER KEY: %s", prefix,
@@ -1670,4 +1670,42 @@ get_random()
   return l;
 }

+static const cipher_name_pair *
+get_cipher_name_pair(const char *cipher_name) {
+  const cipher_name_pair *pair;
+  size_t i = 0;
+
+  /* Search for a cipher name translation */
+  for (; i < cipher_name_translation_table_count; i++)
+{
+  pair = _name_translation_table[i];
+  if (0 == strcmp (cipher_name, pair->openvpn_name) ||
+ 0 == strcmp (cipher_name, pair->lib_name))
+ return pair;
+}
+
+  /* Nothing found, return null */
+  return NULL;
+}
+
+const char *
+translate_cipher_name_from_openvpn (const char *cipher_name) {
+  const cipher_name_pair *pair = get_cipher_name_pair(cipher_name);
+
+  if (NULL == pair)
+return cipher_name;
+
+  return pair->lib_name;
+}
+
+const char *
+translate_cipher_name_to_openvpn (const char *cipher_name) {
+  const cipher_name_pair *pair = get_cipher_name_pair(cipher_name);
+
+  if (NULL == pair)
+return cipher_name;
+
+  return pair->openvpn_name;
+}
+
 #endif /* ENABLE_CRYPTO */
diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h
index 3e030d1..d389ba4 100644
--- a/src/openvpn/crypto_backend.h
+++ b/src/openvpn/crypto_backend.h
@@ -41,6 +41,16 @@
 /* TLS uses a tag of 128 bytes, let's do the same for OpenVPN */
 #define OPENVPN_AEAD_TAG_LENGTH 16

+/** Struct used in cipher name translation table */
+typedef struct {
+  const char *openvpn_name;/**< Cipher name used by OpenVPN */
+  const char *lib_name;/**< Cipher name used by crypto library 
*/
+} cipher_name_pair;
+
+/** Cipher name translation table */
+extern const cipher_name_pair cipher_name_translation_table[];
+extern const size_t cipher_name_translation_table_count;
+
 /*
  * This routine should have additional OpenSSL crypto library initialisations
  * used by both crypto and ssl components of OpenVPN.
@@ -594,4 +604,24 @@ void hmac_ctx_update (hmac_ctx_t *ctx, const uint8_t *src, 
int src_len);
  */
 void hmac_ctx_final (hmac_ctx_t *ctx, uint8_t *dst);

+/**
+ * Translate an OpenVPN cipher name to a crypto library cipher name.
+ *
+ * @param cipher_name  An OpenVPN cipher name
+ *
+ * @return The corresponding crypto library cipher name, or NULL
+ * if no matching cipher name was found.
+ */
+const char * translate_cipher_name_from_openvpn (const char *cipher_name);
+
+/**
+ * Translate a crypto library cipher name to an OpenVPN cipher name.
+ *
+ * @param cipher_name  A crypto library cipher name
+ *
+ * @return The corresponding OpenVPN cipher name, or NULL if no
+ * matching cipher name was found.
+ */
+const char * translate_cipher_name_to_openvpn (const char *cipher_name);
+
 #endif /* CRYPTO_BACKEND_H_ */
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 6fdff4f..d73634c 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -240,17 +240,14 @@ crypto_init_dmalloc (void)
 }
 #endif /* DMALLOC */

-const char *
-translate_cipher_name_from_openvpn (const char *cipher_name) {
-  // OpenSSL doesn't require any translation
-  return cipher_name;
-}
+const cipher_name_pair cipher_name_translation_table[] = {
+{ "AES-128-GCM", "id-aes128-GCM" },
+{ "AES-192-GCM", "id-aes192-GCM" },
+{ "AES-256-GCM", "id-aes256-GCM" },
+};
+const size_t cipher_name_translation_table_count =
+sizeof (cipher_name_translation_table) / sizeof 
(*cipher_name_translation_table);

-const char *
-translate_cipher_name_to_openvpn (const char *cipher_name) {
-  // OpenSSL doesn't require any translation
-  return cipher_name;
-}

 void
 show_available_ciphers ()
@@ -286,9 +283,9 @@ show_available_ciphers ()
  const char *ssl_only = cipher_kt_mode_cbc(cipher) ?
  "" : " (TLS client/server mode)";

- printf ("%s %d bit default key (%s)%s\n", OBJ_nid2sn (nid),
- 

[Openvpn-devel] [PATCH 06/10] Change openvpn_encrypt() to append to work buffer only

2016-02-07 Thread Steffan Karger
Preparation for AEAD cipher modes, which also have to authenticate the
opcode and peer-id of packets.  To supply that information to
openvpn_encrypt(), I want to simply write those to the work buffer
before calling openvpn_encrypt().  That however requires that
openvpn_encrypt() never prepends something to the work buffer.

Signed-off-by: Steffan Karger 
---
 src/openvpn/crypto.c | 55 ++--
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index db52182..e92125e 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -93,6 +93,8 @@ openvpn_encrypt (struct buffer *buf, struct buffer work,
   if (buf->len > 0 && opt)
 {
   const struct key_ctx *ctx = >key_ctx_bi.encrypt;
+  uint8_t *mac_out = NULL;
+  const uint8_t *hmac_start = NULL;

   /* Do Encrypt from buf -> work */
   if (ctx->cipher)
@@ -102,6 +104,17 @@ openvpn_encrypt (struct buffer *buf, struct buffer work,
  const cipher_kt_t *cipher_kt = cipher_ctx_get_cipher_kt (ctx->cipher);
  int outlen;

+ /* initialize work buffer with FRAME_HEADROOM bytes of prepend 
capacity */
+ ASSERT (buf_init (, FRAME_HEADROOM (frame)));
+
+ /* Reserve space for HMAC */
+ if (ctx->hmac)
+   {
+ mac_out = buf_write_alloc (, hmac_ctx_size(ctx->hmac));
+ ASSERT (mac_out);
+ hmac_start = BEND();
+   }
+
  if (cipher_kt_mode_cbc(cipher_kt))
{
  CLEAR (iv_buf);
@@ -137,12 +150,12 @@ openvpn_encrypt (struct buffer *buf, struct buffer work,
  ASSERT (0);
}

- /* initialize work buffer with FRAME_HEADROOM bytes of prepend 
capacity */
- ASSERT (buf_init (, FRAME_HEADROOM (frame)));
-
  /* set the IV pseudo-randomly */
  if (opt->flags & CO_USE_IV)
-   dmsg (D_PACKET_CONTENT, "ENCRYPT IV: %s", format_hex (iv_buf, 
iv_size, 0, ));
+{
+  ASSERT (buf_write(, iv_buf, iv_size));
+  dmsg (D_PACKET_CONTENT, "ENCRYPT IV: %s", format_hex (iv_buf, 
iv_size, 0, ));
+}

  dmsg (D_PACKET_CONTENT, "ENCRYPT FROM: %s",
   format_hex (BPTR (buf), BLEN (buf), 80, ));
@@ -165,27 +178,16 @@ openvpn_encrypt (struct buffer *buf, struct buffer work,
}

  /* Encrypt packet ID, payload */
- ASSERT (cipher_ctx_update (ctx->cipher, BPTR (), , BPTR 
(buf), BLEN (buf)));
+ ASSERT (cipher_ctx_update (ctx->cipher, BEND (), , BPTR 
(buf), BLEN (buf)));
  ASSERT (buf_inc_len(, outlen));

  /* Flush the encryption buffer */
- ASSERT (cipher_ctx_final(ctx->cipher, BPTR () + outlen, 
));
+ ASSERT (cipher_ctx_final(ctx->cipher, BEND (), ));
  ASSERT (buf_inc_len(, outlen));

  /* For all CBC mode ciphers, check the last block is complete */
  ASSERT (cipher_kt_mode (cipher_kt) != OPENVPN_MODE_CBC ||
  outlen == iv_size);
-
- /* prepend the IV to the ciphertext */
- if (opt->flags & CO_USE_IV)
-   {
- uint8_t *output = buf_prepend (, iv_size);
- ASSERT (output);
- memcpy (output, iv_buf, iv_size);
-   }
-
- dmsg (D_PACKET_CONTENT, "ENCRYPT TO: %s",
-  format_hex (BPTR (), BLEN (), 80, ));
}
   else /* No Encryption */
{
@@ -195,22 +197,29 @@ openvpn_encrypt (struct buffer *buf, struct buffer work,
  packet_id_alloc_outgoing (>packet_id.send, , BOOL_CAST 
(opt->flags & CO_PACKET_ID_LONG_FORM));
  ASSERT (packet_id_write (, buf, BOOL_CAST (opt->flags & 
CO_PACKET_ID_LONG_FORM), true));
}
+ if (ctx->hmac)
+   {
+ hmac_start = BPTR(buf);
+ ASSERT (mac_out = buf_prepend (buf, hmac_ctx_size(ctx->hmac)));
+   }
+ buf_write_prepend(buf, BPTR(), BLEN());
  work = *buf;
}

   /* HMAC the ciphertext (or plaintext if !cipher) */
   if (ctx->hmac)
{
- uint8_t *output = NULL;
-
  hmac_ctx_reset (ctx->hmac);
- hmac_ctx_update (ctx->hmac, BPTR(), BLEN());
- output = buf_prepend (, hmac_ctx_size(ctx->hmac));
- ASSERT (output);
- hmac_ctx_final (ctx->hmac, output);
+ hmac_ctx_update (ctx->hmac, hmac_start, BEND() - hmac_start);
+ hmac_ctx_final (ctx->hmac, mac_out);
+ dmsg (D_PACKET_CONTENT, "ENCRYPT HMAC: %s",
+ format_hex (mac_out, hmac_ctx_size(ctx->hmac), 80, ));
}

   *buf = work;
+
+  dmsg (D_PACKET_CONTENT, "ENCRYPT TO: %s",
+ format_hex (BPTR (), BLEN (), 80, ));
 }

   gc_free ();
-- 
2.5.0




[Openvpn-devel] [PATCH 05/10] Move packet_id into crypto_options

2016-02-07 Thread Steffan Karger
Decouples struct key_state and struct crypto_options. No longer updating
self-referential pointers!

Signed-off-by: Steffan Karger 
---
 src/openvpn/crypto.c | 45 -
 src/openvpn/crypto.h | 10 --
 src/openvpn/init.c   | 11 +--
 src/openvpn/openvpn.h|  2 --
 src/openvpn/packet_id.h  |  6 ++
 src/openvpn/ssl.c| 38 +-
 src/openvpn/ssl_common.h |  2 --
 7 files changed, 52 insertions(+), 62 deletions(-)

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 9679fd0..db52182 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -85,8 +85,7 @@ memcmp_constant_time (const void *a, const void *b, size_t 
size) {

 void
 openvpn_encrypt (struct buffer *buf, struct buffer work,
-const struct crypto_options *opt,
-const struct frame* frame)
+struct crypto_options *opt, const struct frame* frame)
 {
   struct gc_arena gc;
   gc_init ();
@@ -111,11 +110,11 @@ openvpn_encrypt (struct buffer *buf, struct buffer work,
  if (opt->flags & CO_USE_IV)
prng_bytes (iv_buf, iv_size);

- /* Put packet ID in plaintext buffer or IV, depending on cipher 
mode */
- if (opt->packet_id)
+ /* Put packet ID in plaintext buffer */
+ if (packet_id_initialized(>packet_id))
{
  struct packet_id_net pin;
- packet_id_alloc_outgoing (>packet_id->send, , 
BOOL_CAST (opt->flags & CO_PACKET_ID_LONG_FORM));
+ packet_id_alloc_outgoing (>packet_id.send, , 
BOOL_CAST (opt->flags & CO_PACKET_ID_LONG_FORM));
  ASSERT (packet_id_write (, buf, BOOL_CAST (opt->flags & 
CO_PACKET_ID_LONG_FORM), true));
}
}
@@ -124,10 +123,11 @@ openvpn_encrypt (struct buffer *buf, struct buffer work,
  struct packet_id_net pin;
  struct buffer b;

- ASSERT (opt->flags & CO_USE_IV);/* IV and packet-ID required 
*/
- ASSERT (opt->packet_id); /*  for this mode. */
+ /* IV and packet-ID required for this mode. */
+ ASSERT (opt->flags & CO_USE_IV);
+ ASSERT (packet_id_initialized(>packet_id));

- packet_id_alloc_outgoing (>packet_id->send, , true);
+ packet_id_alloc_outgoing (>packet_id.send, , true);
  memset (iv_buf, 0, iv_size);
  buf_set_write (, iv_buf, iv_size);
  ASSERT (packet_id_write (, , true, false));
@@ -189,10 +189,10 @@ openvpn_encrypt (struct buffer *buf, struct buffer work,
}
   else /* No Encryption */
{
- if (opt->packet_id)
+ if (packet_id_initialized(>packet_id))
{
  struct packet_id_net pin;
- packet_id_alloc_outgoing (>packet_id->send, , BOOL_CAST 
(opt->flags & CO_PACKET_ID_LONG_FORM));
+ packet_id_alloc_outgoing (>packet_id.send, , BOOL_CAST 
(opt->flags & CO_PACKET_ID_LONG_FORM));
  ASSERT (packet_id_write (, buf, BOOL_CAST (opt->flags & 
CO_PACKET_ID_LONG_FORM), true));
}
  work = *buf;
@@ -233,8 +233,7 @@ err:
  */
 bool
 openvpn_decrypt (struct buffer *buf, struct buffer work,
-const struct crypto_options *opt,
-const struct frame* frame)
+struct crypto_options *opt, const struct frame* frame)
 {
   static const char error_prefix[] = "Authenticate/Decrypt packet error";
   struct gc_arena gc;
@@ -246,6 +245,9 @@ openvpn_decrypt (struct buffer *buf, struct buffer work,
   struct packet_id_net pin;
   bool have_pin = false;

+  dmsg (D_PACKET_CONTENT, "DECRYPT FROM: %s",
+ format_hex (BPTR (buf), BLEN (buf), 80, ));
+
   /* Verify the HMAC */
   if (ctx->hmac)
{
@@ -325,7 +327,7 @@ openvpn_decrypt (struct buffer *buf, struct buffer work,
  {
if (cipher_kt_mode_cbc(cipher_kt))
  {
-   if (opt->packet_id)
+   if (packet_id_initialized(>packet_id))
  {
if (!packet_id_read (, , BOOL_CAST (opt->flags & 
CO_PACKET_ID_LONG_FORM)))
  CRYPT_ERROR ("error reading CBC packet-id");
@@ -336,8 +338,9 @@ openvpn_decrypt (struct buffer *buf, struct buffer work,
  {
struct buffer b;

-   ASSERT (opt->flags & CO_USE_IV);/* IV and packet-ID 
required */
-   ASSERT (opt->packet_id); /*  for this mode. */
+   /* IV and packet-ID required for this mode. */
+   ASSERT (opt->flags & CO_USE_IV);
+   ASSERT (packet_id_initialized(>packet_id));

buf_set_read (, iv_buf, iv_size);
if (!packet_id_read (, , true))
@@ -353,7 +356,7 @@ openvpn_decrypt (struct buffer *buf, struct buffer work,
   

[Openvpn-devel] [PATCH 03/10] Move crypto_options into key_state and stop using context in SSL-mode.

2016-02-07 Thread Steffan Karger
Moving crypto_options into key_state enables us to stop using the global
context for each packet encrypt/decrypt operation. Decoupling the crypto
from the global context removes the need to copy the relevant parts of
crypto_options for each processed packet, but instead enables us to just
pass along a pointer to the related crypto_options.

This paves the way for an efficient GCM cipher mode implementation, but is
probably fruitful too for threading and/or cipher negotiation.

Signed-off-by: Steffan Karger 
---
 src/openvpn/forward.c| 18 ++
 src/openvpn/init.c   | 21 +
 src/openvpn/ssl.c| 36 +++-
 src/openvpn/ssl.h| 15 +++
 src/openvpn/ssl_common.h |  2 ++
 5 files changed, 51 insertions(+), 41 deletions(-)

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 36a99e6..75cd21d 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -432,6 +432,7 @@ encrypt_sign (struct context *c, bool comp_frag)
 {
   struct context_buffers *b = c->c2.buffers;
   const uint8_t *orig_buf = c->c2.buf.data;
+  struct crypto_options *co = NULL;

 #if P2MP_SERVER
   /*
@@ -462,14 +463,18 @@ encrypt_sign (struct context *c, bool comp_frag)
*/
   if (c->c2.tls_multi)
 {
-  tls_pre_encrypt (c->c2.tls_multi, >c2.buf, >c2.crypto_options);
+  tls_pre_encrypt (c->c2.tls_multi, >c2.buf, );
+}
+  else
+{
+  co = >c2.crypto_options;
 }

   /*
* Encrypt the packet and write an optional
* HMAC signature.
*/
-  openvpn_encrypt (>c2.buf, b->encrypt_buf, >c2.crypto_options, 
>c2.frame);
+  openvpn_encrypt (>c2.buf, b->encrypt_buf, co, >c2.frame);
 #endif
   /*
* Get the address we will be sending the packet to.
@@ -774,6 +779,7 @@ process_incoming_link_part1 (struct context *c, struct 
link_socket_info *lsi, bo
*/
   if (c->c2.buf.len > 0)
 {
+  struct crypto_options *co = NULL;
   if (!link_socket_verify_incoming_addr (>c2.buf, lsi, >c2.from))
link_socket_bad_incoming_addr (>c2.buf, lsi, >c2.from);

@@ -790,7 +796,7 @@ process_incoming_link_part1 (struct context *c, struct 
link_socket_info *lsi, bo
   * will load crypto_options with the correct encryption key
   * and return false.
   */
- if (tls_pre_decrypt (c->c2.tls_multi, >c2.from, >c2.buf, 
>c2.crypto_options, floated))
+ if (tls_pre_decrypt (c->c2.tls_multi, >c2.from, >c2.buf, , 
floated))
{
  interval_action (>c2.tmp_int);

@@ -799,6 +805,10 @@ process_incoming_link_part1 (struct context *c, struct 
link_socket_info *lsi, bo
event_timeout_reset (>c2.ping_rec_interval);
}
}
+  else
+   {
+ co = >c2.crypto_options;
+   }
 #if P2MP_SERVER
   /*
* Drop non-TLS packet if client-connect script/plugin has not
@@ -809,7 +819,7 @@ process_incoming_link_part1 (struct context *c, struct 
link_socket_info *lsi, bo
 #endif

   /* authenticate and decrypt the incoming packet */
-  decrypt_status = openvpn_decrypt (>c2.buf, 
c->c2.buffers->decrypt_buf, >c2.crypto_options, >c2.frame);
+  decrypt_status = openvpn_decrypt (>c2.buf, 
c->c2.buffers->decrypt_buf, co, >c2.frame);

   if (!decrypt_status && link_socket_connection_oriented 
(c->c2.link_socket))
{
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 7e6e448..8fc5c5d 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2048,14 +2048,6 @@ init_crypto_pre (struct context *c, const unsigned int 
flags)
packet_id_persist_load (>c1.pid_persist, c->options.packet_id_file);
 }

-  /* Initialize crypto options */
-
-  if (c->options.use_iv)
-c->c2.crypto_options.flags |= CO_USE_IV;
-
-  if (c->options.mute_replay_warnings)
-c->c2.crypto_options.flags |= CO_MUTE_REPLAY_WARNINGS;
-
 #ifdef ENABLE_PREDICTION_RESISTANCE
   if (c->options.use_prediction_resistance)
 rand_ctx_enable_prediction_resistance();
@@ -2074,6 +2066,13 @@ do_init_crypto_static (struct context *c, const unsigned 
int flags)

   init_crypto_pre (c, flags);

+  /* Initialize flags */
+  if (c->options.use_iv)
+c->c2.crypto_options.flags |= CO_USE_IV;
+
+  if (c->options.mute_replay_warnings)
+c->c2.crypto_options.flags |= CO_MUTE_REPLAY_WARNINGS;
+
   /* Initialize packet ID tracking */
   if (options->replay)
 {
@@ -2277,6 +2276,12 @@ do_init_crypto_tls (struct context *c, const unsigned 
int flags)
   /* Set all command-line TLS-related options */
   CLEAR (to);

+  if (options->use_iv)
+to.crypto_flags |= CO_USE_IV;
+
+  if (options->mute_replay_warnings)
+to.crypto_flags |= CO_MUTE_REPLAY_WARNINGS;
+
   to.crypto_flags_and = ~(CO_PACKET_ID_LONG_FORM);
   if (packet_id_long_form)
 to.crypto_flags_or = CO_PACKET_ID_LONG_FORM;
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index d39f131..6aa9284 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c

[Openvpn-devel] [PATCH 04/10] Move key_ctx_bi into crypto_options

2016-02-07 Thread Steffan Karger
The encrypt and decrypt routines use struct crypto_options as their main
information source.  A struct crypto_options would have a pointer to a
struct key_ctx_bi, which had to be updated at the correct moments to keep
them correct.  Instead of doing this administration, just put the struct
key_ctx_bi inside crypto_options.  Makes the code a little simpler too.

Signed-off-by: Steffan Karger 
---
 src/openvpn/crypto.c |  8 
 src/openvpn/crypto.h |  2 +-
 src/openvpn/init.c   |  4 ++--
 src/openvpn/ssl.c| 32 ++--
 src/openvpn/ssl.h|  1 -
 src/openvpn/ssl_common.h |  2 --
 6 files changed, 21 insertions(+), 28 deletions(-)

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 806a995..9679fd0 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -91,9 +91,9 @@ openvpn_encrypt (struct buffer *buf, struct buffer work,
   struct gc_arena gc;
   gc_init ();

-  if (buf->len > 0 && opt->key_ctx_bi)
+  if (buf->len > 0 && opt)
 {
-  struct key_ctx *ctx = >key_ctx_bi->encrypt;
+  const struct key_ctx *ctx = >key_ctx_bi.encrypt;

   /* Do Encrypt from buf -> work */
   if (ctx->cipher)
@@ -240,9 +240,9 @@ openvpn_decrypt (struct buffer *buf, struct buffer work,
   struct gc_arena gc;
   gc_init ();

-  if (buf->len > 0 && opt->key_ctx_bi)
+  if (buf->len > 0 && opt)
 {
-  struct key_ctx *ctx = >key_ctx_bi->decrypt;
+  const struct key_ctx *ctx = >key_ctx_bi.decrypt;
   struct packet_id_net pin;
   bool have_pin = false;

diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index b32a900..1f84284 100644
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -207,7 +207,7 @@ struct key_ctx_bi
  */
 struct crypto_options
 {
-  struct key_ctx_bi *key_ctx_bi;
+  struct key_ctx_bi key_ctx_bi;
 /**< OpenSSL cipher and HMAC contexts for
  *   both sending and receiving
  *   directions. */
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 8fc5c5d..dcc3ccb 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2134,7 +2134,7 @@ do_init_crypto_static (struct context *c, const unsigned 
int flags)
 }

   /* Get key schedule */
-  c->c2.crypto_options.key_ctx_bi = >c1.ks.static_key;
+  c->c2.crypto_options.key_ctx_bi = c->c1.ks.static_key;

   /* Compute MTU parameters */
   crypto_adjust_frame_parameters (>c2.frame,
@@ -2388,7 +2388,7 @@ do_init_crypto_tls (struct context *c, const unsigned int 
flags)
   /* TLS handshake authentication (--tls-auth) */
   if (options->tls_auth_file)
 {
-  to.tls_auth_key = c->c1.ks.tls_auth_key;
+  to.tls_auth.key_ctx_bi = c->c1.ks.tls_auth_key;
   to.tls_auth.pid_persist = >c1.pid_persist;
   to.tls_auth.flags |= CO_PACKET_ID_LONG_FORM;
   crypto_adjust_frame_parameters (,
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 6aa9284..e3a745d 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -786,7 +786,6 @@ key_state_init (struct tls_session *session, struct 
key_state *ks)
  session->opt->replay_time,
  "SSL", ks->key_id);

-  ks->crypto_options.key_ctx_bi = >key;
   ks->crypto_options.packet_id = session->opt->replay ? >packet_id : NULL;
   ks->crypto_options.pid_persist = NULL;
   ks->crypto_options.flags = session->opt->crypto_flags;
@@ -819,7 +818,7 @@ key_state_free (struct key_state *ks, bool clear)

   key_state_ssl_free(>ks_ssl);

-  free_key_ctx_bi (>key);
+  free_key_ctx_bi (>crypto_options.key_ctx_bi);
   free_buf (>plaintext_read_buf);
   free_buf (>plaintext_write_buf);
   free_buf (>ack_write_buf);
@@ -1072,9 +1071,6 @@ tls_multi_init (struct tls_options *tls_options)
   /* get command line derived options */
   ret->opt = *tls_options;

-  /* set up pointer to HMAC object for TLS packet authentication */
-  ret->opt.tls_auth.key_ctx_bi = >opt.tls_auth_key;
-
   /* set up list of keys to be scanned by data channel encrypt and decrypt 
routines */
   ASSERT (SIZE (ret->key_scan) == 3);
   ret->key_scan[0] = >session[TM_ACTIVE].key[KS_PRIMARY];
@@ -1113,8 +1109,7 @@ tls_auth_standalone_init (struct tls_options *tls_options,
   ALLOC_OBJ_CLEAR_GC (tas, struct tls_auth_standalone, gc);

   /* set up pointer to HMAC object for TLS packet authentication */
-  tas->tls_auth_key = tls_options->tls_auth_key;
-  tas->tls_auth_options.key_ctx_bi = >tls_auth_key;
+  tas->tls_auth_options.key_ctx_bi = tls_options->tls_auth.key_ctx_bi;
   tas->tls_auth_options.flags |= CO_PACKET_ID_LONG_FORM;

   /* get initial frame parms, still need to finalize */
@@ -1197,11 +1192,11 @@ tls_multi_free (struct tls_multi *multi, bool clear)
 static bool
 swap_hmac (struct buffer *buf, const struct crypto_options *co, bool incoming)
 {
-  struct key_ctx *ctx;
+  const struct key_ctx *ctx;

   ASSERT (co);

-  ctx = (incoming ? >key_ctx_bi->decrypt : >key_ctx_bi->encrypt);
+  ctx = 

[Openvpn-devel] [PATCH 02/10] Remove reuse of key_type during init of data channel auth and tls-auth

2016-02-07 Thread Steffan Karger
Prepare for using AEAD cipher modes + tls-auth, as tls-auth might want to
use an HMAC, while the data channel uses e.g. GCM tags.  This separates
the two initialisations.  Also, error out (and give a clear error message)
if a user specifies tls-auth but no valid auth algorithm, which makes no
sense at all.

Signed-off-by: Steffan Karger 
---
 src/openvpn/crypto.c  |  9 ++---
 src/openvpn/init.c| 25 +++--
 src/openvpn/openvpn.h |  1 +
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index c18d88b..806a995 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -751,13 +751,8 @@ get_tls_handshake_key (const struct key_type *key_type,
   if (passphrase_file && key_type->hmac_length)
 {
   struct key2 key2;
-  struct key_type kt = *key_type;
   struct key_direction_state kds;

-  /* for control channel we are only authenticating, not encrypting */
-  kt.cipher_length = 0;
-  kt.cipher = NULL;
-
   if (flags & GHK_INLINE)
{
  /* key was specified inline, key text is in passphrase_file */
@@ -800,9 +795,9 @@ get_tls_handshake_key (const struct key_type *key_type,

   /* initialize hmac key in both directions */

-  init_key_ctx (>encrypt, [kds.out_key], , 
OPENVPN_OP_ENCRYPT,
+  init_key_ctx (>encrypt, [kds.out_key], key_type, 
OPENVPN_OP_ENCRYPT,
"Outgoing Control Channel Authentication");
-  init_key_ctx (>decrypt, [kds.in_key], , 
OPENVPN_OP_DECRYPT,
+  init_key_ctx (>decrypt, [kds.in_key], key_type, 
OPENVPN_OP_DECRYPT,
"Incoming Control Channel Authentication");

   CLEAR (key2);
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index d0020b7..7e6e448 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2206,11 +2206,23 @@ do_init_crypto_tls_c1 (struct context *c)
  flags |= GHK_INLINE;
  file = options->tls_auth_file_inline;
}
- get_tls_handshake_key (>c1.ks.key_type,
->c1.ks.tls_auth_key,
-file,
-options->key_direction,
-flags);
+
+ /* Initialize key_type for tls-auth with auth only */
+ CLEAR (c->c1.ks.tls_auth_key_type);
+ if (options->authname && options->authname_defined)
+   {
+ c->c1.ks.tls_auth_key_type.digest = md_kt_get (options->authname);
+ c->c1.ks.tls_auth_key_type.hmac_length =
+ md_kt_size (c->c1.ks.tls_auth_key_type.digest);
+   }
+ else
+   {
+ msg (M_FATAL, "ERROR: tls-auth enabled, but no valid --auth "
+ "algorithm specified ('%s')", options->authname);
+   }
+
+ get_tls_handshake_key (>c1.ks.tls_auth_key_type,
+ >c1.ks.tls_auth_key, file, options->key_direction, flags);
}

 #if 0 /* was: #if ENABLE_INLINE_FILES --  Note that enabling this code will 
break restarts */
@@ -2375,7 +2387,7 @@ do_init_crypto_tls (struct context *c, const unsigned int 
flags)
   to.tls_auth.pid_persist = >c1.pid_persist;
   to.tls_auth.flags |= CO_PACKET_ID_LONG_FORM;
   crypto_adjust_frame_parameters (,
- >c1.ks.key_type,
+ >c1.ks.tls_auth_key_type,
  false, false, true, true);
 }

@@ -3758,6 +3770,7 @@ inherit_context_child (struct context *dest,
   /* inherit SSL context */
   dest->c1.ks.ssl_ctx = src->c1.ks.ssl_ctx;
   dest->c1.ks.tls_auth_key = src->c1.ks.tls_auth_key;
+  dest->c1.ks.tls_auth_key_type = src->c1.ks.tls_auth_key_type;
 #endif

   /* options */
diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
index 3f1df6e..71adf48 100644
--- a/src/openvpn/openvpn.h
+++ b/src/openvpn/openvpn.h
@@ -66,6 +66,7 @@ struct key_schedule
   struct tls_root_ctx ssl_ctx;

   /* optional authentication HMAC key for TLS control channel */
+  struct key_type tls_auth_key_type;
   struct key_ctx_bi tls_auth_key;
 #else  /* ENABLE_CRYPTO */
   int dummy;
-- 
2.5.0




[Openvpn-devel] [PATCH 01/10] Allow NULL argument in cipher_ctx_get_cipher_kt()

2016-02-07 Thread Steffan Karger
Since otherwise we'll have to perform the check before each call.

Signed-off-by: Steffan Karger 
---
 src/openvpn/crypto_backend.h  | 8 
 src/openvpn/crypto_openssl.c  | 2 +-
 src/openvpn/crypto_polarssl.c | 4 +---
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h
index 4c1ce9f..1c23436 100644
--- a/src/openvpn/crypto_backend.h
+++ b/src/openvpn/crypto_backend.h
@@ -308,12 +308,12 @@ int cipher_ctx_mode (const cipher_ctx_t *ctx);
 /**
  * Returns the static cipher parameters for this context.
  *
- * @param ctx  Cipher's context. May not be NULL.
+ * @param ctx  Cipher's context.
  *
- * @return Static cipher parameters for the supplied context.
+ * @return Static cipher parameters for the supplied context, or
+ * NULL if unable to determine cipher parameters.
  */
-const cipher_kt_t *cipher_ctx_get_cipher_kt (const cipher_ctx_t *ctx)
-  __attribute__((nonnull));
+const cipher_kt_t *cipher_ctx_get_cipher_kt (const cipher_ctx_t *ctx);

 /**
  * Resets the given cipher context, setting the IV to the specified value.
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 1d68662..7dabe5d 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -585,7 +585,7 @@ cipher_ctx_mode (const EVP_CIPHER_CTX *ctx)
 const cipher_kt_t *
 cipher_ctx_get_cipher_kt (const cipher_ctx_t *ctx)
 {
-  return EVP_CIPHER_CTX_cipher(ctx);
+  return ctx ? EVP_CIPHER_CTX_cipher(ctx) : NULL;
 }


diff --git a/src/openvpn/crypto_polarssl.c b/src/openvpn/crypto_polarssl.c
index 407a176..0e4c088 100644
--- a/src/openvpn/crypto_polarssl.c
+++ b/src/openvpn/crypto_polarssl.c
@@ -506,9 +506,7 @@ int cipher_ctx_mode (const cipher_context_t *ctx)
 const cipher_kt_t *
 cipher_ctx_get_cipher_kt (const cipher_ctx_t *ctx)
 {
-  ASSERT(NULL != ctx);
-
-  return ctx->cipher_info;
+  return ctx ? ctx->cipher_info : NULL;
 }

 int cipher_ctx_reset (cipher_context_t *ctx, uint8_t *iv_buf)
-- 
2.5.0




[Openvpn-devel] [PATCH] Add support for AEAD (GCM) cipher mode

2016-02-07 Thread Steffan Karger
Hi,

These patches add support for GCM mode ciphers to OpenVPN.  These are
originally inspired by the patch from kruton (trac #301, and
http://thread.gmane.org/gmane.network.openvpn.devel/7653), but most of the
original code has been rewritten.

As discussed in various IRC meetings and at the hackathons, we used this
opportunity to introduce a new - more efficient - packet format.  See
http://sourceforge.net/p/openvpn/mailman/message/33210313/ and the commit
message of patch 8 for more details.

The first patches (1-7) are refactoring in preparation of adding AEAD modes.
Not all of the changes are strictly required, but they made it easier for me to
understand what was going on and debug my AEAD code.  I think they improve the
understandability of the code.  These should not change any behaviour (apart
from adding better log messages).

Patch 8 actually adds the GCM cipher mode.  See it's commit message for more
information on the implementation.

Patch 9 provides polarssl/mbedtls and openssl config file interoperability.

Patch 10 adds a (very) preliminary version of cipher negotation.  I'm not
entirely sure if we should already apply this patch or wait for full cipher
negotiation support.  I'm also not sure when I will have proper negotiation
patches available.

This implementation has been verified to be compatible with openvpn 3 clients
and servers.  To test this you'll need to pretend to fully support IV_NCP and
IV_TCPNL, see e.g. https://github.com/syzzer/openvpn/tree/aead-cipher-modes13.

This has been a spare-time project, and did not yet receive any thorough review
or field testing.  So both are still very much needed.

-Steffan




Re: [Openvpn-devel] GUI repo

2016-02-07 Thread Gert Doering
Hi,

On Sun, Feb 07, 2016 at 09:00:28PM +0500,  ?? wrote:
> if you mean interactive service, keep in mind that people sometimes start
> openvpn as a child of openvpn-gui, not as a service

Interactive Service is running openvpn.exe under control of openvpn-gui,
and you won't notice anything different - except that it magically works
even if the user has no permissions to set routes, and the "want admin!"
flag is not set on the GUI.

And, of course, that bugs in openvpn-gui or openvpn will not lead to a
privilege breach anymore.

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature


[Openvpn-devel] Fwd: [PATCH] Fix misleading socket error code on Windows

2016-02-07 Thread Leonardo
Hi. First of all, this is my first contribution ever to an open source
project. I hope I'm doing this right.

After installing OpenVPN 2.3.10 on my Windows computer and trying to
connect to a VPN server, I was getting this error message:

TCP: connect to [AF_INET]x.x.x.x:80 failed, will try again in 5
seconds: The system tried to join a drive to a directory on a joined
drive.

Hours of googling and no real solution later, I've decided to check
the source code and see if I could find the real cause of this error.
It was a timeout error. Indeed there's a mismapping going on, as
suggested here:

http://sourceforge.net/p/openvpn/mailman/message/33101265/

I have both MinGW-w64 and Visual Studio 2015, and in both headers
WSAETIMEDOUT is defined as ETIMEDOUT which is defined as 138, despite
the online documentation saying that WSAETIMEDOUT should be 10060.

https://msdn.microsoft.com/en-us/library/windows/desktop/ms740668%28v=vs.85%29.aspx

I couldn't find any explanation about this, but based on other
threads, this issue seems to be around for quite some time now. After
the patch below, the right error message comes out:

TCP: connect to [AF_INET]x.x.x.x:80 failed: Connection timed out (WSAETIMEDOUT)

I understand that's not the most elegant solution, but given the above
said, I don't see an alternative. There are many forum threads out
there just because of this misleading error message.


Signed-off-by: Leonardo Basilio 
---
 src/openvpn/socket.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 396fa54..c01ef3d 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -1177,7 +1177,11 @@ openvpn_connect (socket_descriptor_t sd,
 {
   if (--connect_timeout < 0)
 {
+#ifdef WIN32
+  status = 10060;
+#else
   status = ETIMEDOUT;
+#endif
   break;
 }
   openvpn_sleep (1);
--
2.7.0.windows.1



Re: [Openvpn-devel] GUI repo

2016-02-07 Thread Илья Шипицин
if you mean interactive service, keep in mind that people sometimes start
openvpn as a child of openvpn-gui, not as a service

воскресенье, 7 февраля 2016 г. пользователь Selva Nair написал:

> Hi,
>
> We can now remove the HighestAvailable privilege request in GUI, but have
> to keep it in the 2.3.x windows installer which doesn't have iservice. So
> it may be useful to make a release branch in the GUI repo.
>
> Thanks,
>
> Selva
>
>


[Openvpn-devel] GUI repo

2016-02-07 Thread Selva Nair
Hi,

We can now remove the HighestAvailable privilege request in GUI, but have
to keep it in the 2.3.x windows installer which doesn't have iservice. So
it may be useful to make a release branch in the GUI repo.

Thanks,

Selva