go: crypto/x509: verification fails with "cannot parse dnsName" in intermediate
What version of Go are you using (go version)?
go version go1.10 darwin/amd64
Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (go env)?
GOARCH=“amd64” GOBIN=“” GOCACHE=“/Users/shannon/Library/Caches/go-build” GOEXE=“” GOHOSTARCH=“amd64” GOHOSTOS=“darwin” GOOS=“darwin” GOPATH=“/Users/shannon/go” GORACE=“” GOROOT=“/usr/local/go” GOTMPDIR=“” GOTOOLDIR=“/usr/local/go/pkg/tool/darwin_amd64” GCCGO=“gccgo” CC=“clang” CXX=“clang++” CGO_ENABLED=“1” 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=/var/folders/08/9qf64r3d209cvmgznzj49zg40000gn/T/go-build638418624=/tmp/go-build -gno-record-gcc-switches -fno-common”
What did you do?
So it seems at some point in our distant past, we ended up with an intermediate CA that had an altname incorrectly configured. Probably something we munged while configuring Hashicorps Vault as our CA so many months ago.
The end result being that anything written in go and compiled with go 1.10 now fails all our tls connections (where curl, and openssl succeed)
- Example with a dodgy intermediate: https://play.golang.org/p/SeS_uNyqnci
- Same code with a better intermediate: https://play.golang.org/p/-JaU_x318rO
nb, these examples don’t actually run on play due to the time being pinned in the past 😃
In the real world these certs are part of a bundle offered up by nginx, caddy, vault, consul, and individual services so its a tls error like this we see
tls: failed to parse certificate from server: x509: cannot parse dnsName "SnakeOil Intermediate CA 2"
One might argue that the intermediate certificates are invalid, but another equally fair argument is that maybe we don’t need to verify altnames on certificates that are marked for CA’s
X509v3 Basic Constraints: critical
CA:TRUE
The only evidence I have to support that is that curl, openssl, and chrome don’t seem to care.
What did you expect to see?
yay
What did you see instead?
panic: x509: cannot parse dnsName "SnakeOil Intermediate CA 2"
goroutine 1 [running]:
main.main()
/Users/shannon/tlsissue/ca/test/main.go:164 +0x59f
exit status 2
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Reactions: 17
- Comments: 25 (12 by maintainers)
Commits related to this issue
- ci: Use go 1.9 Ilya noticed that the dnsName parser has got more stringent. https://github.com/golang/go/issues/23995 Stick with go 1.9 until that's addressed. — committed to weaveworks/launcher by dlespiau 6 years ago
- [release-branch.go1.10] crypto/x509: parse invalid DNS names and email addresses. Go 1.10 requires that SANs in certificates are valid. However, a non-trivial number of (generally non-WebPKI) certifi... — committed to golang/go by agl 6 years ago
- Use go1.10.1 and go1.9.5 The latest patch versions include two important fixes for the Prometheus project: * https://github.com/golang/go/issues/23995 * https://github.com/golang/go/issues/24059 — committed to prometheus/golang-builder by grobie 6 years ago
- Use go1.10.1 and go1.9.5 The latest patch versions include two important fixes for the Prometheus project: * https://github.com/golang/go/issues/23995 * https://github.com/golang/go/issues/24059 — committed to prometheus/golang-builder by grobie 6 years ago
- Use go1.10.1 and go1.9.5 (#39) The latest patch versions include two important fixes for the Prometheus project: * https://github.com/golang/go/issues/23995 * https://github.com/golang/go/issues... — committed to prometheus/golang-builder by grobie 6 years ago
- Switch from Go 1.10 to 1.11.1 Using Kubernetes from Docker for Mac I ran into this error with the scf-secrets-generator: 2018/10/30 14:20:06 Post https://10.96.0.1:443/api/v1/namespaces/my-nats/conf... — committed to SUSE/scf-helper-release by jandubois 6 years ago
I would definitely be in favor of making CreateCertificate incapable of generating bad certs, but breaking certs that are already out there is causing inconvenience. In fact I’m pro breaking those certs too, but it’d be nice if that was some how optional because people have to work.
I made the title more discoverable, let’s see if this affects a lot of people. (If this affects you, please comment or use the GitHub reaction feature.) I would like Go to be a force for cleanup in the X.509 ecosystem too, but a widespread regression would not be acceptable.
We should check that CreateCertificate will refuse to make invalid dnsNames too before closing.
In my opinion this isn’t a regression in Go. It is an error to put something that isn’t a DNS hostname in a dNSName SAN, and failing to parse/verify such certificates is a reasonable and good thing.
It’s unfortunate that some software was generating invalid certificates, but I think they should be replaced instead of letting Go’s (already intentionally strict) parser accept them.
This will affect all CA certs that have been generated by HashiCorp Vault 0.9.3 or earlier. Those versions always put the CN also into the DNS alt names, without checking if it actually is a DNS name. hashicorp/vault#3953 fixed that in anticipation of the change in Go.
Relaxing the verification for CA certs would greatly reduce pain for Vault operators because they wouldn’t have to re-issue all of their cert chains.
Reopening because of potential 1.10.1 inclusion.
Change https://golang.org/cl/96378 mentions this issue:
crypto/x509: allow invalid SANs in CA certificates.@titanous
The problem is that such software relying on Go for their crypto stack were allowed, by Go, to generate such invalid certificates, and then disallowed, by Go, to actually read them back once Go 1.10 came out. If Go had never allowed these certificates to be generated in the first place due to strict parsing and validity requirements, I’d be more agreeable to your statement, but I strongly suggest an “allow old/broken certs, disallow generating new broken certs” approach based on where we are right now.
We can all agree it would be nice if such broken certs didn’t exist in the first place, but there are plenty of things in the RFC that various libraries only adhere to to some extent or another. In fact, the
x509library explicitly breaks with the spec with verify behavior (https://golang.org/pkg/crypto/x509/#VerifyOptions). Allowing non-DNS-compatible names in DNSNames was likely an unintentional break, unlike the verification logic, but if every crypto stack out there has been successfully accepting such certs that Go created, and Go has been successfully accepting such certs that other CAs created, it’s reasonable for users to assume that this was simply a widely-allowed relaxation of the spec (especially given the complexity of dealing with otherNames with Go’sasn1library, which pretty much ends up necessitating using the out-of-stdlibcryptobyteslib).Given the unexpected spread of this mistake, I’m now at “weak yes” for including this change.
This currently affects our entire vault setup as-well, due to reasons already mentioned by @pschultz. This forces us to re-issue all our CAs, including our root-ca. Making this verification optional would really help us out a lot.
@jamie-digital I found the source of that certificate, and it’s a weird artefact of a breach. The other certificates were for real sites. https://bugzilla.mozilla.org/show_bug.cgi?id=642395
This means it was never trusted as a root, and with
CA:FALSEit’s harmless, so in this case you can safely drop checking for it.