Skip to content

Commit df1118d

Browse files
authored
Improve processor integrity validation (#6556)
## 📖 Description Improves the processor integrity checks by pinning the target files to prevent modification.
1 parent dfad88c commit df1118d

20 files changed

Lines changed: 1057 additions & 121 deletions

File tree

‎.github/actions/spelling/allow.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,8 @@ RESTAPI
290290
RESTARTAPPS
291291
resw
292292
resx
293+
retargeted
294+
retargeting
293295
rethrowing
294296
roadmap
295297
robuffer

‎doc/windows/package-manager/winget/returnCodes.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,3 +237,4 @@ ms.localizationpriority: low
237237
| 0x8A15C111 | -1978285807 | WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE_ADMIN | Loading the module for the configuration unit failed because it requires administrator privileges to run. |
238238
| 0x8A15C112 | -1978285806 | WINGET_CONFIG_ERROR_NOT_SUPPORTED_BY_PROCESSOR | Operation is not supported by the configuration processor. |
239239
| 0x8A15C113 | -1978285805 | WINGET_CONFIG_ERROR_PROCESSOR_HASH_MISMATCH | The DSC processor hash provided does not match hash of the target file. |
240+
| 0x8A15C114 | -1978285804 | WINGET_CONFIG_ERROR_PROCESSOR_PATH_CHANGED | The DSC processor path no longer refers to the file that was verified. |

‎src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3649,6 +3649,10 @@ An unlocalized JSON fragment will follow on another line.</comment>
36493649
<data name="WINGET_CONFIG_ERROR_PROCESSOR_HASH_MISMATCH" xml:space="preserve">
36503650
<value>The DSC processor hash provided does not match hash of the target file.</value>
36513651
</data>
3652+
<data name="WINGET_CONFIG_ERROR_PROCESSOR_PATH_CHANGED" xml:space="preserve">
3653+
<value>The DSC processor path no longer refers to the file that was verified.</value>
3654+
<comment>An error message shown when the file that a configuration processor (DSC) executable path points to has been replaced or swapped after it was verified.</comment>
3655+
</data>
36523656
<data name="NoAdminUninstallForUserScopePackage" xml:space="preserve">
36533657
<value>The package installed for user scope cannot be uninstalled when running with administrator privileges.</value>
36543658
</data>

‎src/AppInstallerSharedLib/Errors.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,7 @@ namespace AppInstaller
298298
WINGET_HRESULT_INFO(WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE_ADMIN, "Loading the module for the configuration unit failed because it requires administrator privileges to run."),
299299
WINGET_HRESULT_INFO(WINGET_CONFIG_ERROR_NOT_SUPPORTED_BY_PROCESSOR, "Operation is not supported by the configuration processor."),
300300
WINGET_HRESULT_INFO(WINGET_CONFIG_ERROR_PROCESSOR_HASH_MISMATCH, "The DSC processor hash provided does not match hash of the target file."),
301+
WINGET_HRESULT_INFO(WINGET_CONFIG_ERROR_PROCESSOR_PATH_CHANGED, "The DSC processor path no longer refers to the file that was verified."),
301302

302303
// Errors without the error bit set
303304
WINGET_HRESULT_INFO(WINGET_INSTALLED_STATUS_INSTALL_LOCATION_NOT_APPLICABLE, "The install location is not applicable."),

‎src/AppInstallerSharedLib/Public/AppInstallerErrors.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,7 @@
235235
#define WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE_ADMIN ((HRESULT)0x8A15C111)
236236
#define WINGET_CONFIG_ERROR_NOT_SUPPORTED_BY_PROCESSOR ((HRESULT)0x8A15C112)
237237
#define WINGET_CONFIG_ERROR_PROCESSOR_HASH_MISMATCH ((HRESULT)0x8A15C113)
238+
#define WINGET_CONFIG_ERROR_PROCESSOR_PATH_CHANGED ((HRESULT)0x8A15C114)
238239

239240
namespace AppInstaller
240241
{

‎src/Directory.Packages.props‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@
3333
<PackageVersion Include="System.Text.RegularExpressions" Version="4.3.1" />
3434
<PackageVersion Include="xunit" Version="2.4.2" />
3535
<PackageVersion Include="xunit.runner.visualstudio" Version="2.4.5" />
36+
<!-- 1.5.x targets xunit v3 reporting; with xunit 2.4.x it silently drops dynamically
37+
skipped tests from the results instead of reporting them as skipped. -->
38+
<PackageVersion Include="Xunit.SkippableFact" Version="1.4.13" />
3639
<PackageVersion Include="YamlDotNet" Version="16.3.0" />
3740
</ItemGroup>
3841
</Project>
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// -----------------------------------------------------------------------------
2+
// <copyright file="PinnedProcessorFile.cs" company="Microsoft Corporation">
3+
// Copyright (c) Microsoft Corporation. Licensed under the MIT License.
4+
// </copyright>
5+
// -----------------------------------------------------------------------------
6+
7+
namespace Microsoft.Management.Configuration.Processor.DSCv3.Helpers
8+
{
9+
using System;
10+
using Microsoft.Win32.SafeHandles;
11+
12+
/// <summary>
13+
/// The handles that pin the processor file.
14+
/// <para>
15+
/// <see cref="NameHandle"/> is opened without following reparse points, so it pins whatever
16+
/// object sits at the given path; nothing else can delete it, rename it, move another file over
17+
/// it, or - if it is a link - repoint it, because all of those require write or delete access.
18+
/// </para>
19+
/// <para>
20+
/// <see cref="ContentHandle"/> pins the file that was actually hashed. It differs from
21+
/// <see cref="NameHandle"/> only when the path is a link, in which case both ends of the link
22+
/// are pinned.
23+
/// </para>
24+
/// </summary>
25+
internal sealed class PinnedProcessorFile : IDisposable
26+
{
27+
private readonly SafeFileHandle nameHandle;
28+
private readonly SafeFileHandle? targetHandle;
29+
private bool disposed = false;
30+
31+
/// <summary>
32+
/// Initializes a new instance of the <see cref="PinnedProcessorFile"/> class.
33+
/// </summary>
34+
/// <param name="nameHandle">The handle that pins the object at the path itself.</param>
35+
/// <param name="targetHandle">The handle that pins the link target, or null if the path is not a link.</param>
36+
public PinnedProcessorFile(SafeFileHandle nameHandle, SafeFileHandle? targetHandle)
37+
{
38+
this.nameHandle = nameHandle;
39+
this.targetHandle = targetHandle;
40+
}
41+
42+
/// <summary>
43+
/// Gets the handle pinning the object at the path itself.
44+
/// </summary>
45+
public SafeFileHandle NameHandle
46+
{
47+
get { return this.nameHandle; }
48+
}
49+
50+
/// <summary>
51+
/// Gets the handle pinning the file whose content was hashed.
52+
/// </summary>
53+
public SafeFileHandle ContentHandle
54+
{
55+
get { return this.targetHandle ?? this.nameHandle; }
56+
}
57+
58+
/// <inheritdoc/>
59+
public void Dispose()
60+
{
61+
if (!this.disposed)
62+
{
63+
this.targetHandle?.Dispose();
64+
this.nameHandle.Dispose();
65+
this.disposed = true;
66+
}
67+
}
68+
}
69+
}
Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
// -----------------------------------------------------------------------------
2+
// <copyright file="ProcessorPathBinding.cs" company="Microsoft Corporation">
3+
// Copyright (c) Microsoft Corporation. Licensed under the MIT License.
4+
// </copyright>
5+
// -----------------------------------------------------------------------------
6+
7+
namespace Microsoft.Management.Configuration.Processor.DSCv3.Helpers
8+
{
9+
using System;
10+
using System.Collections.Generic;
11+
using System.IO;
12+
using Microsoft.Management.Configuration.Processor.Exceptions;
13+
using Microsoft.Win32.SafeHandles;
14+
15+
/// <summary>
16+
/// Verifies the processor path once and then pins it for the lifetime of the object, so that the
17+
/// verified file is the file that gets executed every time; no further checks are required.
18+
/// <para>
19+
/// The pin has two parts:
20+
/// </para>
21+
/// <list type="number">
22+
/// <item><description>
23+
/// A handle to the file itself, opened sharing read only and without following reparse points.
24+
/// Sharing read still allows the file to be executed, but denies every other opener write and
25+
/// delete access. Since renaming a file requires delete access, the verified bytes cannot be
26+
/// modified, replaced, renamed or deleted. If the path is a link, the link target is pinned the
27+
/// same way; a link cannot be retargeted without write access to it, so holding both ends means
28+
/// the path cannot be made to resolve to a different file.
29+
/// </description></item>
30+
/// <item><description>
31+
/// Handles to each ancestor directory of the file's normalized path, opened sharing read and
32+
/// write but not delete. That prevents any component of the path from being renamed or deleted,
33+
/// which would otherwise allow a different file to be placed at the same path without ever
34+
/// touching the pinned file. Creating and deleting files inside those directories is unaffected.
35+
/// </description></item>
36+
/// </list>
37+
/// <para>
38+
/// The path used to launch the processor is the normalized path obtained from the pinned handle,
39+
/// which contains no symbolic links or junctions. No component of it can be re-pointed at another
40+
/// target afterwards either: setting a reparse point on a directory requires it to be empty, and
41+
/// every pinned directory permanently contains the pinned component below it. Each directory is
42+
/// also opened without following reparse points and rejected if it is one, so a component that is
43+
/// swapped for a link while the path is being pinned is detected rather than followed.
44+
/// </para>
45+
/// </summary>
46+
internal sealed class ProcessorPathBinding : IDisposable
47+
{
48+
private const int MaximumPinAttempts = 3;
49+
50+
private readonly object bindingLock = new ();
51+
52+
private string? boundPath = null;
53+
private bool boundIsAlias = false;
54+
private PinnedProcessorFile? pinnedFile = null;
55+
private List<SafeFileHandle> directoryHandles = new ();
56+
private string? launchPath = null;
57+
private bool disposed = false;
58+
59+
/// <summary>
60+
/// Gets a value indicating whether the path has been verified and pinned.
61+
/// </summary>
62+
public bool IsPinned
63+
{
64+
get
65+
{
66+
lock (this.bindingLock)
67+
{
68+
return this.launchPath != null;
69+
}
70+
}
71+
}
72+
73+
/// <summary>
74+
/// Verifies and pins the given path on first use, returning the path to use to launch the
75+
/// processor. Subsequent calls return the already pinned path.
76+
/// </summary>
77+
/// <param name="path">The path to the DSC executable or app execution alias.</param>
78+
/// <param name="expectedHash">The expected SHA256 hash (hex string, case-insensitive).</param>
79+
/// <param name="isAlias">Whether the path is an app execution alias reparse point.</param>
80+
/// <returns>The pinned path to launch the processor with.</returns>
81+
public string EnsurePinned(string path, string expectedHash, bool isAlias)
82+
{
83+
lock (this.bindingLock)
84+
{
85+
ObjectDisposedException.ThrowIf(this.disposed, this);
86+
87+
if (this.launchPath == null)
88+
{
89+
this.Pin(path, expectedHash, isAlias);
90+
}
91+
else if (!string.Equals(this.boundPath, path, StringComparison.OrdinalIgnoreCase) ||
92+
this.boundIsAlias != isAlias)
93+
{
94+
// The pin covers exactly one target; a different one has not been verified.
95+
throw new DscProcessorPathChangedException();
96+
}
97+
98+
return this.launchPath!;
99+
}
100+
}
101+
102+
/// <inheritdoc/>
103+
public void Dispose()
104+
{
105+
lock (this.bindingLock)
106+
{
107+
if (!this.disposed)
108+
{
109+
this.ReleaseDirectoryHandles();
110+
this.pinnedFile?.Dispose();
111+
this.pinnedFile = null;
112+
this.launchPath = null;
113+
this.disposed = true;
114+
}
115+
}
116+
}
117+
118+
private void Pin(string path, string expectedHash, bool isAlias)
119+
{
120+
PinnedProcessorFile file = ProcessorPathIntegrity.VerifyAndOpen(path, expectedHash, isAlias);
121+
122+
try
123+
{
124+
string finalPath = ProcessorPathIntegrity.GetFinalPath(file.ContentHandle);
125+
126+
// Pinning the ancestors is itself done by path, so a rename that happens while the
127+
// ancestors are being pinned could leave a directory pinned that is no longer part of
128+
// the path. Re-reading the final path from the pinned file handle afterwards detects
129+
// that; once the final path is stable across the pinning, no component of it can be
130+
// renamed or deleted any longer.
131+
for (int attempt = 0; ; ++attempt)
132+
{
133+
try
134+
{
135+
this.PinAncestors(finalPath);
136+
137+
string currentPath = ProcessorPathIntegrity.GetFinalPath(file.ContentHandle);
138+
if (string.Equals(currentPath, finalPath, StringComparison.OrdinalIgnoreCase))
139+
{
140+
break;
141+
}
142+
143+
finalPath = currentPath;
144+
}
145+
catch (DscProcessorPathChangedException) when (attempt + 1 < MaximumPinAttempts)
146+
{
147+
// A component of the path was replaced with a reparse point while the
148+
// ancestors were being pinned; start over from where the file is now.
149+
finalPath = ProcessorPathIntegrity.GetFinalPath(file.ContentHandle);
150+
}
151+
152+
this.ReleaseDirectoryHandles();
153+
154+
if (attempt + 1 >= MaximumPinAttempts)
155+
{
156+
throw new DscProcessorPathChangedException();
157+
}
158+
}
159+
160+
this.boundPath = path;
161+
this.boundIsAlias = isAlias;
162+
this.pinnedFile = file;
163+
this.launchPath = finalPath;
164+
}
165+
catch
166+
{
167+
this.ReleaseDirectoryHandles();
168+
file.Dispose();
169+
throw;
170+
}
171+
}
172+
173+
private void PinAncestors(string finalPath)
174+
{
175+
// Stop below the root; a volume root cannot be renamed or deleted, and holding a handle
176+
// to it would interfere with operations such as dismounting the volume.
177+
for (string? directory = Path.GetDirectoryName(finalPath);
178+
directory != null && Path.GetDirectoryName(directory) != null;
179+
directory = Path.GetDirectoryName(directory))
180+
{
181+
this.directoryHandles.Add(ProcessorPathIntegrity.PinDirectory(directory));
182+
}
183+
}
184+
185+
private void ReleaseDirectoryHandles()
186+
{
187+
foreach (SafeFileHandle directoryHandle in this.directoryHandles)
188+
{
189+
directoryHandle.Dispose();
190+
}
191+
192+
this.directoryHandles.Clear();
193+
}
194+
}
195+
}

0 commit comments

Comments
 (0)