From patchwork Tue Jan 23 18:06:20 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Selva Nair X-Patchwork-Id: 207 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director2.mail.ord1d.rsapps.net ([172.30.191.6]) by backend31.mail.ord1d.rsapps.net (Dovecot) with LMTP id 5LGuJpkUaFo4dgAAgoeIoA for ; Wed, 24 Jan 2018 00:07:37 -0500 Received: from proxy6.mail.ord1d.rsapps.net ([172.30.191.6]) by director2.mail.ord1d.rsapps.net (Dovecot) with LMTP id 4wC4CpkUaFpncgAAgYhSiA ; Wed, 24 Jan 2018 00:07:37 -0500 Received: from smtp2.gate.ord1d ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy6.mail.ord1d.rsapps.net (Dovecot) with LMTP id 6kzYFZkUaFryRAAAQyIf0w ; Wed, 24 Jan 2018 00:07:37 -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: smtp2.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: 7e85e328-00c4-11e8-813a-5254004a0287-1-1 Received: from [216.34.181.88] ([216.34.181.88:15156] helo=lists.sourceforge.net) by smtp2.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 C1/0D-31706-994186A5; Wed, 24 Jan 2018 00:07:37 -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 1eeDGd-0006w3-Bb; Wed, 24 Jan 2018 05:06:35 +0000 Received: from sfi-mx-2.v28.ch3.sourceforge.com ([172.29.28.192] 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 1eeDGc-0006vx-6D for openvpn-devel@lists.sourceforge.net; Wed, 24 Jan 2018 05:06:34 +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=k934aPpBcJqnjT6LqZK5UO59YALMTAH0vpYwUU1Dywc=; b=mJ7QMT7ITTV8ja7IcrNe/No8cj XGU0IUUrbDrgp2uO1V0oaVabra7mf6chRp8LE2jbl5fNmVLtbAjdqW1Z9LYu/b4hlYbkmGVerOORw Qw96oTtdEi1u1SILMps3N3dO2dixxBgjlW7gsTaCnyPsQJrzB8pO3Vz91k+SsD7Q/hxI=; 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=k934aPpBcJqnjT6LqZK5UO59YALMTAH0vpYwUU1Dywc=; b=MUfMxDJCfw/4QP5JLXv3ZHLhng PXJvK7H7viWgAyXwKZ7M6qFV7gQHrQSp6NFQMImnQXHMpGyNf4RP3eHMPowcU0Bk2IM4CacvIOKPF LfwtoKbYxnmBMFQrKWEkjecn81B6McQQpeaA80w2heS2n2MGVP1nvaXPcUwk1MDAknmc=; Received: from mail-it0-f65.google.com ([209.85.214.65]) by sfi-mx-2.v28.ch3.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.89) id 1eeDGb-0007th-9S for openvpn-devel@lists.sourceforge.net; Wed, 24 Jan 2018 05:06:34 +0000 Received: by mail-it0-f65.google.com with SMTP id p139so3744266itb.1 for ; Tue, 23 Jan 2018 21:06:33 -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=k934aPpBcJqnjT6LqZK5UO59YALMTAH0vpYwUU1Dywc=; b=OONO5DSV2FjhkmN169VsXNNQa3QaoHpe56fTZXHsIGnwvLQt7d64+I65drGr83lLWs 7OL5tJiETV03nDVIBpsHylaBVeNb1JXii14lcconJi0bMkINqlbzmGX8SRZCjZxEGUNq z908ifT8MoHQyAFMvhnnYVv5sJ08zMc/lvHQ1JCh8RUXyluVcTurbDlG1ljG5E29O6uK XC/U6QYIsaxeW+QjO/Rrn9a9Tv1E0jr5AMa5rFJL4Tt0VC7RYW4MNk0GZvOYXjsdOybF aWTzorABH43FuDVUNVYXXwtyf5/rWNvdwRP3qJFLvhHnj5zd/nrjGSM46inkaQ+oTA8n 9sRg== 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=k934aPpBcJqnjT6LqZK5UO59YALMTAH0vpYwUU1Dywc=; b=hWffXW+8cqEOJrQWK5u6/oYuVNaIRQrGCCd7yYpuMjslwYkkJH/KPmefsTrDc9QVwa A0JnvE42vSUcTWXwAoxIT1vzEWryZ56NftdkMiyYKlNadtr4I/vRlLT+qOSWNhkeWFfp 4vXuNO9vBXY4laaGeFJFKJBK2uRzhmWSxllw3QNourpViE4Ao6qM+kdkT9FKg1p4KcZM jyKjpesQZ+zlE2jqg05NpYGhYVI6vHfsqfMxZg2JKSF6cFp37dCMBKgQhxLNQRYe1fXU BDGQxO4hlB7HL32iz5FufPiJLue8fD0gN4nv+BVVkYXMWqyLPOGMcQkaVhI3oVzsKyVO uOOw== X-Gm-Message-State: AKwxytdAX/CrVrjmeKKTX8th4OskLKUEIikE8GzGYjC0m/w7RmztRDiH JFPPN/fggcnuNyHRJ5n0AwhSZsHX X-Google-Smtp-Source: AH8x225V283+qqFwu+twy6lAXTMjCBt4nv1QacRHIr0ktz2++YVSWENcaHoWZnnsqCvIlWBxkg8vsw== X-Received: by 10.36.253.204 with SMTP id m195mr7668739ith.66.1516770387726; Tue, 23 Jan 2018 21:06:27 -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 140sm289669itx.3.2018.01.23.21.06.26 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 23 Jan 2018 21:06:27 -0800 (PST) From: selva.nair@gmail.com To: openvpn-devel@lists.sourceforge.net Date: Wed, 24 Jan 2018 00:06:20 -0500 Message-Id: <1516770381-29466-3-git-send-email-selva.nair@gmail.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1516770381-29466-1-git-send-email-selva.nair@gmail.com> References: <1516770381-29466-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 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, no trust [209.85.214.65 listed in list.dnswl.org] 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (selva.nair[at]gmail.com) -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: 1eeDGb-0007th-9S Subject: [Openvpn-devel] [PATCH 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 --- src/openvpn/cryptoapi.c | 140 ++++++++++++++++++++++++++++-------------------- 1 file changed, 81 insertions(+), 59 deletions(-) diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c index 00e78b6..d6a9dd4 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; } @@ -467,14 +468,81 @@ 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); + + /* 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) { @@ -549,30 +617,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); @@ -581,52 +632,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))) - { - 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)) + if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA) { - 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; }