From patchwork Wed Feb 12 15:06:06 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,v4,1/2] Skip expired certificates in Windows certificate store X-Patchwork-Submitter: Selva Nair X-Patchwork-Id: 990 Message-Id: <1581519967-16950-1-git-send-email-selva.nair@gmail.com> To: openvpn-devel@lists.sourceforge.net Date: Wed, 12 Feb 2020 10:06:06 -0500 From: selva.nair@gmail.com List-Id: 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 Acked-by: Lev Stipakov --- 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 = &blob; /* 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 *) &hash; - rv = CertFindCertificateInStore(cert_store, X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, - 0, CERT_FIND_HASH, &blob, 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; From patchwork Wed Feb 12 15:06:07 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,v4,2/2] Allow unicode search string in --cryptoapicert option X-Patchwork-Submitter: Selva Nair X-Patchwork-Id: 991 Message-Id: <1581519967-16950-2-git-send-email-selva.nair@gmail.com> To: openvpn-devel@lists.sourceforge.net Date: Wed, 12 Feb 2020 10:06:07 -0500 From: selva.nair@gmail.com List-Id: From: Selva Nair Currently when the certificate is specified as "SUBJ:foo", the string foo is assumed to be ascii. Change that and interpret it as utf-8, convert to a wide string, and flag it as unicode in CertFindCertifcateInStore(). Signed-off-by: Selva Nair Acked-by: Lev Stipakov --- v4: matched to v4 of 1/2 src/openvpn/cryptoapi.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c index b9f1328..1bf74fc 100644 --- a/src/openvpn/cryptoapi.c +++ b/src/openvpn/cryptoapi.c @@ -51,6 +51,7 @@ #include "buffer.h" #include "openssl_compat.h" +#include "win32.h" /* 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. @@ -746,12 +747,13 @@ find_certificate_in_store(const char *cert_prop, HCERTSTORE cert_store) const void *find_param; unsigned char hash[255]; CRYPT_HASH_BLOB blob = {.cbData = 0, .pbData = hash}; + struct gc_arena gc = gc_new(); if (!strncmp(cert_prop, "SUBJ:", 5)) { /* skip the tag */ - find_param = cert_prop + 5; - find_type = CERT_FIND_SUBJECT_STR_A; + find_param = wide_string(cert_prop + 5, &gc); + find_type = CERT_FIND_SUBJECT_STR_W; } else if (!strncmp(cert_prop, "THUMB:", 6)) { @@ -779,7 +781,7 @@ find_certificate_in_store(const char *cert_prop, HCERTSTORE cert_store) if (!*++p) /* unexpected end of string */ { msg(M_WARN, "WARNING: cryptoapicert: error parsing .", cert_prop); - return NULL; + goto out; } if (*p >= '0' && *p <= '9') { @@ -803,7 +805,7 @@ find_certificate_in_store(const char *cert_prop, HCERTSTORE cert_store) } else { msg(M_WARN, "WARNING: cryptoapicert: unsupported certificate specification <%s>", cert_prop); - return NULL; + goto out; } while(true) @@ -824,6 +826,8 @@ find_certificate_in_store(const char *cert_prop, HCERTSTORE cert_store) validity < 0 ? "not yet valid" : "that has expired"); } +out: + gc_free(&gc); return rv; }