changeset 1081:edbf5e5e88f4

(issue118) Extend verify_binary to carry an open file * binverify.c: Change result to a structure containing an open fptr Use in Memory data for windows verification. * mainwindow.cpp, selftest.c: Handle the returend structure * binverifytest.cpp: Test for the exclusive read and update signature. * listutil.c: Add optional fptr parameter to read_file
author Andre Heinecke <andre.heinecke@intevation.de>
date Thu, 11 Sep 2014 12:05:24 +0200
parents 898b1ddcca11
children a12e6172d82c
files common/binverify.c common/binverify.h common/listutil.c common/listutil.h common/selftest.c ui/mainwindow.cpp ui/tests/binverifytest.cpp
diffstat 7 files changed, 172 insertions(+), 56 deletions(-) [+]
line wrap: on
line diff
--- a/common/binverify.c	Thu Sep 11 12:00:10 2014 +0200
+++ b/common/binverify.c	Thu Sep 11 12:05:24 2014 +0200
@@ -10,6 +10,7 @@
 
 #include "strhelp.h"
 #include "logging.h"
+#include "listutil.h"
 #ifdef RELEASE_BUILD
 #include "pubkey-release.h"
 #else
@@ -19,8 +20,13 @@
 bin_verify_result
 verify_binary(const char *filename, size_t name_len)
 {
-  if (!filename || !name_len)
-    return VerifyUnknownError;
+  if (!filename || !name_len) {
+    bin_verify_result retval;
+    retval.fptr = NULL;
+    retval.result = VerifyUnknownError;
+    return retval;
+  }
+
 #ifdef WIN32
   return verify_binary_win(filename, name_len);
 #else
@@ -101,7 +107,7 @@
 bin_verify_result
 verify_binary_win(const char *filename, size_t name_len)
 {
-  bin_verify_result retval = VerifyUnknownError;
+  bin_verify_result retval;
   WCHAR *filenameW = NULL;
   BOOL result = FALSE;
   DWORD dwEncoding = 0,
@@ -112,17 +118,34 @@
   HCRYPTMSG hMsg = NULL;
   PCERT_INFO pSignerCert = NULL;
   PCCERT_CONTEXT pSignerCertContext = NULL;
+  FILE *fptr = NULL;
+  size_t data_size = 0;
+  char *data = NULL;
+  int ret = -1;
+  CRYPT_INTEGER_BLOB blob;
+
+  retval.result = VerifyUnknownError;
+  retval.fptr = NULL;
 
   if (!filename || name_len > MAX_PATH || strlen(filename) != name_len)
     {
       ERRORPRINTF ("Invalid parameters\n");
-      return VerifyUnknownError;
+      return retval;
     }
 
-  filenameW = utf8_to_wchar(filename, name_len);
+  ret = read_file(filename, &data, &data_size, MAX_VALID_BIN_SIZE, &fptr);
 
-  result = CryptQueryObject (CERT_QUERY_OBJECT_FILE,
-                             filenameW,
+  if (ret != 0)
+    {
+      ERRORPRINTF ("Read file failed with error: %i\n", ret);
+      retval.result = VerifyReadFailed;
+      return retval;
+    }
+  blob.cbData = (DWORD) data_size;
+  blob.pbData = (PBYTE) data;
+
+  result = CryptQueryObject (CERT_QUERY_OBJECT_BLOB,
+                             &blob,
                              CERT_QUERY_CONTENT_FLAG_PKCS7_SIGNED_EMBED,
                              CERT_QUERY_FORMAT_FLAG_BINARY,
                              0,
@@ -136,7 +159,7 @@
   if (!result || !hMsg)
     {
       PRINTLASTERROR ("Failed to query crypto object");
-      retval = VerifyReadFailed;
+      retval.result = VerifyReadFailed;
       goto done;
     }
 
@@ -152,7 +175,7 @@
   else
     {
       ERRORPRINTF ("Failed to get signer cert size.");
-      retval = VerifyUnknownError;
+      retval.result = VerifyUnknownError;
       goto done;
     }
 
@@ -163,7 +186,7 @@
                          &dwSignerInfoSize)))
     {
       ERRORPRINTF ("Failed to get signer cert.");
-      retval = VerifyUnknownError;
+      retval.result = VerifyUnknownError;
       goto done;
     }
 
@@ -175,7 +198,7 @@
   if (!pSignerCertContext)
     {
       ERRORPRINTF ("Failed to find signer cert in store.");
-      retval = VerifyUnknownError;
+      retval.result = VerifyUnknownError;
       goto done;
     }
 
@@ -186,7 +209,7 @@
                       pSignerCertContext->pCertInfo))
     {
       ERRORPRINTF ("The signature is invalid. \n");
-      retval = VerifyInvalidSignature;
+      retval.result = VerifyInvalidSignature;
       syslog_error_printf ("Software update embedded signature is invalid.");
       goto done;
     }
@@ -194,22 +217,29 @@
   if(check_certificate(pSignerCertContext))
     {
       DEBUGPRINTF ("Valid signature with pinned certificate.");
-      retval = VerifyValid;
+      retval.result = VerifyValid;
+      retval.fptr = fptr;
       goto done;
     }
   else
     {
       ERRORPRINTF ("Certificate mismatch. \n");
-      retval = VerifyInvalidCertificate;
+      retval.result = VerifyInvalidCertificate;
       syslog_error_printf ("Software update embedded signature "
                            "created with wrong certificate.");
       goto done;
     }
 
 done:
+  xfree(data);
   xfree(filenameW);
   xfree(pSignerCert);
 
+  if (retval.result != VerifyValid)
+    {
+      fclose(fptr);
+    }
+
   if(pSignerCertContext)
     {
       CertFreeCertificateContext(pSignerCertContext);
@@ -226,8 +256,6 @@
 }
 #else /* WIN32 */
 
-#include "listutil.h"
-
 #pragma GCC diagnostic ignored "-Wconversion"
 /* Polarssl mh.h contains a conversion which gcc warns about */
 #include <polarssl/pk.h>
@@ -248,29 +276,34 @@
          sig_size = TRUSTBRIDGE_RSA_KEY_SIZE / 8;
   unsigned char signature[sig_size],
            hash[32];
+  FILE *fptr = NULL;
 
-  bin_verify_result retval = VerifyUnknownError;
+  bin_verify_result retval;
+  retval.result = VerifyUnknownError;
+  retval.fptr = NULL;
   x509_crt codesign_cert;
 
   if (strnlen(filename, name_len + 1) != name_len || name_len == 0)
     {
       ERRORPRINTF ("Invalid call to verify_binary_linux\n");
-      return VerifyUnknownError;
+      retval.result = VerifyUnknownError;
+      return retval;
     }
 
-  ret = read_file(filename, &data, &data_size, MAX_VALID_BIN_SIZE);
+  ret = read_file(filename, &data, &data_size, MAX_VALID_BIN_SIZE, &fptr);
 
   if (ret != 0)
     {
       ERRORPRINTF ("Read file failed with error: %i\n", ret);
-      return VerifyReadFailed;
+      retval.result = VerifyReadFailed;
+      return retval;
     }
 
   /* Fetch the signature from the end of data */
   if (data_size < sig_b64_size + 5)
     {
       ERRORPRINTF ("File to small to contain a signature.\n");
-      retval = VerifyInvalidSignature;
+      retval.result = VerifyInvalidSignature;
       goto done;
     }
 
@@ -280,7 +313,7 @@
       data[data_size - sig_b64_size - 5] != '\r')
     {
       ERRORPRINTF ("Failed to find valid signature line.\n");
-      retval = VerifyInvalidSignature;
+      retval.result = VerifyInvalidSignature;
       goto done;
     }
 
@@ -312,7 +345,8 @@
       errbuf[1019] = '\0'; /* Just to be sure */
       ERRORPRINTF ("x509_crt_parse failed with -0x%04x\n%s\n", -ret, errbuf);
       x509_crt_free(&codesign_cert);
-      return VerifyUnknownError;
+      retval.result = VerifyUnknownError;
+      goto done;
     }
 
   ret = pk_verify(&codesign_cert.pk, POLARSSL_MD_SHA256, hash, 0,
@@ -325,14 +359,22 @@
       errbuf[1019] = '\0'; /* Just to be sure */
       ERRORPRINTF ("pk_verify failed with -0x%04x\n %s\n", -ret, errbuf);
       x509_crt_free(&codesign_cert);
-      retval = VerifyInvalidSignature;
+      retval.result = VerifyInvalidSignature;
       goto done;
     }
   x509_crt_free(&codesign_cert);
 
-  retval = VerifyValid;
+  retval.result = VerifyValid;
+  retval.fptr = fptr;
 
 done:
+  if (retval.result != VerifyValid)
+    {
+      if (fptr)
+        {
+          fclose(fptr);
+        }
+    }
   xfree (data);
   return retval;
 }
--- a/common/binverify.h	Thu Sep 11 12:00:10 2014 +0200
+++ b/common/binverify.h	Thu Sep 11 12:05:24 2014 +0200
@@ -13,13 +13,14 @@
  */
 #include <stdbool.h>
 #include <stddef.h>
+#include <stdio.h>
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
 /**
- * @enum bin_verify_result
+ * @enum verify_result
  * @brief Result of a verification
  */
 typedef enum {
@@ -28,6 +29,19 @@
     VerifyInvalidSignature = 4, /*! Signature was invalid */
     VerifyInvalidCertificate = 5, /*! Certificate mismatch */
     VerifyReadFailed = 6, /*! File exists but could not read the file */
+} verify_result;
+
+/**
+ * A structure containing a verify_result and a reference to the
+ * verified file.
+ */
+typedef struct {
+    /*@{*/
+    verify_result result; /**< the result of the verification */
+    FILE *fptr; /**< Pointer to the open file struct of the verified file
+                    The ptr is only valid if verify_result is VerifyValid
+                    and needs to be closed by the caller in that case.*/
+    /*@}*/
 } bin_verify_result;
 
 /**
@@ -57,14 +71,15 @@
  */
 bin_verify_result verify_binary(const char *filename, size_t name_len);
 
+/**@def Max size of a valid binary in byte */
+#define MAX_VALID_BIN_SIZE (32 * 1024 * 1024)
+
 #ifdef WIN32
 /**
  * @brief windows implementation of verify_binary
  */
 bin_verify_result verify_binary_win(const char *filename, size_t name_len);
 #else /* WIN32 */
-/**@def Max size of a valid binary in byte */
-#define MAX_VALID_BIN_SIZE (32 * 1024 * 1024)
 
 /**
  * @brief linux implementation of verify_binary
--- a/common/listutil.c	Thu Sep 11 12:00:10 2014 +0200
+++ b/common/listutil.c	Thu Sep 11 12:05:24 2014 +0200
@@ -16,6 +16,10 @@
 #include <sys/stat.h>
 #include <string.h>
 
+#ifdef WIN32
+#include <share.h>
+#endif
+
 #include "strhelp.h"
 #include "logging.h"
 
@@ -41,7 +45,7 @@
 #define READ_FILE_INVALID_CALL -5
 int
 read_file(const char *file_name, char **data, size_t *size,
-          const size_t max_size)
+          const size_t max_size, FILE **fptr)
 {
   FILE *f;
   long file_size;
@@ -50,8 +54,23 @@
     {
       return READ_FILE_INVALID_CALL;
     }
-
+#ifdef WIN32
+    {
+      wchar_t *wFilename = utf8_to_wchar(file_name, strlen(file_name));
+      if (!wFilename)
+        {
+          return READ_FILE_UNREADABLE;
+        }
+      /* We open and write protect the file here so that
+         as long as the file is open we can be sure that
+         it was not modified and can use it in subsequent
+         calls based on the filename. */
+      f = _wfsopen(wFilename, L"rb", _SH_DENYWR);
+      xfree(wFilename);
+    }
+#else
   f = fopen(file_name, "rb");
+#endif
   if (f == NULL)
     return READ_FILE_UNREADABLE;
 
@@ -92,7 +111,14 @@
       return READ_FILE_READ_FAILED;
     }
 
-  fclose(f);
+  if (fptr)
+    {
+      *fptr = f;
+    }
+  else
+    {
+      fclose(f);
+    }
 
   (*data)[*size] = '\0';
 
@@ -180,7 +206,7 @@
   *size = 0;
   int ret = 0;
 
-  ret = read_file(file_name, data, size, MAX_FILESIZE);
+  ret = read_file(file_name, data, size, MAX_FILESIZE, NULL);
 
   /* printf ("Ret: %i \n", ret); */
   if (ret != 0)
--- a/common/listutil.h	Thu Sep 11 12:00:10 2014 +0200
+++ b/common/listutil.h	Thu Sep 11 12:05:24 2014 +0200
@@ -13,6 +13,7 @@
 #endif
 
 #include <stddef.h>
+#include <stdio.h>
 
 /**
  * @file listutil.h
@@ -84,17 +85,22 @@
 /**
  *  @brief Read a file into memory.
  *
- * The caller needs to free data
+ * The caller needs to free data. If fptr is not NULL it will
+ * recieve the pointer to the read file structure. The caller
+ * is responsible for closing this.
+ * fptr only needs to be closed and is only valid if the
+ * return value is 0.
  *
  * @param[in] file_name Name of the file.
  * @param[out] data the file content
  * @param[out] size size in bytes of the file content.
  * @param[in] max_size the maximum amount of bytes to read.
+ * @param[out] fptr pointer to recieve the FILE ptr or NULL
  *
  * @return 0 on success an error code otherwise.
  */
 int read_file(const char *file_name, char **data, size_t *size,
-              const size_t max_size);
+              const size_t max_size, FILE **fptr);
 #ifdef __cplusplus
 }
 #endif
--- a/common/selftest.c	Thu Sep 11 12:00:10 2014 +0200
+++ b/common/selftest.c	Thu Sep 11 12:05:24 2014 +0200
@@ -6,6 +6,7 @@
 bool
 selftest()
 {
+  bin_verify_result res;
 #ifdef WIN32
   wchar_t wPath[MAX_PATH];
   char *utf8path = NULL;
@@ -27,7 +28,8 @@
       return false;
     }
 
-  if (verify_binary (utf8path, strlen(utf8path)) != VerifyValid)
+  res = verify_binary (utf8path, strlen(utf8path));
+  if (res.result != VerifyValid)
     {
       ERRORPRINTF ("Verification of the binary failed");
       syslog_error_printf ("Integrity check failed.");
@@ -35,13 +37,17 @@
       return false;
     }
 
+  fclose(res.fptr);
   xfree(utf8path);
   return true;
 #else
-  if (!verify_binary ("/proc/self/exe", 14) != VerifyValid)
+  res = verify_binary ("/proc/self/exe", 14);
+  if (res.result != VerifyValid)
     {
       syslog_error_printf ("Integrity check failed.");
       return false;
     }
+  fclose(res.fptr);
+  return true;
 #endif
 }
--- a/ui/mainwindow.cpp	Thu Sep 11 12:00:10 2014 +0200
+++ b/ui/mainwindow.cpp	Thu Sep 11 12:05:24 2014 +0200
@@ -234,14 +234,15 @@
     }
     bin_verify_result verifyResult = verify_binary(swFileName.toUtf8().constData(),
             swFileName.toUtf8().size());
-    qDebug() << "Binary verify result: " << verifyResult;
-    if (verifyResult != VerifyValid) {
+    qDebug() << "Binary verify result: " << verifyResult.result;
+    if (verifyResult.result != VerifyValid) {
         qDebug() << "Failed to verify downloaded data.";
         QFile::remove(swFileName);
         mSettings.remove("Software/available");
         mSettings.remove("Software/availableDate");
         return;
     }
+    fclose(verifyResult.fptr);
 }
 
 void MainWindow::handleNewList(const QString& fileName, const QDateTime& modDate) {
@@ -289,9 +290,10 @@
 void MainWindow::installNewSW(const QString& fileName, const QDateTime& modDate) {
     QFileInfo instProcInfo = QFileInfo(fileName);
     QString filePath = QDir::toNativeSeparators(instProcInfo.absoluteFilePath());
+    bin_verify_result vres = verify_binary(filePath.toUtf8().constData(),
+            filePath.toUtf8().size());
 
-    if (verify_binary(filePath.toUtf8().constData(),
-            filePath.toUtf8().size()) != VerifyValid) {
+    if (vres.result != VerifyValid) {
         qDebug() << "Invalid software. Not installing";
         return;
       }
@@ -328,6 +330,7 @@
         free(errmsg);
         qDebug() << "Failed to start process: " << qerrmsg;
         setState(NewSoftwareAvailable);
+        fclose(vres.fptr);
         return;
     }
 #else /* WIN32 */
@@ -360,12 +363,15 @@
     qDebug() << "Starting process " << filePath <<" params: " << parameters;
     if (!sudo_started && !QProcess::startDetached(filePath, parameters)) {
         qDebug() << "Failed to start process.";
+        fclose(vres.fptr);
         return;
     }
 
 #endif
+
+    syslog_info_printf ("Installing update: %s\n", fileName.toUtf8().constData());
     /* Installer process should now be running. We exit */
-
+    fclose(vres.fptr);
     closeApp();
 }
 
--- a/ui/tests/binverifytest.cpp	Thu Sep 11 12:00:10 2014 +0200
+++ b/ui/tests/binverifytest.cpp	Thu Sep 11 12:05:24 2014 +0200
@@ -30,17 +30,17 @@
 /* Some general robustness checks */
 void BinVerifyTest::testMiscErrors()
 {
-  QVERIFY (verify_binary (NULL, 10) != VerifyValid);
-  QVERIFY (verify_binary ("foo", 10) != VerifyValid);
-  QVERIFY (verify_binary ("bar", -1) != VerifyValid);
+  QVERIFY (verify_binary (NULL, 10).result != VerifyValid);
+  QVERIFY (verify_binary ("foo", 10).result != VerifyValid);
+  QVERIFY (verify_binary ("bar", -1).result!= VerifyValid);
   /* On windows the next line will check that a valid microsoft
    * signed executable is not valid for us (pinning). On linux
    * it will just fail with a read error which we tested above */
 #ifdef Q_OS_WIN
   QVERIFY (verify_binary ("c:\\Windows\\System32\\mmc.exe",
-                          strlen("c:\\Windows\\System32\\mmc.exe")) != VerifyInvalidCertificate);
+                          strlen("c:\\Windows\\System32\\mmc.exe")).result != VerifyInvalidCertificate);
 #endif
-  QVERIFY (verify_binary ("/dev/null", strlen("/dev/null")) != VerifyValid);
+  QVERIFY (verify_binary ("/dev/null", strlen("/dev/null")).result != VerifyValid);
 }
 
 /* Check that a signature with only a different key (of the same size)
@@ -48,14 +48,16 @@
 void BinVerifyTest::testOtherKey()
 {
     QVERIFY(VerifyInvalidSignature == verify_binary ("fakeinst-other-key" EXE_SUFFIX,
-                strlen("fakeinst-other-key" EXE_SUFFIX)));
+                strlen("fakeinst-other-key" EXE_SUFFIX)).result);
 }
 
 /* Check that an invalid signature is not validated */
 void BinVerifyTest::testInvalidSig()
 {
-    QVERIFY(VerifyValid != verify_binary ("fakeinst-invalid" EXE_SUFFIX,
-                strlen("fakeinst-invalid" EXE_SUFFIX)));
+    bin_verify_result res = verify_binary ("fakeinst-invalid" EXE_SUFFIX,
+                strlen("fakeinst-invalid" EXE_SUFFIX));
+    QVERIFY(VerifyValid != res.result);
+    QVERIFY(res.fptr == NULL);
 }
 
 #ifdef Q_OS_WIN
@@ -64,22 +66,34 @@
 void BinVerifyTest::testOtherCert()
 {
     QVERIFY(VerifyInvalidCertificate == verify_binary ("fakeinst-other-cert" EXE_SUFFIX,
-                strlen("fakeinst-other-cert" EXE_SUFFIX)));
+                strlen("fakeinst-other-cert" EXE_SUFFIX)).result);
 }
 #endif
 
 /* Check that no signature is not validated */
 void BinVerifyTest::testNoSignature()
 {
-    QVERIFY(VerifyValid != verify_binary ("fakeinst" EXE_SUFFIX,
-                strlen("fakeinst" EXE_SUFFIX)));
+    bin_verify_result res = verify_binary ("fakeinst" EXE_SUFFIX,
+                strlen("fakeinst" EXE_SUFFIX));
+    QVERIFY(VerifyValid != res.result);
+    QVERIFY(res.fptr == NULL);
 }
 
 /* Check that a valid signed executable is verified */
 void BinVerifyTest::testValidBinary()
 {
-  QVERIFY (VerifyValid == verify_binary ("fakeinst-signed" EXE_SUFFIX,
-                                         strlen("fakeinst-signed" EXE_SUFFIX)));
+    bin_verify_result res = verify_binary ("fakeinst-signed" EXE_SUFFIX,
+                                          strlen("fakeinst-signed" EXE_SUFFIX));
+    QVERIFY (VerifyValid == res.result);
+    QFile thefile ("fakeinst-signed" EXE_SUFFIX);
+#ifdef WIN32
+    /* Verifies the deny write open mode. But on linuy we dont have it. */
+    QVERIFY (!thefile.open(QIODevice::ReadWrite));
+#endif
+    QVERIFY (res.fptr != NULL);
+    fclose(res.fptr);
+    QVERIFY (thefile.open(QIODevice::ReadWrite));
+    thefile.close();
 }
 
 void BinVerifyTest::testSignatureCreation()
@@ -95,8 +109,9 @@
   bool ret = theDialog->appendTextSignatureToFile (garbage, outfile.fileName());
   QVERIFY(QFile::remove(garbage));
   QVERIFY(ret == true);
-  QVERIFY(VerifyValid == verify_binary (outfile.fileName().toUtf8().constData(),
-                                        outfile.fileName().toUtf8().size()));
+  bin_verify_result res = verify_binary (outfile.fileName().toUtf8().constData(),
+                                        outfile.fileName().toUtf8().size());
+  QVERIFY(VerifyValid == res.result);
 }
 bool g_debug = true;
 

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