From patchwork Sun Oct 1 18:02:50 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: 3371 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7300:7902:b0:d7:3b0f:3938 with SMTP id c2csp926791dyi; Sun, 1 Oct 2023 11:03:57 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGT+2V8HfPDl0YWDU5uZ+2K1x//xANG2SsdYw1GdbV8jLjfFAtiwYSLNzERDZO0skz/Uflg X-Received: by 2002:a05:6870:561e:b0:1d5:53c0:c1fe with SMTP id m30-20020a056870561e00b001d553c0c1femr10529674oao.3.1696183436744; Sun, 01 Oct 2023 11:03:56 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1696183436; cv=none; d=google.com; s=arc-20160816; b=ldUelJ9wnKUY18fuAdnEbmSJ1KUiD/sCrqL6pl8gDs5MC8j8RiPsqTfmxsH7LjIqFA k5hncivch/D4tw3Epb8u2wvqUcIP4kcjZ1BBcy7YSBHTDJyp4BdxIOYyASt6ZXogF9HR SDFLDB4Dj9I/dVlTPj0APbGIW3TJzA9HmpLai1vGwskH8ESbm2/kAgc8gK745TMSSbX4 WZ0x0gTVwug/uJWSmXiQEMeATnhebTLZ7Lm4BSWxGS5jyR8HrrJSnVszVQ+vMg91J1RD cemDfzfZPSLrq0Rqvemfyy/mZIxNxQj0gLEnZbThX7ANh8jfUUcHad3TYgHIfbQMi8JH SWVw== 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=upRJa8ckZ1wYS/LCDXZDDwj4SCYY+MgyDXXUXhOdSXA=; fh=lm0MLPW7DntlrDqRECIiC9JlE1uPxhepE0URYHIf+eE=; b=ck0q+Gwk5Rfp8mwuDbmxMCQfm116Udh8kKdl4NHVQ0z2QZQsRtsCZV7HkF1nr6EYr0 sfRRwyT2ULDqM3ZsApEw2v4mXV9lzMDULU9OpNMvClbvl4GS8xMmy7nyqOra9UKlc/6W jwrPFQPB5Qi8BOCnW8yG6LJbI1skhKDdhLBb0RKxYF5+pKk4Of56I0x6p/+30DcUufcU YTvnQg1bZ7+vQniki10ztqi+V0Ummpyop+dNqkQFTMgmM/+yqnyvuxL2yecL4K1QXNq3 FU19OVi9zQRYyI6ZPBAWXg8ZXaL8TDMAth3yNAHaduB+FoZseP/GurJFgSuEZBi78GVc X+rg== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=W9rsvAv5; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=UyqfFEHw; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=GBrwKPif; 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 t12-20020a6549cc000000b00577f7b6e7b4si24870125pgs.624.2023.10.01.11.03.56 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sun, 01 Oct 2023 11:03:56 -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=W9rsvAv5; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=UyqfFEHw; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=GBrwKPif; 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 1qn0mX-0000w3-CK; Sun, 01 Oct 2023 18:03:03 +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 1qn0mV-0000vx-DI for openvpn-devel@lists.sourceforge.net; Sun, 01 Oct 2023 18:03:01 +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=dxhvAtDwWVOkixEWyXjBjfnnu7uBIegwhF0LxMRTZcY=; b=W9rsvAv59sGIsf+7gVgXTJXOhh 7G2Dgpee0q/W7JiHPi5tXgLFSX43biTt+soTYRA4sHsMHkM5WSzZIEOqhh9aJbD7mcsIBdTVQL1nC gqrMXIeUVTKCXS+2ABBMDQhqjOdJFhHcfYcpuddeeR2VO9B+U/MI+5CuX3YdoCu4Ww6g=; 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=dxhvAtDwWVOkixEWyXjBjfnnu7uBIegwhF0LxMRTZcY=; b=U yqfFEHwaEGXafmkpWB6w5D3WYHwHj/Zaue6hFvGR4u+SU7AEuEgFZVuJun4/3sUhoai1oQ9OSt6m3 81T2ixYyboeSXjcHZe2iwexlkSWK7YRL/bRZ3HLhJH1XVwWOR7b03iEF63eOGe1+XAKRI0s9CSXo3 7mv4ErvExxv+J1wY=; Received: from mail-wr1-f52.google.com ([209.85.221.52]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1qn0mS-00EbJE-Ks for openvpn-devel@lists.sourceforge.net; Sun, 01 Oct 2023 18:03:01 +0000 Received: by mail-wr1-f52.google.com with SMTP id ffacd0b85a97d-32329d935d4so10506686f8f.2 for ; Sun, 01 Oct 2023 11:03:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=openvpn.net; s=google; t=1696183374; x=1696788174; 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=dxhvAtDwWVOkixEWyXjBjfnnu7uBIegwhF0LxMRTZcY=; b=GBrwKPifNEwYT2uOPWzsU2w6hsxdEk1HhjgnGbQP8IZzZrA16yGCJCJKHr4mRnh6NU kPU+sM6RPTpmyvQau+e6L/PChaX7bgF+q4baKTx8tLbU3wX/Juc0QupUjkZfwDtpBjCi KftnGQeCwuze0PtbprofzgdGeE99VuFl/8Dvti1oqsCQXbWeAvQguB8IUkDulndCzRM2 Z7Yhx5w8sf8IvCHMT4nYKjBYHDvRqmZTQE0maCxOW8IdhnKk2fjyM+rzHoBE5wUrR18m X+vBZL/pBHhTbcMk61sy2u4TcdPo7+Iz9KjI0CcKfxehQSbRGUB3uAcUpw6hPPQIAdM9 nj5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696183374; x=1696788174; 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=dxhvAtDwWVOkixEWyXjBjfnnu7uBIegwhF0LxMRTZcY=; b=ffZtXmfx+iI0GcSpiPSMfVjziRDJ7NPFD5quwR6nx5+AAGXrS3TwKrm1D4y0w42A3t /iucH63eddX0GP96YqjM5rn5MeWGlO+7p2FMl7sRafGwk3CaCVCG3zKjdCrZNxREWhky vna8yCYSnY9L2CoGH1fdzae2KYxoAkyI0DGWCfMz2hv5NBG6yzzd+d3QvDOVnygexPNA /YjZ1SC8pDYK1JbQEtVECizmdubxa4sE1cS+rcjnKrmIHsNxz85j7c+x2jmNIGcmROlA lxb0KkOOawyXKn6mstr74YheCrMiS3uh9LbN4gF6IxtDfNJ5lYBrt5JSxurCruQ5R1ev C0aw== X-Gm-Message-State: AOJu0Yz1qQrBYIadWwkvh8To0tUAmB6E/Wqcxm7GycaDEi0iU57YWBr5 qvnvnaEI+NfwJujkpojPKioNgPCj3Ax+5ERYPQs= X-Received: by 2002:a5d:4a08:0:b0:31c:8880:5d0f with SMTP id m8-20020a5d4a08000000b0031c88805d0fmr8273021wrq.11.1696183373620; Sun, 01 Oct 2023 11:02:53 -0700 (PDT) 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 t11-20020a5d42cb000000b0031431fb40fasm5054435wrr.89.2023.10.01.11.02.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 01 Oct 2023 11:02:52 -0700 (PDT) From: "selvanair (Code Review)" X-Google-Original-From: "selvanair (Code Review)" X-Gerrit-PatchSet: 1 Date: Sun, 1 Oct 2023 18:02:50 +0000 To: flichtenheld Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange X-Gerrit-Change-Id: Ic7ec25ac0503a91d5869b8da966d0065f264af22 X-Gerrit-Change-Number: 363 X-Gerrit-Project: openvpn X-Gerrit-ChangeURL: X-Gerrit-Commit: d8b3a9a1345cb0e1c62a02f9a5b62df45b8850f4 References: Message-ID: MIME-Version: 1.0 User-Agent: Gerrit/3.8.2 X-Spam-Score: -0.7 (/) 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: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit Content analysis details: (-0.7 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [209.85.221.52 listed in list.dnswl.org] -0.5 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [209.85.221.52 listed in wl.mailspike.net] 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.0 SPF_PASS SPF: sender matches SPF record 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_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.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.0 T_KAM_HTML_FONT_INVALID Test for Invalidly Named or Formatted Colors in HTML X-Headers-End: 1qn0mS-00EbJE-Ks Subject: [Openvpn-devel] [S] Change in openvpn[master]: Log OpenSSL errors on failure to set certificate 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: selva.nair@gmail.com, openvpn-devel@lists.sourceforge.net, frank@lichtenheld.com Cc: openvpn-devel Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1778577243051585351?= X-GMAIL-MSGID: =?utf-8?q?1778577243051585351?= X-getmail-filter-classifier: gerrit message type newchange Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/363?usp=email to review the following change. Change subject: Log OpenSSL errors on failure to set certificate ...................................................................... Log OpenSSL errors on failure to set certificate Currently we log a bogus error message saying private key password verification failed when SSL_CTX_use_cert_and_key() fails in pkcs11_openssl.c. Instead print OpenSSL error queue and exit promptly. Also log OpenSSL errors when SSL_CTX_use_certiifcate() fails in cryptoapi.c and elsewhere. Such logging could be useful especially when the ceritficate is rejected by OpenSSL due to stricter security restrictions in recent versions of the library. Change-Id: Ic7ec25ac0503a91d5869b8da966d0065f264af22 Signed-off-by: Selva Nair --- M src/openvpn/cryptoapi.c M src/openvpn/pkcs11_openssl.c M src/openvpn/ssl_openssl.c M tests/unit_tests/openvpn/test_cryptoapi.c M tests/unit_tests/openvpn/test_pkcs11.c 5 files changed, 30 insertions(+), 2 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/63/363/1 diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c index 3b92e48..f7e5b67 100644 --- a/src/openvpn/cryptoapi.c +++ b/src/openvpn/cryptoapi.c @@ -51,6 +51,7 @@ #include "openssl_compat.h" #include "win32.h" #include "xkey_common.h" +#include "crypto_openssl.h" #ifndef HAVE_XKEY_PROVIDER @@ -505,6 +506,7 @@ if (SSL_CTX_use_certificate(ssl_ctx, cert) && SSL_CTX_use_PrivateKey(ssl_ctx, privkey)) { + crypto_print_openssl_errors(M_WARN); ret = 1; } diff --git a/src/openvpn/pkcs11_openssl.c b/src/openvpn/pkcs11_openssl.c index 40080ef..aa0819f 100644 --- a/src/openvpn/pkcs11_openssl.c +++ b/src/openvpn/pkcs11_openssl.c @@ -302,7 +302,8 @@ if (!SSL_CTX_use_cert_and_key(ctx->ctx, x509, pkey, NULL, 0)) { - msg(M_WARN, "PKCS#11: Failed to set cert and private key for OpenSSL"); + crypto_print_openssl_errors(M_WARN); + msg(M_FATAL, "PKCS#11: Failed to set cert and private key for OpenSSL"); goto cleanup; } ret = 1; @@ -369,7 +370,8 @@ if (!SSL_CTX_use_certificate(ssl_ctx->ctx, x509)) { - msg(M_WARN, "PKCS#11: Cannot set certificate for openssl"); + crypto_print_openssl_errors(M_WARN); + msg(M_FATAL, "PKCS#11: Cannot set certificate for openssl"); goto cleanup; } ret = 0; diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 0b310de..b5cc9a7 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -857,6 +857,7 @@ /* Load Certificate */ if (!SSL_CTX_use_certificate(ctx->ctx, cert)) { + crypto_print_openssl_errors(M_WARN); crypto_msg(M_FATAL, "Cannot use certificate"); } @@ -1007,6 +1008,7 @@ end: if (!ret) { + crypto_print_openssl_errors(M_WARN); if (cert_file_inline) { crypto_msg(M_FATAL, "Cannot load inline certificate file"); diff --git a/tests/unit_tests/openvpn/test_cryptoapi.c b/tests/unit_tests/openvpn/test_cryptoapi.c index 008f41c..d90bfc3 100644 --- a/tests/unit_tests/openvpn/test_cryptoapi.c +++ b/tests/unit_tests/openvpn/test_cryptoapi.c @@ -58,6 +58,17 @@ return NULL; } +/* replacement for crypto_print_openssl_errors() */ +void +crypto_print_openssl_errors(const unsigned int flags) +{ + unsigned long e; + while ((e = ERR_get_error())) + { + msg(flags, "OpenSSL error %lu: %s\n", e, ERR_error_string(e, NULL)); + } +} + /* tls_libctx is defined in ssl_openssl.c which we do not want to compile in */ OSSL_LIB_CTX *tls_libctx; diff --git a/tests/unit_tests/openvpn/test_pkcs11.c b/tests/unit_tests/openvpn/test_pkcs11.c index 235cc43..b6c130e 100644 --- a/tests/unit_tests/openvpn/test_pkcs11.c +++ b/tests/unit_tests/openvpn/test_pkcs11.c @@ -44,6 +44,17 @@ struct management *management; /* global */ +/* replacement for crypto_print_openssl_errors() */ +void +crypto_print_openssl_errors(const unsigned int flags) +{ + unsigned long e; + while ((e = ERR_get_error())) + { + msg(flags, "OpenSSL error %lu: %s\n", e, ERR_error_string(e, NULL)); + } +} + /* stubs for some unused functions instead of pulling in too many dependencies */ int parse_line(const char *line, char **p, const int n, const char *file,