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

Implement client certificate authentication #385

Merged
merged 8 commits into from
Nov 19, 2015
Merged

Implement client certificate authentication #385

merged 8 commits into from
Nov 19, 2015

Conversation

tmds
Copy link
Contributor

@tmds tmds commented Nov 14, 2015

Implement #332
Replaces #351

@dnfclas
Copy link

dnfclas commented Nov 14, 2015

Hi @tmds, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@tmds
Copy link
Contributor Author

tmds commented Nov 14, 2015

\cc @davidfowl @Tratcher @halter73

@@ -12,6 +12,11 @@ public static class HttpsApplicationBuilderExtensions
{
public static IApplicationBuilder UseKestrelHttps(this IApplicationBuilder app, X509Certificate2 cert)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I remove this in favor of the HttpsConnectionFilterOptions overload?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet. This will be the most common configuration.

@tmds
Copy link
Contributor Author

tmds commented Nov 15, 2015

@benaadams
Copy link
Contributor

I'm having a possibility related issue: https://github.com/dotnet/corefx/issues/4533

using Microsoft.AspNet.Server.Kestrel.Filter;

namespace Microsoft.AspNet.Server.Kestrel.Https
{
public class HttpsConnectionFilter : IConnectionFilter
{
private readonly X509Certificate2 _cert;
private readonly X509Certificate2 _serverCert;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just store the Options object as a field rather than duplicating them all. E.g. https://github.com/aspnet/BasicMiddleware/blob/dev/src/Microsoft.AspNet.HttpOverrides/OverrideHeaderMiddleware.cs#L20

@Tratcher
Copy link
Member

{
previousPrepareRequest?.Invoke(features);

if (clientCertificate != null)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nicer if the code could use SslStream.RemoteCertificate instead of this variable. But RemoteCertificate isn't the same instance as the one passed to the validation callback. The validation callback gets an X509Certificate2 while RemoteCertificate is an X509Certificate.

@tmds
Copy link
Contributor Author

tmds commented Nov 16, 2015

@halter73 fyi, the client certificate is also working on coreclr.
One issue wasn't really an issue but an error by me (forgot to set HttpClient handler).
The other issue (converting X509Certificate to X509Certificates2) isn't causing a problem because the certificate passed to the validation callback is actually an X509Certificates2 so it can be casted.

@davidfowl
Copy link
Member

@tmds can you rebase?

new TlsConnectionFeature {ClientCertificate = clientCertificate});
}

features.Get<IHttpRequestFeature>().Scheme = "https";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes #365 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test to check this?

@halter73
Copy link
Member

:shipit:

if (clientCertificate != null)
{
features.Set<ITlsConnectionFeature>(
new TlsConnectionFeature {ClientCertificate = clientCertificate});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, more spacing

@Tratcher
Copy link
Member

:shipit:

@halter73 halter73 merged commit efec0fe into aspnet:dev Nov 19, 2015
@halter73
Copy link
Member

Thanks "Master T" 😄

@tmds
Copy link
Contributor Author

tmds commented Nov 19, 2015

No problem. Thanks for your feedback.

@Dev8063
Copy link

Dev8063 commented Jan 12, 2016

@tmds @davidfowl @Tratcher

Question - Do we have full support for WCF services, configured in a web.config using X509 certificates? I've got myself upgraded to RC1, but my WCF calls no longer work. Is there any documentation I can use? Do I need to use something other than the web.config to configure my WCF endpoints? Can you point me in the right direction or do we not have support for what I'm trying to do?

Let me know what's going on:

Could not find endpoint element with name '' and contract '' in the ServiceModel client configuration section. This might be because no configuration file was found for your application, or because no endpoint element matching this name could be found in the client element.

@davidfowl
Copy link
Member

Question - Do we have full support for WCF services, configured in a web.config using X509 certificates? I've got myself upgraded to RC1, but my WCF calls no longer work. Is there any documentation I can use? Do I need to use something other than the web.config to configure my WCF endpoints? Can you point me in the right direction or do we not have support for what I'm trying to do?

Not sure what you mean. ASP.NET 5 doesn't support WCF services.

@Dev8063
Copy link

Dev8063 commented Jan 12, 2016

@davidfowl Is there a way around this non-support for WCF services? I suppose we could programmatically load the endpoints. Would Kestrel work with that?

@Dev8063
Copy link

Dev8063 commented Jan 12, 2016

@davidfowl To Clarify, we just want to call WCF endpoints. We are not actually developing WCF endpoints on ASP.NET 5.

Can Kestrel support calling WCF endpoints with X509 certs (multiples)?

@benaadams
Copy link
Contributor

@Dev8063 sounds like you want the coreclr client libraries over here: https://github.com/dotnet/wcf

@Dev8063
Copy link

Dev8063 commented Jan 12, 2016

@benaadams We're not to the point where our app runs off of dnxcore50 (i.e. the subset meant for cloud dev). Do these work with dnx451? I wouldn't see why they would not, but just thought I'd check.

@benaadams
Copy link
Contributor

@Dev8063 you should be able to use regular libraries with the full framework; though you might want to ask in that repository or their glitter chat.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants