TLS: Check EKU in X509 chain checks #2670
Conversation
This is an additional check to match the .NET implementation for TLS cert checks so that we don't treat a cert flagged as non-TLS-server effectively. This ensures that a certificate either doesn't have OIDs here (valid, backwards compatible) or has the server-certificate OID indicating it's valid for consumption over TLS for us.
| }; | ||
| } | ||
|
|
||
| private static readonly Oid _serverAuthOid = new Oid("1.3.6.1.5.5.7.3.1", "1.3.6.1.5.5.7.3.1"); |
There was a problem hiding this comment.
Just wondering why not the friendly name parameter is something like serverAuth..? Or else using the single parameter constructor?
There was a problem hiding this comment.
Good question; in my local testing, the single .ctor usage gives the FriendlyName of "TLS Web Server Authentication" which is perhaps a little confusing since we're not really connection to a "web server", so perhaps new Oid("1.3.6.1.5.5.7.3.1", "TLS Server Authentication"); might be more sensible. Thoughts? @NickCraver also?
There was a problem hiding this comment.
I...didn't know that was a thing... Either sounds good here - cheers for the TIL!
There was a problem hiding this comment.
Mainly "because no one will look at the friendly name". The single param ctor causes the friendly name to be looked up, and Windows XP through Win 8.1 sometimes caused that to kick off a lookup over to the domain controller... so in the libraries we always use the two parameter version when the Oid instance won't be seen outside the library.
There was a problem hiding this comment.
Excellent context, thanks. I don't know if there's anything to actually "fix" here, but that's some great learnin'
6 participants
Further hardening following #2665. This is an additional check to match the .NET implementation for TLS cert checks so that we don't treat a cert flagged as non-TLS-server effectively. This ensures that a certificate either doesn't have OIDs here (valid, backwards compatible) or has the server-certificate OID indicating it's valid for consumption over TLS for us.
Cheers @bartonjs for the report and info here.