From patchwork Tue Apr 27 01:03:00 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gert Doering X-Patchwork-Id: 1777 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director9.mail.ord1d.rsapps.net ([172.31.255.6]) by backend30.mail.ord1d.rsapps.net with LMTP id AD4xBKLvh2ARDgAAIUCqbw (envelope-from ) for ; Tue, 27 Apr 2021 07:04:02 -0400 Received: from proxy12.mail.iad3b.rsapps.net ([172.31.255.6]) by director9.mail.ord1d.rsapps.net with LMTP id iK7jA6Lvh2DGQAAAalYnBA (envelope-from ) for ; Tue, 27 Apr 2021 07:04:02 -0400 Received: from smtp34.gate.iad3b ([172.31.255.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy12.mail.iad3b.rsapps.net with LMTPS id QJrOOKHvh2AeWAAAEsW3lA (envelope-from ) for ; Tue, 27 Apr 2021 07:04:01 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp34.gate.iad3b.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; 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; dmarc=none (p=nil; dis=none) header.from=greenie.muc.de X-Suspicious-Flag: YES X-Classification-ID: 45359af4-a748-11eb-bf1a-5254005e8ddb-1-1 Received: from [216.105.38.7] ([216.105.38.7:54168] helo=lists.sourceforge.net) by smtp34.gate.iad3b.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 05/19-19686-1AFE7806; Tue, 27 Apr 2021 07:04:01 -0400 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.90_1) (envelope-from ) id 1lbLUp-0003Jd-8G; Tue, 27 Apr 2021 11:03:15 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lbLUo-0003JC-4r for openvpn-devel@lists.sourceforge.net; Tue, 27 Apr 2021 11:03:14 +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:References: In-Reply-To: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:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=xveJnvLHtwBcFd7yb0Lm25WAX3jU6a6eZ6RK2Nf8Iss=; b=HWUrRGS9e4btEihApyDSfyi6Qy XU2B/+x7tcaXxslsSLTim+1PETxH7R9pj7yE5gViN37cOUPHdIj+rbQKFRoiYOvZLW+Jnel5tbw0X J2DtW5iIaOvPA7bXGt5r8YtAaXqLOc2oHsjVxDt3Xk2xOli68Y3k6aE5kcSYi05/y3JM=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: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:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=xveJnvLHtwBcFd7yb0Lm25WAX3jU6a6eZ6RK2Nf8Iss=; b=DVDHSSyULDx2gTRBI9xegthbs8 R+RcFgFJ/pkuAXdrwOFcCSoM7hwyCmr6NAYHqp301yAj8QrT0xSRx1PlijKRq/a8QyK3SUY6Ql+tz ZoJH5yiGRsyio0LFUm+dsmgvmzF6MNTNDIXnP50uv42aGUl1HWvEGvRH9gR30FmZkFS4=; Received: from dhcp-174.greenie.muc.de ([193.149.48.174] helo=blue.greenie.muc.de) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.2) id 1lbLUj-00GbLu-0C for openvpn-devel@lists.sourceforge.net; Tue, 27 Apr 2021 11:03:15 +0000 Received: from blue.greenie.muc.de (localhost [127.0.0.1]) by blue.greenie.muc.de (8.15.2/8.14.9) with ESMTP id 13RB31er006961 for ; Tue, 27 Apr 2021 13:03:01 +0200 Received: (from gert@localhost) by blue.greenie.muc.de (8.15.2/8.14.9/Submit) id 13RB31ij006960 for openvpn-devel@lists.sourceforge.net; Tue, 27 Apr 2021 13:03:01 +0200 From: Gert Doering To: openvpn-devel@lists.sourceforge.net Date: Tue, 27 Apr 2021 13:03:00 +0200 Message-Id: <20210427110300.6911-1-gert@greenie.muc.de> X-Mailer: git-send-email 2.26.3 In-Reply-To: <20210426174436.26210-1-gert@greenie.muc.de> References: <20210426174436.26210-1-gert@greenie.muc.de> MIME-Version: 1.0 X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [URIs: muc.de] -0.0 SPF_HELO_PASS SPF: HELO matches SPF record -0.0 SPF_PASS SPF: sender matches SPF record X-Headers-End: 1lbLUj-00GbLu-0C Subject: [Openvpn-devel] [PATCH v2] rewrite parse_hash_fingerprint() 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 The existing code was doing far too much work for too little gain - copying the string segment for scanf(), checking extra for spaces, making the result quite unreadable. Verify each segment with (short-circuited) isxdigit() checks, then feed directly to scanf(), which will stop parsing on ':' or end-of-string. Rewrite error message to differentiate "hash too short" (including number of bytes read) and "hash too long" (it did not terminate when we had enough bytes). While at it, add an option printer for the resulting o->verify_hash list to show_settings(). v2: fix typo in commit message appease whitespace dragon add printing of verify_hash_algo and verify_hash_depth print correct hash length for SHA1 certs fix incorrect assignment to options->verify_hash_algo in c3a7065d5 Signed-off-by: Gert Doering Acked-by: Antonio Quartulli --- src/openvpn/options.c | 69 ++++++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 33 deletions(-) diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 2a5b1393..795a8fc7 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -1082,56 +1082,43 @@ string_substitute(const char *src, int from, int to, struct gc_arena *gc) static struct verify_hash_list * parse_hash_fingerprint(const char *str, int nbytes, int msglevel, struct gc_arena *gc) { - int i; + int i = 0; const char *cp = str; struct verify_hash_list *ret; ALLOC_OBJ_CLEAR_GC(ret, struct verify_hash_list, gc); - char term = 1; + char term = 0; int byte; - char bs[3]; - for (i = 0; i < nbytes; ++i) + while (*cp && i < nbytes) { - if (strlen(cp) < 2) + /* valid segments consist of exactly two hex digits, then ':' or EOS */ + if (!isxdigit(cp[0]) + || !isxdigit(cp[1]) + || (cp[2] != ':' && cp[2] != '\0') + || sscanf(cp, "%x", &byte) != 1) { msg(msglevel, "format error in hash fingerprint: %s", str); + break; } - bs[0] = *cp++; - bs[1] = *cp++; - bs[2] = 0; - /* the format string "%x" passed to sscanf will ignore any space and - * will still try to parse the other character. However, this is not - * expected format for a fingerprint, therefore explictly check for - * blanks in the string and error out if any is found - */ - if (bs[0] == ' ' || bs[1] == ' ') - { - msg(msglevel, "format error in hash fingerprint unexpected blank: %s", - str); - } + ret->hash[i++] = (uint8_t)byte; - byte = 0; - if (sscanf(bs, "%x", &byte) != 1) - { - msg(msglevel, "format error in hash fingerprint hex byte: %s", str); - } - ret->hash[i] = (uint8_t)byte; - term = *cp++; - if (term != ':' && term != 0) - { - msg(msglevel, "format error in hash fingerprint delimiter: %s", str); - } - if (term == 0) + term = cp[2]; + if (term == '\0') { break; } + cp += 3; } - if (term != 0 || i != nbytes-1) + if (i < nbytes) { - msg(msglevel, "hash fingerprint is different length than expected (%d bytes): %s", nbytes, str); + msg(msglevel, "hash fingerprint is wrong length - expected %d bytes, got %d: %s", nbytes, i, str); + } + else if (term != '\0') + { + msg(msglevel, "hash fingerprint too long - expected only %d bytes: %s", nbytes, str); } return ret; } @@ -1791,6 +1778,23 @@ show_settings(const struct options *o) } } SHOW_STR(remote_cert_eku); + if (o->verify_hash) + { + SHOW_INT(verify_hash_algo); + SHOW_INT(verify_hash_depth); + struct gc_arena gc = gc_new(); + struct verify_hash_list *hl = o->verify_hash; + int digest_len = (o->verify_hash_algo == MD_SHA1) ? SHA_DIGEST_LENGTH : + SHA256_DIGEST_LENGTH; + while (hl) + { + char *s = format_hex_ex(hl->hash, digest_len, 0, + 1, ":", &gc); + SHOW_PARM(verify_hash, s, "%s"); + hl = hl->next; + } + gc_free(&gc); + } SHOW_INT(ssl_flags); SHOW_INT(tls_timeout); @@ -8190,7 +8194,6 @@ add_option(struct options *options, if ((!p[2] && !is_inline) || (p[2] && streq(p[2], "SHA1"))) { options->verify_hash_algo = MD_SHA1; - options->verify_hash_algo = SHA_DIGEST_LENGTH; digest_len = SHA_DIGEST_LENGTH; } else if (p[2] && !streq(p[2], "SHA256"))