Re: [Openvpn-devel] [PATCH v4 1/2] Skip expired certificates in Windows certificate store

2020-02-13 Thread Lev Stipakov
Built and tested with MSVC on Windows 10 - code skips expired certificate
and picks valid one.

Tested that code doesn't explode if --cryptoapicert has unsupported value.

Acked-by: Lev Stipakov 
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v4 1/2] Skip expired certificates in Windows certificate store

2020-02-12 Thread selva . nair
From: Selva Nair 

Have the cryptoapicert option find the first matching certificate
in store that is valid at the present time. Currently the first
found item, even if expired, is returned.

This makes it possible to update certifiates in store without having
to delete old ones. As a side effect, if only expired certificates are
found, the connection fails.

Also remove some unnecessary casts.

Tested on Windows 10.
Trac #966

v4: Handle the case when an unknown certificate specification is passed
to find_certificate_in_store().

Note: Warnings printed from find_certificate_in_store() could show up
multiple times as its called for each certificate store. This could
be improved in a future patch.

Signed-off-by: Selva Nair 
---
 src/openvpn/cryptoapi.c | 46 ++
 1 file changed, 34 insertions(+), 12 deletions(-)

diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c
index 2f2eee7..b9f1328 100644
--- a/src/openvpn/cryptoapi.c
+++ b/src/openvpn/cryptoapi.c
@@ -739,27 +739,30 @@ find_certificate_in_store(const char *cert_prop, 
HCERTSTORE cert_store)
  * SUBJ:
  * THUMB:, e.g.
  * THUMB:f6 49 24 41 01 b4 fb 44 0c ce f4 36 ae d0 c4 c9 df 7a b6 28
+ * The first matching certificate that has not expired is returned.
  */
 const CERT_CONTEXT *rv = NULL;
+DWORD find_type;
+const void *find_param;
+unsigned char hash[255];
+CRYPT_HASH_BLOB blob = {.cbData = 0, .pbData = hash};
 
 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);
-
+find_param = cert_prop + 5;
+find_type = CERT_FIND_SUBJECT_STR_A;
 }
 else if (!strncmp(cert_prop, "THUMB:", 6))
 {
-unsigned char hash[255];
-char *p;
+const char *p;
 int i, x = 0;
-CRYPT_HASH_BLOB blob;
+find_type = CERT_FIND_HASH;
+find_param = 
 
 /* skip the tag */
 cert_prop += 6;
-for (p = (char *) cert_prop, i = 0; *p && i < sizeof(hash); i++)
+for (p = cert_prop, i = 0; *p && i < sizeof(hash); i++)
 {
 if (*p >= '0' && *p <= '9')
 {
@@ -775,7 +778,8 @@ find_certificate_in_store(const char *cert_prop, HCERTSTORE 
cert_store)
 }
 if (!*++p)  /* unexpected end of string */
 {
-break;
+msg(M_WARN, "WARNING: cryptoapicert: error parsing 
.", cert_prop);
+return NULL;
 }
 if (*p >= '0' && *p <= '9')
 {
@@ -796,10 +800,28 @@ find_certificate_in_store(const char *cert_prop, 
HCERTSTORE cert_store)
 }
 }
 blob.cbData = i;
-blob.pbData = (unsigned char *) 
-rv = CertFindCertificateInStore(cert_store, X509_ASN_ENCODING | 
PKCS_7_ASN_ENCODING,
-0, CERT_FIND_HASH, , NULL);
+}
+else {
+msg(M_WARN, "WARNING: cryptoapicert: unsupported certificate 
specification <%s>", cert_prop);
+return NULL;
+}
 
+while(true)
+{
+int validity = 1;
+/* this frees previous rv, if not NULL */
+rv = CertFindCertificateInStore(cert_store, X509_ASN_ENCODING | 
PKCS_7_ASN_ENCODING,
+0, find_type, find_param, rv);
+if (rv)
+{
+validity = CertVerifyTimeValidity(NULL, rv->pCertInfo);
+}
+if (!rv || validity == 0)
+{
+break;
+}
+msg(M_WARN, "WARNING: cryptoapicert: ignoring certificate in store 
%s.",
+validity < 0 ? "not yet valid" : "that has expired");
 }
 
 return rv;
-- 
2.1.4



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel