From 4921664f92bb6bd39ed7fd3dd24d58108973af25 Mon Sep 17 00:00:00 2001 From: Sean Hall Date: Sun, 1 Nov 2020 15:16:07 -0600 Subject: Stop encrypting hidden variables. They were persisted in plaintext, and always had to be decrypted and sent to a separate process to actually be used. --- .../inc/BootstrapperEngine.h | 4 - .../inc/BundleExtensionEngine.h | 4 - src/engine/condition.cpp | 2 - src/engine/msiengine.cpp | 1 - src/engine/search.cpp | 1 - src/engine/variable.cpp | 14 - src/engine/variant.cpp | 314 ++------------------- src/engine/variant.h | 13 - src/test/BurnUnitTest/VariantTest.cpp | 50 +--- 9 files changed, 27 insertions(+), 376 deletions(-) (limited to 'src') diff --git a/src/WixToolset.BootstrapperCore.Native/inc/BootstrapperEngine.h b/src/WixToolset.BootstrapperCore.Native/inc/BootstrapperEngine.h index 13229c29..98a9d77d 100644 --- a/src/WixToolset.BootstrapperCore.Native/inc/BootstrapperEngine.h +++ b/src/WixToolset.BootstrapperCore.Native/inc/BootstrapperEngine.h @@ -218,7 +218,6 @@ typedef struct _BAENGINE_FORMATSTRING_ARGS typedef struct _BAENGINE_FORMATSTRING_RESULTS { DWORD cbSize; - // The contents of wzOut may be sensitive, should keep encrypted and SecureZeroFree. LPWSTR wzOut; // Should be initialized to the size of wzOut. DWORD cchOut; @@ -244,7 +243,6 @@ typedef struct _BAENGINE_GETVARIABLENUMERIC_ARGS typedef struct _BAENGINE_GETVARIABLENUMERIC_RESULTS { DWORD cbSize; - // The contents of llValue may be sensitive, if variable is hidden should keep value encrypted and SecureZeroMemory. LONGLONG llValue; } BAENGINE_GETVARIABLENUMERIC_RESULTS; @@ -257,7 +255,6 @@ typedef struct _BAENGINE_GETVARIABLESTRING_ARGS typedef struct _BAENGINE_GETVARIABLESTRING_RESULTS { DWORD cbSize; - // The contents of wzValue may be sensitive, if variable is hidden should keep value encrypted and SecureZeroFree. LPWSTR wzValue; // Should be initialized to the size of wzValue. DWORD cchValue; @@ -272,7 +269,6 @@ typedef struct _BAENGINE_GETVARIABLEVERSION_ARGS typedef struct _BAENGINE_GETVARIABLEVERSION_RESULTS { DWORD cbSize; - // The contents of wzValue may be sensitive, if variable is hidden should keep value encrypted and SecureZeroFree. LPWSTR wzValue; // Should be initialized to the size of wzValue. DWORD cchValue; diff --git a/src/WixToolset.BootstrapperCore.Native/inc/BundleExtensionEngine.h b/src/WixToolset.BootstrapperCore.Native/inc/BundleExtensionEngine.h index becb5be3..003ff635 100644 --- a/src/WixToolset.BootstrapperCore.Native/inc/BundleExtensionEngine.h +++ b/src/WixToolset.BootstrapperCore.Native/inc/BundleExtensionEngine.h @@ -78,7 +78,6 @@ typedef struct _BUNDLE_EXTENSION_ENGINE_FORMATSTRING_ARGS typedef struct _BUNDLE_EXTENSION_ENGINE_FORMATSTRING_RESULTS { DWORD cbSize; - // The contents of wzOut may be sensitive, should keep encrypted and SecureZeroFree. LPWSTR wzOut; // Should be initialized to the size of wzOut. DWORD cchOut; @@ -93,7 +92,6 @@ typedef struct _BUNDLE_EXTENSION_ENGINE_GETVARIABLENUMERIC_ARGS typedef struct _BUNDLE_EXTENSION_ENGINE_GETVARIABLENUMERIC_RESULTS { DWORD cbSize; - // The contents of llValue may be sensitive, if variable is hidden should keep value encrypted and SecureZeroMemory. LONGLONG llValue; } BUNDLE_EXTENSION_ENGINE_GETVARIABLENUMERIC_RESULTS; @@ -106,7 +104,6 @@ typedef struct _BUNDLE_EXTENSION_ENGINE_GETVARIABLESTRING_ARGS typedef struct _BUNDLE_EXTENSION_ENGINE_GETVARIABLESTRING_RESULTS { DWORD cbSize; - // The contents of wzValue may be sensitive, if variable is hidden should keep value encrypted and SecureZeroFree. LPWSTR wzValue; // Should be initialized to the size of wzValue. DWORD cchValue; @@ -121,7 +118,6 @@ typedef struct _BUNDLE_EXTENSION_ENGINE_GETVARIABLEVERSION_ARGS typedef struct _BUNDLE_EXTENSION_ENGINE_GETVARIABLEVERSION_RESULTS { DWORD cbSize; - // The contents of wzValue may be sensitive, if variable is hidden should keep value encrypted and SecureZeroFree. LPWSTR wzValue; // Should be initialized to the size of wzValue. DWORD cchValue; diff --git a/src/engine/condition.cpp b/src/engine/condition.cpp index d6038b34..06591567 100644 --- a/src/engine/condition.cpp +++ b/src/engine/condition.cpp @@ -434,7 +434,6 @@ static HRESULT ParseOperand( HRESULT hr = S_OK; LPWSTR sczFormatted = NULL; - // Symbols don't encrypt their value, so can access the value directly. switch (pContext->NextSymbol.Type) { case BURN_SYMBOL_TYPE_IDENTIFIER: @@ -715,7 +714,6 @@ static HRESULT NextSymbol( pContext->wzRead[n] == L'-' || pContext->wzRead[n] == L'.'); - // Symbols don't encrypt their value, so can access the value directly. hr = VerParseVersion(&pContext->wzRead[1], n - 1, FALSE, &pContext->NextSymbol.Value.pValue); if (FAILED(hr)) { diff --git a/src/engine/msiengine.cpp b/src/engine/msiengine.cpp index 47211309..066734d0 100644 --- a/src/engine/msiengine.cpp +++ b/src/engine/msiengine.cpp @@ -1402,7 +1402,6 @@ LExit: return hr; } -// The contents of psczProperties may be sensitive, should keep encrypted and SecureZeroFree. extern "C" HRESULT MsiEngineConcatProperties( __in_ecount(cProperties) BURN_MSIPROPERTY* rgProperties, __in DWORD cProperties, diff --git a/src/engine/search.cpp b/src/engine/search.cpp index 1dbcf56b..065003c7 100644 --- a/src/engine/search.cpp +++ b/src/engine/search.cpp @@ -1136,7 +1136,6 @@ static HRESULT MsiProductSearch( DWORD dwRelatedProducts = 0; BURN_VARIANT_TYPE type = BURN_VARIANT_TYPE_NONE; BURN_VARIANT value = { }; - // We're not going to encrypt this value, so can access the value directly. switch (pSearch->MsiProductSearch.Type) { diff --git a/src/engine/variable.cpp b/src/engine/variable.cpp index 8b6b318b..4bf73a9b 100644 --- a/src/engine/variable.cpp +++ b/src/engine/variable.cpp @@ -427,9 +427,6 @@ extern "C" HRESULT VariablesParseFromXml( hr = BVariantSetValue(&pVariables->rgVariables[iVariable].Value, &value); ExitOnFailure(hr, "Failed to set value of variable: %ls", sczId); - hr = BVariantSetEncryption(&pVariables->rgVariables[iVariable].Value, fHidden); - ExitOnFailure(hr, "Failed to set variant encryption"); - // prepare next iteration ReleaseNullObject(pixnNode); BVariantUninitialize(&value); @@ -509,7 +506,6 @@ extern "C" void VariablesDump( StrSecureZeroFreeString(sczValue); } -// The contents of pllValue may be sensitive, if variable is hidden should keep value encrypted and SecureZeroMemory. extern "C" HRESULT VariableGetNumeric( __in BURN_VARIABLES* pVariables, __in_z LPCWSTR wzVariable, @@ -541,7 +537,6 @@ LExit: return hr; } -// The contents of psczValue may be sensitive, if variable is hidden should keep encrypted and SecureZeroFree. extern "C" HRESULT VariableGetString( __in BURN_VARIABLES* pVariables, __in_z LPCWSTR wzVariable, @@ -573,7 +568,6 @@ LExit: return hr; } -// The contents of ppValue may be sensitive, if variable is hidden should keep value encrypted and SecureZeroMemory. extern "C" HRESULT VariableGetVersion( __in BURN_VARIABLES* pVariables, __in_z LPCWSTR wzVariable, @@ -632,7 +626,6 @@ LExit: return hr; } -// The contents of psczValue may be sensitive, should keep encrypted and SecureZeroFree. extern "C" HRESULT VariableGetFormatted( __in BURN_VARIABLES* pVariables, __in_z LPCWSTR wzVariable, @@ -661,7 +654,6 @@ extern "C" HRESULT VariableSetNumeric( { BURN_VARIANT variant = { }; - // We're not going to encrypt this value, so can access the value directly. variant.llValue = llValue; variant.Type = BURN_VARIANT_TYPE_NUMERIC; @@ -678,7 +670,6 @@ extern "C" HRESULT VariableSetString( { BURN_VARIANT variant = { }; - // We're not going to encrypt this value, so can access the value directly. variant.sczValue = (LPWSTR)wzValue; variant.Type = fFormatted ? BURN_VARIANT_TYPE_FORMATTED : BURN_VARIANT_TYPE_STRING; @@ -694,7 +685,6 @@ extern "C" HRESULT VariableSetVersion( { BURN_VARIANT variant = { }; - // We're not going to encrypt this value, so can access the value directly. variant.pValue = pValue; variant.Type = BURN_VARIANT_TYPE_VERSION; @@ -710,7 +700,6 @@ extern "C" HRESULT VariableSetVariant( return SetVariableValue(pVariables, wzVariable, pVariant, SET_VARIABLE_NOT_BUILTIN, TRUE); } -// The contents of psczOut may be sensitive, should keep encrypted and SecureZeroFree extern "C" HRESULT VariableFormatString( __in BURN_VARIABLES* pVariables, __in_z LPCWSTR wzIn, @@ -1092,7 +1081,6 @@ LExit: // internal function definitions -// The contents of psczOut may be sensitive, should keep encrypted and SecureZeroFree. static HRESULT FormatString( __in BURN_VARIABLES* pVariables, __in_z LPCWSTR wzIn, @@ -1312,7 +1300,6 @@ LExit: return hr; } -// The contents of psczOut may be sensitive, should keep encrypted and SecureZeroFree. static HRESULT GetFormatted( __in BURN_VARIABLES* pVariables, __in_z LPCWSTR wzVariable, @@ -1581,7 +1568,6 @@ static HRESULT SetVariableValue( } else { - // Assume value isn't encrypted since it's not hidden. switch (pVariant->Type) { case BURN_VARIANT_TYPE_NONE: diff --git a/src/engine/variant.cpp b/src/engine/variant.cpp index 82f465b2..2267ee7b 100644 --- a/src/engine/variant.cpp +++ b/src/engine/variant.cpp @@ -2,8 +2,6 @@ #include "precomp.h" -#define VARIANT_ENCRYPTION_SCOPE CRYPTPROTECTMEMORY_SAME_PROCESS - // internal function declarations static HRESULT GetVersionInternal( @@ -12,25 +10,6 @@ static HRESULT GetVersionInternal( __in BOOL fSilent, __out VERUTIL_VERSION** ppValue ); -static HRESULT BVariantEncryptString( - __in BURN_VARIANT* pVariant, - __in BOOL fEncrypt - ); - -static void BVariantRetrieveNumeric( - __in BURN_VARIANT* pVariant, - __out LONGLONG* pllValue - ); - -static HRESULT BVariantRetrieveDecryptedString( - __in BURN_VARIANT* pVariant, - __out LPWSTR* psczValue - ); - -static void BVariantRetrieveVersion( - __in BURN_VARIANT* pVariant, - __out VERUTIL_VERSION** ppValue - ); // function definitions @@ -46,38 +25,28 @@ extern "C" void BVariantUninitialize( SecureZeroMemory(pVariant, sizeof(BURN_VARIANT)); } -// The contents of pllValue may be sensitive, should keep encrypted and SecureZeroMemory. extern "C" HRESULT BVariantGetNumeric( __in BURN_VARIANT* pVariant, __out LONGLONG* pllValue ) { HRESULT hr = S_OK; - LPWSTR sczValue = NULL; - VERUTIL_VERSION* pVersionValue = NULL; switch (pVariant->Type) { case BURN_VARIANT_TYPE_NUMERIC: - BVariantRetrieveNumeric(pVariant, pllValue); + *pllValue = pVariant->llValue; break; case BURN_VARIANT_TYPE_FORMATTED: __fallthrough; case BURN_VARIANT_TYPE_STRING: - hr = BVariantRetrieveDecryptedString(pVariant, &sczValue); - if (SUCCEEDED(hr)) + hr = StrStringToInt64(pVariant->sczValue, 0, pllValue); + if (FAILED(hr)) { - hr = StrStringToInt64(sczValue, 0, pllValue); - if (FAILED(hr)) - { - hr = DISP_E_TYPEMISMATCH; - } + hr = DISP_E_TYPEMISMATCH; } - StrSecureZeroFreeString(sczValue); break; case BURN_VARIANT_TYPE_VERSION: - BVariantRetrieveVersion(pVariant, &pVersionValue); - - hr = StrStringToInt64(pVersionValue->sczVersion, 0, pllValue); + hr = StrStringToInt64(pVariant->pValue ? pVariant->pValue->sczVersion : NULL, 0, pllValue); if (FAILED(hr)) { hr = DISP_E_TYPEMISMATCH; @@ -91,35 +60,27 @@ extern "C" HRESULT BVariantGetNumeric( return hr; } -// The contents of psczValue may be sensitive, should keep encrypted and SecureZeroFree. extern "C" HRESULT BVariantGetString( __in BURN_VARIANT* pVariant, __out_z LPWSTR* psczValue ) { HRESULT hr = S_OK; - LONGLONG llValue = 0; - VERUTIL_VERSION* pVersionValue = NULL; switch (pVariant->Type) { case BURN_VARIANT_TYPE_NUMERIC: - BVariantRetrieveNumeric(pVariant, &llValue); - if (SUCCEEDED(hr)) - { - hr = StrAllocFormattedSecure(psczValue, L"%I64d", llValue); - ExitOnFailure(hr, "Failed to convert int64 to string."); - } - SecureZeroMemory(&llValue, sizeof(llValue)); + hr = StrAllocFormattedSecure(psczValue, L"%I64d", pVariant->llValue); + ExitOnFailure(hr, "Failed to convert int64 to string."); break; case BURN_VARIANT_TYPE_FORMATTED: __fallthrough; case BURN_VARIANT_TYPE_STRING: - hr = BVariantRetrieveDecryptedString(pVariant, psczValue); + hr = StrAllocStringSecure(psczValue, pVariant->sczValue, 0); + ExitOnFailure(hr, "Failed to copy string value."); break; case BURN_VARIANT_TYPE_VERSION: - BVariantRetrieveVersion(pVariant, &pVersionValue); - - hr = StrAllocStringSecure(psczValue, pVersionValue->sczVersion, 0); + hr = StrAllocStringSecure(psczValue, pVariant->pValue ? pVariant->pValue->sczVersion : NULL, 0); + ExitOnFailure(hr, "Failed to copy version value."); break; default: hr = E_INVALIDARG; @@ -130,7 +91,6 @@ LExit: return hr; } -// The contents of ppValue may be sensitive, should keep encrypted and SecureZeroMemory. extern "C" HRESULT BVariantGetVersion( __in BURN_VARIANT* pVariant, __out VERUTIL_VERSION** ppValue @@ -139,7 +99,6 @@ extern "C" HRESULT BVariantGetVersion( return GetVersionInternal(pVariant, FALSE, FALSE, ppValue); } -// The contents of ppValue may be sensitive, should keep encrypted and SecureZeroMemory. extern "C" HRESULT BVariantGetVersionHidden( __in BURN_VARIANT* pVariant, __in BOOL fHidden, @@ -149,7 +108,6 @@ extern "C" HRESULT BVariantGetVersionHidden( return GetVersionInternal(pVariant, fHidden, FALSE, ppValue); } -// The contents of ppValue may be sensitive, should keep encrypted and SecureZeroMemory. extern "C" HRESULT BVariantGetVersionSilent( __in BURN_VARIANT* pVariant, __in BOOL fSilent, @@ -167,44 +125,28 @@ static HRESULT GetVersionInternal( ) { HRESULT hr = S_OK; - LONGLONG llValue = 0; - LPWSTR sczValue = NULL; - VERUTIL_VERSION* pValue = NULL; switch (pVariant->Type) { case BURN_VARIANT_TYPE_NUMERIC: - BVariantRetrieveNumeric(pVariant, &llValue); - - hr = VerVersionFromQword(llValue, ppValue); + hr = VerVersionFromQword(pVariant->llValue, ppValue); break; case BURN_VARIANT_TYPE_FORMATTED: __fallthrough; case BURN_VARIANT_TYPE_STRING: - hr = BVariantRetrieveDecryptedString(pVariant, &sczValue); - if (SUCCEEDED(hr)) + hr = VerParseVersion(pVariant->sczValue, 0, FALSE, ppValue); + if (SUCCEEDED(hr) && !fSilent && (*ppValue)->fInvalid) { - hr = VerParseVersion(sczValue, 0, FALSE, ppValue); - if (FAILED(hr)) - { - hr = DISP_E_TYPEMISMATCH; - } - else if (!fSilent && (*ppValue)->fInvalid) - { - LogId(REPORT_WARNING, MSG_INVALID_VERSION_COERSION, fHidden ? L"*****" : sczValue); - } + LogId(REPORT_WARNING, MSG_INVALID_VERSION_COERSION, fHidden ? L"*****" : pVariant->sczValue); } - StrSecureZeroFreeString(sczValue); break; case BURN_VARIANT_TYPE_VERSION: - BVariantRetrieveVersion(pVariant, &pValue); - - if (!pValue) + if (!pVariant->pValue) { *ppValue = NULL; } else { - hr = VerCopyVersion(pValue, ppValue); + hr = VerCopyVersion(pVariant->pValue, ppValue); } break; default: @@ -221,7 +163,6 @@ extern "C" HRESULT BVariantSetNumeric( ) { HRESULT hr = S_OK; - BOOL fEncrypt = pVariant->fEncryptString; if (BURN_VARIANT_TYPE_FORMATTED == pVariant->Type || BURN_VARIANT_TYPE_STRING == pVariant->Type) @@ -231,7 +172,6 @@ extern "C" HRESULT BVariantSetNumeric( memset(pVariant, 0, sizeof(BURN_VARIANT)); pVariant->llValue = llValue; pVariant->Type = BURN_VARIANT_TYPE_NUMERIC; - BVariantSetEncryption(pVariant, fEncrypt); return hr; } @@ -244,7 +184,6 @@ extern "C" HRESULT BVariantSetString( ) { HRESULT hr = S_OK; - BOOL fEncrypt = pVariant->fEncryptString; if (!wzValue) // if we're nulling out the string, make the variable NONE. { @@ -257,11 +196,6 @@ extern "C" HRESULT BVariantSetString( { memset(pVariant, 0, sizeof(BURN_VARIANT)); } - else - { - // We're about to copy an unencrypted value. - pVariant->fEncryptString = FALSE; - } hr = StrAllocStringSecure(&pVariant->sczValue, wzValue, cchValue); ExitOnFailure(hr, "Failed to copy string."); @@ -270,7 +204,6 @@ extern "C" HRESULT BVariantSetString( } LExit: - BVariantSetEncryption(pVariant, fEncrypt); return hr; } @@ -280,7 +213,6 @@ extern "C" HRESULT BVariantSetVersion( ) { HRESULT hr = S_OK; - BOOL fEncryptValue = pVariant->fEncryptString; if (!pValue) // if we're nulling out the version, make the variable NONE. { @@ -298,8 +230,6 @@ extern "C" HRESULT BVariantSetVersion( pVariant->Type = BURN_VARIANT_TYPE_VERSION; } - BVariantSetEncryption(pVariant, fEncryptValue); - return hr; } @@ -309,10 +239,6 @@ extern "C" HRESULT BVariantSetValue( ) { HRESULT hr = S_OK; - LONGLONG llValue = 0; - LPWSTR sczValue = NULL; - VERUTIL_VERSION* pVersionValue = NULL; - BOOL fEncrypt = pVariant->fEncryptString; switch (pValue->Type) { @@ -320,35 +246,19 @@ extern "C" HRESULT BVariantSetValue( BVariantUninitialize(pVariant); break; case BURN_VARIANT_TYPE_NUMERIC: - hr = BVariantGetNumeric(pValue, &llValue); - if (SUCCEEDED(hr)) - { - hr = BVariantSetNumeric(pVariant, llValue); - } - SecureZeroMemory(&llValue, sizeof(llValue)); + hr = BVariantSetNumeric(pVariant, pValue->llValue); break; case BURN_VARIANT_TYPE_FORMATTED: __fallthrough; case BURN_VARIANT_TYPE_STRING: - hr = BVariantGetString(pValue, &sczValue); - if (SUCCEEDED(hr)) - { - hr = BVariantSetString(pVariant, sczValue, 0, BURN_VARIANT_TYPE_FORMATTED == pValue->Type); - } - StrSecureZeroFreeString(sczValue); + hr = BVariantSetString(pVariant, pValue->sczValue, 0, BURN_VARIANT_TYPE_FORMATTED == pValue->Type); break; case BURN_VARIANT_TYPE_VERSION: - hr = BVariantGetVersionSilent(pValue, TRUE, &pVersionValue); - if (SUCCEEDED(hr)) - { - hr = BVariantSetVersion(pVariant, pVersionValue); - } + hr = BVariantSetVersion(pVariant, pValue->pValue); break; default: hr = E_INVALIDARG; } - ExitOnFailure(hr, "Failed to copy variant."); - - hr = BVariantSetEncryption(pVariant, fEncrypt); + ExitOnFailure(hr, "Failed to copy variant value."); LExit: return hr; @@ -359,50 +269,7 @@ extern "C" HRESULT BVariantCopy( __out BURN_VARIANT* pTarget ) { - HRESULT hr = S_OK; - LONGLONG llValue = 0; - LPWSTR sczValue = NULL; - VERUTIL_VERSION* pVersionValue = 0; - - BVariantUninitialize(pTarget); - - switch (pSource->Type) - { - case BURN_VARIANT_TYPE_NONE: - break; - case BURN_VARIANT_TYPE_NUMERIC: - hr = BVariantGetNumeric(pSource, &llValue); - if (SUCCEEDED(hr)) - { - hr = BVariantSetNumeric(pTarget, llValue); - } - SecureZeroMemory(&llValue, sizeof(llValue)); - break; - case BURN_VARIANT_TYPE_FORMATTED: __fallthrough; - case BURN_VARIANT_TYPE_STRING: - hr = BVariantGetString(pSource, &sczValue); - if (SUCCEEDED(hr)) - { - hr = BVariantSetString(pTarget, sczValue, 0, BURN_VARIANT_TYPE_FORMATTED == pSource->Type); - } - StrSecureZeroFreeString(sczValue); - break; - case BURN_VARIANT_TYPE_VERSION: - hr = BVariantGetVersionSilent(pSource, TRUE, &pVersionValue); - if (SUCCEEDED(hr)) - { - hr = BVariantSetVersion(pTarget, pVersionValue); - } - break; - default: - hr = E_INVALIDARG; - } - ExitOnFailure(hr, "Failed to copy variant."); - - hr = BVariantSetEncryption(pTarget, pSource->fEncryptString); - -LExit: - return hr; + return BVariantSetValue(pTarget, pSource); } extern "C" HRESULT BVariantChangeType( @@ -412,7 +279,6 @@ extern "C" HRESULT BVariantChangeType( { HRESULT hr = S_OK; BURN_VARIANT variant = { }; - BOOL fEncrypt = pVariant->fEncryptString; if (pVariant->Type == type) { @@ -449,143 +315,7 @@ extern "C" HRESULT BVariantChangeType( BVariantUninitialize(pVariant); memcpy_s(pVariant, sizeof(BURN_VARIANT), &variant, sizeof(BURN_VARIANT)); SecureZeroMemory(&variant, sizeof(BURN_VARIANT)); - BVariantSetEncryption(pVariant, fEncrypt); LExit: return hr; } - -extern "C" HRESULT BVariantSetEncryption( - __in BURN_VARIANT* pVariant, - __in BOOL fEncrypt - ) -{ - HRESULT hr = S_OK; - - if (pVariant->fEncryptString == fEncrypt) - { - // The requested encryption state is already applied. - ExitFunction(); - } - - switch (pVariant->Type) - { - case BURN_VARIANT_TYPE_NONE: - case BURN_VARIANT_TYPE_NUMERIC: - case BURN_VARIANT_TYPE_VERSION: - hr = S_OK; - break; - case BURN_VARIANT_TYPE_FORMATTED: __fallthrough; - case BURN_VARIANT_TYPE_STRING: - hr = BVariantEncryptString(pVariant, fEncrypt); - break; - default: - hr = E_INVALIDARG; - } - ExitOnFailure(hr, "Failed to set the variant's encryption state"); - pVariant->fEncryptString = fEncrypt; - -LExit: - return hr; -} - -static HRESULT BVariantEncryptString( - __in BURN_VARIANT* pVariant, - __in BOOL fEncrypt - ) -{ - HRESULT hr = S_OK; - SIZE_T cbData = 0; - - if (NULL == pVariant->sczValue) - { - ExitFunction(); - } - - cbData = MemSize(pVariant->sczValue); - if (-1 == cbData) - { - hr = E_INVALIDARG; - ExitOnFailure(hr, "Failed to get the size of the string"); - } - - DWORD remainder = fEncrypt ? cbData % CRYP_ENCRYPT_MEMORY_SIZE : 0; - DWORD extraNeeded = 0 < remainder ? CRYP_ENCRYPT_MEMORY_SIZE - remainder : 0; - if ((MAXDWORD - extraNeeded) < cbData) - { - hr = E_INVALIDDATA; - ExitOnFailure(hr, "The string is too big: size %u", cbData); - } - else if (0 < extraNeeded) - { - cbData += extraNeeded; - LPVOID pvNew = NULL; - hr = MemReAllocSecure(static_cast(pVariant->sczValue), cbData, TRUE, &pvNew); - ExitOnFailure(hr, "Failed to resize the string so it could be encrypted"); - pVariant->sczValue = static_cast(pvNew); - } - - if (fEncrypt) - { - hr = CrypEncryptMemory(pVariant->sczValue, static_cast(cbData), VARIANT_ENCRYPTION_SCOPE); - } - else - { - hr = CrypDecryptMemory(pVariant->sczValue, static_cast(cbData), VARIANT_ENCRYPTION_SCOPE); - } - -LExit: - return hr; -} - -static void BVariantRetrieveNumeric( - __in BURN_VARIANT* pVariant, - __out LONGLONG* pllValue - ) -{ - Assert(NULL != pllValue); - - *pllValue = pVariant->llValue; -} - -// The contents of psczValue may be sensitive, should keep encrypted and SecureZeroFree. -static HRESULT BVariantRetrieveDecryptedString( - __in BURN_VARIANT* pVariant, - __out LPWSTR* psczValue - ) -{ - HRESULT hr = S_OK; - - if (!pVariant->sczValue) - { - *psczValue = NULL; - ExitFunction(); - } - - if (pVariant->fEncryptString) - { - hr = BVariantEncryptString(pVariant, FALSE); - ExitOnFailure(hr, "Failed to decrypt string"); - } - - hr = StrAllocStringSecure(psczValue, pVariant->sczValue, 0); - ExitOnFailure(hr, "Failed to copy value."); - - if (pVariant->fEncryptString) - { - hr = BVariantEncryptString(pVariant, TRUE); - } - -LExit: - return hr; -} - -static void BVariantRetrieveVersion( - __in BURN_VARIANT* pVariant, - __out VERUTIL_VERSION** ppValue - ) -{ - Assert(ppValue); - - *ppValue = pVariant->pValue; -} diff --git a/src/engine/variant.h b/src/engine/variant.h index 34d7a187..e460005b 100644 --- a/src/engine/variant.h +++ b/src/engine/variant.h @@ -30,7 +30,6 @@ typedef struct _BURN_VARIANT LPWSTR sczValue; }; BURN_VARIANT_TYPE Type; - BOOL fEncryptString; } BURN_VARIANT; @@ -79,7 +78,6 @@ HRESULT BVariantSetVersion( BVariantSetValue - Convenience function that calls BVariantUninitialize, BVariantSetNumeric, BVariantSetString, or BVariantSetVersion based on the type of pValue. - The encryption state of pVariant is preserved. ********************************************************************/ HRESULT BVariantSetValue( __in BURN_VARIANT* pVariant, @@ -87,8 +85,6 @@ HRESULT BVariantSetValue( ); /******************************************************************** BVariantCopy - creates a copy of pSource. - The encryption state of pTarget is set to - the encryption state of pSource. ********************************************************************/ HRESULT BVariantCopy( __in BURN_VARIANT* pSource, @@ -98,15 +94,6 @@ HRESULT BVariantChangeType( __in BURN_VARIANT* pVariant, __in BURN_VARIANT_TYPE type ); -/******************************************************************** -BVariantSetEncryption - sets the encryption state of pVariant. - If the encryption state matches the requested - state, this function does nothing. -********************************************************************/ -HRESULT BVariantSetEncryption( - __in BURN_VARIANT* pVariant, - __in BOOL fEncrypt - ); #if defined(__cplusplus) } diff --git a/src/test/BurnUnitTest/VariantTest.cpp b/src/test/BurnUnitTest/VariantTest.cpp index 34328f53..43899a2b 100644 --- a/src/test/BurnUnitTest/VariantTest.cpp +++ b/src/test/BurnUnitTest/VariantTest.cpp @@ -68,7 +68,7 @@ namespace Bootstrapper } private: - void InitFormattedValue(BURN_VARIANT* pValue, LPWSTR wzValue, BOOL fHidden, LPCWSTR wz, BURN_VARIANT* pActualValue) + void InitFormattedValue(BURN_VARIANT* pValue, LPWSTR wzValue, BOOL /*fHidden*/, LPCWSTR wz, BURN_VARIANT* pActualValue) { HRESULT hr = S_OK; pValue->Type = BURN_VARIANT_TYPE_FORMATTED; @@ -78,34 +78,18 @@ namespace Bootstrapper hr = BVariantCopy(pValue, pActualValue); NativeAssert::Succeeded(hr, "Failed to copy variant {0}", wz); - - if (fHidden) - { - hr = BVariantSetEncryption(pActualValue, TRUE); - NativeAssert::Succeeded(hr, "Failed to encrypt variant {0}", wz); - - NativeAssert::True(pActualValue->fEncryptString); - } } - void InitNoneValue(BURN_VARIANT* pValue, BOOL fHidden, LPCWSTR wz, BURN_VARIANT* pActualValue) + void InitNoneValue(BURN_VARIANT* pValue, BOOL /*fHidden*/, LPCWSTR wz, BURN_VARIANT* pActualValue) { HRESULT hr = S_OK; pValue->Type = BURN_VARIANT_TYPE_NONE; hr = BVariantCopy(pValue, pActualValue); NativeAssert::Succeeded(hr, "Failed to copy variant {0}", wz); - - if (fHidden) - { - hr = BVariantSetEncryption(pActualValue, TRUE); - NativeAssert::Succeeded(hr, "Failed to encrypt variant {0}", wz); - - NativeAssert::True(pActualValue->fEncryptString); - } } - void InitNumericValue(BURN_VARIANT* pValue, LONGLONG llValue, BOOL fHidden, LPCWSTR wz, BURN_VARIANT* pActualValue) + void InitNumericValue(BURN_VARIANT* pValue, LONGLONG llValue, BOOL /*fHidden*/, LPCWSTR wz, BURN_VARIANT* pActualValue) { HRESULT hr = S_OK; pValue->Type = BURN_VARIANT_TYPE_NUMERIC; @@ -113,17 +97,9 @@ namespace Bootstrapper hr = BVariantCopy(pValue, pActualValue); NativeAssert::Succeeded(hr, "Failed to copy variant {0}", wz); - - if (fHidden) - { - hr = BVariantSetEncryption(pActualValue, TRUE); - NativeAssert::Succeeded(hr, "Failed to encrypt variant {0}", wz); - - NativeAssert::True(pActualValue->fEncryptString); - } } - void InitStringValue(BURN_VARIANT* pValue, LPWSTR wzValue, BOOL fHidden, LPCWSTR wz, BURN_VARIANT* pActualValue) + void InitStringValue(BURN_VARIANT* pValue, LPWSTR wzValue, BOOL /*fHidden*/, LPCWSTR wz, BURN_VARIANT* pActualValue) { HRESULT hr = S_OK; pValue->Type = BURN_VARIANT_TYPE_STRING; @@ -133,17 +109,9 @@ namespace Bootstrapper hr = BVariantCopy(pValue, pActualValue); NativeAssert::Succeeded(hr, "Failed to copy variant {0}", wz); - - if (fHidden) - { - hr = BVariantSetEncryption(pActualValue, TRUE); - NativeAssert::Succeeded(hr, "Failed to encrypt variant {0}", wz); - - NativeAssert::True(pActualValue->fEncryptString); - } } - void InitVersionValue(BURN_VARIANT* pValue, LPCWSTR wzValue, BOOL fHidden, LPCWSTR wz, BURN_VARIANT* pActualValue) + void InitVersionValue(BURN_VARIANT* pValue, LPCWSTR wzValue, BOOL /*fHidden*/, LPCWSTR wz, BURN_VARIANT* pActualValue) { HRESULT hr = S_OK; VERUTIL_VERSION* pVersion = NULL; @@ -159,14 +127,6 @@ namespace Bootstrapper hr = BVariantCopy(pValue, pActualValue); NativeAssert::Succeeded(hr, "Failed to copy variant {0}", wz); - - if (fHidden) - { - hr = BVariantSetEncryption(pActualValue, TRUE); - NativeAssert::Succeeded(hr, "Failed to encrypt variant {0}", wz); - - NativeAssert::True(pActualValue->fEncryptString); - } } finally { -- cgit v1.2.3-55-g6feb