changeset 222:53ea9b975d1c

Cleanup windowsstore.c The cause for the test failure was that ADD_ALWAYS did not add a duplicate in the Root store but did add a duplicate in the Memory Store. We now check if the certificate is already in the store before actually installing it.
author Andre Heinecke <aheinecke@intevation.de>
date Thu, 27 Mar 2014 14:16:22 +0000
parents 431b058e903d
children be22e9deca88
files cinst/windowsstore.c ui/tests/windowsstoretest.cpp ui/tests/windowsstoretest.h
diffstat 3 files changed, 85 insertions(+), 78 deletions(-) [+]
line wrap: on
line diff
--- a/cinst/windowsstore.c	Thu Mar 27 10:23:49 2014 +0000
+++ b/cinst/windowsstore.c	Thu Mar 27 14:16:22 2014 +0000
@@ -7,7 +7,8 @@
 #include "listutil.h"
 #include "strhelp.h"
 
-static LPWSTR getLastErrorMsg()
+static LPWSTR
+getLastErrorMsg()
 {
   LPWSTR bufPtr = NULL;
   DWORD err = GetLastError();
@@ -32,6 +33,43 @@
   return bufPtr;
 }
 
+static PCCERT_CONTEXT
+b64_to_cert_context(char *b64_data, size_t b64_size)
+{
+  size_t buf_size = 0;
+  char *buf = NULL;
+  PCCERT_CONTEXT pCert = NULL;
+  int ret = -1;
+
+  ret = str_base64_decode (&buf, &buf_size, b64_data, b64_size);
+
+  if (ret != 0)
+    {
+      printf ("decoding certificate failed\n");
+      return NULL;
+    }
+
+  pCert = CertCreateContext (CERT_STORE_CERTIFICATE_CONTEXT,
+                             X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
+                             (const PBYTE) buf,
+                             (DWORD) buf_size,
+                             0,
+                             NULL);
+  free (buf); /* Windows has a copy */
+
+  if (pCert == NULL)
+    {
+      LPWSTR error = getLastErrorMsg();
+      if (error)
+        {
+          printf ("Failed to create cert context: %S \n", error);
+          LocalFree (error);
+        }
+      return NULL;
+    }
+  return pCert;
+}
+
 void
 do_remove(HCERTSTORE hStore, char **to_remove)
 {
@@ -45,37 +83,17 @@
 
   for (i=0; to_remove[i]; i++)
     {
-      char *asn1_data = NULL;
-      size_t asn1_size = 0;
-      int ret = -1;
-      DWORD j;
       PCCERT_CONTEXT pc_to_remove = NULL;
 
-      ret = str_base64_decode (&asn1_data, &asn1_size, to_remove[i],
-                               strnlen(to_remove[i], MAX_LINE_LENGTH));
-      /* Decoding / parsing errors in here should not happen at all.
-         The only errors which are not a bug would be out of memory or
-         if the signed certificate list contained an invalid certificate. */
-      if (ret != 0)
-        {
-          printf ("Error base64 certificate.\n");
-          continue;
-        }
+      pc_to_remove = b64_to_cert_context(to_remove[i],
+                                         strnlen(to_remove[i], MAX_LINE_LENGTH));
 
-      printf ("Deleting cert %s\n", to_remove[i]);
-      pc_to_remove = CertCreateContext (CERT_STORE_CERTIFICATE_CONTEXT,
-                                        X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
-                                        (const PBYTE) asn1_data,
-                                        (DWORD) asn1_size,
-                                        0,
-                                        NULL);
-      free (asn1_data); /* Windows has a copy */
       if (pc_to_remove == NULL)
         {
           LPWSTR error = getLastErrorMsg();
           if (error)
             {
-              printf ("Failed to add certificate: %S \n", error);
+              printf ("Failed to create cert context: %S \n", error);
               LocalFree (error);
             }
           continue;
@@ -88,31 +106,6 @@
                                           pc_to_remove,
                                           NULL);
 
-
-        {
-          char pszNameString[256];
-          if(CertGetNameString(   
-             pc_to_remove,   
-             CERT_NAME_SIMPLE_DISPLAY_TYPE,   
-             0,
-             NULL,   
-             pszNameString,   
-             128))
-          {
-            printf("wanted: %s \n",pszNameString);
-          }
-          if(CertGetNameString(   
-             pCert,   
-             CERT_NAME_SIMPLE_DISPLAY_TYPE,   
-             0,
-             NULL,   
-             pszNameString,   
-             128))
-          {
-             printf("got: %s \n",pszNameString);
-          }
-        }
-
       CertFreeCertificateContext (pc_to_remove);
 
       if (pCert == NULL)
@@ -120,18 +113,6 @@
           printf ("Did not find certificate\n");
           continue;
         }
-      for (j = 0; j < pCert->cbCertEncoded; j++) {
-          if (asn1_data[j] != pCert->pbCertEncoded[j]) {
-              printf("%1x", (unsigned)(unsigned char)pCert->pbCertEncoded[j]);
-          }
-      }
-      printf("\nWanted: \n");
-      for (j = 0; j < pCert->cbCertEncoded; j++) {
-          if (asn1_data[j] != pCert->pbCertEncoded[j]) {
-              printf("%1x", (unsigned)(unsigned char)asn1_data[j]);
-          }
-      }
-      printf("\n");
 
       if (!CertDeleteCertificateFromStore (pCert))
         {
@@ -161,27 +142,36 @@
 
   for (i = 0; to_install[i]; i++)
     {
-      size_t cert_len = strnlen (to_install[i], MAX_LINE_LENGTH),
-             buf_size = 0;
-      char *buf = NULL;
+      PCCERT_CONTEXT pc_to_add = NULL;
+      PCCERT_CONTEXT found_cert = NULL;
 
-      ret = str_base64_decode (&buf, &buf_size, to_install[i], cert_len);
+      pc_to_add = b64_to_cert_context(to_install[i],
+                                      strnlen(to_install[i], MAX_LINE_LENGTH));
 
-      if (ret != 0)
+      if (pc_to_add == NULL)
         {
-          printf ("decoding certificate failed\n");
-          return;
+          continue;
         }
 
-      printf ("Adding cert %s\n", to_install[i]);
+      found_cert = CertFindCertificateInStore (hStore,
+                                               X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
+                                               0,
+                                               CERT_FIND_EXISTING,
+                                               pc_to_add,
+                                               NULL);
+      if (found_cert != NULL)
+        {
+          printf ("Certificate already in store\n");
+          CertFreeCertificateContext (found_cert);
+          CertFreeCertificateContext (pc_to_add);
+          continue;
+        }
 
-      ret = CertAddEncodedCertificateToStore (hStore,
-                                              X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
-                                              (PBYTE) buf,
-                                              buf_size,
+      ret = CertAddCertificateContextToStore (hStore,
+                                              pc_to_add,
                                               CERT_STORE_ADD_ALWAYS,
                                               NULL);
-
+      CertFreeCertificateContext (pc_to_add);
       if (!ret)
         {
           LPWSTR error = getLastErrorMsg();
@@ -191,7 +181,6 @@
               LocalFree (error);
             }
         }
-      free (buf);
     }
   return;
 }
--- a/ui/tests/windowsstoretest.cpp	Thu Mar 27 10:23:49 2014 +0000
+++ b/ui/tests/windowsstoretest.cpp	Thu Mar 27 14:16:22 2014 +0000
@@ -6,6 +6,22 @@
 
 #include <QTest>
 
+void WindowsStoreTest::dumpContents() {
+    char pszNameString[256];
+    PCCERT_CONTEXT pCert = NULL;
+    qDebug() << "Currently in store: " ;
+    while((pCert = CertEnumCertificatesInStore(testStore, pCert))) {
+        if(CertGetNameString(pCert,
+                             CERT_NAME_SIMPLE_DISPLAY_TYPE,
+                             0,
+                             NULL,
+                             pszNameString,
+                             128)){
+            qDebug() << "   " << pszNameString ;
+        }
+    }
+}
+
 void WindowsStoreTest::initTestCase() {
     testStore = CertOpenStore(
       CERT_STORE_PROV_MEMORY,    // A memory store
@@ -60,7 +76,6 @@
         QVERIFY(certFound);
         i++;
     }
-    qDebug() << i;
     QVERIFY ((size_t)validList.getInstallCertificates().size() == i);
 
     /* Remove all except one */
@@ -100,7 +115,7 @@
         bool certFound = false;
         QByteArray data = QByteArray::fromRawData((const char*) pCert->pbCertEncoded,
                 pCert->cbCertEncoded);
-        qDebug() << "Have in store: " << data.toBase64();
+        QVERIFY (data.toBase64() != to_remove[0]);
         for (int j = 0; j < validList.getInstallCertificates().size() - 1; j++) {
             const Certificate &cert = validList.getInstallCertificates()[j];
             QByteArray asn1data = QByteArray::fromBase64(
@@ -117,7 +132,8 @@
 
     /* Install all again and remove them afterwards */
     do_install(testStore, to_install);
-    do_remove(testStore, to_remove);
+    do_remove(testStore, to_install);
+
     QVERIFY (CertEnumCertificatesInStore(testStore, pCert) == NULL);
 
     strv_free(to_install);
--- a/ui/tests/windowsstoretest.h	Thu Mar 27 10:23:49 2014 +0000
+++ b/ui/tests/windowsstoretest.h	Thu Mar 27 14:16:22 2014 +0000
@@ -17,6 +17,8 @@
     HCERTSTORE testStore;
     QTemporaryFile tmpFile;
 
+    void dumpContents();
+
 private Q_SLOTS:
     void initTestCase();
     void cleanupTestCase();

http://wald.intevation.org/projects/trustbridge/