diff mbox series

[Openvpn-devel,v3] Do not underestimate number of encrypted/decrypted AEAD blocks

Message ID 20251112112133.1325-1-gert@greenie.muc.de
State New
Headers show
Series [Openvpn-devel,v3] Do not underestimate number of encrypted/decrypted AEAD blocks | expand

Commit Message

Gert Doering Nov. 12, 2025, 11:21 a.m. UTC 
From: Arne Schwabe <arne@rfc2549.org>

Even though the current code typically counts all the encrypted/decrypted
traffic, this is only the case because of the specific implementation
of OpenSSL at the moment.

Instead of counting the length returned by one call only, count all
the encrypted/decrypted bytes.

Other implementations that use AES-GCM (like IPSec, MacSEC, TLS 1.2)
(currently) do not honour these usage limits at all. This is the reason that
I also currently do not consider the lack/improper validation in our code
to be a security vulnerability. In the current state implementations/protocol
that lack this feature altogether are not considered vulnerable.

Reported by: <stephan@srlabs.de>

Change-Id: I429d768fb33ef2c58484287d4091440ad8599053
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1358
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1358
This mail reflects revision 3 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <gert@greenie.muc.de>

Comments

Gert Doering Nov. 12, 2025, 1:02 p.m. UTC | #1 
I agree with SR-Labs that this is a bug - thanks for fixing.

I also do agree with Arne that this is not CVE worthy, as this is a new
safety margin added to 2.7, which is only really relevant if you are
talking about "sustained utilization of >> 10 Gbit/s to the same peer,
exceeding sane AEAD block limits, and not going into tls-renegotiation due
to any other trigger (reneg-sec etc)" - so the chance to actually have
a setup that would hit this is very close to zero, and then you're no 
worse than with 2.6 - or any of the other TLS/AEAD implementations.

Stared at code ("make sense"), fed into the t_server testbed ("just to
be sure").  BB and GHA agree that it compiles and tests fine everywhere.

Your patch has been applied to the master branch.

commit 5e6d478fb6246465fb81060e60348bb0061a94fa
Author: Arne Schwabe
Date:   Wed Nov 12 12:21:27 2025 +0100

     Do not underestimate number of encrypted/decrypted AEAD blocks

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1358
     Message-Id: <20251112112133.1325-1-gert@greenie.muc.de>
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 307d1ee..8049b3a 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -152,15 +152,15 @@ 
     ASSERT(cipher_ctx_update(ctx->cipher, BEND(&work), &outlen, BPTR(buf), BLEN(buf)));
     ASSERT(buf_inc_len(&work, outlen));
 
-    /* update number of plaintext blocks encrypted. Use the (x + (n-1))/n trick
-     * to round up the result to the number of blocks used */
-    const int blocksize = AEAD_LIMIT_BLOCKSIZE;
-    opt->key_ctx_bi.encrypt.plaintext_blocks += (outlen + (blocksize - 1)) / blocksize;
-
     /* Flush the encryption buffer */
     ASSERT(cipher_ctx_final(ctx->cipher, BEND(&work), &outlen));
     ASSERT(buf_inc_len(&work, outlen));
 
+    /* update number of plaintext blocks encrypted. Use the (x + (n-1))/n trick
+     * to round up the result to the number of blocks used */
+    const int blocksize = AEAD_LIMIT_BLOCKSIZE;
+    opt->key_ctx_bi.encrypt.plaintext_blocks += (BLEN(&work) + (blocksize - 1)) / blocksize;
+
     /* if the tag is at end the end, allocate it now */
     if (use_epoch_data_format)
     {
@@ -580,11 +580,10 @@ 
         goto error_exit;
     }
 
-
     /* update number of plaintext blocks decrypted. Use the (x + (n-1))/n trick
      * to round up the result to the number of blocks used. */
     const int blocksize = AEAD_LIMIT_BLOCKSIZE;
-    opt->key_ctx_bi.decrypt.plaintext_blocks += (outlen + (blocksize - 1)) / blocksize;
+    opt->key_ctx_bi.decrypt.plaintext_blocks += (BLEN(&work) + (blocksize - 1)) / blocksize;
 
     *buf = work;