Skip to content

Commit 8894320

Browse files
donhodonho
committed
Launch only program signed with WinGUp's certificate on plugin management
While using -clean or -unzip for plugin management, WinGUp retrieves its own signing certificate, and compare it with the program to be launched. If the certificates do not match, or one of bineries is not signed, the program is not launched. Close #101
1 parent a635ac6 commit 8894320

File tree

3 files changed

+91
-31
lines changed

3 files changed

+91
-31
lines changed

‎src/verifySignedfile.cpp‎

Lines changed: 43 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -62,22 +62,24 @@ void SecurityGuard::writeSecurityError(const std::wstring& prefix, const std::ws
6262
writeLog(expandedLogFileName.c_str(), prefix.c_str(), log2write.c_str());
6363
}
6464

65-
bool SecurityGuard::verifySignedBinary(const std::wstring& filepath)
65+
bool SecurityGuard::initFromSelfCertif()
6666
{
67-
wstring display_name;
68-
wstring key_id_hex;
69-
wstring subject;
70-
wstring authority_key_id_hex;
67+
wchar_t codeSigedBinPath[MAX_PATH]{};
68+
::GetModuleFileName(NULL, codeSigedBinPath, MAX_PATH);
69+
70+
return verifySignatureAndGetInfo(codeSigedBinPath, _signer_display_name, _signer_key_id, _signer_subject, _authority_key_id);
71+
}
7172

73+
bool SecurityGuard::verifySignatureAndGetInfo(const std::wstring& codeSigedBinPath, wstring& display_name, wstring& key_id_hex, wstring& subject, wstring& authority_key_id_hex)
74+
{
7275
//
7376
// Signature verification
7477
//
7578

7679
// Initialize the WINTRUST_FILE_INFO structure.
77-
LPCWSTR pwszfilepath = filepath.c_str();
7880
WINTRUST_FILE_INFO file_data = {};
7981
file_data.cbStruct = sizeof(WINTRUST_FILE_INFO);
80-
file_data.pcwszFilePath = pwszfilepath;
82+
file_data.pcwszFilePath = codeSigedBinPath.c_str();
8183

8284
// Initialise WinTrust data
8385
WINTRUST_DATA winTEXTrust_data = {};
@@ -121,13 +123,13 @@ bool SecurityGuard::verifySignedBinary(const std::wstring& filepath)
121123

122124
if (vtrust)
123125
{
124-
writeSecurityError(filepath.c_str(), L": chain of trust verification failed");
126+
writeSecurityError(codeSigedBinPath, L": chain of trust verification failed");
125127
return false;
126128
}
127129

128130
if (t2)
129131
{
130-
writeSecurityError(filepath.c_str(), L": error encountered while cleaning up after WinVerifyTrust");
132+
writeSecurityError(codeSigedBinPath, L": error encountered while cleaning up after WinVerifyTrust");
131133
return false;
132134
}
133135
}
@@ -143,14 +145,14 @@ bool SecurityGuard::verifySignedBinary(const std::wstring& filepath)
143145
bool status = true;
144146

145147
try {
146-
BOOL result = ::CryptQueryObject(CERT_QUERY_OBJECT_FILE, filepath.c_str(),
148+
BOOL result = ::CryptQueryObject(CERT_QUERY_OBJECT_FILE, codeSigedBinPath.c_str(),
147149
CERT_QUERY_CONTENT_FLAG_PKCS7_SIGNED_EMBED, CERT_QUERY_FORMAT_FLAG_BINARY, 0,
148150
&dwEncoding, &dwContentType, &dwFormatType,
149151
&hStore, &hMsg, NULL);
150152

151153
if (!result)
152154
{
153-
throw string("Checking certificate of ") + ws2s(filepath) + " : " + ws2s(GetLastErrorAsString(GetLastError()));
155+
throw string("Checking certificate of ") + ws2s(codeSigedBinPath) + " : " + ws2s(GetLastErrorAsString(GetLastError()));
154156
}
155157

156158
// Get signer information size.
@@ -235,13 +237,12 @@ bool SecurityGuard::verifySignedBinary(const std::wstring& filepath)
235237

236238

237239
// --- Retrieve Authority Key Identifier (AKI) ---
238-
239-
PCERT_EXTENSION pExtension = ::CertFindExtension(szOID_AUTHORITY_KEY_IDENTIFIER2, // OID for Authority Key Identifier (2.5.29.35)
240-
context->pCertInfo->cExtension, context->pCertInfo->rgExtension);
240+
// OID for Authority Key Identifier (2.5.29.35)
241+
PCERT_EXTENSION pExtension = ::CertFindExtension(szOID_AUTHORITY_KEY_IDENTIFIER2, context->pCertInfo->cExtension, context->pCertInfo->rgExtension);
241242

242243
if (!pExtension)
243-
pExtension = ::CertFindExtension(szOID_AUTHORITY_KEY_IDENTIFIER, // OID for Authority Key Identifier (2.5.29.1)
244-
context->pCertInfo->cExtension, context->pCertInfo->rgExtension);
244+
// OID for Authority Key Identifier (2.5.29.1)
245+
pExtension = ::CertFindExtension(szOID_AUTHORITY_KEY_IDENTIFIER, context->pCertInfo->cExtension, context->pCertInfo->rgExtension);
245246

246247
if (pExtension)
247248
{
@@ -278,15 +279,35 @@ bool SecurityGuard::verifySignedBinary(const std::wstring& filepath)
278279

279280
}
280281
catch (const string& s) {
281-
writeSecurityError((filepath + L" - error while getting certificate information: ").c_str(), s2ws(s).c_str());
282+
wstring msg = codeSigedBinPath;
283+
msg += L" - error while getting certificate information: ";
284+
writeSecurityError(msg, s2ws(s));
282285
status = false;
283286
}
284287
catch (...) {
285288
// Unknown error
286-
writeSecurityError(filepath.c_str(), L": Unknow error while getting certificate information");
289+
writeSecurityError(codeSigedBinPath, L": Unknow error while getting certificate information");
287290
status = false;
288291
}
289292

293+
// Clean up.
294+
295+
if (hStore != NULL) CertCloseStore(hStore, 0);
296+
if (hMsg != NULL) CryptMsgClose(hMsg);
297+
if (pSignerInfo != NULL) LocalFree(pSignerInfo);
298+
299+
return status;
300+
}
301+
302+
bool SecurityGuard::verifySignedBinary(const std::wstring& filepath)
303+
{
304+
wstring display_name;
305+
wstring key_id_hex;
306+
wstring subject;
307+
wstring authority_key_id_hex;
308+
309+
bool status = verifySignatureAndGetInfo(filepath, display_name, key_id_hex, subject, authority_key_id_hex);
310+
290311
//
291312
// fields verifications - if status is true, and demaded parameter string to compare (from the parameter) is not empty, then do compare
292313
//
@@ -298,7 +319,7 @@ bool SecurityGuard::verifySignedBinary(const std::wstring& filepath)
298319
errMsg += _signer_display_name;
299320
errMsg += L" vs unexpected ";
300321
errMsg += display_name;
301-
writeSecurityError(filepath.c_str(), errMsg);
322+
writeSecurityError(filepath, errMsg);
302323
}
303324

304325
if (status && (!_signer_subject.empty() && _signer_subject != subject))
@@ -309,7 +330,7 @@ bool SecurityGuard::verifySignedBinary(const std::wstring& filepath)
309330
errMsg += _signer_subject;
310331
errMsg += L" vs unexpected ";
311332
errMsg += subject;
312-
writeSecurityError(filepath.c_str(), errMsg);
333+
writeSecurityError(filepath, errMsg);
313334
}
314335

315336
if (status && (!_signer_key_id.empty() && stringToUpper(_signer_key_id) != key_id_hex))
@@ -320,7 +341,7 @@ bool SecurityGuard::verifySignedBinary(const std::wstring& filepath)
320341
errMsg += _signer_key_id;
321342
errMsg += L" vs unexpected ";
322343
errMsg += key_id_hex;
323-
writeSecurityError(filepath.c_str(), errMsg);
344+
writeSecurityError(filepath, errMsg);
324345
}
325346

326347
if (status && (!_authority_key_id.empty() && stringToUpper(_authority_key_id) != authority_key_id_hex))
@@ -331,14 +352,8 @@ bool SecurityGuard::verifySignedBinary(const std::wstring& filepath)
331352
errMsg += _authority_key_id;
332353
errMsg += L" vs unexpected ";
333354
errMsg += authority_key_id_hex;
334-
writeSecurityError(filepath.c_str(), errMsg);
355+
writeSecurityError(filepath, errMsg);
335356
}
336357

337-
// Clean up.
338-
339-
if (hStore != NULL) CertCloseStore(hStore, 0);
340-
if (hMsg != NULL) CryptMsgClose(hMsg);
341-
if (pSignerInfo != NULL) LocalFree(pSignerInfo);
342-
343358
return status;
344359
}

‎src/verifySignedfile.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,11 @@ class SecurityGuard final
5353
{
5454
public:
5555
SecurityGuard(){};
56-
bool verifySignedBinary(const std::wstring& filepath);
56+
bool initFromSelfCertif();
5757

58+
bool verifySignatureAndGetInfo(const std::wstring& codeSigedBinPath, std::wstring& display_name, std::wstring& key_id_hex, std::wstring& subject, std::wstring& authority_key_id_hex);
59+
bool verifySignedBinary(const std::wstring& filepath);
60+
5861
void enableChkRevoc() { _doCheckRevocation = true; }
5962
void enableChkTrustChain() { _doCheckChainOfTrust = true; }
6063
void setDisplayName(const std::wstring& signer_display_name) { _signer_display_name = signer_display_name; }

‎src/winmain.cpp‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1461,7 +1461,28 @@ int WINAPI wWinMain(HINSTANCE hInstance, HINSTANCE, PWSTR lpszCmdLine, int)
14611461
deleteFileOrFolder(destPath);
14621462
}
14631463

1464-
::ShellExecute(NULL, L"open", L"explorer.exe", prog2Launch.c_str(), prog2LaunchDir, SW_SHOWNORMAL);
1464+
1465+
1466+
#ifdef _DEBUG
1467+
// Don't check any thing in debug mode
1468+
#else
1469+
// Check signature of the launched program, with the same certif as gup.exe
1470+
SecurityGuard securityGuard4PluginsInstall;
1471+
1472+
if (!securityGuard4PluginsInstall.initFromSelfCertif())
1473+
{
1474+
securityGuard.writeSecurityError(L"Above certificate init error from \"gup -clean\" (remove plugins)", L"");
1475+
return -1;
1476+
}
1477+
1478+
bool isSecured = securityGuard4PluginsInstall.verifySignedBinary(prog2Launch.c_str());
1479+
if (!isSecured)
1480+
{
1481+
securityGuard.writeSecurityError(L"Above certificate verification error from \"gup -clean\" (remove plugins)", L"");
1482+
return -1;
1483+
}
1484+
#endif
1485+
::ShellExecute(NULL, L"open", prog2Launch.c_str(), NULL, prog2LaunchDir, SW_SHOWNORMAL);
14651486

14661487
return 0;
14671488
}
@@ -1480,12 +1501,32 @@ int WINAPI wWinMain(HINSTANCE hInstance, HINSTANCE, PWSTR lpszCmdLine, int)
14801501
WRITE_LOG(GUP_LOG_FILENAME, L"-1 in plugin updater's part - if (isCleanUp && isUnzip) // update: ", L"nbParam < 3");
14811502
return -1;
14821503
}
1504+
14831505
wstring prog2Launch = params[0];
14841506
wchar_t prog2LaunchDir[MAX_PATH];
14851507
lstrcpy(prog2LaunchDir, prog2Launch.c_str());
14861508
::PathRemoveFileSpec(prog2LaunchDir);
14871509
wstring destPathRoot = params[1];
14881510

1511+
#ifdef _DEBUG
1512+
// Don't check any thing in debug mode
1513+
#else
1514+
// Check signature of the launched program, with the same certif as gup.exe
1515+
SecurityGuard securityGuard4PluginsInstall;
1516+
1517+
if (!securityGuard4PluginsInstall.initFromSelfCertif())
1518+
{
1519+
securityGuard.writeSecurityError(L"Above certificate init error from \"gup -unzip\" (install or update plugins)", L"");
1520+
return -1;
1521+
}
1522+
1523+
bool isSecured = securityGuard4PluginsInstall.verifySignedBinary(prog2Launch.c_str());
1524+
if (!isSecured)
1525+
{
1526+
securityGuard.writeSecurityError(L"Above certificate verification error from \"gup -unzip\" (install or update plugins)", L"");
1527+
return -1;
1528+
}
1529+
#endif
14891530
for (size_t i = 2; i < nbParam; ++i)
14901531
{
14911532
wstring destPath = destPathRoot;
@@ -1593,7 +1634,8 @@ int WINAPI wWinMain(HINSTANCE hInstance, HINSTANCE, PWSTR lpszCmdLine, int)
15931634
}
15941635
}
15951636

1596-
::ShellExecute(NULL, L"open", L"explorer.exe", prog2Launch.c_str(), prog2LaunchDir, SW_SHOWNORMAL);
1637+
::ShellExecute(NULL, L"open", prog2Launch.c_str(), NULL, prog2LaunchDir, SW_SHOWNORMAL);
1638+
15971639
return 0;
15981640
}
15991641

0 commit comments

Comments
 (0)