[pki] add signature check on update downloads - part 2

* Closes #158

res/localization/rufus.loc

@@ -539,6 +539,9 @@ t MSG_279 "Non bootable"
 t MSG_280 "Image selection"
 t MSG_281 "(Please select an image)"
 t MSG_282 "Drive locking"
+t MSG_283 "Invalid signature"
+t MSG_284 "The downloaded executable is missing a digital signature."
+t MSG_285 "The downloaded executable was signed by '%s'.\nThis doesn't look right... Are you sure you want to run it?"
 
 ################################################################################
 ############################# TRANSLATOR END COPY ##############################

src/localization_data.h

@@ -392,6 +392,13 @@ const loc_control_id control_id[] = {
 	LOC_CTRL(MSG_280),
 	LOC_CTRL(MSG_281),
 	LOC_CTRL(MSG_282),
+	LOC_CTRL(MSG_283),
+	LOC_CTRL(MSG_284),
+	LOC_CTRL(MSG_285),
+	LOC_CTRL(MSG_286),
+	LOC_CTRL(MSG_287),
+	LOC_CTRL(MSG_288),
+	LOC_CTRL(MSG_289),
 	LOC_CTRL(MSG_MAX),
 	LOC_CTRL(IDOK),
 	LOC_CTRL(IDCANCEL),

src/pki.c

@@ -30,10 +30,12 @@
 
 #include "rufus.h"
 #include "msapi_utf8.h"
+#include "resource.h"
+#include "localization.h"
 
 #define ENCODING (X509_ASN_ENCODING | PKCS_7_ASN_ENCODING)
 
-// Signatures names we accept (may be suffixed, but signature must start with one of those)
+// Signatures names we accept (may be suffixed, but the signature should start with one of those)
 const char* valid_cert_names[] = { "Akeo Consulting", "Akeo Systems", "Pete Batard" };
 
 typedef struct {
@@ -43,9 +45,11 @@ typedef struct {
 } SPROG_PUBLISHERINFO, *PSPROG_PUBLISHERINFO;
 
 // Mostly from https://support.microsoft.com/en-us/kb/323809
-BOOL CheckSignatureAttributes(const char* path)
+static char* GetSignatureName(const char* path)
 {
-	BOOL r = FALSE;
+	static char szSubjectName[128];
+	char* p = NULL;
+	BOOL r;
 	HCERTSTORE hStore = NULL;
 	HCRYPTMSG hMsg = NULL;
 	PCCERT_CONTEXT pCertContext = NULL;
@@ -56,8 +60,6 @@ BOOL CheckSignatureAttributes(const char* path)
 	CERT_INFO CertInfo = { 0 };
 	SPROG_PUBLISHERINFO ProgPubInfo = { 0 };
 	wchar_t *szFileName = utf8_to_wchar(path);
-	char szSubjectName[128];
-	int i;
 
 	// Get message handle and store handle from the signed file.
 	r = CryptQueryObject(CERT_QUERY_OBJECT_FILE, szFileName,
@@ -79,7 +81,6 @@ BOOL CheckSignatureAttributes(const char* path)
 	pSignerInfo = (PCMSG_SIGNER_INFO)calloc(dwSignerInfo, 1);
 	if (!pSignerInfo) {
 		uprintf("PKI: Could not allocate memory for signer information");
-		r = FALSE;
 		goto out;
 	}
 
@@ -97,7 +98,6 @@ BOOL CheckSignatureAttributes(const char* path)
 	pCertContext = CertFindCertificateInStore(hStore, ENCODING, 0, CERT_FIND_SUBJECT_CERT, (PVOID)&CertInfo, NULL);
 	if (!pCertContext) {
 		uprintf("PKI: Failed to locate signer certificate in temporary store: %s", WindowsErrorString());
-		r = FALSE;
 		goto out;
 	}
 
@@ -106,20 +106,11 @@ BOOL CheckSignatureAttributes(const char* path)
 		szSubjectName, sizeof(szSubjectName));
 	if (dwSubjectSize <= 1) {
 		uprintf("PKI: Failed to get Subject Name");
-		r = FALSE;
 		goto out;
 	}
 
-	uprintf("Executable is signed by '%s'", szSubjectName);
+	uprintf("Downloaded executable is signed by '%s'", szSubjectName);
-
+	p = szSubjectName;
-	// Now check the signature name. Make it specific enough (i.e. don't simply check for "Akeo")
-	// so that, besides hacking our server, it'll place an extra hurdle on any malicious entity
-	// into also fooling a C.A. to issue a certificate that passes our test. 
-	for (i = 0; i < ARRAYSIZE(valid_cert_names); i++) {
-		r = (strncmp(szSubjectName, valid_cert_names[i], strlen(valid_cert_names[i])) == 0);
-		if (r)
-			break;
-	}
 
 out:
 	safe_free(szFileName);
@@ -134,7 +125,7 @@ out:
 		CertCloseStore(hStore, 0);
 	if (hMsg != NULL)
 		CryptMsgClose(hMsg);
-	return r;
+	return p;
 }
 
 // From https://msdn.microsoft.com/en-us/library/windows/desktop/aa382384.aspx
@@ -145,20 +136,47 @@ LONG ValidateSignature(HWND hDlg, const char* path)
 	WINTRUST_FILE_INFO trust_file = { 0 };
 	GUID guid_generic_verify =	// WINTRUST_ACTION_GENERIC_VERIFY_V2
 		{ 0xaac56b, 0xcd44, 0x11d0,{ 0x8c, 0xc2, 0x0, 0xc0, 0x4f, 0xc2, 0x95, 0xee } };
-	wchar_t *szFileName = utf8_to_wchar(path);
+	char *signature_name;
+	int i;
+
+	// Check the signature name. Make it specific enough (i.e. don't simply check for "Akeo")
+	// so that, besides hacking our server, it'll place an extra hurdle on any malicious entity
+	// into also fooling a C.A. to issue a certificate that passes our test. 
+	signature_name = GetSignatureName(path);
+	if (signature_name == NULL) {
+		uprintf("PKI: Could not get signature name");
+		MessageBoxU(hDlg, lmprintf(MSG_284), lmprintf(MSG_283), MB_OK | MB_ICONERROR | MB_IS_RTL);
+		return TRUST_E_NOSIGNATURE;
+	}
+	for (i = 0; i < ARRAYSIZE(valid_cert_names); i++) {
+		if (strncmp(signature_name, valid_cert_names[i], strlen(valid_cert_names[i])) == 0)
+			break;
+	}
+	if (i >= ARRAYSIZE(valid_cert_names)) {
+		uprintf("PKI: Signature '%s' doesn't look right...", signature_name);
+		if (MessageBoxU(hDlg, lmprintf(MSG_285, signature_name), lmprintf(MSG_283),
+			MB_YESNO | MB_ICONWARNING | MB_IS_RTL) != IDYES)
+			return TRUST_E_EXPLICIT_DISTRUST;
+	}
 
 	trust_file.cbStruct = sizeof(trust_file);
-	trust_file.pcwszFilePath = szFileName;
+	trust_file.pcwszFilePath = utf8_to_wchar(path);
+	if (trust_file.pcwszFilePath == NULL) {
+		uprintf("PKI: Unable to convert '%s' to UTF16", path);
+		return ERROR_SEVERITY_ERROR | FAC(FACILITY_CERT) | ERROR_NOT_ENOUGH_MEMORY;
+	}
 
 	trust_data.cbStruct = sizeof(trust_data);
 	trust_data.dwUIChoice = WTD_UI_ALL;
+	// We just downloaded from the Internet, so we should be able to check revocation
 	trust_data.fdwRevocationChecks = WTD_REVOKE_WHOLECHAIN;
-	trust_data.dwUnionChoice = WTD_CHOICE_FILE;
-	trust_data.pFile = &trust_file;
 	// 0x400 = WTD_MOTW  for Windows 8.1 or later
 	trust_data.dwProvFlags = WTD_REVOCATION_CHECK_CHAIN | 0x400;
+	trust_data.dwUnionChoice = WTD_CHOICE_FILE;
+	trust_data.pFile = &trust_file;
 
 	r = WinVerifyTrust(NULL, &guid_generic_verify, &trust_data);
-	safe_free(szFileName);
+	safe_free(trust_file.pcwszFilePath);
+
 	return r;
 }

src/resource.h

@@ -430,7 +430,14 @@
 #define MSG_280                         3280
 #define MSG_281                         3281
 #define MSG_282                         3282
-#define MSG_MAX                         3283
+#define MSG_283                         3283
+#define MSG_284                         3284
+#define MSG_285                         3285
+#define MSG_286                         3286
+#define MSG_287                         3287
+#define MSG_288                         3288
+#define MSG_289                         3289
+#define MSG_MAX                         3290
 
 // Next default values for new objects
 // 

src/rufus.c

@@ -2054,8 +2054,6 @@ void SaveVHD(void)
 /*
  * Main dialog callback
  */
-extern BOOL CheckSignatureAttributes(const char* path);
-extern LONG ValidateSignature(HWND hDlg, const char* path);
 static INT_PTR CALLBACK MainCallback(HWND hDlg, UINT message, WPARAM wParam, LPARAM lParam)
 {
 	static DWORD DeviceNum = 0, LastRefresh = 0;
@@ -2274,9 +2272,6 @@ static INT_PTR CALLBACK MainCallback(HWND hDlg, UINT message, WPARAM wParam, LPA
 			break;
 #ifdef RUFUS_TEST
 		case IDC_TEST:
-			uprintf("CHK: %d", CheckSignatureAttributes("C:\\Downloads\\rufus-2.4.exe"));
-			SetLastError(ValidateSignature(hDlg, "C:\\Downloads\\rufus-2.4.exe"));
-			uprintf("VAL: %s", WindowsErrorString());
 			break;
 #endif
 		case IDC_ADVANCED:

src/rufus.h

@@ -32,7 +32,7 @@
 /* Program options */
 #define RUFUS_DEBUG                 // print debug info to Debug facility
 /* Features not ready for prime time and that may *DESTROY* your data - USE AT YOUR OWN RISKS! */
-#define RUFUS_TEST
+// #define RUFUS_TEST
 /* Languages for which translators are M.I.A. and that we could use help with */
 #define LOST_TRANSLATORS            { "ms-MY" }	// NB: locales MUST be <= 5 chars
 /* Probability of getting the M.I.A. translator message. For more on this, see LostTranslatorCheck() */
@@ -449,6 +449,7 @@ extern BOOL IsBootableImage(const char* path);
 extern BOOL AppendVHDFooter(const char* vhd_path);
 extern int IsHDD(DWORD DriveIndex, uint16_t vid, uint16_t pid, const char* strid);
 extern void LostTranslatorCheck(void);
+extern LONG ValidateSignature(HWND hDlg, const char* path);
 
 DWORD WINAPI FormatThread(void* param);
 DWORD WINAPI SaveImageThread(void* param);

src/rufus.rc

@@ -32,7 +32,7 @@ LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL
 
 IDD_DIALOG DIALOGEX 12, 12, 242, 376
 STYLE DS_SETFONT | DS_MODALFRAME | DS_CENTER | WS_MINIMIZEBOX | WS_POPUP | WS_CAPTION | WS_SYSMENU
-CAPTION "Rufus 2.5.766"
+CAPTION "Rufus 2.5.767"
 FONT 8, "Segoe UI Symbol", 400, 0, 0x0
 BEGIN
     LTEXT           "Device",IDS_DEVICE_TXT,9,6,200,8
@@ -319,8 +319,8 @@ END
 //
 
 VS_VERSION_INFO VERSIONINFO
- FILEVERSION 2,5,766,0
+ FILEVERSION 2,5,767,0
- PRODUCTVERSION 2,5,766,0
+ PRODUCTVERSION 2,5,767,0
  FILEFLAGSMASK 0x3fL
 #ifdef _DEBUG
  FILEFLAGS 0x1L
@@ -337,13 +337,13 @@ BEGIN
         BEGIN
             VALUE "CompanyName", "Akeo Consulting (http://akeo.ie)"
             VALUE "FileDescription", "Rufus"
-            VALUE "FileVersion", "2.5.766"
+            VALUE "FileVersion", "2.5.767"
             VALUE "InternalName", "Rufus"
             VALUE "LegalCopyright", "© 2011-2015 Pete Batard (GPL v3)"
             VALUE "LegalTrademarks", "http://www.gnu.org/copyleft/gpl.html"
             VALUE "OriginalFilename", "rufus.exe"
             VALUE "ProductName", "Rufus"
-            VALUE "ProductVersion", "2.5.766"
+            VALUE "ProductVersion", "2.5.767"
         END
     END
     BLOCK "VarFileInfo"

src/stdlg.c

@@ -1435,11 +1435,11 @@ INT_PTR CALLBACK update_subclass_callback(HWND hDlg, UINT message, WPARAM wParam
  */
 INT_PTR CALLBACK NewVersionCallback(HWND hDlg, UINT message, WPARAM wParam, LPARAM lParam)
 {
-	int i;
-	HWND hNotes;
 	char cmdline[] = APPLICATION_NAME " -w 150";
 	static char* filepath = NULL;
 	static int download_status = 0;
+	LONG i;
+	HWND hNotes;
 	STARTUPINFOA si;
 	PROCESS_INFORMATION pi;
 	HFONT hyperlink_font = NULL;
@@ -1497,12 +1497,15 @@ INT_PTR CALLBACK NewVersionCallback(HWND hDlg, UINT message, WPARAM wParam, LPAR
 				break;
 			case 2:		// Launch newer version and close this one
 				Sleep(1000);	// Add a delay on account of antivirus scanners
+
+				if (ValidateSignature(hDlg, filepath) != NO_ERROR)
+					break;
+
 				memset(&si, 0, sizeof(si));
 				memset(&pi, 0, sizeof(pi));
 				si.cb = sizeof(si);
 				if (!CreateProcessU(filepath, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi)) {
 					PrintInfo(0, MSG_214);
-					// TODO: produce a message box and add a retry, as the file may be scanned by the Antivirus
 					uprintf("Failed to launch new application: %s\n", WindowsErrorString());
 				} else {
 					PrintInfo(0, MSG_213);