Update winget server com security (#4577)

Change:

- Explicitly set COM access permissions for packaged com invocations.
Leave access permissions as default and do not register COM objects for
manual invocation so that only RPC channel can be used for manual
activation.
- Update LaunchAndActivationString to allow Self, System, Built-in Admin and AppContainer
only, require at least MediumIL for non-AC.
- Move Configuration to a separate COM server, use default permission.

A separate pr will be sent to update AppInstaller manifest.

Validation:
Validated manually with Microsoft Store invocation, Powershell
invocation (elevated and non elevated), test sample code and Devhome
invocation (on package management and configuration).

Also specifically validated Store invocation with Built-in Administrator
sign-in (previously not working).

.github/actions/spelling/expect.txt

@@ -91,7 +91,7 @@ createmanifestmetadata
 cswinrt
 ctc
 currentuser
-DACL
+dacl
 datetimeoffset
 debian
 dedupe
@@ -401,7 +401,8 @@ roy
 runspace
 runtimeclass
 ryfu
-rzkzqaqjwj
+rzkzqaqjwj
+sacl
 SARL
 SASURL
 schematab

src/AppInstallerCLIPackage/Package.appxmanifest

@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>
 <Package 
   xmlns="http://schemas.microsoft.com/appx/manifest/foundation/windows10"
   xmlns:uap="http://schemas.microsoft.com/appx/manifest/uap/windows10"
@@ -61,7 +61,7 @@
         </uap5:Extension>
         <com:Extension Category="windows.comServer">
           <com:ComServer>
-            <com:ExeServer Executable="WinGetServer\WindowsPackageManagerServer.exe" DisplayName="Windows Package Manager Server" LaunchAndActivationPermission="O:SYG:SYD:(A;;11;;;WD)(A;;11;;;RC)(A;;11;;;AC)(A;;11;;;AN)S:P(ML;;NX;;;S-1-16-0)">
+            <com:ExeServer Executable="WinGetServer\WindowsPackageManagerServer.exe" DisplayName="Windows Package Manager Server" LaunchAndActivationPermission="O:SYG:SYD:(A;;11;;;IU)(A;;11;;;SY)(A;;11;;;BA)(A;;11;;;AC)S:P(ML;;NX;;;S-1-16-8192)">
               <com:Class Id ="74CB3139-B7C5-4B9E-9388-E6616DEA288C" DisplayName="PackageManager Server">
               </com:Class>
               <com:Class Id ="1BD8FF3A-EC50-4F69-AEEE-DF4C9D3BAA96" DisplayName="FindPackagesOptions Server">
@@ -74,15 +74,21 @@
               </com:Class>
               <com:Class Id ="AA2A5C04-1AD9-46C4-B74F-6B334AD7EB8C" DisplayName="UninstallOptions Server">
               </com:Class>
-              <com:Class Id ="C9ED7917-66AB-4E31-A92A-F65F18EF7933" DisplayName="Configuration Statics Server">
-              </com:Class>
               <com:Class Id ="8EF324ED-367C-4880-83E5-BB2ABD0B72F6" DisplayName="DownloadOptions Server">
               </com:Class>
               <com:Class Id ="6484A61D-50FA-41F0-B71E-F4370C6EB37C" DisplayName="AuthenticationArguments Server">
               </com:Class>
             </com:ExeServer>
           </com:ComServer>
         </com:Extension>
+        <com:Extension Category="windows.comServer">
+          <com:ComServer>
+            <com:ExeServer Executable="WinGetServer\WindowsPackageManagerServer.exe" DisplayName="Windows Package Manager Configuration Server">
+              <com:Class Id ="C9ED7917-66AB-4E31-A92A-F65F18EF7933" DisplayName="Configuration Statics Server">
+              </com:Class>
+            </com:ExeServer>
+          </com:ComServer>
+        </com:Extension>
       </Extensions>
     </Application>
   </Applications>
@@ -116,4 +122,4 @@
     <rescap:Capability Name="packageManagement" />
     <rescap:Capability Name="unvirtualizedResources" />
   </Capabilities>
-</Package>
+</Package>

src/WinGetServer/WinMain.cpp

@@ -16,6 +16,7 @@
 #include <memory>
 #include <string>
 #include <string_view>
+#include <vector>
 
 // Holds the wwinmain open until COM tells us there are no more server connections
 wil::unique_event _comServerExitEvent;
@@ -100,6 +101,49 @@ extern "C" HRESULT CreateInstance(
     return S_OK;
 }
 
+HRESULT InitializeComSecurity()
+{
+    wil::unique_hlocal_security_descriptor securityDescriptor;
+    // Allow Self, System, Built-in Admin and App Container access. 3 is COM_RIGHTS_EXECUTE | COM_RIGHTS_EXECUTE_LOCAL
+    std::string securityDescriptorString = "O:SYG:SYD:(A;;3;;;PS)(A;;3;;;SY)(A;;3;;;BA)(A;;3;;;AC)";
+    RETURN_LAST_ERROR_IF(!ConvertStringSecurityDescriptorToSecurityDescriptorA(securityDescriptorString.c_str(), SDDL_REVISION_1, &securityDescriptor, nullptr));
+
+    // Make absolute security descriptor as CoInitializeSecurity required
+    SECURITY_DESCRIPTOR absoluteSecurityDescriptor;
+    DWORD securityDescriptorSize = sizeof(SECURITY_DESCRIPTOR);
+
+    DWORD daclSize = 0;
+    DWORD saclSize = 0;
+    DWORD ownerSize = 0;
+    DWORD groupSize = 0;
+
+    // Get required size
+    BOOL result = MakeAbsoluteSD(securityDescriptor.get(), &absoluteSecurityDescriptor, &securityDescriptorSize, nullptr, &daclSize, nullptr, &saclSize, nullptr, &ownerSize, nullptr, &groupSize);
+    RETURN_HR_IF_MSG(E_FAIL, result || GetLastError() != ERROR_INSUFFICIENT_BUFFER, "MakeAbsoluteSD failed to return buffer sizes");
+
+    std::vector<BYTE> dacl(daclSize);
+    std::vector<BYTE> sacl(saclSize);
+    std::vector<BYTE> owner(ownerSize);
+    std::vector<BYTE> group(groupSize);
+
+    RETURN_LAST_ERROR_IF(!MakeAbsoluteSD(securityDescriptor.get(), &absoluteSecurityDescriptor, &securityDescriptorSize, (PACL)dacl.data(), &daclSize, (PACL)sacl.data(), &saclSize, (PACL)owner.data(), &ownerSize, (PACL)group.data(), &groupSize));
+
+    // Initialize com security
+    RETURN_IF_FAILED(CoInitializeSecurity(
+        &absoluteSecurityDescriptor, // Security descriptor
+        -1, // Authentication services count. -1 is let com choose.
+        nullptr, // Authentication services array
+        nullptr, // Reserved
+        RPC_C_AUTHN_LEVEL_DEFAULT, // Authentication level.
+        RPC_C_IMP_LEVEL_IDENTIFY, // Impersonation level. Identify client.
+        nullptr, // Authentication list
+        EOAC_NONE, // Additional capabilities
+        nullptr // Reserved
+    ));
+
+    return S_OK;
+}
+
 int __stdcall wWinMain(_In_ HINSTANCE, _In_opt_ HINSTANCE, _In_ LPWSTR cmdLine, _In_ int)
 {
     wil::SetResultLoggingCallback(&WindowsPackageManagerServerWilResultLoggingCallback);
@@ -115,8 +159,6 @@ int __stdcall wWinMain(_In_ HINSTANCE, _In_opt_ HINSTANCE, _In_ LPWSTR cmdLine,
         RETURN_IF_FAILED(globalOptions->Set(COMGLB_EXCEPTION_HANDLING, COMGLB_EXCEPTION_DONOT_HANDLE_ANY));
     }
 
-    RETURN_IF_FAILED(WindowsPackageManagerServerInitialize());
-
     // Command line parsing
     int argc = 0;
     LPWSTR* argv = CommandLineToArgvW(cmdLine, &argc);
@@ -130,18 +172,29 @@ int __stdcall wWinMain(_In_ HINSTANCE, _In_opt_ HINSTANCE, _In_ LPWSTR cmdLine,
         manualActivation = true;
     }
 
+    // For packaged com activation, initialize com security.
+    // For manual activation, leave as default. We'll not register objects for manual activation.
+    if (!manualActivation)
+    {
+        // This must be called after IGlobalOptions (fast rundown setting cannot be changed after CoInitializeSecurity)
+        // This must be called before WindowsPackageManagerServerInitialize (when setting the logs
+        // to Windows.Storage folders, automatic CoInitializeSecurity is triggered)
+        RETURN_IF_FAILED(InitializeComSecurity());
+    }
+
+    RETURN_IF_FAILED(WindowsPackageManagerServerInitialize());
+
     _comServerExitEvent.create();
     RETURN_IF_FAILED(WindowsPackageManagerServerModuleCreate(&_releaseNotifier));
     try
     {
-        // Register all the CoCreatableClassWrlCreatorMapInclude classes
-        RETURN_IF_FAILED(WindowsPackageManagerServerModuleRegister());
-
         // Manual reset event to notify the client that the server is available.
         wil::unique_event manualResetEvent;
 
         if (manualActivation)
         {
+            // For manual activation, do not register com objects
+            // so that only RPC channel can be used.
             HANDLE hMutex = NULL;
             hMutex = CreateMutex(NULL, FALSE, TEXT("WinGetServerMutex"));
             RETURN_LAST_ERROR_IF_NULL(hMutex);
@@ -157,6 +210,11 @@ int __stdcall wWinMain(_In_ HINSTANCE, _In_opt_ HINSTANCE, _In_ LPWSTR cmdLine,
             manualResetEvent = CreateOrOpenServerStartEvent();
             manualResetEvent.SetEvent();
         }
+        else
+        {
+            // Register all the CoCreatableClassWrlCreatorMapInclude classes
+            RETURN_IF_FAILED(WindowsPackageManagerServerModuleRegister());
+        }
 
         _comServerExitEvent.wait();
 
@@ -165,7 +223,10 @@ int __stdcall wWinMain(_In_ HINSTANCE, _In_opt_ HINSTANCE, _In_ LPWSTR cmdLine,
             manualResetEvent.reset();
         }
 
-        RETURN_IF_FAILED(WindowsPackageManagerServerModuleUnregister());
+        if (!manualActivation)
+        {
+            RETURN_IF_FAILED(WindowsPackageManagerServerModuleUnregister());
+        }
     }
     CATCH_RETURN()