From patchwork Fri Jan 26 04:53:10 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Selva Nair X-Patchwork-Id: 217 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director3.mail.ord1d.rsapps.net ([172.30.191.6]) by backend31.mail.ord1d.rsapps.net (Dovecot) with LMTP id xMEuFlRPa1rUFQAAgoeIoA for ; Fri, 26 Jan 2018 10:55:00 -0500 Received: from proxy16.mail.ord1d.rsapps.net ([172.30.191.6]) by director3.mail.ord1d.rsapps.net (Dovecot) with LMTP id i7QEKFRPa1pyZQAAkXNnRw ; Fri, 26 Jan 2018 10:55:00 -0500 Received: from smtp17.gate.ord1d ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy16.mail.ord1d.rsapps.net (Dovecot) with LMTP id 41GLFVRPa1o/bQAAetu3IA ; Fri, 26 Jan 2018 10:55:00 -0500 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.34.181.88] Authentication-Results: smtp17.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=gmail.com; dmarc=fail (p=none; dis=none) header.from=gmail.com X-Classification-ID: 438033c0-02b1-11e8-b9ca-5254008de1cb-1-1 Received: from [216.34.181.88] ([216.34.181.88:45678] helo=lists.sourceforge.net) by smtp17.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 99/6C-21774-45F4B6A5; Fri, 26 Jan 2018 10:55:00 -0500 Received: from localhost ([127.0.0.1] helo=sfs-ml-2.v29.ch3.sourceforge.com) by sfs-ml-2.v29.ch3.sourceforge.com with esmtp (Exim 4.89) (envelope-from ) id 1ef6KY-00081l-Br; Fri, 26 Jan 2018 15:54:18 +0000 Received: from sfi-mx-1.v28.ch3.sourceforge.com ([172.29.28.191] helo=mx.sourceforge.net) by sfs-ml-2.v29.ch3.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.89) (envelope-from ) id 1ef6KW-00081e-Tf for openvpn-devel@lists.sourceforge.net; Fri, 26 Jan 2018 15:54:16 +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:Sender:Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding: 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=LnjpamFmCzH/KlWrlby5WveyaUgXAfQhyvYZqicVhEE=; b=jIgEamnjtq8LG9Xj3Gy6mcURTg NTc471Cdlib78Cd5+ElE2oOg5HxL+y1sMzzhc7g+3lLs9KT9kS/ofVSzY1UA7BhtyCS4fiDaSSsvo fIWMB6LlFrDfD2oXwYjI/bCOYcDRltytO5rLNWRV2eFJTXm3aq4OhDjo/UvVWSVxMIAQ=; 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:Sender:Reply-To :MIME-Version:Content-Type:Content-Transfer-Encoding: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=LnjpamFmCzH/KlWrlby5WveyaUgXAfQhyvYZqicVhEE=; b=MiMiFkV5JbdiT9hx+nL0DFxzas 5aEgPMxa6Z/Xz6fQ0s5XK6tSsgkjN5SpvjGXDe720pJhWGDFWt0NUPuWN1OK2WfQGRryEXQuDKnyG avzEpe+oesazz/+OLYHj0UD8nBnwgbceF17ORpvju6BYMopXR+SSNkydk3PGqfFJCwfo=; Received: from mail-io0-f195.google.com ([209.85.223.195]) by sfi-mx-1.v28.ch3.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.89) id 1ef6KV-0003rl-RE for openvpn-devel@lists.sourceforge.net; Fri, 26 Jan 2018 15:54:16 +0000 Received: by mail-io0-f195.google.com with SMTP id z6so863489iob.11 for ; Fri, 26 Jan 2018 07:54:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=LnjpamFmCzH/KlWrlby5WveyaUgXAfQhyvYZqicVhEE=; b=QYtO24aFZb8Kg70O/uCp0kmZtxgpNzSnnzOyCYvImU+pUQkh1l7c5dt9al7moBuilC Omt2dozkn9Q14VKRJEgMwvNZ5TL92ggIYXOhgps4U9ivQOdn4gsirWRVV0vrRhnD9gsz pH8OPMXBgyQg6yH8gJJ2/ha+odg1XkqlR/ZDGSoUF3o4msL0/FGG3EVSXOIPI/58RyX5 Two9ZV1qHeXX0UQMIsWfwhXpBc7SXjOOXlURvi6QHQ734SwyWeTAtReOq5vr3SZC+fo4 TFtj96GLXEYWbMbB3PCSD8wwggCZQR0mvwbB1KTjvqQAKuelzj4ZBYJMEy9X2fiU+1wO FGcA== 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=LnjpamFmCzH/KlWrlby5WveyaUgXAfQhyvYZqicVhEE=; b=oSllG7sHMBG362BMFwwJn+blKbXXUGzRw8BMPaNMId2zBbt7w8AjOMHHO+v7SZG+rd jYmr6qEMtVXvebKUOKLXCKAAmwx8r+Pbdhy5J1n8SK+qFFWj3PRkNkAHdc/M2/YVEdPO DD9qfVQh0KLtK38FqwplKw6F0hXUeImzx60pKNoO72GOUcfppwSSaowrXzRReG/U3BVU YgXZswDC87EIc6ehp81K9PlMubQ10poYwfWG18ZdD4qUY4qiKuIiNpBSdmeUq2h6W1go O5w4108Ca6uU6SSYcUQL5P25q+sqb5KLZW+RLyDqUu1LIg9iiRh7mD99P3Nq1Idopb2A S0VA== X-Gm-Message-State: AKwxyteBBEAMxf7hp+dkE4ieCLZOpvPh6XAMB096I68o5iREcdc1mFkJ tmIfxGZwCM+i0VmO2oJcqZ6qCpS9 X-Google-Smtp-Source: AH8x2245H6MEv2jW1BEe/FL2X1fd0VrppeyDELYpyXr2aU9L96tNaYpDiMWOPiE6XzR7CkRy1DI2Fw== X-Received: by 10.107.164.70 with SMTP id n67mr16999656ioe.88.1516982050254; Fri, 26 Jan 2018 07:54:10 -0800 (PST) Received: from saturn.home.sansel.ca (CPE40167ea0e1c2-CM788df74daaa0.cpe.net.cable.rogers.com. [99.228.215.92]) by smtp.gmail.com with ESMTPSA id m186sm666445ith.2.2018.01.26.07.54.09 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 26 Jan 2018 07:54:09 -0800 (PST) From: selva.nair@gmail.com To: openvpn-devel@lists.sourceforge.net Date: Fri, 26 Jan 2018 10:53:10 -0500 Message-Id: <1516981990-4610-1-git-send-email-selva.nair@gmail.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <93167c2a-c7a2-de57-f793-7fdc2004bfb2@fox-it.com> References: <93167c2a-c7a2-de57-f793-7fdc2004bfb2@fox-it.com> X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (selva.nair[at]gmail.com) -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, no trust [209.85.223.195 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -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 Message has at least one valid DKIM or DK signature 0.0 AWL AWL: Adjusted score from AWL reputation of From: address X-Headers-End: 1ef6KV-0003rl-RE Subject: [Openvpn-devel] [PATCH v2 2/3] Move setting private key to a function in prep for EC support 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: Selva Nair - Also add reference counting to CAPI_DATA (application data): When the application data is assigned to the private key we free it in the key's finish method. Proper error handling requires to keep track of whether data is assigned to the key or not before an error occurs. For this purpose, add a reference count to CAPI_DATA struct and increment it when it is assigned to the key or its method. CAPI_DATA_free now frees the data only if ref_count <= 0 Signed-off-by: Selva Nair --- v2: In response to Steffan's review: check the return value of EVP_PKEY_get0_RSA() src/openvpn/cryptoapi.c | 144 ++++++++++++++++++++++++++++-------------------- 1 file changed, 85 insertions(+), 59 deletions(-) diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c index 0349191..8c66b5f 100644 --- a/src/openvpn/cryptoapi.c +++ b/src/openvpn/cryptoapi.c @@ -106,12 +106,13 @@ typedef struct _CAPI_DATA { HCRYPTPROV_OR_NCRYPT_KEY_HANDLE crypt_prov; DWORD key_spec; BOOL free_crypt_prov; + int ref_count; } CAPI_DATA; static void CAPI_DATA_free(CAPI_DATA *cd) { - if (!cd) + if (!cd || cd->ref_count-- > 0) { return; } @@ -466,14 +467,85 @@ find_certificate_in_store(const char *cert_prop, HCERTSTORE cert_store) return rv; } +static int +ssl_ctx_set_rsakey(SSL_CTX *ssl_ctx, CAPI_DATA *cd, EVP_PKEY *pkey) +{ + RSA *rsa = NULL, *pub_rsa; + RSA_METHOD *my_rsa_method = NULL; + bool rsa_method_set = false; + + my_rsa_method = RSA_meth_new("Microsoft Cryptography API RSA Method", + RSA_METHOD_FLAG_NO_CHECK); + check_malloc_return(my_rsa_method); + RSA_meth_set_pub_enc(my_rsa_method, rsa_pub_enc); + RSA_meth_set_pub_dec(my_rsa_method, rsa_pub_dec); + RSA_meth_set_priv_enc(my_rsa_method, rsa_priv_enc); + RSA_meth_set_priv_dec(my_rsa_method, rsa_priv_dec); + RSA_meth_set_init(my_rsa_method, NULL); + RSA_meth_set_finish(my_rsa_method, finish); + RSA_meth_set0_app_data(my_rsa_method, cd); + + rsa = RSA_new(); + if (rsa == NULL) + { + SSLerr(SSL_F_SSL_CTX_USE_CERTIFICATE_FILE, ERR_R_MALLOC_FAILURE); + goto err; + } + + pub_rsa = EVP_PKEY_get0_RSA(pkey); + if (!pub_rsa) + { + goto err; + } + + /* Our private key is external, so we fill in only n and e from the public key */ + const BIGNUM *n = NULL; + const BIGNUM *e = NULL; + RSA_get0_key(pub_rsa, &n, &e, NULL); + BIGNUM *rsa_n = BN_dup(n); + BIGNUM *rsa_e = BN_dup(e); + if (!rsa_n || !rsa_e || !RSA_set0_key(rsa, rsa_n, rsa_e, NULL)) + { + BN_free(rsa_n); /* ok to free even if NULL */ + BN_free(rsa_e); + msg(M_NONFATAL, "ERROR: %s: out of memory", __func__); + goto err; + } + RSA_set_flags(rsa, RSA_flags(rsa) | RSA_FLAG_EXT_PKEY); + if (!RSA_set_method(rsa, my_rsa_method)) + { + goto err; + } + rsa_method_set = true; /* flag that method pointer will get freed with the key */ + cd->ref_count++; /* with method, cd gets assigned to the key as well */ + + if (!SSL_CTX_use_RSAPrivateKey(ssl_ctx, rsa)) + { + goto err; + } + /* SSL_CTX_use_RSAPrivateKey() increased the reference count in 'rsa', so + * we decrease it here with RSA_free(), or it will never be cleaned up. */ + RSA_free(rsa); + return 1; + +err: + if (rsa) + { + RSA_free(rsa); + } + if (my_rsa_method && !rsa_method_set) + { + RSA_meth_free(my_rsa_method); + } + return 0; +} + int SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop) { HCERTSTORE cs; X509 *cert = NULL; - RSA *rsa = NULL, *pub_rsa; CAPI_DATA *cd = calloc(1, sizeof(*cd)); - RSA_METHOD *my_rsa_method = NULL; if (cd == NULL) { @@ -548,30 +620,13 @@ SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop) } } - my_rsa_method = RSA_meth_new("Microsoft Cryptography API RSA Method", - RSA_METHOD_FLAG_NO_CHECK); - check_malloc_return(my_rsa_method); - RSA_meth_set_pub_enc(my_rsa_method, rsa_pub_enc); - RSA_meth_set_pub_dec(my_rsa_method, rsa_pub_dec); - RSA_meth_set_priv_enc(my_rsa_method, rsa_priv_enc); - RSA_meth_set_priv_dec(my_rsa_method, rsa_priv_dec); - RSA_meth_set_init(my_rsa_method, NULL); - RSA_meth_set_finish(my_rsa_method, finish); - RSA_meth_set0_app_data(my_rsa_method, cd); - - rsa = RSA_new(); - if (rsa == NULL) - { - SSLerr(SSL_F_SSL_CTX_USE_CERTIFICATE_FILE, ERR_R_MALLOC_FAILURE); - goto err; - } - /* Public key in cert is NULL until we call SSL_CTX_use_certificate(), * so we do it here then... */ if (!SSL_CTX_use_certificate(ssl_ctx, cert)) { goto err; } + /* the public key */ EVP_PKEY *pkey = X509_get0_pubkey(cert); @@ -580,52 +635,23 @@ SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop) X509_free(cert); cert = NULL; - if (!(pub_rsa = EVP_PKEY_get0_RSA(pkey))) + if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA) { - msg(M_WARN, "cryptoapicert requires an RSA certificate"); - goto err; - } - - /* Our private key is external, so we fill in only n and e from the public key */ - const BIGNUM *n = NULL; - const BIGNUM *e = NULL; - RSA_get0_key(pub_rsa, &n, &e, NULL); - if (!RSA_set0_key(rsa, BN_dup(n), BN_dup(e), NULL)) - { - goto err; - } - RSA_set_flags(rsa, RSA_flags(rsa) | RSA_FLAG_EXT_PKEY); - if (!RSA_set_method(rsa, my_rsa_method)) - { - goto err; + if (!ssl_ctx_set_rsakey(ssl_ctx, cd, pkey)) + { + goto err; + } } - - if (!SSL_CTX_use_RSAPrivateKey(ssl_ctx, rsa)) + else { + msg(M_WARN, "cryptoapicert requires an RSA certificate"); goto err; } - /* SSL_CTX_use_RSAPrivateKey() increased the reference count in 'rsa', so - * we decrease it here with RSA_free(), or it will never be cleaned up. */ - RSA_free(rsa); + cd->ref_count--; /* so that cd will get freed with the private key */ return 1; err: - if (cert) - { - X509_free(cert); - } - if (rsa) - { - RSA_free(rsa); - } - else - { - if (my_rsa_method) - { - free(my_rsa_method); - } - CAPI_DATA_free(cd); - } + CAPI_DATA_free(cd); return 0; }