diff mbox series

[Openvpn-devel,v3,21/21] Always use 8192 bytes for ERR_BUF_SIZE

Message ID 20211019183127.614175-22-arne@rfc2549.org
State Not Applicable
Headers
Series OpenSSL 3.0 improvements for OpenVPN |

Commit Message

Arne Schwabe Oct. 19, 2021, 7:31 a.m. UTC 
  The signature messages required by external key managed also break
the 1280 limit. To also avoid this surprise of different behaviour
with PKCS11 enabled/disable, always use the larger size.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/error.h | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

Selva Nair Oct. 20, 2021, 11:08 a.m. UTC | #1 
Hi,

On Tue, Oct 19, 2021 at 2:32 PM Arne Schwabe <arne@rfc2549.org> wrote:

> The signature messages required by external key managed also break
> the 1280 limit. To also avoid this surprise of different behaviour
> with PKCS11 enabled/disable, always use the larger size.
>

This may be enough in most cases, but to be safer, shall we increase it to
10240? I have seen up to 6 K handshake messages when undigested messages
are to be passed to the management interface and that's already 8K with
base64 overhead.

That said, I think we need a better solution. As it stands it's a bit silly
that we keep allocating required buffers dynamically at the origination
point, a number of places in manage.c, all the way up to x_msg_va() in
error.c where it gets silently truncated to a hard-coded size.

Can we use the return value of vsnprintf() in x_msg_va() to determine the
size of the buffer required? Not sure whether that is reliable on all
platforms. Only the size of m1 has to be determined as m2 has a
more-or-less predictable overhead on top of that.

Selva
<div dir="ltr"><div>Hi,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Oct 19, 2021 at 2:32 PM Arne Schwabe &lt;<a href="mailto:arne@rfc2549.org">arne@rfc2549.org</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The signature messages required by external key managed also break<br>
the 1280 limit. To also avoid this surprise of different behaviour<br>
with PKCS11 enabled/disable, always use the larger size.<br></blockquote><div><br></div><div>This may be enough in most cases, but to be safer, shall we increase it to 10240? I have seen up to 6 K handshake messages when undigested messages are to be passed to the management interface and that&#39;s already 8K with base64 overhead.</div><div><br></div><div>That said, I think we need a better solution. As it stands it&#39;s a bit silly that we keep allocating required buffers dynamically at the origination point, a number of places in manage.c, all the way up to x_msg_va() in error.c where it gets silently truncated to a hard-coded size. </div><div><br></div><div>Can we use the return value of vsnprintf() in x_msg_va() to determine the size of the buffer required? Not sure whether that is reliable on all platforms. Only the size of m1 has to be determined as m2 has a more-or-less predictable overhead on top of that.</div><div><br></div><div>Selva</div></div></div>
Gert Doering Jan. 21, 2022, 2:32 a.m. UTC | #2 
Hi,

On Tue, Oct 19, 2021 at 08:31:27PM +0200, Arne Schwabe wrote:
> The signature messages required by external key managed also break
> the 1280 limit. To also avoid this surprise of different behaviour
> with PKCS11 enabled/disable, always use the larger size.

JFTR, this patch has been obsoleted by Selva's eeb019acee57e commit.

This is now "large" for PKCS11 *or* ENABLE_MANAGEMENT, so for the
original purpose, fixed.

All original comments still apply, like, "make this less silly"

gert
diff mbox series

Patch

diff --git a/src/openvpn/error.h b/src/openvpn/error.h
index 533354b3c..c36a82659 100644
--- a/src/openvpn/error.h
+++ b/src/openvpn/error.h
@@ -36,12 +36,8 @@ 
 #endif
 
 /* #define ABORT_ON_ERROR */
-
-#ifdef ENABLE_PKCS11
 #define ERR_BUF_SIZE 8192
-#else
-#define ERR_BUF_SIZE 1280
-#endif
+
 
 struct gc_arena;