[PATCH 1 of 2] (issue118) Extend verify_binary to carry an open file
Wald Commits
scm-commit at wald.intevation.org
Thu Sep 11 12:06:22 CEST 2014
# HG changeset patch
# User Andre Heinecke <andre.heinecke at intevation.de>
# Date 1410429924 -7200
# Node ID edbf5e5e88f4cb2c5b18e54709a46cd9ba47efc4
# Parent 898b1ddcca11529d813b7923346c4d56609642d9
(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
diff -r 898b1ddcca11 -r edbf5e5e88f4 common/binverify.c
--- 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;
}
diff -r 898b1ddcca11 -r edbf5e5e88f4 common/binverify.h
--- 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
diff -r 898b1ddcca11 -r edbf5e5e88f4 common/listutil.c
--- 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)
diff -r 898b1ddcca11 -r edbf5e5e88f4 common/listutil.h
--- 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
diff -r 898b1ddcca11 -r edbf5e5e88f4 common/selftest.c
--- 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
}
diff -r 898b1ddcca11 -r edbf5e5e88f4 ui/mainwindow.cpp
--- 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();
}
diff -r 898b1ddcca11 -r edbf5e5e88f4 ui/tests/binverifytest.cpp
--- 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;
More information about the Trustbridge-commits
mailing list