@joebigelow / wix / commits / 4921664f

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.

Sean Hall committed Nov 1, 2020 at 15:16 UTC 4921664f92bb6bd39ed7fd3dd24d58108973af25
9 files changed +27 -376
src/WixToolset.BootstrapperCore.Native/inc/BootstrapperEngine.h
-4
@@ -218,7 +218,6 @@ typedef struct _BAENGINE_FORMATSTRING_ARGS
218 typedef struct _BAENGINE_FORMATSTRING_RESULTS
219 {
220 DWORD cbSize;
221 - // The contents of wzOut may be sensitive, should keep encrypted and SecureZeroFree.
221 LPWSTR wzOut;
222 // Should be initialized to the size of wzOut.
223 DWORD cchOut;
@@ -244,7 +243,6 @@ typedef struct _BAENGINE_GETVARIABLENUMERIC_ARGS
243 typedef struct _BAENGINE_GETVARIABLENUMERIC_RESULTS
244 {
245 DWORD cbSize;
247 - // The contents of llValue may be sensitive, if variable is hidden should keep value encrypted and SecureZeroMemory.
246 LONGLONG llValue;
247 } BAENGINE_GETVARIABLENUMERIC_RESULTS;
248
@@ -257,7 +255,6 @@ typedef struct _BAENGINE_GETVARIABLESTRING_ARGS
255 typedef struct _BAENGINE_GETVARIABLESTRING_RESULTS
256 {
257 DWORD cbSize;
260 - // The contents of wzValue may be sensitive, if variable is hidden should keep value encrypted and SecureZeroFree.
258 LPWSTR wzValue;
259 // Should be initialized to the size of wzValue.
260 DWORD cchValue;
@@ -272,7 +269,6 @@ typedef struct _BAENGINE_GETVARIABLEVERSION_ARGS
269 typedef struct _BAENGINE_GETVARIABLEVERSION_RESULTS
270 {
271 DWORD cbSize;
275 - // The contents of wzValue may be sensitive, if variable is hidden should keep value encrypted and SecureZeroFree.
272 LPWSTR wzValue;
273 // Should be initialized to the size of wzValue.
274 DWORD cchValue;
src/WixToolset.BootstrapperCore.Native/inc/BundleExtensionEngine.h
-4
@@ -78,7 +78,6 @@ typedef struct _BUNDLE_EXTENSION_ENGINE_FORMATSTRING_ARGS
78 typedef struct _BUNDLE_EXTENSION_ENGINE_FORMATSTRING_RESULTS
79 {
80 DWORD cbSize;
81 - // The contents of wzOut may be sensitive, should keep encrypted and SecureZeroFree.
81 LPWSTR wzOut;
82 // Should be initialized to the size of wzOut.
83 DWORD cchOut;
@@ -93,7 +92,6 @@ typedef struct _BUNDLE_EXTENSION_ENGINE_GETVARIABLENUMERIC_ARGS
92 typedef struct _BUNDLE_EXTENSION_ENGINE_GETVARIABLENUMERIC_RESULTS
93 {
94 DWORD cbSize;
96 - // The contents of llValue may be sensitive, if variable is hidden should keep value encrypted and SecureZeroMemory.
95 LONGLONG llValue;
96 } BUNDLE_EXTENSION_ENGINE_GETVARIABLENUMERIC_RESULTS;
97
@@ -106,7 +104,6 @@ typedef struct _BUNDLE_EXTENSION_ENGINE_GETVARIABLESTRING_ARGS
104 typedef struct _BUNDLE_EXTENSION_ENGINE_GETVARIABLESTRING_RESULTS
105 {
106 DWORD cbSize;
109 - // The contents of wzValue may be sensitive, if variable is hidden should keep value encrypted and SecureZeroFree.
107 LPWSTR wzValue;
108 // Should be initialized to the size of wzValue.
109 DWORD cchValue;
@@ -121,7 +118,6 @@ typedef struct _BUNDLE_EXTENSION_ENGINE_GETVARIABLEVERSION_ARGS
118 typedef struct _BUNDLE_EXTENSION_ENGINE_GETVARIABLEVERSION_RESULTS
119 {
120 DWORD cbSize;
124 - // The contents of wzValue may be sensitive, if variable is hidden should keep value encrypted and SecureZeroFree.
121 LPWSTR wzValue;
122 // Should be initialized to the size of wzValue.
123 DWORD cchValue;
src/engine/condition.cpp
-2
@@ -434,7 +434,6 @@ static HRESULT ParseOperand(
434 HRESULT hr = S_OK;
435 LPWSTR sczFormatted = NULL;
436
437 - // Symbols don't encrypt their value, so can access the value directly.
437 switch (pContext->NextSymbol.Type)
438 {
439 case BURN_SYMBOL_TYPE_IDENTIFIER:
@@ -715,7 +714,6 @@ static HRESULT NextSymbol(
714 pContext->wzRead[n] == L'-' ||
715 pContext->wzRead[n] == L'.');
716
718 - // Symbols don't encrypt their value, so can access the value directly.
717 hr = VerParseVersion(&pContext->wzRead[1], n - 1, FALSE, &pContext->NextSymbol.Value.pValue);
718 if (FAILED(hr))
719 {
src/engine/msiengine.cpp
-1
@@ -1402,7 +1402,6 @@ LExit:
1402 return hr;
1403 }
1404
1405 -// The contents of psczProperties may be sensitive, should keep encrypted and SecureZeroFree.
1405 extern "C" HRESULT MsiEngineConcatProperties(
1406 __in_ecount(cProperties) BURN_MSIPROPERTY* rgProperties,
1407 __in DWORD cProperties,
src/engine/search.cpp
-1
@@ -1136,7 +1136,6 @@ static HRESULT MsiProductSearch(
1136 DWORD dwRelatedProducts = 0;
1137 BURN_VARIANT_TYPE type = BURN_VARIANT_TYPE_NONE;
1138 BURN_VARIANT value = { };
1139 - // We're not going to encrypt this value, so can access the value directly.
1139
1140 switch (pSearch->MsiProductSearch.Type)
1141 {
src/engine/variable.cpp
-14
@@ -427,9 +427,6 @@ extern "C" HRESULT VariablesParseFromXml(
427 hr = BVariantSetValue(&pVariables->rgVariables[iVariable].Value, &value);
428 ExitOnFailure(hr, "Failed to set value of variable: %ls", sczId);
429
430 - hr = BVariantSetEncryption(&pVariables->rgVariables[iVariable].Value, fHidden);
431 - ExitOnFailure(hr, "Failed to set variant encryption");
432 -
430 // prepare next iteration
431 ReleaseNullObject(pixnNode);
432 BVariantUninitialize(&value);
@@ -509,7 +506,6 @@ extern "C" void VariablesDump(
506 StrSecureZeroFreeString(sczValue);
507 }
508
512 -// The contents of pllValue may be sensitive, if variable is hidden should keep value encrypted and SecureZeroMemory.
509 extern "C" HRESULT VariableGetNumeric(
510 __in BURN_VARIABLES* pVariables,
511 __in_z LPCWSTR wzVariable,
@@ -541,7 +537,6 @@ LExit:
537 return hr;
538 }
539
544 -// The contents of psczValue may be sensitive, if variable is hidden should keep encrypted and SecureZeroFree.
540 extern "C" HRESULT VariableGetString(
541 __in BURN_VARIABLES* pVariables,
542 __in_z LPCWSTR wzVariable,
@@ -573,7 +568,6 @@ LExit:
568 return hr;
569 }
570
576 -// The contents of ppValue may be sensitive, if variable is hidden should keep value encrypted and SecureZeroMemory.
571 extern "C" HRESULT VariableGetVersion(
572 __in BURN_VARIABLES* pVariables,
573 __in_z LPCWSTR wzVariable,
@@ -632,7 +626,6 @@ LExit:
626 return hr;
627 }
628
635 -// The contents of psczValue may be sensitive, should keep encrypted and SecureZeroFree.
629 extern "C" HRESULT VariableGetFormatted(
630 __in BURN_VARIABLES* pVariables,
631 __in_z LPCWSTR wzVariable,
@@ -661,7 +654,6 @@ extern "C" HRESULT VariableSetNumeric(
654 {
655 BURN_VARIANT variant = { };
656
664 - // We're not going to encrypt this value, so can access the value directly.
657 variant.llValue = llValue;
658 variant.Type = BURN_VARIANT_TYPE_NUMERIC;
659
@@ -678,7 +670,6 @@ extern "C" HRESULT VariableSetString(
670 {
671 BURN_VARIANT variant = { };
672
681 - // We're not going to encrypt this value, so can access the value directly.
673 variant.sczValue = (LPWSTR)wzValue;
674 variant.Type = fFormatted ? BURN_VARIANT_TYPE_FORMATTED : BURN_VARIANT_TYPE_STRING;
675
@@ -694,7 +685,6 @@ extern "C" HRESULT VariableSetVersion(
685 {
686 BURN_VARIANT variant = { };
687
697 - // We're not going to encrypt this value, so can access the value directly.
688 variant.pValue = pValue;
689 variant.Type = BURN_VARIANT_TYPE_VERSION;
690
@@ -710,7 +700,6 @@ extern "C" HRESULT VariableSetVariant(
700 return SetVariableValue(pVariables, wzVariable, pVariant, SET_VARIABLE_NOT_BUILTIN, TRUE);
701 }
702
713 -// The contents of psczOut may be sensitive, should keep encrypted and SecureZeroFree
703 extern "C" HRESULT VariableFormatString(
704 __in BURN_VARIABLES* pVariables,
705 __in_z LPCWSTR wzIn,
@@ -1092,7 +1081,6 @@ LExit:
1081
1082 // internal function definitions
1083
1095 -// The contents of psczOut may be sensitive, should keep encrypted and SecureZeroFree.
1084 static HRESULT FormatString(
1085 __in BURN_VARIABLES* pVariables,
1086 __in_z LPCWSTR wzIn,
@@ -1312,7 +1300,6 @@ LExit:
1300 return hr;
1301 }
1302
1315 -// The contents of psczOut may be sensitive, should keep encrypted and SecureZeroFree.
1303 static HRESULT GetFormatted(
1304 __in BURN_VARIABLES* pVariables,
1305 __in_z LPCWSTR wzVariable,
@@ -1581,7 +1568,6 @@ static HRESULT SetVariableValue(
1568 }
1569 else
1570 {
1584 - // Assume value isn't encrypted since it's not hidden.
1571 switch (pVariant->Type)
1572 {
1573 case BURN_VARIANT_TYPE_NONE:
src/engine/variant.cpp
+22 -292
@@ -2,8 +2,6 @@
2
3 #include "precomp.h"
4
5 -#define VARIANT_ENCRYPTION_SCOPE CRYPTPROTECTMEMORY_SAME_PROCESS
6 -
5 // internal function declarations
6
7 static HRESULT GetVersionInternal(
@@ -12,25 +10,6 @@ static HRESULT GetVersionInternal(
10 __in BOOL fSilent,
11 __out VERUTIL_VERSION** ppValue
12 );
15 -static HRESULT BVariantEncryptString(
16 - __in BURN_VARIANT* pVariant,
17 - __in BOOL fEncrypt
18 - );
19 -
20 -static void BVariantRetrieveNumeric(
21 - __in BURN_VARIANT* pVariant,
22 - __out LONGLONG* pllValue
23 - );
24 -
25 -static HRESULT BVariantRetrieveDecryptedString(
26 - __in BURN_VARIANT* pVariant,
27 - __out LPWSTR* psczValue
28 - );
29 -
30 -static void BVariantRetrieveVersion(
31 - __in BURN_VARIANT* pVariant,
32 - __out VERUTIL_VERSION** ppValue
33 - );
13
14 // function definitions
15
@@ -46,38 +25,28 @@ extern "C" void BVariantUninitialize(
25 SecureZeroMemory(pVariant, sizeof(BURN_VARIANT));
26 }
27
49 -// The contents of pllValue may be sensitive, should keep encrypted and SecureZeroMemory.
28 extern "C" HRESULT BVariantGetNumeric(
29 __in BURN_VARIANT* pVariant,
30 __out LONGLONG* pllValue
31 )
32 {
33 HRESULT hr = S_OK;
56 - LPWSTR sczValue = NULL;
57 - VERUTIL_VERSION* pVersionValue = NULL;
34
35 switch (pVariant->Type)
36 {
37 case BURN_VARIANT_TYPE_NUMERIC:
62 - BVariantRetrieveNumeric(pVariant, pllValue);
38 + *pllValue = pVariant->llValue;
39 break;
40 case BURN_VARIANT_TYPE_FORMATTED: __fallthrough;
41 case BURN_VARIANT_TYPE_STRING:
66 - hr = BVariantRetrieveDecryptedString(pVariant, &sczValue);
67 - if (SUCCEEDED(hr))
42 + hr = StrStringToInt64(pVariant->sczValue, 0, pllValue);
43 + if (FAILED(hr))
44 {
69 - hr = StrStringToInt64(sczValue, 0, pllValue);
70 - if (FAILED(hr))
71 - {
72 - hr = DISP_E_TYPEMISMATCH;
73 - }
45 + hr = DISP_E_TYPEMISMATCH;
46 }
75 - StrSecureZeroFreeString(sczValue);
47 break;
48 case BURN_VARIANT_TYPE_VERSION:
78 - BVariantRetrieveVersion(pVariant, &pVersionValue);
79 -
80 - hr = StrStringToInt64(pVersionValue->sczVersion, 0, pllValue);
49 + hr = StrStringToInt64(pVariant->pValue ? pVariant->pValue->sczVersion : NULL, 0, pllValue);
50 if (FAILED(hr))
51 {
52 hr = DISP_E_TYPEMISMATCH;
@@ -91,35 +60,27 @@ extern "C" HRESULT BVariantGetNumeric(
60 return hr;
61 }
62
94 -// The contents of psczValue may be sensitive, should keep encrypted and SecureZeroFree.
63 extern "C" HRESULT BVariantGetString(
64 __in BURN_VARIANT* pVariant,
65 __out_z LPWSTR* psczValue
66 )
67 {
68 HRESULT hr = S_OK;
101 - LONGLONG llValue = 0;
102 - VERUTIL_VERSION* pVersionValue = NULL;
69
70 switch (pVariant->Type)
71 {
72 case BURN_VARIANT_TYPE_NUMERIC:
107 - BVariantRetrieveNumeric(pVariant, &llValue);
108 - if (SUCCEEDED(hr))
109 - {
110 - hr = StrAllocFormattedSecure(psczValue, L"%I64d", llValue);
111 - ExitOnFailure(hr, "Failed to convert int64 to string.");
112 - }
113 - SecureZeroMemory(&llValue, sizeof(llValue));
73 + hr = StrAllocFormattedSecure(psczValue, L"%I64d", pVariant->llValue);
74 + ExitOnFailure(hr, "Failed to convert int64 to string.");
75 break;
76 case BURN_VARIANT_TYPE_FORMATTED: __fallthrough;
77 case BURN_VARIANT_TYPE_STRING:
117 - hr = BVariantRetrieveDecryptedString(pVariant, psczValue);
78 + hr = StrAllocStringSecure(psczValue, pVariant->sczValue, 0);
79 + ExitOnFailure(hr, "Failed to copy string value.");
80 break;
81 case BURN_VARIANT_TYPE_VERSION:
120 - BVariantRetrieveVersion(pVariant, &pVersionValue);
121 -
122 - hr = StrAllocStringSecure(psczValue, pVersionValue->sczVersion, 0);
82 + hr = StrAllocStringSecure(psczValue, pVariant->pValue ? pVariant->pValue->sczVersion : NULL, 0);
83 + ExitOnFailure(hr, "Failed to copy version value.");
84 break;
85 default:
86 hr = E_INVALIDARG;
@@ -130,7 +91,6 @@ LExit:
91 return hr;
92 }
93
133 -// The contents of ppValue may be sensitive, should keep encrypted and SecureZeroMemory.
94 extern "C" HRESULT BVariantGetVersion(
95 __in BURN_VARIANT* pVariant,
96 __out VERUTIL_VERSION** ppValue
@@ -139,7 +99,6 @@ extern "C" HRESULT BVariantGetVersion(
99 return GetVersionInternal(pVariant, FALSE, FALSE, ppValue);
100 }
101
142 -// The contents of ppValue may be sensitive, should keep encrypted and SecureZeroMemory.
102 extern "C" HRESULT BVariantGetVersionHidden(
103 __in BURN_VARIANT* pVariant,
104 __in BOOL fHidden,
@@ -149,7 +108,6 @@ extern "C" HRESULT BVariantGetVersionHidden(
108 return GetVersionInternal(pVariant, fHidden, FALSE, ppValue);
109 }
110
152 -// The contents of ppValue may be sensitive, should keep encrypted and SecureZeroMemory.
111 extern "C" HRESULT BVariantGetVersionSilent(
112 __in BURN_VARIANT* pVariant,
113 __in BOOL fSilent,
@@ -167,44 +125,28 @@ static HRESULT GetVersionInternal(
125 )
126 {
127 HRESULT hr = S_OK;
170 - LONGLONG llValue = 0;
171 - LPWSTR sczValue = NULL;
172 - VERUTIL_VERSION* pValue = NULL;
128
129 switch (pVariant->Type)
130 {
131 case BURN_VARIANT_TYPE_NUMERIC:
177 - BVariantRetrieveNumeric(pVariant, &llValue);
178 -
179 - hr = VerVersionFromQword(llValue, ppValue);
132 + hr = VerVersionFromQword(pVariant->llValue, ppValue);
133 break;
134 case BURN_VARIANT_TYPE_FORMATTED: __fallthrough;
135 case BURN_VARIANT_TYPE_STRING:
183 - hr = BVariantRetrieveDecryptedString(pVariant, &sczValue);
184 - if (SUCCEEDED(hr))
136 + hr = VerParseVersion(pVariant->sczValue, 0, FALSE, ppValue);
137 + if (SUCCEEDED(hr) && !fSilent && (*ppValue)->fInvalid)
138 {
186 - hr = VerParseVersion(sczValue, 0, FALSE, ppValue);
187 - if (FAILED(hr))
188 - {
189 - hr = DISP_E_TYPEMISMATCH;
190 - }
191 - else if (!fSilent && (*ppValue)->fInvalid)
192 - {
193 - LogId(REPORT_WARNING, MSG_INVALID_VERSION_COERSION, fHidden ? L"*****" : sczValue);
194 - }
139 + LogId(REPORT_WARNING, MSG_INVALID_VERSION_COERSION, fHidden ? L"*****" : pVariant->sczValue);
140 }
196 - StrSecureZeroFreeString(sczValue);
141 break;
142 case BURN_VARIANT_TYPE_VERSION:
199 - BVariantRetrieveVersion(pVariant, &pValue);
200 -
201 - if (!pValue)
143 + if (!pVariant->pValue)
144 {
145 *ppValue = NULL;
146 }
147 else
148 {
207 - hr = VerCopyVersion(pValue, ppValue);
149 + hr = VerCopyVersion(pVariant->pValue, ppValue);
150 }
151 break;
152 default:
@@ -221,7 +163,6 @@ extern "C" HRESULT BVariantSetNumeric(
163 )
164 {
165 HRESULT hr = S_OK;
224 - BOOL fEncrypt = pVariant->fEncryptString;
166
167 if (BURN_VARIANT_TYPE_FORMATTED == pVariant->Type ||
168 BURN_VARIANT_TYPE_STRING == pVariant->Type)
@@ -231,7 +172,6 @@ extern "C" HRESULT BVariantSetNumeric(
172 memset(pVariant, 0, sizeof(BURN_VARIANT));
173 pVariant->llValue = llValue;
174 pVariant->Type = BURN_VARIANT_TYPE_NUMERIC;
234 - BVariantSetEncryption(pVariant, fEncrypt);
175
176 return hr;
177 }
@@ -244,7 +184,6 @@ extern "C" HRESULT BVariantSetString(
184 )
185 {
186 HRESULT hr = S_OK;
247 - BOOL fEncrypt = pVariant->fEncryptString;
187
188 if (!wzValue) // if we're nulling out the string, make the variable NONE.
189 {
@@ -257,11 +196,6 @@ extern "C" HRESULT BVariantSetString(
196 {
197 memset(pVariant, 0, sizeof(BURN_VARIANT));
198 }
260 - else
261 - {
262 - // We're about to copy an unencrypted value.
263 - pVariant->fEncryptString = FALSE;
264 - }
199
200 hr = StrAllocStringSecure(&pVariant->sczValue, wzValue, cchValue);
201 ExitOnFailure(hr, "Failed to copy string.");
@@ -270,7 +204,6 @@ extern "C" HRESULT BVariantSetString(
204 }
205
206 LExit:
273 - BVariantSetEncryption(pVariant, fEncrypt);
207 return hr;
208 }
209
@@ -280,7 +213,6 @@ extern "C" HRESULT BVariantSetVersion(
213 )
214 {
215 HRESULT hr = S_OK;
283 - BOOL fEncryptValue = pVariant->fEncryptString;
216
217 if (!pValue) // if we're nulling out the version, make the variable NONE.
218 {
@@ -298,8 +230,6 @@ extern "C" HRESULT BVariantSetVersion(
230 pVariant->Type = BURN_VARIANT_TYPE_VERSION;
231 }
232
301 - BVariantSetEncryption(pVariant, fEncryptValue);
302 -
233 return hr;
234 }
235
@@ -309,10 +239,6 @@ extern "C" HRESULT BVariantSetValue(
239 )
240 {
241 HRESULT hr = S_OK;
312 - LONGLONG llValue = 0;
313 - LPWSTR sczValue = NULL;
314 - VERUTIL_VERSION* pVersionValue = NULL;
315 - BOOL fEncrypt = pVariant->fEncryptString;
242
243 switch (pValue->Type)
244 {
@@ -320,35 +246,19 @@ extern "C" HRESULT BVariantSetValue(
246 BVariantUninitialize(pVariant);
247 break;
248 case BURN_VARIANT_TYPE_NUMERIC:
323 - hr = BVariantGetNumeric(pValue, &llValue);
324 - if (SUCCEEDED(hr))
325 - {
326 - hr = BVariantSetNumeric(pVariant, llValue);
327 - }
328 - SecureZeroMemory(&llValue, sizeof(llValue));
249 + hr = BVariantSetNumeric(pVariant, pValue->llValue);
250 break;
251 case BURN_VARIANT_TYPE_FORMATTED: __fallthrough;
252 case BURN_VARIANT_TYPE_STRING:
332 - hr = BVariantGetString(pValue, &sczValue);
333 - if (SUCCEEDED(hr))
334 - {
335 - hr = BVariantSetString(pVariant, sczValue, 0, BURN_VARIANT_TYPE_FORMATTED == pValue->Type);
336 - }
337 - StrSecureZeroFreeString(sczValue);
253 + hr = BVariantSetString(pVariant, pValue->sczValue, 0, BURN_VARIANT_TYPE_FORMATTED == pValue->Type);
254 break;
255 case BURN_VARIANT_TYPE_VERSION:
340 - hr = BVariantGetVersionSilent(pValue, TRUE, &pVersionValue);
341 - if (SUCCEEDED(hr))
342 - {
343 - hr = BVariantSetVersion(pVariant, pVersionValue);
344 - }
256 + hr = BVariantSetVersion(pVariant, pValue->pValue);
257 break;
258 default:
259 hr = E_INVALIDARG;
260 }
349 - ExitOnFailure(hr, "Failed to copy variant.");
350 -
351 - hr = BVariantSetEncryption(pVariant, fEncrypt);
261 + ExitOnFailure(hr, "Failed to copy variant value.");
262
263 LExit:
264 return hr;
@@ -359,50 +269,7 @@ extern "C" HRESULT BVariantCopy(
269 __out BURN_VARIANT* pTarget
270 )
271 {
362 - HRESULT hr = S_OK;
363 - LONGLONG llValue = 0;
364 - LPWSTR sczValue = NULL;
365 - VERUTIL_VERSION* pVersionValue = 0;
366 -
367 - BVariantUninitialize(pTarget);
368 -
369 - switch (pSource->Type)
370 - {
371 - case BURN_VARIANT_TYPE_NONE:
372 - break;
373 - case BURN_VARIANT_TYPE_NUMERIC:
374 - hr = BVariantGetNumeric(pSource, &llValue);
375 - if (SUCCEEDED(hr))
376 - {
377 - hr = BVariantSetNumeric(pTarget, llValue);
378 - }
379 - SecureZeroMemory(&llValue, sizeof(llValue));
380 - break;
381 - case BURN_VARIANT_TYPE_FORMATTED: __fallthrough;
382 - case BURN_VARIANT_TYPE_STRING:
383 - hr = BVariantGetString(pSource, &sczValue);
384 - if (SUCCEEDED(hr))
385 - {
386 - hr = BVariantSetString(pTarget, sczValue, 0, BURN_VARIANT_TYPE_FORMATTED == pSource->Type);
387 - }
388 - StrSecureZeroFreeString(sczValue);
389 - break;
390 - case BURN_VARIANT_TYPE_VERSION:
391 - hr = BVariantGetVersionSilent(pSource, TRUE, &pVersionValue);
392 - if (SUCCEEDED(hr))
393 - {
394 - hr = BVariantSetVersion(pTarget, pVersionValue);
395 - }
396 - break;
397 - default:
398 - hr = E_INVALIDARG;
399 - }
400 - ExitOnFailure(hr, "Failed to copy variant.");
401 -
402 - hr = BVariantSetEncryption(pTarget, pSource->fEncryptString);
403 -
404 -LExit:
405 - return hr;
272 + return BVariantSetValue(pTarget, pSource);
273 }
274
275 extern "C" HRESULT BVariantChangeType(
@@ -412,7 +279,6 @@ extern "C" HRESULT BVariantChangeType(
279 {
280 HRESULT hr = S_OK;
281 BURN_VARIANT variant = { };
415 - BOOL fEncrypt = pVariant->fEncryptString;
282
283 if (pVariant->Type == type)
284 {
@@ -449,143 +315,7 @@ extern "C" HRESULT BVariantChangeType(
315 BVariantUninitialize(pVariant);
316 memcpy_s(pVariant, sizeof(BURN_VARIANT), &variant, sizeof(BURN_VARIANT));
317 SecureZeroMemory(&variant, sizeof(BURN_VARIANT));
452 - BVariantSetEncryption(pVariant, fEncrypt);
318
319 LExit:
320 return hr;
321 }
457 -
458 -extern "C" HRESULT BVariantSetEncryption(
459 - __in BURN_VARIANT* pVariant,
460 - __in BOOL fEncrypt
461 - )
462 -{
463 - HRESULT hr = S_OK;
464 -
465 - if (pVariant->fEncryptString == fEncrypt)
466 - {
467 - // The requested encryption state is already applied.
468 - ExitFunction();
469 - }
470 -
471 - switch (pVariant->Type)
472 - {
473 - case BURN_VARIANT_TYPE_NONE:
474 - case BURN_VARIANT_TYPE_NUMERIC:
475 - case BURN_VARIANT_TYPE_VERSION:
476 - hr = S_OK;
477 - break;
478 - case BURN_VARIANT_TYPE_FORMATTED: __fallthrough;
479 - case BURN_VARIANT_TYPE_STRING:
480 - hr = BVariantEncryptString(pVariant, fEncrypt);
481 - break;
482 - default:
483 - hr = E_INVALIDARG;
484 - }
485 - ExitOnFailure(hr, "Failed to set the variant's encryption state");
486 - pVariant->fEncryptString = fEncrypt;
487 -
488 -LExit:
489 - return hr;
490 -}
491 -
492 -static HRESULT BVariantEncryptString(
493 - __in BURN_VARIANT* pVariant,
494 - __in BOOL fEncrypt
495 - )
496 -{
497 - HRESULT hr = S_OK;
498 - SIZE_T cbData = 0;
499 -
500 - if (NULL == pVariant->sczValue)
501 - {
502 - ExitFunction();
503 - }
504 -
505 - cbData = MemSize(pVariant->sczValue);
506 - if (-1 == cbData)
507 - {
508 - hr = E_INVALIDARG;
509 - ExitOnFailure(hr, "Failed to get the size of the string");
510 - }
511 -
512 - DWORD remainder = fEncrypt ? cbData % CRYP_ENCRYPT_MEMORY_SIZE : 0;
513 - DWORD extraNeeded = 0 < remainder ? CRYP_ENCRYPT_MEMORY_SIZE - remainder : 0;
514 - if ((MAXDWORD - extraNeeded) < cbData)
515 - {
516 - hr = E_INVALIDDATA;
517 - ExitOnFailure(hr, "The string is too big: size %u", cbData);
518 - }
519 - else if (0 < extraNeeded)
520 - {
521 - cbData += extraNeeded;
522 - LPVOID pvNew = NULL;
523 - hr = MemReAllocSecure(static_cast<LPVOID>(pVariant->sczValue), cbData, TRUE, &pvNew);
524 - ExitOnFailure(hr, "Failed to resize the string so it could be encrypted");
525 - pVariant->sczValue = static_cast<LPWSTR>(pvNew);
526 - }
527 -
528 - if (fEncrypt)
529 - {
530 - hr = CrypEncryptMemory(pVariant->sczValue, static_cast<DWORD>(cbData), VARIANT_ENCRYPTION_SCOPE);
531 - }
532 - else
533 - {
534 - hr = CrypDecryptMemory(pVariant->sczValue, static_cast<DWORD>(cbData), VARIANT_ENCRYPTION_SCOPE);
535 - }
536 -
537 -LExit:
538 - return hr;
539 -}
540 -
541 -static void BVariantRetrieveNumeric(
542 - __in BURN_VARIANT* pVariant,
543 - __out LONGLONG* pllValue
544 - )
545 -{
546 - Assert(NULL != pllValue);
547 -
548 - *pllValue = pVariant->llValue;
549 -}
550 -
551 -// The contents of psczValue may be sensitive, should keep encrypted and SecureZeroFree.
552 -static HRESULT BVariantRetrieveDecryptedString(
553 - __in BURN_VARIANT* pVariant,
554 - __out LPWSTR* psczValue
555 - )
556 -{
557 - HRESULT hr = S_OK;
558 -
559 - if (!pVariant->sczValue)
560 - {
561 - *psczValue = NULL;
562 - ExitFunction();
563 - }
564 -
565 - if (pVariant->fEncryptString)
566 - {
567 - hr = BVariantEncryptString(pVariant, FALSE);
568 - ExitOnFailure(hr, "Failed to decrypt string");
569 - }
570 -
571 - hr = StrAllocStringSecure(psczValue, pVariant->sczValue, 0);
572 - ExitOnFailure(hr, "Failed to copy value.");
573 -
574 - if (pVariant->fEncryptString)
575 - {
576 - hr = BVariantEncryptString(pVariant, TRUE);
577 - }
578 -
579 -LExit:
580 - return hr;
581 -}
582 -
583 -static void BVariantRetrieveVersion(
584 - __in BURN_VARIANT* pVariant,
585 - __out VERUTIL_VERSION** ppValue
586 - )
587 -{
588 - Assert(ppValue);
589 -
590 - *ppValue = pVariant->pValue;
591 -}
src/engine/variant.h
-13
@@ -30,7 +30,6 @@ typedef struct _BURN_VARIANT
30 LPWSTR sczValue;
31 };
32 BURN_VARIANT_TYPE Type;
33 - BOOL fEncryptString;
33 } BURN_VARIANT;
34
35
@@ -79,7 +78,6 @@ HRESULT BVariantSetVersion(
78 BVariantSetValue - Convenience function that calls BVariantUninitialize,
79 BVariantSetNumeric, BVariantSetString, or
80 BVariantSetVersion based on the type of pValue.
82 - The encryption state of pVariant is preserved.
81 ********************************************************************/
82 HRESULT BVariantSetValue(
83 __in BURN_VARIANT* pVariant,
@@ -87,8 +85,6 @@ HRESULT BVariantSetValue(
85 );
86 /********************************************************************
87 BVariantCopy - creates a copy of pSource.
90 - The encryption state of pTarget is set to
91 - the encryption state of pSource.
88 ********************************************************************/
89 HRESULT BVariantCopy(
90 __in BURN_VARIANT* pSource,
@@ -98,15 +94,6 @@ HRESULT BVariantChangeType(
94 __in BURN_VARIANT* pVariant,
95 __in BURN_VARIANT_TYPE type
96 );
101 -/********************************************************************
102 -BVariantSetEncryption - sets the encryption state of pVariant.
103 - If the encryption state matches the requested
104 - state, this function does nothing.
105 -********************************************************************/
106 -HRESULT BVariantSetEncryption(
107 - __in BURN_VARIANT* pVariant,
108 - __in BOOL fEncrypt
109 - );
97
98 #if defined(__cplusplus)
99 }
src/test/BurnUnitTest/VariantTest.cpp
+5 -45
@@ -68,7 +68,7 @@ namespace Bootstrapper
68 }
69
70 private:
71 - void InitFormattedValue(BURN_VARIANT* pValue, LPWSTR wzValue, BOOL fHidden, LPCWSTR wz, BURN_VARIANT* pActualValue)
71 + void InitFormattedValue(BURN_VARIANT* pValue, LPWSTR wzValue, BOOL /*fHidden*/, LPCWSTR wz, BURN_VARIANT* pActualValue)
72 {
73 HRESULT hr = S_OK;
74 pValue->Type = BURN_VARIANT_TYPE_FORMATTED;
@@ -78,34 +78,18 @@ namespace Bootstrapper
78
79 hr = BVariantCopy(pValue, pActualValue);
80 NativeAssert::Succeeded(hr, "Failed to copy variant {0}", wz);
81 -
82 - if (fHidden)
83 - {
84 - hr = BVariantSetEncryption(pActualValue, TRUE);
85 - NativeAssert::Succeeded(hr, "Failed to encrypt variant {0}", wz);
86 -
87 - NativeAssert::True(pActualValue->fEncryptString);
88 - }
81 }
82
91 - void InitNoneValue(BURN_VARIANT* pValue, BOOL fHidden, LPCWSTR wz, BURN_VARIANT* pActualValue)
83 + void InitNoneValue(BURN_VARIANT* pValue, BOOL /*fHidden*/, LPCWSTR wz, BURN_VARIANT* pActualValue)
84 {
85 HRESULT hr = S_OK;
86 pValue->Type = BURN_VARIANT_TYPE_NONE;
87
88 hr = BVariantCopy(pValue, pActualValue);
89 NativeAssert::Succeeded(hr, "Failed to copy variant {0}", wz);
98 -
99 - if (fHidden)
100 - {
101 - hr = BVariantSetEncryption(pActualValue, TRUE);
102 - NativeAssert::Succeeded(hr, "Failed to encrypt variant {0}", wz);
103 -
104 - NativeAssert::True(pActualValue->fEncryptString);
105 - }
90 }
91
108 - void InitNumericValue(BURN_VARIANT* pValue, LONGLONG llValue, BOOL fHidden, LPCWSTR wz, BURN_VARIANT* pActualValue)
92 + void InitNumericValue(BURN_VARIANT* pValue, LONGLONG llValue, BOOL /*fHidden*/, LPCWSTR wz, BURN_VARIANT* pActualValue)
93 {
94 HRESULT hr = S_OK;
95 pValue->Type = BURN_VARIANT_TYPE_NUMERIC;
@@ -113,17 +97,9 @@ namespace Bootstrapper
97
98 hr = BVariantCopy(pValue, pActualValue);
99 NativeAssert::Succeeded(hr, "Failed to copy variant {0}", wz);
116 -
117 - if (fHidden)
118 - {
119 - hr = BVariantSetEncryption(pActualValue, TRUE);
120 - NativeAssert::Succeeded(hr, "Failed to encrypt variant {0}", wz);
121 -
122 - NativeAssert::True(pActualValue->fEncryptString);
123 - }
100 }
101
126 - void InitStringValue(BURN_VARIANT* pValue, LPWSTR wzValue, BOOL fHidden, LPCWSTR wz, BURN_VARIANT* pActualValue)
102 + void InitStringValue(BURN_VARIANT* pValue, LPWSTR wzValue, BOOL /*fHidden*/, LPCWSTR wz, BURN_VARIANT* pActualValue)
103 {
104 HRESULT hr = S_OK;
105 pValue->Type = BURN_VARIANT_TYPE_STRING;
@@ -133,17 +109,9 @@ namespace Bootstrapper
109
110 hr = BVariantCopy(pValue, pActualValue);
111 NativeAssert::Succeeded(hr, "Failed to copy variant {0}", wz);
136 -
137 - if (fHidden)
138 - {
139 - hr = BVariantSetEncryption(pActualValue, TRUE);
140 - NativeAssert::Succeeded(hr, "Failed to encrypt variant {0}", wz);
141 -
142 - NativeAssert::True(pActualValue->fEncryptString);
143 - }
112 }
113
146 - void InitVersionValue(BURN_VARIANT* pValue, LPCWSTR wzValue, BOOL fHidden, LPCWSTR wz, BURN_VARIANT* pActualValue)
114 + void InitVersionValue(BURN_VARIANT* pValue, LPCWSTR wzValue, BOOL /*fHidden*/, LPCWSTR wz, BURN_VARIANT* pActualValue)
115 {
116 HRESULT hr = S_OK;
117 VERUTIL_VERSION* pVersion = NULL;
@@ -159,14 +127,6 @@ namespace Bootstrapper
127
128 hr = BVariantCopy(pValue, pActualValue);
129 NativeAssert::Succeeded(hr, "Failed to copy variant {0}", wz);
162 -
163 - if (fHidden)
164 - {
165 - hr = BVariantSetEncryption(pActualValue, TRUE);
166 - NativeAssert::Succeeded(hr, "Failed to encrypt variant {0}", wz);
167 -
168 - NativeAssert::True(pActualValue->fEncryptString);
169 - }
130 }
131 finally
132 {