Skip to content

Commit

Permalink
Consolidate approach for conditional tests that check for privileged …
Browse files Browse the repository at this point in the history
…processes (dotnet#78793)

* Consolidate approach for conditional tests that check for privileged processes

* Simpler privileged process helpers

* Revert SLN file changes

* Remove platform-specific helpers for privileged process conditions

* Fix System.IO tests for unix-specific privileged process scenarios

* Browser does not support privileged processes

* Fix System.IO tests for unix-specific privileged process scenarios

* Enable the Environment.IsPrivilegedProcess test for browser

* PR feedback

Omit [PlatformSpecific] in modified tests that are in platform-specific test files.

Remove [Outerloop] attributes from privileged process tests.

Better multithreaded handling in PlatformDetection.IsPrivilegedProcess
  • Loading branch information
jeffhandley authored Nov 30, 2022
1 parent c7460d0 commit 5553691
Show file tree
Hide file tree
Showing 47 changed files with 139 additions and 179 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public static partial class MountHelper
// Reduce the risk we accidentally stop running these altogether
// on Windows, due to a bug in CreateSymbolicLink
if (!success && PlatformDetection.IsWindows)
Assert.True(!PlatformDetection.IsWindowsAndElevated);
Assert.False(PlatformDetection.IsPrivilegedProcess);

return success;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ public static int RunAsSudo(string commandLine)

public static unsafe bool IsProcessElevated()
{
// Browser does not have the concept of an elevated process
if (RuntimeInformation.IsOSPlatform(OSPlatform.Create("BROWSER")))
{
return false;
}

if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
uint userId = Interop.Sys.GetEUid();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,8 @@ namespace System.IO
/// <summary>Base class for test classes the use temporary files that need to be cleaned up.</summary>
public abstract partial class FileCleanupTestBase : IDisposable
{
private static readonly Lazy<bool> s_isElevated = new Lazy<bool>(() => AdminHelpers.IsProcessElevated());

private string fallbackGuid = Guid.NewGuid().ToString("N").Substring(0, 10);

protected static bool IsProcessElevated => s_isElevated.Value;

/// <summary>Initialize the test class base. This creates the associated test directory.</summary>
protected FileCleanupTestBase(string tempDirectory = null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,6 @@ public static partial class PlatformDetection
public static bool IsNotFedoraOrRedHatFamily => !IsFedora && !IsRedHatFamily;
public static bool IsNotDebian10 => !IsDebian10;

public static bool IsSuperUser => IsBrowser || IsWindows ? false : libc.geteuid() == 0;

public static bool IsUnixAndSuperUser => !IsWindows && IsSuperUser;

public static Version OpenSslVersion => !IsOSXLike && !IsWindows && !IsAndroid ?
GetOpenSslVersion() :
throw new PlatformNotSupportedException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,31 +248,8 @@ public static bool IsInAppContainer
}
}

public static bool CanRunImpersonatedTests => PlatformDetection.IsNotWindowsNanoServer && PlatformDetection.IsWindowsAndElevated;
public static bool CanRunImpersonatedTests => PlatformDetection.IsNotWindowsNanoServer && PlatformDetection.IsWindows && PlatformDetection.IsPrivilegedProcess;

public static bool IsWindowsX86OrX64 => PlatformDetection.IsWindows && (PlatformDetection.IsX86Process || PlatformDetection.IsX64Process);

private static int s_isWindowsElevated = -1;
public static bool IsWindowsAndElevated
{
get
{
if (s_isWindowsElevated != -1)
return s_isWindowsElevated == 1;

if (!IsWindows || IsInAppContainer)
{
s_isWindowsElevated = 0;
return false;
}

s_isWindowsElevated = AdminHelpers.IsProcessElevated() ? 1 : 0;

return s_isWindowsElevated == 1;
}
}

public static bool IsWindowsAndNotElevated
=> IsWindows && !IsWindowsAndElevated;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,23 @@ public static partial class PlatformDetection
public static bool Is64BitProcess => IntPtr.Size == 8;
public static bool IsNotWindows => !IsWindows;

private static volatile int s_isPrivilegedProcess = -1;
public static bool IsPrivilegedProcess
{
get
{
int p = s_isPrivilegedProcess;
if (p == -1)
{
s_isPrivilegedProcess = p = AdminHelpers.IsProcessElevated() ? 1 : 0;
}

return p == 1;
}
}

public static bool IsNotPrivilegedProcess => !IsPrivilegedProcess;

public static bool IsMarshalGetExceptionPointersSupported => !IsMonoRuntime && !IsNativeAot;

private static readonly Lazy<bool> s_isCheckedRuntime = new Lazy<bool>(() => AssemblyConfigurationEquals("Checked"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ public sealed partial class WindowsTestAccount : IDisposable

public WindowsTestAccount(string userName)
{
Assert.True(PlatformDetection.IsWindowsAndElevated);
Assert.True(PlatformDetection.IsWindows);
Assert.True(PlatformDetection.IsPrivilegedProcess);

_userName = userName;
Password = GeneratePassword();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ public class ProcessStartInfoTests : ProcessTestBase
private const string ItemSeparator = "CAFF9451396B4EEF8A5155A15BDC2080"; // random string that shouldn't be in any env vars; used instead of newline to separate env var strings

private static bool IsAdmin_IsNotNano_RemoteExecutorIsSupported
=> PlatformDetection.IsWindowsAndElevated && PlatformDetection.IsNotWindowsNanoServer && RemoteExecutor.IsSupported;
=> PlatformDetection.IsWindows && PlatformDetection.IsNotWindowsNanoServer
&& PlatformDetection.IsPrivilegedProcess && RemoteExecutor.IsSupported;

[Fact]
public void TestEnvironmentProperty()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace System.Diagnostics.Tests
{
public partial class ProcessTests : ProcessTestBase
{
private static bool IsRemoteExecutorSupportedAndOnUnixAndSuperUser => RemoteExecutor.IsSupported && PlatformDetection.IsUnixAndSuperUser;
private static bool IsRemoteExecutorSupportedAndPrivilegedProcess => RemoteExecutor.IsSupported && PlatformDetection.IsPrivilegedProcess;

[Fact]
private void TestWindowApisUnix()
Expand Down Expand Up @@ -456,7 +456,7 @@ public void ProcessStart_UseShellExecuteTrue_OpenUrl_SuccessfullyReadsArgumentAr
}
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsUnixAndSuperUser))]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsPrivilegedProcess))]
public void TestPriorityClassUnix()
{
CreateDefaultProcess();
Expand All @@ -478,11 +478,11 @@ public void TestPriorityClassUnix()
}
catch (Win32Exception ex)
{
Assert.True(!PlatformDetection.IsSuperUser, $"Failed even though superuser {ex.ToString()}");
Assert.False(PlatformDetection.IsPrivilegedProcess, $"Failed even though superuser {ex.ToString()}");
}
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsUnixAndSuperUser))]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsPrivilegedProcess))]
public void TestBasePriorityOnUnix()
{
CreateDefaultProcess();
Expand All @@ -499,7 +499,7 @@ public void TestBasePriorityOnUnix()
}
catch (Win32Exception ex)
{
Assert.True(!PlatformDetection.IsSuperUser, $"Failed even though superuser {ex.ToString()}");
Assert.False(PlatformDetection.IsPrivilegedProcess, $"Failed even though superuser {ex.ToString()}");
}
}

Expand Down Expand Up @@ -596,8 +596,7 @@ public void TestCheckChildProcessUserAndGroupIds()
/// Tests when running as root and starting a new process as a normal user,
/// the new process doesn't have elevated privileges.
/// </summary>
[ConditionalTheory(nameof(IsRemoteExecutorSupportedAndOnUnixAndSuperUser))]
[OuterLoop("Needs sudo access")]
[ConditionalTheory(nameof(IsRemoteExecutorSupportedAndPrivilegedProcess))]
[InlineData(true)]
[InlineData(false)]
public unsafe void TestCheckChildProcessUserAndGroupIdsElevated(bool useRootGroups)
Expand Down Expand Up @@ -743,7 +742,6 @@ private static async Task<bool> TryWaitProcessReapedAsync(int pid, int timeoutMs
/// Tests the ProcessWaitState reference count drops to zero.
/// </summary>
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
[PlatformSpecific(TestPlatforms.AnyUnix)] // Test validates Unix implementation
public async Task TestProcessWaitStateReferenceCount()
{
using (var exitedEventSemaphore = new SemaphoreSlim(0, 1))
Expand Down Expand Up @@ -833,7 +831,6 @@ public void TestProcessRecycledPid()
Assert.True(foundRecycled);
}

[PlatformSpecific(TestPlatforms.AnyUnix)]
[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
[InlineData("/dev/stdin", O_RDONLY)]
[InlineData("/dev/stdout", O_WRONLY)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2546,7 +2546,8 @@ public void Start_PassesArgumentsList_WhichGetsEscaped()
}
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindowsAndNotElevated))]
[PlatformSpecific(TestPlatforms.Windows)]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotPrivilegedProcess))]
public void NonElevatedUser_QueryProcessNameOfSystemProcess()
{
const string Services = "services";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;
#if USE_MDT_EVENTSOURCE
using Microsoft.Diagnostics.Tracing;
Expand All @@ -14,7 +15,7 @@ public partial class FuzzyTests
{
static partial void Test_Write_Fuzzy_TestEtw(List<SubTest> tests, EventSource logger)
{
if (TestUtilities.IsProcessElevated)
if (PlatformDetection.IsPrivilegedProcess)
{
using (var listener = new EtwListener())
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@ namespace BasicEventSourceTests
{
public partial class TestEventCounter
{
// Specifies whether the process is elevated or not.
private static bool IsProcessElevated => Environment.IsPrivilegedProcess;

[ConditionalFact(nameof(IsProcessElevated))]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsPrivilegedProcess))]
[ActiveIssue("https://github.com/dotnet/runtime/issues/25035")]
public void Test_Write_Metric_ETW()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ namespace BasicEventSourceTests
{
internal class TestUtilities
{
// Specifies whether the process is elevated or not.
internal static bool IsProcessElevated => Environment.IsPrivilegedProcess;

/// <summary>
/// Confirms that there are no EventSources running.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,8 @@ namespace BasicEventSourceTests
public partial class TestsManifestGeneration
{
// Specifies whether the process is elevated or not.
private static bool IsProcessElevated => Environment.IsPrivilegedProcess;
private static bool IsProcessElevatedAndNotWindowsNanoServerAndRemoteExecutorSupported =>
IsProcessElevated && PlatformDetection.IsNotWindowsNanoServer && RemoteExecutor.IsSupported;
PlatformDetection.IsPrivilegedProcess && PlatformDetection.IsNotWindowsNanoServer && RemoteExecutor.IsSupported;

/// ETW only works with elevated process
[ConditionalFact(nameof(IsProcessElevatedAndNotWindowsNanoServerAndRemoteExecutorSupported))]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public void Test_BadEventSource_MismatchedIds_WithEtwListener()
{
// We expect only one session to be on when running the test but if a ETW session was left
// hanging, it will confuse the EventListener tests.
if (TestUtilities.IsProcessElevated)
if (PlatformDetection.IsPrivilegedProcess)
{
EtwListener.EnsureStopped();
}
Expand All @@ -31,7 +31,7 @@ public void Test_BadEventSource_MismatchedIds_WithEtwListener()

var listenerGenerators = new List<Func<Listener>> { () => new EventListenerListener() };

if (TestUtilities.IsProcessElevated)
if (PlatformDetection.IsPrivilegedProcess)
{
listenerGenerators.Add(() => new EtwListener());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@ namespace BasicEventSourceTests
public partial class TestsWrite
{
// Specifies whether the process is elevated or not.
private static bool IsProcessElevated => Environment.IsPrivilegedProcess;
private static bool IsProcessElevatedAndNotWindowsNanoServer =>
IsProcessElevated && PlatformDetection.IsNotWindowsNanoServer; // ActiveIssue: https://github.com/dotnet/runtime/issues/26197
PlatformDetection.IsPrivilegedProcess && PlatformDetection.IsNotWindowsNanoServer; // ActiveIssue: https://github.com/dotnet/runtime/issues/26197

/// <summary>
/// Tests the EventSource.Write[T] method (can only use the self-describing mechanism).
Expand All @@ -30,7 +29,7 @@ public void Test_Write_T_ETW()

[ActiveIssue("https://github.com/dotnet/runtime/issues/21295", TargetFrameworkMonikers.NetFramework)]
[ActiveIssue("https://github.com/dotnet/runtime/issues/25035")]
[ConditionalFact(nameof(IsProcessElevated))]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsPrivilegedProcess))]
public void Test_Write_T_In_Manifest_Serialization_WithEtwListener()
{
using (var eventListener = new EventListenerListener())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@ namespace BasicEventSourceTests
partial class TestsWriteEvent
{
// Specifies whether the process is elevated or not.
private static bool IsProcessElevated => Environment.IsPrivilegedProcess;
private static bool IsProcessElevatedAndNotWindowsNanoServer =>
IsProcessElevated && PlatformDetection.IsNotWindowsNanoServer; // ActiveIssue: https://github.com/dotnet/runtime/issues/26197
PlatformDetection.IsPrivilegedProcess && PlatformDetection.IsNotWindowsNanoServer; // ActiveIssue: https://github.com/dotnet/runtime/issues/26197

/// <summary>
/// Tests WriteEvent using the manifest based mechanism.
Expand Down Expand Up @@ -84,7 +83,7 @@ public void Test_WriteEvent_ByteArray_SelfDescribing_ETW()

static partial void Test_WriteEvent_AddEtwTests(List<SubTest> tests, EventSourceTest logger)
{
if (!IsProcessElevated)
if (!PlatformDetection.IsPrivilegedProcess)
{
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,7 @@ namespace BasicEventSourceTests
{
public partial class TestsWriteEventToListener
{
// Specifies whether the process is elevated or not.
private static bool IsProcessElevated => Environment.IsPrivilegedProcess;

[ConditionalFact(nameof(IsProcessElevated))]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsPrivilegedProcess))]
public void Test_WriteEvent_TransferEvents()
{
TestUtilities.CheckNoEventSourcesRunning("Start");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public static IEnumerable<object[]> GetFormatsAndSpecialFiles()
}
}

[ConditionalTheory(nameof(IsRemoteExecutorSupportedAndOnUnixAndSuperUser))]
[ConditionalTheory(nameof(IsRemoteExecutorSupportedAndPrivilegedProcess))]
[MemberData(nameof(GetFormatsAndSpecialFiles))]
public void Extract_SpecialFiles(TarEntryFormat format, TarEntryType entryType)
{
Expand All @@ -36,7 +36,7 @@ public void Extract_SpecialFiles(TarEntryFormat format, TarEntryType entryType)
Verify_Extract_SpecialFiles(destination, entry, entryType);
}

[ConditionalTheory(nameof(IsRemoteExecutorSupportedAndOnUnixAndSuperUser))]
[ConditionalTheory(nameof(IsRemoteExecutorSupportedAndPrivilegedProcess))]
[MemberData(nameof(GetFormatsAndSpecialFiles))]
public async Task Extract_SpecialFiles_Async(TarEntryFormat format, TarEntryType entryType)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace System.Formats.Tar.Tests
{
public partial class TarFile_ExtractToDirectory_File_Tests : TarTestsBase
{
[ConditionalFact(nameof(IsUnixButNotSuperUser))]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotPrivilegedProcess))]
public void Extract_SpecialFiles_Unix_Unelevated_ThrowsUnauthorizedAccess()
{
string originalFileName = GetTarFilePath(CompressionMethod.Uncompressed, TestTarFormat.ustar, "specialfiles");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace System.Formats.Tar.Tests
{
public partial class TarFile_ExtractToDirectoryAsync_File_Tests : TarTestsBase
{
[ConditionalFact(nameof(IsUnixButNotSuperUser))]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotPrivilegedProcess))]
public async Task Extract_SpecialFiles_Unix_Unelevated_ThrowsUnauthorizedAccess_Async()
{
using (TempDirectory root = new TempDirectory())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ namespace System.Formats.Tar.Tests
{
public partial class TarReader_TarEntry_ExtractToFile_Tests : TarTestsBase
{
[PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.tvOS)] // https://github.com/dotnet/runtime/issues/68360
[ConditionalFact(nameof(IsUnixButNotSuperUser), nameof(IsNotLinuxBionic))]
[SkipOnPlatform(TestPlatforms.tvOS, "https://github.com/dotnet/runtime/issues/68360")]
[SkipOnPlatform(TestPlatforms.LinuxBionic, "Not supported on Bionic")]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotPrivilegedProcess))]
public void SpecialFile_Unelevated_Throws()
{
using TempDirectory root = new TempDirectory();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ namespace System.Formats.Tar.Tests
{
public partial class TarReader_TarEntry_ExtractToFileAsync_Tests : TarTestsBase
{
[PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.tvOS)] // https://github.com/dotnet/runtime/issues/68360
[ConditionalFact(nameof(IsUnixButNotSuperUser), nameof(IsNotLinuxBionic))]
[SkipOnPlatform(TestPlatforms.LinuxBionic, "Unsupported on Bionic")]
[SkipOnPlatform(TestPlatforms.tvOS, "https://github.com/dotnet/runtime/issues/68360")]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotPrivilegedProcess))]
public async Task SpecialFile_Unelevated_Throws_Async()
{
using (TempDirectory root = new TempDirectory())
Expand Down
Loading

0 comments on commit 5553691

Please sign in to comment.