From patchwork Fri Dec 8 16:29:54 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "plaisthos (Code Review)" X-Patchwork-Id: 3513 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7300:8d12:b0:fc:24ac:f0cb with SMTP id i18csp3710090dys; Fri, 8 Dec 2023 08:31:05 -0800 (PST) X-Google-Smtp-Source: AGHT+IGaE/5yxdgcmqsTfDNzAm8W9d+sQsWGU3AoGOkXp9oZU7PaOJM6DF8vbXzbyONAUHCAG5u0 X-Received: by 2002:a17:90a:c082:b0:286:5123:ddaf with SMTP id o2-20020a17090ac08200b002865123ddafmr656817pjs.3.1702053064674; Fri, 08 Dec 2023 08:31:04 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1702053064; cv=none; d=google.com; s=arc-20160816; b=IjTodxr3hxLp8N+Z2+sNZ2mFyJwCVrHx/5cTVbvy72a49UR+S/epMnlnaeISKOTGQJ qQ23XaHTfsCFAQdvCxMP7uUfgAVwKJNs+yE0NV2wkj32ItdK720Dppsy+yOc1LMJISa7 c0LtWl3u9VqqGpQIjjXBGEExTitNAh7HrrvCy/9UUNeNDAX72Gv2Xv9ArkZKCsObnSy6 dTUzvY3aQNZFm9AJET802jw+N/pZWNkh8i02qHDr64ziuJJcbg++i7nqRORQ4KSl+Q+A pE7m6piSI0rB+oH+bB0ytf5khXAKYNFbiwI8Y7MQqTXWNJ6KnqW+tn7vOwVreltHzZPA cnjQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:cc:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:user-agent :mime-version:message-id:references:auto-submitted:to:date:from :dkim-signature:dkim-signature:dkim-signature; bh=5tyxOqV4vw/Dfwo/ZvA2JOb580tm44aDdbOmrnv3yCw=; fh=GFP4qDxgyJ2WEPo/oeLZg3Mj4NqvY1j2nTvTt7psNwg=; b=sfxJoLEy362YLNd5Wq6XW6FjjOgdHJIvlbXqn1pKWURLgwLTdzj1WGuEf6A/eW7Ngd 34oD2Eg/c4EaZgpbXH0djMvXVifJ0k6b3GGP+35l+1kADsnnYtvD9R6IzUnh2DldIaW0 cRhLCYGTn9D7XeKpHL3qBp/zWRvTpYOQEIFUDQhP84l27VNY/ysvLNV8ni+/qqluSi9f 9xRjJnZ25HK12OVZSvdl1YXmFsQPP7koP0cMJBF5JmjhkK7wf8oHcGDQg7A+B98Vh/Ga vKgrulyAFHcn1PuxNcVJonaciCx0LmwCPUuUXcO12HnkT2hU/B3QljwOyyKcQ3C/NFcu hD3g== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=gd+9mRi5; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=Nw9yq3SK; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=Bf7PnkVD; 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; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=openvpn.net Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id h5-20020a17090a130500b00286b22b913fsi1922871pja.37.2023.12.08.08.31.04 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 08 Dec 2023 08:31:04 -0800 (PST) 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=gd+9mRi5; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=Nw9yq3SK; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=Bf7PnkVD; 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; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=openvpn.net Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1rBdk0-0001Nd-46; Fri, 08 Dec 2023 16:30:16 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1rBdjv-0001NF-C0 for openvpn-devel@lists.sourceforge.net; Fri, 08 Dec 2023 16:30:11 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Type:Content-Transfer-Encoding:MIME-Version :Message-ID:Reply-To:References:Subject:List-Unsubscribe:List-Id:Cc:To:Date: From:Sender:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:List-Help: List-Subscribe:List-Post:List-Owner:List-Archive; bh=cDzaERLxh8SXX7+SELYGZOKABi0ChXG2MQuPAzRdLLo=; b=gd+9mRi5E6B4+lg2pBDbAbS5Jc C/8QLxp+ejgvS4rp82H7HRRZA4oyBU+4KZmeECt5+MM4a/1VuJE0ZN/QAXcIZVARH4zzAUcynnAPV mUDfzyHnUJ81wnsZ70qMwpiqJYX7kmT4qrUp/atsmujL88JTKQ0eCM6wCXY5tI3MRtxk=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Type:Content-Transfer-Encoding:MIME-Version:Message-ID:Reply-To: References:Subject:List-Unsubscribe:List-Id:Cc:To:Date:From:Sender:Content-ID :Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To: Resent-Cc:Resent-Message-ID:In-Reply-To:List-Help:List-Subscribe:List-Post: List-Owner:List-Archive; bh=cDzaERLxh8SXX7+SELYGZOKABi0ChXG2MQuPAzRdLLo=; b=N w9yq3SK5oGKr+HVM6NXSOBIYyXP9/iY53CEFbVRvkcI6b73ewgEuJYvaWRVlOLyRR1UMgzhnMs/um J2HAEFWC0SbGJSVh1AjOrZpZbyvr2sV9UbAfkPP+D+50EjgAu60yRRU3ctxj58cJJBl69ppXSryvy tC3yXZN4wtbEhcmQ=; Received: from mail-wm1-f51.google.com ([209.85.128.51]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1rBdjr-0007mx-Rx for openvpn-devel@lists.sourceforge.net; Fri, 08 Dec 2023 16:30:09 +0000 Received: by mail-wm1-f51.google.com with SMTP id 5b1f17b1804b1-40c039e9719so25742905e9.1 for ; Fri, 08 Dec 2023 08:30:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=openvpn.net; s=google; t=1702052996; x=1702657796; darn=lists.sourceforge.net; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:to:date:from:from:to:cc :subject:date:message-id:reply-to; bh=cDzaERLxh8SXX7+SELYGZOKABi0ChXG2MQuPAzRdLLo=; b=Bf7PnkVD9ZK6jIiORtffN7bGtVYsys6KhVpUdWXq1jzVX4CkSGaR9XUdOK9P7lQeMc bXk0UbVU2ngvuf71O2tOmsBkv+TimArk/Auu/QdSm7wFkwvLFQd6TlT2eHBphC59lDyq BEYFnkfkq/P3awd/2WZyF2T0/gr0IK5nUroEBJSEfW3z0H/Dz6WKUI6N8G6AOM8vgWMV wZrGcQpTHea1kbfRI/hWYMtiklEWCZEWTmxQfpltretzlggr7ud/8xtxhkHtFbSKBkTW oKhOhGSOSyONsVv0kJnSWjLRet7FSqLdxTb05CCOo/CRZ7Q24Eo3hPDmccG5u3tD2Tib f02w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702052996; x=1702657796; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:to:date:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=cDzaERLxh8SXX7+SELYGZOKABi0ChXG2MQuPAzRdLLo=; b=Db3wTxBJ/lsf+2knrai5hddgn7Q8jc1NV3zBIcJ47aG0fyGJh+oge16tu67GyHx2l4 ZrIPiHHUghbaZ8jqwsmtBTQKZPdMPNYrV6yqWavCkJFqZam9DX4JmCh8fQHj1b/9o+VP YxE2zfF28I/XVE/PjtgJpXueKQ0BipqxChMED59Qrn+yURBaAcGBxQgtYEXW7q2sFgcE PF22+ePx2MuzDYvRX/bO7iqhg/IqOc9MzTAtzkVWBgKaR/DAD0CklaFRm4MouD1JDD23 X/9dYHozOKITNyoWk+TTx7e/MyCEp0yLGCu7GYOkpSSV7gdl+x5ynaFxS17OFR04+U+D cfdg== X-Gm-Message-State: AOJu0YyQYTWWc9oOGuqk1PiRsVdadRnQdAV3FLn7vhMjDhzYQXtRtfZZ 4jF2Iv/GE3yNocKUef33NAn0GMQ3YEtQsZeIFJU= X-Received: by 2002:a05:600c:364a:b0:40c:2b24:325e with SMTP id y10-20020a05600c364a00b0040c2b24325emr111018wmq.184.1702052995653; Fri, 08 Dec 2023 08:29:55 -0800 (PST) Received: from gerrit.openvpn.in (ec2-18-159-0-78.eu-central-1.compute.amazonaws.com. [18.159.0.78]) by smtp.gmail.com with ESMTPSA id jg8-20020a05600ca00800b0040b54335d57sm5651137wmb.17.2023.12.08.08.29.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 08 Dec 2023 08:29:55 -0800 (PST) From: "flichtenheld (Code Review)" X-Google-Original-From: "flichtenheld (Code Review)" X-Gerrit-PatchSet: 1 Date: Fri, 8 Dec 2023 16:29:54 +0000 To: plaisthos Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange X-Gerrit-Change-Id: I0abeec9f862aea1f6a8fdf350fa0008cf2e5d613 X-Gerrit-Change-Number: 476 X-Gerrit-Project: openvpn X-Gerrit-ChangeURL: X-Gerrit-Commit: b9cfe9ef20d42f12b573c6ca632d2d97b2400ffb References: Message-ID: <76c50ee7fb8a7f7f54543ce7b5abe5be887f126b-HTML@gerrit.openvpn.net> MIME-Version: 1.0 User-Agent: Gerrit/3.8.2 X-Spam-Score: -0.2 (/) X-Spam-Report: Spam detection software, running on the system "util-spamd-1.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: Attention is currently required from: plaisthos. Hello plaisthos, I'd like you to do a code review. Please visit Content analysis details: (-0.2 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [209.85.128.51 listed in list.dnswl.org] -0.0 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [209.85.128.51 listed in wl.mailspike.net] 0.0 WEIRD_PORT URI: Uses non-standard port number for HTTP 0.0 HTML_MESSAGE BODY: HTML included in message -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain -0.0 T_SCC_BODY_TEXT_LINE No description available. 0.0 T_KAM_HTML_FONT_INVALID Test for Invalidly Named or Formatted Colors in HTML X-Headers-End: 1rBdjr-0007mx-Rx Subject: [Openvpn-devel] [M] Change in openvpn[master]: misc: make get_auth_challenge static 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: , Reply-To: frank@lichtenheld.com, arne-openvpn@rfc2549.org, openvpn-devel@lists.sourceforge.net Cc: openvpn-devel Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1784731994373313267?= X-GMAIL-MSGID: =?utf-8?q?1784731994373313267?= X-getmail-filter-classifier: gerrit message type newchange Attention is currently required from: plaisthos. Hello plaisthos, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/476?usp=email to review the following change. Change subject: misc: make get_auth_challenge static ...................................................................... misc: make get_auth_challenge static Not used outside of misc.c. Rename to parse_auth_challenge since it really just parses the string that you put in into the struct. Add doxygen documentation. Change-Id: I0abeec9f862aea1f6a8fdf350fa0008cf2e5d613 Signed-off-by: Frank Lichtenheld --- M src/openvpn/misc.c M src/openvpn/misc.h M src/openvpn/ssl.h 3 files changed, 103 insertions(+), 85 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/76/476/1 diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index ce6e4fd..9352843 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -124,6 +124,88 @@ } return true; } + +/** + * Parses an authentication challenge string and returns an auth_challenge_info structure. + * The authentication challenge string should follow the dynamic challenge/response protocol. + * + * See doc/management-notes.txt for more info on the the dynamic challenge/response protocol + * implemented here. + * + * @param auth_challenge The authentication challenge string to parse. + * @param gc The gc_arena structure for memory allocation. + * + * @return A pointer to the parsed auth_challenge_info structure, or NULL if parsing fails. + */ +static struct auth_challenge_info * +parse_auth_challenge(const char *auth_challenge, struct gc_arena *gc) +{ + if (auth_challenge) + { + struct auth_challenge_info *ac; + const int len = strlen(auth_challenge); + char *work = (char *) gc_malloc(len+1, false, gc); + char *cp; + + struct buffer b; + buf_set_read(&b, (const uint8_t *)auth_challenge, len); + + ALLOC_OBJ_CLEAR_GC(ac, struct auth_challenge_info, gc); + + /* parse prefix */ + if (!buf_parse(&b, ':', work, len)) + { + return NULL; + } + if (strcmp(work, "CRV1")) + { + return NULL; + } + + /* parse flags */ + if (!buf_parse(&b, ':', work, len)) + { + return NULL; + } + for (cp = work; *cp != '\0'; ++cp) + { + const char c = *cp; + if (c == 'E') + { + ac->flags |= CR_ECHO; + } + else if (c == 'R') + { + ac->flags |= CR_RESPONSE; + } + } + + /* parse state ID */ + if (!buf_parse(&b, ':', work, len)) + { + return NULL; + } + ac->state_id = string_alloc(work, gc); + + /* parse user name */ + if (!buf_parse(&b, ':', work, len)) + { + return NULL; + } + ac->user = (char *) gc_malloc(strlen(work)+1, true, gc); + openvpn_base64_decode(work, (void *)ac->user, -1); + + /* parse challenge text */ + ac->challenge_text = string_alloc(BSTR(&b), gc); + + return ac; + } + else + { + return NULL; + } +} + #endif /* ifdef ENABLE_MANAGEMENT */ /* @@ -287,7 +369,7 @@ #ifdef ENABLE_MANAGEMENT if (auth_challenge && (flags & GET_USER_PASS_DYNAMIC_CHALLENGE) && response_from_stdin) { - struct auth_challenge_info *ac = get_auth_challenge(auth_challenge, &gc); + struct auth_challenge_info *ac = parse_auth_challenge(auth_challenge, &gc); if (ac) { char *response = (char *) gc_malloc(USER_PASS_LEN, false, &gc); @@ -392,83 +474,6 @@ return true; } -#ifdef ENABLE_MANAGEMENT - -/* - * See management/management-notes.txt for more info on the - * the dynamic challenge/response protocol implemented here. - */ -struct auth_challenge_info * -get_auth_challenge(const char *auth_challenge, struct gc_arena *gc) -{ - if (auth_challenge) - { - struct auth_challenge_info *ac; - const int len = strlen(auth_challenge); - char *work = (char *) gc_malloc(len+1, false, gc); - char *cp; - - struct buffer b; - buf_set_read(&b, (const uint8_t *)auth_challenge, len); - - ALLOC_OBJ_CLEAR_GC(ac, struct auth_challenge_info, gc); - - /* parse prefix */ - if (!buf_parse(&b, ':', work, len)) - { - return NULL; - } - if (strcmp(work, "CRV1")) - { - return NULL; - } - - /* parse flags */ - if (!buf_parse(&b, ':', work, len)) - { - return NULL; - } - for (cp = work; *cp != '\0'; ++cp) - { - const char c = *cp; - if (c == 'E') - { - ac->flags |= CR_ECHO; - } - else if (c == 'R') - { - ac->flags |= CR_RESPONSE; - } - } - - /* parse state ID */ - if (!buf_parse(&b, ':', work, len)) - { - return NULL; - } - ac->state_id = string_alloc(work, gc); - - /* parse user name */ - if (!buf_parse(&b, ':', work, len)) - { - return NULL; - } - ac->user = (char *) gc_malloc(strlen(work)+1, true, gc); - openvpn_base64_decode(work, (void *)ac->user, -1); - - /* parse challenge text */ - ac->challenge_text = string_alloc(BSTR(&b), gc); - - return ac; - } - else - { - return NULL; - } -} - -#endif /* ifdef ENABLE_MANAGEMENT */ - void purge_user_pass(struct user_pass *up, const bool force) { diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h index b000b72..98de90a 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -86,8 +86,6 @@ const char *challenge_text; }; -struct auth_challenge_info *get_auth_challenge(const char *auth_challenge, struct gc_arena *gc); - /* * Challenge response info on client as pushed by server. */ @@ -120,12 +118,31 @@ #define GET_USER_PASS_INLINE_CREDS (1<<10) /* indicates that auth_file is actually inline creds */ +/** + * Retrieves the user credentials from various sources depending on the flags. + * + * @param up The user_pass structure to store the retrieved credentials. + * @param auth_file The path to the authentication file. Might be NULL. + * @param prefix The prefix to prepend to user prompts. + * @param flags Additional flags to control the behavior of the function. + * @param auth_challenge The authentication challenge string. + * @return true if the user credentials were successfully retrieved, false otherwise. + */ bool get_user_pass_cr(struct user_pass *up, const char *auth_file, const char *prefix, const unsigned int flags, const char *auth_challenge); +/** + * Retrieves the user credentials from various sources depending on the flags. + * + * @param up The user_pass structure to store the retrieved credentials. + * @param auth_file The path to the authentication file. Might be NULL. + * @param prefix The prefix to prepend to user prompts. + * @param flags Additional flags to control the behavior of the function. + * @return true if the user credentials were successfully retrieved, false otherwise. + */ static inline bool get_user_pass(struct user_pass *up, const char *auth_file, diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 6ba6ff8..71b99db 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -410,11 +410,7 @@ bool ssl_clean_auth_token(void); #ifdef ENABLE_MANAGEMENT -/* - * ssl_get_auth_challenge will parse the server-pushed auth-failed - * reason string and return a dynamically allocated - * auth_challenge_info struct. - */ + void ssl_purge_auth_challenge(void); void ssl_put_auth_challenge(const char *cr_str);