| Message ID | 20230309120031.3780130-1-arne@rfc2549.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel] Ensure n = 2 is set in key2 structer in tls_crypt_v2_unwrap_client_key | expand |
Hi, On 09/03/2023 13:00, Arne Schwabe wrote: > The ASSERT in xor_key2 assumes that all methods that load a key2 struct > correctly set n=2. However, tls_crypt_v2_unwrap_client_key loads a key > without setting n = 2, trigerring the assert. trigerring -> triggering > > Closes and reported in https://github.com/OpenVPN/openvpn/issues/272 > > Change-Id: Iaeb163d83b95818e0b26faf9d25e7737dc8ecb23 > Signed-off-by: Arne Schwabe <arne@rfc2549.org> I can easily reproduce the issue ad verify that indeed the patch fixes it. By checking the code I can see that we set 'n' whenever we initialize the keys member. However, this was not happening in tls_crypt_v2_unwrap_client_key() thus leading to the assert being triggered. Acked-by: Antonio Quartulli <a@unstable.cc> > --- > src/openvpn/tls_crypt.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c > index 8882d5de0..4f22f8af7 100644 > --- a/src/openvpn/tls_crypt.c > +++ b/src/openvpn/tls_crypt.c > @@ -533,6 +533,7 @@ tls_crypt_v2_unwrap_client_key(struct key2 *client_key, struct buffer *metadata, > } > memcpy(&client_key->keys, BPTR(&plaintext), sizeof(client_key->keys)); > ASSERT(buf_advance(&plaintext, sizeof(client_key->keys))); > + client_key->n = 2; > > if (!buf_copy(metadata, &plaintext)) > {
Trivial enough :-) - wording fixes applied ("structer", "trigerring").
Interesting that Antonio could reproduce this while it "just works"
in my test beds.
Your patch has been applied to the master and release/2.6 branch.
commit 85832307fcb41c229ccb7ba83984726757eb32f7 (master)
commit ae6068842854278de70264218516b0e4fcdfc6d9 (release/2.6)
Author: Arne Schwabe
Date: Thu Mar 9 13:00:31 2023 +0100
Ensure n = 2 is set in key2 struct in tls_crypt_v2_unwrap_client_key
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20230309120031.3780130-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26363.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c index 8882d5de0..4f22f8af7 100644 --- a/src/openvpn/tls_crypt.c +++ b/src/openvpn/tls_crypt.c @@ -533,6 +533,7 @@ tls_crypt_v2_unwrap_client_key(struct key2 *client_key, struct buffer *metadata, } memcpy(&client_key->keys, BPTR(&plaintext), sizeof(client_key->keys)); ASSERT(buf_advance(&plaintext, sizeof(client_key->keys))); + client_key->n = 2; if (!buf_copy(metadata, &plaintext)) {
The ASSERT in xor_key2 assumes that all methods that load a key2 struct correctly set n=2. However, tls_crypt_v2_unwrap_client_key loads a key without setting n = 2, trigerring the assert. Closes and reported in https://github.com/OpenVPN/openvpn/issues/272 Change-Id: Iaeb163d83b95818e0b26faf9d25e7737dc8ecb23 Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/tls_crypt.c | 1 + 1 file changed, 1 insertion(+)