Re: [Openvpn-devel] [PATCH v3] Parse static challenge response in auth-pam plugin

2018-07-25 Thread Joe Bell
On Wed, Jul 25, 2018 at 4:43 PM, Gert Doering  wrote:

> Hi,
>
> On Wed, Jul 25, 2018 at 04:31:05PM -0500, Joe Bell wrote:
> > I don't know if it is appropriate to reply to this post in this manner,
>
> It is and it helps :-)
>
> We want test reports, as in "I am using this, because it is a useful
> feature for me, and it works fine!" - not as strong as a full code
> review, but an indication that something is beneficial at least.
>

Terrific.  Indeed, I am using this now, and quite successfully in a staging
environment.


>
> [..]
> > If there is anything I can do to provide a list of testcases, write-up on
> > its configuration and usage, etc., in aide of getting the patch merged
> and
> > released, please let me know.
>
> If you could find a few spare weeks for me, that would help quite a bit :-)
>
> Jokes aside, a bit of writeup how this looks in the client config, how
> the client prompts will then look, and how you configure the server would
> be nice - just here on the list, so google can find it.
>
>
I will do a full writeup, but the magic comes from

plugin /usr/lib/openvpn/plugins/openvpn-plugin-auth-pam.so "openvpn login:
USERNAME Password: PASSWORD Verification OTP"

in the OpenVPN server.conf, as well as:

auth required /lib/security/pam_google_authenticator.so
secret=/etc/openvpn/google-authenticator/${USER} user=gauth
auth required pam_ldap.so

for the contents of /etc/pam.d/openvpn. (Note that I'm using LDAP for
username/password, this should be replaced by the appropriate PAM module
for other types.)

Then, on the client configuration (.ovpn):

auth-user-pass
static-challenge "Enter Google Authenticator Token" 1

Now obviously there are a lot of details i've left out (setting up Google
Authenticator, etc.) but once I compiled OpenVPN with the base64 patch and
the plugin patch, it was smiles all around.  I will work to put together a
full write-up of zero to sixty with screenshots, etc.

I've been delighted to cobble this together with my previous knowledge of
OpenVPN configurations, finding Selva's patches, and pointers on how to
pull this all together, but without a doubt it would have been delightful
to have it out-of-the-box.  Thanks for the feedback and encouragement to
push for this.

Joe
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v3] Parse static challenge response in auth-pam plugin

2018-07-25 Thread Gert Doering
Hi,

On Wed, Jul 25, 2018 at 04:31:05PM -0500, Joe Bell wrote:
> I don't know if it is appropriate to reply to this post in this manner, 

It is and it helps :-)

We want test reports, as in "I am using this, because it is a useful
feature for me, and it works fine!" - not as strong as a full code
review, but an indication that something is beneficial at least.

[..]
> If there is anything I can do to provide a list of testcases, write-up on
> its configuration and usage, etc., in aide of getting the patch merged and
> released, please let me know.

If you could find a few spare weeks for me, that would help quite a bit :-)

Jokes aside, a bit of writeup how this looks in the client config, how
the client prompts will then look, and how you configure the server would
be nice - just here on the list, so google can find it.

Actually, this is something useful for $payingcustomer, so I hope to
find time to review and test it myself "soonish"...  unfortunately, 
$otherpayingcustomers are screaming more loudly for me to get other stuff
done first, with a hard deadline next Tuesday...

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v3] Parse static challenge response in auth-pam plugin

2018-07-25 Thread Joe Bell
I don't know if it is appropriate to reply to this post in this manner, but
Selva's static challenge response in the PAM plugin would be a great
addition; I've applied this and the base64 patch and can successfully use
the implementation with Tunnelblick (which is supporting static-challenge
as of 3.7.7beta03, as well as the current version of the Windows OpenVPN
client (I'm using Google Authenticator for the OTP).

If there is anything I can do to provide a list of testcases, write-up on
its configuration and usage, etc., in aide of getting the patch merged and
released, please let me know.

Many thanks,
Joe


On Tue, Jul 24, 2018 at 9:34 PM,  wrote:

> From: Selva Nair 
>
> If static challenge is in use, the password passed to the plugin by openvpn
> is of the form "SCRV1:base64-pass:base64-response". Parse this string to
> separate it into password and response and use them to respond to queries
> in the pam conversation function.
>
> On the plugin parameters line the substitution keyword for the static
> challenge response is "OTP". For example, for pam config named "test" that
> prompts for "user", "password" and "pin", use
>
> plugin openvpn-auth-pam.so "test user USERNAME password PASSWORD pin OTP"
>
> Signed-off-by: Selva Nair 
>
> ---
> v2: Depends on the base64 export patch
> v3: match password string with "SCRV1:" instead of "SCRV1"
> (pointed out by Joe Bell )
>
>  src/plugins/auth-pam/README.auth-pam | 15 +---
>  src/plugins/auth-pam/auth-pam.c  | 75 ++
> +-
>  2 files changed, 84 insertions(+), 6 deletions(-)
>
> diff --git a/src/plugins/auth-pam/README.auth-pam
> b/src/plugins/auth-pam/README.auth-pam
> index e123690..9081565 100644
> --- a/src/plugins/auth-pam/README.auth-pam
> +++ b/src/plugins/auth-pam/README.auth-pam
> @@ -36,19 +36,20 @@ pairs to answer PAM module queries.
>
>  For example:
>
> -  plugin openvpn-auth-pam.so "login login USERNAME password PASSWORD"
> +  plugin openvpn-auth-pam.so "login login USERNAME password PASSWORD pin
> OTP"
>
>  tells auth-pam to (a) use the "login" PAM module, (b) answer a
> -"login" query with the username given by the OpenVPN client, and
> -(c) answer a "password" query with the password given by the
> -OpenVPN client.  This provides flexibility in dealing with the different
> +"login" query with the username given by the OpenVPN client,
> +(c) answer a "password" query with the password, and (d) answer a
> +"pin" query with the OTP given by the OpenVPN client.
> +This provides flexibility in dealing with different
>  types of query strings which different PAM modules might generate.
>  For example, suppose you were using a PAM module called
>  "test" which queried for "name" rather than "login":
>
>plugin openvpn-auth-pam.so "test name USERNAME password PASSWORD"
>
> -While "USERNAME" "COMMONNAME" and "PASSWORD" are special strings which
> substitute
> +While "USERNAME" "COMMONNAME" "PASSWORD" and "OTP" are special strings
> which substitute
>  to client-supplied values, it is also possible to name literal values
>  to use as PAM module query responses.  For example, suppose that the
>  login module queried for a third parameter, "domain" which
> @@ -61,6 +62,10 @@ the operation of this plugin:
>
>client-cert-not-required
>username-as-common-name
> +  static-challenge
> +
> +Use of --static challenege is required to pass a pin (represented by
> "OTP" in
> +parameter substituion) or a second password.
>
>  Run OpenVPN with --verb 7 or higher to get debugging output from
>  this plugin, including the list of queries presented by the
> diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-
> pam.c
> index 26b0eeb..e22ce5f 100644
> --- a/src/plugins/auth-pam/auth-pam.c
> +++ b/src/plugins/auth-pam/auth-pam.c
> @@ -6,6 +6,7 @@
>   * packet compression.
>   *
>   *  Copyright (C) 2002-2018 OpenVPN Inc 
> + *  Copyright (C) 2016-2018 Selva Nair 
>   *
>   *  This program is free software; you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License version 2
> @@ -64,6 +65,7 @@
>
>  /* Pointers to functions exported from openvpn */
>  static plugin_secure_memzero_t plugin_secure_memzero = NULL;
> +static plugin_base64_decode_t plugin_base64_decode = NULL;
>
>  /*
>   * Plugin state, used by foreground
> @@ -87,6 +89,7 @@ struct auth_pam_context
>   *  "USERNAME" -- substitute client-supplied username
>   *  "PASSWORD" -- substitute client-specified password
>   *  "COMMONNAME" -- substitute client certificate common name
> + *  "OTP" -- substitute static challenge response if available
>   */
>
>  #define N_NAME_VALUE 16
> @@ -111,6 +114,7 @@ struct user_pass {
>  char username[128];
>  char password[128];
>  char common_name[128];
> +char response[128];
>
>  const struct name_value_list *name_value_list;
>  };
> @@ -276,6 +280,66 @@ name_value_match(const char *query, const char *match)
>  return 

Re: [Openvpn-devel] [PATCH] [openvpn-gui] Update system tray to populate Windows VPN flyout

2018-07-25 Thread Илья Шипицин
чт, 26 июл. 2018 г. в 1:04, Kevin Kane via Openvpn-devel <
openvpn-devel@lists.sourceforge.net>:

> Alright, I found the SMTP server and sent the patches out again with git
> send-email. Let me know how those look.
>


looks good from patchwork point of view:

https://patchwork.openvpn.net/project/openvpn2/list/


>
> -Original Message-
> From: Gert Doering 
> Sent: Wednesday, July 25, 2018 11:18 AM
> To: Kevin Kane 
> Cc: Gert Doering ; openvpn-devel <
> openvpn-devel@lists.sourceforge.net>
> Subject: Re: [Openvpn-devel] [PATCH] [openvpn-gui] Update system tray to
> populate Windows VPN flyout
>
> Hi,
>
> On Wed, Jul 25, 2018 at 06:08:19PM +, Kevin Kane wrote:
> > Ugh. Thanks, Outlook. I'd have to use a personal e-mail account to use
> something other than Exchange.
> >
> > The other option is for me to add the files to the e-mail as
> attachments. Is that acceptable, or do you really need the patch text to be
> in the message body?
>
> For applying with "git am" and for reviewing with "I can just quote parts
> of the patch and comment on it", attachments in anything that is not
> text/plain format are also not ideal, but we can try that.
>
> We've had a few patches come in recently that were attachment in a signed
> mail, and neither patchwork nor git could handle these mails in a
> satisfying way - as in: patchwork did not recognize them as "patch", and
> "git am" declared "no patch in there" - so "save patch as individual file,
> git am, manually add in-reply-to to the auto- generated "patch applied"
> reply mail, etc.
>
> It's a bit of try and error what works in a given environment.
>
> (git send-email talking SMTP to Exchange might actually work more nicely
> than outlook.  Maybe that was just one particularily weird version of
> Exchange, or "too many virus scanner plugins hooked in", given that this
> was Sophos...)
>
> gert
>
> --
> "If was one thing all people took for granted, was conviction that if you
> feed honest figures into a computer, honest figures come out. Never
> doubted  it myself till I met a computer with a sense of humor."
>  Robert A. Heinlein, The Moon is a Harsh
> Mistress
>
> Gert Doering - Munich, Germany
> g...@greenie.muc.de
>
>
> --
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> ___
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] [openvpn-gui] Update system tray to populate Windows VPN flyout

2018-07-25 Thread Kevin Kane via Openvpn-devel
Alright, I found the SMTP server and sent the patches out again with git 
send-email. Let me know how those look.

-Original Message-
From: Gert Doering  
Sent: Wednesday, July 25, 2018 11:18 AM
To: Kevin Kane 
Cc: Gert Doering ; openvpn-devel 

Subject: Re: [Openvpn-devel] [PATCH] [openvpn-gui] Update system tray to 
populate Windows VPN flyout

Hi,

On Wed, Jul 25, 2018 at 06:08:19PM +, Kevin Kane wrote:
> Ugh. Thanks, Outlook. I'd have to use a personal e-mail account to use 
> something other than Exchange.
> 
> The other option is for me to add the files to the e-mail as attachments. Is 
> that acceptable, or do you really need the patch text to be in the message 
> body?

For applying with "git am" and for reviewing with "I can just quote parts of 
the patch and comment on it", attachments in anything that is not text/plain 
format are also not ideal, but we can try that.

We've had a few patches come in recently that were attachment in a signed mail, 
and neither patchwork nor git could handle these mails in a satisfying way - as 
in: patchwork did not recognize them as "patch", and "git am" declared "no 
patch in there" - so "save patch as individual file, git am, manually add 
in-reply-to to the auto- generated "patch applied" reply mail, etc.

It's a bit of try and error what works in a given environment.

(git send-email talking SMTP to Exchange might actually work more nicely than 
outlook.  Maybe that was just one particularily weird version of Exchange, or 
"too many virus scanner plugins hooked in", given that this was Sophos...)

gert

--
"If was one thing all people took for granted, was conviction that if you  feed 
honest figures into a computer, honest figures come out. Never doubted  it 
myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v2] [openvpn-gui] Update system tray to populate Windows VPN flyout

2018-07-25 Thread Kevin Kane via Openvpn-devel
Add a DLL to be wired in as a custom dialer, which introduces new build 
dependencies
Add copyright notices as required where Microsoft has contributed code

Signed-off-by: Kevin Kane 
---
 .gitignore   |   3 +
 BUILD.rst|   1 +
 Makefile.am  |  10 ++-
 configure.ac |   4 ++
 dialer.c | 171 +++
 tray.c   | 117 +++
 tray.h   |   2 +
 7 files changed, 307 insertions(+), 1 deletion(-)
 create mode 100644 dialer.c

diff --git a/.gitignore b/.gitignore
index ac37772..e36e0a6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,6 +10,8 @@
 *.log
 *.tar.gz
 *.bak
+*.lo
+*.la
 
 .deps
 Makefile
@@ -30,3 +32,4 @@ depcomp
 stamp-h1
 install-sh
 missing
+libtool
diff --git a/BUILD.rst b/BUILD.rst
index 9354784..3c70b52 100644
--- a/BUILD.rst
+++ b/BUILD.rst
@@ -24,6 +24,7 @@ their dependencies. You can install these packages using the 
standard
 - mingw64-x86_64-gcc-core
 - mingw64-x86_64-g++
 - mingw64-x86_64-openssl
+- libtool
 
 
 Build
diff --git a/Makefile.am b/Makefile.am
index 8301087..d8435ed 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2,6 +2,7 @@
 #
 #  Copyright (C) 2004 Mathias Sundman 
 #2010 Heiko Hund 
+#  Portions Copyright (C) 2018 Microsoft Corporation
 #
 #  This program is free software; you can redistribute it and/or modify
 #  it under the terms of the GNU General Public License as published by
@@ -22,6 +23,7 @@ RCCOMPILE = $(WINDRES) $(DEFS) $(DEFAULT_INCLUDES) 
$(INCLUDES) \
 $(AM_CPPFLAGS) $(CPPFLAGS)
 
 AUTOMAKE_OPTIONS = foreign 1.9
+ACLOCAL_AMFLAGS = -I m4
 
 MAINTAINERCLEANFILES = \
config.log config.status \
@@ -106,7 +108,13 @@ openvpn_gui_LDADD = \
-lnetapi32 \
-lole32 \
-lshlwapi \
-   -lsecur32
+   -lsecur32 \
+   -lrasapi32
 
 openvpn-gui-res.o: $(openvpn_gui_RESOURCES) $(srcdir)/openvpn-gui-res.h
$(RCCOMPILE) -i $< -o $@
+
+lib_LTLIBRARIES  = libopenvpndialer.la
+libopenvpndialer_la_SOURCES  = dialer.c
+libopenvpndialer_la_LDFLAGS  = -no-undefined
+libopenvpndialer_la_LIBADD   = -lrasapi32
\ No newline at end of file
diff --git a/configure.ac b/configure.ac
index 9a2ba75..4fe6b81 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2,6 +2,7 @@ dnl  OpenVPN-GUI -- A Windows GUI for OpenVPN.
 dnl
 dnl  Copyright (C) 2004 Mathias Sundman 
 dnl2010 Heiko Hund 
+dnl  Portions Copyright (C) 2018 Microsoft Corporation
 dnl
 dnl  This program is free software; you can redistribute it and/or modify
 dnl  it under the terms of the GNU General Public License as published by
@@ -29,10 +30,13 @@ AC_DEFINE([CORE_COPYRIGHT_YEAR_END], ["2018"], [Last 
copyright year for daemon i
 AC_CONFIG_AUX_DIR([.])
 AM_CONFIG_HEADER([config.h])
 AC_CONFIG_SRCDIR([main.h])
+AC_CONFIG_MACRO_DIRS([m4])
 AM_INIT_AUTOMAKE
 AC_CANONICAL_HOST
 AC_USE_SYSTEM_EXTENSIONS
 AC_PROG_CC_C99
+LT_INIT([win32-dll])
+AC_LIBTOOL_WIN32_DLL
 AC_CHECK_TOOL([WINDRES], [windres])
 
 AC_ARG_ENABLE(
diff --git a/dialer.c b/dialer.c
new file mode 100644
index 000..86a0f39
--- /dev/null
+++ b/dialer.c
@@ -0,0 +1,171 @@
+/*
+ *  OpenVPN-GUI -- A Windows GUI for OpenVPN.
+ *
+ *  Copyright (C) 2018 Microsoft Corporation
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program (see the file COPYING included with this
+ *  distribution); if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifdef HAVE_CONFIG_H
+#include 
+#endif
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+__declspec(dllexport)
+DWORD WINAPI RasCustomDial(
+HINSTANCE hInstDll __attribute__((unused)),
+LPRASDIALEXTENSIONS lpRasDialExtensions __attribute__((unused)),
+LPWSTR lpszPhonebook __attribute__((unused)),
+LPRASDIALPARAMSW lpRasDialParams __attribute__((unused)),
+DWORD dwNotifierType __attribute__((unused)),
+LPVOID lpvNotifier __attribute__((unused)),
+LPHRASCONN lphRasConn __attribute__((unused)),
+DWORD dwFlags __attribute__((unused)))
+{
+return ERROR_CALL_NOT_IMPLEMENTED;
+}
+
+/* Copied from OpenVPN-GUI main.c */
+static const TCHAR OpenVPNGuiClassName[] = _T("OpenVPN-GUI");
+
+/* Copied from OpenVPN-GUI tray.h */
+#define IDM_CONNECTMENU 300
+
+static const TCHAR WindowCaption[] = _T("Walrus VPN");
+
+__declspec(dllexport)
+BOOL WINAPI RasCustomDialDlg(
+HINSTANCE hInstDll 

[Openvpn-devel] [PATCH v2] [openvpn-build] Install/uninstall dialer DLL as part of Windows installer operation

2018-07-25 Thread Kevin Kane via Openvpn-devel
Signed-off-by: Kevin Kane 
---
 windows-nsis/openvpn.nsi | 4 
 1 file changed, 4 insertions(+)

diff --git a/windows-nsis/openvpn.nsi b/windows-nsis/openvpn.nsi
index e92904d..aff7cb3 100755
--- a/windows-nsis/openvpn.nsi
+++ b/windows-nsis/openvpn.nsi
@@ -1,6 +1,7 @@
 ; 
 ; * Copyright (C) 2002-2010 OpenVPN Technologies, Inc.   *
 ; * Copyright (C)  2012 Alon Bar-Lev  *
+; * Portions Copyright (C) 2018 Microsoft Corporation*
 ; *  This program is free software; you can redistribute it and/or modify*
 ; *  it under the terms of the GNU General Public License version 2  *
 ; *  as published by the Free Software Foundation.   *
@@ -443,8 +444,10 @@ Section /o "${PACKAGE_NAME} GUI" SecOpenVPNGUI
 
${If} ${RunningX64}
File "${OPENVPN_ROOT_X86_64}\bin\openvpn-gui.exe"
+   File "${OPENVPN_ROOT_X86_64}\bin\libopenvpndialer-0.dll"
${Else}
File "${OPENVPN_ROOT_I686}\bin\openvpn-gui.exe"
+   File "${OPENVPN_ROOT_I686}\bin\libopenvpndialer-0.dll"
${EndIf}
 
${If} ${SectionIsSelected} ${SecAddShortcutsWorkaround}
@@ -770,6 +773,7 @@ Section "Uninstall"
${EndIf}
 
Delete "$INSTDIR\bin\openvpn-gui.exe"
+   Delete "$INSTDIR\bin\libopenvpndialer-0.dll"
Delete "$DESKTOP\${PACKAGE_NAME} GUI.lnk"
 
Delete "$INSTDIR\bin\openvpn.exe"
-- 
2.17.1.windows.2


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v2] [openvpn] Register/unregister trusted custom dialer DLL when installing/uninstalling service

2018-07-25 Thread Kevin Kane via Openvpn-devel
Add copyright notice as required where Microsoft has contributed code

Signed-off-by: Kevin Kane 
---
 src/openvpnserv/service.c | 241 +++---
 1 file changed, 222 insertions(+), 19 deletions(-)

diff --git a/src/openvpnserv/service.c b/src/openvpnserv/service.c
index 7157bea2..edb21b0d 100644
--- a/src/openvpnserv/service.c
+++ b/src/openvpnserv/service.c
@@ -6,6 +6,7 @@
  *
  * Copyright (C) 1993 - 2000.  Microsoft Corporation.  All rights reserved.
  *  2013 Heiko Hund 
+ * Portions Copyright (C) 2018 Microsoft Corporation
  */
 
 #include "service.h"
@@ -53,6 +54,195 @@ ReportStatusToSCMgr(SERVICE_STATUS_HANDLE service, 
SERVICE_STATUS *status)
 return res;
 }
 
+static const TCHAR DialerDllName[] = TEXT("libopenvpndialer-0.dll");
+static const TCHAR RegValueName[] = TEXT("CustomDLL");
+
+static int
+HandleDialerRegistration(int uninstall)
+{
+TCHAR path[512], customDllString[1024];
+HKEY parametersKey;
+LONG result;
+DWORD customDllSize = sizeof(customDllString);
+
+/* Assumption is that the dialer DLL is installed to the same bin 
directory as everything else. */
+if (GetModuleFileName(NULL, path, sizeof(path)) == 0)
+{
+_tprintf(TEXT("Unable to get module path - %lu\n"), GetLastError());
+return 1;
+}
+
+/* The version of NSIS we use to create the installer doesn't yet have 
support for 
+ * writing multi-string registry entries, which we need to do in order to 
register our
+ * custom dialer DLL. Instead, we'll update this registry entry on 
install/uninstall.
+ */
+
+TCHAR *lastBackslash = _tcsrchr(path, TEXT('\\'));
+if (NULL == lastBackslash)
+{
+_tprintf(TEXT("Could not locate last backslash in path: %s\n"), path);
+return 1;
+}
+
+lastBackslash[1] = TEXT('\0');
+
+/* Bounds checking. */
+if ((_tcslen(path) + _countof(DialerDllName)) > _countof(path))
+{
+_tprintf(TEXT("Out of buffer adding dialer filename to path"));
+return 1;
+}
+
+_tcscat(path, DialerDllName);
+
+result = RegOpenKeyEx(HKEY_LOCAL_MACHINE, 
+TEXT("SYSTEM\\CurrentControlSet\\Services\\RasMan\\Parameters"), 
+0, 
+KEY_ALL_ACCESS,
+);
+if (ERROR_SUCCESS != result)
+{
+_tprintf(TEXT("Could not open RasMan parameters key: %ld\n"), result);
+return 1;
+}
+
+/* Must RegCloseKey(parametersKey) from this point */
+
+result = RegGetValue(parametersKey,
+NULL,
+RegValueName,
+RRF_RT_REG_MULTI_SZ,
+NULL,
+customDllString,
+);
+
+/* If we're installing, the key being absent is okay. Any other error is 
fatal. */
+if (ERROR_FILE_NOT_FOUND == result)
+{
+if (0 != uninstall)
+{
+_tprintf(TEXT("CustomDLL value was not found; skipping 
unregistration step\n"));
+RegCloseKey(parametersKey);
+return 0; /* Not a fatal error but nothing to do */
+}
+else
+{
+result = ERROR_SUCCESS;
+customDllString[0] = TEXT('\0');
+customDllSize = 0;
+}
+}
+else if (ERROR_SUCCESS != result)
+{
+_tprintf(TEXT("Could not open CustomDLL value: %ld\n"), result);
+RegCloseKey(parametersKey);
+return 1;
+}
+
+/* Determine if the custom dialer DLL is present in the registry setting. 
This is a multi-string so we can't 
+ * use _tcsstr.
+ */
+TCHAR* p;
+for (p = customDllString; *p != TEXT('\0'); p += _tcslen(p) + 1)
+{
+if (0 == _tcsicmp(path, p))
+{
+/* It is. p points to where it begins. */
+break;
+}
+}
+/* If it's not present, p points at a NULL terminator. */
+
+/* If we're installing and the DLL is present in the registry, do nothing. 
*/
+/* If we're uninstalling and the DLL is not present in the registry, do 
nothing. */
+if ( ((0 == uninstall) && (*p != TEXT('\0'))) ||
+ ((0 != uninstall) && (*p == TEXT('\0'))) )
+{
+RegCloseKey(parametersKey);
+return 0;
+}
+
+/* If we're installing and the DLL is not present in the registry, append 
it at point p. */
+if ((0 == uninstall) && (*p == TEXT('\0')))
+{
+/* Make sure the string buffer has enough space left. p points at the 
second in the double-terminating null
+ * (or the sole NULL if the registry entry was empty or absent).
+ */
+if (((p - customDllString) + _tcslen(path) + 2) > 
_countof(customDllString))
+{
+_tprintf(TEXT("Not enough buffer to create new CustomDLL 
string\n"));
+RegCloseKey(parametersKey);
+return 1;
+}
+_tcscpy(p, path);
+/* Add second terminating NULL we overwrote with the copy. */
+p += _tcslen(p) + 1;
+*p++ = TEXT('\0');
+
+/* Set in the registry. */
+

Re: [Openvpn-devel] [PATCH] [openvpn-gui] Update system tray to populate Windows VPN flyout

2018-07-25 Thread Gert Doering
Hi,

On Wed, Jul 25, 2018 at 06:08:19PM +, Kevin Kane wrote:
> Ugh. Thanks, Outlook. I'd have to use a personal e-mail account to use 
> something other than Exchange.
> 
> The other option is for me to add the files to the e-mail as attachments. Is 
> that acceptable, or do you really need the patch text to be in the message 
> body?

For applying with "git am" and for reviewing with "I can just quote
parts of the patch and comment on it", attachments in anything that is
not text/plain format are also not ideal, but we can try that.

We've had a few patches come in recently that were attachment in a 
signed mail, and neither patchwork nor git could handle these mails
in a satisfying way - as in: patchwork did not recognize them as
"patch", and "git am" declared "no patch in there" - so "save patch
as individual file, git am, manually add in-reply-to to the auto-
generated "patch applied" reply mail, etc.

It's a bit of try and error what works in a given environment.

(git send-email talking SMTP to Exchange might actually work more 
nicely than outlook.  Maybe that was just one particularily weird
version of Exchange, or "too many virus scanner plugins hooked in",
given that this was Sophos...)

gert

-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] [openvpn-gui] Update system tray to populate Windows VPN flyout

2018-07-25 Thread Kevin Kane via Openvpn-devel
Ugh. Thanks, Outlook. I'd have to use a personal e-mail account to use 
something other than Exchange.

The other option is for me to add the files to the e-mail as attachments. Is 
that acceptable, or do you really need the patch text to be in the message body?

-Original Message-
From: Gert Doering  
Sent: Wednesday, July 25, 2018 11:00 AM
To: Kevin Kane 
Cc: openvpn-devel 
Subject: Re: [Openvpn-devel] [PATCH] [openvpn-gui] Update system tray to 
populate Windows VPN flyout

Hi,

as a side note, your mail client massacres leading whitespace, so the patch is 
all squeezed to the left side:

On Wed, Jul 25, 2018 at 05:01:39PM +, Kevin Kane via Openvpn-devel wrote:
> diff --git a/Makefile.am b/Makefile.am index 8301087..d8435ed 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -2,6 +2,7 @@
> #
> #  Copyright (C) 2004 Mathias Sundman 
> #2010 Heiko Hund 
> +#  Portions Copyright (C) 2018 Microsoft Corporation
> #

... which makes it impossible to apply without manually adding the leading 
space in non-changed lines.

Interesting enough...

>   *
>   *  Copyright (C) 2004 Mathias Sundman 
>   *2010 Heiko Hund 
> + *  Portions Copyright (C) 2018 Microsoft Corporation
>   *
>   *  This program is free software; you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License as published 
> by

... the effect differs...


But anyway.  If possible, use "git send-email" (which is much better at not 
massacring things than the standard clients) - and point it to something which 
is not Exchange.  Unfortunately, we've seen that as well, git-send-email 
sending via SMTP to Exchange, and Exchange hacking and slaying at the patch...

thanks :)

gert
--
"If was one thing all people took for granted, was conviction that if you  feed 
honest figures into a computer, honest figures come out. Never doubted  it 
myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] [openvpn-gui] Update system tray to populate Windows VPN flyout

2018-07-25 Thread Gert Doering
Hi,

as a side note, your mail client massacres leading whitespace, so
the patch is all squeezed to the left side:

On Wed, Jul 25, 2018 at 05:01:39PM +, Kevin Kane via Openvpn-devel wrote:
> diff --git a/Makefile.am b/Makefile.am
> index 8301087..d8435ed 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -2,6 +2,7 @@
> #
> #  Copyright (C) 2004 Mathias Sundman 
> #2010 Heiko Hund 
> +#  Portions Copyright (C) 2018 Microsoft Corporation
> #

... which makes it impossible to apply without manually adding the
leading space in non-changed lines.

Interesting enough...

>   *
>   *  Copyright (C) 2004 Mathias Sundman 
>   *2010 Heiko Hund 
> + *  Portions Copyright (C) 2018 Microsoft Corporation
>   *
>   *  This program is free software; you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License as published by

... the effect differs...


But anyway.  If possible, use "git send-email" (which is much better
at not massacring things than the standard clients) - and point it to
something which is not Exchange.  Unfortunately, we've seen that as
well, git-send-email sending via SMTP to Exchange, and Exchange hacking 
and slaying at the patch...

thanks :)

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] Incoming: patches for the dialer feature

2018-07-25 Thread Selva Nair
Hi,

On Wed, Jul 25, 2018 at 1:45 PM, Gert Doering  wrote:
> Hi,
>
> On Wed, Jul 25, 2018 at 01:34:44PM -0400, Selva Nair wrote:
>> Do we have an experimental branch where we could add this so that we do
>> not lose track of it?
>
> If you tell me you want that and how I should name it, I'll add one.
>
> OTOH it's in patchwork as well.

Yeah, I forgot about patchwork --  keeping it in there may be good
enough. Experimentation can happen in private repos.

Selva

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] Incoming: patches for the dialer feature

2018-07-25 Thread Gert Doering
Hi,

On Wed, Jul 25, 2018 at 01:34:44PM -0400, Selva Nair wrote:
> Do we have an experimental branch where we could add this so that we do
> not lose track of it?

If you tell me you want that and how I should name it, I'll add one.

OTOH it's in patchwork as well.

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] [openvpn-gui] Update system tray to populate Windows VPN flyout

2018-07-25 Thread Kevin Kane via Openvpn-devel
I agree. This was the consensus when we discussed this feature before, but 
there was some interest in possibly taking this work in an experimental branch 
and later building something more production-ready on top of the plumbing work. 
It is with that expectation I'm offering it upstream.

-Original Message-
From: Selva Nair  
Sent: Wednesday, July 25, 2018 10:28 AM
To: Kevin Kane 
Cc: openvpn-devel 
Subject: Re: [Openvpn-devel] [PATCH] [openvpn-gui] Update system tray to 
populate Windows VPN flyout

Hi,

On Wed, Jul 25, 2018 at 1:01 PM, Kevin Kane via Openvpn-devel 
 wrote:
> From ed96e2d91a0eb9ecdaab8d7104f397f7d77e5ced Mon Sep 17 00:00:00 2001
>
> From: Kevin Kane 
>
> Date: Fri, 13 Jul 2018 09:50:00 -0700
>
> Subject: Update system tray to populate Windows VPN flyout
>
>
>
> Add a DLL to be wired in as a custom dialer, which introduces new 
> build dependencies
>
> Add copyright notices as required where Microsoft has contributed code
>
>
>
> Signed-off-by: Kevin Kane 
>

As discussed in a previous thread, I'm not sure we should have this in its 
current form:

(i) It adds and entry per config to network connections which will be a 
nuisance for users who have far too many configs (there are some users with 
100's of config files) -- so we at least need an option to enable/disable this.

(ii) This only supports connect, not disconnect and the entry does not show 
whether its connected or not -- so this is a feature of very dubious utility.

(iii) Communicating with the GUI using COPYDATA message (we support connect, 
disconnect, status etc.) should be preferred over hard-coding IDM_CONNECTMENU.

Selva
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] Incoming: patches for the dialer feature

2018-07-25 Thread Selva Nair
Hi,

On Wed, Jul 25, 2018 at 1:01 PM, Kevin Kane via Openvpn-devel
 wrote:
> Ok, I’ve gotten clearance to contribute the dialer feature from Microsoft’s
> OpenVPN fork back upstream. As previously discussed, this feature isn’t
> production-ready because the integration I did was quick and dirty – it
> mainly just sends commands to a running OpenVPN-GUI process in the same way
> as the context menu already does, with some fragile logic to locate the
> running OpenVPN process.
>
>
>
> That being said, the spelunking I did to hook up the plumbing with the
> Windows shell may prove useful in the future for something more fleshed out,
> and so I’m offering this up in case you’d all like to do something more
> involved with it.

Yeah, this is a good starting point for hooking to the GUI, hopefully
someone will do the "fleshing out" to make it more useful.

Do we have an experimental branch where we could add this so that we do
not lose track of it?

Selva

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] [openvpn-gui] Update system tray to populate Windows VPN flyout

2018-07-25 Thread Selva Nair
Hi,

On Wed, Jul 25, 2018 at 1:01 PM, Kevin Kane via Openvpn-devel
 wrote:
> From ed96e2d91a0eb9ecdaab8d7104f397f7d77e5ced Mon Sep 17 00:00:00 2001
>
> From: Kevin Kane 
>
> Date: Fri, 13 Jul 2018 09:50:00 -0700
>
> Subject: Update system tray to populate Windows VPN flyout
>
>
>
> Add a DLL to be wired in as a custom dialer, which introduces new build
> dependencies
>
> Add copyright notices as required where Microsoft has contributed code
>
>
>
> Signed-off-by: Kevin Kane 
>

As discussed in a previous thread, I'm not sure we should have this in
its current form:

(i) It adds and entry per config to network connections which will be
a nuisance for users who have far too many configs (there are some
users with 100's of config files) -- so we at least need an option to
enable/disable this.

(ii) This only supports connect, not disconnect and the entry does not
show whether its connected or not -- so this is a feature of very
dubious utility.

(iii) Communicating with the GUI using COPYDATA message (we support
connect, disconnect, status etc.) should be preferred over hard-coding
IDM_CONNECTMENU.

Selva

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH] [openvpn] Register/unregister trusted custom dialer DLL when installing/uninstalling service

2018-07-25 Thread Kevin Kane via Openvpn-devel
>From 58cc8b37f567da867e3a6e2efa4c15de36495a79 Mon Sep 17 00:00:00 2001
From: Kevin Kane 
Date: Fri, 13 Jul 2018 09:44:00 -0700
Subject: Register/unregister trusted custom dialer DLL when
installing/uninstalling service

Add copyright notice as required where Microsoft has contributed code

Signed-off-by: Kevin Kane 
---
src/openvpnserv/service.c | 241 +++---
1 file changed, 222 insertions(+), 19 deletions(-)

diff --git a/src/openvpnserv/service.c b/src/openvpnserv/service.c
index 7157bea2..edb21b0d 100644
--- a/src/openvpnserv/service.c
+++ b/src/openvpnserv/service.c
@@ -6,6 +6,7 @@
  *
  * Copyright (C) 1993 - 2000.  Microsoft Corporation.  All rights reserved.
  *  2013 Heiko Hund 
+ * Portions Copyright (C) 2018 Microsoft Corporation
  */

#include "service.h"
@@ -53,6 +54,195 @@ ReportStatusToSCMgr(SERVICE_STATUS_HANDLE service, 
SERVICE_STATUS *status)
 return res;
}

+static const TCHAR DialerDllName[] = TEXT("libopenvpndialer-0.dll");
+static const TCHAR RegValueName[] = TEXT("CustomDLL");
+
+static int
+HandleDialerRegistration(int uninstall)
+{
+TCHAR path[512], customDllString[1024];
+HKEY parametersKey;
+LONG result;
+DWORD customDllSize = sizeof(customDllString);
+
+/* Assumption is that the dialer DLL is installed to the same bin 
directory as everything else. */
+if (GetModuleFileName(NULL, path, sizeof(path)) == 0)
+{
+_tprintf(TEXT("Unable to get module path - %lu\n"), GetLastError());
+return 1;
+}
+
+/* The version of NSIS we use to create the installer doesn't yet have 
support for
+ * writing multi-string registry entries, which we need to do in order to 
register our
+ * custom dialer DLL. Instead, we'll update this registry entry on 
install/uninstall.
+ */
+
+TCHAR *lastBackslash = _tcsrchr(path, TEXT('\\'));
+if (NULL == lastBackslash)
+{
+_tprintf(TEXT("Could not locate last backslash in path: %s\n"), path);
+return 1;
+}
+
+lastBackslash[1] = TEXT('\0');
+
+/* Bounds checking. */
+if ((_tcslen(path) + _countof(DialerDllName)) > _countof(path))
+{
+_tprintf(TEXT("Out of buffer adding dialer filename to path"));
+return 1;
+}
+
+_tcscat(path, DialerDllName);
+
+result = RegOpenKeyEx(HKEY_LOCAL_MACHINE,
+TEXT("SYSTEM\\CurrentControlSet\\Services\\RasMan\\Parameters"),
+0,
+KEY_ALL_ACCESS,
+);
+if (ERROR_SUCCESS != result)
+{
+_tprintf(TEXT("Could not open RasMan parameters key: %ld\n"), result);
+return 1;
+}
+
+/* Must RegCloseKey(parametersKey) from this point */
+
+result = RegGetValue(parametersKey,
+NULL,
+RegValueName,
+RRF_RT_REG_MULTI_SZ,
+NULL,
+customDllString,
+);
+
+/* If we're installing, the key being absent is okay. Any other error is 
fatal. */
+if (ERROR_FILE_NOT_FOUND == result)
+{
+if (0 != uninstall)
+{
+_tprintf(TEXT("CustomDLL value was not found; skipping 
unregistration step\n"));
+RegCloseKey(parametersKey);
+return 0; /* Not a fatal error but nothing to do */
+}
+else
+{
+result = ERROR_SUCCESS;
+customDllString[0] = TEXT('\0');
+customDllSize = 0;
+}
+}
+else if (ERROR_SUCCESS != result)
+{
+_tprintf(TEXT("Could not open CustomDLL value: %ld\n"), result);
+RegCloseKey(parametersKey);
+return 1;
+}
+
+/* Determine if the custom dialer DLL is present in the registry setting. 
This is a multi-string so we can't
+ * use _tcsstr.
+ */
+TCHAR* p;
+for (p = customDllString; *p != TEXT('\0'); p += _tcslen(p) + 1)
+{
+if (0 == _tcsicmp(path, p))
+{
+/* It is. p points to where it begins. */
+break;
+}
+}
+/* If it's not present, p points at a NULL terminator. */
+
+/* If we're installing and the DLL is present in the registry, do nothing. 
*/
+/* If we're uninstalling and the DLL is not present in the registry, do 
nothing. */
+if ( ((0 == uninstall) && (*p != TEXT('\0'))) ||
+ ((0 != uninstall) && (*p == TEXT('\0'))) )
+{
+RegCloseKey(parametersKey);
+return 0;
+}
+
+/* If we're installing and the DLL is not present in the registry, append 
it at point p. */
+if ((0 == uninstall) && (*p == TEXT('\0')))
+{
+/* Make sure the string buffer has enough space left. p points at the 
second in the double-terminating null
+ * (or the sole NULL if the registry entry was empty or absent).
+ */
+if (((p - customDllString) + _tcslen(path) + 2) > 
_countof(customDllString))
+{
+_tprintf(TEXT("Not enough buffer to create new CustomDLL 
string\n"));
+RegCloseKey(parametersKey);
+return 1;
+ 

[Openvpn-devel] [PATCH] [openvpn-build] Install/uninstall dialer DLL as part of Windows installer operation

2018-07-25 Thread Kevin Kane via Openvpn-devel
>From 30e851ffafcc9ad76928d796f9b18144c8d79040 Mon Sep 17 00:00:00 2001
From: Kevin Kane 
Date: Fri, 13 Jul 2018 09:47:43 -0700
Subject: Install/uninstall dialer DLL as part of Windows installer operation

Signed-off-by: Kevin Kane 
---
windows-nsis/openvpn.nsi | 4 
1 file changed, 4 insertions(+)

diff --git a/windows-nsis/openvpn.nsi b/windows-nsis/openvpn.nsi
index e92904d..aff7cb3 100755
--- a/windows-nsis/openvpn.nsi
+++ b/windows-nsis/openvpn.nsi
@@ -1,6 +1,7 @@
; 
; * Copyright (C) 2002-2010 OpenVPN Technologies, Inc.   *
; * Copyright (C)  2012 Alon Bar-Lev  *
+; * Portions Copyright (C) 2018 Microsoft Corporation*
; *  This program is free software; you can redistribute it and/or modify*
; *  it under the terms of the GNU General Public License version 2  *
; *  as published by the Free Software Foundation.   *
@@ -443,8 +444,10 @@ Section /o "${PACKAGE_NAME} GUI" SecOpenVPNGUI

   ${If} ${RunningX64}
File 
"${OPENVPN_ROOT_X86_64}\bin\openvpn-gui.exe"
+ File 
"${OPENVPN_ROOT_X86_64}\bin\libopenvpndialer-0.dll"
${Else}
File "${OPENVPN_ROOT_I686}\bin\openvpn-gui.exe"
+ File 
"${OPENVPN_ROOT_I686}\bin\libopenvpndialer-0.dll"
${EndIf}

   ${If} ${SectionIsSelected} ${SecAddShortcutsWorkaround}
@@ -770,6 +773,7 @@ Section "Uninstall"
${EndIf}

   Delete "$INSTDIR\bin\openvpn-gui.exe"
+ Delete "$INSTDIR\bin\libopenvpndialer-0.dll"
Delete "$DESKTOP\${PACKAGE_NAME} GUI.lnk"

   Delete "$INSTDIR\bin\openvpn.exe"
--
2.17.1.windows.2

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v3 6/7] tls-crypt-v2: implement tls-crypt-v2 handshake

2018-07-25 Thread Steffan Karger
This makes clients send-and-use, and servers receive-unwrap-and-use
tls-crypt-v2 client keys, which completes the on-the-wire work.

Signed-off-by: Steffan Karger 
---
v3: include length in WKc, rebase on curent master / v3 patch set

 src/openvpn/init.c| 41 +++-
 src/openvpn/openvpn.h |  2 +
 src/openvpn/options.c |  6 +--
 src/openvpn/ssl.c | 79 ---
 src/openvpn/ssl_common.h  |  6 +++
 src/openvpn/tls_crypt.c   | 57 ++
 src/openvpn/tls_crypt.h   | 14 ++
 tests/unit_tests/openvpn/Makefile.am  |  2 +-
 tests/unit_tests/openvpn/test_tls_crypt.c |  6 +++
 9 files changed, 192 insertions(+), 21 deletions(-)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index ef7b422..01667c3 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2435,6 +2435,11 @@ key_schedule_free(struct key_schedule *ks, bool 
free_ssl_ctx)
 if (tls_ctx_initialised(>ssl_ctx) && free_ssl_ctx)
 {
 tls_ctx_free(>ssl_ctx);
+   /* XXX TODO merge conflict here - is this still correct? */
+free_key_ctx_bi(>tls_wrap_key);
+free_key_ctx(>tls_crypt_v2_server_key);
+buf_clear(>tls_crypt_v2_wkc);
+free_buf(>tls_crypt_v2_wkc);
 }
 CLEAR(*ks);
 }
@@ -2618,6 +2623,24 @@ do_init_crypto_tls_c1(struct context *c)
 /* initialize tls-auth/crypt key */
 do_init_tls_wrap_key(c);
 
+/* tls-crypt with client-specific keys (--tls-crypt-v2) */
+if (options->tls_crypt_v2_file)
+{
+if (options->tls_server)
+{
+tls_crypt_v2_init_server_key(>c1.ks.tls_crypt_v2_server_key,
+ true, options->tls_crypt_v2_file,
+ options->tls_crypt_v2_inline);
+}
+else
+{
+tls_crypt_v2_init_client_key(>c1.ks.tls_wrap_key,
+ >c1.ks.tls_crypt_v2_wkc,
+ options->tls_crypt_v2_file,
+ options->tls_crypt_v2_inline);
+}
+}
+
 c->c1.ciphername = options->ciphername;
 c->c1.authname = options->authname;
 c->c1.keysize = options->keysize;
@@ -2843,13 +2866,28 @@ do_init_crypto_tls(struct context *c, const unsigned 
int flags)
 }
 
 /* TLS handshake encryption (--tls-crypt) */
-if (options->ce.tls_crypt_file)
+if (options->ce.tls_crypt_file
+|| (options->ce.tls_crypt_v2_file && options->tls_client))
 {
 to.tls_wrap.mode = TLS_WRAP_CRYPT;
 to.tls_wrap.opt.key_ctx_bi = c->c1.ks.tls_wrap_key;
 to.tls_wrap.opt.pid_persist = >c1.pid_persist;
 to.tls_wrap.opt.flags |= CO_PACKET_ID_LONG_FORM;
 tls_crypt_adjust_frame_parameters();
+
+if (options->tls_crypt_v2_file)
+{
+to.tls_wrap.tls_crypt_v2_wkc = >c1.ks.tls_crypt_v2_wkc;
+}
+}
+
+if (options->tls_crypt_v2_file)
+{
+to.tls_crypt_v2 = true;
+if (options->tls_server)
+{
+to.tls_wrap.tls_crypt_v2_server_key = 
c->c1.ks.tls_crypt_v2_server_key;
+}
 }
 
 /* If we are running over TCP, allow for
@@ -4386,6 +4424,7 @@ inherit_context_child(struct context *dest,
 dest->c1.ks.ssl_ctx = src->c1.ks.ssl_ctx;
 dest->c1.ks.tls_wrap_key = src->c1.ks.tls_wrap_key;
 dest->c1.ks.tls_auth_key_type = src->c1.ks.tls_auth_key_type;
+dest->c1.ks.tls_crypt_v2_server_key = src->c1.ks.tls_crypt_v2_server_key;
 /* inherit pre-NCP ciphers */
 dest->c1.ciphername = src->c1.ciphername;
 dest->c1.authname = src->c1.authname;
diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
index c5c767c..96cff87 100644
--- a/src/openvpn/openvpn.h
+++ b/src/openvpn/openvpn.h
@@ -66,6 +66,8 @@ struct key_schedule
 /* optional TLS control channel wrapping */
 struct key_type tls_auth_key_type;
 struct key_ctx_bi tls_wrap_key;
+struct key_ctx tls_crypt_v2_server_key;
+struct buffer tls_crypt_v2_wkc; /**< Wrapped client key */
 };
 
 /*
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index acab042..8ce5cf4 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2742,10 +2742,10 @@ options_postprocess_verify_ce(const struct options 
*options, const struct connec
 {
 msg(M_USAGE, "--tls-auth and --tls-crypt are mutually exclusive");
 }
-if (options->tls_client && ((ce->tls_auth_file && 
ce->tls_crypt_v2_file)
-|| (ce->tls_crypt_file && ce->tls_crypt_v2_file)))
+if (options->client && options->ce.tls_crypt_v2_file
+&& (options->ce.tls_auth_file || options->ce.tls_crypt_file))
 {
-msg(M_USAGE, "--tls-auth, 

[Openvpn-devel] [PATCH v3 7/7] tls-crypt-v2: add script hook to verify metadata

2018-07-25 Thread Steffan Karger
To allow rejecting incoming connections very early in the handshake,
add a --tls-crypt-v2-verify option that allows administators to
run an external command to verify the metadata from the client key.
See doc/tls-crypt-v2.txt for more details.

Because of the extra dependencies, this requires adding a mock
parse_line() to the tls-crypt unit tests.

Signed-off-by: Antonio Quartulli 
Signed-off-by: Steffan Karger 
---
v3: rebase on curent master / v3 patch set

 Changes.rst   | 12 ++
 doc/openvpn.8 | 35 ++--
 src/openvpn/init.c|  1 +
 src/openvpn/options.c |  7 
 src/openvpn/options.h |  2 +
 src/openvpn/ssl.c | 16 ---
 src/openvpn/ssl_common.h  |  1 +
 src/openvpn/tls_crypt.c   | 69 ++-
 src/openvpn/tls_crypt.h   |  3 +-
 tests/unit_tests/openvpn/Makefile.am  | 12 --
 tests/unit_tests/openvpn/test_tls_crypt.c | 18 +++-
 11 files changed, 161 insertions(+), 15 deletions(-)

diff --git a/Changes.rst b/Changes.rst
index a6090cf..e77b3d7 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -1,3 +1,15 @@
+Overview of changes in 2.5
+==
+
+New features
+
+Client-specific tls-crypt keys (``--tls-crypt-v2``)
+``tls-crypt-v2`` adds the ability to supply each client with a unique
+tls-crypt key.  This allows large organisations and VPN providers to profit
+from the same DoS and TLS stack protection that small deployments can
+already achieve using ``tls-auth`` or ``tls-crypt``.
+
+
 Overview of changes in 2.4
 ==
 
diff --git a/doc/openvpn.8 b/doc/openvpn.8
index 597c0c4..cb61a53 100644
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -5296,9 +5296,38 @@ If supplied, include the supplied
 in the wrapped client key.  This metadata must be supplied in base64\-encoded
 form.  The metadata must be at most 735 bytes long (980 bytes in base64).
 
-.B TODO
-Metadata handling is not yet implemented.  This text will be updated by the
-commit that introduces metadata handling.
+If no metadata is supplied, OpenVPN will use a 64-bit unix timestamp
+representing the current time in UTC, encoded in network order, as metadata for
+the generated key.
+
+Servers can use
+.B \-\-tls\-crypt\-v2\-verify
+to specify a metadata verification command.
+.\"*
+.TP
+.B \-\-tls\-crypt\-v2\-verify cmd
+
+Run command
+.B cmd
+to verify the metadata of the client-specific tls-crypt-v2 key of a connecting
+client.  This allows server administrators to reject client connections, before
+exposing the TLS stack (including the notoriously dangerous X.509 and ASN.1
+stacks) to the connecting client.
+
+OpenVPN supplies the following env vars to the command:
+.RS
+.IP \[bu] 2
+script_type is set to "tls-crypt-v2-verify"
+.IP \[bu]
+metadata_type is set to "0" is the metadata was user supplied, or "1" if it's a
+64-bit unix timestamp representing the key creation time.
+.IP \[bu]
+metadata_file contains the filename of a temporary file that contains the 
client
+metadata.
+.RE
+
+.IP
+The command can reject the connection by exitingwith a non-zero exit code.
 .\"*
 .TP
 .B \-\-askpass [file]
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 01667c3..a349d42 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2887,6 +2887,7 @@ do_init_crypto_tls(struct context *c, const unsigned int 
flags)
 if (options->tls_server)
 {
 to.tls_wrap.tls_crypt_v2_server_key = 
c->c1.ks.tls_crypt_v2_server_key;
+to.tls_crypt_v2_verify_script = 
c->options.tls_crypt_v2_verify_script;
 }
 }
 
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 8ce5cf4..bd324fd 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -629,6 +629,8 @@ static const char usage_message[] =
 "--tls-crypt-v2-genkey client|server keyfile [base64 metadata]: Generate 
a\n"
 "  fresh tls-crypt-v2 client or server key, and store to\n"
 "  keyfile.  If supplied, include metadata in wrapped 
key.\n"
+"--tls-crypt-v2-verify cmd : Run command cmd to verify the metadata of 
the\n"
+"  client-supplied tls-crypt-v2 client key\n"
 "--askpass [file]: Get PEM password from controlling tty before we 
daemonize.\n"
 "--auth-nocache  : Don't cache --askpass or --auth-user-pass passwords.\n"
 "--crl-verify crl ['dir']: Check peer certificate against a CRL.\n"
@@ -8176,6 +8178,11 @@ add_option(struct options *options,
 options->tls_crypt_v2_metadata = p[3];
 }
 }
+else if (streq(p[0], "tls-crypt-v2-verify") && p[1] && !p[2])
+{
+VERIFY_PERMISSION(OPT_P_GENERAL);
+

[Openvpn-devel] [PATCH v3 2/7] tls-crypt-v2: add specification to doc/

2018-07-25 Thread Steffan Karger
This is a preliminary description of tls-crypt-v2.  It should give a good
impression about the reasoning and design behind tls-crypt-v2, but might
need some polishing and updating.

Signed-off-by: Steffan Karger 
---
v3: Include length in WKc

 doc/tls-crypt-v2.txt | 170 +++
 1 file changed, 170 insertions(+)
 create mode 100644 doc/tls-crypt-v2.txt

diff --git a/doc/tls-crypt-v2.txt b/doc/tls-crypt-v2.txt
new file mode 100644
index 000..cc6453c
--- /dev/null
+++ b/doc/tls-crypt-v2.txt
@@ -0,0 +1,170 @@
+Client-specific tls-crypt keys (--tls-crypt-v2)
+===
+
+This document describes the ``--tls-crypt-v2`` option, which enables OpenVPN
+to use client-specific ``--tls-crypt`` keys.
+
+Rationale
+-
+
+``--tls-auth`` and ``tls-crypt`` use a pre-shared group key, which is shared
+among all clients and servers in an OpenVPN deployment.  If any client or
+server is compromised, the attacker will have access to this shared key, and it
+will no longer provide any security.  To reduce the risk of loosing pre-shared
+keys, ``tls-crypt-v2`` adds the ability to supply each client with a unique
+tls-crypt key.  This allows large organisations and VPN providers to profit
+from the same DoS and TLS stack protection that small deployments can already
+achieve using ``tls-auth`` or ``tls-crypt``.
+
+Also, for ``tls-crypt``, even if all these peers succeed in keeping the key
+secret, the key lifetime is limited to roughly 8000 years, divided by the
+number of clients (see the ``--tls-crypt`` section of the man page).  Using
+client-specific keys, we lift this lifetime requirement to roughly 8000 years
+for each client key (which "Should Be Enough For Everybody (tm)").
+
+
+Introduction
+
+
+``tls-crypt-v2`` uses an encrypted cookie mechanism to introduce
+client-specific tls-crypt keys without introducing a lot of server-side state.
+The client-specific key is encrypted using a server key.  The server key is the
+same for all servers in a group.  When a client connects, it first sends the
+encrypted key to the server, such that the server can decrypt the key and all
+messages can thereafter be encrypted using the client-specific key.
+
+A wrapped (encrypted and authenticated) client-specific key can also contain
+metadata.  The metadata is wrapped together with the key, and can be used to
+allow servers to identify clients and/or key validity.  This allows the server
+to abort the connection immediately after receiving the first packet, rather
+than performing an entire TLS handshake.  Aborting the connection this early
+greatly improves the DoS resilience and reduces attack service against
+malicious clients that have the ``tls-crypt`` or ``tls-auth`` key.  This is
+particularly relevant for large deployments (think lost key or disgruntled
+employee) and VPN providers (clients are not trusted).
+
+To allow for a smooth transition, ``tls-crypt-v2`` is designed such that a
+server can enable both ``tls-crypt-v2`` and either ``tls-crypt`` or
+``tls-auth``.  This is achieved by introducing a P_CONTROL_HARD_RESET_CLIENT_V3
+opcode, that indicates that the client wants to use ``tls-crypt-v2`` for the
+current connection.
+
+For an exact specification and more details, read the Implementation section.
+
+
+Implementation
+--
+
+When setting up a tls-crypt-v2 group (similar to generating a tls-crypt or
+tls-auth key previously):
+
+1. Generate a tls-crypt-v2 server key using OpenVPN's ``--genkey``.  This key
+   contains 4 512-bit keys, of which we use:
+
+   * the first 256 bits of key 1 as AES-256-CTR encryption key ``Ke``
+   * the first 256 bits of key 2 as HMAC-SHA-256 authentication key ``Ka``
+
+2. Add the tls-crypt-v2 server key to all server configs
+   (``tls-crypt-v2 /path/to/server.key``)
+
+
+When provisioning a client, create a client-specific tls-crypt key:
+
+1. Generate 2048 bits client-specific key ``Kc``
+2. Optionally generate metadata
+3. Create a wrapped client key ``WKc``, using the same nonce-misuse-resistant
+   SIV conruction we use for tls-crypt:
+
+   ``len = len(Kc || metadata)`` (16 bit, network byte order)
+
+   ``T = HMAC-SHA256(Ka, len || Kc || metadata)``
+
+   ``IV = 128 most significant bits of T``
+
+   ``WKc = T || AES-256-CTR(Ke, IV, Kc || metadata || len)``
+
+4. Create a tls-crypt-v2 client key: PEM-encode ``Kc || WKc`` and store in a
+   file, using the header ``-BEGIN OpenVPN tls-crypt-v2 client key-``
+   and the footer ``-END OpenVPN tls-crypt-v2 client key-``.  (The PEM
+   format is simple, and following PEM allows us to use the crypto lib function
+   for en/decoding.)
+5. Add the tls-crypt-v2 client key to the client config
+   (``tls-crypt-v2 /path/to/client-specific.key``)
+
+
+When setting up the openvpn connection:
+
+1. The client reads the tls-crypt-v2 key from its config, and:
+
+   1. loads ``Kc`` as its tls-crypt key,
+   2. stores 

[Openvpn-devel] [PATCH v3 4/7] tls-crypt-v2: add unwrap_client_key

2018-07-25 Thread Steffan Karger
Add helper functions to unwrap tls-crypt-v2 client keys.

Signed-off-by: Steffan Karger 
---
v3: Include length in WKc

 src/openvpn/buffer.h  |   7 +
 src/openvpn/tls_crypt.c   | 120 ++
 tests/unit_tests/openvpn/test_tls_crypt.c | 253 +++---
 3 files changed, 360 insertions(+), 20 deletions(-)

diff --git a/src/openvpn/buffer.h b/src/openvpn/buffer.h
index 180dd56..0c730ae 100644
--- a/src/openvpn/buffer.h
+++ b/src/openvpn/buffer.h
@@ -830,6 +830,13 @@ buf_read_u32(struct buffer *buf, bool *good)
 }
 }
 
+/** Return true if buffer contents are equal */
+static inline bool
+buf_equal(const struct buffer *a, const struct buffer *b)
+{
+return BLEN(a) == BLEN(b) && 0 == memcmp(BPTR(a), BPTR(b), BLEN(a));
+}
+
 /**
  * Compare src buffer contents with match.
  * *NOT* constant time. Do not use when comparing HMACs.
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 103a4fc..6437eb1 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -433,6 +433,114 @@ tls_crypt_v2_wrap_client_key(struct buffer *wkc,
 return buf_copy(wkc, );
 }
 
+static bool
+tls_crypt_v2_unwrap_client_key(struct key2 *client_key, struct buffer 
*metadata,
+   struct buffer wrapped_client_key,
+   struct key_ctx *server_key)
+{
+const char *error_prefix = __func__;
+bool ret = false;
+struct gc_arena gc = gc_new();
+/* The crypto API requires one extra cipher block of buffer head room when
+ * decrypting, which nicely matches the tag size of WKc.  So
+ * TLS_CRYPT_V2_MAX_WKC_LEN is always large enough for the plaintext. */
+uint8_t plaintext_buf_data[TLS_CRYPT_V2_MAX_WKC_LEN] = { 0 };
+struct buffer plaintext = { 0 };
+
+dmsg(D_TLS_DEBUG_MED, "%s: unwrapping client key (len=%d): %s", __func__,
+ BLEN(_client_key), format_hex(BPTR(_client_key),
+   BLEN(_client_key),
+   0, ));
+
+if (TLS_CRYPT_V2_MAX_WKC_LEN < BLEN(_client_key))
+{
+CRYPT_ERROR("wrapped client key too big");
+}
+
+/* Decrypt client key and metadata */
+uint16_t net_len = 0;
+const uint8_t *tag = BPTR(_client_key);
+
+if (!buf_advance(_client_key, TLS_CRYPT_TAG_SIZE))
+{
+CRYPT_ERROR("failed to read tag");
+}
+
+if (BLEN(_client_key) < sizeof(net_len))
+{
+CRYPT_ERROR("failed to read length");
+}
+memcpy(_len, BEND(_client_key) - sizeof(net_len),
+   sizeof(net_len));
+buf_inc_len(_client_key, -(int)sizeof(net_len));
+
+if (ntohs(net_len) != BLEN(_client_key))
+{
+dmsg(D_TLS_DEBUG_LOW, "%s: net_len=%u, BLEN=%i", __func__,
+ ntohs(net_len), BLEN(_client_key));
+CRYPT_ERROR("invalid length");
+}
+
+if (!cipher_ctx_reset(server_key->cipher, tag))
+{
+CRYPT_ERROR("failed to initialize IV");
+}
+buf_set_write(, plaintext_buf_data, sizeof(plaintext_buf_data));
+int outlen = 0;
+if (!cipher_ctx_update(server_key->cipher, BPTR(), ,
+   BPTR(_client_key),
+   BLEN(_client_key)))
+{
+CRYPT_ERROR("could not decrypt client key");
+}
+ASSERT(buf_inc_len(, outlen));
+
+if (!cipher_ctx_final(server_key->cipher, BEND(), ))
+{
+CRYPT_ERROR("cipher final failed");
+}
+ASSERT(buf_inc_len(, outlen));
+
+/* Check authentication */
+uint8_t tag_check[TLS_CRYPT_TAG_SIZE] = { 0 };
+hmac_ctx_reset(server_key->hmac);
+hmac_ctx_update(server_key->hmac, (void*)_len, sizeof(net_len));
+hmac_ctx_update(server_key->hmac, BPTR(),
+BLEN());
+hmac_ctx_final(server_key->hmac, tag_check);
+
+if (memcmp_constant_time(tag, tag_check, sizeof(tag_check)))
+{
+dmsg(D_CRYPTO_DEBUG, "tag  : %s",
+ format_hex(tag, sizeof(tag_check), 0, ));
+dmsg(D_CRYPTO_DEBUG, "tag_check: %s",
+ format_hex(tag_check, sizeof(tag_check), 0, ));
+CRYPT_ERROR("client key authentication error");
+}
+
+if (buf_len() < sizeof(client_key->keys))
+{
+CRYPT_ERROR("failed to read client key");
+}
+memcpy(_key->keys, BPTR(), sizeof(client_key->keys));
+ASSERT(buf_advance(, sizeof(client_key->keys)));
+
+if(!buf_copy(metadata, ))
+{
+CRYPT_ERROR("metadata too large for supplied buffer");
+}
+
+ret = true;
+error_exit:
+if (!ret)
+{
+secure_memzero(client_key, sizeof(*client_key));
+}
+buf_clear();
+gc_free();
+return ret;
+}
+
 void
 tls_crypt_v2_write_server_key_file(const char *filename)
 {
@@ -542,6 +650,18 @@ tls_crypt_v2_write_client_key_file(const char *filename, 
const char *b64_metadat
 tls_crypt_v2_init_client_key(_client_key, _wrapped_client_key,
  filename, 

[Openvpn-devel] [PATCH v3 5/7] tls-crypt-v2: add P_CONTROL_HARD_RESET_CLIENT_V3 opcode

2018-07-25 Thread Steffan Karger
Not used yet, but prepare for sending and receiving tls-crypt-v2 handshake
messages.

Signed-off-by: Steffan Karger 
---
v3: rebase on curent master / v3 patch set

 src/openvpn/ps.c |  3 ++-
 src/openvpn/ssl.c| 23 ++-
 src/openvpn/ssl.h|  5 -
 src/openvpn/ssl_common.h |  2 ++
 4 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/src/openvpn/ps.c b/src/openvpn/ps.c
index 25ab374..adec072 100644
--- a/src/openvpn/ps.c
+++ b/src/openvpn/ps.c
@@ -985,7 +985,8 @@ is_openvpn_protocol(const struct buffer *buf)
 {
 return p[0] == 0
&& p[1] >= 14
-   && p[2] == (P_CONTROL_HARD_RESET_CLIENT_V2<= 2)
 {
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index dcb5445..757754f 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -792,6 +792,9 @@ packet_opcode_name(int op)
 case P_CONTROL_HARD_RESET_SERVER_V2:
 return "P_CONTROL_HARD_RESET_SERVER_V2";
 
+case P_CONTROL_HARD_RESET_CLIENT_V3:
+return "P_CONTROL_HARD_RESET_CLIENT_V3";
+
 case P_CONTROL_SOFT_RESET_V1:
 return "P_CONTROL_SOFT_RESET_V1";
 
@@ -864,7 +867,8 @@ is_hard_reset(int op, int key_method)
 
 if (!key_method || key_method >= 2)
 {
-if (op == P_CONTROL_HARD_RESET_CLIENT_V2 || op == 
P_CONTROL_HARD_RESET_SERVER_V2)
+if (op == P_CONTROL_HARD_RESET_CLIENT_V2 || op == 
P_CONTROL_HARD_RESET_SERVER_V2
+|| op == P_CONTROL_HARD_RESET_CLIENT_V3)
 {
 return true;
 }
@@ -1095,8 +1099,15 @@ tls_session_init(struct tls_multi *multi, struct 
tls_session *session)
 }
 else /* session->opt->key_method >= 2 */
 {
-session->initial_opcode = session->opt->server ?
-  P_CONTROL_HARD_RESET_SERVER_V2 : 
P_CONTROL_HARD_RESET_CLIENT_V2;
+if (session->opt->server)
+{
+session->initial_opcode = P_CONTROL_HARD_RESET_SERVER_V2;
+}
+else
+{
+session->initial_opcode = session->opt->tls_crypt_v2 ?
+P_CONTROL_HARD_RESET_CLIENT_V3 : 
P_CONTROL_HARD_RESET_CLIENT_V2;
+}
 }
 
 /* Initialize control channel authentication parameters */
@@ -3427,7 +3438,8 @@ tls_pre_decrypt(struct tls_multi *multi,
 {
 /* verify client -> server or server -> client connection */
 if (((op == P_CONTROL_HARD_RESET_CLIENT_V1
-  || op == P_CONTROL_HARD_RESET_CLIENT_V2) && 
!multi->opt.server)
+  || op == P_CONTROL_HARD_RESET_CLIENT_V2
+  || op == P_CONTROL_HARD_RESET_CLIENT_V3) && 
!multi->opt.server)
 || ((op == P_CONTROL_HARD_RESET_SERVER_V1
  || op == P_CONTROL_HARD_RESET_SERVER_V2) && 
multi->opt.server))
 {
@@ -3812,7 +3824,8 @@ tls_pre_decrypt_lite(const struct tls_auth_standalone 
*tas,
 /* this packet is from an as-yet untrusted source, so
  * scrutinize carefully */
 
-if (op != P_CONTROL_HARD_RESET_CLIENT_V2)
+if (op != P_CONTROL_HARD_RESET_CLIENT_V2
+&& op != P_CONTROL_HARD_RESET_CLIENT_V3)
 {
 /*
  * This can occur due to bogus data or DoS packets.
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 72227d9..852debd 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -63,9 +63,12 @@
 #define P_CONTROL_HARD_RESET_CLIENT_V2 7 /* initial key from client, 
forget previous state */
 #define P_CONTROL_HARD_RESET_SERVER_V2 8 /* initial key from server, 
forget previous state */
 
+/* indicates key_method >= 2 and client-specific tls-crypt key */
+#define P_CONTROL_HARD_RESET_CLIENT_V3 10/* initial key from client, 
forget previous state */
+
 /* define the range of legal opcodes */
 #define P_FIRST_OPCODE 1
-#define P_LAST_OPCODE  9
+#define P_LAST_OPCODE  10
 
 /*
  * Set the max number of acknowledgments that can "hitch a ride" on an outgoing
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index 08ef6ff..e3c852a 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -286,6 +286,8 @@ struct tls_options
 const char *config_authname;
 bool ncp_enabled;
 
+bool tls_crypt_v2;
+
 /** TLS handshake wrapping state */
 struct tls_wrap_ctx tls_wrap;
 
-- 
2.7.4


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v3 3/7] tls-crypt-v2: generate client keys

2018-07-25 Thread Steffan Karger
As a first step towards a full tls-crypt-v2 implementation, add
functionality to generate tls-crypt-v2 client keys.

Signed-off-by: Steffan Karger 
---
v3: Include length in WKc

 doc/openvpn.8   |  51 +
 src/openvpn/init.c  |  35 +-
 src/openvpn/integer.h   |  10 ++
 src/openvpn/options.c   |  66 ++-
 src/openvpn/options.h   |  14 +++
 src/openvpn/tls_crypt.c | 288 
 src/openvpn/tls_crypt.h |  81 --
 tests/t_lpback.sh   |  40 ++-
 8 files changed, 565 insertions(+), 20 deletions(-)

diff --git a/doc/openvpn.8 b/doc/openvpn.8
index f01b48b..597c0c4 100644
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -5248,6 +5248,57 @@ degrading to the same security as using
 That is, the control channel still benefits from the extra protection against
 active man\-in\-the\-middle\-attacks and DoS attacks, but may no longer offer
 extra privacy and post\-quantum security on top of what TLS itself offers.
+
+For large setups or setups where clients are not trusted, consider using
+.B \-\-tls\-crypt\-v2
+instead.  That uses per\-client unique keys, and thereby improves the bounds to
+\fR'rotate a client key at least once per 8000 years'.
+.\"*
+.TP
+.B \-\-tls\-crypt\-v2 keyfile
+
+Use client\-specific tls\-crypt keys.
+
+For clients,
+.B keyfile
+is a client\-specific tls\-crypt key.  Such a key can be generated using the
+.B \-\-tls\-crypt\-v2\-genkey
+option.
+
+For servers,
+.B keyfile
+is used to unwrap client\-specific keys supplied by the client during 
connection
+setup.  This key must be the same as the key used to generate the
+client\-specific key (see
+.B \-\-tls\-crypt\-v2\-genkey\fR).
+
+On servers, this option can be used together with the
+.B \-\-tls\-auth
+or
+.B \-\-tls\-crypt
+option.  In that case, the server will detect whether the client is using
+client\-specific keys, and automatically select the right mode.
+.\"*
+.TP
+.B \-\-tls\-crypt\-v2\-genkey client|server keyfile [metadata]
+
+If the first parameter equals "server", generate a \-\-tls\-crypt\-v2 server
+key and store the key in
+.B keyfile\fR.
+
+
+If the first parameter equals "client", generate a \-\-tls\-crypt\-v2 client
+key, and store the key in
+.B keyfile\fR.
+
+If supplied, include the supplied
+.B metadata
+in the wrapped client key.  This metadata must be supplied in base64\-encoded
+form.  The metadata must be at most 735 bytes long (980 bytes in base64).
+
+.B TODO
+Metadata handling is not yet implemented.  This text will be updated by the
+commit that introduces metadata handling.
 .\"*
 .TP
 .B \-\-askpass [file]
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index f432106..ef7b422 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1028,6 +1028,11 @@ print_openssl_info(const struct options *options)
 bool
 do_genkey(const struct options *options)
 {
+/* should we disable paging? */
+if (options->mlock && (options->genkey || 
options->tls_crypt_v2_genkey_file))
+{
+platform_mlockall(true);
+}
 if (options->genkey)
 {
 int nbits_written;
@@ -1035,11 +1040,6 @@ do_genkey(const struct options *options)
 notnull(options->shared_secret_file,
 "shared secret output file (--secret)");
 
-if (options->mlock) /* should we disable paging? */
-{
-platform_mlockall(true);
-}
-
 nbits_written = write_key_file(2, options->shared_secret_file);
 
 msg(D_GENKEY | M_NOPREFIX,
@@ -1047,6 +1047,31 @@ do_genkey(const struct options *options)
 options->shared_secret_file);
 return true;
 }
+if (options->tls_crypt_v2_genkey_type)
+{
+if(!strcmp(options->tls_crypt_v2_genkey_type, "server"))
+{
+
tls_crypt_v2_write_server_key_file(options->tls_crypt_v2_genkey_file);
+return true;
+}
+else if (options->tls_crypt_v2_genkey_type
+ && !strcmp(options->tls_crypt_v2_genkey_type, "client"))
+{
+if (!options->tls_crypt_v2_file)
+{
+msg(M_USAGE, "--tls-crypt-v2-gen-client-key requires a server 
key to be set via --tls-crypt-v2");
+}
+
+
tls_crypt_v2_write_client_key_file(options->tls_crypt_v2_genkey_file,
+options->tls_crypt_v2_metadata, options->tls_crypt_v2_file,
+options->tls_crypt_v2_inline);
+return true;
+}
+else
+{
+msg(M_USAGE, "--tls-crypt-v2-genkey type should be \"client\" or 
\"server\"");
+}
+}
 return false;
 }
 
diff --git a/src/openvpn/integer.h b/src/openvpn/integer.h
index a7e19d3..b1ae0ed 100644
--- a/src/openvpn/integer.h
+++ b/src/openvpn/integer.h
@@ -26,6 +26,16 @@
 
 

[Openvpn-devel] [PATCH v3 1/7] Introduce buffer_write_file()

2018-07-25 Thread Steffan Karger
Rewrite buf_write_string_file to buffer_write_file, which is simpler to
use and can deal with not-null-terminated strings.  Mostly implemented so
this can be easily reused for tls-crypt-v2 (client) key files.

Signed-off-by: Steffan Karger 
---
v3: split change out of "generate client key", reuse in write_key_file()

 src/openvpn/buffer.c | 31 ---
 src/openvpn/buffer.h |  7 ++-
 src/openvpn/crypto.c | 30 +-
 3 files changed, 31 insertions(+), 37 deletions(-)

diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index 0972139..20e2b9c 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -343,16 +343,33 @@ convert_to_one_line(struct buffer *buf)
 }
 }
 
-/* NOTE: requires that string be null terminated */
-void
-buf_write_string_file(const struct buffer *buf, const char *filename, int fd)
+bool
+buffer_write_file(const char *filename, const struct buffer *buf)
 {
-const int len = strlen((char *) BPTR(buf));
-const int size = write(fd, BPTR(buf), len);
-if (size != len)
+bool ret = false;
+int fd = platform_open(filename, O_CREAT | O_TRUNC | O_WRONLY,
+   S_IRUSR | S_IWUSR);
+if (fd == -1)
+{
+msg(M_ERRNO, "Cannot open file '%s' for write", filename);
+goto cleanup;
+}
+
+const int size = write(fd, BPTR(buf), BLEN(buf));
+if (size != BLEN(buf))
 {
-msg(M_ERR, "Write error on file '%s'", filename);
+msg(M_ERRNO, "Write error on file '%s'", filename);
+goto cleanup;
+}
+
+ret = true;
+cleanup:
+if (close(fd))
+{
+msg(M_ERRNO, "Close error on file %s", filename);
+ret = false;
 }
+return ret;
 }
 
 /*
diff --git a/src/openvpn/buffer.h b/src/openvpn/buffer.h
index 6b6025e..180dd56 100644
--- a/src/openvpn/buffer.h
+++ b/src/openvpn/buffer.h
@@ -469,11 +469,8 @@ const char *skip_leading_whitespace(const char *str);
 
 void string_null_terminate(char *str, int len, int capacity);
 
-/*
- * Write string in buf to file descriptor fd.
- * NOTE: requires that string be null terminated.
- */
-void buf_write_string_file(const struct buffer *buf, const char *filename, int 
fd);
+/** Write buffer contents to file */
+bool buffer_write_file(const char *filename, const struct buffer *buf);
 
 /*
  * write a string to the end of a buffer that was
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 5381ef2..35bd243 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -1427,27 +1427,19 @@ write_key_file(const int nkeys, const char *filename)
 {
 struct gc_arena gc = gc_new();
 
-int fd, i;
-int nbits = 0;
+int nbits = nkeys * sizeof(struct key) * 8;
 
 /* must be large enough to hold full key file */
 struct buffer out = alloc_buf_gc(2048, );
-struct buffer nbits_head_text = alloc_buf_gc(128, );
 
 /* how to format the ascii file representation of key */
 const int bytes_per_line = 16;
 
-/* open key file */
-fd = platform_open(filename, O_CREAT | O_TRUNC | O_WRONLY, S_IRUSR | 
S_IWUSR);
-
-if (fd == -1)
-{
-msg(M_ERR, "Cannot open shared secret file '%s' for write", filename);
-}
-
+/* write header */
+buf_printf(, "#\n# %d bit OpenVPN static key\n#\n", nbits);
 buf_printf(, "%s\n", static_key_head);
 
-for (i = 0; i < nkeys; ++i)
+for (int i = 0; i < nkeys; ++i)
 {
 struct key key;
 char *fmt;
@@ -1463,9 +1455,6 @@ write_key_file(const int nkeys, const char *filename)
 "\n",
 );
 
-/* increment random bits counter */
-nbits += sizeof(key) * 8;
-
 /* write to holding buffer */
 buf_printf(, "%s\n", fmt);
 
@@ -1476,17 +1465,8 @@ write_key_file(const int nkeys, const char *filename)
 
 buf_printf(, "%s\n", static_key_foot);
 
-/* write number of bits */
-buf_printf(_head_text, "#\n# %d bit OpenVPN static key\n#\n", nbits);
-buf_write_string_file(_head_text, filename, fd);
-
 /* write key file, now formatted in out, to file */
-buf_write_string_file(, filename, fd);
-
-if (close(fd))
-{
-msg(M_ERR, "Close error on shared secret file %s", filename);
-}
+buffer_write_file(filename, );
 
 /* zero memory which held file content (memory will be freed by GC) */
 buf_clear();
-- 
2.7.4


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel