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.

Sign up for GitHub

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

Jump to bottom

crypto/tls: Support providing additional _intermediate_ certs for verification in tls.Config #31791

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

crypto/tls: Support providing additional _intermediate_ certs for verification in tls.Config #31791

alex opened this issue May 1, 2019 · 3 comments
Labels
FrozenDueToAge Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone
Proposal

Comments

@alex
Copy link
Contributor

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
Copy link
Contributor

FiloSottile commented May 1, 2019 •
edited

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.

@FiloSottile FiloSottile added Proposal Proposal-Crypto Proposal related to crypto packages or other security issues labels May 1, 2019
@FiloSottile FiloSottile added this to the Proposal milestone May 1, 2019
@FiloSottile FiloSottile mentioned this issue May 1, 2019
Closed
@alex
Copy link
Contributor Author

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
Copy link

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
@FiloSottile
crypto/tls: add ExampleConfig_VerifyPeerCertificate
29cfb4d 
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>
@golang golang locked and limited conversation to collaborators Sep 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
None yet
Milestone
Proposal
Development

No branches or pull requests
3 participants
@alex @FiloSottile @gopherbot