From 47ce0ab5489b215a4ca31542c863617564de104a Mon Sep 17 00:00:00 2001 From: Viktor Hofer Date: Tue, 21 May 2019 07:40:49 +0200 Subject: [PATCH] Reenable ModifyStore tests (dotnet/corefx#37827) Commit migrated from https://github.com/dotnet/corefx/commit/11eba7947e9736109c588a1cfcb77987cb80f570 --- .../tests/ChainTests.cs | 6 +- ...rity.Cryptography.X509Certificates.Tests.csproj | 1 - .../tests/TestEnvironmentConfiguration.Unix.cs | 111 --------------------- .../tests/TestEnvironmentConfiguration.cs | 13 +-- .../tests/X509FilesystemTests.Unix.cs | 27 +++-- 5 files changed, 16 insertions(+), 142 deletions(-) delete mode 100644 src/libraries/System.Security.Cryptography.X509Certificates/tests/TestEnvironmentConfiguration.Unix.cs diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs index 0b197ce..3b61c25 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs @@ -15,8 +15,6 @@ namespace System.Security.Cryptography.X509Certificates.Tests { public static class ChainTests { - internal static bool CanModifyStores { get; } = TestEnvironmentConfiguration.CanModifyStores; - private static bool TrustsMicrosoftDotComRoot { get @@ -125,8 +123,8 @@ namespace System.Security.Cryptography.X509Certificates.Tests } } + [Fact] [PlatformSpecific(TestPlatforms.AnyUnix)] - [ConditionalFact(nameof(CanModifyStores))] public static void VerifyChainFromHandle_Unix() { using (var microsoftDotCom = new X509Certificate2(TestData.MicrosoftDotComSslCertBytes)) @@ -461,7 +459,7 @@ namespace System.Security.Cryptography.X509Certificates.Tests } } - [ConditionalFact(nameof(TrustsMicrosoftDotComRoot), nameof(CanModifyStores))] + [ConditionalFact(nameof(TrustsMicrosoftDotComRoot))] [OuterLoop(/* Modifies user certificate store */)] public static void BuildChain_MicrosoftDotCom_WithRootCertInUserAndSystemRootCertStores() { diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj b/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj index e132f60..a673997 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj @@ -86,7 +86,6 @@ Common\System\IO\PersistedFiles.Names.Unix.cs - diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/TestEnvironmentConfiguration.Unix.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/TestEnvironmentConfiguration.Unix.cs deleted file mode 100644 index b351e89..0000000 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/TestEnvironmentConfiguration.Unix.cs +++ /dev/null @@ -1,111 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System.IO; - -namespace System.Security.Cryptography.X509Certificates.Tests -{ - internal static partial class TestEnvironmentConfiguration - { - static partial void DetermineCanModifyStores(ref bool canModify) - { - try - { - canModify = DetermineCanModifyStores(); - } - catch - { - // This is a little counterintuitive. If the capability probe fails, - // assert that the feature works. Then we'll hopefully get diagnosable - // errors out of the test failures. - canModify = true; - } - } - - private static bool DetermineCanModifyStores() - { - // Check the directory permissions and whether the filesystem supports chmod. - // The only real expected failure from this method is that at the very end - // `stat.Mode == mode` will fail, because fuseblk (NTFS) returns success on chmod, - // but is a no-op. - - uint userId = Interop.Sys.GetEUid(); - string certStoresFeaturePath = PersistedFiles.GetUserFeatureDirectory("cryptography", "x509stores"); - - Directory.CreateDirectory(certStoresFeaturePath); - - // Check directory permissions: - - Interop.Sys.FileStatus dirStat; - if (Interop.Sys.Stat(certStoresFeaturePath, out dirStat) != 0) - { - return false; - } - - if (dirStat.Uid != userId) - { - return false; - } - - if ((dirStat.Mode & (int)Interop.Sys.Permissions.S_IRWXU) != (int)Interop.Sys.Permissions.S_IRWXU) - { - return false; - } - - string probeFilename = - Path.Combine(certStoresFeaturePath, $"{Guid.NewGuid().ToString("N")}.chmod"); - - try - { - using (FileStream stream = new FileStream(probeFilename, FileMode.Create)) - { - Interop.Sys.FileStatus stat; - if (Interop.Sys.FStat(stream.SafeFileHandle, out stat) != 0) - { - return false; - } - - if (stat.Uid != userId) - { - return false; - } - - // The product code here has a lot of stuff it does. - // This capabilities probe will just check that chmod works. - int mode = stat.Mode; - - // Flip all of the O bits. - mode ^= (int)Interop.Sys.Permissions.S_IRWXO; - - if (Interop.Sys.FChMod(stream.SafeFileHandle, mode) < 0) - { - return false; - } - - // Verify the chmod applied. - if (Interop.Sys.FStat(stream.SafeFileHandle, out stat) != 0) - { - return false; - } - - // On fuseblk (NTFS) this will return false, because the fchmod - // call returned success without being able to actually apply - // mode-bits. - return stat.Mode == mode; - } - } - finally - { - try - { - File.Delete(probeFilename); - } - catch - { - // Ignore any failure on delete. - } - } - } - } -} diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/TestEnvironmentConfiguration.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/TestEnvironmentConfiguration.cs index 348f780..d31eaac 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/TestEnvironmentConfiguration.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/TestEnvironmentConfiguration.cs @@ -4,20 +4,9 @@ namespace System.Security.Cryptography.X509Certificates.Tests { - internal static partial class TestEnvironmentConfiguration + internal static class TestEnvironmentConfiguration { - internal static bool CanModifyStores { get; } - internal static bool RunManualTests { get; } = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("CRYPTOGRAPHY_MANUAL_TESTS")); - - static TestEnvironmentConfiguration() - { - bool canModifyStores = true; - DetermineCanModifyStores(ref canModifyStores); - CanModifyStores = canModifyStores; - } - - static partial void DetermineCanModifyStores(ref bool canModify); } } diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/X509FilesystemTests.Unix.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/X509FilesystemTests.Unix.cs index 334e072..3421e2c 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/X509FilesystemTests.Unix.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/X509FilesystemTests.Unix.cs @@ -15,7 +15,6 @@ namespace System.Security.Cryptography.X509Certificates.Tests [Collection("X509Filesystem")] public static class X509FilesystemTests { - private static bool CanModifyStores { get; } = TestEnvironmentConfiguration.CanModifyStores; private static bool RunManualTests { get; } = TestEnvironmentConfiguration.RunManualTests; [OuterLoop] @@ -126,7 +125,7 @@ namespace System.Security.Cryptography.X509Certificates.Tests }); } - [ConditionalFact(nameof(CanModifyStores))] + [Fact] [OuterLoop(/* Alters user/machine state */)] private static void X509Store_AddOne() { @@ -157,7 +156,7 @@ namespace System.Security.Cryptography.X509Certificates.Tests }); } - [ConditionalFact(nameof(CanModifyStores))] + [Fact] [OuterLoop(/* Alters user/machine state */)] private static void X509Store_AddOneAfterUpgrade() { @@ -197,7 +196,7 @@ namespace System.Security.Cryptography.X509Certificates.Tests }); } - [ConditionalFact(nameof(CanModifyStores))] + [Fact] [OuterLoop(/* Alters user/machine state */)] private static void X509Store_DowngradePermissions() { @@ -220,7 +219,7 @@ namespace System.Security.Cryptography.X509Certificates.Tests }); } - [ConditionalFact(nameof(CanModifyStores))] + [Fact] [OuterLoop(/* Alters user/machine state */)] private static void X509Store_AddAfterDispose() { @@ -243,7 +242,7 @@ namespace System.Security.Cryptography.X509Certificates.Tests }); } - [ConditionalFact(nameof(CanModifyStores))] + [Fact] [OuterLoop(/* Alters user/machine state */)] private static void X509Store_AddAndClear() { @@ -267,7 +266,7 @@ namespace System.Security.Cryptography.X509Certificates.Tests }); } - [ConditionalFact(nameof(CanModifyStores))] + [Fact] [OuterLoop(/* Alters user/machine state */)] private static void X509Store_AddDuplicate() { @@ -289,7 +288,7 @@ namespace System.Security.Cryptography.X509Certificates.Tests }); } - [ConditionalFact(nameof(CanModifyStores))] + [Fact] [OuterLoop(/* Alters user/machine state */)] private static void X509Store_AddTwo() { @@ -320,7 +319,7 @@ namespace System.Security.Cryptography.X509Certificates.Tests }); } - [ConditionalFact(nameof(CanModifyStores))] + [Fact] [OuterLoop(/* Alters user/machine state */)] private static void X509Store_AddTwo_UpgradePrivateKey() { @@ -382,7 +381,7 @@ namespace System.Security.Cryptography.X509Certificates.Tests }); } - [ConditionalFact(nameof(CanModifyStores))] + [Fact] [OuterLoop(/* Alters user/machine state */)] private static void X509Store_AddTwo_UpgradePrivateKey_NoDowngrade() { @@ -442,7 +441,7 @@ namespace System.Security.Cryptography.X509Certificates.Tests }); } - [ConditionalFact(nameof(CanModifyStores))] + [Fact] [OuterLoop(/* Alters user/machine state */)] private static void X509Store_DistinctCollections() { @@ -483,7 +482,7 @@ namespace System.Security.Cryptography.X509Certificates.Tests }); } - [ConditionalFact(nameof(CanModifyStores))] + [Fact] [OuterLoop(/* Alters user/machine state */)] private static void X509Store_Add4_Remove1() { @@ -532,7 +531,7 @@ namespace System.Security.Cryptography.X509Certificates.Tests }); } - [ConditionalTheory(nameof(CanModifyStores))] + [Theory] [OuterLoop(/* Alters user/machine state */)] [InlineData(false)] [InlineData(true)] @@ -579,7 +578,7 @@ namespace System.Security.Cryptography.X509Certificates.Tests }); } - [ConditionalFact(nameof(CanModifyStores))] + [Fact] [OuterLoop( /* Alters user/machine state */)] private static void X509Store_FiltersDuplicateOnLoad() { -- 2.7.4