Skip to content
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

Closed
ncw opened this issue Feb 27, 2014 · 8 comments
Closed

crypto/tls: client-cert EKU is enforced. #7423

ncw opened this issue Feb 27, 2014 · 8 comments
Milestone

Comments

@ncw
Copy link
Contributor

ncw commented Feb 27, 2014

I think Go is making a mistake in assuming that client certificates
without the extended usage attributes (ie most client certificates)
should be denied use as a client authentication.

What steps will reproduce the problem?

Here are a couple of programs and a script to demonstrate the problem

https://gist.github.com/ncw/9253562

What is the expected output?

Working SSL connection

What do you see instead?

tls: client's certificate's extended key usage doesn't permit it to be used for client
authentication

Which operating system are you using?

Linux Ubuntu 13.10

Which version are you using?  (run 'go version' or 'gccgo --version')

(latest head)

go version devel +abd51e52924a Thu Feb 27 01:45:22 2014 -0800 linux/amd64

Please provide any additional information below.

Here is the README from the gist

This demonstrates a bug in Go

First generate the certificates with

./makecert.sh test@test.com

Run the server in one terminal

go run server.go

Run the client in the other

go run client.go

You'll see an SSL negotiation failure

The Server says

2014/02/27 16:33:51 server: listening
2014/02/27 16:33:53 server: accepted from 127.0.0.1:43700
2014/02/27 16:33:53 server: conn: type assert to TLS succeedded
2014/02/27 16:33:53 server: handshake failed: tls: client's certificate's extended key
usage doesn't permit it to be used for client authentication
exit status 1

And the Client says

2014/02/27 16:33:53 client: dial: remote error: handshake failure
exit status 1

If you then try to add the extended usage to the certificate with the -addtrust
clientAuth (uncomment in makecert.sh) you get this from the Client

2014/02/27 16:36:58 server: loadkeys: crypto/tls: failed to parse certificate PEM data
exit status 1

Because the client can no longer read the certificate which now starts with

-----BEGIN TRUSTED CERTIFICATE-----
MIIDizCCAnMCAQEwDQYJKoZIhvcNAQEFBQAwgZExCzAJBgNVBAYTAkRFMQwwCgYD

The relevant code is in crypo/tls/handshake_server.go

        ok := false
        for _, ku := range certs[0].ExtKeyUsage {
            if ku == x509.ExtKeyUsageClientAuth {
                ok = true
                break
            }
        }
        if !ok {
            c.sendAlert(alertHandshakeFailure)
            return nil, errors.New("tls: client's certificate's extended key usage doesn't permit it to be used for client authentication")
        }

I think Go is making a mistake in assuming that client certificates
without the extended usage attributes (ie most client certificates)
should be denied use as a client authentication.

So removing this code would fix the problem or possibly Go should
assume certificates without the extended trust attributes should be
allowed rather than rejected.

From the OpenSSL docs

http://www.openssl.org/docs/apps/x509.html

> Trust settings currently are only used with a root CA. They allow a
> finer control over the purposes the root CA can be used for. For
> example a CA may be trusted for SSL client but not SSL server use.
>
> See the description of the verify utility for more information on
> the meaning of trust settings.
>
> Future versions of OpenSSL will recognize trust settings on any
> certificate: not just root CAs.

Indicating that OpenSSL doesn't use these trust settings at all
(confirmed with my experiments with Apache).
@rsc
Copy link
Contributor

rsc commented Mar 3, 2014

Comment 1:

Labels changed: added repo-main, release-go1.3maybe.

Owner changed to @agl.

Status changed to Thinking.

@agl
Copy link
Contributor

agl commented Mar 5, 2014

Comment 2:

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?

@ncw
Copy link
Contributor Author

ncw commented Mar 5, 2014

Comment 3:

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

@agl
Copy link
Contributor

agl commented Mar 5, 2014

Comment 4:

"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.

@ncw
Copy link
Contributor Author

ncw commented Mar 5, 2014

Comment 5:

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

@rsc
Copy link
Contributor

rsc commented Apr 3, 2014

Comment 6:

Status changed to WorkingAsIntended.

@rsc rsc added this to the Go1.3 milestone Apr 14, 2015
@GuySirton
Copy link

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:

This extension MAY, at the option of the certificate issuer, be either critical or non-critical. If the extension is present, then the certificate MUST only be used for one of the purposes indicated.

RFC5280:

If the extension is present, then the certificate MUST only be used for one of the purposes indicated.

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:
http://tools.ietf.org/html/rfc5280#section-4.2.1.12
http://blogs.msdn.com/b/kaushal/archive/2012/02/18/client-certificates-v-s-server-certificates.aspx
http://security.stackexchange.com/questions/68491/recommended-key-usage-for-a-client-certificate

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.

@ianlancetaylor
Copy link
Contributor

This issue was closed over a year ago. Please open a new issue. It can refer to this one if you like. Thanks.

This issue was closed.
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