diff options
author | Jeremy Barton <jbarton@microsoft.com> | 2017-07-25 18:06:35 +0300 |
---|---|---|
committer | Jeremy Barton <jbarton@microsoft.com> | 2017-08-04 20:32:32 +0300 |
commit | fec742c21f867501ad3c9ff9a8d5ba48fe475ab2 (patch) | |
tree | b0eebde61bb06d530da047ee4fc7c9017480c81d /src | |
parent | 151dfc77c6e9350b47ac646bd27feb92d2bbd70e (diff) |
Make OpenSSL verify self-issued signature integrity to match Windows
Windows checks signatures on self-issued certificates (to prove they are
self-signed), but OpenSSL doesn't by default, opting to save on the CPU.
Before this change the behavior was:
* Windows: NotSignatureValid
* macOS: UntrustedRoot
* Linux: UntrustedRoot
With this change it becomes
* Windows: NotSignatureValid
* macOS: UntrustedRoot
* Linux: NotSignatureValid | UntrustedRoot
While additional work could be done to not report UntrustedRoot for Linux
chains, it doesn't seem as worthwhile.
macOS will continue not reporting NotSignatureValid, because there's no API
to allow changing the behavior.
Diffstat (limited to 'src')
3 files changed, 54 insertions, 1 deletions
diff --git a/src/Native/Unix/System.Security.Cryptography.Native/opensslshim.h b/src/Native/Unix/System.Security.Cryptography.Native/opensslshim.h index 9cd689d991..3e405b09da 100644 --- a/src/Native/Unix/System.Security.Cryptography.Native/opensslshim.h +++ b/src/Native/Unix/System.Security.Cryptography.Native/opensslshim.h @@ -325,6 +325,7 @@ int EC_POINT_set_affine_coordinates_GF2m(const EC_GROUP *group, EC_POINT *p, PER_FUNCTION_BLOCK(X509_STORE_CTX_get_error_depth, true) \ PER_FUNCTION_BLOCK(X509_STORE_CTX_init, true) \ PER_FUNCTION_BLOCK(X509_STORE_CTX_new, true) \ + PER_FUNCTION_BLOCK(X509_STORE_CTX_set_flags, true) \ PER_FUNCTION_BLOCK(X509_STORE_CTX_set_verify_cb, true) \ PER_FUNCTION_BLOCK(X509_STORE_free, true) \ PER_FUNCTION_BLOCK(X509_STORE_new, true) \ @@ -615,6 +616,7 @@ FOR_ALL_OPENSSL_FUNCTIONS #define X509_STORE_CTX_get_error_depth X509_STORE_CTX_get_error_depth_ptr #define X509_STORE_CTX_init X509_STORE_CTX_init_ptr #define X509_STORE_CTX_new X509_STORE_CTX_new_ptr +#define X509_STORE_CTX_set_flags X509_STORE_CTX_set_flags_ptr #define X509_STORE_CTX_set_verify_cb X509_STORE_CTX_set_verify_cb_ptr #define X509_STORE_free X509_STORE_free_ptr #define X509_STORE_new X509_STORE_new_ptr diff --git a/src/Native/Unix/System.Security.Cryptography.Native/pal_x509.cpp b/src/Native/Unix/System.Security.Cryptography.Native/pal_x509.cpp index 0a3dfcc2c8..3118c9aa2c 100644 --- a/src/Native/Unix/System.Security.Cryptography.Native/pal_x509.cpp +++ b/src/Native/Unix/System.Security.Cryptography.Native/pal_x509.cpp @@ -208,7 +208,14 @@ extern "C" void CryptoNative_X509StoreCtxDestroy(X509_STORE_CTX* v) extern "C" int32_t CryptoNative_X509StoreCtxInit(X509_STORE_CTX* ctx, X509_STORE* store, X509* x509, X509Stack* extraStore) { - return X509_STORE_CTX_init(ctx, store, x509, extraStore); + int32_t val = X509_STORE_CTX_init(ctx, store, x509, extraStore); + + if (val != 0) + { + X509_STORE_CTX_set_flags(ctx, X509_V_FLAG_CHECK_SS_SIGNATURE); + } + + return val; } extern "C" int32_t CryptoNative_X509VerifyCert(X509_STORE_CTX* ctx) diff --git a/src/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs b/src/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs index 3d1d44ec5a..7cf8db6023 100644 --- a/src/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs +++ b/src/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Runtime.InteropServices; using System.Text; using System.Threading; using Xunit; @@ -630,5 +631,48 @@ namespace System.Security.Cryptography.X509Certificates.Tests using (var chain = X509Chain.Create()) Assert.NotNull(chain); } + + [Fact] + public static void InvalidSelfSignedSignature() + { + X509ChainStatusFlags expectedFlags; + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + expectedFlags = X509ChainStatusFlags.NotSignatureValid; + } + else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) + { + expectedFlags = X509ChainStatusFlags.UntrustedRoot; + } + else + { + expectedFlags = + X509ChainStatusFlags.NotSignatureValid | + X509ChainStatusFlags.UntrustedRoot; + } + + byte[] certBytes = (byte[])TestData.MicrosoftDotComRootBytes.Clone(); + // The signature goes up to the very last byte, so flip some bits in it. + certBytes[certBytes.Length - 1] ^= 0xFF; + + using (var cert = new X509Certificate2(certBytes)) + using (ChainHolder holder = new ChainHolder()) + { + X509Chain chain = holder.Chain; + X509ChainPolicy policy = chain.ChainPolicy; + policy.VerificationTime = cert.NotBefore.AddDays(3); + policy.RevocationMode = X509RevocationMode.NoCheck; + + chain.Build(cert); + + X509ChainStatusFlags allFlags = + chain.ChainStatus.Select(cs => cs.Status).Aggregate( + X509ChainStatusFlags.NoError, + (a, b) => a | b); + + Assert.Equal(expectedFlags, allFlags); + } + } } } |