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
crypto/tls: client-cert EKU is enforced. #7423
Comments
Labels changed: added repo-main, release-go1.3maybe. Owner changed to @agl. Status changed to Thinking. |
Checking the EKU is pretty obscure and if the TLS code doesn't do it, I don't believe that most people will remember. This is exactly the sort of obscure check that should be done by default. I assume, since you are filing the bug, you have some client-certs that don't have the right EKU? Ideally you would create the certs with the correct EKU in the first place but, if not, can't you use RequireAnyClientCert and take care of the verification yourself, with any options you wish? |
I have some client certs that don't have an EKU at all. I don't think the Extended Key Usage should be checked by the tls code at all for these certificates. You can tell these because they start with "BEGIN CERTIFICATE" rather than "BEGIN TRUSTED CERTIFICATE". There appears to be no way to use a non EKU certificate as a client certificate at the moment because of the code above. I haven't been able to create a client certificate with the EKU that the tls code can read (see the -addtrust clientAuth bit above) the tls code doesn't seem to be able to read certificates which start with "BEGIN TRUSTED CERTIFICATE". As I see it client certs are in a kind of catch 22 place - they require the EKU, but the tls code can't read certificates in that format. An alternative might be another kind of verification maybe RequireAndVerifyClientCertIgnoringEKU, or RequireAndVerifyClientCert and RequireAndVerifyClientCertWithEku |
"BEGIN TRUSTED CERTIFICATE" is an OpenSSL specific format. It's not expected to be processed by Go code - they are not normal certificates. In OpenSSL EKUs are set by the extendedKeyUsage directive in openssl.cnf (see https://www.openssl.org/docs/apps/x509v3_config.html#Extended_Key_Usage_). The -addtrust option is something different. If you wish to do something different with the verification of client certs you can use RequireAnyClientCert and then verify the chain from tls.Conn.ConnectionState() however you wish. |
Thank you very much for the hint - I've manged to make client certificates which are acceptable to Go and work correctly using -extfile openssl.conf -extensions ssl_client And this in openssl.conf [ ssl_client ] extendedKeyUsage = clientAuth I now believe that this isn't a bug in tls, merely a bug in my understanding of openssl, so please accept my apologies and close the issue. I've updated the gist with a working version: https://gist.github.com/ncw/9253562 |
I would like to propose this issue is re-opened. I am using Go in an existing system where there are an existing set of certs that are already used for client authentication. Because of live upgrade considerations generating new certs is difficult and the existing certs do not specify EKU. From my reading of the standard the way we are currently using the certificates is compliant. One option I have to fit within this system is to patch the libraries which I'd rather not do. I'll try the @agl RequireAnyClientCert proposal above as well. If that works then this isn't as critical for me... From RFC3280:
RFC5280:
If I'm reading it correctly, the existing code (ftls/handshake_server.go) appears to treat a missing EKU the same as if the EKU is specified without the client auth usage but as far I can tell there's no real justification in the standard for doing this? ok := false
for _, ku := range certs[0].ExtKeyUsage {
if ku == x509.ExtKeyUsageClientAuth {
ok = true
break
}
} Further reference: I'd be happy to propose a patch if there's agreement there is an issue. Either changing the default behaviour or providing some way of overriding could solve my problem. Any other suggestions would be appreciated. |
This issue was closed over a year ago. Please open a new issue. It can refer to this one if you like. Thanks. |
The text was updated successfully, but these errors were encountered: