From patchwork Fri Jan 19 17:52:54 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Selva Nair X-Patchwork-Id: 202 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 W+EfH+nLYlqQMwAAgoeIoA for ; Fri, 19 Jan 2018 23:56:09 -0500 Received: from proxy12.mail.ord1d.rsapps.net ([172.30.191.6]) by director3.mail.ord1d.rsapps.net (Dovecot) with LMTP id a+D8HunLYlobawAAkXNnRw ; Fri, 19 Jan 2018 23:56:09 -0500 Received: from smtp1.gate.ord1c ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy12.mail.ord1d.rsapps.net (Dovecot) with LMTP id ujluHenLYloRSwAA7PHxkg ; Fri, 19 Jan 2018 23:56:09 -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: smtp1.gate.ord1c.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: 3acd8dd0-fd9e-11e7-b394-842b2b47c027-1-1 Received: from [216.34.181.88] ([216.34.181.88:52066] helo=lists.sourceforge.net) by smtp1.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 A9/34-21207-9EBC26A5; Fri, 19 Jan 2018 23:56:09 -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 1ecl9P-0006ML-Ra; Sat, 20 Jan 2018 04:53:07 +0000 Received: from sfi-mx-4.v28.ch3.sourceforge.com ([172.29.28.194] 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 1ecl9O-0006MF-SB for openvpn-devel@lists.sourceforge.net; Sat, 20 Jan 2018 04:53:06 +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=M7tHE/j3lp26aFluoVZE7spfPRZzjNrIOCK5mGS1vbY=; b=CjsECmAK7fJci/NIiqBbAVaJEE p4umsMU+qJ2U9CzwFK3n0M34mLx1pUXGp9blu8DIZ7oS1DFtlr7z2eom67zRCDeYybhGx21MPSXfd vCf3NMhZ4QcdsxOVpQybi6Ymf+FhO5kanUSdGti6dRw0skbyA5ABcnpH7rwy9EI/jP0E=; 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=M7tHE/j3lp26aFluoVZE7spfPRZzjNrIOCK5mGS1vbY=; b=DEpj15/gNJT0OSI4GO9Y3QrL4C WyWlifg3AYZb8NghavjNNlv+9PZ9liknXjo6DOPWJSplaoa9uBn9Ed09l2xl9SQdTGToeBpjjc0jd IaUyE0znpaRPzAWtrrmtB3G0moNa+e42Iu22Jx9uOh6AdPExRRqzddwhDYu8CqF23hZg=; Received: from mail-it0-f68.google.com ([209.85.214.68]) by sfi-mx-4.v28.ch3.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.89) id 1ecl9N-0008Ph-7b for openvpn-devel@lists.sourceforge.net; Sat, 20 Jan 2018 04:53:06 +0000 Received: by mail-it0-f68.google.com with SMTP id p124so4392713ite.1 for ; Fri, 19 Jan 2018 20:53:05 -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=M7tHE/j3lp26aFluoVZE7spfPRZzjNrIOCK5mGS1vbY=; b=I2nRoRKfjIUd/XeIB7uQvsJIHjtJr6U76vnVOklJTmx+Rzcl8P1R2AmxvyPv9vXcLq H16QNi1/YTVY3z12KOBJTPP6l8sPy23BiklIgYmMnXnPJNq9GgMMmmc5k+SV3DvYsPd9 ih3+IY2tYOAHrk0ynZf17QNKpdmdE9UdpTg/kHdQBEHwGkI1GhUB4at3UQQ7XE24tVgR +O8vZci7+z4Nbhen2869Idl6qUiZzP7AQ4H+WHc8NBOsnJFwj5srGOVg3B1gNYuYKcQO QvmwOTNS5tbIMrv+6atJHg+PysPb9Tsl9zN30oP4UMoaFwAJSdv96fhxn1uzXaUu4cvm Mt+A== 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=M7tHE/j3lp26aFluoVZE7spfPRZzjNrIOCK5mGS1vbY=; b=YZrjdJ5N28PZ1cQ41dWjRjONrauEpz2MDwTUj8wa1jh4pgOyYl3N2hVQgbig+6h+xc BhtRxFeO+ZcPDqIqpU+KMcuJ4PAQc/uKpFvcqjcEyhmUVg1SubFqm6KnH1g1O8+Q9nBk vBXvb0Mi+uOnqvKt90xgwqjCvYj1EQ9tPlrlPma/54nRjuqQRxJt6bSqIGG+Wfmq6KYs 1/DDsU5z+y/57vp197CQdBC58iZTpGROJO+65OAgB2LhxM9NcfvWRH2c3LHpEVKv2gJP BQkdH4XTilH8/l8oMjUGNcqu3CId5rEU3FMDqCAPeT2UIHufDfzN1uEVrNiCfuD4lPJU oobw== X-Gm-Message-State: AKwxytcifzFOn7cXs7vgE7GIfm5jnTkwvng/GxTanP8/LOEZ7MSAPUY4 hZmi5ypLTkp47Ktw0zwXsA8TvX1b X-Google-Smtp-Source: AH8x227aQGnnjYOmLiXTzJrvg1LVLXf3KmryGPxVs12nEjEZZuqpdjh5LbIyIQDvohb3wG2+XMvP7Q== X-Received: by 10.36.84.69 with SMTP id t66mr482572ita.49.1516423979530; Fri, 19 Jan 2018 20:52:59 -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 l68sm1566868ith.25.2018.01.19.20.52.58 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 19 Jan 2018 20:52:58 -0800 (PST) From: selva.nair@gmail.com To: openvpn-devel@lists.sourceforge.net Date: Fri, 19 Jan 2018 23:52:54 -0500 Message-Id: <1516423974-22159-1-git-send-email-selva.nair@gmail.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1515378076-5774-3-git-send-email-selva.nair@gmail.com> References: <1515378076-5774-3-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.68 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: 1ecl9N-0008Ph-7b Subject: [Openvpn-devel] [PATCH v2 2/2] TLS v1.2 support for cryptoapicert -- RSA only 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 - If an NCRYPT handle for the private key can be obtained, use NCryptSignHash from the Cryptography NG API to sign the hash. This should work for all keys in the Windows certifiate stores but may fail for keys in a legacy token, for example. In such cases, we disable TLS v1.2 and fall back to the current behaviour. A warning is logged unless TLS version is already restricted to <= 1.1 Signed-off-by: Selva Nair Acked-by: Steffan Karger Tested-by: Selva Nair --- Depends on patches: patch 200: https://patchwork.openvpn.net/patch/200/ patch 201: https://patchwork.openvpn.net/patch/201/ v2: Based on Stefan's review: - Replace SSL_CTX_get(set)_option with SSL_CTX_get(set)_max_proto_version - Wrap some lines to 80 chars - replace M_INFO by D_LOW in a low level debug message Also removed some defines added in v1 that are actually present in mingw versions that we target src/openvpn/Makefile.am | 2 +- src/openvpn/cryptoapi.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++--- src/openvpn/options.c | 18 ----------- 3 files changed, 81 insertions(+), 24 deletions(-) diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am index fcc22d6..1a2f42e 100644 --- a/src/openvpn/Makefile.am +++ b/src/openvpn/Makefile.am @@ -132,5 +132,5 @@ openvpn_LDADD = \ $(OPTIONAL_DL_LIBS) if WIN32 openvpn_SOURCES += openvpn_win32_resources.rc block_dns.c block_dns.h -openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm -lfwpuclnt -lrpcrt4 +openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm -lfwpuclnt -lrpcrt4 -lncrypt endif diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c index 4f2c636..f155123 100644 --- a/src/openvpn/cryptoapi.c +++ b/src/openvpn/cryptoapi.c @@ -42,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -83,6 +84,7 @@ #define CRYPTOAPI_F_CRYPT_SIGN_HASH 106 #define CRYPTOAPI_F_LOAD_LIBRARY 107 #define CRYPTOAPI_F_GET_PROC_ADDRESS 108 +#define CRYPTOAPI_F_NCRYPT_SIGN_HASH 109 static ERR_STRING_DATA CRYPTOAPI_str_functs[] = { { ERR_PACK(ERR_LIB_CRYPTOAPI, 0, 0), "microsoft cryptoapi"}, @@ -95,12 +97,13 @@ static ERR_STRING_DATA CRYPTOAPI_str_functs[] = { { ERR_PACK(0, CRYPTOAPI_F_CRYPT_SIGN_HASH, 0), "CryptSignHash" }, { ERR_PACK(0, CRYPTOAPI_F_LOAD_LIBRARY, 0), "LoadLibrary" }, { ERR_PACK(0, CRYPTOAPI_F_GET_PROC_ADDRESS, 0), "GetProcAddress" }, + { ERR_PACK(0, CRYPTOAPI_F_NCRYPT_SIGN_HASH, 0), "NCryptSignHash" }, { 0, NULL } }; typedef struct _CAPI_DATA { const CERT_CONTEXT *cert_context; - HCRYPTPROV crypt_prov; + HCRYPTPROV_OR_NCRYPT_KEY_HANDLE crypt_prov; DWORD key_spec; BOOL free_crypt_prov; } CAPI_DATA; @@ -210,6 +213,41 @@ rsa_pub_dec(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, in return 0; } +/** + * Sign the hash in 'from' using NCryptSignHash(). This requires an NCRYPT + * key handle in cd->crypt_prov. On return the signature is in 'to'. Returns + * the length of the signature or 0 on error. + * For now we support only RSA and the padding is assumed to be PKCS1 v1.5 + */ +static int +priv_enc_CNG(const CAPI_DATA *cd, const unsigned char *from, int flen, + unsigned char *to, int tlen, int padding) +{ + NCRYPT_KEY_HANDLE hkey = cd->crypt_prov; + DWORD len; + ASSERT(cd->key_spec == CERT_NCRYPT_KEY_SPEC); + + msg(D_LOW, "Signing hash using CNG: data size = %d", flen); + + /* The hash OID is already in 'from'. So set the hash algorithm + * in the padding info struct to NULL. + */ + BCRYPT_PKCS1_PADDING_INFO padinfo = {NULL}; + DWORD status; + + status = NCryptSignHash(hkey, padding? &padinfo : NULL, (BYTE*) from, flen, + to, tlen, &len, padding? BCRYPT_PAD_PKCS1 : 0); + if (status != ERROR_SUCCESS) + { + SetLastError(status); + CRYPTOAPIerr(CRYPTOAPI_F_NCRYPT_SIGN_HASH); + len = 0; + } + + /* Unlike CAPI, CNG signature is in big endian order. No reversing needed. */ + return len; +} + /* sign arbitrary data */ static int rsa_priv_enc(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, int padding) @@ -230,6 +268,11 @@ rsa_priv_enc(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, i RSAerr(RSA_F_RSA_OSSL_PRIVATE_ENCRYPT, RSA_R_UNKNOWN_PADDING_TYPE); return 0; } + if (cd->key_spec == CERT_NCRYPT_KEY_SPEC) + { + return priv_enc_CNG(cd, from, flen, to, RSA_size(rsa), padding); + } + /* Unfortunately, there is no "CryptSign()" function in CryptoAPI, that would * be way to straightforward for M$, I guess... So we have to do it this * tricky way instead, by creating a "Hash", and load the already-made hash @@ -322,7 +365,14 @@ finish(RSA *rsa) } if (cd->crypt_prov && cd->free_crypt_prov) { - CryptReleaseContext(cd->crypt_prov, 0); + if (cd->key_spec == CERT_NCRYPT_KEY_SPEC) + { + NCryptFreeObject(cd->crypt_prov); + } + else + { + CryptReleaseContext(cd->crypt_prov, 0); + } } if (cd->cert_context) { @@ -458,8 +508,11 @@ SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop) } /* set up stuff to use the private key */ - if (!CryptAcquireCertificatePrivateKey(cd->cert_context, CRYPT_ACQUIRE_COMPARE_KEY_FLAG, - NULL, &cd->crypt_prov, &cd->key_spec, &cd->free_crypt_prov)) + /* We prefer to get an NCRYPT key handle so that TLS1.2 can be supported */ + DWORD flags = CRYPT_ACQUIRE_COMPARE_KEY_FLAG + | CRYPT_ACQUIRE_PREFER_NCRYPT_KEY_FLAG; + if (!CryptAcquireCertificatePrivateKey(cd->cert_context, flags, NULL, + &cd->crypt_prov, &cd->key_spec, &cd->free_crypt_prov)) { /* if we don't have a smart card reader here, and we try to access a * smart card certificate, we get: @@ -470,6 +523,21 @@ SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop) /* here we don't need to do CryptGetUserKey() or anything; all necessary key * info is in cd->cert_context, and then, in cd->crypt_prov. */ + /* if we do not have an NCRYPT key handle restrict TLS to v1.1 or lower */ + int max_version = SSL_CTX_get_max_proto_version(ssl_ctx); + if ((!max_version || max_version > TLS1_1_VERSION) + && cd->key_spec != CERT_NCRYPT_KEY_SPEC) + { + msg(M_WARN,"WARNING: cryptoapicert: private key is in a legacy store." + " Restricting TLS version to 1.1"); + if (!SSL_CTX_set_max_proto_version(ssl_ctx, TLS1_1_VERSION)) + { + msg(M_NONFATAL,"ERROR: cryptoapicert: unable to set max TLS version" + " to 1.1. Try config option --tls-version-min 1.1"); + goto err; + } + } + my_rsa_method = RSA_meth_new("Microsoft Cryptography API RSA Method", RSA_METHOD_FLAG_NO_CHECK); check_malloc_return(my_rsa_method); @@ -550,7 +618,14 @@ err: { if (cd->free_crypt_prov && cd->crypt_prov) { - CryptReleaseContext(cd->crypt_prov, 0); + if (cd->key_spec == CERT_NCRYPT_KEY_SPEC) + { + NCryptFreeObject(cd->crypt_prov); + } + else + { + CryptReleaseContext(cd->crypt_prov, 0); + } } if (cd->cert_context) { diff --git a/src/openvpn/options.c b/src/openvpn/options.c index b240e2e..220c2e5 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3018,24 +3018,6 @@ options_postprocess_mutate(struct options *o) } #endif -#ifdef ENABLE_CRYPTOAPI - if (o->cryptoapi_cert) - { - const int tls_version_max = - (o->ssl_flags >> SSLF_TLS_VERSION_MAX_SHIFT) - &SSLF_TLS_VERSION_MAX_MASK; - - if (tls_version_max == TLS_VER_UNSPEC || tls_version_max > TLS_VER_1_1) - { - msg(M_WARN, "Warning: cryptapicert used, setting maximum TLS " - "version to 1.1."); - o->ssl_flags &= ~(SSLF_TLS_VERSION_MAX_MASK - <ssl_flags |= (TLS_VER_1_1 << SSLF_TLS_VERSION_MAX_SHIFT); - } - } -#endif /* ENABLE_CRYPTOAPI */ - #if P2MP /* * Save certain parms before modifying options via --pull