From patchwork Fri Dec 8 16:29:52 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: 3509 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7300:8d12:b0:fc:24ac:f0cb with SMTP id i18csp3709821dys; Fri, 8 Dec 2023 08:30:39 -0800 (PST) X-Google-Smtp-Source: AGHT+IHSOLqKDPNz+gfSEZE4cFdSm2AXj/8B9ffhSPaL2ko1bacCmOG6cPZesIce63NBFpYMR+AB X-Received: by 2002:a05:6a20:1609:b0:188:67f:ff2c with SMTP id l9-20020a056a20160900b00188067fff2cmr683430pzj.0.1702053038769; Fri, 08 Dec 2023 08:30:38 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1702053038; cv=none; d=google.com; s=arc-20160816; b=bv4fxx/fq/1cwMQ2pJ34FKKFhW7xh7IJh0Fau9T+kH93xBjaOwZ8h5Q5O39CsAxmBF engExLJ39DQ7s+6SZXUOnS+ABmijozDbce5mnqie38t3946GxysCXdU+UAT9JgfNt9r+ LOa2kr5aLzizcXk81lJQLwJJAJ5SRs2DMPdUNGlgOucvtiHeXwfx/N+sd1BsqJNgpvDr 4TNkV+DuyHJA+ql69Nngq+HdMgi4Kwknk92n0NTBvYNwZNQouZSjBNwzK5EDzmtmuGPn yEk5jJEF5IAOkBcbXagkMsfF5qaSf/bvuKCKKUJWbiKCj+h4Rk4hsYU1mHQuNRqtb3HD /Ezw== 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=YhAnzMt6DsiTX/vHsCtV0RBtDR820EsCRvZf3JHospw=; fh=GFP4qDxgyJ2WEPo/oeLZg3Mj4NqvY1j2nTvTt7psNwg=; b=k9cBi73V0loRGDkJKlU8Oh2RZPpW1d7A4tFVseLbglKlLYa9kEc8jr78Zu6jnRa/gH enLaMw0ZomCvbVn2nHFNpiRf29WNuAPca8D9852TmAd/fbxwEzbBGLNyZRZcbpoTlLAY 3/CRqxiHbw1L+UnURTqUvvoIjnYo8xWjEAamGyw3XAwHWbB6lESLE2e1j0ELIbbTqNHj cGfNI3CTd3MyUO7SndzFctB+bQMDbyQ1TFIC1p4PqkGz2pB2/RfbY+qmDhQdQ3UkNx9t WgQozMZ3ZnrsjP7Zlin/vFvnksu4tM3vdtln+thZ01/av/AmLig4b3v87wKYnNOgCV3g B3eQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=doB+Qrih; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=ahmgzFpa; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=bhFCHTyo; 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 m125-20020a632683000000b005c6183ccab1si1761469pgm.529.2023.12.08.08.30.38 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 08 Dec 2023 08:30:38 -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=doB+Qrih; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=ahmgzFpa; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=bhFCHTyo; 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-3.v29.lw.sourceforge.com) by sfs-ml-3.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1rBdk1-0006e8-DI; Fri, 08 Dec 2023 16:30:16 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-3.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1rBdjt-0006dt-Tq for openvpn-devel@lists.sourceforge.net; Fri, 08 Dec 2023 16:30:08 +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=rdGgbFKgcwP9lzaX3GQGKv34Y2oG+uBHwW3zCbvjuHQ=; b=doB+Qrihc3g0vmIPxVB8fN+w90 Xi8OhDcSSVqVszRa7lbnCObQOUV7IipHw7do8Gkpeg0aO7MwLY6AtYbCh3IQ16e0/96rSHVNAIown dI2k2zseb+xFPTRJ1AD6NmaP1djZ46bMJfTMls4kZv/hZMubtBfvs993nPbxMHLoioRI=; 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=rdGgbFKgcwP9lzaX3GQGKv34Y2oG+uBHwW3zCbvjuHQ=; b=a hmgzFpaL1qRXhuQ2AOJsWH50E/Hkb8ilPcgRUN4Bmt3ui8L7rwaXN7ONrbkBgle85FVzDObtrZfhw ecbfSsb+AFD3x1o9EpCGU0cs3E1xxWdNSY4dIyt77il+j56rq2nobukHGA1/6PiIKQsdp3QuQkOoY rMyGrYoluqODKM20=; Received: from mail-wr1-f44.google.com ([209.85.221.44]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1rBdjr-0007le-RH for openvpn-devel@lists.sourceforge.net; Fri, 08 Dec 2023 16:30:08 +0000 Received: by mail-wr1-f44.google.com with SMTP id ffacd0b85a97d-33338c47134so2071310f8f.1 for ; Fri, 08 Dec 2023 08:30:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=openvpn.net; s=google; t=1702052993; x=1702657793; 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=rdGgbFKgcwP9lzaX3GQGKv34Y2oG+uBHwW3zCbvjuHQ=; b=bhFCHTyodKKscBVWeYlbIQjj/zGNAOj+GqpYBQmrFIPt8xj73TZfSiNww/4JeCo+rD WDJu7kxLrg6qFrFw/zexM8W9PVBHAHQwt1QlLKzGxxga62RflYmaecQoZdmuDl+CyEnG EK0cbf//rygSrpqer0ocl44PiFeq2yfMIdcSUwi2Z2kk2rbGBiUHqzbAL2Iy55tgUbzT CNTPqWntNYQxGIdcL8TgXHuhFAtuAfrWhVAtk+m4Vcvh28kIKAaIsEMP84MMzVof1S9i fIvZlz4NS8pSbuXlwHcJ7lX+OMZoJ2Lw3a8woUGIZQTQ99KLV/NGrRSqoPlChh1cBpN1 UpXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702052993; x=1702657793; 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=rdGgbFKgcwP9lzaX3GQGKv34Y2oG+uBHwW3zCbvjuHQ=; b=PJbFYfI5CBJtv241xNxrdilJ/pMhuYevGnx6AfAytfM7bmvPrZxwc1/oQATlMAlVK1 sUt6hnfpo3ljeYTWdq9LkoTrCH/3MU+J1nHT4EqEkcwi5nAFwG0JnN2rdwbc1lLKf20R FbY+rEGWKWRE3sxPt+x97Mv3U7BZn/KNA2+Ae8hnDyunMlNKUjk24AldVvSS4qG2pDeD k+6lW41P4CZgll9LwEK0Bdr5F5BpoFpVVIpS1LWqZg+FP1QbI09Etvcp39C3fER0FKZu JeWUWBfvohI8XkU6G/SXOimlxh/+XDAa+x2F0HAgeRKotxXWC++CJ4YL9rJUcAYFLtwH LfIg== X-Gm-Message-State: AOJu0Yz/A3P1Bp46oNzIom2jVeG5YmI1unHfkxsHRi9cj0CZMNT4Cdk+ SPffmLg5ndcswUDcKVNucMQmqD3zIsbPBiYrcKU= X-Received: by 2002:adf:f743:0:b0:336:101b:2234 with SMTP id z3-20020adff743000000b00336101b2234mr139955wrp.118.1702052993297; Fri, 08 Dec 2023 08:29:53 -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 w13-20020a5d404d000000b003333f9200d8sm2367598wrp.84.2023.12.08.08.29.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 08 Dec 2023 08:29:52 -0800 (PST) From: "flichtenheld (Code Review)" X-Google-Original-From: "flichtenheld (Code Review)" X-Gerrit-PatchSet: 1 Date: Fri, 8 Dec 2023 16:29:52 +0000 To: plaisthos Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange X-Gerrit-Change-Id: Icabc8acf75638c86c8c395e9ffecba7a7226cd97 X-Gerrit-Change-Number: 474 X-Gerrit-Project: openvpn X-Gerrit-ChangeURL: X-Gerrit-Commit: b67c7dd81ccabadd0cfc4c1a07f774e3739c9d8d References: Message-ID: 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.221.44 listed in list.dnswl.org] -0.0 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [209.85.221.44 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-0007le-RH Subject: [Openvpn-devel] [M] Change in openvpn[master]: test_user_pass: Check fatal errors for empty username/password 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?1784731967313201585?= X-GMAIL-MSGID: =?utf-8?q?1784731967313201585?= 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/+/474?usp=email to review the following change. Change subject: test_user_pass: Check fatal errors for empty username/password ...................................................................... test_user_pass: Check fatal errors for empty username/password Required a fix to mock_msg to make tests of M_FATAL possible at all. This also tests some cases which arguably should throw a fatal error but do not. Change-Id: Icabc8acf75638c86c8c395e9ffecba7a7226cd97 Signed-off-by: Frank Lichtenheld --- A tests/unit_tests/openvpn/input/appears_empty.txt A tests/unit_tests/openvpn/input/empty.txt M tests/unit_tests/openvpn/mock_msg.c M tests/unit_tests/openvpn/test_user_pass.c 4 files changed, 68 insertions(+), 2 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/74/474/1 diff --git a/tests/unit_tests/openvpn/input/appears_empty.txt b/tests/unit_tests/openvpn/input/appears_empty.txt new file mode 100644 index 0000000..ffb749a --- /dev/null +++ b/tests/unit_tests/openvpn/input/appears_empty.txt @@ -0,0 +1,3 @@ + + +(contains \t\n\t\n) diff --git a/tests/unit_tests/openvpn/input/empty.txt b/tests/unit_tests/openvpn/input/empty.txt new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/tests/unit_tests/openvpn/input/empty.txt diff --git a/tests/unit_tests/openvpn/mock_msg.c b/tests/unit_tests/openvpn/mock_msg.c index d74efaa..2fcad9d 100644 --- a/tests/unit_tests/openvpn/mock_msg.c +++ b/tests/unit_tests/openvpn/mock_msg.c @@ -38,7 +38,6 @@ #include "error.h" unsigned int x_debug_level = 0; /* Default to (almost) no debugging output */ -bool fatal_error_triggered = false; void mock_set_debug_level(int level) @@ -58,11 +57,14 @@ { if (flags & M_FATAL) { - fatal_error_triggered = true; printf("FATAL ERROR:"); } vprintf(format, arglist); printf("\n"); + if (flags & M_FATAL) + { + mock_assert(false, "FATAL ERROR", __FILE__, __LINE__); + } } void diff --git a/tests/unit_tests/openvpn/test_user_pass.c b/tests/unit_tests/openvpn/test_user_pass.c index e468d3f..600ec80 100644 --- a/tests/unit_tests/openvpn/test_user_pass.c +++ b/tests/unit_tests/openvpn/test_user_pass.c @@ -172,6 +172,24 @@ reset_user_pass(&up); + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + /*FIXME: silently removes control characters but does not error out */ + assert_true(get_user_pass_cr(&up, "\t\n\t", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, ""); + assert_string_equal(up.password, ""); + + reset_user_pass(&up); + + /* Test Assertion */ + /*FIXME: query_user_exec() called even though nothing queued */ + /*FIXME? username error thrown very late in stdin handling */ + will_return(query_user_exec_builtin, true); + expect_assert_failure(get_user_pass_cr(&up, "\nipassword\n", "UT", flags, NULL)); + + reset_user_pass(&up); + expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT Password:"); will_return(query_user_exec_builtin, "cpassword"); will_return(query_user_exec_builtin, true); @@ -223,6 +241,15 @@ reset_user_pass(&up); + expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT Username:"); + expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT Password:"); + will_return(query_user_exec_builtin, ""); + will_return(query_user_exec_builtin, "cpassword"); + will_return(query_user_exec_builtin, true); + expect_assert_failure(get_user_pass_cr(&up, "stdin", "UT", flags, NULL)); + + reset_user_pass(&up); + flags |= GET_USER_PASS_PASSWORD_ONLY; expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT Password:"); will_return(query_user_exec_builtin, "cpassword"); @@ -231,6 +258,18 @@ assert_true(up.defined); assert_string_equal(up.username, "user"); assert_string_equal(up.password, "cpassword"); + + reset_user_pass(&up); + + flags |= GET_USER_PASS_PASSWORD_ONLY; + expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT Password:"); + will_return(query_user_exec_builtin, ""); + will_return(query_user_exec_builtin, true); + /*FIXME? does not error out on empty password */ + assert_true(get_user_pass_cr(&up, "stdin", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "user"); + assert_string_equal(up.password, ""); } static void @@ -254,6 +293,17 @@ reset_user_pass(&up); + snprintf(authfile, PATH_MAX, "%s/%s", srcdir, "input/appears_empty.txt"); + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + /*FIXME? does not error out */ + assert_true(get_user_pass_cr(&up, authfile, "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, ""); + assert_string_equal(up.password, ""); + + reset_user_pass(&up); + snprintf(authfile, PATH_MAX, "%s/%s", srcdir, "input/user_only.txt"); expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT Password:"); will_return(query_user_exec_builtin, "cpassword"); @@ -266,6 +316,11 @@ reset_user_pass(&up); + snprintf(authfile, PATH_MAX, "%s/%s", srcdir, "input/empty.txt"); + expect_assert_failure(get_user_pass_cr(&up, authfile, "UT", flags, NULL)); + + reset_user_pass(&up); + flags |= GET_USER_PASS_PASSWORD_ONLY; snprintf(authfile, PATH_MAX, "%s/%s", srcdir, "input/user_only.txt"); /*FIXME: query_user_exec() called even though nothing queued */ @@ -274,6 +329,12 @@ assert_true(up.defined); assert_string_equal(up.username, "user"); assert_string_equal(up.password, "fuser"); + + reset_user_pass(&up); + + flags |= GET_USER_PASS_PASSWORD_ONLY; + snprintf(authfile, PATH_MAX, "%s/%s", srcdir, "input/empty.txt"); + expect_assert_failure(get_user_pass_cr(&up, authfile, "UT", flags, NULL)); } const struct CMUnitTest user_pass_tests[] = {