Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -83,17 +83,17 @@
<GenerateProgramFile>false</GenerateProgramFile>
</PropertyGroup>

<PropertyGroup>
<ImplicitUsings>enable</ImplicitUsings>
</PropertyGroup>

<!-- Disable NuGet audit in official builds. MSBuild does not correctly handle
WarningsNotAsErrors for NuGet warnings (https://github.com/dotnet/msbuild/issues/10801),
so audit findings become errors via TreatWarningsAsErrors. -->
<PropertyGroup Condition="'$(OfficialBuild)' == 'true'">
<NuGetAudit>false</NuGetAudit>
</PropertyGroup>

<PropertyGroup>
<ImplicitUsings>enable</ImplicitUsings>
</PropertyGroup>

<!-- Global usings -->
<!-- See: https://learn.microsoft.com/dotnet/core/project-sdk/msbuild-props#using -->
<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,27 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.CommandLine;

namespace Microsoft.DotNet.Cli.Commands.Workload.Elevate;

internal sealed class WorkloadElevateCommandDefinition : WorkloadCommandDefinitionBase
{
/// <summary>
/// Optional, hidden argument supplied by the unelevated client at server launch with the value of
/// the client's <see cref="System.IO.Path.GetTempPath"/>. Used by the elevated server to accept
/// IPC-supplied paths that originate from the client's temp directory when it differs from the
/// server's (e.g., over-the-shoulder UAC, custom TEMP env vars).
/// </summary>
public readonly Option<string> ClientTempOption = new("--client-temp")
{
Hidden = true
};

public WorkloadElevateCommandDefinition()
: base("elevate", CommandDefinitionStrings.WorkloadElevateCommandDescription)
{
Hidden = true;
Options.Add(ClientTempOption);
}
}
19 changes: 19 additions & 0 deletions src/Cli/dotnet/Commands/Workload/Elevate/WorkloadElevateCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.CommandLine;
using Microsoft.DotNet.Cli.Commands.Workload.Install;
using Microsoft.DotNet.Cli.Installer.Windows;
using Microsoft.DotNet.Cli.Utils;

namespace Microsoft.DotNet.Cli.Commands.Workload.Elevate;
Expand All @@ -14,6 +15,24 @@ public override int Execute()
{
if (OperatingSystem.IsWindows())
{
// Capture the unelevated client's temp directory (if supplied) so path validators
// can accept IPC-supplied paths that originate from it. Optional and ignored when null
// or unparseable; in either case validators fall back to the server's own temp.
// Use the inherited _parseResult field rather than the primary constructor parameter to avoid
// CS9107 (capturing the parameter into the type's state on top of the base ctor passthrough).
string? clientTemp = _parseResult.GetValue(Definition.ClientTempOption);
if (!string.IsNullOrWhiteSpace(clientTemp))
{
try
{
InstallerBase.TrustedClientTempDirectory = Path.GetFullPath(clientTemp);
}
catch
{
// Ignore malformed values.
}
}

NetSdkMsiInstallerServer? server = null;

try
Expand Down
9 changes: 5 additions & 4 deletions src/Cli/dotnet/Commands/Workload/Install/MsiInstallerBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -146,18 +146,19 @@ internal static string GetDotNetHome()
/// <param name="logFile">The path of the log file.</param>
protected void ConfigureInstall(string logFile)
{
string validatedLogFile = WindowsUtils.ValidateLogFilePath(logFile);

// Turn off the MSI UI.
_ = WindowsInstaller.SetInternalUI(InstallUILevel.None);

// The log file must be created before calling MsiEnableLog and we should avoid having active handles
// against it.
FileStream logFileStream = File.Create(logFile);
FileStream logFileStream = File.Create(validatedLogFile);
logFileStream.Close();
uint error = WindowsInstaller.EnableLog(InstallLogMode.DEFAULT | InstallLogMode.VERBOSE, logFile, InstallLogAttributes.NONE);
uint error = WindowsInstaller.EnableLog(InstallLogMode.DEFAULT | InstallLogMode.VERBOSE, validatedLogFile, InstallLogAttributes.NONE);

// We can report issues with the log file creation, but shouldn't fail the workload operation.
LogError(error, $"Failed to configure log file: {logFile}");
LogError(error, $"Failed to configure log file: {validatedLogFile}");
}

/// <summary>
Expand Down Expand Up @@ -312,7 +313,7 @@ public Dictionary<string, string> GetGlobalJsonWorkloadSetVersions(SdkFeatureBan
protected uint InstallMsi(string packagePath, string logFile)
{
// Make sure the package we're going to run is coming from the cache.
if (!packagePath.StartsWith(Cache.PackageCacheRoot))
if (!WindowsUtils.ValidatePackagePath(packagePath, Cache.PackageCacheRoot))
{
return Error.INSTALL_PACKAGE_INVALID;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,18 +174,12 @@ public static NetSdkMsiInstallerServer Create(bool verifySignatures)
throw new SecurityException(string.Format(CliCommandStrings.NoTrustWithParentPID, ParentProcess?.Id));
}

// Configure pipe DACLs
SecurityIdentifier authenticatedUserIdentifier = new(WellKnownSidType.AuthenticatedUserSid, null);
// Configure pipe DACLs. GetPipeClientIdentifier resolves the parent process's user SID
// and will throw SecurityException if the token cannot be read, preventing the server
// from starting with an insecure configuration.
SecurityIdentifier clientIdentifier = WindowsUtils.GetPipeClientIdentifier();
SecurityIdentifier currentOwnerIdentifier = WindowsIdentity.GetCurrent().Owner;
PipeSecurity pipeSecurity = new();

// The current user has full control and should be running as Administrator.
pipeSecurity.SetOwner(currentOwnerIdentifier);
pipeSecurity.AddAccessRule(new PipeAccessRule(currentOwnerIdentifier, PipeAccessRights.FullControl, AccessControlType.Allow));

// Restrict read/write access to authenticated users
pipeSecurity.AddAccessRule(new PipeAccessRule(authenticatedUserIdentifier,
PipeAccessRights.Read | PipeAccessRights.Write | PipeAccessRights.Synchronize, AccessControlType.Allow));
PipeSecurity pipeSecurity = WindowsUtils.CreatePipeSecurity(currentOwnerIdentifier, clientIdentifier);

// Initialize the named pipe for dispatching commands. The name of the pipe is based off the server PID since
// the client knows this value and ensures both processes can generate the same name.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,16 @@ public override void Elevate()
{
if (!IsElevated && !HasElevated)
{
// Pass the unelevated client's temp directory to the elevated server so it can validate
// IPC-supplied paths (e.g., the workload pack manifest extracted by the client) against it.
// Quoted to handle profile paths that contain spaces. Optional on the server side; if
// omitted or unparseable, the server falls back to its own Path.GetTempPath().
string clientTemp = Path.GetFullPath(Path.GetTempPath()).TrimEnd(Path.DirectorySeparatorChar);

// Use the path of the current host, otherwise we risk resolving against the wrong SDK version.
// To trigger UAC, UseShellExecute must be true and Verb must be "runas".
ProcessStartInfo startInfo = new($@"""{Environment.ProcessPath}""",
$@"""{Assembly.GetExecutingAssembly().Location}"" workload elevate")
$@"""{Assembly.GetExecutingAssembly().Location}"" workload elevate --client-temp ""{clientTemp}""")
{
Verb = "runas",
UseShellExecute = true,
Expand Down
9 changes: 9 additions & 0 deletions src/Cli/dotnet/Installer/Windows/InstallerBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,15 @@ protected InstallElevationContextBase ElevationContext
/// </summary>
public static readonly Process ParentProcess;

/// <summary>
/// The fully-qualified path of the unelevated client's temp directory, as supplied at server launch
/// via the <c>--client-temp</c> argument. Used by path validators to accept manifest/log paths that
/// originate from the client when the client and server resolve different values for
/// <see cref="Path.GetTempPath"/> (e.g., over-the-shoulder UAC, custom TEMP env vars).
/// <see langword="null"/> if not supplied.
/// </summary>
public static string TrustedClientTempDirectory { get; set; }

/// <summary>
/// Gets the processor architecture.
/// </summary>
Expand Down
35 changes: 29 additions & 6 deletions src/Cli/dotnet/Installer/Windows/MsiPackageCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,34 @@ internal class MsiPackageCache(InstallElevationContextBase elevationContext, ISe
/// <param name="manifestPath">The JSON manifest associated with the workload pack MSI.</param>
public void CachePayload(string packageId, string packageVersion, string manifestPath)
{
if (!File.Exists(manifestPath))
// Validate that packageId and packageVersion do not contain path traversal characters
// to prevent an IPC client from constructing paths outside the package cache.
if (!WindowsUtils.ValidatePathComponent(packageId))
{
throw new ArgumentException($"Invalid package ID: {packageId}");
}

if (!WindowsUtils.ValidatePathComponent(packageVersion))
{
throw new FileNotFoundException($"CachePayload: Manifest file not found: {manifestPath}");
throw new ArgumentException($"Invalid package version: {packageVersion}");
}

// Validate that the manifest path resolves to a location under the elevated server's temp
// directory or the unelevated client's temp directory (when supplied at server launch via
// --client-temp). This prevents an IPC client from coercing the elevated server into reading
// or moving arbitrary files.
string fullManifestPath = Path.GetFullPath(manifestPath);

if (!WindowsUtils.ValidateManifestPath(fullManifestPath))
{
throw new ArgumentException($"CachePayload: Manifest path is not under an allowed temp directory: {manifestPath}");
}

if (!File.Exists(fullManifestPath))
{
throw new FileNotFoundException($"CachePayload: Manifest file not found: {fullManifestPath}");
}

Elevate();

if (IsElevated)
Expand All @@ -63,14 +86,14 @@ public void CachePayload(string packageId, string packageVersion, string manifes

// We cannot assume that the MSI adjacent to the manifest is the one to cache. We'll trust
// the manifest to provide the MSI filename.
MsiManifest msiManifest = JsonConvert.DeserializeObject<MsiManifest>(File.ReadAllText(manifestPath));
MsiManifest msiManifest = JsonConvert.DeserializeObject<MsiManifest>(File.ReadAllText(fullManifestPath));
// Only use the filename+extension of the payload property in case the manifest has been altered.
string msiPath = Path.Combine(Path.GetDirectoryName(manifestPath), Path.GetFileName(msiManifest.Payload));
string msiPath = Path.Combine(Path.GetDirectoryName(fullManifestPath), Path.GetFileName(msiManifest.Payload));

string cachedMsiPath = Path.Combine(packageDirectory, Path.GetFileName(msiPath));
string cachedManifestPath = Path.Combine(packageDirectory, Path.GetFileName(manifestPath));
string cachedManifestPath = Path.Combine(packageDirectory, Path.GetFileName(fullManifestPath));

SecurityUtils.MoveAndSecureFile(manifestPath, cachedManifestPath, Log);
SecurityUtils.MoveAndSecureFile(fullManifestPath, cachedManifestPath, Log);
SecurityUtils.MoveAndSecureFile(msiPath, cachedMsiPath, Log);
}
else if (IsClient)
Expand Down
6 changes: 6 additions & 0 deletions src/Cli/dotnet/Installer/Windows/NativeMethods.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#nullable disable

using System.Runtime.Versioning;
using Microsoft.Win32.SafeHandles;

namespace Microsoft.DotNet.Cli.Installer.Windows;

Expand All @@ -13,4 +14,9 @@ internal class NativeMethods
[DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)]
[DefaultDllImportSearchPaths(DllImportSearchPath.System32)]
public static extern uint FormatMessage(uint dwFlags, nint lpSource, uint dwMessageId, uint dwLanguageId, StringBuilder lpBuffer, uint nSize, nint Arguments);

[DllImport("advapi32.dll", SetLastError = true)]
[DefaultDllImportSearchPaths(DllImportSearchPath.System32)]
[return: MarshalAs(UnmanagedType.Bool)]
public static extern bool OpenProcessToken(nint processHandle, uint desiredAccess, out SafeAccessTokenHandle tokenHandle);
}
Loading
Loading