[Openvpn-devel] [PATCH applied] Re: Fix tls_ctx_client/server_new leaving error on OpenSSL error stack

2020-04-22 Thread Gert Doering
Acked-by: Gert Doering 

"Explanation and Code make sense, Debian testing confirmed it fixes
the problem observed" (which was a user error in the end, but led to an 
unexpected error in openvpn).

Basic client test run with openssl 1.1.1 on Linux/Gentoo.

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

commit 75aa88af774abaa168bf72e43e1dbb57be14c044 (master)
commit 125654bfa6f99a251b581522182e85748dd8043a (release/2.4)
Author: Arne Schwabe
Date:   Tue Apr 21 12:11:22 2020 +0200

 Fix tls_ctx_client/server_new leaving error on OpenSSL error stack

 Acked-by: Gert Doering 
 Message-Id: <20200421101122.24284-1-a...@rfc2549.org>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19802.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 v9] convert *_inline attributes to bool

2020-04-22 Thread Antonio Quartulli
Carrying around the INLINE_TAG is not really efficient,
because it requires a strcmp() to be performed every
time we want to understand if the data is stored inline
or not.

Convert all the *_inline attributes to bool to make the
logic easier and checks more efficient.

Signed-off-by: Antonio Quartulli 
---

Changes from v8:
- rebased on top of latest master
- changed auth-token secret file inlining logic too (not present before)
- adapted test_tls_crypt.c
- adapted test_auth_token.c


Changes from v7:
- rebased on top of latest master (68e0b9db)


Changes from v6:
- rebased on top of latest master


Changes from v5:
- fix function invocation alignment in 
options.c:options_postprocess_filechecks()
- fix typ0 in function invocation in options.c:options_postprocess_filechecks()
- fix doxygen comment for function tls_ctx_reload_crl() in ssl.c


Changes from v4:
- remove newline accidentally added in v4


Changes from v3:
- some code style adjustment in options.c:check_inline_file()
- move print_if_inline() from misc.c to crypto.c and rename it to
  print_key_filename()
- make comment of check_file_access_inline() and
  check_file_access_chroot_inline() doxygen compliant
- remove *is_inline argument in check_inline_file() and use its
  return value instead
- move declarations of is_inline to narrower scope in options.c
- move return type of plugin_option_list_add() to its own line


Changes from v2:
- fix indentation in ssl_openssl.c
- do not attempt to push inline'd options
- do not attempt to parse inline'd plugin
- introduce check_file_access_inline() and check_file_access_chroot_inline()
- introduce OPT_P_INLINE to specify when an option is allowed to
  be inline. Options not having this permission will fail to be
  parsed if is_inline is true


Changes from v1:
- remove the INLINE_TAG from the options parsing logic at all. Now a
  boolean variable is passed around
- add print_if_inline() helper function (to misc.c/h) to make sure we
  never print the inline data, but only the INLINE tag. Such function
  checks also for NULL pointers
- make sure print_if_inline() is always used when printing possibly
  inline data
- remove the INLINE_TAG from the options parsing logic at all. Now a
  boolean variable is passed around
- fix alignment error in comment
- remove CHKACC_INLINE from check_file_access() logic: this function
  is now not invoked at all in case of inline data



 src/openvpn/auth_token.c   |   2 +-
 src/openvpn/auth_token.h   |   2 +-
 src/openvpn/crypto.c   |  47 +--
 src/openvpn/crypto.h   |  25 +-
 src/openvpn/init.c |   9 +-
 src/openvpn/misc.c |   6 +-
 src/openvpn/misc.h |   3 +-
 src/openvpn/options.c  | 329 +++--
 src/openvpn/options.h  |  32 +-
 src/openvpn/plugin.c   |   5 +-
 src/openvpn/plugin.h   |   3 +-
 src/openvpn/push.c |   2 +-
 src/openvpn/push.h |   3 +-
 src/openvpn/ssl.c  |   4 +-
 src/openvpn/ssl_backend.h  |  55 ++--
 src/openvpn/ssl_common.h   |   2 +-
 src/openvpn/ssl_mbedtls.c  |  61 ++--
 src/openvpn/ssl_openssl.c  |  85 +++---
 src/openvpn/tls_crypt.c|   8 +-
 src/openvpn/tls_crypt.h|  43 +--
 tests/unit_tests/openvpn/test_auth_token.c |   6 +-
 tests/unit_tests/openvpn/test_tls_crypt.c  |   3 +-
 22 files changed, 402 insertions(+), 333 deletions(-)

diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
index 6275299d..d9212ee7 100644
--- a/src/openvpn/auth_token.c
+++ b/src/openvpn/auth_token.c
@@ -135,7 +135,7 @@ auth_token_write_server_key_file(const char *filename)
 
 void
 auth_token_init_secret(struct key_ctx *key_ctx, const char *key_file,
-   const char *key_inline)
+   bool key_inline)
 {
 struct key_type kt = auth_token_kt();
 
diff --git a/src/openvpn/auth_token.h b/src/openvpn/auth_token.h
index 4b014d44..6f34b760 100644
--- a/src/openvpn/auth_token.h
+++ b/src/openvpn/auth_token.h
@@ -74,7 +74,7 @@ verify_auth_token(struct user_pass *up, struct tls_multi 
*multi,
  */
 void
 auth_token_init_secret(struct key_ctx *key_ctx, const char *key_file,
-   const char *key_inline);
+   bool key_inline);
 
 
 /**
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 1678cba8..672aa14a 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -1184,27 +1184,38 @@ test_crypto(struct crypto_options *co, struct frame 
*frame)
 gc_free();
 }
 
+const char *
+print_key_filename(const char *str, bool is_inline)
+{
+if (is_inline)
+{
+return INLINE_FILE_TAG;
+}
+
+return np(str);
+}
+
 void
 crypto_read_openvpn_key(const struct 

Re: [Openvpn-devel] [Openvpn-users] new openssl = new OpenVPN release ?

2020-04-22 Thread Jan Just Keijser

Hi Arne,

On 22/04/20 10:13, Arne Schwabe wrote:

SSL_check_chain() function".

Which we don't, I just grepped through our source tree.

So, unless I misunderstand something about OpenSSL intricacies, I think
we're safe - no new installers needed, and OpenVPN is not in risk.



the advisory applies only to application that use the SSL_check_chain()
function as part of a TLS 1.3 handshake. AFAIK, iIn OpenVPN 2.4 we don't
do anything with TLS 1.3 just yet, so this security advisory does not
apply to OpenVPN. Also note that this bug appears only in OpenSSL 1.1.1
[d-f] , so anything older is fine as well.

Hu? OpenVPN 2.4 supports TLS 1.3 just fine. We have support for it in
tls-version-min and also tls-ciphersuites which is TLS 1.3 specific.


what I meant was that OpenVPN 2.4 does not do any *specific* with any of 
the new features of TLS 1.3, like the new psk callbacks etc. If the 
control session is negotiated using TLS 1.3 then sure, OpenVPN will use 
that, but other that OpenVPN does not make use of any of the new 
features or crypto algorithms that come with OpenSSL 1.1.1 or TLS 1.3 
(chacha20 anyone ;) ? )


cheers,

JJK



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


Re: [Openvpn-devel] [Openvpn-users] new openssl = new OpenVPN release ?

2020-04-22 Thread Gert Doering
Hi,

On Wed, Apr 22, 2020 at 10:21:52AM +0200, Christian Hesse wrote:
> > So, speaking to myself again :-) - I've looked at the advisory, and
> > it talks about "Server or client applications that call the 
> > SSL_check_chain() function".
> 
> Are you sure that openvpn code does not call any openssl function that calls
> SSL_check_chain() then? Did not check, but I guess that's possible.

This is one of the OpenSSL intricacies I wouldn't know.

OTOH, I would expect the advisory then to be worded as "a TLS 1.3 
handshake might crash" not "... applications that call ... 
SSL_check_chain()", which sounds very specific to me.

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] [Openvpn-users] new openssl = new OpenVPN release ?

2020-04-22 Thread Christian Hesse
Gert Doering  on Tue, 2020/04/21 20:59:
> Hi,
> 
> On Tue, Apr 21, 2020 at 08:37:35PM +0200, Gert Doering wrote:
> > On Tue, Apr 21, 2020 at 02:15:43PM -0400, mike tancsa wrote:  
> > >     Will the sec issue with OpenSSL force a new release of OpenVPN ?
> > > 
> > > https://www.openssl.org/news/secadv/20200421.txt  
> 
> So, speaking to myself again :-) - I've looked at the advisory, and
> it talks about "Server or client applications that call the 
> SSL_check_chain() function".
> 
> Which we don't, I just grepped through our source tree.
> 
> So, unless I misunderstand something about OpenSSL intricacies, I think
> we're safe - no new installers needed, and OpenVPN is not in risk.

Are you sure that openvpn code does not call any openssl function that calls
SSL_check_chain() then? Did not check, but I guess that's possible.
-- 
main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}


pgpnhhPdodxsP.pgp
Description: OpenPGP digital signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [Openvpn-users] new openssl = new OpenVPN release ?

2020-04-22 Thread Arne Schwabe
>> SSL_check_chain() function".
>>
>> Which we don't, I just grepped through our source tree.
>>
>> So, unless I misunderstand something about OpenSSL intricacies, I think
>> we're safe - no new installers needed, and OpenVPN is not in risk.
>>
>>
> the advisory applies only to application that use the SSL_check_chain()
> function as part of a TLS 1.3 handshake. AFAIK, iIn OpenVPN 2.4 we don't
> do anything with TLS 1.3 just yet, so this security advisory does not
> apply to OpenVPN. Also note that this bug appears only in OpenSSL 1.1.1
> [d-f] , so anything older is fine as well.
Hu? OpenVPN 2.4 supports TLS 1.3 just fine. We have support for it in
tls-version-min and also tls-ciphersuites which is TLS 1.3 specific.

Arne



signature.asc
Description: OpenPGP digital signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [Openvpn-users] new openssl = new OpenVPN release ?

2020-04-22 Thread Jan Just Keijser

Hi Gert,

On 21/04/20 20:59, Gert Doering wrote:

Hi,

On Tue, Apr 21, 2020 at 08:37:35PM +0200, Gert Doering wrote:

On Tue, Apr 21, 2020 at 02:15:43PM -0400, mike tancsa wrote:

     Will the sec issue with OpenSSL force a new release of OpenVPN ?

https://www.openssl.org/news/secadv/20200421.txt

So, speaking to myself again :-) - I've looked at the advisory, and
it talks about "Server or client applications that call the
SSL_check_chain() function".

Which we don't, I just grepped through our source tree.

So, unless I misunderstand something about OpenSSL intricacies, I think
we're safe - no new installers needed, and OpenVPN is not in risk.


the advisory applies only to application that use the SSL_check_chain() 
function as part of a TLS 1.3 handshake. AFAIK, iIn OpenVPN 2.4 we don't 
do anything with TLS 1.3 just yet, so this security advisory does not 
apply to OpenVPN. Also note that this bug appears only in OpenSSL 1.1.1 
[d-f] , so anything older is fine as well.


cheers,

JJK



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