[Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)

[Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)

[Faidon Liambotis]

Alon Bar-Lev wrote:
> On 9/22/07, Faidon Liambotis <paravoid@...334...> wrote:
>> Alon Bar-Lev wrote:
>>> So you need to use CertVerifyCertificateChainPolicy() with CERT_CHAIN_POLICY_SSL
>> I'm no Microsoft developer (adn I don't want to be to be honest) but if
>> I understand it right, it's better to call CertGetCertificateChain() as
>> I am doing.
> 
> You need to use both, one for create the chain and the other to verify
> that it meets with system CTL for SSL.
Seems that you are right. Below you will find -v4 of the patch that does
that.

Also, my previous version didn't actually check for revocations,
contrary to what I documented.
I added CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT to the dwFlags of
CertGetCertificateChain.

Let me know what you think.

Thanks,
Faidon

--

diff -urp openvpn-2.1_rc4.orig/cryptoapi.c openvpn-2.1_rc4/cryptoapi.c
--- openvpn-2.1_rc4.orig/cryptoapi.c	2007-04-26 00:38:46.000000000 +0300
+++ openvpn-2.1_rc4/cryptoapi.c	2007-09-23 11:07:14.000000000 +0300
@@ -45,9 +45,15 @@
 #define CERT_STORE_READONLY_FLAG 0x00008000
 #define CERT_STORE_OPEN_EXISTING_FLAG 0x00004000
 #define CRYPT_ACQUIRE_COMPARE_KEY_FLAG 0x00000004
+#define CERT_CHAIN_REVOCATION_CHECK_END_CERT           0x10000000
+#define CERT_CHAIN_REVOCATION_CHECK_CHAIN              0x20000000
+#define CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT 0x40000000
+#define CERT_CHAIN_REVOCATION_CHECK_CACHE_ONLY         0x80000000
 static HINSTANCE crypt32dll = NULL;
 static BOOL WINAPI (*CryptAcquireCertificatePrivateKey) (PCCERT_CONTEXT pCert, DWORD dwFlags,
     void *pvReserved, HCRYPTPROV *phCryptProv, DWORD *pdwKeySpec, BOOL *pfCallerFreeProv) = NULL;
+static PCCERT_CONTEXT WINAPI (*CertCreateCertificateContext) (DWORD dwCertEncodingType,
+     const BYTE *pbCertEncoded, DWORD cbCertEncoded) = NULL;
 #endif
 
 /* Size of an SSL signature: MD5+SHA1 */
@@ -65,6 +71,9 @@ static BOOL WINAPI (*CryptAcquireCertifi
 #define CRYPTOAPI_F_CRYPT_SIGN_HASH			    106
 #define CRYPTOAPI_F_LOAD_LIBRARY			    107
 #define CRYPTOAPI_F_GET_PROC_ADDRESS			    108
+#define CRYPTOAPI_F_CERT_CREATE_CERT_CONTEXT		    109
+#define CRYPTOAPI_F_CERT_GET_CERT_CHAIN			    110
+#define CRYPTOAPI_F_CERT_VERIFY_CERT_CHAIN_POLICY	    111
 
 static ERR_STRING_DATA CRYPTOAPI_str_functs[] =	{
     { ERR_PACK(ERR_LIB_CRYPTOAPI, 0, 0),				    "microsoft cryptoapi"},
@@ -77,6 +86,9 @@ static ERR_STRING_DATA CRYPTOAPI_str_fun
     { 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_CERT_CREATE_CERT_CONTEXT, 0),		    "CertCreateCertificateContext" },
+    { ERR_PACK(0, CRYPTOAPI_F_CERT_GET_CERT_CHAIN, 0),			    "CertGetCertificateChain" },
+    { ERR_PACK(0, CRYPTOAPI_F_CERT_VERIFY_CERT_CHAIN_POLICY, 0),	    "CertVerifyCertificateChainPolicy" },
     { 0, NULL }
 };
 
@@ -364,7 +376,7 @@ int SSL_CTX_use_CryptoAPI_certificate(SS
     }
 
     /* cert_context->pbCertEncoded is the cert X509 DER encoded. */
-    cert = d2i_X509(NULL, (unsigned char **) &cd->cert_context->pbCertEncoded,
+    cert = d2i_X509(NULL, (const unsigned char **) &cd->cert_context->pbCertEncoded,
 		    cd->cert_context->cbCertEncoded);
     if (cert == NULL) {
 	SSLerr(SSL_F_SSL_CTX_USE_CERTIFICATE_FILE, ERR_R_ASN1_LIB);
@@ -461,3 +473,96 @@ int SSL_CTX_use_CryptoAPI_certificate(SS
     }
     return 0;
 }
+
+int CryptoAPI_verify_certificate(X509 *x509)
+{
+  int ret = -1;
+  int len;
+  unsigned char *buf = NULL;
+
+  PCCERT_CONTEXT pCertContext = NULL;
+  PCCERT_CHAIN_CONTEXT pChainContext = NULL;
+  CERT_ENHKEY_USAGE EnhkeyUsage;
+  CERT_USAGE_MATCH CertUsage;  
+  CERT_CHAIN_PARA ChainPara;
+  CERT_CHAIN_POLICY_PARA PolicyPara;
+  CERT_CHAIN_POLICY_STATUS PolicyStatus;
+
+  /* Convert from internal X509 format to DER */
+  len = i2d_X509(x509, &buf);
+  if (len < 0) {
+	SSLerr(SSL_F_SSL_CTX_USE_CERTIFICATE_FILE, ERR_R_ASN1_LIB);
+	goto err;
+  }
+
+#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 (CertCreateCertificateContext == NULL) {
+	CertCreateCertificateContext = GetProcAddress(crypt32dll,
+		"CertCreateCertificateContext");
+	if (CertCreateCertificateContext == NULL) {
+	    CRYPTOAPIerr(CRYPTOAPI_F_GET_PROC_ADDRESS);
+	    goto err;
+	}
+    }
+#endif
+
+  /* Create a certificate context based on the above certificate */
+  pCertContext = CertCreateCertificateContext(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
+  	buf, len);
+  if (pCertContext == NULL) {
+	CRYPTOAPIerr(CRYPTOAPI_F_CERT_CREATE_CERT_CONTEXT);
+	goto err;
+  }
+
+  /* Create an empty issuer list */
+  EnhkeyUsage.cUsageIdentifier = 0;
+  EnhkeyUsage.rgpszUsageIdentifier = NULL;
+  CertUsage.dwType = USAGE_MATCH_TYPE_AND;
+  CertUsage.Usage  = EnhkeyUsage;
+
+  /* Searching and matching criteria to be used when building the chain */
+  ChainPara.cbSize = sizeof(CERT_CHAIN_PARA);
+  ChainPara.RequestedUsage = CertUsage;
+
+  /* Get the certificate chain of our certificate */
+  if (!CertGetCertificateChain(NULL, pCertContext, NULL, NULL, &ChainPara,
+	CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT,
+	NULL, &pChainContext)) {
+	CRYPTOAPIerr(CRYPTOAPI_F_CERT_GET_CERT_CHAIN);
+	goto err;
+  }
+
+  PolicyPara.cbSize = sizeof(CERT_CHAIN_POLICY_PARA);
+  PolicyPara.dwFlags = 0;
+  PolicyPara.pvExtraPolicyPara = NULL;
+
+  PolicyStatus.cbSize = sizeof(CERT_CHAIN_POLICY_STATUS);
+
+  if (!CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_SSL, pChainContext,
+					&PolicyPara, &PolicyStatus)) {
+	CRYPTOAPIerr(CRYPTOAPI_F_CERT_VERIFY_CERT_CHAIN_POLICY);
+	goto err;
+  }
+
+  /* return 1 when the certificate is trusted, 0 when it's not */
+  ret = !PolicyStatus.dwError;
+
+  err:
+    if (buf)
+	OPENSSL_free(buf);
+    if (pChainContext)
+	CertFreeCertificateChain(pChainContext);
+    if (pCertContext)
+	CertFreeCertificateContext(pCertContext);
+
+  return ret;
+}
diff -urp openvpn-2.1_rc4.orig/cryptoapi.h openvpn-2.1_rc4/cryptoapi.h
--- openvpn-2.1_rc4.orig/cryptoapi.h	2007-04-26 00:38:46.000000000 +0300
+++ openvpn-2.1_rc4/cryptoapi.h	2007-09-23 11:06:05.000000000 +0300
@@ -2,6 +2,7 @@
 #define _CRYPTOAPI_H_
 
 int SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop);
+int CryptoAPI_verify_certificate(X509 *x509);
 
 
 #endif /* !_CRYPTOAPI_H_ */
diff -urp openvpn-2.1_rc4.orig/init.c openvpn-2.1_rc4/init.c
--- openvpn-2.1_rc4.orig/init.c	2007-04-26 00:38:46.000000000 +0300
+++ openvpn-2.1_rc4/init.c	2007-09-23 11:06:05.000000000 +0300
@@ -1558,6 +1558,10 @@ do_init_crypto_tls (struct context *c, c
   to.remote_cert_eku = options->remote_cert_eku;
   to.es = c->c2.es;
 
+#ifdef WIN32
+  to.cryptoapi_ca = options->cryptoapi_ca;
+#endif
+
 #ifdef ENABLE_DEBUG
   to.gremlin = c->options.gremlin;
 #endif
diff -urp openvpn-2.1_rc4.orig/openvpn.8 openvpn-2.1_rc4/openvpn.8
--- openvpn-2.1_rc4.orig/openvpn.8	2007-04-26 00:38:46.000000000 +0300
+++ openvpn-2.1_rc4/openvpn.8	2007-09-23 11:06:05.000000000 +0300
@@ -124,6 +124,7 @@ openvpn \- secure IP tunnel daemon.
 [\ \fB\-\-connect\-retry\-max\fR\ \fIn\fR\ ]
 [\ \fB\-\-crl\-verify\fR\ \fIcrl\fR\ ]
 [\ \fB\-\-cryptoapicert\fR\ \fIselect\-string\fR\ ]
+[\ \fB\-\-cryptoapica\fR\ ]
 [\ \fB\-\-daemon\fR\ \fI[progname]\fR\ ]
 [\ \fB\-\-dev\-node\fR\ \fInode\fR\ ]
 [\ \fB\-\-dev\-type\fR\ \fIdevice\-type\fR\ ]
@@ -3769,7 +3770,26 @@ To select a certificate, based on certif
 
 The thumbprint hex string can easily be copy-and-pasted from the Windows
 Certificate Store GUI.
+.\"*********************************************************
+.TP
+.B --cryptoapica
+Use the Windows Certficate System Store to verify presented certificates
+(Windows Only).
+
+Use this option instead of or in addition to
+.B --ca.
+
+This makes it possible to verify certificates against the Certificate Store
+instead of having OpenVPN read the certificate authority (CA) file.
+Windows Certificate Store checks Certificate Revocation Lists (CRLs)
+automatically and will reject revoked certificates.
 
+The Certificate Store includes by default many known Certificate Authorities.
+Use this setting with extreme caution and preferrably in combination with
+.B --tls-verify
+or
+.B --tls-remote
+options.
 .\"*********************************************************
 .TP
 .B --key-method m
diff -urp openvpn-2.1_rc4.orig/options.c openvpn-2.1_rc4/options.c
--- openvpn-2.1_rc4.orig/options.c	2007-04-26 00:38:46.000000000 +0300
+++ openvpn-2.1_rc4/options.c	2007-09-23 11:06:05.000000000 +0300
@@ -450,6 +450,8 @@ static const char usage_message[] =
 #ifdef WIN32
   "--cryptoapicert select-string : Load the certificate and private key from the\n"
   "                  Windows Certificate System Store.\n"
+  "--cryptoapica   : Check against Certificate Authorities stored in Windows\n"
+  "                  Certificate System Store.\n"
 #endif
   "--tls-cipher l  : A list l of allowable TLS ciphers separated by : (optional).\n"
   "                : Use --show-tls to see a list of supported TLS ciphers.\n"
@@ -1236,6 +1238,7 @@ show_settings (const struct options *o)
   SHOW_STR (pkcs12_file);
 #ifdef WIN32
   SHOW_STR (cryptoapi_cert);
+  SHOW_BOOL (cryptoapi_ca);
 #endif
   SHOW_STR (cipher_list);
   SHOW_STR (tls_verify);
@@ -1809,8 +1812,8 @@ options_postprocess (struct options *opt
 #ifdef WIN32
       if (options->cryptoapi_cert)
 	{
-	  if ((!(options->ca_file)) && (!(options->ca_path)))
-	    msg(M_USAGE, "You must define CA file (--ca) or CA path (--capath)");
+	  if ((!(options->ca_file)) && (!(options->ca_path)) && (!(options->cryptoapi_ca)))
+	    msg(M_USAGE, "You must define CA file (--ca) or CA path (--capath) or CryptoAPI CA (--cryptoapica)");
           if (options->cert_file)
 	    msg(M_USAGE, "Parameter --cert cannot be used when --cryptoapicert is also specified.");
           if (options->priv_key_file)
@@ -1831,8 +1834,13 @@ options_postprocess (struct options *opt
         }
       else
         {
+#ifdef WIN32
+	  if ((!(options->ca_file)) && (!(options->ca_path)) && (!(options->cryptoapi_ca)))
+	    msg(M_USAGE, "You must define CA file (--ca) or CA path (--capath) or CryptoAPI CAs (--cryptoapica)");
+#else
 	  if ((!(options->ca_file)) && (!(options->ca_path)))
 	    msg(M_USAGE, "You must define CA file (--ca) or CA path (--capath)");
+#endif
 	  if (pull)
 	    {
 	      const int sum = (options->cert_file != NULL) + (options->priv_key_file != NULL);
@@ -4889,6 +4897,11 @@ add_option (struct options *options,
       VERIFY_PERMISSION (OPT_P_GENERAL);
       options->cryptoapi_cert = p[1];
     }
+  else if (streq (p[0], "cryptoapica"))
+    {
+      VERIFY_PERMISSION (OPT_P_GENERAL);
+      options->cryptoapi_ca = true;
+    }
 #endif
   else if (streq (p[0], "key") && p[1])
     {
diff -urp openvpn-2.1_rc4.orig/options.h openvpn-2.1_rc4/options.h
--- openvpn-2.1_rc4.orig/options.h	2007-04-26 00:38:46.000000000 +0300
+++ openvpn-2.1_rc4/options.h	2007-09-23 11:06:05.000000000 +0300
@@ -424,6 +424,7 @@ struct options
 
 #ifdef WIN32
   const char *cryptoapi_cert;
+  bool cryptoapi_ca;
 #endif
 
   /* data channel key exchange method */
diff -urp openvpn-2.1_rc4.orig/proxy.c openvpn-2.1_rc4/proxy.c
--- openvpn-2.1_rc4.orig/proxy.c	2007-04-26 00:38:46.000000000 +0300
+++ openvpn-2.1_rc4/proxy.c	2007-09-23 11:06:05.000000000 +0300
@@ -39,7 +39,7 @@
 #include "base64.h"
 #include "ntlm.h"
 
-#ifdef WIN32
+#if defined(WIN32) && !defined(__MINGW32__)
 #include "ieproxy.h"
 #endif
 
diff -urp openvpn-2.1_rc4.orig/ssl.c openvpn-2.1_rc4/ssl.c
--- openvpn-2.1_rc4.orig/ssl.c	2007-04-26 00:38:46.000000000 +0300
+++ openvpn-2.1_rc4/ssl.c	2007-09-23 11:06:05.000000000 +0300
@@ -543,13 +543,19 @@ verify_callback (int preverify_ok, X509_
   msg (D_LOW, "X509: %s", subject);
 #endif
 
-  /* did peer present cert which was signed our root cert? */
+  /* did peer present cert which was signed by our root cert? */
   if (!preverify_ok)
     {
-      /* Remote site specified a certificate, but it's not correct */
-      msg (D_TLS_ERRORS, "VERIFY ERROR: depth=%d, error=%s: %s",
-	   ctx->error_depth, X509_verify_cert_error_string (ctx->error), subject);
-      goto err;			/* Reject connection */
+#ifdef WIN32
+     /* if cryptoapica was not enabled, fail; otherwise check against the CA Store */
+     if (!(opt->cryptoapi_ca && CryptoAPI_verify_certificate(ctx->current_cert)))
+#endif
+      {
+        /* Remote site specified a certificate, but it's not correct */
+        msg (D_TLS_ERRORS, "VERIFY ERROR: depth=%d, error=%s: %s",
+	     ctx->error_depth, X509_verify_cert_error_string (ctx->error), subject);
+        goto err;			/* Reject connection */
+      }
     }
 
   /* warn if cert chain is too deep */
diff -urp openvpn-2.1_rc4.orig/ssl.h openvpn-2.1_rc4/ssl.h
--- openvpn-2.1_rc4.orig/ssl.h	2007-04-26 00:38:46.000000000 +0300
+++ openvpn-2.1_rc4/ssl.h	2007-09-23 11:06:05.000000000 +0300
@@ -415,6 +415,9 @@ struct tls_options
   int ns_cert_type;
   unsigned remote_cert_ku[MAX_PARMS];
   const char *remote_cert_eku;
+#ifdef WIN32
+  bool cryptoapi_ca;
+#endif
 
   /* allow openvpn config info to be
      passed over control channel */

Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)

[Faidon Liambotis]

Faidon Liambotis wrote:
> Alon Bar-Lev wrote:
>> You need to use both, one for create the chain and the other to verify
>> that it meets with system CTL for SSL.
> Seems that you are right. Below you will find -v4 of the patch that does
> that.
> 
> Also, my previous version didn't actually check for revocations,
> contrary to what I documented.
> I added CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT to the dwFlags of
> CertGetCertificateChain.
> 
> Let me know what you think.
Alon, ping?

I'd really like to have this patch merged this time.

If you have any more comments or objections, just say so so I can fix them.

If you don't, just say so, so it can get merged.

Thanks,
Faidon

Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)

[Alon Bar-Lev]

On 9/27/07, Faidon Liambotis <paravoid@...334...> wrote:
> Alon, ping?

Oh... missed your post.
Please don't keep change the subject line.
And replay all to every correction.

> I'd really like to have this patch merged this time.
> If you don't, just say so, so it can get merged.

Only James can merge patches...
I only review this as I can help you improve it.

> If you have any more comments or objections, just say so so I can fix them.

Some more notes:
1. For CertGetCertificateChain() you should use the following flags:
CERT_CHAIN_REVOCATION_CHECK_CHAIN
So that CA may suicide.

2. CertVerifyCertificateChainPolicy()

PolicyPara extra should be set:
fdwChecks should be SECURITY_FLAG_IGNORE_CERT_CN_INVALID
as we check subject name within OpenVPN.

dwAuthType should be AUTHTYPE_CLIENT or AUTHTYPE_SERVER depending on
the configuration. I think that peer-to-peer should be
AUTHTYPE_SERVER.

3. I am not sure you require:
ChainPara.RequestedUsage = <something>
Have you tried to leave it zero?

4. I guess a cleaner code would load crypt32.dll once in the module,
moving the LoadLibrary and GetProcAddress to a single function,
calling it from initialization code.
Also if you already mess with this module, I guess a cleanup code
should be added to free the library during termination... :)

Best Regards,
Alon Bar-Lev.

Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)

[Jason R. Coombs]

I'd really like to see this feature in a future build.

Is there anything more that needs to be done to integrate this into 2.1?
I can help with code cleanup/refactoring.  I don't have a development
environment set up, though, so I'd be working blind.

Cheers,
Jason

On 9/23/07, Faidon Liambotis Wrote:
Alon Bar-Lev wrote:
> On 9/22/07, Faidon Liambotis <paravoid@...1131...> wrote:
>> Alon Bar-Lev wrote:
>>> So you need to use CertVerifyCertificateChainPolicy() with
CERT_CHAIN_POLICY_SSL
>> I'm no Microsoft developer (adn I don't want to be to be honest) but
if
>> I understand it right, it's better to call CertGetCertificateChain()
as
>> I am doing.
> 
> You need to use both, one for create the chain and the other to verify
> that it meets with system CTL for SSL.
Seems that you are right. Below you will find -v4 of the patch that does
that.

Also, my previous version didn't actually check for revocations,
contrary to what I documented.
I added CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT to the dwFlags of
CertGetCertificateChain.

Let me know what you think.

Thanks,
Faidon

Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)

[Alon Bar-Lev]

Hello,

I cleaned it up a little but still things should be done:

1. Add initialize code and load all entry points for this module at
one place, single LoadLibrary etc...
2. Add cleanup code to unload all static module resources.
3. Check SSL role by OpenSSL configuration (client or server), see
TODO signature.
4. Cleanup warnings.

Available at [1], I did not check it as I don't have active Windows
configuration now.
Can you please complete it?

Alon.

[1] http://svn.openvpn.net/projects/openvpn/contrib/alon/BETA21-ms-chk-2/openvpn

On 9/24/08, Jason R. Coombs <jaraco@...1130...> wrote:
> I'd really like to see this feature in a future build.
>
>  Is there anything more that needs to be done to integrate this into 2.1?
>  I can help with code cleanup/refactoring.  I don't have a development
>  environment set up, though, so I'd be working blind.
>
>  Cheers,
>  Jason
>
>  On 9/23/07, Faidon Liambotis Wrote:
>  Alon Bar-Lev wrote:
>  > On 9/22/07, Faidon Liambotis <paravoid@...1131...> wrote:
>  >> Alon Bar-Lev wrote:
>  >>> So you need to use CertVerifyCertificateChainPolicy() with
>  CERT_CHAIN_POLICY_SSL
>  >> I'm no Microsoft developer (adn I don't want to be to be honest) but
>  if
>  >> I understand it right, it's better to call CertGetCertificateChain()
>  as
>  >> I am doing.
>  >
>  > You need to use both, one for create the chain and the other to verify
>  > that it meets with system CTL for SSL.
>  Seems that you are right. Below you will find -v4 of the patch that does
>  that.
>
>  Also, my previous version didn't actually check for revocations,
>  contrary to what I documented.
>  I added CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT to the dwFlags of
>  CertGetCertificateChain.
>
>  Let me know what you think.
>
>  Thanks,
>  Faidon
>
>
>  -------------------------------------------------------------------------
>  This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
>  Build the coolest Linux based applications with Moblin SDK & win great prizes
>  Grand prize is a trip for two to an Open Source event anywhere in the world
>  http://moblin-contest.org/redirect.php?banner_id=100&url=/
>  _______________________________________________
>  Openvpn-devel mailing list
>  Openvpn-devel@lists.sourceforge.net
>  https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>

Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)

[Jason R. Coombs]

[-- Attachment #1: Type: text/plain, Size: 3779 bytes --]

Alon,
	I've started working on (1) and (2).  Attached is the updated cryptoapi.c.  Would you prefer a patch when changes involve a single file?  Can you tell me what you think (is this moving in the right direction)?  Initially, I've only moved the LoadLibrary code into its own function, but it's still called from the same place.  Should I go further and move this initialization code somewhere else?  If so, can you suggest where I should look to hook in the initialization?

	As for (2), I've created a function that unloads the library and clears out the variables... but it's not called from anywhere.  I guess if I know where the initialization will go, then I can find a good place from which to call the cleanup code.

	As for (3), where is the SSL role defined?  Or, alternatively, what is the procedure when cryptoapi isn't used to verify the certificate is in the correct role?

	I hope to contribute further.  This is my first time looking at the openvpn source, so please bear with me (or let me know it's not worth your time).

Regards,
Jason

-----Original Message-----
From: Alon Bar-Lev [mailto:alon.barlev@...277...] 
Sent: Thursday, 25 September, 2008 01:10
To: Jason R. Coombs
Cc: openvpn-devel@lists.sourceforge.net
Subject: Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)

Hello,

I cleaned it up a little but still things should be done:

1. Add initialize code and load all entry points for this module at
one place, single LoadLibrary etc...
2. Add cleanup code to unload all static module resources.
3. Check SSL role by OpenSSL configuration (client or server), see
TODO signature.
4. Cleanup warnings.

Available at [1], I did not check it as I don't have active Windows
configuration now.
Can you please complete it?

Alon.

[1] http://svn.openvpn.net/projects/openvpn/contrib/alon/BETA21-ms-chk-2/openvpn

On 9/24/08, Jason R. Coombs <jaraco@...1130...> wrote:
> I'd really like to see this feature in a future build.
>
>  Is there anything more that needs to be done to integrate this into 2.1?
>  I can help with code cleanup/refactoring.  I don't have a development
>  environment set up, though, so I'd be working blind.
>
>  Cheers,
>  Jason
>
>  On 9/23/07, Faidon Liambotis Wrote:
>  Alon Bar-Lev wrote:
>  > On 9/22/07, Faidon Liambotis <paravoid@...1131...> wrote:
>  >> Alon Bar-Lev wrote:
>  >>> So you need to use CertVerifyCertificateChainPolicy() with
>  CERT_CHAIN_POLICY_SSL
>  >> I'm no Microsoft developer (adn I don't want to be to be honest) but
>  if
>  >> I understand it right, it's better to call CertGetCertificateChain()
>  as
>  >> I am doing.
>  >
>  > You need to use both, one for create the chain and the other to verify
>  > that it meets with system CTL for SSL.
>  Seems that you are right. Below you will find -v4 of the patch that does
>  that.
>
>  Also, my previous version didn't actually check for revocations,
>  contrary to what I documented.
>  I added CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT to the dwFlags of
>  CertGetCertificateChain.
>
>  Let me know what you think.
>
>  Thanks,
>  Faidon
>
>
>  -------------------------------------------------------------------------
>  This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
>  Build the coolest Linux based applications with Moblin SDK & win great prizes
>  Grand prize is a trip for two to an Open Source event anywhere in the world
>  http://moblin-contest.org/redirect.php?banner_id=100&url=/
>  _______________________________________________
>  Openvpn-devel mailing list
>  Openvpn-devel@lists.sourceforge.net
>  https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>

[-- Attachment #2: cryptoapi.c --]
[-- Type: application/octet-stream, Size: 20434 bytes --]

/*
 * Copyright (c) 2004 Peter 'Luna' Runestig <peter@runestig.com>
 * All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without modifi-
 * cation, are permitted provided that the following conditions are met:
 *
 *   o  Redistributions of source code must retain the above copyright notice,
 *      this list of conditions and the following disclaimer.
 *
 *   o  Redistributions in binary form must reproduce the above copyright no-
 *      tice, this list of conditions and the following disclaimer in the do-
 *      cumentation and/or other materials provided with the distribution.
 *
 *   o  The names of the contributors may not be used to endorse or promote
 *      products derived from this software without specific prior written
 *      permission.
 *
 * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
 * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
 * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
 * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LI-
 * ABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUEN-
 * TIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
 * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEV-
 * ER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABI-
 * LITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
 * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */

#include "syshead.h"

#if defined(WIN32) && defined(USE_CRYPTO) && defined(USE_SSL)

#include <openssl/ssl.h>
#include <openssl/err.h>
#include <windows.h>
#include <wincrypt.h>
#include <stdio.h>
#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 AUTHTYPE_CLIENT 1
#define AUTHTYPE_SERVER 2
#define CALG_SSL3_SHAMD5 (ALG_CLASS_HASH | ALG_TYPE_ANY | ALG_SID_SSL3SHAMD5)
#define CERT_SYSTEM_STORE_LOCATION_SHIFT 16
#define CERT_SYSTEM_STORE_CURRENT_USER_ID 1
#define CERT_SYSTEM_STORE_CURRENT_USER (CERT_SYSTEM_STORE_CURRENT_USER_ID << CERT_SYSTEM_STORE_LOCATION_SHIFT)
#define CERT_STORE_READONLY_FLAG 0x00008000
#define CERT_STORE_OPEN_EXISTING_FLAG 0x00004000
#define CRYPT_ACQUIRE_COMPARE_KEY_FLAG 0x00000004
#define CERT_CHAIN_REVOCATION_CHECK_END_CERT           0x10000000
#define CERT_CHAIN_REVOCATION_CHECK_CHAIN              0x20000000
#define CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT 0x40000000
#define CERT_CHAIN_REVOCATION_CHECK_CACHE_ONLY         0x80000000
#endif

static HINSTANCE crypt32dll = NULL;
static BOOL WINAPI (*my_CryptAcquireCertificatePrivateKey) (PCCERT_CONTEXT pCert, DWORD dwFlags,
    void *pvReserved, HCRYPTPROV *phCryptProv, DWORD *pdwKeySpec, BOOL *pfCallerFreeProv) = NULL;
static PCCERT_CONTEXT WINAPI (*my_CertCreateCertificateContext) (DWORD dwCertEncodingType,
     const BYTE *pbCertEncoded, DWORD cbCertEncoded) = NULL;

/* Size of an SSL signature: MD5+SHA1 */
#define SSL_SIG_LENGTH	36

/* try to funnel any Windows/CryptoAPI error messages to OpenSSL ERR_... */
#define ERR_LIB_CRYPTOAPI (ERR_LIB_USER + 69)	/* 69 is just a number... */
#define CRYPTOAPIerr(f)   err_put_ms_error(GetLastError(), (f), __FILE__, __LINE__)
#define CRYPTOAPI_F_CERT_OPEN_SYSTEM_STORE		    100
#define CRYPTOAPI_F_CERT_FIND_CERTIFICATE_IN_STORE	    101
#define CRYPTOAPI_F_CRYPT_ACQUIRE_CERTIFICATE_PRIVATE_KEY   102
#define CRYPTOAPI_F_CRYPT_CREATE_HASH			    103
#define CRYPTOAPI_F_CRYPT_GET_HASH_PARAM		    104
#define CRYPTOAPI_F_CRYPT_SET_HASH_PARAM		    105
#define CRYPTOAPI_F_CRYPT_SIGN_HASH			    106
#define CRYPTOAPI_F_LOAD_LIBRARY			    107
#define CRYPTOAPI_F_GET_PROC_ADDRESS			    108
#define CRYPTOAPI_F_CERT_CREATE_CERT_CONTEXT		    109
#define CRYPTOAPI_F_CERT_GET_CERT_CHAIN			    110
#define CRYPTOAPI_F_CERT_VERIFY_CERT_CHAIN_POLICY	    111

static ERR_STRING_DATA CRYPTOAPI_str_functs[] =	{
    { ERR_PACK(ERR_LIB_CRYPTOAPI, 0, 0),				    "microsoft cryptoapi"},
    { ERR_PACK(0, CRYPTOAPI_F_CERT_OPEN_SYSTEM_STORE, 0),		    "CertOpenSystemStore" },
    { ERR_PACK(0, CRYPTOAPI_F_CERT_FIND_CERTIFICATE_IN_STORE, 0),	    "CertFindCertificateInStore" },
    { ERR_PACK(0, CRYPTOAPI_F_CRYPT_ACQUIRE_CERTIFICATE_PRIVATE_KEY, 0),    "CryptAcquireCertificatePrivateKey" },
    { ERR_PACK(0, CRYPTOAPI_F_CRYPT_CREATE_HASH, 0),			    "CryptCreateHash" },
    { ERR_PACK(0, CRYPTOAPI_F_CRYPT_GET_HASH_PARAM, 0),			    "CryptGetHashParam" },
    { ERR_PACK(0, CRYPTOAPI_F_CRYPT_SET_HASH_PARAM, 0),			    "CryptSetHashParam" },
    { 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_CERT_CREATE_CERT_CONTEXT, 0),		    "CertCreateCertificateContext" },
    { ERR_PACK(0, CRYPTOAPI_F_CERT_GET_CERT_CHAIN, 0),			    "CertGetCertificateChain" },
    { ERR_PACK(0, CRYPTOAPI_F_CERT_VERIFY_CERT_CHAIN_POLICY, 0),	    "CertVerifyCertificateChainPolicy" },
    { 0, NULL }
};

typedef struct _CAPI_DATA {
    const CERT_CONTEXT *cert_context;
    HCRYPTPROV crypt_prov;
    DWORD key_spec;
    BOOL free_crypt_prov;
} CAPI_DATA;

static char *ms_error_text(DWORD ms_err)
{
    LPVOID lpMsgBuf = NULL;
    char *rv = NULL;

    FormatMessage(
	FORMAT_MESSAGE_ALLOCATE_BUFFER |
	FORMAT_MESSAGE_FROM_SYSTEM |
	FORMAT_MESSAGE_IGNORE_INSERTS,
	NULL, ms_err,
	MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), /* Default language */
	(LPTSTR) &lpMsgBuf, 0, NULL);
    if (lpMsgBuf) {
	char *p;
	rv = strdup(lpMsgBuf);
	LocalFree(lpMsgBuf);
	/* trim to the left */
	if (rv)
	    for (p = rv + strlen(rv) - 1; p >= rv; p--) {
		if (isspace(*p))
		    *p = '\0';
		else
		    break;
	    }
    }
    return rv;
}

static void err_put_ms_error(DWORD ms_err, int func, const char *file, int line)
{
    static int init = 0;
#   define ERR_MAP_SZ 16
    static struct {
	int err;
	DWORD ms_err;	    /* I don't think we get more than 16 *different* errors */
    } err_map[ERR_MAP_SZ];  /* in here, before we give up the whole thing...        */
    int i;

    if (ms_err == 0)
	/* 0 is not an error */
	return;
    if (!init) {
	ERR_load_strings(ERR_LIB_CRYPTOAPI, CRYPTOAPI_str_functs);
	memset(&err_map, 0, sizeof(err_map));
	init++;
    }
    /* since MS error codes are 32 bit, and the ones in the ERR_... system is
     * only 12, we must have a mapping table between them.  */
    for (i = 0; i < ERR_MAP_SZ; i++) {
	if (err_map[i].ms_err == ms_err) {
	    ERR_PUT_error(ERR_LIB_CRYPTOAPI, func, err_map[i].err, file, line);
	    break;
	} else if (err_map[i].ms_err == 0 ) {
	    /* end of table, add new entry */
	    ERR_STRING_DATA *esd = calloc(2, sizeof(*esd));
	    if (esd == NULL)
		break;
	    err_map[i].ms_err = ms_err;
	    err_map[i].err = esd->error = i + 100;
	    esd->string = ms_error_text(ms_err);
	    ERR_load_strings(ERR_LIB_CRYPTOAPI, esd);
	    ERR_PUT_error(ERR_LIB_CRYPTOAPI, func, err_map[i].err, file, line);
	    break;
	}
    }
}

/* encrypt */
static int rsa_pub_enc(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, int padding)
{
    /* I haven't been able to trigger this one, but I want to know if it happens... */
    assert(0);

    return 0;
}

/* verify arbitrary data */
static int rsa_pub_dec(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, int padding)
{
    /* I haven't been able to trigger this one, but I want to know if it happens... */
    assert(0);

    return 0;
}

/* sign arbitrary data */
static int rsa_priv_enc(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, int padding)
{
    CAPI_DATA *cd = (CAPI_DATA *) rsa->meth->app_data;
    HCRYPTHASH hash;
    DWORD hash_size, len, i;
    unsigned char *buf;

    if (cd == NULL) {
	RSAerr(RSA_F_RSA_EAY_PRIVATE_ENCRYPT, ERR_R_PASSED_NULL_PARAMETER);
	return 0;
    }
    if (padding != RSA_PKCS1_PADDING) {
	/* AFAICS, CryptSignHash() *always* uses PKCS1 padding. */
	RSAerr(RSA_F_RSA_EAY_PRIVATE_ENCRYPT, RSA_R_UNKNOWN_PADDING_TYPE);
	return 0;
    }
    /* 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
     * from 'from' into it.  */
    /* For now, we only support NID_md5_sha1 */
    if (flen != SSL_SIG_LENGTH) {
	RSAerr(RSA_F_RSA_EAY_PRIVATE_ENCRYPT, RSA_R_INVALID_MESSAGE_LENGTH);
	return 0;
    }
    if (!CryptCreateHash(cd->crypt_prov, CALG_SSL3_SHAMD5, 0, 0, &hash)) {
	CRYPTOAPIerr(CRYPTOAPI_F_CRYPT_CREATE_HASH);
	return 0;
    }
    len = sizeof(hash_size);
    if (!CryptGetHashParam(hash, HP_HASHSIZE, (BYTE *) &hash_size, &len, 0)) {
	CRYPTOAPIerr(CRYPTOAPI_F_CRYPT_GET_HASH_PARAM);
        CryptDestroyHash(hash);
	return 0;
    }
    if ((int) hash_size != flen) {
	RSAerr(RSA_F_RSA_EAY_PRIVATE_ENCRYPT, RSA_R_INVALID_MESSAGE_LENGTH);
        CryptDestroyHash(hash);
	return 0;
    }
    if (!CryptSetHashParam(hash, HP_HASHVAL, (BYTE * ) from, 0)) {
	CRYPTOAPIerr(CRYPTOAPI_F_CRYPT_SET_HASH_PARAM);
        CryptDestroyHash(hash);
	return 0;
    }

    len = RSA_size(rsa);
    buf = malloc(len);
    if (buf == NULL) {
	RSAerr(RSA_F_RSA_EAY_PRIVATE_ENCRYPT, ERR_R_MALLOC_FAILURE);
        CryptDestroyHash(hash);
	return 0;
    }
    if (!CryptSignHash(hash, cd->key_spec, NULL, 0, buf, &len)) {
	CRYPTOAPIerr(CRYPTOAPI_F_CRYPT_SIGN_HASH);
        CryptDestroyHash(hash);
        free(buf);
	return 0;
    }
    /* and now, we have to reverse the byte-order in the result from CryptSignHash()... */
    for (i = 0; i < len; i++)
	to[i] = buf[len - i - 1];
    free(buf);

    CryptDestroyHash(hash);
    return len;
}

/* decrypt */
static int rsa_priv_dec(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, int padding)
{
    /* I haven't been able to trigger this one, but I want to know if it happens... */
    assert(0);

    return 0;
}

/* called at RSA_new */
static int init(RSA *rsa)
{

    return 0;
}

/* called at RSA_free */
static int finish(RSA *rsa)
{
    CAPI_DATA *cd = (CAPI_DATA *) rsa->meth->app_data;

    if (cd == NULL)
	return 0;
    if (cd->crypt_prov && cd->free_crypt_prov)
	CryptReleaseContext(cd->crypt_prov, 0);
    if (cd->cert_context)
	CertFreeCertificateContext(cd->cert_context);
    free(rsa->meth->app_data);
    free((char *) rsa->meth);
    rsa->meth = NULL;
    return 1;
}

static const CERT_CONTEXT *find_certificate_in_store(const char *cert_prop, HCERTSTORE cert_store)
{
    /* Find, and use, the desired certificate from the store. The
     * 'cert_prop' certificate search string can look like this:
     * SUBJ:<certificate substring to match>
     * THUMB:<certificate thumbprint hex value>, e.g.
     *     THUMB:f6 49 24 41 01 b4 fb 44 0c ce f4 36 ae d0 c4 c9 df 7a b6 28
     */
    const CERT_CONTEXT *rv = NULL;

    if (!strncmp(cert_prop, "SUBJ:", 5)) {
	/* skip the tag */
	cert_prop += 5;
	rv = CertFindCertificateInStore(cert_store, X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
		0, CERT_FIND_SUBJECT_STR_A, cert_prop, NULL);

    } else if (!strncmp(cert_prop, "THUMB:", 6)) {
	unsigned char hash[255];
	char *p;
	int i, x = 0;
	CRYPT_HASH_BLOB blob;

	/* skip the tag */
	cert_prop += 6;
	for (p = (char *) cert_prop, i = 0; *p && i < sizeof(hash); i++) {
	    if (*p >= '0' && *p <= '9')
		x = (*p - '0') << 4;
	    else if (*p >= 'A' && *p <= 'F')
		x = (*p - 'A' + 10) << 4;
	    else if (*p >= 'a' && *p <= 'f')
		x = (*p - 'a' + 10) << 4;
	    if (!*++p)	/* unexpected end of string */
		break;
	    if (*p >= '0' && *p <= '9')
		x += *p - '0';
	    else if (*p >= 'A' && *p <= 'F')
		x += *p - 'A' + 10;
	    else if (*p >= 'a' && *p <= 'f')
		x += *p - 'a' + 10;
	    hash[i] = x;
	    /* skip any space(s) between hex numbers */
	    for (p++; *p && *p == ' '; p++);
	}
	blob.cbData = i;
	blob.pbData = (unsigned char *) &hash;
	rv = CertFindCertificateInStore(cert_store, X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
		0, CERT_FIND_HASH, &blob, NULL);

    }

    return rv;
}

int ensure_library_procs_loaded()
{
    /* 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 (my_CryptAcquireCertificatePrivateKey == NULL) {
	my_CryptAcquireCertificatePrivateKey = GetProcAddress(crypt32dll,
		"my_CryptAcquireCertificatePrivateKey");
	if (my_CryptAcquireCertificatePrivateKey == NULL) {
	    CRYPTOAPIerr(CRYPTOAPI_F_GET_PROC_ADDRESS);
	    goto err;
	}
    }
    if (my_CertCreateCertificateContext == NULL) {
	my_CertCreateCertificateContext = GetProcAddress(crypt32dll,
		"CertCreateCertificateContext");
	if (my_CertCreateCertificateContext == NULL) {
	    CRYPTOAPIerr(CRYPTOAPI_F_GET_PROC_ADDRESS);
	    goto err;
	}
	return 1;

err:
	return 0;
}

/* TODO: where to call this? */
void unload_library_procs()
{
	my_CryptAcquireCertificatePrivateKey = NULL;
	my_CertCreateCertificateContext = NULL;
	if (crypt32dll != NULL) {
		FreeLibrary(crypt32dll);
		crypt32dll = NULL;
	}
}

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 = calloc(1, sizeof(*my_rsa_method));

    if (cd == NULL || my_rsa_method == NULL) {
	SSLerr(SSL_F_SSL_CTX_USE_CERTIFICATE_FILE, ERR_R_MALLOC_FAILURE);
	goto err;
    }
    /* search CURRENT_USER first, then LOCAL_MACHINE */
    cs = CertOpenStore((LPCSTR) CERT_STORE_PROV_SYSTEM, 0, 0, CERT_SYSTEM_STORE_CURRENT_USER |
		       CERT_STORE_OPEN_EXISTING_FLAG | CERT_STORE_READONLY_FLAG, L"MY");
    if (cs == NULL) {
	CRYPTOAPIerr(CRYPTOAPI_F_CERT_OPEN_SYSTEM_STORE);
	goto err;
    }
    cd->cert_context = find_certificate_in_store(cert_prop, cs);
    CertCloseStore(cs, 0);
    if (!cd->cert_context) {
	cs = CertOpenStore((LPCSTR) CERT_STORE_PROV_SYSTEM, 0, 0, CERT_SYSTEM_STORE_LOCAL_MACHINE |
			   CERT_STORE_OPEN_EXISTING_FLAG | CERT_STORE_READONLY_FLAG, L"MY");
	if (cs == NULL) {
	    CRYPTOAPIerr(CRYPTOAPI_F_CERT_OPEN_SYSTEM_STORE);
	    goto err;
	}
	cd->cert_context = find_certificate_in_store(cert_prop, cs);
	CertCloseStore(cs, 0);
	if (cd->cert_context == NULL) {
	    CRYPTOAPIerr(CRYPTOAPI_F_CERT_FIND_CERTIFICATE_IN_STORE);
	    goto err;
	}
    }

    /* cert_context->pbCertEncoded is the cert X509 DER encoded. */
    cert = d2i_X509(NULL, (const unsigned char **) &cd->cert_context->pbCertEncoded,
		    cd->cert_context->cbCertEncoded);
    if (cert == NULL) {
	SSLerr(SSL_F_SSL_CTX_USE_CERTIFICATE_FILE, ERR_R_ASN1_LIB);
	goto err;
    }

    /* set up stuff to use the private key */
    ensure_library_procs_loaded() || goto err;

    if (!my_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:
	 * "Error 1223: The operation was canceled by the user." */
	CRYPTOAPIerr(CRYPTOAPI_F_CRYPT_ACQUIRE_CERTIFICATE_PRIVATE_KEY);
	goto err;
    }
    /* 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.  */

    my_rsa_method->name = "Microsoft CryptoAPI RSA Method";
    my_rsa_method->rsa_pub_enc = rsa_pub_enc;
    my_rsa_method->rsa_pub_dec = rsa_pub_dec;
    my_rsa_method->rsa_priv_enc = rsa_priv_enc;
    my_rsa_method->rsa_priv_dec = rsa_priv_dec;
    /* my_rsa_method->init = init; */
    my_rsa_method->finish = finish;
    my_rsa_method->flags = RSA_METHOD_FLAG_NO_CHECK;
    my_rsa_method->app_data = (char *) cd;

    rsa = RSA_new();
    if (rsa == NULL) {
	SSLerr(SSL_F_SSL_CTX_USE_CERTIFICATE_FILE, ERR_R_MALLOC_FAILURE);
	goto err;
    }

    /* cert->cert_info->key->pkey 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 */
    pub_rsa = cert->cert_info->key->pkey->pkey.rsa;
    /* SSL_CTX_use_certificate() increased the reference count in 'cert', so
     * we decrease it here with X509_free(), or it will never be cleaned up. */
    X509_free(cert);
    cert = NULL;

    /* I'm not sure about what we have to fill in in the RSA, trying out stuff... */
    /* rsa->n indicates the key size */
    rsa->n = BN_dup(pub_rsa->n);
    rsa->flags |= RSA_FLAG_EXT_PKEY;
    if (!RSA_set_method(rsa, my_rsa_method))
	goto err;

    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 (cert)
	X509_free(cert);
    if (rsa)
	RSA_free(rsa);
    else {
	if (my_rsa_method)
	    free(my_rsa_method);
	if (cd) {
	    if (cd->free_crypt_prov && cd->crypt_prov)
		CryptReleaseContext(cd->crypt_prov, 0);
	    if (cd->cert_context)
		CertFreeCertificateContext(cd->cert_context);
	    free(cd);
	}
    }
    return 0;
}


int CryptoAPI_verify_certificate(X509 *x509)
{
  int ret = -1;
  int len;
  unsigned char *buf = NULL;

  PCCERT_CONTEXT pCertContext = NULL;
  PCCERT_CHAIN_CONTEXT pChainContext = NULL;
  CERT_CHAIN_PARA ChainPara;
  CERT_CHAIN_POLICY_PARA PolicyPara;
  CERT_CHAIN_POLICY_STATUS PolicyStatus;
  SSL_EXTRA_CERT_CHAIN_POLICY_PARA SSLPara;

  /* Convert from internal X509 format to DER */
  len = i2d_X509(x509, &buf);
  if (len < 0) {
	SSLerr(SSL_F_SSL_CTX_USE_CERTIFICATE_FILE, ERR_R_ASN1_LIB);
	goto err;
  }

  ensure_library_procs_loaded() || goto err;

  /* Create a certificate context based on the above certificate */
  pCertContext = my_CertCreateCertificateContext(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
  	buf, len);
  if (pCertContext == NULL) {
	CRYPTOAPIerr(CRYPTOAPI_F_CERT_CREATE_CERT_CONTEXT);
	goto err;
  }

  /* Searching and matching criteria to be used when building the chain */
  memset(&ChainPara, 0, sizeof(ChainPara));
  ChainPara.cbSize = sizeof(ChainPara);

  /* Get the certificate chain of our certificate */
  if (!CertGetCertificateChain(NULL, pCertContext, NULL, NULL, &ChainPara,
	CERT_CHAIN_REVOCATION_CHECK_CHAIN,
	NULL, &pChainContext)) {
	CRYPTOAPIerr(CRYPTOAPI_F_CERT_GET_CERT_CHAIN);
	goto err;
  }

  memset(&SSLPara, 0, sizeof(SSLPara));
  SSLPara.cbStruct = sizeof(SSLPara);
  // TODO: Specify client or server using OpenVPN configuration
  SSLPara.dwAuthType = ((((((AUTHTYPE_CLIENT | AUTHTYPE_SERVER))))));
  SSLPara.fdwChecks = SECURITY_FLAG_IGNORE_CERT_CN_INVALID;
  SSLPara.pwszServerName = NULL;
  memset(&PolicyPara, 0, sizeof(PolicyPara));
  PolicyPara.cbSize = sizeof(PolicyPara);
  PolicyPara.pvExtraPolicyPara = &SSLPara;

  memset(&PolicyStatus, 0, sizeof(PolicyStatus));
  PolicyStatus.cbSize = sizeof(PolicyStatus);

  if (!CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_SSL, pChainContext,
					&PolicyPara, &PolicyStatus)) {
	CRYPTOAPIerr(CRYPTOAPI_F_CERT_VERIFY_CERT_CHAIN_POLICY);
	goto err;
  }

  /* return 1 when the certificate is trusted, 0 when it's not */
  ret = !PolicyStatus.dwError;

  err:
    if (buf)
	OPENSSL_free(buf);
    if (pChainContext)
	CertFreeCertificateChain(pChainContext);
    if (pCertContext)
	CertFreeCertificateContext(pCertContext);

  return ret;
}

#else
static void dummy (void) {}
#endif				/* WIN32 */

Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re:[PATCH v3] Use CryptoAPI CA store)

[Dave]

...
> Alon,
> 	I've started working on (1) and (2).  Attached is the 
> updated cryptoapi.c.  Would you prefer a patch when changes 
> involve a single file?  Can you tell me what you think (is 
...
> 	As for (3), where is the SSL role defined?  Or, 
> alternatively, what is the procedure when cryptoapi isn't 
> used to verify the certificate is in the correct role?
...
> Jason
...

Oh, Happy Day!  Now I don't have to do it.  I've wanted this for some time,
and forgot that someone had already started working on it a year ago.

The ssl server role is controlled by the options --tls-server and
--tls-client (these are also set internally by the various 'helper' macros
like --client and --server).  So, if you have the 'options' object, then the
member

    options->tls_server

will indicate if you are in server mode.  It is only used in an interesting
way in ssl.c, as far as I can see.  There is also a --tls-client, but I
don't see it being used in an interesting way anyway.  (Interesting ==
something other than setting the value or printing output).


As far as dynamically loading the crypto api dll; the comments state this is
a hack to work around MingW's incompleteness?  Implying that -- barring
incompleteness -- the dll would have otherwise been loaded implicitly?  If
so then I would suggest loading it on application startup and unloading on
shutdown.  This is analogous to things like 'WSAStartup' and 'OleInit'.  On
the other hand, the 'load' function looks idempotent, so it can be called
safely as many times as you like.  Just make sure to call it before you need
it.  Also, it is not strictly necessary to call 'unload' unless you are
really wanting to truly unload for some technical reason, or just like to be
tidy like that.  The OS will happily 'unload' it upon process shutdown.

I can't comment on 'code cleanup' because that is subjective.

-Dave

Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)

[Alon Bar-Lev]

Hi!

I prefer to receive patches...
Anyway, this is not exactly what I meant.
Please review latest head.
I did not test this, but it should be correct now as far as the
changes are concerned.
It may not work as the validation process was never tested.

Alon.

On 9/27/08, Jason R. Coombs <jaraco@...1130...> wrote:
> Alon,
>         I've started working on (1) and (2).  Attached is the updated cryptoapi.c.  Would you prefer a patch when changes involve a single file?  Can you tell me what you think (is this moving in the right direction)?  Initially, I've only moved the LoadLibrary code into its own function, but it's still called from the same place.  Should I go further and move this initialization code somewhere else?  If so, can you suggest where I should look to hook in the initialization?
>
>         As for (2), I've created a function that unloads the library and clears out the variables... but it's not called from anywhere.  I guess if I know where the initialization will go, then I can find a good place from which to call the cleanup code.
>
>         As for (3), where is the SSL role defined?  Or, alternatively, what is the procedure when cryptoapi isn't used to verify the certificate is in the correct role?
>
>         I hope to contribute further.  This is my first time looking at the openvpn source, so please bear with me (or let me know it's not worth your time).
>
>  Regards,
>
> Jason
>
>
>  -----Original Message-----
>  From: Alon Bar-Lev [mailto:alon.barlev@...277...]
>  Sent: Thursday, 25 September, 2008 01:10
>  To: Jason R. Coombs
>  Cc: openvpn-devel@lists.sourceforge.net
>  Subject: Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)
>
>  Hello,
>
>  I cleaned it up a little but still things should be done:
>
>  1. Add initialize code and load all entry points for this module at
>  one place, single LoadLibrary etc...
>  2. Add cleanup code to unload all static module resources.
>  3. Check SSL role by OpenSSL configuration (client or server), see
>  TODO signature.
>  4. Cleanup warnings.
>
>  Available at [1], I did not check it as I don't have active Windows
>  configuration now.
>  Can you please complete it?
>
>  Alon.
>
>  [1] http://svn.openvpn.net/projects/openvpn/contrib/alon/BETA21-ms-chk-2/openvpn
>
>  On 9/24/08, Jason R. Coombs <jaraco@...1130...> wrote:
>  > I'd really like to see this feature in a future build.
>  >
>  >  Is there anything more that needs to be done to integrate this into 2.1?
>  >  I can help with code cleanup/refactoring.  I don't have a development
>  >  environment set up, though, so I'd be working blind.
>  >
>  >  Cheers,
>  >  Jason
>  >
>  >  On 9/23/07, Faidon Liambotis Wrote:
>  >  Alon Bar-Lev wrote:
>  >  > On 9/22/07, Faidon Liambotis <paravoid@...1131...> wrote:
>  >  >> Alon Bar-Lev wrote:
>  >  >>> So you need to use CertVerifyCertificateChainPolicy() with
>  >  CERT_CHAIN_POLICY_SSL
>  >  >> I'm no Microsoft developer (adn I don't want to be to be honest) but
>  >  if
>  >  >> I understand it right, it's better to call CertGetCertificateChain()
>  >  as
>  >  >> I am doing.
>  >  >
>  >  > You need to use both, one for create the chain and the other to verify
>  >  > that it meets with system CTL for SSL.
>  >  Seems that you are right. Below you will find -v4 of the patch that does
>  >  that.
>  >
>  >  Also, my previous version didn't actually check for revocations,
>  >  contrary to what I documented.
>  >  I added CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT to the dwFlags of
>  >  CertGetCertificateChain.
>  >
>  >  Let me know what you think.
>  >
>  >  Thanks,
>  >  Faidon
>  >
>  >
>  >  -------------------------------------------------------------------------
>  >  This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
>  >  Build the coolest Linux based applications with Moblin SDK & win great prizes
>  >  Grand prize is a trip for two to an Open Source event anywhere in the world
>  >  http://moblin-contest.org/redirect.php?banner_id=100&url=/
>  >  _______________________________________________
>  >  Openvpn-devel mailing list
>  >  Openvpn-devel@lists.sourceforge.net
>  >  https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>  >
>
>

Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)

[Alon Bar-Lev]

Any news?

On 9/27/08, Alon Bar-Lev <alon.barlev@...277...> wrote:
> Hi!
>
>  I prefer to receive patches...
>  Anyway, this is not exactly what I meant.
>  Please review latest head.
>  I did not test this, but it should be correct now as far as the
>  changes are concerned.
>  It may not work as the validation process was never tested.
>
>
>  Alon.
>
>
>  On 9/27/08, Jason R. Coombs <jaraco@...1130...> wrote:
>  > Alon,
>  >         I've started working on (1) and (2).  Attached is the updated cryptoapi.c.  Would you prefer a patch when changes involve a single file?  Can you tell me what you think (is this moving in the right direction)?  Initially, I've only moved the LoadLibrary code into its own function, but it's still called from the same place.  Should I go further and move this initialization code somewhere else?  If so, can you suggest where I should look to hook in the initialization?
>  >
>  >         As for (2), I've created a function that unloads the library and clears out the variables... but it's not called from anywhere.  I guess if I know where the initialization will go, then I can find a good place from which to call the cleanup code.
>  >
>  >         As for (3), where is the SSL role defined?  Or, alternatively, what is the procedure when cryptoapi isn't used to verify the certificate is in the correct role?
>  >
>  >         I hope to contribute further.  This is my first time looking at the openvpn source, so please bear with me (or let me know it's not worth your time).
>  >
>  >  Regards,
>  >
>  > Jason
>  >
>  >
>  >  -----Original Message-----
>  >  From: Alon Bar-Lev [mailto:alon.barlev@...277...]
>  >  Sent: Thursday, 25 September, 2008 01:10
>  >  To: Jason R. Coombs
>  >  Cc: openvpn-devel@lists.sourceforge.net
>  >  Subject: Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)
>  >
>  >  Hello,
>  >
>  >  I cleaned it up a little but still things should be done:
>  >
>  >  1. Add initialize code and load all entry points for this module at
>  >  one place, single LoadLibrary etc...
>  >  2. Add cleanup code to unload all static module resources.
>  >  3. Check SSL role by OpenSSL configuration (client or server), see
>  >  TODO signature.
>  >  4. Cleanup warnings.
>  >
>  >  Available at [1], I did not check it as I don't have active Windows
>  >  configuration now.
>  >  Can you please complete it?
>  >
>  >  Alon.
>  >
>  >  [1] http://svn.openvpn.net/projects/openvpn/contrib/alon/BETA21-ms-chk-2/openvpn
>  >
>  >  On 9/24/08, Jason R. Coombs <jaraco@...1130...> wrote:
>  >  > I'd really like to see this feature in a future build.
>  >  >
>  >  >  Is there anything more that needs to be done to integrate this into 2.1?
>  >  >  I can help with code cleanup/refactoring.  I don't have a development
>  >  >  environment set up, though, so I'd be working blind.
>  >  >
>  >  >  Cheers,
>  >  >  Jason
>  >  >
>  >  >  On 9/23/07, Faidon Liambotis Wrote:
>  >  >  Alon Bar-Lev wrote:
>  >  >  > On 9/22/07, Faidon Liambotis <paravoid@...1131...> wrote:
>  >  >  >> Alon Bar-Lev wrote:
>  >  >  >>> So you need to use CertVerifyCertificateChainPolicy() with
>  >  >  CERT_CHAIN_POLICY_SSL
>  >  >  >> I'm no Microsoft developer (adn I don't want to be to be honest) but
>  >  >  if
>  >  >  >> I understand it right, it's better to call CertGetCertificateChain()
>  >  >  as
>  >  >  >> I am doing.
>  >  >  >
>  >  >  > You need to use both, one for create the chain and the other to verify
>  >  >  > that it meets with system CTL for SSL.
>  >  >  Seems that you are right. Below you will find -v4 of the patch that does
>  >  >  that.
>  >  >
>  >  >  Also, my previous version didn't actually check for revocations,
>  >  >  contrary to what I documented.
>  >  >  I added CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT to the dwFlags of
>  >  >  CertGetCertificateChain.
>  >  >
>  >  >  Let me know what you think.
>  >  >
>  >  >  Thanks,
>  >  >  Faidon
>  >  >
>  >  >
>  >  >  -------------------------------------------------------------------------
>  >  >  This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
>  >  >  Build the coolest Linux based applications with Moblin SDK & win great prizes
>  >  >  Grand prize is a trip for two to an Open Source event anywhere in the world
>  >  >  http://moblin-contest.org/redirect.php?banner_id=100&url=/
>  >  >  _______________________________________________
>  >  >  Openvpn-devel mailing list
>  >  >  Openvpn-devel@lists.sourceforge.net
>  >  >  https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>  >  >
>  >
>  >
>

Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)

[Faidon Liambotis]

Hi,

Alon Bar-Lev wrote:
> On 9/27/08, Alon Bar-Lev <alon.barlev@...277...> wrote:
>>  I prefer to receive patches...
>>  Anyway, this is not exactly what I meant.
>>  Please review latest head.
>>  I did not test this, but it should be correct now as far as the
>>  changes are concerned.
>>  It may not work as the validation process was never tested.
>
> Any news?
Thanks for reviving this. I built it and tried it and seems to work.
I didn't test with revoked or expired certificates, however.

As for warnings there's just a trivial one:
	cryptoapi.c:429: warning: passing arg 2 of `d2i_X509' from 	
			 incompatible pointer type

Regards,
Faidon

Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)

[Alon Bar-Lev]

On 10/7/08, Faidon Liambotis <paravoid@...334...> wrote:
>  Thanks for reviving this. I built it and tried it and seems to work.
>  I didn't test with revoked or expired certificates, however.
>
>  As for warnings there's just a trivial one:
>         cryptoapi.c:429: warning: passing arg 2 of `d2i_X509' from
>                          incompatible pointer type
>

Thank you!
Can anyone else check it out with revocation/expiration?

Alon.

Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)

[Jason R. Coombs]

[-- Attachment #1: Type: text/plain, Size: 1182 bytes --]

Faidon,

If you send me a binary build for Windows 32-bit, I'll test it against expired 
and revoked certs.  I presume I don't need a server configured for this test; 
it should fail client side before attempting to connect?

Jason

-----Original Message-----
From: Faidon Liambotis [mailto:paravoid@...334...]
Sent: Tuesday, 07 October, 2008 15:53
To: Alon Bar-Lev
Cc: Jason R. Coombs; openvpn-devel@lists.sourceforge.net
Subject: Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH 
v3] Use CryptoAPI CA store)

Hi,

Alon Bar-Lev wrote:
> On 9/27/08, Alon Bar-Lev <alon.barlev@...277...> wrote:
>>  I prefer to receive patches...
>>  Anyway, this is not exactly what I meant.
>>  Please review latest head.
>>  I did not test this, but it should be correct now as far as the
>>  changes are concerned.
>>  It may not work as the validation process was never tested.
>
> Any news?
Thanks for reviving this. I built it and tried it and seems to work.
I didn't test with revoked or expired certificates, however.

As for warnings there's just a trivial one:
	cryptoapi.c:429: warning: passing arg 2 of `d2i_X509' from
			 incompatible pointer type

Regards,
Faidon

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 7001 bytes --]

Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)

[Alon Bar-Lev]

Binaries are at [1].

It is not enough to test it on client, we need to verify that the
validation works correctly on both ends, as capi has different policy
for servers and clients.

Alon.

[1] http://alon.barlev.googlepages.com/openvpn-mscapi-test-1.tar.bz2

On 10/7/08, Jason R. Coombs <jaraco@...1130...> wrote:
> Faidon,
>
>  If you send me a binary build for Windows 32-bit, I'll test it against expired
>  and revoked certs.  I presume I don't need a server configured for this test;
>  it should fail client side before attempting to connect?
>
>
>  Jason
>
>
>  -----Original Message-----
>  From: Faidon Liambotis [mailto:paravoid@...334...]
>  Sent: Tuesday, 07 October, 2008 15:53
>  To: Alon Bar-Lev
>  Cc: Jason R. Coombs; openvpn-devel@lists.sourceforge.net
>  Subject: Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH
>  v3] Use CryptoAPI CA store)
>
>
> Hi,
>
>  Alon Bar-Lev wrote:
>  > On 9/27/08, Alon Bar-Lev <alon.barlev@...277...> wrote:
>  >>  I prefer to receive patches...
>  >>  Anyway, this is not exactly what I meant.
>  >>  Please review latest head.
>  >>  I did not test this, but it should be correct now as far as the
>  >>  changes are concerned.
>  >>  It may not work as the validation process was never tested.
>  >
>  > Any news?
>  Thanks for reviving this. I built it and tried it and seems to work.
>  I didn't test with revoked or expired certificates, however.
>
>  As for warnings there's just a trivial one:
>         cryptoapi.c:429: warning: passing arg 2 of `d2i_X509' from
>                          incompatible pointer type
>
>  Regards,
>  Faidon
>
> -------------------------------------------------------------------------
>  This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
>  Build the coolest Linux based applications with Moblin SDK & win great prizes
>  Grand prize is a trip for two to an Open Source event anywhere in the world
>  http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
>  Openvpn-devel mailing list
>  Openvpn-devel@lists.sourceforge.net
>  https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
>
>

Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)

[Jason R. Coombs]

[-- Attachment #1: Type: text/plain, Size: 3372 bytes --]

Alon,
	I've tested the client functionality and the basic functionality works great 
(not testing against expired or revoked certificates).

	I then created a test for expired certs (incorrectly) by specifying an 
expired _client_ certificate.  Curiously, OpenVPN did not complain about the 
expired client certificate, but rather proceeded to attempt a connection with 
it (which subsequently failed to establish TLS I suspect because the server 
didn't have the corresponding public cert).  So I think I may have discovered 
a limitation of the pre-existing cryptoapicert function.

	So to recap:

	Cryptoapicert client mode: fails to verify expired cert.
	Cryptoapica client mode: works!
	Cryptoapica client mode expired/revoked cert: untested
	Cryptoapica server mode: untested

	I'm in a conference this week, but will continue to test as time permits.

Jason

-----Original Message-----
From: Alon Bar-Lev [mailto:alon.barlev@...277...]
Sent: Tuesday, 07 October, 2008 16:56
To: Jason R. Coombs
Cc: Faidon Liambotis; openvpn-devel@lists.sourceforge.net
Subject: Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH 
v3] Use CryptoAPI CA store)

Binaries are at [1].

It is not enough to test it on client, we need to verify that the
validation works correctly on both ends, as capi has different policy
for servers and clients.

Alon.

[1] http://alon.barlev.googlepages.com/openvpn-mscapi-test-1.tar.bz2

On 10/7/08, Jason R. Coombs <jaraco@...1130...> wrote:
> Faidon,
>
>  If you send me a binary build for Windows 32-bit, I'll test it against 
> expired
>  and revoked certs.  I presume I don't need a server configured for this 
> test;
>  it should fail client side before attempting to connect?
>
>
>  Jason
>
>
>  -----Original Message-----
>  From: Faidon Liambotis [mailto:paravoid@...334...]
>  Sent: Tuesday, 07 October, 2008 15:53
>  To: Alon Bar-Lev
>  Cc: Jason R. Coombs; openvpn-devel@lists.sourceforge.net
>  Subject: Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: 
> [PATCH
>  v3] Use CryptoAPI CA store)
>
>
> Hi,
>
>  Alon Bar-Lev wrote:
>  > On 9/27/08, Alon Bar-Lev <alon.barlev@...277...> wrote:
>  >>  I prefer to receive patches...
>  >>  Anyway, this is not exactly what I meant.
>  >>  Please review latest head.
>  >>  I did not test this, but it should be correct now as far as the
>  >>  changes are concerned.
>  >>  It may not work as the validation process was never tested.
>  >
>  > Any news?
>  Thanks for reviving this. I built it and tried it and seems to work.
>  I didn't test with revoked or expired certificates, however.
>
>  As for warnings there's just a trivial one:
>         cryptoapi.c:429: warning: passing arg 2 of `d2i_X509' from
>                          incompatible pointer type
>
>  Regards,
>  Faidon
>
> -------------------------------------------------------------------------
>  This SF.Net email is sponsored by the Moblin Your Move Developer's 
> challenge
>  Build the coolest Linux based applications with Moblin SDK & win great 
> prizes
>  Grand prize is a trip for two to an Open Source event anywhere in the world
>  http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
>  Openvpn-devel mailing list
>  Openvpn-devel@lists.sourceforge.net
>  https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
>
>

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 7001 bytes --]

Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)

[Alon Bar-Lev]

Please note that the other side should detect the expiration.
Hence, if you revoke the client certificate the server should report
this and vice versa.
Please also make sure that both sides using the same functionality.

On 10/9/08, Jason R. Coombs <jaraco@...1130...> wrote:
> Alon,
>         I've tested the client functionality and the basic functionality works great
>  (not testing against expired or revoked certificates).
>
>         I then created a test for expired certs (incorrectly) by specifying an
>  expired _client_ certificate.  Curiously, OpenVPN did not complain about the
>  expired client certificate, but rather proceeded to attempt a connection with
>  it (which subsequently failed to establish TLS I suspect because the server
>  didn't have the corresponding public cert).  So I think I may have discovered
>  a limitation of the pre-existing cryptoapicert function.
>
>         So to recap:
>
>         Cryptoapicert client mode: fails to verify expired cert.
>         Cryptoapica client mode: works!
>         Cryptoapica client mode expired/revoked cert: untested
>         Cryptoapica server mode: untested
>
>         I'm in a conference this week, but will continue to test as time permits.
>
>
>  Jason
>
>  -----Original Message-----
>  From: Alon Bar-Lev [mailto:alon.barlev@...277...]
>
> Sent: Tuesday, 07 October, 2008 16:56
>  To: Jason R. Coombs
>
> Cc: Faidon Liambotis; openvpn-devel@lists.sourceforge.net
>  Subject: Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH
>  v3] Use CryptoAPI CA store)
>
>  Binaries are at [1].
>
>  It is not enough to test it on client, we need to verify that the
>  validation works correctly on both ends, as capi has different policy
>  for servers and clients.
>
>  Alon.
>
>  [1] http://alon.barlev.googlepages.com/openvpn-mscapi-test-1.tar.bz2
>
>  On 10/7/08, Jason R. Coombs <jaraco@...1130...> wrote:
>  > Faidon,
>  >
>  >  If you send me a binary build for Windows 32-bit, I'll test it against
>  > expired
>  >  and revoked certs.  I presume I don't need a server configured for this
>  > test;
>  >  it should fail client side before attempting to connect?
>  >
>  >
>  >  Jason
>  >
>  >
>  >  -----Original Message-----
>  >  From: Faidon Liambotis [mailto:paravoid@...334...]
>  >  Sent: Tuesday, 07 October, 2008 15:53
>  >  To: Alon Bar-Lev
>  >  Cc: Jason R. Coombs; openvpn-devel@lists.sourceforge.net
>  >  Subject: Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re:
>  > [PATCH
>  >  v3] Use CryptoAPI CA store)
>  >
>  >
>  > Hi,
>  >
>  >  Alon Bar-Lev wrote:
>  >  > On 9/27/08, Alon Bar-Lev <alon.barlev@...277...> wrote:
>  >  >>  I prefer to receive patches...
>  >  >>  Anyway, this is not exactly what I meant.
>  >  >>  Please review latest head.
>  >  >>  I did not test this, but it should be correct now as far as the
>  >  >>  changes are concerned.
>  >  >>  It may not work as the validation process was never tested.
>  >  >
>  >  > Any news?
>  >  Thanks for reviving this. I built it and tried it and seems to work.
>  >  I didn't test with revoked or expired certificates, however.
>  >
>  >  As for warnings there's just a trivial one:
>  >         cryptoapi.c:429: warning: passing arg 2 of `d2i_X509' from
>  >                          incompatible pointer type
>  >
>  >  Regards,
>  >  Faidon
>  >
>  > -------------------------------------------------------------------------
>  >  This SF.Net email is sponsored by the Moblin Your Move Developer's
>  > challenge
>  >  Build the coolest Linux based applications with Moblin SDK & win great
>  > prizes
>  >  Grand prize is a trip for two to an Open Source event anywhere in the world
>  >  http://moblin-contest.org/redirect.php?banner_id=100&url=/
>  > _______________________________________________
>  >  Openvpn-devel mailing list
>  >  Openvpn-devel@lists.sourceforge.net
>  >  https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>  >
>  >
>  >
>
>

Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH v3] Use CryptoAPI CA store)

[Alon Bar-Lev]

New binaries at [1], it should also print the validation status.

[1] http://alon.barlev.googlepages.com/openvpn-mscapi-test-2.tar.bz2

On 10/9/08, Alon Bar-Lev <alon.barlev@...277...> wrote:
> Please note that the other side should detect the expiration.
>  Hence, if you revoke the client certificate the server should report
>  this and vice versa.
>  Please also make sure that both sides using the same functionality.
>
>
>  On 10/9/08, Jason R. Coombs <jaraco@...1130...> wrote:
>  > Alon,
>  >         I've tested the client functionality and the basic functionality works great
>  >  (not testing against expired or revoked certificates).
>  >
>  >         I then created a test for expired certs (incorrectly) by specifying an
>  >  expired _client_ certificate.  Curiously, OpenVPN did not complain about the
>  >  expired client certificate, but rather proceeded to attempt a connection with
>  >  it (which subsequently failed to establish TLS I suspect because the server
>  >  didn't have the corresponding public cert).  So I think I may have discovered
>  >  a limitation of the pre-existing cryptoapicert function.
>  >
>  >         So to recap:
>  >
>  >         Cryptoapicert client mode: fails to verify expired cert.
>  >         Cryptoapica client mode: works!
>  >         Cryptoapica client mode expired/revoked cert: untested
>  >         Cryptoapica server mode: untested
>  >
>  >         I'm in a conference this week, but will continue to test as time permits.
>  >
>  >
>  >  Jason
>  >
>  >  -----Original Message-----
>  >  From: Alon Bar-Lev [mailto:alon.barlev@...277...]
>  >
>  > Sent: Tuesday, 07 October, 2008 16:56
>  >  To: Jason R. Coombs
>  >
>  > Cc: Faidon Liambotis; openvpn-devel@lists.sourceforge.net
>  >  Subject: Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re: [PATCH
>  >  v3] Use CryptoAPI CA store)
>  >
>  >  Binaries are at [1].
>  >
>  >  It is not enough to test it on client, we need to verify that the
>  >  validation works correctly on both ends, as capi has different policy
>  >  for servers and clients.
>  >
>  >  Alon.
>  >
>  >  [1] http://alon.barlev.googlepages.com/openvpn-mscapi-test-1.tar.bz2
>  >
>  >  On 10/7/08, Jason R. Coombs <jaraco@...1130...> wrote:
>  >  > Faidon,
>  >  >
>  >  >  If you send me a binary build for Windows 32-bit, I'll test it against
>  >  > expired
>  >  >  and revoked certs.  I presume I don't need a server configured for this
>  >  > test;
>  >  >  it should fail client side before attempting to connect?
>  >  >
>  >  >
>  >  >  Jason
>  >  >
>  >  >
>  >  >  -----Original Message-----
>  >  >  From: Faidon Liambotis [mailto:paravoid@...334...]
>  >  >  Sent: Tuesday, 07 October, 2008 15:53
>  >  >  To: Alon Bar-Lev
>  >  >  Cc: Jason R. Coombs; openvpn-devel@lists.sourceforge.net
>  >  >  Subject: Re: [Openvpn-devel] [PATCH v4] Use CryptoAPI CA store (was Re:
>  >  > [PATCH
>  >  >  v3] Use CryptoAPI CA store)
>  >  >
>  >  >
>  >  > Hi,
>  >  >
>  >  >  Alon Bar-Lev wrote:
>  >  >  > On 9/27/08, Alon Bar-Lev <alon.barlev@...277...> wrote:
>  >  >  >>  I prefer to receive patches...
>  >  >  >>  Anyway, this is not exactly what I meant.
>  >  >  >>  Please review latest head.
>  >  >  >>  I did not test this, but it should be correct now as far as the
>  >  >  >>  changes are concerned.
>  >  >  >>  It may not work as the validation process was never tested.
>  >  >  >
>  >  >  > Any news?
>  >  >  Thanks for reviving this. I built it and tried it and seems to work.
>  >  >  I didn't test with revoked or expired certificates, however.
>  >  >
>  >  >  As for warnings there's just a trivial one:
>  >  >         cryptoapi.c:429: warning: passing arg 2 of `d2i_X509' from
>  >  >                          incompatible pointer type
>  >  >
>  >  >  Regards,
>  >  >  Faidon
>  >  >
>  >  > -------------------------------------------------------------------------
>  >  >  This SF.Net email is sponsored by the Moblin Your Move Developer's
>  >  > challenge
>  >  >  Build the coolest Linux based applications with Moblin SDK & win great
>  >  > prizes
>  >  >  Grand prize is a trip for two to an Open Source event anywhere in the world
>  >  >  http://moblin-contest.org/redirect.php?banner_id=100&url=/
>  >  > _______________________________________________
>  >  >  Openvpn-devel mailing list
>  >  >  Openvpn-devel@lists.sourceforge.net
>  >  >  https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>  >  >
>  >  >
>  >  >
>  >
>  >
>