From 2f4287fdcee83b30e0f7f3ce548bcdff2ee85e1f Mon Sep 17 00:00:00 2001 From: Sean Hall Date: Mon, 3 May 2021 14:41:33 -0500 Subject: Bring back Burn's implementation of signature verification. partial #6447 --- src/burn/engine/cache.cpp | 131 ++++++++++++++++++++++++ src/burn/engine/payload.cpp | 24 +++++ src/burn/engine/payload.h | 5 + src/burn/engine/precomp.h | 1 + src/burn/stub/stub.vcxproj | 4 +- src/burn/test/BurnUnitTest/BurnUnitTest.vcxproj | 2 +- 6 files changed, 164 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/burn/engine/cache.cpp b/src/burn/engine/cache.cpp index 44267cbc..61416aed 100644 --- a/src/burn/engine/cache.cpp +++ b/src/burn/engine/cache.cpp @@ -17,6 +17,11 @@ static LPWSTR vsczDefaultUserPackageCache = NULL; static LPWSTR vsczDefaultMachinePackageCache = NULL; static LPWSTR vsczCurrentMachinePackageCache = NULL; +static HRESULT CacheVerifyPayloadSignature( + __in BURN_PAYLOAD* pPayload, + __in_z LPCWSTR wzUnverifiedPayloadPath, + __in HANDLE hFile + ); static HRESULT CalculateWorkingFolder( __in_z LPCWSTR wzBundleId, __deref_out_z LPWSTR* psczWorkingFolder @@ -131,6 +136,10 @@ static HRESULT VerifyHash( __in LPPROGRESS_ROUTINE pfnProgress, __in LPVOID pContext ); +static HRESULT VerifyPayloadAgainstCertChain( + __in BURN_PAYLOAD* pPayload, + __in PCCERT_CHAIN_CONTEXT pChainContext + ); static HRESULT SendCacheBeginMessage( __in PFN_BURNCACHEMESSAGEHANDLER pfnCacheMessageHandler, __in LPVOID pContext, @@ -1133,6 +1142,56 @@ LExit: return hr; } +static HRESULT CacheVerifyPayloadSignature( + __in BURN_PAYLOAD* pPayload, + __in_z LPCWSTR wzUnverifiedPayloadPath, + __in HANDLE hFile + ) +{ + HRESULT hr = S_OK; + LONG er = ERROR_SUCCESS; + + GUID guidAuthenticode = WINTRUST_ACTION_GENERIC_VERIFY_V2; + WINTRUST_FILE_INFO wfi = { }; + WINTRUST_DATA wtd = { }; + CRYPT_PROVIDER_DATA* pProviderData = NULL; + CRYPT_PROVIDER_SGNR* pSigner = NULL; + + // Verify the payload assuming online. + wfi.cbStruct = sizeof(wfi); + wfi.pcwszFilePath = wzUnverifiedPayloadPath; + wfi.hFile = hFile; + + wtd.cbStruct = sizeof(wtd); + wtd.dwUnionChoice = WTD_CHOICE_FILE; + wtd.pFile = &wfi; + wtd.dwStateAction = WTD_STATEACTION_VERIFY; + wtd.dwProvFlags = WTD_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT; + wtd.dwUIChoice = WTD_UI_NONE; + + er = ::WinVerifyTrust(static_cast(INVALID_HANDLE_VALUE), &guidAuthenticode, &wtd); + if (er) + { + // Verify the payload assuming offline. + wtd.dwProvFlags |= WTD_CACHE_ONLY_URL_RETRIEVAL; + + er = ::WinVerifyTrust(static_cast(INVALID_HANDLE_VALUE), &guidAuthenticode, &wtd); + ExitOnWin32Error(er, hr, "Failed authenticode verification of payload: %ls", wzUnverifiedPayloadPath); + } + + pProviderData = ::WTHelperProvDataFromStateData(wtd.hWVTStateData); + ExitOnNullWithLastError(pProviderData, hr, "Failed to get provider state from authenticode certificate."); + + pSigner = ::WTHelperGetProvSignerFromChain(pProviderData, 0, FALSE, 0); + ExitOnNullWithLastError(pSigner, hr, "Failed to get signer chain from authenticode certificate."); + + hr = VerifyPayloadAgainstCertChain(pPayload, pSigner->pChainContext); + ExitOnFailure(hr, "Failed to verify expected payload against actual certificate chain."); + +LExit: + return hr; +} + extern "C" void CacheCleanup( __in BOOL fPerMachine, __in_z LPCWSTR wzBundleId @@ -1563,6 +1622,10 @@ static HRESULT VerifyThenTransferPayload( switch (pPayload->verification) { + case BURN_PAYLOAD_VERIFICATION_AUTHENTICODE: + hr = CacheVerifyPayloadSignature(pPayload, wzUnverifiedPayloadPath, hFile); + ExitOnFailure(hr, "Failed to verify payload signature: %ls", wzCachedPath); + break; case BURN_PAYLOAD_VERIFICATION_HASH: hr = VerifyHash(pPayload->pbHash, pPayload->cbHash, pPayload->qwFileSize, TRUE, wzUnverifiedPayloadPath, hFile, BURN_CACHE_STEP_HASH, pfnCacheMessageHandler, pfnProgress, pContext); ExitOnFailure(hr, "Failed to verify payload hash: %ls", wzCachedPath); @@ -1705,6 +1768,10 @@ static HRESULT VerifyFileAgainstPayload( switch (pPayload->verification) { + case BURN_PAYLOAD_VERIFICATION_AUTHENTICODE: + hr = CacheVerifyPayloadSignature(pPayload, wzVerifyPath, hFile); + ExitOnFailure(hr, "Failed to verify signature of payload: %ls", pPayload->sczKey); + break; case BURN_PAYLOAD_VERIFICATION_HASH: fVerifyFileSize = TRUE; @@ -2144,6 +2211,70 @@ LExit: return hr; } +static HRESULT VerifyPayloadAgainstCertChain( + __in BURN_PAYLOAD* pPayload, + __in PCCERT_CHAIN_CONTEXT pChainContext + ) +{ + HRESULT hr = S_OK; + PCCERT_CONTEXT pChainElementCertContext = NULL; + + BYTE rgbPublicKeyIdentifier[SHA1_HASH_LEN] = { }; + DWORD cbPublicKeyIdentifier = sizeof(rgbPublicKeyIdentifier); + BYTE* pbThumbprint = NULL; + DWORD cbThumbprint = 0; + + // Walk up the chain looking for a certificate in the chain that matches our expected public key identifier + // and thumbprint (if a thumbprint was provided). + HRESULT hrChainVerification = E_NOTFOUND; // assume we won't find a match. + for (DWORD i = 0; i < pChainContext->rgpChain[0]->cElement; ++i) + { + pChainElementCertContext = pChainContext->rgpChain[0]->rgpElement[i]->pCertContext; + + // Get the certificate's public key identifier. + if (!::CryptHashPublicKeyInfo(NULL, CALG_SHA1, 0, X509_ASN_ENCODING, &pChainElementCertContext->pCertInfo->SubjectPublicKeyInfo, rgbPublicKeyIdentifier, &cbPublicKeyIdentifier)) + { + ExitWithLastError(hr, "Failed to get certificate public key identifier."); + } + + // Compare the certificate's public key identifier with the payload's public key identifier. If they + // match, we're one step closer to the a positive result. + if (pPayload->cbCertificateRootPublicKeyIdentifier == cbPublicKeyIdentifier && + 0 == memcmp(pPayload->pbCertificateRootPublicKeyIdentifier, rgbPublicKeyIdentifier, cbPublicKeyIdentifier)) + { + // If the payload specified a thumbprint for the certificate, verify it. + if (pPayload->pbCertificateRootThumbprint) + { + hr = CertReadProperty(pChainElementCertContext, CERT_SHA1_HASH_PROP_ID, &pbThumbprint, &cbThumbprint); + ExitOnFailure(hr, "Failed to read certificate thumbprint."); + + if (pPayload->cbCertificateRootThumbprint == cbThumbprint && + 0 == memcmp(pPayload->pbCertificateRootThumbprint, pbThumbprint, cbThumbprint)) + { + // If we got here, we found that our payload public key identifier and thumbprint + // matched an element in the certficate chain. + hrChainVerification = S_OK; + break; + } + + ReleaseNullMem(pbThumbprint); + } + else // no thumbprint match necessary so we're good to go. + { + hrChainVerification = S_OK; + break; + } + } + } + hr = hrChainVerification; + ExitOnFailure(hr, "Failed to find expected public key in certificate chain."); + +LExit: + ReleaseMem(pbThumbprint); + + return hr; +} + static HRESULT SendCacheBeginMessage( __in PFN_BURNCACHEMESSAGEHANDLER pfnCacheMessageHandler, __in LPVOID pContext, diff --git a/src/burn/engine/payload.cpp b/src/burn/engine/payload.cpp index 392a3dd4..84c32eec 100644 --- a/src/burn/engine/payload.cpp +++ b/src/burn/engine/payload.cpp @@ -132,6 +132,28 @@ extern "C" HRESULT PayloadsParseFromXml( fValidFileSize = TRUE; } + // @CertificateAuthorityKeyIdentifier + hr = XmlGetAttributeEx(pixnNode, L"CertificateRootPublicKeyIdentifier", &scz); + if (E_NOTFOUND != hr) + { + ExitOnFailure(hr, "Failed to get @CertificateRootPublicKeyIdentifier."); + + hr = StrAllocHexDecode(scz, &pPayload->pbCertificateRootPublicKeyIdentifier, &pPayload->cbCertificateRootPublicKeyIdentifier); + ExitOnFailure(hr, "Failed to hex decode @CertificateRootPublicKeyIdentifier."); + + pPayload->verification = BURN_PAYLOAD_VERIFICATION_AUTHENTICODE; + } + + // @CertificateThumbprint + hr = XmlGetAttributeEx(pixnNode, L"CertificateRootThumbprint", &scz); + if (E_NOTFOUND != hr) + { + ExitOnFailure(hr, "Failed to get @CertificateRootThumbprint."); + + hr = StrAllocHexDecode(scz, &pPayload->pbCertificateRootThumbprint, &pPayload->cbCertificateRootThumbprint); + ExitOnFailure(hr, "Failed to hex decode @CertificateRootThumbprint."); + } + // @Hash hr = XmlGetAttributeEx(pixnNode, L"Hash", &scz); if (E_NOTFOUND != hr) @@ -191,6 +213,8 @@ extern "C" void PayloadUninitialize( ReleaseStr(pPayload->sczKey); ReleaseStr(pPayload->sczFilePath); ReleaseMem(pPayload->pbHash); + ReleaseMem(pPayload->pbCertificateRootThumbprint); + ReleaseMem(pPayload->pbCertificateRootPublicKeyIdentifier); ReleaseStr(pPayload->sczSourcePath); ReleaseStr(pPayload->sczLocalFilePath); ReleaseStr(pPayload->downloadSource.sczUrl); diff --git a/src/burn/engine/payload.h b/src/burn/engine/payload.h index 6fc6de5e..c12fbe66 100644 --- a/src/burn/engine/payload.h +++ b/src/burn/engine/payload.h @@ -26,6 +26,7 @@ enum BURN_PAYLOAD_STATE enum BURN_PAYLOAD_VERIFICATION { BURN_PAYLOAD_VERIFICATION_NONE, + BURN_PAYLOAD_VERIFICATION_AUTHENTICODE, BURN_PAYLOAD_VERIFICATION_HASH, BURN_PAYLOAD_VERIFICATION_UPDATE_BUNDLE, }; @@ -41,6 +42,10 @@ typedef struct _BURN_PAYLOAD DWORD64 qwFileSize; LPWSTR sczFilePath; // file path relative to the execute location + BYTE* pbCertificateRootPublicKeyIdentifier; + DWORD cbCertificateRootPublicKeyIdentifier; + BYTE* pbCertificateRootThumbprint; + DWORD cbCertificateRootThumbprint; BYTE* pbHash; DWORD cbHash; BURN_PAYLOAD_VERIFICATION verification; diff --git a/src/burn/engine/precomp.h b/src/burn/engine/precomp.h index 11b594da..c1822381 100644 --- a/src/burn/engine/precomp.h +++ b/src/burn/engine/precomp.h @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include diff --git a/src/burn/stub/stub.vcxproj b/src/burn/stub/stub.vcxproj index b75f17f4..e7bdf6e3 100644 --- a/src/burn/stub/stub.vcxproj +++ b/src/burn/stub/stub.vcxproj @@ -56,14 +56,14 @@ $(ProjectDir)..\engine\inc - cabinet.lib;crypt32.lib;msi.lib;rpcrt4.lib;shlwapi.lib;wininet.lib;wuguid.lib;engine.res + cabinet.lib;crypt32.lib;msi.lib;rpcrt4.lib;shlwapi.lib;wininet.lib;wintrust.lib;wuguid.lib;engine.res true true - cabinet.dll;crypt32.dll;msi.dll;shlwapi.dll;version.dll;wininet.dll + cabinet.dll;crypt32.dll;msi.dll;shlwapi.dll;version.dll;wininet.dll;wintrust.dll diff --git a/src/burn/test/BurnUnitTest/BurnUnitTest.vcxproj b/src/burn/test/BurnUnitTest/BurnUnitTest.vcxproj index 7ee27258..85481821 100644 --- a/src/burn/test/BurnUnitTest/BurnUnitTest.vcxproj +++ b/src/burn/test/BurnUnitTest/BurnUnitTest.vcxproj @@ -40,7 +40,7 @@ $(ProjectAdditionalIncludeDirectories);..\..\engine;..\..\..\api\burn\WixToolset.BootstrapperCore.Native\inc;..\..\..\libs\dutil\WixToolset.Dutil\inc - cabinet.lib;crypt32.lib;msi.lib;rpcrt4.lib;shlwapi.lib;wininet.lib;$(RootBuildFolder)libs\$(Configuration)\$(PlatformToolset)\$(PlatformTarget)\dutil.lib + cabinet.lib;crypt32.lib;msi.lib;rpcrt4.lib;shlwapi.lib;wininet.lib;wintrust.lib;$(RootBuildFolder)libs\$(Configuration)\$(PlatformToolset)\$(PlatformTarget)\dutil.lib -- cgit v1.2.3-55-g6feb