Message ID | 20211019183127.614175-22-arne@rfc2549.org |
---|---|
State | Not Applicable |
Headers | show |
Series | OpenSSL 3.0 improvements for OpenVPN | expand |
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 <<a href="mailto:arne@rfc2549.org">arne@rfc2549.org</a>> 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's already 8K with base64 overhead.</div><div><br></div><div>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. </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>
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 --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;
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(-)