Skip to contentSkip to conversation

remove wrapper code for Windows CryptoAPI function

PATCH In review Heiko Hund 2 participants 2 messages Aug 17, 2011 – Feb 21, 2012

lore
Share & tools
lore

Heiko Hund

author
#0 lore raw

PATCH 1/1 remove wrapper code for Windows CryptoAPI function

The CryptoAPI function CryptAcquireCertificatePrivateKey() was previously unsupported in MinGW. With recent w32api headers it's now defined, mostly. Since the code used to load the CryptoAPI DLL is prone to a DLL hijacking attack [1], it's now a good time to get rid of wrapper completely.

Just a few macros left that may still be undefined on some systems using the original MinGW headers.

[1] http://isc.sans.edu/diary.html?storyid=9445

Signed-off-by Heiko Hund <[email protected]...>
---
 cryptoapi.c |   43 +++++++++++++------------------------------
 1 files changed, 13 insertions(+), 30 deletions(-)
cryptoapi.c +13 -30
index 3365cd7..0ba27eb 100644
@@ -40,21 +40,23 @@
 #include <ctype.h>
 #include <assert.h>
 
-#ifdef __MINGW32_VERSION
-/* MinGW w32api is incomplete when it comes to CryptoAPI, as per version 3.1
- * anyway. This is a hack around that problem. */
-#define CALG_SSL3_SHAMD5 (ALG_CLASS_HASH | ALG_TYPE_ANY | ALG_SID_SSL3SHAMD5)
+/* MinGW w32api 3.17 is still incomplete when it comes to CryptoAPI while
+ * MinGW32-w64 defines all macros used. This is a hack around that problem.
+ */
+#ifndef CERT_SYSTEM_STORE_LOCATION_SHIFT
 #define CERT_SYSTEM_STORE_LOCATION_SHIFT 16
+#endif
+#ifndef CERT_SYSTEM_STORE_CURRENT_USER_ID
 #define CERT_SYSTEM_STORE_CURRENT_USER_ID 1
+#endif
+#ifndef CERT_SYSTEM_STORE_CURRENT_USER
 #define CERT_SYSTEM_STORE_CURRENT_USER (CERT_SYSTEM_STORE_CURRENT_USER_ID << CERT_SYSTEM_STORE_LOCATION_SHIFT)
+#endif
+#ifndef CERT_STORE_READONLY_FLAG
 #define CERT_STORE_READONLY_FLAG 0x00008000
+#endif
+#ifndef CERT_STORE_OPEN_EXISTING_FLAG
 #define CERT_STORE_OPEN_EXISTING_FLAG 0x00004000
-#define CRYPT_ACQUIRE_COMPARE_KEY_FLAG 0x00000004
-static HINSTANCE crypt32dll = NULL;
-static BOOL WINAPI (*OpenVPNCryptAcquireCertificatePrivateKey) (PCCERT_CONTEXT pCert, DWORD dwFlags,
-  void *pvReserved, HCRYPTPROV *phCryptProv, DWORD *pdwKeySpec, BOOL *pfCallerFreeProv) = NULL;
-#else
-#define OpenVPNCryptAcquireCertificatePrivateKey CryptAcquireCertificatePrivateKey
 #endif
 
 /* Size of an SSL signature: MD5+SHA1 */

@@ -379,26 +381,7 @@ int SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop)
     }
 
     /* set up stuff to use the private key */
-#ifdef __MINGW32_VERSION
-    /* MinGW w32api is incomplete when it comes to CryptoAPI, as per version 3.1
-     * anyway. This is a hack around that problem. */
-    if (crypt32dll == NULL) {
-	crypt32dll = LoadLibrary("crypt32");
-	if (crypt32dll == NULL) {
-	    CRYPTOAPIerr(CRYPTOAPI_F_LOAD_LIBRARY);
-	    goto err;
-	}
-    }
-    if (OpenVPNCryptAcquireCertificatePrivateKey == NULL) {
-	OpenVPNCryptAcquireCertificatePrivateKey = GetProcAddress(crypt32dll,
-		"CryptAcquireCertificatePrivateKey");
-	if (OpenVPNCryptAcquireCertificatePrivateKey == NULL) {
-	    CRYPTOAPIerr(CRYPTOAPI_F_GET_PROC_ADDRESS);
-	    goto err;
-	}
-    }
-#endif
-    if (!OpenVPNCryptAcquireCertificatePrivateKey(cd->cert_context, CRYPT_ACQUIRE_COMPARE_KEY_FLAG,
+    if (!CryptAcquireCertificatePrivateKey(cd->cert_context, CRYPT_ACQUIRE_COMPARE_KEY_FLAG,
 	    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:
Signature · 1 line
1.7.5.4
#1 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 6 lines · Heiko Hund

The CryptoAPI function CryptAcquireCertificatePrivateKey() was previously unsupported in MinGW. With recent w32api headers it's now defined, mostly. Since the code used to load the CryptoAPI DLL is prone to a DLL hijacking attack [1], it's now a good time to get rid of wrapper completely.

Quoted 8 lines

Just a few macros left that may still be undefined on some systems using the original MinGW headers.

[1] http://isc.sans.edu/diary.html?storyid=9445

Signed-off-by Heiko Hund <[email protected]...> --- cryptoapi.c |

43 +++++++++++++------------------------------ 1 files changed, 13 insertions(+), 30 deletions(-)

One more old patch recovered from the dust. Applied to master branch on
- -testing and -stable trees.

commit 14a382a3f1c70fdbc822bcad27096040ed394661

Author: Heiko Hund <[email protected]...>
Date:   Wed Aug 17 15:53:01 2011 +0000
    remove wrapper code for Windows CryptoAPI function

    Signed-off-by: Heiko Hund <[email protected]...>
    Acked-by: James Yonan <[email protected]...>
    URL: http://article.gmane.org/gmane.network.openvpn.devel/4979
    Signed-off-by: David Sommerseth <[email protected]...>

kind regards,

David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk9DoPMACgkQDC186MBRfromnwCeP8qqwfaEtZ1//ql+AAVM3RMG 6M0AoInMli3/80vp1zFCL42O26iUyTUP
=LpaP
-----END PGP SIGNATURE-----