From patchwork Thu Feb 22 16:03:19 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Selva Nair X-Patchwork-Id: 243 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director10.mail.ord1d.rsapps.net ([172.28.255.1]) by backend30.mail.ord1d.rsapps.net (Dovecot) with LMTP id hh1NGNmEj1oTWAAAIUCqbw for ; Thu, 22 Feb 2018 22:04:57 -0500 Received: from proxy1.mail.ord1c.rsapps.net ([172.28.255.1]) by director10.mail.ord1d.rsapps.net (Dovecot) with LMTP id O3blJdmEj1p5SQAApN4f7A ; Thu, 22 Feb 2018 22:04:57 -0500 Received: from smtp44.gate.ord1c ([172.28.255.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy1.mail.ord1c.rsapps.net (Dovecot) with LMTP id Occ0IdmEj1olDgAA2VeTtA ; Thu, 22 Feb 2018 22:04:57 -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.105.38.7] Authentication-Results: smtp44.gate.ord1c.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; dkim=fail (signature verification failed) header.d=gmail.com; dmarc=fail (p=none; dis=none) header.from=gmail.com X-Classification-ID: 52d69152-1846-11e8-9a5b-bc305bf5395c-1-1 Received: from [216.105.38.7] ([216.105.38.7:5992] helo=lists.sourceforge.net) by smtp44.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.1.56364 r(Core:4.2.1.14)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 3B/90-04550-7D48F8A5; Thu, 22 Feb 2018 22:04:56 -0500 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.89) (envelope-from ) id 1ep3e6-0001tB-Si; Fri, 23 Feb 2018 03:03:38 +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.89) (envelope-from ) id 1ep3e5-0001t5-Hh for openvpn-devel@lists.sourceforge.net; Fri, 23 Feb 2018 03:03:37 +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=mpKfrNqhPz61Cupk6nX2lnAWicNx8cUX+WTx2iou5gA=; b=SyGcdMvSK6TCZn+09wqYM/FJ4U MIj+WV2tqwptXX7KO1SOBAUpmF4lUtZDLuwTrv1gIZvWF3AzoyANFQX8ibEc3hmt9o+VG4VNJv2kt hdKyPUEerLhWJeTUsxvTUcoKAUDXTngXcIkOFRs433mm6n6tpcWG7+6Um6ivTepWo4VU=; 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=mpKfrNqhPz61Cupk6nX2lnAWicNx8cUX+WTx2iou5gA=; b=RS86AVU0l+i1LxXVAGMHXOsj8s 3pZyDlRSaAl9K5b65ojWziTEjh+y0NjSoxgeQ76zR+De7CcnaNgCYjyWdRXclBfg6EeO2+55JOWTc i9K4cOR0CmxF3WN+BuK2iHPr0UnJG759zb5Rz1c4w2wXiVqC8O2bi68So7bSCrpdnhXQ=; Received: from sfi-lb-mx.v20.lw.sourceforge.com ([172.30.20.201] helo=mail-io0-f196.google.com) by sfi-mx-3.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.89) id 1ep3e3-0006Qt-NJ for openvpn-devel@lists.sourceforge.net; Fri, 23 Feb 2018 03:03:37 +0000 Received: by mail-io0-f196.google.com with SMTP id g21so8303996ioj.5 for ; Thu, 22 Feb 2018 19:03:35 -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=mpKfrNqhPz61Cupk6nX2lnAWicNx8cUX+WTx2iou5gA=; b=ODfipFZ38m41S4RKW+Mlo+/qpJnU1nHbGLFp1TKAJiHh+XkVdGxGF9CmAfQ968iU5P DP/FaYXIw5hjeRVUimPrUAKb8cGTxiGu1/VH8cYqO5I1IoKWS79UsEEQUpMjsrHbgQGG 0+WP+cuAGFFDTn7PdDZIPFE1OORZKrJKgO0S+teEAvQQfAXtOOlN7nYRJnBGoNOeFvT+ GayWRwFT5BsQA/0xRttRAcs9qxFqv1CH+Xj/1hx/Y7yFIiKfIXUwJhh9v3bbiPSvN7Zh V18HnKWDV53kF0qm+qZjqfzac0DgwzrmIkhZI/Xp4wD7L0X8CM9NTOpHCTNIeqgGxrkG ocmw== 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=mpKfrNqhPz61Cupk6nX2lnAWicNx8cUX+WTx2iou5gA=; b=DhwuX7bAUCZMV2wzgb6r2rKn//F7wO6luWe5eMWZ2sI4d0rgfrfQ+dYtSaJ1d5UlVp IKH5Q4jWRUjSWws2feU7ZQCmf6Kw3EKPKXr1LsuTVvIwICM7OjEJ2/Sy4CNfRkMNRaNw nCu0J+LdW9yyal+CH2Ixdp7CQ9B58LtypXFSxgO4kr3PGx14L+WGvavyp7/86sWQqniT 5OnWripDikoUFBRUXINT6EDJv3NTEK3j5TfPAdxrcDEgz9L2vajNXKEuY1dMen9vlmvD 0Qunupf4TeML67U+Ma6IfeG5kPJCABIJ2nNb/nFyk7EAggw6/48e5iw5L+/YKhIWNYsD ZkuQ== X-Gm-Message-State: APf1xPAvsKR+JadTK7HYVqOtsYd7j00MOB0+B58mWlp1MGAB2/xD/YXx LJ/k1wzlzpg31naEgJ7zdw2p0uRd X-Google-Smtp-Source: AG47ELv840uNVkoFTDNA030deOWtfY+snn35LiLV3tXdetrhlkrF9oBUt08K97m027mdXd76SNRh8g== X-Received: by 10.107.232.7 with SMTP id f7mr198814ioh.130.1519355009851; Thu, 22 Feb 2018 19:03:29 -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 m3sm572070ita.16.2018.02.22.19.03.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 22 Feb 2018 19:03:28 -0800 (PST) From: selva.nair@gmail.com To: openvpn-devel@lists.sourceforge.net Date: Thu, 22 Feb 2018 22:03:19 -0500 Message-Id: <1519354999-18496-1-git-send-email-selva.nair@gmail.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1516981990-4610-1-git-send-email-selva.nair@gmail.com> References: <1516981990-4610-1-git-send-email-selva.nair@gmail.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) 1.3 RCVD_IN_RP_RNBL RBL: Relay in RNBL, https://senderscore.org/blacklistlookup/ [99.228.215.92 listed in bl.score.senderscore.com] 1.0 SPF_SOFTFAIL SPF: sender does not match SPF record (softfail) -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.6 AWL AWL: Adjusted score from AWL reputation of From: address X-Headers-End: 1ep3e3-0006Qt-NJ Subject: [Openvpn-devel] [PATCH v3 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 Acked-by: Steffan Karger --- v2: In response to Steffan's review: check the return value of EVP_PKEY_get0_RSA() v3: Use CAPI_DATA_free() instead of cd->ref_count-- 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..1097286 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); + CAPI_DATA_free(cd); /* this will do a ref_count-- */ 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; }