From 9a213487841961da89c2467a0f9f9df4414f7596 Mon Sep 17 00:00:00 2001 From: Jeffrey Jangli Date: Wed, 1 Jan 2020 19:35:00 +0100 Subject: [PATCH 1/3] Improved unit test. (#90) Thanks. --- ...entBlockTest_And_DestroyEnvironmentBlockTest.cs | 53 ++++++++++++++++++++++ UnitTests/PInvoke/UserEnv/UserEnv.csproj | 1 + UnitTests/PInvoke/UserEnv/UserEnvTests.cs | 16 ++----- 3 files changed, 57 insertions(+), 13 deletions(-) create mode 100644 UnitTests/PInvoke/UserEnv/CreateEnvironmentBlockTest_And_DestroyEnvironmentBlockTest.cs diff --git a/UnitTests/PInvoke/UserEnv/CreateEnvironmentBlockTest_And_DestroyEnvironmentBlockTest.cs b/UnitTests/PInvoke/UserEnv/CreateEnvironmentBlockTest_And_DestroyEnvironmentBlockTest.cs new file mode 100644 index 00000000..c3f12bf5 --- /dev/null +++ b/UnitTests/PInvoke/UserEnv/CreateEnvironmentBlockTest_And_DestroyEnvironmentBlockTest.cs @@ -0,0 +1,53 @@ +using NUnit.Framework; +using System; +using static Vanara.PInvoke.AdvApi32; +using static Vanara.PInvoke.Kernel32; +using static Vanara.PInvoke.UserEnv; + +namespace Vanara.PInvoke.Tests +{ + public partial class UserEnvTests + { + [Test] + public void CreateEnvironmentBlockTest_And_DestroyEnvironmentBlockTest() + { + SafeHTOKEN hToken; + + using (hToken = SafeHTOKEN.FromProcess(GetCurrentProcess(), TokenAccess.TOKEN_IMPERSONATE | TokenAccess.TOKEN_DUPLICATE | TokenAccess.TOKEN_READ).Duplicate(SECURITY_IMPERSONATION_LEVEL.SecurityImpersonation)) + { + Assert.IsFalse(hToken.IsClosed); + + Assert.That(CreateEnvironmentBlock(out var environmentBlock, hToken, false), ResultIs.Successful); + + + // Test all environment variables. + + var allEnvironmentVariables = Environment.GetEnvironmentVariables(); + + foreach (var envVar in environmentBlock) + { + var envVarName = envVar.Split('=')[0]; + + if (allEnvironmentVariables.Contains(envVarName)) + { + var envVarValue = Environment.GetEnvironmentVariable(envVarName); + + Assert.AreEqual(allEnvironmentVariables[envVarName], envVarValue); + + TestContext.WriteLine(envVar); + } + + else + { + TestContext.WriteLine(); + TestContext.WriteLine($"*** UNAVAILABLE: {envVar}"); + TestContext.WriteLine(); + } + } + } + + + Assert.IsTrue(hToken.IsClosed); + } + } +} diff --git a/UnitTests/PInvoke/UserEnv/UserEnv.csproj b/UnitTests/PInvoke/UserEnv/UserEnv.csproj index bfce3dab..893a30e0 100644 --- a/UnitTests/PInvoke/UserEnv/UserEnv.csproj +++ b/UnitTests/PInvoke/UserEnv/UserEnv.csproj @@ -44,6 +44,7 @@ + diff --git a/UnitTests/PInvoke/UserEnv/UserEnvTests.cs b/UnitTests/PInvoke/UserEnv/UserEnvTests.cs index dc151897..5e7cdf16 100644 --- a/UnitTests/PInvoke/UserEnv/UserEnvTests.cs +++ b/UnitTests/PInvoke/UserEnv/UserEnvTests.cs @@ -1,19 +1,9 @@ using NUnit.Framework; -using static Vanara.PInvoke.AdvApi32; -using static Vanara.PInvoke.Kernel32; -using static Vanara.PInvoke.UserEnv; namespace Vanara.PInvoke.Tests { - public class UserEnvTests + [TestFixture()] + public partial class UserEnvTests { - [Test] - public void CreateDestroyEnvironmentBlockTest() - { - using var hTok = SafeHTOKEN.FromProcess(GetCurrentProcess(), TokenAccess.TOKEN_IMPERSONATE | TokenAccess.TOKEN_DUPLICATE | TokenAccess.TOKEN_READ).Duplicate(SECURITY_IMPERSONATION_LEVEL.SecurityImpersonation); - Assert.That(CreateEnvironmentBlock(out var env, hTok, false), ResultIs.Successful); - Assert.That(env, Has.Exactly(1).StartsWith("Path=")); - TestContext.Write(string.Join("\r\n", env)); - } } -} \ No newline at end of file +} From a3fb9986991ff1234f7fe51633a5071937a90472 Mon Sep 17 00:00:00 2001 From: Jeffrey Jangli Date: Wed, 1 Jan 2020 23:41:44 +0100 Subject: [PATCH 2/3] Fixed memory leaks when using WindowsIdentity.GetCurrent() (#91) --- BITS/BackgroundCopyManager.cs | 5 ++++- PInvoke/Security/AdvApi32/PSID.cs | 11 +++++++++- Security/AccessControl/SystemSecurity.cs | 25 ++++++++++++++++++++-- UnitTests/BITS/JobPropTest.cs | 5 ++++- UnitTests/PInvoke/NetApi32/NetApi32Tests.cs | 5 ++++- UnitTests/PInvoke/Security/AdvApi32/AuditTests.cs | 23 +++++++++++++++++--- .../Security/AccessControl/SystemSecurityTests.cs | 6 +++++- 7 files changed, 70 insertions(+), 10 deletions(-) diff --git a/BITS/BackgroundCopyManager.cs b/BITS/BackgroundCopyManager.cs index 06cb089c..2a25001b 100644 --- a/BITS/BackgroundCopyManager.cs +++ b/BITS/BackgroundCopyManager.cs @@ -134,7 +134,10 @@ namespace Vanara.IO /// Checks if the current user has administrator rights. internal static bool IsCurrentUserAdministrator() { - var wp = new WindowsPrincipal(WindowsIdentity.GetCurrent()); + using var identity = WindowsIdentity.GetCurrent(); + + var wp = new WindowsPrincipal(identity); + return wp.IsInRole(WindowsBuiltInRole.Administrator); } diff --git a/PInvoke/Security/AdvApi32/PSID.cs b/PInvoke/Security/AdvApi32/PSID.cs index 239b0c17..f1c389d9 100644 --- a/PInvoke/Security/AdvApi32/PSID.cs +++ b/PInvoke/Security/AdvApi32/PSID.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; using System.Runtime.InteropServices; +using System.Security.Principal; using Vanara.Extensions; using Vanara.InteropServices; using static Vanara.PInvoke.Kernel32; @@ -54,7 +55,15 @@ namespace Vanara.PInvoke /// Gets the SID for the current user /// The current user's SID. - public static SafePSID Current => new SafePSID(System.Security.Principal.WindowsIdentity.GetCurrent().User); + public static SafePSID Current + { + get + { + using var identity = WindowsIdentity.GetCurrent(); + + return new SafePSID(identity.User); + } + } /// A SID representing the Everyone Group (S-1-1-0). public static SafePSID Everyone => CreateWellKnown(WELL_KNOWN_SID_TYPE.WinWorldSid); diff --git a/Security/AccessControl/SystemSecurity.cs b/Security/AccessControl/SystemSecurity.cs index 4869aa3b..1c906e87 100644 --- a/Security/AccessControl/SystemSecurity.cs +++ b/Security/AccessControl/SystemSecurity.cs @@ -308,7 +308,17 @@ namespace Vanara.Security.AccessControl /// Name of the user. public AccountPrivileges(SystemSecurity parent, string userName = null) { - ctrl = parent; user = userName ?? WindowsIdentity.GetCurrent().Name; + ctrl = parent; + + if (!string.IsNullOrEmpty(userName)) + user = userName; + + else + { + using var identity = WindowsIdentity.GetCurrent(); + + user = identity.Name; + } } /// Gets or sets the enablement of the specified privilege. @@ -356,7 +366,18 @@ namespace Vanara.Security.AccessControl /// Name of the user. public LogonRights(SystemSecurity parent, string userName = null) { - ctrl = parent; user = userName ?? WindowsIdentity.GetCurrent().Name; + ctrl = parent; + + if (!string.IsNullOrEmpty(userName)) + user = userName; + + else + { + using var identity = WindowsIdentity.GetCurrent(); + + user = identity.Name; + } + } /// Gets the logon rights for the current user. diff --git a/UnitTests/BITS/JobPropTest.cs b/UnitTests/BITS/JobPropTest.cs index 7d69a794..e4d9c33e 100644 --- a/UnitTests/BITS/JobPropTest.cs +++ b/UnitTests/BITS/JobPropTest.cs @@ -1,5 +1,6 @@ using NUnit.Framework; using System; +using System.Security.Principal; namespace Vanara.IO.Tests { @@ -89,7 +90,9 @@ namespace Vanara.IO.Tests Assert.That(() => job.OnDemand = true, Throws.Nothing); Assert.That(job.OnDemand, Is.EqualTo(true)); - Assert.That(job.Owner, Is.EqualTo(System.Security.Principal.WindowsIdentity.GetCurrent().User)); + + using var identity = WindowsIdentity.GetCurrent(); + Assert.That(job.Owner, Is.EqualTo(identity.User)); Assert.That(job.OwnerIntegrityLevel, Is.EqualTo(8192)); diff --git a/UnitTests/PInvoke/NetApi32/NetApi32Tests.cs b/UnitTests/PInvoke/NetApi32/NetApi32Tests.cs index 0fad69f1..4c3fc0aa 100644 --- a/UnitTests/PInvoke/NetApi32/NetApi32Tests.cs +++ b/UnitTests/PInvoke/NetApi32/NetApi32Tests.cs @@ -149,7 +149,10 @@ namespace Vanara.PInvoke.Tests Assert.That(() => e.First(i => i.lgrpi0_name == val), Throws.Nothing); var info = NetLocalGroupGetInfo(null, val); Assert.That(info.lgrpi1_name, Is.EqualTo(val)); - var sidmem = new SafeHGlobalHandle(System.Security.Principal.WindowsIdentity.GetCurrent().User.GetBytes()); + + using var identity = WindowsIdentity.GetCurrent(); + var sidmem = new SafeHGlobalHandle(identity.User.GetBytes()); + NetLocalGroupAddMembers(null, val, new[] { new LOCALGROUP_MEMBERS_INFO_0 { lgrmi0_sid = (IntPtr)sidmem } }); var m = NetLocalGroupGetMembers(null, val); Assert.That(m, Is.Not.Empty); diff --git a/UnitTests/PInvoke/Security/AdvApi32/AuditTests.cs b/UnitTests/PInvoke/Security/AdvApi32/AuditTests.cs index 4ed9c03a..40f1679f 100644 --- a/UnitTests/PInvoke/Security/AdvApi32/AuditTests.cs +++ b/UnitTests/PInvoke/Security/AdvApi32/AuditTests.cs @@ -17,7 +17,21 @@ namespace Vanara.PInvoke.Tests public static IEnumerable Categories => AuditEnumerateCategories(); - public static SafePSID CurUserSid => pCurSid ?? (pCurSid = new SafePSID(WindowsIdentity.GetCurrent().User.GetBytes())); + + public static SafePSID CurUserSid + { + get + { + if (null != pCurSid) + return pCurSid; + + + using var identity = WindowsIdentity.GetCurrent(); + + return pCurSid = new SafePSID(identity.User.GetBytes()); + } + } + public static IEnumerable PerUserPolicy => AuditEnumeratePerUserPolicy(); @@ -46,8 +60,11 @@ namespace Vanara.PInvoke.Tests [Test()] public void AuditComputeEffectivePolicyByTokenTest() { - using (var hTok = new SafeHTOKEN(WindowsIdentity.GetCurrent().Token)) - Assert.That(AuditComputeEffectivePolicyByToken(hTok, new[] { regAudit }), Is.Not.Empty); + using var identity = WindowsIdentity.GetCurrent(); + + using var hTok = new SafeHTOKEN(identity.Token); + + Assert.That(AuditComputeEffectivePolicyByToken(hTok, new[] { regAudit }), Is.Not.Empty); } [Test] diff --git a/UnitTests/Security/AccessControl/SystemSecurityTests.cs b/UnitTests/Security/AccessControl/SystemSecurityTests.cs index 179438c3..b5834d34 100644 --- a/UnitTests/Security/AccessControl/SystemSecurityTests.cs +++ b/UnitTests/Security/AccessControl/SystemSecurityTests.cs @@ -62,7 +62,11 @@ namespace Vanara.Security.AccessControl.Tests using (ss = new SystemSecurity(SystemSecurity.DesiredAccess.LookupNames)) { IList sa = null; - Assert.That(() => sa = ss.GetAccountInfo(false, false, WindowsIdentity.GetCurrent().User, new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null)), Throws.Nothing); + + using var identity = WindowsIdentity.GetCurrent(); + + Assert.That(() => sa = ss.GetAccountInfo(false, false, identity.User, new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null)), Throws.Nothing); + foreach (var sai in sa) TestContext.WriteLine($"{sai.SidType}:{sai.Name}"); } From ce4b71e33cc826f586793db4b35c9db7bbc4c431 Mon Sep 17 00:00:00 2001 From: Jeffrey Jangli Date: Thu, 2 Jan 2020 04:55:22 +0100 Subject: [PATCH 3/3] Added WellKnownSidType.ServiceSid to method AccountUtils.IsServiceAccount (#96) Thanks --- Security/AccountUtils.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Security/AccountUtils.cs b/Security/AccountUtils.cs index 5a1c7fcc..f92ce676 100644 --- a/Security/AccountUtils.cs +++ b/Security/AccountUtils.cs @@ -8,19 +8,22 @@ namespace Vanara.Security { public static bool IsAdmin(this WindowsIdentity id) => new WindowsPrincipal(id).IsInRole(WindowsBuiltInRole.Administrator); + public static bool IsServiceAccount(this WindowsIdentity id) { try { var acct = new NTAccount(id.Name); - var si = (SecurityIdentifier)acct.Translate(typeof(SecurityIdentifier)); - return (si.IsWellKnown(WellKnownSidType.LocalSystemSid) || si.IsWellKnown(WellKnownSidType.NetworkServiceSid) || - si.IsWellKnown(WellKnownSidType.LocalServiceSid)); + + var si = (SecurityIdentifier) acct.Translate(typeof(SecurityIdentifier)); + + return si.IsWellKnown(WellKnownSidType.LocalSystemSid) || si.IsWellKnown(WellKnownSidType.NetworkServiceSid) || si.IsWellKnown(WellKnownSidType.LocalServiceSid) || si.IsWellKnown(WellKnownSidType.ServiceSid); } catch { } return false; } + /// Runs the specified function as the impersonated Windows identity. /// The impersonated identity under which to run the function. /// The System.Func to run.