Validate certificate EKU

[danroth27]

When setting up SSL with Kestrel if the cert has an EKU we should validate that the cert is for server usage.

[muratg]

@blowdart your thoughts on this one? (OK to do in 2.1?)

[blowdart]

No, as we patched full framework to check for this last month. It needs to be right on the server too.

[natemcmaster]

@muratg @halter73 I might be able to poach this one if we still want to get it in for 2.0.0

[muratg]

Thanks @natemcmaster! If we can't get to it, it's probably OK.

[davidfowl]

Is this really important for 2.0? Wasn't this for the identity service scenario?

[danroth27]

@davidfowl This was requested by @blowdart and applies to HTTPS with Kestrel independent of the identity as a service work.

[blowdart]

It's very separate.

Both Core and Framework now check the EKUs of the servers they're attaching to. So if you bind an invalid certificate then HttpClient et al will refuse to connect. This check on startup ensures that you're not using a certificate that isn't meant to be for a server, and so warns you a lot earlier than waiting for an HttpClient to attach.

It's a nice to have, but it's very very very nice, because it'll make things easier to diagnose.

[davidfowl]

I think it should be moved to 2.1 but if @natemcmaster can do it and it's low risk (has a low chance of regressions on all 3 OSes) then sure.

[natemcmaster]

The check is actually fairly simple. The hardest part of doing this was figuring out how to make a test cert with an EKU extension. I have a PR open to implement EKU validation here: #1940.

[muratg]

@halter73 / @davidfowl could you review the PR?

@natemcmaster if this is risky, let's move it out to 2.1.

[natemcmaster]

In #1940, we added validation of the Extended Key Usage extension. In #1944, I'm adding the validation for the normal Key Usage extension as well. According to the RFC, validation of these extensions is an orthogonal concern so we don't need both, however, it seems best to be consistent and check both types of key usage extensions.

[natemcmaster]

Talked with @blowdart in person. We can address the Key Usage extension separately, and do that post 2.1. Closing this as resolved with changes in #1940. I've opened #1946 to address this.