commit b0bd0fbbfafdf6a3314554314876493418927800
parent d1083328e48f7b053042609fbba9f619c4048a02
Author: Madhusudhan-MSFT <53235553+Madhusudhan-MSFT@users.noreply.github.com>
Date: Fri, 25 Aug 2023 13:09:09 -0700
Guard WinRT InProc Window Package Manager Deployment APIs by EnableAp… (#3537)
* Guard WinRT InProc Window Package Manager Deployment APIs by EnableAppInstaller GroupPolicy
[WHY:]
WINRT InProcess Deployment APIs are current not guarded by EnableAppInstaller GroupPolicy.
where as COM InProcess/OutProcess API are guarded by group policy.
[WHAT:]
- This fix is introducing the Group Policy check for WinRT InProcess deployment APIs
to block them if EnableAppInstaller is disabled.
- Added some group policy tests for the InProcess/ OutOfProcess Interops to validate
group policy indeed blocks the Object creation when policy is disabled.
[How Tested:]
- Copied the fix and applied wingetdev package
- GroupPolicyForInterops test run to ensure they all behave as expected.
* Addressed spell checker issues
* Fixed additional spell checker issue.
* Addressing InProcess WinRT Interop Group policy test failure
- CS WinRT/C++ WinRT doesn't surface application specific error code
return from DllActivationFactory implementation and it by design
but it will replace application specific HRESULT one of the standard
HRESULT from RoActionationInstance function call
hence InProc test would expect TragetInvocation Exception with
2nd level Inner Exception returns class not registered error code
* Addressed spell checker issues related to comments
* Debug only change and it will be reverted with right fix in the follow up
* Updating InProc GroupPolicy Interop test to expect TargetInvocationException when
policy is disabled as the actual error status will not surface to test
* Enforce group policy check for Projected WinRT InProc/ COM Out of Proc & Group Policy test updates
- Added an abstract base class implementation that
- ActivationFactoryInstanceInitializer - WinRT InProc
- LocalServerInstanceInitializer - Out of Proc
derives from that enforces winget group policy when disabled and
it throws GroupPolicyException.
- Updated the GroupPolicyInterop tests to expect GroupPolicyException for the
attempts to create interop classes when winget group policy disabled.
* Addressed detected Spellcheck issues
* Fix for test constants indentation causing build failure.
* Microsoft.WinGet.SharedLib.GroupPolicy error code mapping improvements
- Adding error code mapping to GroupPolicyException based on the FailureType
- Updated GroupPolicy tests to validate expected errorcode as part of
test assertion
---------
Co-authored-by: Madhusudhan Gumbalapura Sudarshan <Madhusudhan.Sudarshan@microsoft.com>
Diffstat:
10 files changed, 190 insertions(+), 9 deletions(-)
diff --git a/src/AppInstallerCLIE2ETests/Constants.cs b/src/AppInstallerCLIE2ETests/Constants.cs
@@ -125,6 +125,9 @@ namespace AppInstallerCLIE2ETests
public const string SimpleTestModuleName = "xE2ETestResource";
public const string LocalModuleDescriptor = "[Local]";
+ // Group Policy Error Message
+ public const string BlockByWinGetPolicyErrorMessage = "This operation is disabled by Group Policy : Enable Windows Package Manager";
+
/// <summary>
/// Error codes.
/// </summary>
diff --git a/src/AppInstallerCLIE2ETests/Interop/GroupPolicyForInterop.cs b/src/AppInstallerCLIE2ETests/Interop/GroupPolicyForInterop.cs
@@ -0,0 +1,93 @@
+// -----------------------------------------------------------------------------
+// <copyright file="GroupPolicyForInterop.cs" company="Microsoft Corporation">
+// Copyright (c) Microsoft Corporation. Licensed under the MIT License.
+// </copyright>
+// -----------------------------------------------------------------------------
+
+namespace AppInstallerCLIE2ETests.Interop
+{
+ using Microsoft.Management.Deployment;
+ using Microsoft.Management.Deployment.Projection;
+ using Microsoft.WinGet.SharedLib.Exceptions;
+ using NUnit.Framework;
+
+ /// <summary>
+ /// Group Policy Tests for COM/WinRT Interop classes.
+ /// </summary>
+ [TestFixtureSource(typeof(InstanceInitializersSource), nameof(InstanceInitializersSource.InProcess), Category = nameof(InstanceInitializersSource.InProcess))]
+ [TestFixtureSource(typeof(InstanceInitializersSource), nameof(InstanceInitializersSource.OutOfProcess), Category = nameof(InstanceInitializersSource.OutOfProcess))]
+ public class GroupPolicyForInterop : BaseInterop
+ {
+ /// <summary>
+ /// Initializes a new instance of the <see cref="GroupPolicyForInterop"/> class.
+ /// </summary>
+ /// <param name="initializer">Initializer.</param>
+ public GroupPolicyForInterop(IInstanceInitializer initializer)
+ : base(initializer)
+ {
+ }
+
+ /// <summary>
+ /// Test setup.
+ /// </summary>
+ [SetUp]
+ public void SetUp()
+ {
+ GroupPolicyHelper.DeleteExistingPolicies();
+ }
+
+ /// <summary>
+ /// Clean up.
+ /// </summary>
+ [TearDown]
+ public void CleanUp()
+ {
+ GroupPolicyHelper.DeleteExistingPolicies();
+ }
+
+ /// <summary>
+ /// Validates disabling WinGetPolicy should block COM/WinRT Objects creation (InProcess and OutOfProcess).
+ /// </summary>
+ [Test]
+ public void DisableWinGetPolicy()
+ {
+ GroupPolicyHelper.EnableWinget.Disable();
+
+ GroupPolicyException groupPolicyException = Assert.Catch<GroupPolicyException>(() => { PackageManager packageManager = this.TestFactory.CreatePackageManager(); });
+ Assert.AreEqual(Constants.BlockByWinGetPolicyErrorMessage, groupPolicyException.Message);
+ Assert.AreEqual(Constants.ErrorCode.ERROR_BLOCKED_BY_POLICY, groupPolicyException.HResult);
+
+ groupPolicyException = Assert.Catch<GroupPolicyException>(() => { FindPackagesOptions findPackagesOptions = this.TestFactory.CreateFindPackagesOptions(); });
+ Assert.AreEqual(Constants.BlockByWinGetPolicyErrorMessage, groupPolicyException.Message);
+ Assert.AreEqual(Constants.ErrorCode.ERROR_BLOCKED_BY_POLICY, groupPolicyException.HResult);
+
+ groupPolicyException = Assert.Catch<GroupPolicyException>(() => { CreateCompositePackageCatalogOptions createCompositePackageCatalogOptions = this.TestFactory.CreateCreateCompositePackageCatalogOptions(); });
+ Assert.AreEqual(Constants.BlockByWinGetPolicyErrorMessage, groupPolicyException.Message);
+ Assert.AreEqual(Constants.ErrorCode.ERROR_BLOCKED_BY_POLICY, groupPolicyException.HResult);
+
+ groupPolicyException = Assert.Catch<GroupPolicyException>(() => { InstallOptions installOptions = this.TestFactory.CreateInstallOptions(); });
+ Assert.AreEqual(Constants.BlockByWinGetPolicyErrorMessage, groupPolicyException.Message);
+ Assert.AreEqual(Constants.ErrorCode.ERROR_BLOCKED_BY_POLICY, groupPolicyException.HResult);
+
+ groupPolicyException = Assert.Catch<GroupPolicyException>(() => { UninstallOptions uninstallOptions = this.TestFactory.CreateUninstallOptions(); });
+ Assert.AreEqual(Constants.BlockByWinGetPolicyErrorMessage, groupPolicyException.Message);
+ Assert.AreEqual(Constants.ErrorCode.ERROR_BLOCKED_BY_POLICY, groupPolicyException.HResult);
+
+ groupPolicyException = Assert.Catch<GroupPolicyException>(() => { DownloadOptions downloadOptions = this.TestFactory.CreateDownloadOptions(); });
+ Assert.AreEqual(Constants.BlockByWinGetPolicyErrorMessage, groupPolicyException.Message);
+ Assert.AreEqual(Constants.ErrorCode.ERROR_BLOCKED_BY_POLICY, groupPolicyException.HResult);
+
+ groupPolicyException = Assert.Catch<GroupPolicyException>(() => { PackageMatchFilter packageMatchFilter = this.TestFactory.CreatePackageMatchFilter(); });
+ Assert.AreEqual(Constants.BlockByWinGetPolicyErrorMessage, groupPolicyException.Message);
+ Assert.AreEqual(Constants.ErrorCode.ERROR_BLOCKED_BY_POLICY, groupPolicyException.HResult);
+
+ // PackageManagerSettings is not implemented in context OutOfProcDev
+ if (this.TestFactory.Context == ClsidContext.InProc)
+ {
+ groupPolicyException = Assert.Catch<GroupPolicyException>(() => { PackageManagerSettings packageManagerSettings = this.TestFactory.CreatePackageManagerSettings(); });
+ Assert.AreEqual(Constants.BlockByWinGetPolicyErrorMessage, groupPolicyException.Message);
+ Assert.AreEqual(Constants.ErrorCode.ERROR_BLOCKED_BY_POLICY, groupPolicyException.HResult);
+ }
+ }
+ }
+}
diff --git a/src/Microsoft.Management.Deployment.Projection/Initializers/ActivationFactoryInstanceInitializer.cs b/src/Microsoft.Management.Deployment.Projection/Initializers/ActivationFactoryInstanceInitializer.cs
@@ -3,6 +3,8 @@
namespace Microsoft.Management.Deployment.Projection
{
+ using Microsoft.Management.Deployment.Projection.Initializers;
+
/// <summary>
/// Activation factory instance initializer requires that:
/// - DllGetActivationFactory is exported
@@ -11,13 +13,13 @@ namespace Microsoft.Management.Deployment.Projection
/// to match the projected classes (e.g. PackageManager) namespace
/// More details: https://docs.microsoft.com/en-us/windows/apps/develop/platform/csharp-winrt/#winrt-type-activation
/// </summary>
- public class ActivationFactoryInstanceInitializer : IInstanceInitializer
+ public class ActivationFactoryInstanceInitializer : PolicyEnforcedInstanceInitializer
{
/// <summary>
/// In-process context
/// </summary>
- public ClsidContext Context => ClsidContext.InProc;
-
+ public override ClsidContext Context => ClsidContext.InProc;
+
/// <summary>
/// Calls default projected class constructor implemented by CsWinRT.
/// Default constructor uses DllGetActivationFactory to create an object
@@ -25,6 +27,6 @@ namespace Microsoft.Management.Deployment.Projection
/// </summary>
/// <typeparam name="T">Projected class type</typeparam>
/// <returns>Instance of the provided type.</returns>
- public T CreateInstance<T>() where T : new() => new();
+ protected override T CreateInstanceInternal<T>() => new();
}
}
diff --git a/src/Microsoft.Management.Deployment.Projection/Initializers/LocalServerInstanceInitializer.cs b/src/Microsoft.Management.Deployment.Projection/Initializers/LocalServerInstanceInitializer.cs
@@ -2,16 +2,17 @@
// Licensed under the MIT License.
namespace Microsoft.Management.Deployment.Projection
-{
+{
+ using Microsoft.Management.Deployment.Projection.Initializers;
using WinRT;
// Out-of-process COM instance initializer.
- public class LocalServerInstanceInitializer : IInstanceInitializer
+ public class LocalServerInstanceInitializer : PolicyEnforcedInstanceInitializer
{
/// <summary>
/// Out-of-process context.
/// </summary>
- public ClsidContext Context => UseDevClsids ? ClsidContext.OutOfProcDev : ClsidContext.OutOfProc;
+ public override ClsidContext Context => UseDevClsids ? ClsidContext.OutOfProcDev : ClsidContext.OutOfProc;
/// <summary>
/// Allow lower trust registration.
@@ -29,8 +30,7 @@ namespace Microsoft.Management.Deployment.Projection
/// </summary>
/// <typeparam name="T">Projected class type.</typeparam>
/// <returns>Instance of the provided type.</returns>
- public T CreateInstance<T>()
- where T : new()
+ protected override T CreateInstanceInternal<T>()
{
var clsid = ClassesDefinition.GetClsid<T>(Context);
var iid = ClassesDefinition.GetIid<T>();
diff --git a/src/Microsoft.Management.Deployment.Projection/Initializers/PolicyEnforcedInstanceInitializer.cs b/src/Microsoft.Management.Deployment.Projection/Initializers/PolicyEnforcedInstanceInitializer.cs
@@ -0,0 +1,38 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+namespace Microsoft.Management.Deployment.Projection.Initializers
+{
+ using Microsoft.WinGet.SharedLib.Exceptions;
+ using Microsoft.WinGet.SharedLib.PolicySettings;
+
+ /// <summary>
+ /// An abstract base that enforces group policy before creating derived class instance.
+ /// </summary>
+ public abstract class PolicyEnforcedInstanceInitializer : IInstanceInitializer
+ {
+ /// <summary>
+ /// CLSID context.
+ /// </summary>
+ public abstract ClsidContext Context { get; }
+
+ /// <summary>
+ /// Create instance of the provided type.
+ /// </summary>
+ /// <typeparam name="T">Projected class type.</typeparam>
+ /// <returns>Instance of the provided type.</returns>
+ public T CreateInstance<T>() where T : new()
+ {
+ GroupPolicy groupPolicy = GroupPolicy.GetInstance();
+
+ if (!groupPolicy.IsEnabled(Policy.WinGet))
+ {
+ throw new GroupPolicyException(Policy.WinGet, GroupPolicyFailureType.BlockedByPolicy);
+ }
+
+ return this.CreateInstanceInternal<T>();
+ }
+
+ protected abstract T CreateInstanceInternal<T>() where T : new();
+ }
+}
diff --git a/src/Microsoft.Management.Deployment.Projection/Microsoft.Management.Deployment.Projection.csproj b/src/Microsoft.Management.Deployment.Projection/Microsoft.Management.Deployment.Projection.csproj
@@ -26,5 +26,6 @@
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
<ReferenceOutputAssembly>True</ReferenceOutputAssembly>
</ProjectReference>
+ <ProjectReference Include="..\PowerShell\Microsoft.WinGet.SharedLib\Microsoft.WinGet.SharedLib.csproj" />
</ItemGroup>
</Project>
diff --git a/src/PowerShell/Microsoft.WinGet.SharedLib/Exceptions/ErrorCodes.cs b/src/PowerShell/Microsoft.WinGet.SharedLib/Exceptions/ErrorCodes.cs
@@ -0,0 +1,19 @@
+// -----------------------------------------------------------------------------
+// <copyright file="ErrorCodes.cs" company="Microsoft Corporation">
+// Copyright (c) Microsoft Corporation. Licensed under the MIT License.
+// </copyright>
+// -----------------------------------------------------------------------------
+
+namespace Microsoft.WinGet.SharedLib.Exceptions
+{
+ /// <summary>
+ /// This should match the ones in AppInstallerErrors.h.
+ /// </summary>
+ internal static class ErrorCodes
+ {
+#pragma warning disable SA1600 // ElementsMustBeDocumented
+ internal const int AppInstallerCLIErrorInternalError = unchecked((int)0x8A150001);
+ internal const int AppInstallerCLIErrorBlockedByPolicy = unchecked((int)0x8A15003A);
+#pragma warning restore SA1600 // ElementsMustBeDocumented
+ }
+}
diff --git a/src/PowerShell/Microsoft.WinGet.SharedLib/Exceptions/GroupPolicyException.cs b/src/PowerShell/Microsoft.WinGet.SharedLib/Exceptions/GroupPolicyException.cs
@@ -24,6 +24,7 @@ namespace Microsoft.WinGet.SharedLib.Exceptions
public GroupPolicyException(Policy policy, GroupPolicyFailureType policyFailureType)
: base(string.Format(policyFailureType.GetFailureString(), policy.GetResourceString()))
{
+ this.HResult = policyFailureType.GetErrorCode();
}
/// <summary>
@@ -34,6 +35,7 @@ namespace Microsoft.WinGet.SharedLib.Exceptions
public GroupPolicyException(GroupPolicyFailureType policyFailureType, Exception innerException)
: base(policyFailureType.GetFailureString(), innerException)
{
+ this.HResult = policyFailureType.GetErrorCode();
}
}
}
diff --git a/src/PowerShell/Microsoft.WinGet.SharedLib/Extensions/EnumPolicyExtension.cs b/src/PowerShell/Microsoft.WinGet.SharedLib/Extensions/EnumPolicyExtension.cs
@@ -6,6 +6,7 @@
namespace Microsoft.WinGet.SharedLib.Extensions
{
+ using Microsoft.WinGet.SharedLib.Exceptions;
using Microsoft.WinGet.SharedLib.PolicySettings;
using Microsoft.WinGet.SharedLib.Resources;
@@ -70,5 +71,23 @@ namespace Microsoft.WinGet.SharedLib.Extensions
default: return string.Empty;
}
}
+
+ /// <summary>
+ /// Gets ErrorCode mapped to GroupPolicyFailureType.
+ /// </summary>
+ /// <param name="policyFailure">GroupPolicyFailureType.</param>
+ /// <returns>ErrorCode.</returns>
+ public static int GetErrorCode(this GroupPolicyFailureType policyFailure)
+ {
+ switch (policyFailure)
+ {
+ case GroupPolicyFailureType.BlockedByPolicy:
+ return ErrorCodes.AppInstallerCLIErrorBlockedByPolicy;
+ case GroupPolicyFailureType.NotFound:
+ case GroupPolicyFailureType.LoadError:
+ default:
+ return ErrorCodes.AppInstallerCLIErrorInternalError;
+ }
+ }
}
}
diff --git a/src/WindowsPackageManager/main.cpp b/src/WindowsPackageManager/main.cpp
@@ -13,6 +13,8 @@
#include <AppInstallerFileLogger.h>
#include <AppInstallerStrings.h>
#include <AppInstallerTelemetry.h>
+#include <AppInstallerErrors.h>
+#include <winget/GroupPolicy.h>
#include <ComClsids.h>
using namespace winrt::Microsoft::Management::Deployment;
@@ -112,6 +114,8 @@ extern "C"
WINDOWS_PACKAGE_MANAGER_API WindowsPackageManagerInProcModuleGetActivationFactory(HSTRING classId, void** factory) try
{
+ RETURN_HR_IF(APPINSTALLER_CLI_ERROR_BLOCKED_BY_POLICY, !::AppInstaller::Settings::GroupPolicies().IsEnabled(::AppInstaller::Settings::TogglePolicy::Policy::WinGet));
+
return WINRT_GetActivationFactory(classId, factory);
}
CATCH_RETURN();
