From patchwork Tue Mar 14 13:09:31 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 3128 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7300:2310:b0:9f:bfa4:120f with SMTP id r16csp2261484dye; Tue, 14 Mar 2023 06:10:48 -0700 (PDT) X-Google-Smtp-Source: AK7set9XXdx3yEM33Pa/2inXUvQqJiTnZfGz5vTj2gHrjpYc/sp3q4qZeTJwp9wUGbbryO82AofY X-Received: by 2002:a05:6a20:49a3:b0:d4:122a:d3da with SMTP id fs35-20020a056a2049a300b000d4122ad3damr6947096pzb.42.1678799447997; Tue, 14 Mar 2023 06:10:47 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1678799447; cv=none; d=google.com; s=arc-20160816; b=zT8RElV/3fFjLXowLjmd/7SEGUHlygKyjpPHUJOzjJ+xXaByGG1hVMB3lDfjzRO2q7 qomiX6RXQHRgLJ09nvEzHatbhOYAZ9sOjZlMvffMzxbmQw4nVkU8CHLdgnYX3ss73ENl TpfH35MlNYCJM9bDNqUFjnULY9I+DKpwDMycIezewZOUrQVuHNnSkIApqOvvJb78G26+ 7tj+6UOqrRv0tdMhyqsUBJab74afuEJDiN9dpeMyMdifyu1beFUZ6pOE0w6KvkwmB9TX KSW3JmP2n8jY4naBCu7IMTRTZpq3P/xEsxASCQ8oeq7SpPZHxq8/GEe//5Fn846Z09t1 y0lA== 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=NtfdFGmtkr4L9YoOF++RRENZWzV0Qj2RvGUIbaYOybc=; b=qoVE7b4C2RHcJWE/Rzlwh8EeN+WyUUIzLhCmAZAYf/mEHTrgWxc9TLtu6EuHMoSSZQ ovnFL+Lx/PExw8GtrDheqRi3yGQBCzBhksUf9/1K8qU6ERA7G+gz2iP9l3Na4hfyYYIM Y6Aukenl6SaMJuhjB8Sa2j11lOvfhWqmMD3Z5IQraLdDFRixZeccQvAP8/LBmreUXHek fer3GJkXPRI1/MQ5F2DTcYMJCV3oBjTGmPCRULShjdVm0aKtarWeGCurwbzja1hdNIlj qa4QgD1fRk/OzOKCa6UgelROSx/MXxMSX2qMPGrUCVrzLSEqATKZTGKhOmK85Rzp5wKm qsvA== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=YGJ3Ocb+; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=ZefCgqXs; 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 s1-20020a63e801000000b005073e3342eesi2171596pgh.143.2023.03.14.06.10.47 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 14 Mar 2023 06:10:47 -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=YGJ3Ocb+; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=ZefCgqXs; 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 1pc4PY-0002hx-UF; Tue, 14 Mar 2023 13:09:53 +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 1pc4PN-0002hp-Kc for openvpn-devel@lists.sourceforge.net; Tue, 14 Mar 2023 13:09:42 +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=NSq3uwqnagw5veQ9PSrxht+8egzCrSauMqJpe5/UU/4=; b=YGJ3Ocb+bFX4Ukf1IYOFalmPMH SN6JCIdFGyF4bgxlcPbhtnmbrZ+f/+wTUwfun+ouWU2wvvXEmMkEWf5nBc/f77p+9OOat4N/2XPa/ Ci4x1yS7NuJ/gSYsZ61ITGL0IWaDNQywEVHmvjth1fGMwRWix64ry45QCLgEinqwVJsI=; 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=NSq3uwqnagw5veQ9PSrxht+8egzCrSauMqJpe5/UU/4=; b=Z efCgqXsnBFtbac3ERg7d9UaicSNnlqsmWHWDpEKX/ZAsmfGSWHM0hRJXs3dO5k/P9upTEpW44EQPk jtLVIcCi7/8UqXjzqCL6trUGX9IXZLpA16GlIBcEW7gx1ou/lTwx/pyeKArQlVgUvrBDJfq0s4m4f XzwG/I3PAo6bZxHY=; 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 1pc4PL-0005PC-4h for openvpn-devel@lists.sourceforge.net; Tue, 14 Mar 2023 13:09:40 +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 1pc4PD-000E8Z-PT for openvpn-devel@lists.sourceforge.net; Tue, 14 Mar 2023 14:09:31 +0100 Received: (nullmailer pid 174038 invoked by uid 10006); Tue, 14 Mar 2023 13:09:31 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Tue, 14 Mar 2023 14:09:31 +0100 Message-Id: <20230314130931.173992-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 open_tun_dco_generic already allocates the actual_name string, this shadows the allocation in the FreeBSD/Linux specific methods. 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: 1pc4PL-0005PC-4h Subject: [Openvpn-devel] [PATCH v3] Fix memory leaks in HMAC initial packet id and dco open tun 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?1760348809549875386?= X-GMAIL-MSGID: =?utf-8?q?1760348809549875386?= The open_tun_dco_generic already allocates the actual_name string, this shadows the allocation in the FreeBSD/Linux specific methods. 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. Found-By: clang with asan Patch v2: rebase. Include linux bits accidentially forgotten. Patch v3: fix also tls-crypt. Change-Id: I3c344af047abe94c0178bde1781eb450f10d157d Signed-off-by: Arne Schwabe --- src/openvpn/dco_freebsd.c | 1 - src/openvpn/dco_linux.c | 1 - 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 +- 7 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c index ecca2a076..225b3cf88 100644 --- a/src/openvpn/dco_freebsd.c +++ b/src/openvpn/dco_freebsd.c @@ -232,7 +232,6 @@ create_interface(struct tuntap *tt, const char *dev) } snprintf(tt->dco.ifname, IFNAMSIZ, "%s", ifr.ifr_data); - tt->actual_name = string_alloc(tt->dco.ifname, NULL); /* see "Interface Flags" in ifnet(9) */ int i = IFF_POINTOPOINT | IFF_MULTICAST; diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c index b2fdbf53f..e5cea3c71 100644 --- a/src/openvpn/dco_linux.c +++ b/src/openvpn/dco_linux.c @@ -457,7 +457,6 @@ open_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx, const char *dev) msg(M_FATAL, "DCO: cannot retrieve ifindex for interface %s", dev); } - tt->actual_name = string_alloc(dev, NULL); tt->dco.dco_message_peer_id = -1; ovpn_dco_register(&tt->dco); 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,