Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Commit

Permalink
More SSL error queue cleaning (#29171)
Browse files Browse the repository at this point in the history 
* Cleaning up more instances leaving errors on SSL queue

* Fixes #25676
  • Loading branch information
Paulo Janotti committed Apr 18, 2018
1 parent ab8e814 commit c0083e0
Show file tree
Hide file tree
Showing 6 changed files with 127 additions and 59 deletions.
5 changes: 5 additions & 0 deletions src/Common/src/Interop/Unix/System.Net.Security.Native/Interop.Initialization.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ static SslInitializer()

//Call ssl specific initializer
Ssl.EnsureLibSslInitialized();
if (Interop.Crypto.ErrPeekLastError() != 0)
{
// It is going to be wrapped in a type load exception but will have the error information
throw Interop.Crypto.CreateOpenSslCryptographicException();
}
}
#endif

Expand Down
1 change: 0 additions & 1 deletion src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.AcceptAllCerts.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ public void SingletonReturnsTrue()
Assert.True(HttpClientHandler.DangerousAcceptAnyServerCertificateValidator(null, null, null, SslPolicyErrors.None));
}

[ActiveIssue(25676, TestPlatforms.Linux)]
[Theory]
[InlineData(SslProtocols.Tls, false)] // try various protocols to ensure we correctly set versions even when accepting all certs
[InlineData(SslProtocols.Tls, true)]
Expand Down
18 changes: 11 additions & 7 deletions ...curity.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/CertificatePal.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,13 @@ public static ICertificatePal FromBlob(byte[] rawData, SafePasswordHandle passwo
Debug.Assert(password != null);

ICertificatePal cert;
Exception openSslException;

if (TryReadX509Der(rawData, out cert) ||
TryReadX509Pem(rawData, out cert) ||
PkcsFormatReader.TryReadPkcs7Der(rawData, out cert) ||
PkcsFormatReader.TryReadPkcs7Pem(rawData, out cert) ||
PkcsFormatReader.TryReadPkcs12(rawData, password, out cert))
PkcsFormatReader.TryReadPkcs12(rawData, password, out cert, out openSslException))
{
if (cert == null)
{
Expand All @@ -51,7 +52,8 @@ public static ICertificatePal FromBlob(byte[] rawData, SafePasswordHandle passwo
}

// Unsupported
throw Interop.Crypto.CreateOpenSslCryptographicException();
Debug.Assert(openSslException != null);
throw openSslException;
}

public static ICertificatePal FromFile(string fileName, SafePasswordHandle password, X509KeyStorageFlags keyStorageFlags)
Expand Down Expand Up @@ -104,22 +106,21 @@ private static ICertificatePal FromBio(SafeBioHandle bio, SafePasswordHandle pas
// Rewind, try again.
RewindBio(bio, bioPosition);

if (PkcsFormatReader.TryReadPkcs12(bio, password, out certPal))
// Capture the exception so in case of failure, the call to BioSeek does not override it.
Exception openSslException;
if (PkcsFormatReader.TryReadPkcs12(bio, password, out certPal, out openSslException))
{
return certPal;
}

// Since we aren't going to finish reading, leaving the buffer where it was when we got
// it seems better than leaving it in some arbitrary other position.
//
// But, before seeking back to start, save the Exception representing the last reported
// OpenSSL error in case the last BioSeek would change it.
Exception openSslException = Interop.Crypto.CreateOpenSslCryptographicException();

// Use BioSeek directly for the last seek attempt, because any failure here should instead
// report the already created (but not yet thrown) exception.
Interop.Crypto.BioSeek(bio, bioPosition);

Debug.Assert(openSslException != null);
throw openSslException;
}

Expand All @@ -141,6 +142,7 @@ internal static bool TryReadX509Der(byte[] rawData, out ICertificatePal certPal)
{
certHandle.Dispose();
certPal = null;
Interop.Crypto.ErrClearError();
return false;
}

Expand All @@ -156,6 +158,7 @@ internal static bool TryReadX509Pem(SafeBioHandle bio, out ICertificatePal certP
{
cert.Dispose();
certPal = null;
Interop.Crypto.ErrClearError();
return false;
}

Expand All @@ -182,6 +185,7 @@ internal static bool TryReadX509Der(SafeBioHandle bio, out ICertificatePal fromB
{
cert.Dispose();
fromBio = null;
Interop.Crypto.ErrClearError();
return false;
}

Expand Down
84 changes: 58 additions & 26 deletions ...y.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/OpenSslPkcs12Reader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,35 +22,17 @@ private OpenSslPkcs12Reader(SafePkcs12Handle pkcs12Handle)
_pkcs12Handle = pkcs12Handle;
}

public static bool TryRead(byte[] data, out OpenSslPkcs12Reader pkcs12Reader)
{
SafePkcs12Handle handle = Interop.Crypto.DecodePkcs12(data, data.Length);
public static bool TryRead(byte[] data, out OpenSslPkcs12Reader pkcs12Reader) =>
TryRead(data, out pkcs12Reader, out _, captureException: false);

if (!handle.IsInvalid)
{
pkcs12Reader = new OpenSslPkcs12Reader(handle);
return true;
}
public static bool TryRead(byte[] data, out OpenSslPkcs12Reader pkcs12Reader, out Exception openSslException) =>
TryRead(data, out pkcs12Reader, out openSslException, captureException: true);

handle.Dispose();
pkcs12Reader = null;
return false;
}
public static bool TryRead(SafeBioHandle fileBio, out OpenSslPkcs12Reader pkcs12Reader) =>
TryRead(fileBio, out pkcs12Reader, out _, captureException: false);

public static bool TryRead(SafeBioHandle fileBio, out OpenSslPkcs12Reader pkcs12Reader)
{
SafePkcs12Handle p12 = Interop.Crypto.DecodePkcs12FromBio(fileBio);

if (!p12.IsInvalid)
{
pkcs12Reader = new OpenSslPkcs12Reader(p12);
return true;
}

p12.Dispose();
pkcs12Reader = null;
return false;
}
public static bool TryRead(SafeBioHandle fileBio, out OpenSslPkcs12Reader pkcs12Reader, out Exception openSslException) =>
TryRead(fileBio, out pkcs12Reader, out openSslException, captureException: true);

public void Dispose()
{
Expand Down Expand Up @@ -132,5 +114,55 @@ public List<OpenSslX509CertificateReader> ReadCertificates()

return certs;
}

private static bool TryRead(byte[] data, out OpenSslPkcs12Reader pkcs12Reader, out Exception openSslException, bool captureException)
{
SafePkcs12Handle handle = Interop.Crypto.DecodePkcs12(data, data.Length);
openSslException = null;

if (!handle.IsInvalid)
{
pkcs12Reader = new OpenSslPkcs12Reader(handle);
return true;
}

handle.Dispose();
pkcs12Reader = null;
if (captureException)
{
openSslException = Interop.Crypto.CreateOpenSslCryptographicException();
}
else
{
Interop.Crypto.ErrClearError();
}

return false;
}

private static bool TryRead(SafeBioHandle fileBio, out OpenSslPkcs12Reader pkcs12Reader, out Exception openSslException, bool captureException)
{
SafePkcs12Handle p12 = Interop.Crypto.DecodePkcs12FromBio(fileBio);
openSslException = null;

if (!p12.IsInvalid)
{
pkcs12Reader = new OpenSslPkcs12Reader(p12);
return true;
}

p12.Dispose();
pkcs12Reader = null;
if (captureException)
{
openSslException = Interop.Crypto.CreateOpenSslCryptographicException();
}
else
{
Interop.Crypto.ErrClearError();
}

return false;
}
}
}
61 changes: 44 additions & 17 deletions ...rity.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/PkcsFormatReader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,15 @@ internal static bool IsPkcs7(byte[] rawData)
{
using (SafePkcs7Handle pkcs7 = Interop.Crypto.DecodePkcs7(rawData, rawData.Length))
{
if (!pkcs7.IsInvalid)
if (pkcs7.IsInvalid)
{
Interop.Crypto.ErrClearError();
}
else
{
return true;
}

}

using (SafeBioHandle bio = Interop.Crypto.CreateMemoryBio())
Expand All @@ -29,7 +34,13 @@ internal static bool IsPkcs7(byte[] rawData)

using (SafePkcs7Handle pkcs7 = Interop.Crypto.PemReadBioPkcs7(bio))
{
return !pkcs7.IsInvalid;
if (pkcs7.IsInvalid)
{
Interop.Crypto.ErrClearError();
return false;
}

return true;
}
}
}
Expand All @@ -38,15 +49,27 @@ internal static bool IsPkcs7Der(SafeBioHandle fileBio)
{
using (SafePkcs7Handle pkcs7 = Interop.Crypto.D2IPkcs7Bio(fileBio))
{
return !pkcs7.IsInvalid;
if (pkcs7.IsInvalid)
{
Interop.Crypto.ErrClearError();
return false;
}

return true;
}
}

internal static bool IsPkcs7Pem(SafeBioHandle fileBio)
{
using (SafePkcs7Handle pkcs7 = Interop.Crypto.PemReadBioPkcs7(fileBio))
{
return !pkcs7.IsInvalid;
if (pkcs7.IsInvalid)
{
Interop.Crypto.ErrClearError();
return false;
}

return true;
}
}

Expand Down Expand Up @@ -90,6 +113,7 @@ private static bool TryReadPkcs7Der(
{
certPal = null;
certPals = null;
Interop.Crypto.ErrClearError();
return false;
}

Expand All @@ -109,6 +133,7 @@ private static bool TryReadPkcs7Der(
{
certPal = null;
certPals = null;
Interop.Crypto.ErrClearError();
return false;
}

Expand Down Expand Up @@ -172,6 +197,7 @@ private static bool TryReadPkcs7Pem(
{
certPal = null;
certPals = null;
Interop.Crypto.ErrClearError();
return false;
}

Expand Down Expand Up @@ -218,46 +244,46 @@ private static bool TryReadPkcs7(
return true;
}

internal static bool TryReadPkcs12(byte[] rawData, SafePasswordHandle password, out ICertificatePal certPal)
internal static bool TryReadPkcs12(byte[] rawData, SafePasswordHandle password, out ICertificatePal certPal, out Exception openSslException)
{
List<ICertificatePal> ignored;

return TryReadPkcs12(rawData, password, true, out certPal, out ignored);

return TryReadPkcs12(rawData, password, true, out certPal, out ignored, out openSslException);
}

internal static bool TryReadPkcs12(SafeBioHandle bio, SafePasswordHandle password, out ICertificatePal certPal)
internal static bool TryReadPkcs12(SafeBioHandle bio, SafePasswordHandle password, out ICertificatePal certPal, out Exception openSslException)
{
List<ICertificatePal> ignored;

return TryReadPkcs12(bio, password, true, out certPal, out ignored);
return TryReadPkcs12(bio, password, true, out certPal, out ignored, out openSslException);
}

internal static bool TryReadPkcs12(byte[] rawData, SafePasswordHandle password, out List<ICertificatePal> certPals)
internal static bool TryReadPkcs12(byte[] rawData, SafePasswordHandle password, out List<ICertificatePal> certPals, out Exception openSslException)
{
ICertificatePal ignored;

return TryReadPkcs12(rawData, password, false, out ignored, out certPals);
return TryReadPkcs12(rawData, password, false, out ignored, out certPals, out openSslException);
}

internal static bool TryReadPkcs12(SafeBioHandle bio, SafePasswordHandle password, out List<ICertificatePal> certPals)
internal static bool TryReadPkcs12(SafeBioHandle bio, SafePasswordHandle password, out List<ICertificatePal> certPals, out Exception openSslException)
{
ICertificatePal ignored;

return TryReadPkcs12(bio, password, false, out ignored, out certPals);
return TryReadPkcs12(bio, password, false, out ignored, out certPals, out openSslException);
}

private static bool TryReadPkcs12(
byte[] rawData,
SafePasswordHandle password,
bool single,
out ICertificatePal readPal,
out List<ICertificatePal> readCerts)
out List<ICertificatePal> readCerts,
out Exception openSslException)
{
// DER-PKCS12
OpenSslPkcs12Reader pfx;

if (!OpenSslPkcs12Reader.TryRead(rawData, out pfx))
if (!OpenSslPkcs12Reader.TryRead(rawData, out pfx, out openSslException))
{
readPal = null;
readCerts = null;
Expand All @@ -275,12 +301,13 @@ private static bool TryReadPkcs12(
SafePasswordHandle password,
bool single,
out ICertificatePal readPal,
out List<ICertificatePal> readCerts)
out List<ICertificatePal> readCerts,
out Exception openSslException)
{
// DER-PKCS12
OpenSslPkcs12Reader pfx;

if (!OpenSslPkcs12Reader.TryRead(bio, out pfx))
if (!OpenSslPkcs12Reader.TryRead(bio, out pfx, out openSslException))
{
readPal = null;
readCerts = null;
Expand Down
Loading

0 comments on commit c0083e0

Please sign in to comment.