From patchwork Fri Sep 29 06:25:23 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steffan Karger X-Patchwork-Id: 5 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director1.mail.ord1d.rsapps.net ([172.30.157.6]) by backend31.mail.ord1d.rsapps.net (Dovecot) with LMTP id ngVZBCh8zlk5egAAgoeIoA for ; Fri, 29 Sep 2017 13:00:24 -0400 Received: from proxy13.mail.ord1d.rsapps.net ([172.30.157.58]) by director1.mail.ord1d.rsapps.net (Dovecot) with LMTP id xNNYO2hZzlkxagAANGzteQ ; Fri, 29 Sep 2017 13:00:24 -0400 Received: from smtp30.gate.ord1d ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy13.mail.ord1d.rsapps.net (Dovecot) with LMTP id flKrDRB8zlmgRgAAgjf6aA ; Fri, 29 Sep 2017 13:00:24 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-MessageSniffer-Scan-Result: 0 X-MessageSniffer-Rules: 0-298-1143-1360-w 0-298-1143-1724-w 0-298-0-15535-f X-CMAE-Scan-Result: 0 X-CNFS-Analysis: v=2.2 cv=K5dgJGeI c=1 sm=1 tr=0 a=Q8DxjiC8O3VT/NpP1XjEZQ==:117 a=Q8DxjiC8O3VT/NpP1XjEZQ==:17 a=kj9zAlcOel0A:10 a=MKtGQD3n3ToA:10 a=1oJP67jkp3AA:10 a=2JCJgTwv5E4A:10 a=9hBjR9qptiMA:10 a=WiVod9pSvdkA:10 a=ZZnuYtJkoWoA:10 a=Y4BXLp1PAAAA:8 a=9sSjY8p1AAAA:8 a=P_JWiMecAAAA:8 a=FP58Ms26AAAA:8 a=PxNDMBkARldKargVxEsA:9 a=CjuIK1q_8ugA:10 a=-FEs8UIgK8oA:10 a=NWVoK91CQyQA:10 a=byfke0vc68LhOSAAcvKj:22 a=ub54wNWiXv_DzeFsgEJW:22 a=D0-HAvA3Hk9NMREbgwuX:22 X-Orig-To: justin@openvpn.net X-Originating-Ip: [216.34.181.88] Authentication-Results: smtp30.gate.ord1d.rsapps.net; iprev=pass policy.iprev="216.34.181.88"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dkim=fail (signature verification failed) header.d=karger-me.20150623.gappssmtp.com; dmarc=none (p=nil; dis=none) header.from=karger.me X-Classification-ID: aeaf9c62-a537-11e7-9df0-5254001e8e38-1-1 Received: from [216.34.181.88] ([216.34.181.88:16948] helo=lists.sourceforge.net) by smtp30.gate.ord1d.rsapps.net (envelope-from ) (ecelerity 4.2.1.56364 r(Core:4.2.1.14)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id BB/A2-22656-72C7EC95; Fri, 29 Sep 2017 13:00:23 -0400 Received: from localhost ([127.0.0.1] helo=sfs-ml-1.v29.ch3.sourceforge.com) by sfs-ml-1.v29.ch3.sourceforge.com with esmtp (Exim 4.89) (envelope-from ) id 1dxyZr-0001sZ-2X; Fri, 29 Sep 2017 16:55:51 +0000 Received: from sog-mx-4.v43.ch3.sourceforge.com ([172.29.43.194] helo=mx.sourceforge.net) by sfs-ml-1.v29.ch3.sourceforge.com with esmtps (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.89) (envelope-from ) id 1dxyZq-0001sT-AH for openvpn-devel@lists.sourceforge.net; Fri, 29 Sep 2017 16:55:50 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=0LaEBxKKFBwNu033KFxSHDLvoQMOKPUvg3deBBOsfM4=; b=BcF5innh471/9HpgFlJzai66iXMvmznXIxfjpHNdneQdYrfutZDZY4EDRJDJECCU3h6Gp2XEbcWPL3c8sCdPpQIDJ1uExoyvz9dY1kuHdLBC1A/aatgtDitxA8gzgv0k63yc8nIdKJqAe+B1A+WsBwDOrEBUn9vyQMBr/9Pesy4=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=0LaEBxKKFBwNu033KFxSHDLvoQMOKPUvg3deBBOsfM4=; b=ZuIDyPLUgwLMT6bfWqzKW2XqwVzzhB8Bdz6fZBW8pEHviyfEPJm9QWUwIJlS46WD/ZiEn7qZIJq/wRWW9TLL0iWSiM7PG/znR+c9qhkV+dp40TQPmNoaz1Ak1Uw2q47DZLusZM+Bj3gBu+0S1ZjIwgsBhnHkEPySk1/ZSer1h/g=; Received-SPF: pass (sog-mx-4.v43.ch3.sourceforge.com: domain of karger.me designates 209.85.192.195 as permitted sender) client-ip=209.85.192.195; envelope-from=steffan@karger.me; helo=mail-pf0-f195.google.com; Received: from mail-pf0-f195.google.com ([209.85.192.195]) by sog-mx-4.v43.ch3.sourceforge.com with esmtps (TLSv1:AES128-SHA:128) (Exim 4.76) id 1dxyZo-0001Tq-0K for openvpn-devel@lists.sourceforge.net; Fri, 29 Sep 2017 16:55:50 +0000 Received: by mail-pf0-f195.google.com with SMTP id h4so134057pfk.0 for ; Fri, 29 Sep 2017 09:55:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=karger-me.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=0LaEBxKKFBwNu033KFxSHDLvoQMOKPUvg3deBBOsfM4=; b=Uq5p4kFx1WANopaZGlxlzePYcyPd7QwsiRjQ89VANwlIZikYhigV1C9N9oQW4zYx6G RSSmzouRUOIpqV7hG2UuSuQeiH+AoZ0UtBwzp/3/wbe9GYL1vWhJKKUL4gK6cTO9+/4Y Ocz27GYCSccHzGM7PaFz44S57alBu3pKUAxTCykX9y08/FOaQJZZbOuiDdtdpCoOWUf3 bRCpNb6Wv2niJ8Axm9KALOSizknhrg4QQR5XaEQVXphs9b1o/+/RfpTId5FwnWXdjMRr 3cxDS+ofU6tBxMNJgrDfHmma/gswQf4x5CZsUX2I0xt/FRctM9heD5eDZDHv2ngqpPg2 xw9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=0LaEBxKKFBwNu033KFxSHDLvoQMOKPUvg3deBBOsfM4=; b=MdH0Q37c+DEQiloEteVU0v9z7Kxzl7eQ1hx1GboEKGLXXgrvpz/7HqBHqB8WoE8Ifp YSZa4R7xAz/MSGJrSVlJ14blErUauS5Emm0BOiVktK4zScjpje9sckiJtIxe2nn36dVU O08X3SXMu8md42+sLIrRiDaulkce+OS68uWfDY/DWz8pwcq7vIQJ6rhO32fWaCjOmVjR faXH3uvEJUBvNHiNGQOJKFZ/rU/1oMPI94/1TTBP6hc6eY/8CR5bwuK4T40H1zvPKEHf BuGmVnzlSjzfpibh6YdOA4zj/xoZQUZbLe5lXKS8oFicI2o9ceJckE0Yb1/sPyD5CMyC U+7A== X-Gm-Message-State: AHPjjUgo7aFDV6Ju7CAbtGqdOHOvVwbZdKGUfqaLionh1sizw9VtMTiB Xa36YByhcoc+zYd4r1AB5meQTxPZtRU= X-Google-Smtp-Source: AOwi7QDndK1y2hiiXGKuzhYO0GbyMzp0BdhNADklKIJcECvBao0wGFuSnuscv8FKhriNCaNTmWobHw== X-Received: by 10.98.155.76 with SMTP id r73mr8238154pfd.182.1506702329633; Fri, 29 Sep 2017 09:25:29 -0700 (PDT) Received: from localhost.localdomain ([122.147.252.5]) by smtp.gmail.com with ESMTPSA id b65sm7581695pfj.97.2017.09.29.09.25.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 29 Sep 2017 09:25:29 -0700 (PDT) From: Steffan Karger To: openvpn-devel@lists.sourceforge.net Date: Fri, 29 Sep 2017 18:25:23 +0200 Message-Id: <20170929162523.16595-1-steffan@karger.me> X-Mailer: git-send-email 2.11.0 In-Reply-To: References: X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, no trust [209.85.192.195 listed in list.dnswl.org] -1.5 SPF_CHECK_PASS SPF reports sender host as permitted sender for sender-domain 0.5 RCVD_IN_SORBS_SPAM RBL: SORBS: sender is a spam source [209.85.192.195 listed in dnsbl.sorbs.net] -0.0 SPF_PASS SPF: sender matches SPF record 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature X-Headers-End: 1dxyZo-0001Tq-0K Subject: [Openvpn-devel] [PATCH 1/2 v2] Don't throw fatal errors from create_temp_file() 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: , MIME-Version: 1.0 Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox From: Steffan Karger This function is called in response to connecting clients, and can fail when I/O fails for some (possibly temporary) reason. In such cases we should not exit the process, but just reject the connecting client. This commit changes the function to actually return NULL on errors, and (where needed) changes the callers to check for and handle errors. Since the tls-crypt-v2 metadata code also calls create_temp_file() when clients connect, I consider this a prerequisite for tls-crypt-v2. Signed-off-by: Steffan Karger --- v2: - Move '||' to new line to conform to Knuth-style wrapping - Re-add explicit warning message when pf plugin init fails src/openvpn/misc.c | 6 +++--- src/openvpn/ssl_verify.c | 32 +++++++++++++++++++++----------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index 33262320..90632706 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -740,7 +740,7 @@ create_temp_file(const char *directory, const char *prefix, struct gc_arena *gc) retfname = gen_path(directory, BSTR(&fname), gc); if (!retfname) { - msg(M_FATAL, "Failed to create temporary filename and path"); + msg(M_WARN, "Failed to create temporary filename and path"); return NULL; } @@ -755,14 +755,14 @@ create_temp_file(const char *directory, const char *prefix, struct gc_arena *gc) else if (fd == -1 && errno != EEXIST) { /* Something else went wrong, no need to retry. */ - msg(M_FATAL | M_ERRNO, "Could not create temporary file '%s'", + msg(M_WARN | M_ERRNO, "Could not create temporary file '%s'", retfname); return NULL; } } while (attempts < 6); - msg(M_FATAL, "Failed to create temporary file after %i attempts", attempts); + msg(M_WARN, "Failed to create temporary file after %i attempts", attempts); return NULL; } diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 9cd36d7a..de54fb74 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -547,14 +547,14 @@ verify_cert_export_cert(openvpn_x509_cert_t *peercert, const char *tmp_dir, stru FILE *peercert_file; const char *peercert_filename = ""; - if (!tmp_dir) + /* create tmp file to store peer cert */ + if (!tmp_dir + || !(peercert_filename = create_temp_file(tmp_dir, "pcf", gc))) { + msg (M_WARN, "Failed to create peer cert file"); return NULL; } - /* create tmp file to store peer cert */ - peercert_filename = create_temp_file(tmp_dir, "pcf", gc); - /* write peer-cert in tmp-file */ peercert_file = fopen(peercert_filename, "w+"); if (!peercert_file) @@ -589,10 +589,13 @@ verify_cert_call_command(const char *verify_command, struct env_set *es, if (verify_export_cert) { - if ((tmp_file = verify_cert_export_cert(cert, verify_export_cert, &gc))) + tmp_file = verify_cert_export_cert(cert, verify_export_cert, &gc); + if (!tmp_file) { - setenv_str(es, "peer_cert", tmp_file); + ret = false; + goto cleanup; } + setenv_str(es, "peer_cert", tmp_file); } argv_parse_cmd(&argv, verify_command); @@ -609,6 +612,7 @@ verify_cert_call_command(const char *verify_command, struct env_set *es, } } +cleanup: gc_free(&gc); argv_reset(&argv); @@ -879,21 +883,21 @@ key_state_rm_auth_control_file(struct key_state *ks) } } -static void +static bool key_state_gen_auth_control_file(struct key_state *ks, const struct tls_options *opt) { struct gc_arena gc = gc_new(); - const char *acf; key_state_rm_auth_control_file(ks); - acf = create_temp_file(opt->tmp_dir, "acf", &gc); + const char *acf = create_temp_file(opt->tmp_dir, "acf", &gc); if (acf) { ks->auth_control_file = string_alloc(acf, NULL); setenv_str(opt->es, "auth_control_file", ks->auth_control_file); - } /* FIXME: Should have better error handling? */ + } gc_free(&gc); + return acf; } static unsigned int @@ -1184,7 +1188,12 @@ verify_user_pass_plugin(struct tls_session *session, const struct user_pass *up, #ifdef PLUGIN_DEF_AUTH /* generate filename for deferred auth control file */ - key_state_gen_auth_control_file(ks, session->opt); + if (!key_state_gen_auth_control_file(ks, session->opt)) + { + msg (D_TLS_ERRORS, "TLS Auth Error (%s): " + "could not create deferred auth control file", __func__); + goto cleanup; + } #endif /* call command */ @@ -1209,6 +1218,7 @@ verify_user_pass_plugin(struct tls_session *session, const struct user_pass *up, msg(D_TLS_ERRORS, "TLS Auth Error (verify_user_pass_plugin): peer provided a blank username"); } +cleanup: return retval; }