crypto/x509: Certificate.Verify does not consistently return UnknownAuthorityError

[rittneje]

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

$ go version
go version go1.19.6 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
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/rittneje/.cache/go-build"
GOENV="/home/rittneje/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/rittneje/go-workspace/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/rittneje/go-workspace"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/jrittner/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/rittneje/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19.6"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/rittneje/go-workspace/src/certtest/go.mod"
GOWORK=""
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 -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3903595383=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I made three certificates - root, intermediate, and leaf. The intermediate is already expired. I then ran Certificate.Verify on the leaf certificate.

https://go.dev/play/p/WIWEm7L0Bel

What did you expect to see?

I expected to get back an x509.CertificateInvalidError wrapped inside an x509.UnknownAuthorityError.

What did you see instead?

I got back an unwrapped x509.CertificateInvalidError. Consequently, the resulting error message has insufficient context.

This is due to a scoping/shadowing bug in Certificate.buildChains.

go/src/crypto/x509/verify.go

Lines 926 to 933 in dd16258

	err = candidate.isValid(certType, currentChain, opts)
	if err != nil {
	if hintErr == nil {
	hintErr = err
	hintCert = candidate
	}
	return
	}

That err is the named return value from the outer scope. So at the end of the function, because it is non-nil, it is never wrapped.

go/src/crypto/x509/verify.go

Lines 955 to 957 in dd16258

	if len(chains) == 0 && err == nil {
	err = UnknownAuthorityError{c, hintErr, hintCert}
	}

[dmitshur]

Thanks for catching and reporting this.

CC @golang/security.

[rolandshoemaker]

Probably we should (a) always return a UnknownAuthorityError from Verify, wrapping errors discovered during chain building and (b) implement Unwrap on UnknownAuthorityError so that the hint can be accessed, since it is essentially a wrapped error.

There are currently cases where we expect to return a different error type though (as indicated by the fact we explicitly test for those cases). Perhaps it's reasonable to break those, especially if we implement Unwrap, but it is worth consideration.