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: Support providing additional _intermediate_ certs for verification in tls.Config #31791

Closed
alex opened this issue May 1, 2019 · 3 comments

Comments

@alex
Copy link
Contributor

@alex alex commented May 1, 2019

What version of Go are you using (go version)?

$ go version
go version go1.12.4 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/alex/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/alex/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build103295892=/tmp/go-build -gno-record-gcc-switches"

What did you do?

In a TLS connection, the server is supposed to send a chain of certificates that connects from the leaf to the root founding in a client's trust store. Unfortunately some servers are misconfigured, and do not send the necessary intermediates (generally they only serve a leaf, although every once in a while you see one that serves the wrong intermediate).

In order to work around this, developers often find either the leaf or the intermediate for the server they're trying to communicate with, and add it to the trust store. This is unfortunate because it's relatively brittle (particularly if they add the leaf, which should rotate regularly), and it's less secure -- if the root is removed from their trust store, connections will continue to be trusted.

Instead, if there was a way to provide tls.Config with additional Intermediates, this could be used more safely -- it would not override the trusted roots, merely allow users to compensate for misconfigured servers.

I believe fixing this would look like adding a Intermediates x509.CertPool onto tls.Config and then copying it over for verification around here https://github.com/golang/go/blob/master/src/crypto/tls/handshake_client.go#L827 (and the equivalent for the server side, naturally).

@FiloSottile

This comment has been minimized.

Copy link
Member

@FiloSottile FiloSottile commented May 1, 2019

This is possible by setting InsecureSkipVerify and VerifyPeerCertificate. I'm inclined not to add a dedicated option because it's not that much of a common use case, and I suspect that it might require custom logic based on the peer's certificates anyway, so the VerifyPeerCertificate callback is the right place for that logic.

Opened #31792 to add an example.

@alex

This comment has been minimized.

Copy link
Contributor Author

@alex alex commented May 1, 2019

Thanks for the quick reply! I'll defer my contemplation of whether I want to advocate more for this proposal until the example is added to the docs, so I can have a sense of what the level of complexity involved is. Thanks!

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Sep 5, 2019

Change https://golang.org/cl/193620 mentions this issue: crypto/tls: add ExampleConfig_VerifyPeerCertificate

gopherbot pushed a commit that referenced this issue Nov 9, 2019
Setting InsecureSkipVerify and VerifyPeerCertificate is the recommended
way to customize and override certificate validation.

However, there is boilerplate involved and it usually requires first
reimplementing the default validation strategy to then customize it.
Provide an example that does the same thing as the default as a starting
point.

Examples of where we directed users to do something similar are in
issues #35467, #31791, #28754, #21971, and #24151.

Fixes #31792

Change-Id: Id033e9fa3cac9dff1f7be05c72dfb34b4f973fd4
Reviewed-on: https://go-review.googlesource.com/c/go/+/193620
Reviewed-by: Adam Langley <agl@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.