@@ -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);
}
}
@@ -3881,6 +3882,8 @@ do_close_tls(struct context *c)
md_ctx_cleanup(c->c2.pulled_options_state);
md_ctx_free(c->c2.pulled_options_state);
}
+
+ tls_auth_standalone_free(c->c2.tls_auth_standalone);
}
/*
@@ -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;
}
+void
+tls_auth_standalone_free(struct tls_auth_standalone *tas)
+{
+ if (!tas)
+ {
+ return;
+ }
+
+ packet_id_free(&tas->tls_wrap.opt.packet_id);
+}
+
/*
* Set local and remote option compatibility strings.
* Used to verify compatibility of local and remote option
@@ -180,6 +180,12 @@ void tls_multi_init_finalize(struct tls_multi *multi, int tls_mtu);
struct tls_auth_standalone *tls_auth_standalone_init(struct tls_options *tls_options,
struct gc_arena *gc);
+/**
+ * Frees a standalone tls-auth verification object.
+ * @param tas the object to free. May be NULL.
+ */
+void tls_auth_standalone_free(struct tls_auth_standalone *tas);
+
/*
* Setups the control channel frame size parameters from the data channel
* parameters
@@ -434,7 +434,10 @@ tls_reset_standalone(struct tls_wrap_ctx *ctx,
uint8_t header,
bool request_resend_wkc)
{
- struct buffer buf = alloc_buf(tas->frame.buf.payload_size);
+ /* Copy buffer here to point at the same data but allow tls_wrap_control
+ * to potentially change buf to point to another buffer without
+ * modifying the buffer in tas */
+ struct buffer buf = tas->workbuf;
ASSERT(buf_init(&buf, tas->frame.buf.headroom));
/* Reliable ACK structure */
@@ -461,7 +464,8 @@ tls_reset_standalone(struct tls_wrap_ctx *ctx,
buf_write_u16(&buf, EARLY_NEG_FLAG_RESEND_WKC);
}
- /* Add tls-auth/tls-crypt wrapping, this might replace buf */
+ /* Add tls-auth/tls-crypt wrapping, this might replace buf with
+ * ctx->work */
tls_wrap_control(ctx, header, &buf, own_sid);
return buf;
@@ -77,6 +77,7 @@
struct tls_auth_standalone
{
struct tls_wrap_ctx tls_wrap;
+ struct buffer workbuf;
struct frame frame;
};
@@ -220,7 +221,6 @@ read_control_auth(struct buffer *buf,
* This function creates a reset packet using the information
* from the tls pre decrypt state.
*
- * The returned buf needs to be free with \c free_buf
*/
struct buffer
tls_reset_standalone(struct tls_wrap_ctx *ctx,
@@ -563,6 +563,7 @@ test_generate_reset_packet_plain(void **ut_state)
tas.tls_wrap.mode = TLS_WRAP_NONE;
struct frame frame = { {.headroom = 200, .payload_size = 1400}, 0};
tas.frame = frame;
+ tas.workbuf = alloc_buf(1600);
uint8_t header = 0 | (P_CONTROL_HARD_RESET_CLIENT_V2 << P_OPCODE_SHIFT);
@@ -580,6 +581,7 @@ test_generate_reset_packet_plain(void **ut_state)
free_buf(&buf2);
free_tls_pre_decrypt_state(&state);
+ free_buf(&tas.workbuf);
free_buf(&buf);
}
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. Also packet_id_init will allocated a buffer with malloc and not using a gc_arena, so we need to manually also free it. Found-By: clang with asan Change-Id: I0cff44f79ee7e3bcf7b5981fc94f469c15f21af3 Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/init.c | 3 +++ src/openvpn/ssl.c | 12 ++++++++++++ src/openvpn/ssl.h | 6 ++++++ src/openvpn/ssl_pkt.c | 8 ++++++-- src/openvpn/ssl_pkt.h | 2 +- tests/unit_tests/openvpn/test_pkt.c | 2 ++ 6 files changed, 30 insertions(+), 3 deletions(-)