From 4646be2e449fe09673107b2ba8a8a5fe4f676250 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 16 Dec 2020 14:41:23 +0100 Subject: [PATCH 1/6] try to re-enable TestCheckChildProcessUserAndGroupIds test --- .../System.Diagnostics.Process/tests/ProcessTests.Unix.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index 99272c05f0c2e3..aacc6c2becd7b6 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -541,7 +541,6 @@ private static int CheckUserAndGroupIds(string userId, string groupId, string gr } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/28922", TestPlatforms.AnyUnix)] public unsafe void TestCheckChildProcessUserAndGroupIds() { string userName = GetCurrentRealUserName(); From eba057f6eea859a8f4167cb06ec487c22975eeb3 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 18 Dec 2020 10:54:57 +0100 Subject: [PATCH 2/6] lets see if id -G returns the same info as getgroups method from libc --- .../tests/ProcessTests.Unix.cs | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index aacc6c2becd7b6..6c4ea1b602fc53 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -12,6 +12,7 @@ using System.Threading.Tasks; using System.Security; using Xunit; +using Xunit.Sdk; using Microsoft.DotNet.RemoteExecutor; using Microsoft.DotNet.XUnitExtensions; @@ -546,7 +547,16 @@ public unsafe void TestCheckChildProcessUserAndGroupIds() string userName = GetCurrentRealUserName(); string userId = GetUserId(userName); string userGroupId = GetUserGroupId(userName); - string userGroupIds = GetUserGroupIds(userName); + string userGroupIds = GetUserGroupIdsJoined(userName); + + HashSet groupIdsFromIdGCommand = new HashSet(GetUserGroupIds(userName)); + HashSet groupIdsFromLibc = GetGroups(); + + if (!groupIdsFromIdGCommand.SetEquals(groupIdsFromLibc)) + { + throw new XunitException($"id -G returned: {string.Join(", ", groupIdsFromIdGCommand)}{Environment.NewLine}getgroups returned: {string.Join(", ", groupIdsFromLibc)}"); + } + // If this test runs as the user, we expect to be able to match the user groups exactly. // Except on OSX, where getgrouplist may return a list of groups truncated to NGROUPS_MAX. bool checkGroupsExact = userId == geteuid().ToString() && @@ -581,7 +591,7 @@ public unsafe void TestCheckChildProcessUserAndGroupIdsElevated(bool useRootGrou string userId = GetUserId(username); string userGroupId = GetUserGroupId(username); - string userGroupIds = GetUserGroupIds(username); + string userGroupIds = GetUserGroupIdsJoined(username); if (bool.Parse(useRootGroupsArg)) { @@ -616,11 +626,16 @@ private static string GetUserId(string username) private static string GetUserGroupId(string username) => StartAndReadToEnd("id", new[] { "-g", username }).Trim('\n'); - private static string GetUserGroupIds(string username) + private static IEnumerable GetUserGroupIds(string username) { string[] groupIds = StartAndReadToEnd("id", new[] { "-G", username }) .Split(new[] { ' ', '\n' }, StringSplitOptions.RemoveEmptyEntries); - return string.Join(",", groupIds.Select(s => uint.Parse(s)).OrderBy(id => id)); + return groupIds.Select(s => uint.Parse(s)).OrderBy(id => id); + } + + private static string GetUserGroupIdsJoined(string username) + { + return string.Join(",", GetUserGroupIds(username)); } private static string GetCurrentRealUserName() From d0371c98a1ea82a3df7a0edfd4f026c39171baf5 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 18 Dec 2020 13:13:35 +0100 Subject: [PATCH 3/6] use one way of getting expected and actual data in both processes --- .../tests/ProcessTests.Unix.cs | 24 +++---------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index 6c4ea1b602fc53..7758b624c0eca9 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -547,15 +547,7 @@ public unsafe void TestCheckChildProcessUserAndGroupIds() string userName = GetCurrentRealUserName(); string userId = GetUserId(userName); string userGroupId = GetUserGroupId(userName); - string userGroupIds = GetUserGroupIdsJoined(userName); - - HashSet groupIdsFromIdGCommand = new HashSet(GetUserGroupIds(userName)); - HashSet groupIdsFromLibc = GetGroups(); - - if (!groupIdsFromIdGCommand.SetEquals(groupIdsFromLibc)) - { - throw new XunitException($"id -G returned: {string.Join(", ", groupIdsFromIdGCommand)}{Environment.NewLine}getgroups returned: {string.Join(", ", groupIdsFromLibc)}"); - } + string userGroupIds = GetGroupIds(); // If this test runs as the user, we expect to be able to match the user groups exactly. // Except on OSX, where getgrouplist may return a list of groups truncated to NGROUPS_MAX. @@ -591,7 +583,7 @@ public unsafe void TestCheckChildProcessUserAndGroupIdsElevated(bool useRootGrou string userId = GetUserId(username); string userGroupId = GetUserGroupId(username); - string userGroupIds = GetUserGroupIdsJoined(username); + string userGroupIds = GetGroupIds(); if (bool.Parse(useRootGroupsArg)) { @@ -626,17 +618,7 @@ private static string GetUserId(string username) private static string GetUserGroupId(string username) => StartAndReadToEnd("id", new[] { "-g", username }).Trim('\n'); - private static IEnumerable GetUserGroupIds(string username) - { - string[] groupIds = StartAndReadToEnd("id", new[] { "-G", username }) - .Split(new[] { ' ', '\n' }, StringSplitOptions.RemoveEmptyEntries); - return groupIds.Select(s => uint.Parse(s)).OrderBy(id => id); - } - - private static string GetUserGroupIdsJoined(string username) - { - return string.Join(",", GetUserGroupIds(username)); - } + private static string GetGroupIds() => string.Join(",", GetGroups()); private static string GetCurrentRealUserName() { From b280668b589bab61dba36835eb98425837b3d3ff Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 18 Dec 2020 15:08:28 +0100 Subject: [PATCH 4/6] the input can be empty --- .../System.Diagnostics.Process/tests/ProcessTests.Unix.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index 7758b624c0eca9..1643979a43cd56 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -527,7 +527,7 @@ private static int CheckUserAndGroupIds(string userId, string groupId, string gr Assert.Equal(groupId, getgid().ToString()); Assert.Equal(groupId, getegid().ToString()); - var expectedGroups = new HashSet(groupIdsJoined.Split(',').Select(s => uint.Parse(s))); + var expectedGroups = new HashSet(groupIdsJoined.Split(',', StringSplitOptions.RemoveEmptyEntries).Select(s => uint.Parse(s))); if (bool.Parse(checkGroupsExact)) { From b6b82d4eab52cad68acb976a9976ebec536ce8b5 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 18 Dec 2020 20:11:58 +0100 Subject: [PATCH 5/6] use 1000 4 24 27 30 46 116 126 to get expected and 1000 4 24 27 30 46 116 126 to get the current --- .../tests/ProcessTests.Unix.cs | 44 ++++++++----------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index 1643979a43cd56..20aed758edd236 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -531,11 +531,11 @@ private static int CheckUserAndGroupIds(string userId, string groupId, string gr if (bool.Parse(checkGroupsExact)) { - AssertExtensions.Equal(expectedGroups, GetGroups()); + AssertExtensions.Equal(expectedGroups, GetCurrentUserGroupIds()); } else { - Assert.Subset(expectedGroups, GetGroups()); + Assert.Subset(expectedGroups, GetCurrentUserGroupIds()); } return RemoteExecutor.SuccessExitCode; @@ -547,7 +547,7 @@ public unsafe void TestCheckChildProcessUserAndGroupIds() string userName = GetCurrentRealUserName(); string userId = GetUserId(userName); string userGroupId = GetUserGroupId(userName); - string userGroupIds = GetGroupIds(); + string userGroupIds = GetUserGroupIds(userName); // If this test runs as the user, we expect to be able to match the user groups exactly. // Except on OSX, where getgrouplist may return a list of groups truncated to NGROUPS_MAX. @@ -583,7 +583,7 @@ public unsafe void TestCheckChildProcessUserAndGroupIdsElevated(bool useRootGrou string userId = GetUserId(username); string userGroupId = GetUserGroupId(username); - string userGroupIds = GetGroupIds(); + string userGroupIds = GetUserGroupIds(username); if (bool.Parse(useRootGroupsArg)) { @@ -618,7 +618,20 @@ private static string GetUserId(string username) private static string GetUserGroupId(string username) => StartAndReadToEnd("id", new[] { "-g", username }).Trim('\n'); - private static string GetGroupIds() => string.Join(",", GetGroups()); + private static string GetUserGroupIds(string username) + { + string[] groupIds = StartAndReadToEnd("id", new[] { "-G", username }) + .Split(new[] { ' ', '\n' }, StringSplitOptions.RemoveEmptyEntries); + return string.Join(",", groupIds.Select(s => uint.Parse(s)).OrderBy(id => id)); + } + + private static HashSet GetCurrentUserGroupIds() + { + string[] groupIds = StartAndReadToEnd("id", new[] { "-G" }) + .Split(new[] { ' ', '\n' }, StringSplitOptions.RemoveEmptyEntries); + + return new HashSet(groupIds.Select(s => uint.Parse(s))); + } private static string GetCurrentRealUserName() { @@ -902,27 +915,6 @@ private static void ChMod(string filename, string mode) [DllImport("libc")] private static extern uint getgid(); - [DllImport("libc", SetLastError = true)] - private static extern unsafe int getgroups(int size, uint* list); - - private static unsafe HashSet GetGroups() - { - int maxSize = 128; - Span groups = stackalloc uint[maxSize]; - fixed (uint* pGroups = groups) - { - int rv = getgroups(maxSize, pGroups); - if (rv == -1) - { - // If this throws with EINVAL, maxSize should be increased. - throw new Win32Exception(); - } - - // Return this as a HashSet to filter out duplicates. - return new HashSet(groups.Slice(0, rv).ToArray()); - } - } - [DllImport("libc")] private static extern int seteuid(uint euid); From 721fb97f442d14882943c8462d64f84616014d10 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 18 Dec 2020 20:18:02 +0100 Subject: [PATCH 6/6] remove changes that are not needed --- .../System.Diagnostics.Process/tests/ProcessTests.Unix.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index 20aed758edd236..c6da2b1bbc7501 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -12,7 +12,6 @@ using System.Threading.Tasks; using System.Security; using Xunit; -using Xunit.Sdk; using Microsoft.DotNet.RemoteExecutor; using Microsoft.DotNet.XUnitExtensions; @@ -527,7 +526,7 @@ private static int CheckUserAndGroupIds(string userId, string groupId, string gr Assert.Equal(groupId, getgid().ToString()); Assert.Equal(groupId, getegid().ToString()); - var expectedGroups = new HashSet(groupIdsJoined.Split(',', StringSplitOptions.RemoveEmptyEntries).Select(s => uint.Parse(s))); + var expectedGroups = new HashSet(groupIdsJoined.Split(',').Select(s => uint.Parse(s))); if (bool.Parse(checkGroupsExact)) { @@ -548,7 +547,6 @@ public unsafe void TestCheckChildProcessUserAndGroupIds() string userId = GetUserId(userName); string userGroupId = GetUserGroupId(userName); string userGroupIds = GetUserGroupIds(userName); - // If this test runs as the user, we expect to be able to match the user groups exactly. // Except on OSX, where getgrouplist may return a list of groups truncated to NGROUPS_MAX. bool checkGroupsExact = userId == geteuid().ToString() && @@ -629,7 +627,6 @@ private static HashSet GetCurrentUserGroupIds() { string[] groupIds = StartAndReadToEnd("id", new[] { "-G" }) .Split(new[] { ' ', '\n' }, StringSplitOptions.RemoveEmptyEntries); - return new HashSet(groupIds.Select(s => uint.Parse(s))); }