Browse code

TLS v1.2 support for cryptoapicert -- RSA only

- 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 <selva.nair@gmail.com>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <1516423974-22159-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16288.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Selva Nair authored on 2018/01/20 13:52:54
Showing 3 changed files
... ...
@@ -132,5 +132,5 @@ openvpn_LDADD = \
132 132
 	$(OPTIONAL_DL_LIBS)
133 133
 if WIN32
134 134
 openvpn_SOURCES += openvpn_win32_resources.rc block_dns.c block_dns.h
135
-openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm -lfwpuclnt -lrpcrt4
135
+openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm -lfwpuclnt -lrpcrt4 -lncrypt
136 136
 endif
... ...
@@ -42,6 +42,7 @@
42 42
 #include <openssl/err.h>
43 43
 #include <windows.h>
44 44
 #include <wincrypt.h>
45
+#include <ncrypt.h>
45 46
 #include <stdio.h>
46 47
 #include <ctype.h>
47 48
 #include <assert.h>
... ...
@@ -83,6 +84,7 @@
83 83
 #define CRYPTOAPI_F_CRYPT_SIGN_HASH                         106
84 84
 #define CRYPTOAPI_F_LOAD_LIBRARY                            107
85 85
 #define CRYPTOAPI_F_GET_PROC_ADDRESS                        108
86
+#define CRYPTOAPI_F_NCRYPT_SIGN_HASH                        109
86 87
 
87 88
 static ERR_STRING_DATA CRYPTOAPI_str_functs[] = {
88 89
     { ERR_PACK(ERR_LIB_CRYPTOAPI, 0, 0),                                    "microsoft cryptoapi"},
... ...
@@ -95,12 +97,13 @@ static ERR_STRING_DATA CRYPTOAPI_str_functs[] = {
95 95
     { ERR_PACK(0, CRYPTOAPI_F_CRYPT_SIGN_HASH, 0),                          "CryptSignHash" },
96 96
     { ERR_PACK(0, CRYPTOAPI_F_LOAD_LIBRARY, 0),                             "LoadLibrary" },
97 97
     { ERR_PACK(0, CRYPTOAPI_F_GET_PROC_ADDRESS, 0),                         "GetProcAddress" },
98
+    { ERR_PACK(0, CRYPTOAPI_F_NCRYPT_SIGN_HASH, 0),                         "NCryptSignHash" },
98 99
     { 0, NULL }
99 100
 };
100 101
 
101 102
 typedef struct _CAPI_DATA {
102 103
     const CERT_CONTEXT *cert_context;
103
-    HCRYPTPROV crypt_prov;
104
+    HCRYPTPROV_OR_NCRYPT_KEY_HANDLE crypt_prov;
104 105
     DWORD key_spec;
105 106
     BOOL free_crypt_prov;
106 107
 } CAPI_DATA;
... ...
@@ -210,6 +213,41 @@ rsa_pub_dec(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, in
210 210
     return 0;
211 211
 }
212 212
 
213
+/**
214
+ * Sign the hash in 'from' using NCryptSignHash(). This requires an NCRYPT
215
+ * key handle in cd->crypt_prov. On return the signature is in 'to'. Returns
216
+ * the length of the signature or 0 on error.
217
+ * For now we support only RSA and the padding is assumed to be PKCS1 v1.5
218
+ */
219
+static int
220
+priv_enc_CNG(const CAPI_DATA *cd, const unsigned char *from, int flen,
221
+              unsigned char *to, int tlen, int padding)
222
+{
223
+    NCRYPT_KEY_HANDLE hkey = cd->crypt_prov;
224
+    DWORD len;
225
+    ASSERT(cd->key_spec == CERT_NCRYPT_KEY_SPEC);
226
+
227
+    msg(D_LOW, "Signing hash using CNG: data size = %d", flen);
228
+
229
+    /* The hash OID is already in 'from'.  So set the hash algorithm
230
+     * in the padding info struct to NULL.
231
+     */
232
+    BCRYPT_PKCS1_PADDING_INFO padinfo = {NULL};
233
+    DWORD status;
234
+
235
+    status = NCryptSignHash(hkey, padding? &padinfo : NULL, (BYTE*) from, flen,
236
+                            to, tlen, &len, padding? BCRYPT_PAD_PKCS1 : 0);
237
+    if (status != ERROR_SUCCESS)
238
+    {
239
+        SetLastError(status);
240
+        CRYPTOAPIerr(CRYPTOAPI_F_NCRYPT_SIGN_HASH);
241
+        len = 0;
242
+    }
243
+
244
+    /* Unlike CAPI, CNG signature is in big endian order. No reversing needed. */
245
+    return len;
246
+}
247
+
213 248
 /* sign arbitrary data */
214 249
 static int
215 250
 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
230 230
         RSAerr(RSA_F_RSA_OSSL_PRIVATE_ENCRYPT, RSA_R_UNKNOWN_PADDING_TYPE);
231 231
         return 0;
232 232
     }
233
+    if (cd->key_spec == CERT_NCRYPT_KEY_SPEC)
234
+    {
235
+        return priv_enc_CNG(cd, from, flen, to, RSA_size(rsa), padding);
236
+    }
237
+
233 238
     /* Unfortunately, there is no "CryptSign()" function in CryptoAPI, that would
234 239
      * be way to straightforward for M$, I guess... So we have to do it this
235 240
      * tricky way instead, by creating a "Hash", and load the already-made hash
... ...
@@ -322,7 +365,14 @@ finish(RSA *rsa)
322 322
     }
323 323
     if (cd->crypt_prov && cd->free_crypt_prov)
324 324
     {
325
-        CryptReleaseContext(cd->crypt_prov, 0);
325
+        if (cd->key_spec == CERT_NCRYPT_KEY_SPEC)
326
+        {
327
+            NCryptFreeObject(cd->crypt_prov);
328
+        }
329
+        else
330
+        {
331
+            CryptReleaseContext(cd->crypt_prov, 0);
332
+        }
326 333
     }
327 334
     if (cd->cert_context)
328 335
     {
... ...
@@ -458,8 +508,11 @@ SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop)
458 458
     }
459 459
 
460 460
     /* set up stuff to use the private key */
461
-    if (!CryptAcquireCertificatePrivateKey(cd->cert_context, CRYPT_ACQUIRE_COMPARE_KEY_FLAG,
462
-                                           NULL, &cd->crypt_prov, &cd->key_spec, &cd->free_crypt_prov))
461
+    /* We prefer to get an NCRYPT key handle so that TLS1.2 can be supported */
462
+    DWORD flags = CRYPT_ACQUIRE_COMPARE_KEY_FLAG
463
+                  | CRYPT_ACQUIRE_PREFER_NCRYPT_KEY_FLAG;
464
+    if (!CryptAcquireCertificatePrivateKey(cd->cert_context, flags, NULL,
465
+                    &cd->crypt_prov, &cd->key_spec, &cd->free_crypt_prov))
463 466
     {
464 467
         /* if we don't have a smart card reader here, and we try to access a
465 468
          * smart card certificate, we get:
... ...
@@ -470,6 +523,21 @@ SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop)
470 470
     /* here we don't need to do CryptGetUserKey() or anything; all necessary key
471 471
      * info is in cd->cert_context, and then, in cd->crypt_prov.  */
472 472
 
473
+    /* if we do not have an NCRYPT key handle restrict TLS to v1.1 or lower */
474
+    int max_version = SSL_CTX_get_max_proto_version(ssl_ctx);
475
+    if ((!max_version || max_version > TLS1_1_VERSION)
476
+        && cd->key_spec != CERT_NCRYPT_KEY_SPEC)
477
+    {
478
+        msg(M_WARN,"WARNING: cryptoapicert: private key is in a legacy store."
479
+            " Restricting TLS version to 1.1");
480
+        if (!SSL_CTX_set_max_proto_version(ssl_ctx, TLS1_1_VERSION))
481
+        {
482
+            msg(M_NONFATAL,"ERROR: cryptoapicert: unable to set max TLS version"
483
+                " to 1.1. Try config option --tls-version-min 1.1");
484
+            goto err;
485
+        }
486
+    }
487
+
473 488
     my_rsa_method = RSA_meth_new("Microsoft Cryptography API RSA Method",
474 489
                                   RSA_METHOD_FLAG_NO_CHECK);
475 490
     check_malloc_return(my_rsa_method);
... ...
@@ -550,7 +618,14 @@ err:
550 550
         {
551 551
             if (cd->free_crypt_prov && cd->crypt_prov)
552 552
             {
553
-                CryptReleaseContext(cd->crypt_prov, 0);
553
+                if (cd->key_spec == CERT_NCRYPT_KEY_SPEC)
554
+                {
555
+                    NCryptFreeObject(cd->crypt_prov);
556
+                }
557
+                else
558
+                {
559
+                    CryptReleaseContext(cd->crypt_prov, 0);
560
+                }
554 561
             }
555 562
             if (cd->cert_context)
556 563
             {
... ...
@@ -3018,24 +3018,6 @@ options_postprocess_mutate(struct options *o)
3018 3018
     }
3019 3019
 #endif
3020 3020
 
3021
-#ifdef ENABLE_CRYPTOAPI
3022
-    if (o->cryptoapi_cert)
3023
-    {
3024
-        const int tls_version_max =
3025
-            (o->ssl_flags >> SSLF_TLS_VERSION_MAX_SHIFT)
3026
-            &SSLF_TLS_VERSION_MAX_MASK;
3027
-
3028
-        if (tls_version_max == TLS_VER_UNSPEC || tls_version_max > TLS_VER_1_1)
3029
-        {
3030
-            msg(M_WARN, "Warning: cryptapicert used, setting maximum TLS "
3031
-                "version to 1.1.");
3032
-            o->ssl_flags &= ~(SSLF_TLS_VERSION_MAX_MASK
3033
-                              <<SSLF_TLS_VERSION_MAX_SHIFT);
3034
-            o->ssl_flags |= (TLS_VER_1_1 << SSLF_TLS_VERSION_MAX_SHIFT);
3035
-        }
3036
-    }
3037
-#endif /* ENABLE_CRYPTOAPI */
3038
-
3039 3021
 #if P2MP
3040 3022
     /*
3041 3023
      * Save certain parms before modifying options via --pull