openvpn-devel Mailing List for OpenVPN

I have added wintun's MSM (official distribution way) to windows client installer: https://staging.openvpn.net/openvpn2/openvpn-install-2.5_git-I607-Win10.exe To use wintun driver instead of tap-window6, add "windows-driver wintun" to your VPN profile. Fresh performance numbers (with recent -O2 fix to windows build system): Server - openvpn 2.4.4 tap-windows6: 390Mbit/s wintun: 730Mbit/s Server - openvpn3 with kernel acceleration tap-windows6: 405Mbit/s wintun: 1.05Gbit/s -- -Lev 

The final patches of the auth-token hmac support patches had a typo in the P2MP_SERVER fencing breaking --disable-server builds. It used #if instead of #ifdef. While at it, also fix another missing P2MP_SERVER fencing causing the compiler to complain about an unused variable in push.c Signed-off-by: David Sommerseth <da...@op...> --- src/openvpn/push.c | 2 ++ src/openvpn/ssl_common.h | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/openvpn/push.c b/src/openvpn/push.c index dc1a536a..49b9d1be 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -39,7 +39,9 @@ #if P2MP +#ifdef P2MP_SERVER static char push_reply_cmd[] = "PUSH_REPLY"; +#endif /* * Auth username/password diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index 406601bc..5e21009e 100644 --- a/src/openvpn/ssl_common.h +++ b/src/openvpn/ssl_common.h @@ -530,7 +530,7 @@ struct tls_multi time_t tas_last; #endif -#if P2MP_SERVER +#ifdef P2MP_SERVER /* * An error message to send to client on AUTH_FAILED */ -- 2.17.1 

When building with --disable-management, the compiler complains with implicit declaration of function ‘ssl_clean_auth_token’. This is due to the ssl_clean_auth_token() function being declared inside an #ifdef ENABLE_MANAGEMENT fence where it should not be. Signed-off-by: David Sommerseth <da...@op...> --- src/openvpn/ssl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index e98c54c7..a944ca3a 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -431,6 +431,8 @@ void ssl_purge_auth(const bool auth_user_pass_only); void ssl_set_auth_token(const char *token); +bool ssl_clean_auth_token(void); + #ifdef ENABLE_MANAGEMENT /* * ssl_get_auth_challenge will parse the server-pushed auth-failed @@ -439,8 +441,6 @@ void ssl_set_auth_token(const char *token); */ void ssl_purge_auth_challenge(void); -bool ssl_clean_auth_token(void); - void ssl_put_auth_challenge(const char *cr_str); #endif -- 2.17.1 

A couple of places the documentation was not clear enough or not even correct. Just improve this to avoid confusion later on. Signed-off-by: David Sommerseth <da...@op...> --- src/openvpn/auth_token.h | 2 -- src/openvpn/ssl_common.h | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/openvpn/auth_token.h b/src/openvpn/auth_token.h index c10afde9..4b014d44 100644 --- a/src/openvpn/auth_token.h +++ b/src/openvpn/auth_token.h @@ -61,8 +61,6 @@ generate_auth_token(const struct user_pass *up, struct tls_multi *multi); * Verifies the auth token to be in the format that generate_auth_token * create and checks if the token is valid. * - * Also calls generate_auth_token to update the auth-token to extend - * its validity */ unsigned verify_auth_token(struct user_pass *up, struct tls_multi *multi, diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index 5e21009e..8dd08862 100644 --- a/src/openvpn/ssl_common.h +++ b/src/openvpn/ssl_common.h @@ -559,7 +559,7 @@ struct tls_multi * Auth-token is only valid for an empty username * and not the username actually supplied from the client * - * OpenVPN 3 clients sometimes the empty username with a + * OpenVPN 3 clients sometimes wipes or replaces the username with a * username hint from their config. */ int auth_token_state_flags; -- 2.17.1 

During the review of the auth-token-hmac patches from Arne Schwabe, there were still a few minor issues left out. To avoid holding back further inclusion, we decided to add these patches on top of the patches from Arne. The first patch, --disable-server fix, is only needed when running ./configure with --disable-server. Otherwise the existing patches works fine. The second patch, --disable-management fix, just fixes a long outstanding compiler warning. And the final patch with comment updates are just clarifications and minor improvements. David Sommerseth (3): auth-token: Fix building with --disable-server auth-token: Fix compiler complaints with --disable-management Improve the comments related to auth-token-hmac patches src/openvpn/auth_token.h | 2 -- src/openvpn/push.c | 2 ++ src/openvpn/ssl.h | 4 ++-- src/openvpn/ssl_common.h | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) -- kind regards, David Sommerseth OpenVPN Inc 

On 17/09/2019 14:10, Arne Schwabe wrote: > From: Arne Schwabe <ar...@op...> > > This allows an external authentication method > (e.g. management interface) to track the connection and distinguish a > reconnection from multiple connections. > > Addtionally this now also checks to workaround a problem with > OpenVPN 3 core that sometimes uses a username hint from the config > instead of using an empty username if the token would be valid > with an empty username. Accepting such token can be only done > explicitly when the external-auth keyword to auth-gen-token is present. > > Patch V2: Add Empty variants to work around behaviour in openvpn 3 > Patch V3: document the behaviour of external-auth better in the man page, > rename 'auth' parameter to 'external-auth' > Patch V4: Rebase on current master > Patch V6: Fix tls_lock_username rejecting clients with empty username > when explicitly accepting them with external-auth > Patch V7: Fix compiling with disable-server > --- > doc/openvpn.8 | 37 +++++++++- > src/openvpn/auth_token.c | 156 ++++++++++++++++++++++++++++++++++++--- > src/openvpn/auth_token.h | 15 +++- > src/openvpn/init.c | 1 + > src/openvpn/manage.c | 4 +- > src/openvpn/options.c | 14 +++- > src/openvpn/options.h | 4 +- > src/openvpn/ssl_common.h | 10 ++- > src/openvpn/ssl_verify.c | 70 ++++++++++++------ > 9 files changed, 270 insertions(+), 41 deletions(-) This patch works ... but there are some comments which got ignored from the previous patch review. I will send a patch for this, as this is mostly documentation errors, not code errors. So, I'll approve this now; these patches needs to be completed now. Acked-By: David Sommerseth <da...@op...> -- kind regards, David Sommerseth OpenVPN Inc 

On 17/09/2019 14:10, Arne Schwabe wrote: > The previous auth-token implementation had a serious problem, especially when > paired with an unpatched OpenVPN client that keeps trying the auth-token > (commit e61b401a). > > The auth-token-gen implementation forgot the auth-token on reconnect, this > lead to reconnect with auth-token never working. > > This new implementation implements the auth-token in a stateles variant. By > using HMAC to sign the auth-token the server can verify if a token has been > authenticated and by checking the embedded timestamp in the token it can > also verify that the auth-token is still valid. > > Using the new config directive auth-gen-token-secret instead of > extending auth-gen-token (--auth-gen-token [lifetime] [secret-key]) was > chosen to allow inlinening the secret key. > > Patch V2: cleaned up code, use refactored read_pem_key_file function > Patch V3: clarify some design decision in the commit message > Patch V4: Use ephermal_generate_key > Patch V5: Use C99 PRIu64 instead of %lld int printf like statement, > fix strict aliasing > Patch V6: Rebase on master > Patch V7: fix compiling with --disable-server > --- > doc/openvpn.8 | 25 ++++ > src/openvpn/Makefile.am | 1 + > src/openvpn/auth_token.c | 273 +++++++++++++++++++++++++++++++++++++++ > src/openvpn/auth_token.h | 116 +++++++++++++++++ > src/openvpn/init.c | 34 ++++- > src/openvpn/openvpn.h | 1 + > src/openvpn/options.c | 22 +++- > src/openvpn/options.h | 4 + > src/openvpn/push.c | 70 ++++++++-- > src/openvpn/push.h | 8 ++ > src/openvpn/ssl.c | 7 +- > src/openvpn/ssl_common.h | 36 ++++-- > src/openvpn/ssl_verify.c | 184 ++++++++++++-------------- > 13 files changed, 646 insertions(+), 135 deletions(-) > create mode 100644 src/openvpn/auth_token.c > create mode 100644 src/openvpn/auth_token.h This looks good. The changes was a bit bigger than expected, but it this (and the rest of the patches) passes compiling on our buildbot rig - including --disable-server. Acked-By: David Sommerseth <da...@op...> -- kind regards, David Sommerseth OpenVPN Inc 

Hi, Our community meetings will alternate between Wed 11:30 CEST and Thu 20:00 CEST. Next meetings have been scheduled to - Wed 2nd October 11:30 CEST - Thu 10th October 20:00 CEST - Wed 16th October 11:30 CEST - Thu 24th October 20:00 CEST - Wed 30th October 11:30 CEST The place is #openvpn-meeting IRC channel at Freenode. Meeting agendas and summaries are in here: <https://community.openvpn.net/openvpn/wiki/IrcMeetings> Samuli 

Hi, Here's the summary of the IRC meeting. --- COMMUNITY MEETING Place: #openvpn-meeting on irc.freenode.net Date: Thursday 26th September 2019 Time: 20:00 CEST (18:00 UTC) Planned meeting topics for this meeting were here: <https://community.openvpn.net/openvpn/wiki/Topics-2019-09-26> Your local meeting time is easy to check from services such as <http://www.timeanddate.com/worldclock> SUMMARY dazo, lev, mattock, ordex and zx2c4 participated in this meeting. --- Discussed broken buildslaves/builds and noted that they still need to be fixed. --- Discussed relative merits of Buildbot and Jenkins. Dazo has had some strange issues with Buildbot and upgrading Buildbot might be a worthy effort. Migrating to Jenkins would be a big effort and it is unclear if it would be a better solution in our particular use-case. --- Noted that mattock still needs to produce a tap-windows6 test installer with PRs from here: <https://github.com/OpenVPN/tap-windows6/pulls> As mentioned in last week's meeting summary if testing of that installer goes well and Selva gives his ACK on <https://github.com/OpenVPN/tap-windows6/pull/86> we will include the new driver in the OpenVPN 2.4.8 installer. -- Discussed the problem of OpenVPN Inc. people getting dragged into internal projects due to internal pressure and thus unintentionally deprioritizing community work. The OpenVPN 3 team has recently tried to allocate Friday for community work. Mattock will attempt to follow suit going forward. -- Agreed that we want wintun in OpenVPN 2.5 installers. According to zx2c4 the upcoming Wintun 1.0 API version will be stable. As far as Wireguard is concerned the API needs no changes at the moment, but zx2c4 is still waiting for input from external parties. Wintun installation can be handled either as Merge Modules (MSM) in an MSI installer. Or a silent wintun MSI installer can be embedded into an MSI installer. -- Full chatlog attached. 

Your patch has been applied to the master and release/2.4 branch. Thanks. commit 7e4a261cc92a813f9e9ba9ee91c6e08de9d843f8 (master) commit ab34d883901e8e59abf5cc5990f1f206c9b0dc58 (release/2.4) Author: Kyle Evans Date: Fri Sep 6 19:44:59 2019 +0200 tests/t_lpback.sh: Switch sed(1) to POSIX-compatible regex. Signed-off-by: Matthias Andree <mat...@gm...> Acked-by: Gert Doering <ge...@gr...> Message-Id: <201...@gm...> URL: https://www.mail-archive.com/ope...@li.../msg18806.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

Hi, On Fri, Sep 06, 2019 at 07:44:59PM +0200, Matthias Andree wrote: > --- a/tests/t_lpback.sh > +++ b/tests/t_lpback.sh > @@ -26,7 +26,7 @@ trap "rm -f key.$$ log.$$ ; exit 1" 0 3 > > # Get list of supported ciphers from openvpn --show-ciphers output > CIPHERS=$(${top_builddir}/src/openvpn/openvpn --show-ciphers | \ > - sed -e '/The following/,/^$/d' -e s'/ .*//' -e '/^\s*$/d') > + sed -e '/The following/,/^$/d' -e s'/ .*//' -e '/^[[:space:]]*$/d') > I wanted to test this across our zoo of funny platforms - namely stuff like "OpenSolaris" or "AIX", and did not find the time yet. To my amazement, all platforms seem to support the [[:space:]] notation correctly, at least the output of openvpn --show-ciphers | sed -e '/^[[:space:]]*$/d' is correctly relieved of all empty lines. Even AIX. Thus: Acked-By: ge...@gr... will merge tomorrow 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 ge...@gr... 

Ping again - please review. Am 06.09.19 um 19:44 schrieb Matthias Andree: > From: Kyle Evans <ke...@Fr...> > > A test run with FreeBSD PR 229925 'Disallow escaping ordinary characters in regex(3)' > reveals one sed expression that uses the GNU-extension "\s". > Given that this is the only occurrence and it's a trivial fix, update it to be POSIX-compatible. > > Signed-off-by: Matthias Andree <mat...@gm...> > --- > tests/t_lpback.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/t_lpback.sh b/tests/t_lpback.sh > index 2052c626..bb8a1d51 100755 > --- a/tests/t_lpback.sh > +++ b/tests/t_lpback.sh > @@ -26,7 +26,7 @@ trap "rm -f key.$$ log.$$ ; exit 1" 0 3 > > # Get list of supported ciphers from openvpn --show-ciphers output > CIPHERS=$(${top_builddir}/src/openvpn/openvpn --show-ciphers | \ > - sed -e '/The following/,/^$/d' -e s'/ .*//' -e '/^\s*$/d') > + sed -e '/The following/,/^$/d' -e s'/ .*//' -e '/^[[:space:]]*$/d') > > # SK, 2014-06-04: currently the DES-EDE3-CFB1 implementation of OpenSSL is > # broken (see http://rt.openssl.org/Ticket/Display.html?id=2867), so exclude > -- > 2.21.0 > > > > _______________________________________________ > Openvpn-devel mailing list > Ope...@li... > https://lists.sourceforge.net/lists/listinfo/openvpn-devel 

Forgot copy this to the list -- sending again On Mon, Sep 23, 2019 at 6:19 AM Arne Schwabe <ar...@rf...> 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 <https://community.openvpn.net/openvpn/ticket/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 

Acked-by: Gert Doering <ge...@gr...> 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 <sel...@gm...> Acked-by: Gert Doering <ge...@gr...> Message-Id: <156...@gm...> URL: https://www.mail-archive.com/ope...@li.../msg18715.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, 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 <si...@ro...> 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 <le...@op...> Acked-by: Simon Rozman <si...@ro...> Message-Id: <156...@gm...> URL: https://www.mail-archive.com/ope...@li.../msg18854.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, 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 ge...@gr... 

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(&out, "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 

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(&out, "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 

Hi, On Sun, Jul 28, 2019 at 4:34 PM <sel...@gm...> wrote: > > From: Selva Nair <sel...@gm...> > > 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 <sel...@gm...> > --- > 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 

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 <https://community.openvpn.net/openvpn/ticket/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 

Hi, LGTM Best regards, Simon > -----Original Message----- > From: Lev Stipakov <lst...@gm...> > Sent: Monday, September 23, 2019 11:08 AM > To: ope...@li... > Subject: [Openvpn-devel] [PATCH] tapctl: add optional "hardware id" > parameter > > From: Lev Stipakov <le...@op...> > > 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 <le...@op...> 

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 

From: Lev Stipakov <le...@op...> 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 <le...@op...> --- 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, &pInterfaceList, TRUE); + DWORD dwResult = tap_list_interfaces(NULL, NULL, &pInterfaceList, 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, &bRebootRequired, &guidInterface); + dwResult = tap_create_interface(NULL, NULL, NULL, &bRebootRequired, &guidInterface); 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, &pInterfaceList, FALSE); + DWORD dwResult = tap_list_interfaces(NULL, NULL, &pInterfaceList, 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, &pInterfaceList, TRUE); + DWORD dwResult = tap_list_interfaces(NULL, NULL, &pInterfaceList, 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, &pInterfaceList, TRUE); + DWORD dwResult = tap_list_interfaces(NULL, NULL, &pInterfaceList, 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, &pInterfaceList, FALSE); + uiResult = tap_list_interfaces(NULL, NULL, &pInterfaceList, 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 <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 <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[]) szName = argv[++i]; } else + if (_tcsicmp(argv[i], TEXT("--hwid")) == 0) + { + szHwId = argv[++i]; + } + else { _ftprintf(stderr, TEXT("Unknown option \"%s\". Please, use \"tapctl help create\" to list supported options. Ignored.\n"), argv[i]); } @@ -190,6 +203,7 @@ _tmain(int argc, LPCTSTR argv[]) DWORD dwResult = tap_create_interface( NULL, TEXT("Virtual Ethernet"), + szHwId, &bRebootRequired, &guidInterface); if (dwResult != ERROR_SUCCESS) @@ -202,7 +216,7 @@ _tmain(int argc, LPCTSTR argv[]) { /* Get the list of all available interfaces. */ struct tap_interface_node *pInterfaceList = NULL; - dwResult = tap_list_interfaces(NULL, &pInterfaceList, TRUE); + dwResult = tap_list_interfaces(NULL, szHwId, &pInterfaceList, TRUE); if (dwResult != ERROR_SUCCESS) { _ftprintf(stderr, TEXT("Enumerating interfaces failed (error 0x%x).\n"), dwResult); @@ -257,9 +271,24 @@ create_delete_interface: } else if (_tcsicmp(argv[1], TEXT("list")) == 0) { + LPCTSTR szHwId = NULL; + + /* Parse options. */ + for (int i = 2; i < argc; i++) + { + if (_tcsicmp(argv[i], TEXT("--hwid")) == 0) + { + szHwId = argv[++i]; + } + else + { + _ftprintf(stderr, TEXT("Unknown option \"%s\". Please, use \"tapctl help list\" to list supported options. Ignored.\n"), argv[i]); + } + } + /* Output list of TUN/TAP interfaces. */ struct tap_interface_node *pInterfaceList = NULL; - DWORD dwResult = tap_list_interfaces(NULL, &pInterfaceList, FALSE); + DWORD dwResult = tap_list_interfaces(NULL, szHwId, &pInterfaceList, FALSE); if (dwResult != ERROR_SUCCESS) { _ftprintf(stderr, TEXT("Enumerating TUN/TAP interfaces failed (error 0x%x).\n"), dwResult); @@ -290,7 +319,7 @@ create_delete_interface: { /* The argument failed to covert to GUID. Treat it as the interface name. */ struct tap_interface_node *pInterfaceList = NULL; - DWORD dwResult = tap_list_interfaces(NULL, &pInterfaceList, FALSE); + DWORD dwResult = tap_list_interfaces(NULL, NULL, &pInterfaceList, FALSE); if (dwResult != ERROR_SUCCESS) { _ftprintf(stderr, TEXT("Enumerating TUN/TAP interfaces failed (error 0x%x).\n"), dwResult); diff --git a/src/tapctl/tap.c b/src/tapctl/tap.c index e75db35..464ce72 100644 --- a/src/tapctl/tap.c +++ b/src/tapctl/tap.c @@ -41,7 +41,7 @@ const static GUID GUID_DEVCLASS_NET = { 0x4d36e972L, 0xe325, 0x11ce, { 0xbf, 0xc1, 0x08, 0x00, 0x2b, 0xe1, 0x03, 0x18 } }; -const static TCHAR szzHardwareIDs[] = TEXT("root\\") TEXT(TAP_WIN_COMPONENT_ID) TEXT("\0"); +const static TCHAR szzDefaultHardwareIDs[] = TEXT("root\\") TEXT(TAP_WIN_COMPONENT_ID) TEXT("\0"); const static TCHAR szInterfaceRegKeyPathTemplate[] = TEXT("SYSTEM\\CurrentControlSet\\Control\\Network\\%") TEXT(PRIsLPOLESTR) TEXT("\\%") TEXT(PRIsLPOLESTR) TEXT("\\Connection"); #define INTERFACE_REGKEY_PATH_MAX (_countof(TEXT("SYSTEM\\CurrentControlSet\\Control\\Network\\")) - 1 + 38 + _countof(TEXT("\\")) - 1 + 38 + _countof(TEXT("\\Connection"))) @@ -448,6 +448,7 @@ DWORD tap_create_interface( _In_opt_ HWND hwndParent, _In_opt_ LPCTSTR szDeviceDescription, + _In_opt_ LPCTSTR szHwId, _Inout_ LPBOOL pbRebootRequired, _Out_ LPGUID pguidInterface) { @@ -459,6 +460,11 @@ tap_create_interface( return ERROR_BAD_ARGUMENTS; } + if (szHwId == NULL) + { + szHwId = szzDefaultHardwareIDs; + } + /* Create an empty device info set for network adapter device class. */ HDEVINFO hDevInfoList = SetupDiCreateDeviceInfoList(&GUID_DEVCLASS_NET, hwndParent); if (hDevInfoList == INVALID_HANDLE_VALUE) @@ -512,7 +518,7 @@ tap_create_interface( hDevInfoList, &devinfo_data, SPDRP_HARDWAREID, - (const BYTE *)szzHardwareIDs, sizeof(szzHardwareIDs))) + (const BYTE *)szHwId, (DWORD)((_tcslen(szHwId) + 1) * sizeof(TCHAR)))) { dwResult = GetLastError(); msg(M_NONFATAL, "%s: SetupDiSetDeviceRegistryProperty failed", __FUNCTION__); @@ -616,7 +622,7 @@ tap_create_interface( /* Search the list of hardware IDs. */ for (LPTSTR szHwdID = drvinfo_detail_data->HardwareID; szHwdID && szHwdID[0]; szHwdID += _tcslen(szHwdID) + 1) { - if (_tcsicmp(szHwdID, szzHardwareIDs) == 0) + if (_tcsicmp(szHwdID, szHwId) == 0) { /* Matching hardware ID found. Select the driver. */ if (!SetupDiSetSelectedDriver( @@ -643,7 +649,7 @@ tap_create_interface( if (dwlDriverVersion == 0) { dwResult = ERROR_NOT_FOUND; - msg(M_NONFATAL, "%s: No driver for device \"%" PRIsLPTSTR "\" installed.", __FUNCTION__, szzHardwareIDs); + msg(M_NONFATAL, "%s: No driver for device \"%" PRIsLPTSTR "\" installed.", __FUNCTION__, szHwId); goto cleanup_DriverInfoList; } @@ -953,6 +959,7 @@ cleanup_szInterfaceId: DWORD tap_list_interfaces( _In_opt_ HWND hwndParent, + _In_opt_ LPCTSTR szHwId, _Out_ struct tap_interface_node **ppInterface, _In_ BOOL bAll) { @@ -963,6 +970,11 @@ tap_list_interfaces( return ERROR_BAD_ARGUMENTS; } + if (szHwId == NULL) + { + szHwId = szzDefaultHardwareIDs; + } + /* Create a list of network devices. */ HDEVINFO hDevInfoList = SetupDiGetClassDevsEx( &GUID_DEVCLASS_NET, @@ -1034,7 +1046,7 @@ tap_list_interfaces( /* Check that hardware ID is REG_SZ/REG_MULTI_SZ, and optionally if it matches ours. */ if (dwDataType == REG_SZ) { - if (!bAll && _tcsicmp(szzDeviceHardwareIDs, szzHardwareIDs) != 0) + if (!bAll && _tcsicmp(szzDeviceHardwareIDs, szHwId) != 0) { /* This is not our device. Skip it. */ goto cleanup_szzDeviceHardwareIDs; @@ -1051,7 +1063,7 @@ tap_list_interfaces( /* This is not our device. Skip it. */ goto cleanup_szzDeviceHardwareIDs; } - else if (_tcsicmp(szHwdID, szzHardwareIDs) == 0) + else if (_tcsicmp(szHwdID, szHwId) == 0) { /* This is our device. */ break; diff --git a/src/tapctl/tap.h b/src/tapctl/tap.h index 2437d05..f74a39d 100644 --- a/src/tapctl/tap.h +++ b/src/tapctl/tap.h @@ -37,6 +37,10 @@ * @param szDeviceDescription A pointer to a NULL-terminated string that supplies the text * description of the device. This pointer is optional and can be NULL. * + * @param szHwId A pointer to a NULL-terminated string that supplies the hardware id + * of the device. This pointer is optional and can be NULL. Default value + * is root\tap0901. + * * @param pbRebootRequired A pointer to a BOOL flag. If the interface installation requires * a system restart, this flag is set to TRUE. Otherwise, the flag is * left unmodified. This allows the flag to be globally initialized to @@ -50,6 +54,7 @@ DWORD tap_create_interface( _In_opt_ HWND hwndParent, _In_opt_ LPCTSTR szDeviceDescription, + _In_opt_ LPCTSTR szHwId, _Inout_ LPBOOL pbRebootRequired, _Out_ LPGUID pguidInterface); @@ -116,6 +121,10 @@ struct tap_interface_node * and can be NULL. If a specific top-level window is not required, set * hwndParent to NULL. * + * @param szHwId A pointer to a NULL-terminated string that supplies the hardware id + * of the device. This pointer is optional and can be NULL. Default value + * is root\tap0901. + * * @param ppInterfaceList A pointer to the list to receive pointer to the first interface in * the list. After the list is no longer required, free it using * tap_free_interface_list(). @@ -128,6 +137,7 @@ struct tap_interface_node DWORD tap_list_interfaces( _In_opt_ HWND hwndParent, + _In_opt_ LPCTSTR szHwId, _Out_ struct tap_interface_node **ppInterfaceList, _In_ BOOL bAll); -- 2.7.4 

Am 18.09.19 um 14:01 schrieb Gert Doering: > Your patch has been applied to the release/2.4 branch. > > Sorry for the delay. Vacation, and too many distractions. > > Lightly tested on an OpenSSL 1.1, a mbedTLS build and an LibreSSL 2.7.2 > on OpenBSD 6.3 - with OpenSSL and mbedTLS, it builds and passes all > tests. > > With LibreSSL 2.7.2, it fails due to > > ./../../openvpn.git/src/openvpn/ssl_openssl.c:1873: undefined reference to `SSL_get1_supported_ciphers' > > which looks like this: > > #if (OPENSSL_VERSION_NUMBER < 0x1010000fL) > STACK_OF(SSL_CIPHER) *sk = SSL_get_ciphers(ssl); > #else > STACK_OF(SSL_CIPHER) *sk = SSL_get1_supported_ciphers(ssl); > #endif > > this is code which has been in release/2.4 for quite some time (part of > the TLS 1.3 support, commit e8467c864, "--show-tls" enhancements) - so > if it doesn't break for you, I assume that the call was added to more > recent LibreSSL versions. I was testing against LibreSSL 2.9.2, the oldest for FreeBSD, and this particular call is listed in the OpenBSD 6.5 changelog here: https://www.openbsd.org/plus65.html "Provided SSL_get_client_ciphers() and SSL_get1_supported_ciphers() (part of the OpenSSL 1.1 API)." But I haven't figured out when or where this was added to LibreSSL releases. It really looks to me that there isn't a strategy for LibreSSL, but I'll not backport things to old LibreSSL version, the answer should be "upgrade or else leave it to your packager/distributor". 

Hi, Reviving this thread/patch as now users are running into this padding issue (trac 1216 <https://community.openvpn.net/openvpn/ticket/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