From patchwork Tue Mar 14 14:48:53 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 3129 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7300:2310:b0:9f:bfa4:120f with SMTP id r16csp2332569dye; Tue, 14 Mar 2023 07:49:31 -0700 (PDT) X-Google-Smtp-Source: AK7set+zTRlkcjP2Rwin2arSwEhvJzU6Y7V6Z3mjJMq21zQL85nBNLUTBmboYvlIV4Hz9K1LvI8u X-Received: by 2002:a05:6a20:4f10:b0:d3:d236:f5b7 with SMTP id gi16-20020a056a204f1000b000d3d236f5b7mr8636793pzb.26.1678805371489; Tue, 14 Mar 2023 07:49:31 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1678805371; cv=none; d=google.com; s=arc-20160816; b=NxMnG/TSSU/2FszrDs0ZKAtyjBfA4H7h7LLod/4h3iN0TxkUNkjKY4TR1ZiLfgkfF7 HkRPNpEXNetKIv9xfLMTIHC6NLKmD4btGectvVVEEU+eBLtGAqdE5T8EpTlfRCX9J0c+ f82zEr0E/2hQncQPgtgCRbUudv9W9JgoBkOuWTHcpS2V/6hMXo18ATLUQ416YFz2JLOv FqIwNUSfW/Hgaj5ohV6+IOEvAQ8HopVedsw0fRoZ9/pjoUoQ/dGPuQnImiCm+Pw+T83q Q0VNGsfay6t9cCjdR+xANeJcoYOae4govUVOOfUAFxMa8S/dN/Y4OP8ARG0sb7ncvxiJ EIBg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:content-transfer-encoding:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence:subject :mime-version:message-id:date:to:from:dkim-signature:dkim-signature; bh=A9R0IgGRQDmHJ4D1zHacHr5d5C9wpnu1ZPefYZWwfE0=; b=XBtVe2+YWvm/fnTJNfyJ2t7vS1R2udkfSLOhlpBfNa8EGB/Lp2g7YrMDTt2wvpON1D 3WteK8L//coj3TNksSxPkFktUvWOtOtLU47AmVjEEZKHX0YvaPdyIUnNyj2neVSm3I/P 5LZvVabmmiOL0rdG0uT7KBouRsLmBOx+zy4htXVmA3nkBL8ajnlIYMlBf1glJFV/667j QtgqYjCjXuFRQ/Z7jCYkFCq2AeXi3+DX0ObMYu6hzemXZqMqolrOFfRiwZdM0M6nPPVB DruC0Ip33QnoRUnE6MCwTVoocnA7DgjeyPhRYal/B/1poAPhyzgf3LxQCu150IAjXVZi PmCA== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=Ky+ZWXeN; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=LYK29rUW; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id s26-20020a63525a000000b005001709205bsi2374038pgl.722.2023.03.14.07.49.31 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 14 Mar 2023 07:49:31 -0700 (PDT) Received-SPF: pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) client-ip=216.105.38.7; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=Ky+ZWXeN; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=LYK29rUW; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net Received: from [127.0.0.1] (helo=sfs-ml-1.v29.lw.sourceforge.com) by sfs-ml-1.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1pc5xe-0005jv-B3; Tue, 14 Mar 2023 14:49:10 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-1.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1pc5xc-0005jR-35 for openvpn-devel@lists.sourceforge.net; Tue, 14 Mar 2023 14:49:08 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:Message-Id: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=VLGRzLH8JFgDUBHKJ2zbrgZwo0r6Z+etn6DOW0HL4/A=; b=Ky+ZWXeNzfyEdY+9FIaPL5oHD4 g+Uy51YCzv0+tQHldbb9m+WaloWPNxCEMgi3xp7nENrYZuIa32fMmFrNjCJBhBDrYulrmO0kXRFBD zZqzDGhSzF51wH8qGoN22l3IlOt3gS05iVqfHM7zCYSNhRAKLvKMAV1egBn02YEhFTcQ=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:Message-Id:Date:Subject:To:From: Sender:Reply-To:Cc:Content-Type:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To: References:List-Id:List-Help:List-Unsubscribe:List-Subscribe:List-Post: List-Owner:List-Archive; bh=VLGRzLH8JFgDUBHKJ2zbrgZwo0r6Z+etn6DOW0HL4/A=; b=L YK29rUWzqqwY/IOk80fJgaSvrZMALcpNeqln89Ukd5qfcTqJy/nhLxx0UJdAOCqy/1v1SmctA3+k3 1BZYQGCTsih9YHprR2MP7Pv4rch6eLzTmariuMfa467B3XX0uoGpEpAH+oPNJB2KrhFDiT/UYSvcG /MSAcW55eWnvz20c=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1pc5xa-0000Nu-IM for openvpn-devel@lists.sourceforge.net; Tue, 14 Mar 2023 14:49:08 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.95 (FreeBSD)) (envelope-from ) id 1pc5xO-000EdC-M6 for openvpn-devel@lists.sourceforge.net; Tue, 14 Mar 2023 15:48:54 +0100 Received: (nullmailer pid 182158 invoked by uid 10006); Tue, 14 Mar 2023 14:48:54 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Tue, 14 Mar 2023 15:48:53 +0100 Message-Id: <20230314144854.182110-1-arne@rfc2549.org> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 X-Spam-Score: 0.3 (/) X-Spam-Report: Spam detection software, running on the system "util-spamd-2.v13.lw.sourceforge.com", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: 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 bi [...] Content analysis details: (0.3 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.0 SPF_NONE SPF: sender does not publish an SPF Record X-Headers-End: 1pc5xa-0000Nu-IM Subject: [Openvpn-devel] [PATCH 1/2] Fix memory leaks in HMAC initial packet id X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1760355021191894084?= X-GMAIL-MSGID: =?utf-8?q?1760355021191894084?= 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 --- 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(-) 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); } } @@ -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); } /* 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; } +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 diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 58ff4b9b4..a050cd5c9 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -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 diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c index 17a7f8917..8b3391e76 100644 --- a/src/openvpn/ssl_pkt.c +++ b/src/openvpn/ssl_pkt.c @@ -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; diff --git a/src/openvpn/ssl_pkt.h b/src/openvpn/ssl_pkt.h index ec7b48daf..ef4852e5d 100644 --- a/src/openvpn/ssl_pkt.h +++ b/src/openvpn/ssl_pkt.h @@ -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, diff --git a/tests/unit_tests/openvpn/test_pkt.c b/tests/unit_tests/openvpn/test_pkt.c index f11e52a11..8574afc6a 100644 --- a/tests/unit_tests/openvpn/test_pkt.c +++ b/tests/unit_tests/openvpn/test_pkt.c @@ -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); }