crypto/x509: ParseCertificate and ParseCertificates return different errors with a single bad certificate supplied

[LujunWeng]

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

$ go version
go version go1.15.6 darwin/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="/Users/******/Library/Caches/go-build"
GOENV="/Users/******/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/******/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/******/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
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=/var/folders/nm/1lmrr0s56lvb_00hv_ydg3300000gn/T/go-build015401011=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

package main

import (
	"crypto/x509"
	"encoding/pem"
	"fmt"
)

const badCerts = `-----BEGIN CERTIFICATE-----
-----END CERTIFICATE-----`

func useParseCertificate(certpem string) {
	block, _ := pem.Decode([]byte(certpem))
	if block == nil {
		fmt.Printf("Failed in parsing pem string\n")
		return
	}

	cert, err := x509.ParseCertificate(block.Bytes)
	fmt.Printf("Certificate: %v, Error: %v\n", cert, err)
}

func useParseCertificates(certpem string) {
	block, _ := pem.Decode([]byte(certpem))
	if block == nil {
		fmt.Printf("Failed in parsing pem string\n")
		return
	}

	certs, err := x509.ParseCertificates(block.Bytes)
	fmt.Printf("Certificates: %v, Error: %v\n", certs, err)
}

func main() {
	useParseCertificate(badCerts)
	useParseCertificates(badCerts)
}

What did you expect to see?

Both x509.ParseCertificate and x509.ParseCertificates return non-nil error.

What did you see instead?

x509.ParseCertificate returns a non-nil error while x509.ParseCertificates returns nil for error.
Output

Certificate: <nil>, Error: asn1: syntax error: sequence truncated
Certificates: [], Error: <nil>

It could be argued that x509.ParserCertificates returns empty slice in this case, but I don't think it can justify a "nil" error. Especially, the returned results are inconsistent here.

[toothrot]

/cc @FiloSottile

[antong]

x509.ParseCertificates is for parsing a sequence of concatenated, DER encoded certificates. It makes no sense to parse a single certificate PEM block and feed the DER to x509.ParseCertificates. The block contains only a single certificate. It is common to have multiple certificates in the same text, but then you would have concatenated PEM blocks. You would then decode the PEM blocks in a loop, checking for the block type (i.e., "CERTIFICATE"), and calling x509.ParseCertificate separately for each parsed block. Look at x509.AppendCertsFromPEM for an example.

The issue is about how the ParseCertificate and ParseCertificates functions behave. These take DER as input, so the initial PEM or invalid text form is irrelevant. Here is an equivalent, reduced version of the code example in the issue description, with the text and PEM decoding removed: https://play.golang.org/p/qC-VAaodZ9T

This shows how the functions don't really know about the invalid "badCerts". ParseCertificates does not have the information that there ever was an ill-formed PEM certificate. I would say it is the expected behavior for x509.ParseCertificates to return the empty slice without errors when given a zero length input. It is much like that splitting the empty string gives an empty slice. Bonus: And it has the distributive property "parse(der1+der2) = parse(der1)+parse(der2)".