From bc80566e3b85f0cf00127babb3942c08fccaa013 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 30 May 2018 08:59:28 +0200 Subject: [PATCH 1/5] Skip certificates we can't read when populating machine store. This is a regression in 2.1.0 (https://github.com/dotnet/corefx/pull/29351). Fixes https://github.com/dotnet/corefx/issues/29942. --- .../src/Internal/Cryptography/Pal.Unix/StorePal.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/StorePal.cs b/src/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/StorePal.cs index 3c3cdb20c59e..57b8f025b88b 100644 --- a/src/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/StorePal.cs +++ b/src/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/StorePal.cs @@ -254,7 +254,12 @@ private static void LoadMachineStores() { using (SafeBioHandle fileBio = Interop.Crypto.BioNewFile(file.FullName, "rb")) { - Interop.Crypto.CheckValidOpenSslHandle(fileBio); + // The handle may be invalid, for example when we don't have read permission for the file. + if (fileBio.IsInvalid) + { + Interop.Crypto.ErrClearError(); + continue; + } ICertificatePal pal; From fd2a7c30b26825216ac35922ad51e471ffbfc123 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 30 May 2018 21:03:38 +0200 Subject: [PATCH 2/5] Add test --- ...rity.Cryptography.X509Certificates.Tests.csproj | 6 +++ .../tests/TestData.cs | 48 ++++++++++++++++++++++ .../tests/X509StoreTests.cs | 48 +++++++++++++++++++++- 3 files changed, 101 insertions(+), 1 deletion(-) diff --git a/src/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj b/src/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj index 09f919a4f438..f1d383dff6d5 100644 --- a/src/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj +++ b/src/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj @@ -90,6 +90,12 @@ + + + {69e46a6f-9966-45a5-8945-2559fe337827} + RemoteExecutorConsoleApp + + diff --git a/src/System.Security.Cryptography.X509Certificates/tests/TestData.cs b/src/System.Security.Cryptography.X509Certificates/tests/TestData.cs index 25b55d437b9d..5be9fd735ae4 100644 --- a/src/System.Security.Cryptography.X509Certificates/tests/TestData.cs +++ b/src/System.Security.Cryptography.X509Certificates/tests/TestData.cs @@ -80,6 +80,54 @@ internal static class TestData XgSpRm3m9Xp5QL0fzehF1a7iXT71dcfmZmNgzNWahIeNJDD37zTQYx2xQmdKDku/ Og7vtpU6pzjkJZIIpohmgg== -----END CERTIFICATE----- +"); + + // 'cert.pem' generated using 'openssl req -x509 -newkey rsa:2048 -keyout key.pem -out cert.pem -days 365' + public static readonly byte[] SelfSigned1PemBytes = ByteUtils.AsciiBytes( + @"-----BEGIN CERTIFICATE----- +MIIDWjCCAkKgAwIBAgIJAJpCQ7mtFWHeMA0GCSqGSIb3DQEBCwUAMEIxCzAJBgNV +BAYTAlhYMRUwEwYDVQQHDAxEZWZhdWx0IENpdHkxHDAaBgNVBAoME0RlZmF1bHQg +Q29tcGFueSBMdGQwHhcNMTgwNTMwMTgyNjM1WhcNMTkwNTMwMTgyNjM1WjBCMQsw +CQYDVQQGEwJYWDEVMBMGA1UEBwwMRGVmYXVsdCBDaXR5MRwwGgYDVQQKDBNEZWZh +dWx0IENvbXBhbnkgTHRkMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA +pfYZTHjzei9U3QxiIIjESsf9z3Bfl8FAQLIU+OeICN3upnDvTgeWM/Jw7LwiuHhu +XvSawPwQ8ONvUeSG/wfyjYyTB7VBpVnNi6oTR6E1WSuiu0iT3qlDHVwArTI5DvIM +FzP3/AT1Ub5SvwVbWiR2za6wuUIsryyLz5+zCwGr+J/Xbmta/H9IT9NLwmDJCZQe +4Q4hCWhf7FKdXWt59y9PofVnE7R8CKNfUKr6GA+gy+SEtM/cHgqox5PErnV9b14U +uVROnRUyo1bFwTOdoW3zf5S4VZ4pFPJHNYACHEMiE0eNgfJf+QeyPUPN50neEAbf +kQYkeEET8dW6JlDFrAI4wwIDAQABo1MwUTAdBgNVHQ4EFgQUK+C/eGYPlV+KaTvj +tF6lJaKmo3EwHwYDVR0jBBgwFoAUK+C/eGYPlV+KaTvjtF6lJaKmo3EwDwYDVR0T +AQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAZUjvDMhGc45TLRHKO5rsyifN +g7qb3dO5vtD/JWeo+wyMYcBHIANIIxYrkT0dRBQWQerVDBvsAESahM3f0SdszGac +6y1qxQWxfjxRiCwrEQ7JVZkmspYLbOxaS1T2IZUo3D7VJReyna6r11EKy7i49Toa +KmrhTLBsHV+MUgPRtupiOOu0fXqfxpXE7XEvi0hyv8PKli+Oww2Zyt1jTTvv2RTA +eJRqTUNUbWEDesXAOh5CY6Xjfg7Gt6IYQHt0JMw29pXB3TV2uyXuvFNsc725cPbW +JCuC9TGQRUAUj+LZ43tTrfaZ7g5L80/eRrvlx5MIJSsX8cev8pZYx224WRtk/w== +-----END CERTIFICATE----- +"); + + // 'cert.pem' generated using 'openssl req -x509 -newkey rsa:2048 -keyout key.pem -out cert.pem -days 365' + public static readonly byte[] SelfSigned2PemBytes = ByteUtils.AsciiBytes( + @"-----BEGIN CERTIFICATE----- +MIIDWjCCAkKgAwIBAgIJAM6YQ4PrC9jaMA0GCSqGSIb3DQEBCwUAMEIxCzAJBgNV +BAYTAlhYMRUwEwYDVQQHDAxEZWZhdWx0IENpdHkxHDAaBgNVBAoME0RlZmF1bHQg +Q29tcGFueSBMdGQwHhcNMTgwNTMwMTgyNjQ4WhcNMTkwNTMwMTgyNjQ4WjBCMQsw +CQYDVQQGEwJYWDEVMBMGA1UEBwwMRGVmYXVsdCBDaXR5MRwwGgYDVQQKDBNEZWZh +dWx0IENvbXBhbnkgTHRkMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA +7vkM6zrhXJFtrV63lUb4fsjZG2JYvSRGYv4Y/rwe7VLVdTYvMjosyvKCHJ4Frmtb +YU4jJeB+859mQAd3IOBEhgUJuJ6gC8cOJAwUFJNUabeuafXG2zw/U+396csRKr11 +iBUpvooFJR7KLWrqPKXhK5yESV1k7OzSSZs4owmyIvSaGQO2T63S39OYJhq8ZUlO ++MznaOQGp2J+JWncZo9XCpiotZwdNtw5k+F1g3NAz4/+Vkah/SfQhcNCfJyfVDCX +IwBS+gz9i1BIw6s+SLYtkp167yyizmVIWoXtkgCPaeG0FzBPAhL9GDLTItJ/V/O5 +F9SjbvS+4rUIuPSn7NdodwIDAQABo1MwUTAdBgNVHQ4EFgQUq4v4TrvYrsbKDRGF +bMnj3++P9B4wHwYDVR0jBBgwFoAUq4v4TrvYrsbKDRGFbMnj3++P9B4wDwYDVR0T +AQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAS4ZKEGVfIrHMmYAZe9p3jm15 +85OIPLlM4q6QjRccLJ4t2+xYLRU9PNa2Qmz8l+SFOy9XN9yJp79lSi1gN4lJ8ZdI +kwu+zPEzwsJnb6f3zRu2RQUeAkaShDCEdxpyKHEky1KVG2nOa3cKp+pqzN4DQ3Rp +cJCjcP5ncNJ0bbCZTS7w0oVvX5JhBWIigw3CN5rL2rf83CTPPBzUype0bt97sBSs +dxIPtH9l/q9OgdaCrPE8KBqcwXsfNlFwYGjkqmN/v7WaysBRdblHcoWmry3YsaK2 +/tZo6lmYOHpdqL0OdDwlldToY7QdL1coICfHas5Ony49OHTCUZz6G/AS+3a3gQ== +-----END CERTIFICATE----- "); public const string PfxDataPassword = "12345"; diff --git a/src/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs b/src/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs index 2bc94e2fec55..96b63ceaaa6b 100644 --- a/src/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs +++ b/src/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs @@ -2,11 +2,14 @@ // 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.Diagnostics; +using System.IO; +using System.Runtime.InteropServices; using Xunit; namespace System.Security.Cryptography.X509Certificates.Tests { - public class X509StoreTests + public class X509StoreTests : RemoteExecutorTestBase { [Fact] public static void OpenMyStore() @@ -496,5 +499,48 @@ public static void UnixCannotModifyDisallowedStore(bool useEnum, OpenFlags openF Assert.Equal(0, store.Certificates.Count); } } + + [Fact] + [PlatformSpecific(TestPlatforms.AnyUnix)] + private void X509Store_MachineStoreLoadSkipsInvalidFiles() + { + // We create a folder for our machine store and use it by setting SSL_CERT_{DIR,FILE}. + // In the store we'll add some invalid files, but we start and finish with a valid file. + // This is to account for the order in which the store is populated. + string sslCertDir = GetTestFilePath(); + Directory.CreateDirectory(sslCertDir); + + // Valid file. + File.WriteAllBytes(Path.Combine(sslCertDir, "0.pem"), TestData.SelfSigned1PemBytes); + + // File with invalid content. + File.WriteAllText(Path.Combine(sslCertDir, "1.pem"), "This is not a valid cert"); + + // File which is not readable by the current user. + string unreadableFileName = Path.Combine(sslCertDir, "2.pem"); + File.WriteAllText(unreadableFileName, string.Empty); + chmod(unreadableFileName, 0); + + // Valid file. + File.WriteAllBytes(Path.Combine(sslCertDir, "3.pem"), TestData.SelfSigned2PemBytes); + + var psi = new ProcessStartInfo(); + psi.Environment.Add("SSL_CERT_DIR", sslCertDir); + psi.Environment.Add("SSL_CERT_FILE", "/nonexisting"); + RemoteInvoke(() => + { + using (var store = new X509Store(StoreName.Root, StoreLocation.LocalMachine)) + { + store.Open(OpenFlags.OpenExistingOnly); + + // Check nr of certificates in store. + Assert.Equal(2, store.Certificates.Count); + } + return SuccessExitCode; + }, new RemoteInvokeOptions { StartInfo = psi }).Dispose(); + } + + [DllImport("libc")] + private static extern int chmod(string path, int mode); } } From 0a4ffb5f547bc990f9bc0e849ee0e4c4f816c720 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 31 May 2018 08:17:34 +0200 Subject: [PATCH 3/5] Assert chmod returns 0 --- .../tests/X509StoreTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs b/src/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs index 96b63ceaaa6b..82dba736c108 100644 --- a/src/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs +++ b/src/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs @@ -519,7 +519,7 @@ private void X509Store_MachineStoreLoadSkipsInvalidFiles() // File which is not readable by the current user. string unreadableFileName = Path.Combine(sslCertDir, "2.pem"); File.WriteAllText(unreadableFileName, string.Empty); - chmod(unreadableFileName, 0); + Assert.Equal(0, chmod(unreadableFileName, 0)); // Valid file. File.WriteAllBytes(Path.Combine(sslCertDir, "3.pem"), TestData.SelfSigned2PemBytes); From 2e5c99e9a6e0bc114459947a3ba3f87b145632d1 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 31 May 2018 09:39:43 +0200 Subject: [PATCH 4/5] Skip test on OSX --- .../tests/X509StoreTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs b/src/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs index 82dba736c108..ba1e9142f716 100644 --- a/src/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs +++ b/src/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs @@ -501,7 +501,7 @@ public static void UnixCannotModifyDisallowedStore(bool useEnum, OpenFlags openF } [Fact] - [PlatformSpecific(TestPlatforms.AnyUnix)] + [PlatformSpecific(TestPlatforms.Linux)] // Windows/OSX doesn't use SSL_CERT_{DIR,FILE}. private void X509Store_MachineStoreLoadSkipsInvalidFiles() { // We create a folder for our machine store and use it by setting SSL_CERT_{DIR,FILE}. From 80a678f1a5d29ec913e36afdda028d6fee5a7785 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 4 Jun 2018 15:59:01 +0200 Subject: [PATCH 5/5] Add valid content to the unreadable cert file --- .../tests/TestData.cs | 24 ++++++++++++++++++++++ .../tests/X509StoreTests.cs | 4 ++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/System.Security.Cryptography.X509Certificates/tests/TestData.cs b/src/System.Security.Cryptography.X509Certificates/tests/TestData.cs index 5be9fd735ae4..ff8c1614be5a 100644 --- a/src/System.Security.Cryptography.X509Certificates/tests/TestData.cs +++ b/src/System.Security.Cryptography.X509Certificates/tests/TestData.cs @@ -128,6 +128,30 @@ internal static class TestData dxIPtH9l/q9OgdaCrPE8KBqcwXsfNlFwYGjkqmN/v7WaysBRdblHcoWmry3YsaK2 /tZo6lmYOHpdqL0OdDwlldToY7QdL1coICfHas5Ony49OHTCUZz6G/AS+3a3gQ== -----END CERTIFICATE----- +"); + + // 'cert.pem' generated using 'openssl req -x509 -newkey rsa:2048 -keyout key.pem -out cert.pem -days 365' + public static readonly byte[] SelfSigned3PemBytes = ByteUtils.AsciiBytes( + @"-----BEGIN CERTIFICATE----- +MIIDWjCCAkKgAwIBAgIJANzv9IQvr0bwMA0GCSqGSIb3DQEBCwUAMEIxCzAJBgNV +BAYTAlhYMRUwEwYDVQQHDAxEZWZhdWx0IENpdHkxHDAaBgNVBAoME0RlZmF1bHQg +Q29tcGFueSBMdGQwHhcNMTgwNjA0MTMzMjIxWhcNMTkwNjA0MTMzMjIxWjBCMQsw +CQYDVQQGEwJYWDEVMBMGA1UEBwwMRGVmYXVsdCBDaXR5MRwwGgYDVQQKDBNEZWZh +dWx0IENvbXBhbnkgTHRkMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA +wy+py+hFxSmCGTZmHrQm1Yobzxf34l+J8VD33ObGV1qIFFulxz8pnUU4gKf6FQNU +wAbezJ78Eqsjt4c7mwnGTdavWSZyDJ136bQzn52wsTOGRfUBe1vt+SMy7h8Nhhf3 +ejRHQVsZKNfiGOekmjBKFLliavo6I8j80UsmpvAJ+TWnYpVQBf/EzBQ21ddIF6jD +nl2ZhcvWHvS63utWwXW68xkDXsLvjiat22YScRKnkkNAIvbBY4rvV1KwahUPaMTS +zWywr6caHxlKp7McZ4MJVIqUAeZUn4KYgSksi2IsfPA7qi8WpSaKGsOZFBD79DJC +wqzdLLBzEtg6okzgC5nFtwIDAQABo1MwUTAdBgNVHQ4EFgQUgKAUBaaA1XD8KqGg +1XEr74W4lrkwHwYDVR0jBBgwFoAUgKAUBaaA1XD8KqGg1XEr74W4lrkwDwYDVR0T +AQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEArNBpG8oCDKX9ERbMgvgm3qWk +FKmx+h58aiZVoMwfBsf2njZ6BzRoEOvluMDPe+pt8hhST5yaOsMUYIqrn+s692I9 +17JRfrFhCp+4GT8oe/ZnSNTPm2zOzm0VXFkfDF53YGzdGTWXLH+pJpw4drCNoBoA +yloyF/JJGJ2ZMbnwuDtsPbpjup8qHLiQYjxj4hUWyXU+nbytGK/i8z8HHc7acOpd +9+MXEcKwUkthXzG0M/0bzz4GwWZ6PPmbI5EEqFGBzMef58/mbHDigl9/o3kUlJtB +tcCZhP5KEu6XKKc1GcTqbyA0vi92YyyZViUa36hhVrNqPxtpclir+lcnNgnlqg== +-----END CERTIFICATE----- "); public const string PfxDataPassword = "12345"; diff --git a/src/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs b/src/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs index ba1e9142f716..151067587bf6 100644 --- a/src/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs +++ b/src/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs @@ -518,11 +518,11 @@ private void X509Store_MachineStoreLoadSkipsInvalidFiles() // File which is not readable by the current user. string unreadableFileName = Path.Combine(sslCertDir, "2.pem"); - File.WriteAllText(unreadableFileName, string.Empty); + File.WriteAllBytes(unreadableFileName, TestData.SelfSigned2PemBytes); Assert.Equal(0, chmod(unreadableFileName, 0)); // Valid file. - File.WriteAllBytes(Path.Combine(sslCertDir, "3.pem"), TestData.SelfSigned2PemBytes); + File.WriteAllBytes(Path.Combine(sslCertDir, "3.pem"), TestData.SelfSigned3PemBytes); var psi = new ProcessStartInfo(); psi.Environment.Add("SSL_CERT_DIR", sslCertDir);