| Message ID | 20220628094144.17471-1-a@unstable.cc |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel] tls-crypt-v2: bail out if the client key is too small | expand |
Am 28.06.22 um 11:41 schrieb Antonio Quartulli: > The tls-crypt-v2 key should be at least 2 bytes long in order to read > the actual length. > > Bail out if the key is too short. > Failing to do so will lead to a read out of the buffer boundary. Actually not. We read from BEND(), so this is defined for TCP since the minimum length there is 3 bytes (pkt len + opcode). For UDP we might read past the beginning of the packet but since they are buffers coming from the packet stack we have the headroom/tailroom, so might read some random data (but not out of bound!). So we copy some more or less random number into net_len/wkc_len but without actually reading from undefined memory. The next line will then almost definitively fail if (!buf_advance(&wrapped_client_key, BLEN(&wrapped_client_key) - wkc_len)) Since BLEN(wrapped_client_key) is 0 or 1 or wkc_len has to be 0 or 1 to not fail this check and then in turn tls_crypt_v2_unwrap_client_key will fail at if (BLEN(&wrapped_client_key) < sizeof(net_len)) > While at it improve the error message a bit. > Acked-BY: Arne Schwabe <arne@rfc2549.org>
I have taken the "why is this not a problem?" text from Arne's ACK and
included it into the commit message.
Your patch has been applied to the master and release/2.5 branch.
commit 462339a45089ef655faf02232d7d792def9b8afb (master)
commit ce24bec7e2518d4ea7aa931021454d1191f4906b (release/2.5)
Author: Antonio Quartulli
Date: Tue Jun 28 11:41:44 2022 +0200
tls-crypt-v2: bail out if the client key is too small
Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20220628094144.17471-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24580.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 88730a99..2fc79111 100644 --- a/src/openvpn/tls_crypt.c +++ b/src/openvpn/tls_crypt.c @@ -557,7 +557,8 @@ tls_crypt_v2_extract_client_key(struct buffer *buf, if (BLEN(&wrapped_client_key) < sizeof(net_len)) { - msg(D_TLS_ERRORS, "failed to read length"); + msg(D_TLS_ERRORS, "Can not read tls-crypt-v2 client key length"); + return false; } memcpy(&net_len, BEND(&wrapped_client_key) - sizeof(net_len), sizeof(net_len));
The tls-crypt-v2 key should be at least 2 bytes long in order to read the actual length. Bail out if the key is too short. Failing to do so will lead to a read out of the buffer boundary. While at it improve the error message a bit. Signed-off-by: Antonio Quartulli <a@unstable.cc> --- src/openvpn/tls_crypt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)