Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Desktop] System.AccessViolationException on HttpClient with Certificates #25240

Closed
StefanoD opened this issue Feb 28, 2018 · 8 comments
Closed

Comments

@StefanoD
Copy link

Hi,
I think, this bug relates to #20628, respective to #20117.
When I'm using mutual authentification per SSL and I send something to a server, then I get a System.AccessViolationException without any stack trace.

Here is my custom class which extends HttpClient and uses a client certificate and has a custom certificate validation:

public class QlcHttpsClient : HttpClient
{
	private static readonly QlcHttpsClient instance = new QlcHttpsClient();
	public QlcHttpsClient() : base(GetHttpMessageHandler())
	{
	}

	public static QlcHttpsClient Instance
	{
		get { return instance; }
	}

	private static HttpMessageHandler GetHttpMessageHandler()
	{
		X509Certificate2 cert = new X509Certificate2();
		cert.Import(GetClientCertificate(), "mypass", X509KeyStorageFlags.DefaultKeySet);
		var clientHandler = new WebRequestHandler();
		clientHandler.ClientCertificates.Add(cert);
		clientHandler.ServerCertificateValidationCallback = RemoteCertificateValidationCallback;

		return clientHandler;
	}
	private static byte[] GetClientCertificate()
	{
		var res = new ResourceManager("QLCClient.resources", typeof(QlcTestResultsController).Assembly);
		byte[] rawCert = (byte[]) res.GetObject("my_cert");
		return rawCert;
	}

	private static bool RemoteCertificateValidationCallback(object sender, X509Certificate certificate, X509Chain chain, SslPolicyErrors sslPolicyErrors)
	{
		string thumbprint = certificate.GetCertHashString();
		string expectedThumbprint = "5D772BB5C085AB490E2773DAA54EC27F3605BDF9";
		if (certificate.Issuer.Contains("CN=MyCN") && thumbprint.Equals(expectedThumbprint, StringComparison.OrdinalIgnoreCase))
		{
			return true;
		}
		return false;
	}
}

When I step with Visual Studio line by line, then I get the named Exception on SendAsync().

class QlcHttpTransmitter
{
	private static readonly QlcHttpsClient client = QlcHttpsClient.Instance;

	public static async Task<HttpResponseMessage> Send(string pathValue, HttpContent content, HttpMethod httpMethod)
	{
		UriBuilder builder = QlcUriBuilder.GetUriBuilder(pathValue);
		HttpRequestMessage httpRequestMessage = new HttpRequestMessage(httpMethod, builder.Uri)
		{
			Content = content
		};
		HttpResponseMessage response = await client.SendAsync(httpRequestMessage); // Throws System.AccessViolationException

		return response;
	}
}

I'm using .Net Framework 4.6.2 and I get this Exception only in my system test. But when I migrate to .Net Framework 4.7.1 then I get this also in production code. So I think the bug is always there and it depends on the runtime behaviour.

@davidsh
Copy link
Contributor

davidsh commented Feb 28, 2018

Can you repro this problem with .NET Core? If this only happens on .NET Framework, we should move this issue to that bug database.

cc: @karelz @stephentoub

@StefanoD
Copy link
Author

Sorry, my employee won't give me the time to reproduce this with .Net Core.
I didn't know where to report this and I thought the implementation of this class were the same, so I opened an issue here.

@davidsh
Copy link
Contributor

davidsh commented Mar 15, 2018

Since this code use WebRequestHandler, it is specific to .NET Framework. That handler class is not available on .NET Core.

var clientHandler = new WebRequestHandler();

@karelz karelz changed the title System.AccessViolationException on HttpClient with Certificates [Desktop] System.AccessViolationException on HttpClient with Certificates Mar 15, 2018
@StefanoD
Copy link
Author

Ok, thx. Where shall I report it?

@davidsh
Copy link
Contributor

davidsh commented Mar 15, 2018

@karelz pls advise on best way to report .NET Framework bugs.

@StefanoD, I do have a comment about your current snippet of repro code:

	private static byte[] GetClientCertificate()
	{
		var res = new ResourceManager("QLCClient.resources", typeof(QlcTestResultsController).Assembly);
		byte[] rawCert = (byte[]) res.GetObject("my_cert");
		return rawCert;
	}

It is not clear from this code whether or not the client certificate is good. Where did those bytes[] come from? Normally a client certificate needs to be trusted and have the private key accessible. And that usually happens only when retrieving a client certificate from a certificate store. Is this certificate something that is in PFX format, for example?

So, it's possible that the AccessViolationException you are getting is "normal". The fact that you didn't get a good stack trace is probably because the AV is at the unmanaged layer. You probably need to set your Visual Studio debugging environment to do both managed and native debugging.

My guess is that this is not a bug in .NET Framework, but rather an improper way of storing and using client certificates.

@StefanoD
Copy link
Author

@davidsh The client certificate has been created by me and is normally not trusted. But this is not a problem, because my peer does a custom validation (by checking the fingerprint).
The certificate is an an embedded ressource (*.resx) and retrieved as shown in the code and it is indeed a pfx file.

As I said before: The AccessViolationException didn't always happen.

How should I store my certificate? I want it to be embedded.

@davidsh
Copy link
Contributor

davidsh commented Mar 16, 2018

Have you been able to repro the AccessViolationException and with both native and managed debugging turned on in Visual Studio? I think it would be best to get a good stack trace of this error.

Is it possible for you to create a complete Visual Studio solution of the repro and attach it here as a zip file? That would allow us to troubleshoot this directly.

@karelz
Copy link
Member

karelz commented May 6, 2018

Looks like we don't have enough information / repro to make it actionable. Closing.
If there is a repro or more detailed analysis, feel free to reopen.

@karelz karelz closed this as completed May 6, 2018
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 3.0 milestone Jan 31, 2020
@dotnet dotnet locked as resolved and limited conversation to collaborators Dec 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants