Re: [Openvpn-devel] [PATCH v5] Introduce dynamic tls-crypt for secure soft_reset/session renegotiation

2023-01-11 Thread Frank Lichtenheld
On Mon, Jan 09, 2023 at 05:38:10PM +0100, Arne Schwabe wrote:
> Currently we have only one slot for renegotiation of the session/keys.
> If a replayed/faked packet is inserted by a malicous attacker, the
> legimate peer cannot renegotiate anymore.
> 
> This commit introduces dynamic tls-crypt. When both peer support this

"peers"

> feature, both peer create a dynamic tls-crypt key using TLS EKM (export key
> material) and will enforce using that key and tls-crypt for all
> renegotiations. This also add an additional protection layer for
> renegotiations to be taken over by an illegimate client, binding the
> renegotiations tightly to the original session. Especially when 2FA, webauth
> or similar authentication is used, many third party setup ignore the need
> to secure renegotiation with an auth-token.
> 

Acked-By: Frank Lichtenheld 

Stared at code, read documentation, stared more at code. Looks like it
does what it should. Ran master+patch t_client tests against a master+patch 
server.
Log messages look sensible. Also tried running a connection with --reneg-sec 15
just to prove that renegotiations still work.

I can't prove/judge whether it actually uses a different key and whether the 
crypto
claims from the commit message are actually true.

Regards,
-- 
  Frank Lichtenheld


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


[Openvpn-devel] [PATCH v5] Introduce dynamic tls-crypt for secure soft_reset/session renegotiation

2023-01-09 Thread Arne Schwabe
Currently we have only one slot for renegotiation of the session/keys.
If a replayed/faked packet is inserted by a malicous attacker, the
legimate peer cannot renegotiate anymore.

This commit introduces dynamic tls-crypt. When both peer support this
feature, both peer create a dynamic tls-crypt key using TLS EKM (export key
material) and will enforce using that key and tls-crypt for all
renegotiations. This also add an additional protection layer for
renegotiations to be taken over by an illegimate client, binding the
renegotiations tightly to the original session. Especially when 2FA, webauth
or similar authentication is used, many third party setup ignore the need
to secure renegotiation with an auth-token.

Since one of tls-crypt/tls-crypt-v2 purposes is to provide poor man's post
quantum crypto guarantees, we have to ensure that the dynamic key tls-crypt
key that replace the original tls-crypt key is as strong as the orginal key
to avoid problems if there is a weak RNG or TLS EKM produces weak keys. We
ensure this but XORing the original key with the key from TLS EKM. If
tls-crypt/tls-cryptv2 is not active, we use just the key generated by
TLS EKM. We also do not use hashing or anything else on the original key
before XOR to avoid any potential of a structure in the key or something
else that might weaken post-quantum use cases.

OpenVPN 2.x reserves the TM_ACTIVE session for renegotiations. When a
SOFT_RESET_V1 packet is received, the active TLS session is moved from
KS_PRIMARY to KS_SECONDARY. Here an attacker could theorectically send a
faked/replayed SOFT_RESET_V1 and first packet containing the TLS client
hello. If this happens, the session is blocked until the TLS
renegotiation attempt times out, blocking the legimitate client.

Using a dynamic tls-crypt key here blocks any SOFT_RESET_V1 (and following
packets) as replay and fake packets will not have a matching
authentication/encryption and will be discarded.

HARD_RESET packets that are from a reconnecting peer are instead put in the
TM_UNTRUSTED/KS_PRIMARY slot until they are sufficiently verified, so the
dyanmic tls-crypt key is not used here. Replay/fake packets also do not
block the legimitate client.

This commit delays the purging of the original tls-crypt key data from
directly after passing it to crypto library to tls_wrap_free. We do this
to allow us mixing the new exported key with the original key. Since we
need to be able to generate the secure renegotiation key, deleting
the key after generating a secure renegotiation key is not an option.
Even when the client does not support secure renegotiation, deleting the
key is not a good option since when the reconnecting client or (especially
in p2p mode with float) another client does the reconnect, we might need
to generate a secure renegotiation key. Delaying the deletion of the key
has also little effect as the key is still present in the OpenSSL/mbed TLS
structures in the tls_wrap structure, so only the number of times  the keys
is in memory would be reduced.

Patch v2: fix spellings of reneg and renegotiations.
Patch v3: expand comment to original_tlscrypt_keydata and commit message,
  add Changes.rst
Patch v4: improve commit message, Changes.rst
Patch v5: fix spelling/grammar mistakes. Add more comments.

Signed-off-by: Arne Schwabe 
---
 Changes.rst   |  6 ++
 src/openvpn/auth_token.h  |  2 +-
 src/openvpn/crypto.c  |  7 +-
 src/openvpn/crypto.h  | 16 +++-
 src/openvpn/init.c|  8 +-
 src/openvpn/multi.c   |  4 +
 src/openvpn/openvpn.h |  3 +
 src/openvpn/options.c |  4 +
 src/openvpn/push.c|  5 ++
 src/openvpn/ssl.c | 25 --
 src/openvpn/ssl.h |  5 ++
 src/openvpn/ssl_backend.h |  1 +
 src/openvpn/ssl_common.h  | 13 
 src/openvpn/ssl_ncp.c |  4 +
 src/openvpn/ssl_pkt.c |  2 +-
 src/openvpn/ssl_pkt.h | 26 +++
 src/openvpn/tls_crypt.c   | 93 ---
 src/openvpn/tls_crypt.h   | 22 +-
 tests/unit_tests/openvpn/test_pkt.c   | 17 -
 tests/unit_tests/openvpn/test_tls_crypt.c | 85 +
 20 files changed, 319 insertions(+), 29 deletions(-)

diff --git a/Changes.rst b/Changes.rst
index 47933ae09..d05f7624a 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -118,6 +118,12 @@ Tun MTU can be pushed
 directive ``--tun-mtu-max`` has been introduced to increase the maximum
 pushable MTU size (defaults to 1600).
 
+Secure renegotiation
+When both peers are OpenVPN 2.6.0+, OpenVPN will use secure renegotiation
+using a dynamically created tls-crypt key. This ensure that only the
+previously authenticated peer can do trigger renegotiation and complete
+