Skip to content

Commit 86fda9f

Browse files
authored
Fix DynamicProcessor crash when working on individual units without a set (#5304)
Added unit test. Manually validated as well.
1 parent 2f344f0 commit 86fda9f

File tree

2 files changed

+67
-26
lines changed

2 files changed

+67
-26
lines changed

src/AppInstallerCLICore/ConfigurationDynamicRuntimeFactory.cpp

Lines changed: 37 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -163,42 +163,49 @@ namespace AppInstaller::CLI::ConfigurationRemoting
163163
m_dynamicFactory(std::move(dynamicFactory)), m_configurationSet(configurationSet)
164164
{
165165
#ifndef AICLI_DISABLE_TEST_HOOKS
166-
m_enableTestMode = GetConfigurationSetMetadataOverride(m_configurationSet, EnableTestModeTestGuid);
167-
m_enableRestrictedIntegrityLevel = GetConfigurationSetMetadataOverride(m_configurationSet, EnableRestrictedIntegrityLevelTestGuid);
168-
m_forceHighIntegrityLevelUnits = GetConfigurationSetMetadataOverride(m_configurationSet, ForceHighIntegrityLevelUnitsTestGuid);
166+
if (m_configurationSet)
167+
{
168+
m_enableTestMode = GetConfigurationSetMetadataOverride(m_configurationSet, EnableTestModeTestGuid);
169+
m_enableRestrictedIntegrityLevel = GetConfigurationSetMetadataOverride(m_configurationSet, EnableRestrictedIntegrityLevelTestGuid);
170+
m_forceHighIntegrityLevelUnits = GetConfigurationSetMetadataOverride(m_configurationSet, ForceHighIntegrityLevelUnitsTestGuid);
171+
}
169172

170173
m_currentIntegrityLevel = m_enableTestMode ? Security::IntegrityLevel::Medium : Security::GetEffectiveIntegrityLevel();
171174
#else
172175
m_currentIntegrityLevel = Security::GetEffectiveIntegrityLevel();
173176
#endif
174177

175178
m_setIntegrityLevel = m_currentIntegrityLevel;
176-
m_setIntegrityLevel = SecurityContextToIntegrityLevel(m_configurationSet.Environment().Context());
177179

178-
// Check for multiple integrity level requirements
179-
bool multipleIntegrityLevels = false;
180-
bool higherIntegrityLevelsThanCurrent = false;
181-
for (const auto& environment : m_configurationSet.GetUnitEnvironments())
180+
if (m_configurationSet)
182181
{
183-
auto integrityLevel = SecurityContextToIntegrityLevel(environment.Context());
184-
if (integrityLevel != m_currentIntegrityLevel)
185-
{
186-
multipleIntegrityLevels = true;
182+
m_setIntegrityLevel = SecurityContextToIntegrityLevel(m_configurationSet.Environment().Context());
187183

188-
if (ToIntegral(m_currentIntegrityLevel) < ToIntegral(integrityLevel))
184+
// Check for multiple integrity level requirements
185+
bool multipleIntegrityLevels = false;
186+
bool higherIntegrityLevelsThanCurrent = false;
187+
for (const auto& environment : m_configurationSet.GetUnitEnvironments())
188+
{
189+
auto integrityLevel = SecurityContextToIntegrityLevel(environment.Context());
190+
if (integrityLevel != m_currentIntegrityLevel)
189191
{
190-
higherIntegrityLevelsThanCurrent = true;
191-
break;
192+
multipleIntegrityLevels = true;
193+
194+
if (ToIntegral(m_currentIntegrityLevel) < ToIntegral(integrityLevel))
195+
{
196+
higherIntegrityLevelsThanCurrent = true;
197+
break;
198+
}
192199
}
193200
}
194-
}
195201

196-
// Prevent supplied parameters from crossing integrity levels
197-
for (const auto& parameter : m_configurationSet.Parameters())
198-
{
199-
if (parameter.ProvidedValue() != nullptr)
202+
// Prevent supplied parameters from crossing integrity levels
203+
for (const auto& parameter : m_configurationSet.Parameters())
200204
{
201-
THROW_HR_IF(WINGET_CONFIG_ERROR_PARAMETER_INTEGRITY_BOUNDARY, higherIntegrityLevelsThanCurrent || (multipleIntegrityLevels && parameter.IsSecure()));
205+
if (parameter.ProvidedValue() != nullptr)
206+
{
207+
THROW_HR_IF(WINGET_CONFIG_ERROR_PARAMETER_INTEGRITY_BOUNDARY, higherIntegrityLevelsThanCurrent || (multipleIntegrityLevels && parameter.IsSecure()));
208+
}
202209
}
203210
}
204211

@@ -219,13 +226,16 @@ namespace AppInstaller::CLI::ConfigurationRemoting
219226
std::call_once(m_createUnitSetProcessorsOnce,
220227
[&]()
221228
{
222-
for (const auto& environment : m_configurationSet.GetUnitEnvironments())
229+
if (m_configurationSet)
223230
{
224-
Security::IntegrityLevel requiredIntegrityLevel = SecurityContextToIntegrityLevel(environment.Context());
225-
226-
if (m_setProcessors.find(requiredIntegrityLevel) == m_setProcessors.end())
231+
for (const auto& environment : m_configurationSet.GetUnitEnvironments())
227232
{
228-
CreateSetProcessorForIntegrityLevel(requiredIntegrityLevel);
233+
Security::IntegrityLevel requiredIntegrityLevel = SecurityContextToIntegrityLevel(environment.Context());
234+
235+
if (m_setProcessors.find(requiredIntegrityLevel) == m_setProcessors.end())
236+
{
237+
CreateSetProcessorForIntegrityLevel(requiredIntegrityLevel);
238+
}
229239
}
230240
}
231241
});
@@ -240,6 +250,7 @@ namespace AppInstaller::CLI::ConfigurationRemoting
240250
auto itr = m_setProcessors.find(requiredIntegrityLevel);
241251
if (itr == m_setProcessors.end())
242252
{
253+
THROW_WIN32_IF_MSG(ERROR_NOT_SUPPORTED, !m_configurationSet, "Using configuration unit integrity level other than current level without a configuration set is not supported.");
243254
itr = CreateSetProcessorForIntegrityLevel(requiredIntegrityLevel);
244255
}
245256

src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationMixedElevationTests.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,5 +269,35 @@ public async Task SecureParameterAcrossIntegrityBoundaryFails()
269269
// Once they are implemented, swap to the appropriate error mechanism for the parameter integrity boundary.
270270
Assert.Throws<NotImplementedException>(() => processor.ApplySet(configurationSet, ApplyConfigurationSetFlags.None));
271271
}
272+
273+
/// <summary>
274+
/// Verifies that DynamicFactoryProcessors do not break on empty set when working with only units.
275+
/// </summary>
276+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
277+
[Fact]
278+
public async Task CreateDynamicFactoryProcessorsWithEmptyConfigurationSet()
279+
{
280+
string resourceName = "E2ETestResourcePID";
281+
string moduleName = "xE2ETestResource";
282+
Version version = new Version("0.0.0.1");
283+
284+
string tempDirectory = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
285+
Directory.CreateDirectory(tempDirectory);
286+
287+
ConfigurationUnit unit = this.ConfigurationUnit();
288+
unit.Metadata.Add("version", version.ToString());
289+
unit.Metadata.Add("module", moduleName);
290+
unit.Settings.Add("directoryPath", tempDirectory);
291+
unit.Type = resourceName;
292+
unit.Intent = ConfigurationUnitIntent.Inform;
293+
294+
IConfigurationSetProcessorFactory dynamicFactory = await this.fixture.ConfigurationStatics.CreateConfigurationSetProcessorFactoryAsync(Helpers.Constants.DynamicRuntimeHandlerIdentifier);
295+
296+
ConfigurationProcessor processor = this.CreateConfigurationProcessorWithDiagnostics(dynamicFactory);
297+
298+
var result = await processor.GetUnitSettingsAsync(unit);
299+
300+
Assert.NotNull(result);
301+
}
272302
}
273303
}

0 commit comments

Comments
 (0)