Hi, On Wed, Mar 15, 2023 at 04:04:20PM +0100, Arne Schwabe wrote: > The HMAC leaks are just forgotten frees/deinitialisations. tls_wrap_control > will sometimes return the original buffer (non tls-crypt) and sometimes > tls_wrap.work, handling this buffer lifetime is a bit more complicated. > Instead > of further complicating that code just give our work buffer the same lifetime > as the other one inside tls_wrap.work as that is also more consistent.
This test passes all I've thrown at it, except stare-at-code...
Maybe I am misreading it, but want to make sure.
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index 124ac76bd..fa2681dc7 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -3483,6 +3483,7 @@ do_init_frame_tls(struct context *c)
> frame_print(&c->c2.tls_auth_standalone->frame, D_MTU_INFO,
> "TLS-Auth MTU parms");
> c->c2.tls_auth_standalone->tls_wrap.work =
> alloc_buf_gc(BUF_SIZE(&c->c2.frame), &c->c2.gc);
> + c->c2.tls_auth_standalone->workbuf =
> alloc_buf_gc(BUF_SIZE(&c->c2.frame), &c->c2.gc);
> }
At this place, we have a tls_auth_standalone() (!= NULL) already, which
got initialized by tls_auth_standalone_init().
We allocate tas->workbuf here.
> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
> index 78cec90a1..f2331f712 100644
> --- a/src/openvpn/ssl.c
> +++ b/src/openvpn/ssl.c
> @@ -1358,9 +1358,21 @@ tls_auth_standalone_init(struct tls_options
> *tls_options,
> packet_id_init(&tas->tls_wrap.opt.packet_id, tls_options->replay_window,
> tls_options->replay_time, "TAS", 0);
>
> + tas->workbuf = alloc_buf_gc(tas->frame.buf.payload_size, gc);
> return tas;
> }
This is tls_auth_standalone_init(), which has already allocated
a tas->workbuf.
So my reading is "we allocate this twice" - which is not a super big
problem because it's both in the gc_arena and will be freed on session
exit (mmmh, which session, anyway?) - but it's still "too many bufs",
plus "readers of that code will get confused".
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany [email protected]
signature.asc
Description: PGP signature
_______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
