Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Support ClientCertificate #332

Closed
tmds opened this issue Nov 5, 2015 · 13 comments
Closed

Support ClientCertificate #332

tmds opened this issue Nov 5, 2015 · 13 comments
Assignees
Milestone

Comments

@tmds
Copy link
Contributor

tmds commented Nov 5, 2015

Is this available or planned?

@davidfowl
Copy link
Member

There are no plans to add client certs to kestrel.

@tmds
Copy link
Contributor Author

tmds commented Nov 5, 2015

ok, consider this a feature request :)
Should be doable via HttpsConnectionFilter?

@tmds tmds changed the title Use ClientCertificate Support ClientCertificate Nov 5, 2015
@davidfowl
Copy link
Member

That's what I'm thinking. A separate package (not part of the HttpsConnectionFilter) that adds this support via a filter. If you're going to do it, probably worth doing a POC just to see what it would look like.

/cc @halter73 @lodejard @Tratcher

@tmds
Copy link
Contributor Author

tmds commented Nov 5, 2015

HttpsConnectionFilter contains the SSLStream which provides the access to the client certificate.
The certificate could be stored in the ConnectionFilterContext and set on the HttpContext.

This would allow to filter out the clients via the "ConnectionFilterContext".

I can't think up of an alternative unless there is a cast to SSLStream to retrieve the certificate at a later point.

@tmds
Copy link
Contributor Author

tmds commented Nov 6, 2015

Had a closer look at the HttpsConnectionFilter approach:

Can you give some feedback or suggest alternatives?

@Tratcher
Copy link
Member

Tratcher commented Nov 6, 2015

Ideally there'd be a way to add the ITlsConnectionFeature for HTTPS requests and then make GetClientCertificateAsync/ClientCertificate lazily retrieve the SslStream.RemoteCertificate.

@tmds
Copy link
Contributor Author

tmds commented Nov 6, 2015

@Tratcher What part should be lazy?
What's the rationale for having an async method?
Isn't the ClientCertificate available as soon as AuthenticateAsServer completes?

@davidfowl I hear your preference to separate this feature from HttpsConnectionFilter but this seems the only place where you can get a hold of the SslStream. Casting at an other location may give you an adapter.

@Tratcher
Copy link
Member

Tratcher commented Nov 6, 2015

GetClientCertificateAsync exists because the client certificate can be negotiate after the initial connection on some servers (e.g. WebListener). I thought SslStream could do that too but apparently that's not exposed.

I don't see how you can separate this from HttpsConnectionFilter, it's an SSL feature.

@davidfowl
Copy link
Member

Yep my bad, it's tied to ssl

@tmds
Copy link
Contributor Author

tmds commented Nov 10, 2015

See pull request.

The public API changed to include a ClientCertificateMode.
Note that the SSL handshake is actually different depending on whether the server accepts a certificate or not.

I haven't written any tests yet but did some manual testing using a separate 4.5 application.
It looks like a valid certificate will be needed to implement a test.
During the SSL handshake, the server sends a list of certificate authorities it trusts. The WebRequestHandler won't send a certificate if it can't match it to one of the CA's (so it won't send a self signed certificate). It took me some debugging using wireshark and System.Net.Sockets logging to figure this out.

@halter73 halter73 added this to the 1.0.0-rc2 milestone Nov 13, 2015
@halter73 halter73 added the task label Nov 13, 2015
@leastprivilege
Copy link

Well - at least the IIS platform plumbing could forward the validated client client cert to kestrel. More control would be nice - but that would be a start.

@davidfowl
Copy link
Member

@leastprivilege that's a different feature and has nothing to do with Kestrel aspnet/IISIntegration#27

@halter73
Copy link
Member

Just merged #385, so I'm closing this.

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

6 participants