Re: [Openvpn-devel] [PATCH v5 2/2] Add support for OpenSSL TLS 1.3 when using management-external-key

2019-09-23 Thread Selva Nair
Forgot copy this to the list -- sending again

On Mon, Sep 23, 2019 at 6:19 AM Arne Schwabe  wrote:
>
> Am 20.09.19 um 22:55 schrieb Selva Nair:
> > Hi,
> >
> > Reviving this thread/patch as now users are running into this padding
> > issue (trac 1216 ).
> >
> > IIRC, we more-or-less agreed upon adding an argument (nopadding, pss etc..)
> > to >PK_SIGN for new clients and erroring out with old clients that
> > cannot sign with PSS padding.
> >
> > Selva
> >
> Yeah.
>
> We did not really to a conclusion if we wanted backwards compatibility
> or not. Since it seems that OpenSSL 1.1.1 requires the management-client
> to understand the new way of signatures anyway, I would say we require
> the management client to be able to understand the signature in any case.
>
> I think the missing bit of piece for the patch is if we want to error
> out early if we detect a config that *might* not work (the nopadding
> argument or any other argument to the management-external-key) or if we
> do not error at this point and fail then when we actually require PSS
> signature. I am more for the first version because otherwise you end up
> with configurations that work fine until the server is upgraded to
> OpenSSL 1.1.1 and then the client stops working without anything being
> change (yes I realise that is already the case at the moment)

Well, I can live with that ---  at least we'll be able to tell the
users to update their client to get the signature request, which
is not the case now.

Selva


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


[Openvpn-devel] [PATCH applied] Re: Handle PSS padding in cryptoapicert

2019-09-23 Thread Gert Doering
Acked-by: Gert Doering 

Sorry for slacking.  

I have stared at the patch a bit, compared to the master patch, and built 
with mingw & openssl 1.0.2n & openssl 1.1.0j on ubuntu 16 (which went fine),
didn't test 1.1.1 as my build system was a bit less than cooperative 
today :-/

This is not a "full review" and I haven't actually *tested* the resulting 
binary, but since the changes are close enough to the two joined commits 
in master and those have been reviewed and tested, "this should be good 
enough".

Your patch has been applied to the release/2.4 branch.

commit 1ed687e72cc1cc46ed1f5f8d9d825db6693e4b3e
Author: Selva Nair
Date:   Sun Jul 28 16:34:21 2019 -0400

 Handle PSS padding in cryptoapicert

 Signed-off-by: Selva Nair 
 Acked-by: Gert Doering 
 Message-Id: <1564346061-5683-1-git-send-email-selva.n...@gmail.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18715.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



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


[Openvpn-devel] [PATCH applied] Re: tapctl: add optional 'hardware id' parameter

2019-09-23 Thread Gert Doering
Taking Simon's "LGTM" as an ACK (plus some own light staring at the
code changes which seem to make sense).  Test built ("it compiles,
ship it!") on ubuntu 1604/mingw.

Your patch has been applied to the master branch.

Acked-by: Simon Rozman 

commit e9ce348c93b99e76959b89739fbd74c43ee50152
Author: Lev Stipakov
Date:   Mon Sep 23 12:08:02 2019 +0300

 tapctl: add optional 'hardware id' parameter

 Signed-off-by: Lev Stipakov 
 Acked-by: Simon Rozman 
 Message-Id: <1569229682-9731-1-git-send-email-lstipa...@gmail.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18854.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



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


Re: [Openvpn-devel] [PATCH] Only announce IV_NCP=2 when we are willing to support these ciphers

2019-09-23 Thread Gert Doering
Hi,

On Mon, Sep 23, 2019 at 03:32:24PM +0200, Arne Schwabe wrote:
> +if (!(tls_item_in_cipher_list("AES-128-GCM", options->ncp_ciphers)
> +  && tls_item_in_cipher_list("AES-256-GCM", 
> options->ncp_ciphers)))

What about AES-192-GCM?  What *exactly* does IV_NCP=2 guarantee?

Can we have something nicer for cipher negotiation instead?

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
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] Only announce IV_NCP=2 when we are willing to support these ciphers

2019-09-23 Thread Antonio Quartulli
Hi,

On 23/09/2019 15:32, Arne Schwabe wrote:
> We currently always announce IV_NCP=2 when we support these ciphers even
> when we do not accept them. This lead to a server pushing a AES-GCM-128
> cipher to clients and the client then rejecting it.
> ---
>  src/openvpn/init.c   | 1 +
>  src/openvpn/openvpn.h| 1 +
>  src/openvpn/options.c| 7 +++
>  src/openvpn/ssl.c| 4 +++-
>  src/openvpn/ssl_common.h | 1 +
>  5 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index b5a034dc..32f7bc7a 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -2795,6 +2795,7 @@ do_init_crypto_tls(struct context *c, const unsigned 
> int flags)
>  to.tcp_mode = link_socket_proto_connection_oriented(options->ce.proto);
>  to.config_ciphername = c->c1.ciphername;
>  to.config_authname = c->c1.authname;
> +to.config_ncp_ciphers = c->c1.ncp_ciphers;


I can't find where config_ncp_ciphers is used and I can't find where
ncp_ciphers is set...something is missing?


>  to.ncp_enabled = options->ncp_enabled;
>  to.transition_window = options->transition_window;
>  to.handshake_window = options->handshake_window;
> diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
> index 29d21f0a..1fdbfe3c 100644
> --- a/src/openvpn/openvpn.h
> +++ b/src/openvpn/openvpn.h
> @@ -208,6 +208,7 @@ struct context_1
>  
>  const char *ciphername; /**< Data channel cipher from config file */
>  const char *authname;   /**< Data channel auth from config file */
> +const char *ncp_ciphers;/**< NCP Ciphers */
>  int keysize;/**< Data channel keysize from config file */
>  #endif
>  };
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index c84b9d5e..cb25db5b 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -7687,6 +7687,13 @@ add_option(struct options *options,
>  {
>  VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_INSTANCE);
>  options->ncp_ciphers = p[1];
> +
> +if (!(tls_item_in_cipher_list("AES-128-GCM", options->ncp_ciphers)
> +  && tls_item_in_cipher_list("AES-256-GCM", 
> options->ncp_ciphers)))
> +{
> +msg(M_INFO, "Not including AES-128-GCM and AES-256-GCM in 
> ncp-ciphers "
> +"disables announcing NCP support with IV_NCP=2");
> +}

The condition and the text message are not agreeing with each other.

How about making the if condition easier to read by applying De Morgan's
law:


if (!tls_item_in_cipher_list("AES-128-GCM", options->ncp_ciphers)
|| !tls_item_in_cipher_list("AES-256-GCM", options->ncp_ciphers)))


This reads to me as: "Not enabling AES-128 or AES-256 disables NCP"
(Which I think is what you wanted to express in the debug message?)

What do you think?

>  }
>  else if (streq(p[0], "ncp-disable") && !p[1])
>  {
> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
> index d5833ac6..f1719303 100644
> --- a/src/openvpn/ssl.c
> +++ b/src/openvpn/ssl.c
> @@ -2327,7 +2327,9 @@ push_peer_info(struct buffer *buf, struct tls_session 
> *session)
>  
>  /* support for Negotiable Crypto Parameters */
>  if (session->opt->ncp_enabled
> -&& (session->opt->mode == MODE_SERVER || session->opt->pull))
> +&& (session->opt->mode == MODE_SERVER || session->opt->pull)
> +&& tls_item_in_cipher_list("AES-128-GCM", 
> session->opt->config_ncp_ciphers)
> +&& tls_item_in_cipher_list("AES-256-GCM", 
> session->opt->config_ncp_ciphers))
>  {
>  buf_printf(, "IV_NCP=2\n");
>  }
> diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
> index 0312c1f8..133c3c22 100644
> --- a/src/openvpn/ssl_common.h
> +++ b/src/openvpn/ssl_common.h
> @@ -290,6 +290,7 @@ struct tls_options
>  
>  const char *config_ciphername;
>  const char *config_authname;
> +const char *config_ncp_ciphers;
>  bool ncp_enabled;
>  
>  bool tls_crypt_v2;
> 


Cheers,

-- 
Antonio Quartulli


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


[Openvpn-devel] [PATCH] Only announce IV_NCP=2 when we are willing to support these ciphers

2019-09-23 Thread Arne Schwabe
We currently always announce IV_NCP=2 when we support these ciphers even
when we do not accept them. This lead to a server pushing a AES-GCM-128
cipher to clients and the client then rejecting it.
---
 src/openvpn/init.c   | 1 +
 src/openvpn/openvpn.h| 1 +
 src/openvpn/options.c| 7 +++
 src/openvpn/ssl.c| 4 +++-
 src/openvpn/ssl_common.h | 1 +
 5 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index b5a034dc..32f7bc7a 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2795,6 +2795,7 @@ do_init_crypto_tls(struct context *c, const unsigned int 
flags)
 to.tcp_mode = link_socket_proto_connection_oriented(options->ce.proto);
 to.config_ciphername = c->c1.ciphername;
 to.config_authname = c->c1.authname;
+to.config_ncp_ciphers = c->c1.ncp_ciphers;
 to.ncp_enabled = options->ncp_enabled;
 to.transition_window = options->transition_window;
 to.handshake_window = options->handshake_window;
diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
index 29d21f0a..1fdbfe3c 100644
--- a/src/openvpn/openvpn.h
+++ b/src/openvpn/openvpn.h
@@ -208,6 +208,7 @@ struct context_1
 
 const char *ciphername; /**< Data channel cipher from config file */
 const char *authname;   /**< Data channel auth from config file */
+const char *ncp_ciphers;/**< NCP Ciphers */
 int keysize;/**< Data channel keysize from config file */
 #endif
 };
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index c84b9d5e..cb25db5b 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -7687,6 +7687,13 @@ add_option(struct options *options,
 {
 VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_INSTANCE);
 options->ncp_ciphers = p[1];
+
+if (!(tls_item_in_cipher_list("AES-128-GCM", options->ncp_ciphers)
+  && tls_item_in_cipher_list("AES-256-GCM", options->ncp_ciphers)))
+{
+msg(M_INFO, "Not including AES-128-GCM and AES-256-GCM in 
ncp-ciphers "
+"disables announcing NCP support with IV_NCP=2");
+}
 }
 else if (streq(p[0], "ncp-disable") && !p[1])
 {
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index d5833ac6..f1719303 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -2327,7 +2327,9 @@ push_peer_info(struct buffer *buf, struct tls_session 
*session)
 
 /* support for Negotiable Crypto Parameters */
 if (session->opt->ncp_enabled
-&& (session->opt->mode == MODE_SERVER || session->opt->pull))
+&& (session->opt->mode == MODE_SERVER || session->opt->pull)
+&& tls_item_in_cipher_list("AES-128-GCM", 
session->opt->config_ncp_ciphers)
+&& tls_item_in_cipher_list("AES-256-GCM", 
session->opt->config_ncp_ciphers))
 {
 buf_printf(, "IV_NCP=2\n");
 }
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index 0312c1f8..133c3c22 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -290,6 +290,7 @@ struct tls_options
 
 const char *config_ciphername;
 const char *config_authname;
+const char *config_ncp_ciphers;
 bool ncp_enabled;
 
 bool tls_crypt_v2;
-- 
2.22.0



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


Re: [Openvpn-devel] [PATCH v2 for 2.4] Handle PSS padding in cryptoapicert

2019-09-23 Thread Selva Nair
Hi,

On Sun, Jul 28, 2019 at 4:34 PM  wrote:
>
> From: Selva Nair 
>
> For PSS padding, CNG requires the digest to be signed
> and the digest algorithm in use, which are not accessible
> via the rsa_sign and rsa_priv_enc callbacks of OpenSSL.
> This patch uses the EVP_KEY interface to hook to
> evp_pkey_sign callback if OpenSSL version is > 1.1.0.
>
> Mapping of OpenSSL hash algorithm types to CNG is moved
> to a function for code-reuse.
>
> To test, both the server and client should be built with
> OpenSSL 1.1.1 and use TLS version >= 1.2
>
> Tested on Windows 7 client against a Linux server.
>
> Signed-off-by: Selva Nair 
> ---
> v2: rebased to release/2.4 after siglen -> *siglen change

As this is required for cryptoapicert +  OpenSSL 1.1.1,
nudging for a review and have it included in the next release.

We have already merged this into git master, but the patch here
is slightly different because of context differences and code
refactorings in 2.5.

Thanks,

Selva


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


Re: [Openvpn-devel] [PATCH v5 2/2] Add support for OpenSSL TLS 1.3 when using management-external-key

2019-09-23 Thread Arne Schwabe
Am 20.09.19 um 22:55 schrieb Selva Nair:
> Hi,
> 
> Reviving this thread/patch as now users are running into this padding
> issue (trac 1216 ).
> 
> IIRC, we more-or-less agreed upon adding an argument (nopadding, pss etc..)
> to >PK_SIGN for new clients and erroring out with old clients that
> cannot sign with PSS padding.
> 
> Selva
> 
Yeah.

We did not really to a conclusion if we wanted backwards compatibility
or not. Since it seems that OpenSSL 1.1.1 requires the management-client
to understand the new way of signatures anyway, I would say we require
the management client to be able to understand the signature in any case.

I think the missing bit of piece for the patch is if we want to error
out early if we detect a config that *might* not work (the nopadding
argument or any other argument to the management-external-key) or if we
do not error at this point and fail then when we actually require PSS
signature. I am more for the first version because otherwise you end up
with configurations that work fine until the server is upgraded to
OpenSSL 1.1.1 and then the client stops working without anything being
change (yes I realise that is already the case at the moment)

Arne


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


Re: [Openvpn-devel] [PATCH] tapctl: add optional "hardware id" parameter

2019-09-23 Thread Simon Rozman
Hi,

LGTM

Best regards,
Simon

> -Original Message-
> From: Lev Stipakov 
> Sent: Monday, September 23, 2019 11:08 AM
> To: openvpn-devel@lists.sourceforge.net
> Subject: [Openvpn-devel] [PATCH] tapctl: add optional "hardware id"
> parameter
> 
> From: Lev Stipakov 
> 
> If parameter is not specified, default value "root\tap0901"
> is used.
> 
> This enables tapctl to work with different tun drivers, like "tapoas"
> (from OpenVPN Connect) or "wintun".
> 
> Signed-off-by: Lev Stipakov 


smime.p7s
Description: S/MIME cryptographic signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/7] Visual Studio: upgrade project files to VS2019

2019-09-23 Thread Lev Stipakov
Since distributing own wintun binaries goes against recommended way (which
is MSM modules), here are steps to try out openvpn with wintun (which is
even simpler than previous way):

 - Install wireguard windows client from https://www.wireguard.com/install/
 - Download patched openvpn binaries from
https://staging.openvpn.net/openvpn2/openvpn2-wintun-support.zip
 - Unpack downloaded archive and run in administrative command prompt:
c:\Temp\openvpn>tapctl.exe create --hwid wintun
 - That's it! You have created wintun network adapter and ready to use
openvpn with wintun.

This uses tapctl, which is openvpn 2.5+ tool to manipulate tun/tap
adapters, which I have patched to work with wintun (
https://patchwork.openvpn.net/patch/833/).

See more detailed instruction here: http://staging.openvpn.net/openvpn2/

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


[Openvpn-devel] [PATCH] tapctl: add optional "hardware id" parameter

2019-09-23 Thread Lev Stipakov
From: Lev Stipakov 

If parameter is not specified, default value "root\tap0901"
is used.

This enables tapctl to work with different tun drivers,
like "tapoas" (from OpenVPN Connect) or "wintun".

Signed-off-by: Lev Stipakov 
---
 src/openvpnmsica/msica_op.c | 10 +-
 src/openvpnmsica/openvpnmsica.c |  2 +-
 src/tapctl/main.c   | 37 +
 src/tapctl/tap.c| 24 ++--
 src/tapctl/tap.h| 10 ++
 5 files changed, 67 insertions(+), 16 deletions(-)

diff --git a/src/openvpnmsica/msica_op.c b/src/openvpnmsica/msica_op.c
index 3b9878d..63aa6c8 100644
--- a/src/openvpnmsica/msica_op.c
+++ b/src/openvpnmsica/msica_op.c
@@ -446,7 +446,7 @@ msica_op_tap_interface_create_exec(
 
 /* Get all available network interfaces. */
 struct tap_interface_node *pInterfaceList = NULL;
-DWORD dwResult = tap_list_interfaces(NULL, , TRUE);
+DWORD dwResult = tap_list_interfaces(NULL, NULL, , TRUE);
 if (dwResult == ERROR_SUCCESS)
 {
 /* Does interface exist? */
@@ -457,7 +457,7 @@ msica_op_tap_interface_create_exec(
 /* No interface with a same name found. Create one. */
 BOOL bRebootRequired = FALSE;
 GUID guidInterface;
-dwResult = tap_create_interface(NULL, NULL, , 
);
+dwResult = tap_create_interface(NULL, NULL, NULL, 
, );
 if (dwResult == ERROR_SUCCESS)
 {
 /* Set interface name. */
@@ -601,7 +601,7 @@ msica_op_tap_interface_delete_by_name_exec(
 
 /* Get available TUN/TAP interfaces. */
 struct tap_interface_node *pInterfaceList = NULL;
-DWORD dwResult = tap_list_interfaces(NULL, , FALSE);
+DWORD dwResult = tap_list_interfaces(NULL, NULL, , FALSE);
 if (dwResult == ERROR_SUCCESS)
 {
 /* Does interface exist? */
@@ -659,7 +659,7 @@ msica_op_tap_interface_delete_by_guid_exec(
 
 /* Get all available interfaces. */
 struct tap_interface_node *pInterfaceList = NULL;
-DWORD dwResult = tap_list_interfaces(NULL, , TRUE);
+DWORD dwResult = tap_list_interfaces(NULL, NULL, , TRUE);
 if (dwResult == ERROR_SUCCESS)
 {
 /* Does interface exist? */
@@ -718,7 +718,7 @@ msica_op_tap_interface_set_name_exec(
 
 /* Get all available network interfaces. */
 struct tap_interface_node *pInterfaceList = NULL;
-DWORD dwResult = tap_list_interfaces(NULL, , TRUE);
+DWORD dwResult = tap_list_interfaces(NULL, NULL, , TRUE);
 if (dwResult == ERROR_SUCCESS)
 {
 /* Does interface exist? */
diff --git a/src/openvpnmsica/openvpnmsica.c b/src/openvpnmsica/openvpnmsica.c
index f5ad229..16381ea 100644
--- a/src/openvpnmsica/openvpnmsica.c
+++ b/src/openvpnmsica/openvpnmsica.c
@@ -478,7 +478,7 @@ FindTAPInterfaces(_In_ MSIHANDLE hInstall)
 
 /* Get all TUN/TAP network interfaces. */
 struct tap_interface_node *pInterfaceList = NULL;
-uiResult = tap_list_interfaces(NULL, , FALSE);
+uiResult = tap_list_interfaces(NULL, NULL, , FALSE);
 if (uiResult != ERROR_SUCCESS)
 {
 goto cleanup_CoInitialize;
diff --git a/src/tapctl/main.c b/src/tapctl/main.c
index 04c03dd..bf21586 100644
--- a/src/tapctl/main.c
+++ b/src/tapctl/main.c
@@ -81,7 +81,9 @@ static const TCHAR usage_message_create[] =
 TEXT("   specified, a default interface name is chosen by 
Windows.   \n")
 TEXT("   Note: This name can also be specified as OpenVPN's 
--dev-node   \n")
 TEXT("   option.   
  \n")
-TEXT("\n")
+TEXT("--hwid   Interface hardware id. Default value is 
root\\tap0901, which\n")
+TEXT("   describes tap-windows6 driver. To work with wintun 
driver,  \n")
+TEXT("   specify 'wintun'. 
  \n")
 TEXT("Output:\n")
 TEXT("\n")
 TEXT("This command prints newly created TUN/TAP interface's GUID to 
stdout.  \n")
@@ -96,6 +98,11 @@ static const TCHAR usage_message_list[] =
 TEXT("\n")
 TEXT("tapctl list\n")
 TEXT("\n")
+TEXT("Options:\n")
+TEXT("\n")
+TEXT("--hwid   Interface hardware id. Default value is 
root\\tap0901, which\n")
+TEXT("   describes tap-windows6 driver. To work with wintun 
driver,  \n")
+TEXT("   specify 'wintun'. 
  \n")
 TEXT("Output:\n")
 TEXT("\n")
 TEXT("This command prints all TUN/TAP interfaces to stdout.
  \n")
@@ -170,6 +177,7 @@ _tmain(int argc, LPCTSTR argv[])
 else if (_tcsicmp(argv[1], TEXT("create")) == 0)
 {
 LPCTSTR szName = NULL;
+LPCTSTR szHwId = NULL;
 
 /* Parse options. */
 for (int i = 2; i < argc; i++)
@@ -179,6 +187,11 @@ _tmain(int argc, LPCTSTR argv[])