crypto/x509: Certificate no longer encodable using encoding/gob in Go1.22

[VictorLowther]

Go version

go 1.22.0 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/victor/.cache/go-build'
GOENV='/home/victor/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/victor/go/pkg/mod'
GONOPROXY='gitlab.com/rackn'
GONOSUMDB='gitlab.com/rackn'
GOOS='linux'
GOPATH='/home/victor/go'
GOPRIVATE='gitlab.com/rackn'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/lib/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.0'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2733416375=/tmp/go-build -gno-record-gcc-switches'

What did you do?

A project of mine directly embeds the *x509.Certificate struct in a larger struct, which is encoded and decoded using encoding/gob. The struct added fields that use a new x509.OID structure, which contains a single non-exported field.

Test program is here on the Go playground. Running it under Go 1.22 displays the error, and running it under Go 1.21 does not.

What did you see happen?

go run main.go
2024/02/09 08:42:03 Failed to gob encode enpty x.509 cert: gob: type x509.OID has no exported fields

What did you expect to see?

go run main.go
2024/02/09 08:41:47 Encoded empty x.509 cert

Adding MarshalBinary and UnmarshalBinary methods to *x509.OID allows encoding to succeed. Patch against 1.22.0:

diff --git a/src/crypto/x509/oid.go b/src/crypto/x509/oid.go
index 5359af624b..0e2b872539 100644
--- a/src/crypto/x509/oid.go
+++ b/src/crypto/x509/oid.go
@@ -24,6 +24,15 @@ type OID struct {
        der []byte
 }

+func (o *OID) MarshalBinary() ([]byte, error) {
+       return append([]byte{}, o.der...), nil
+}
+
+func (o *OID) UnmarshalBinary(b []byte) error {
+       o.der = append(o.der[:0], b...)
+       return nil
+}
+
 func newOIDFromDER(der []byte) (OID, bool) {
        if len(der) == 0 || der[len(der)-1]&0x80 != 0 {
                return OID{}, false

[mateusz834]

CC @rolandshoemaker

[Akaame]

Has there been any progress on this matter?

[VictorLowther]

As far as I can tell, no. I didn'r see a fix go into 1.22.1

[rolandshoemaker]

While I'm not strictly opposed to fixing this, we've never explicitly claimed that Certificate is compatible with encoding/gob, so I don't think we will consider this a breaking change, and I'm not particularly sure how much I want to encourage using encoding/gob here.

Is there a particular reason to use encoding/gob over the DER encoding of a certificate (which is available via Certificate.Raw), which is likely to be significantly smaller than a gob encoded struct (which will, itself, contain the raw DER encoding of the certificate, since it is a part of the struct)?

If its just because it's embedded in another struct, this problem can be worked around by defining your own type which wraps x509.Certificate (i.e. type gobCert *x509.Certificate) and implements MarshalBinary/UnmarshalBinary methods which encode it (i.e. by just returning the Raw field, and calling x509.ParseCertificate respectively). This may actually end up being more performant because of the specialized nature of the x509 parser. i.e. https://go.dev/play/p/exGyeSWuFAL.

[VictorLowther]

The struct in question is embedded in the replicated state machine for a Raft cluster, making dealing with marshalling changes on the fly like that challenging -- changing how the struct is marshalled like that would involve manual intervention and downtime for our customers, whereas encoding/gob will transparently just work across changes like this as long as the OID field implements MarshalBinary/UnmarshalBinary with some possibly minor fixups to arrange for the Policies field to be filled out if it isn't already.

And while y'all never claimed that Certificate is compatible with encoding/gob, Hyrum's Law still applies and y'all made a change that broke encoding/gob that could have been avoided by making the OID a []byte directly instead of doing the single private struct field thing, and making it the only thing in an x509.Certificate that is not exported.

[rsc]

There are ways to work around this, yes, but if gob worked before on these types we should probably keep it working one way or another. We have run into similar issues in the past and in general treat those wire changes as subject to compatibility. Unfortunately, the fix seems to be to add new methods, which as a matter of policy we don't do in point releases. So the fix (new methods) would have to wait for Go 1.23. Unless we have evidence of widespread impact, that may be the best approach.

[tycho]

How widespread is widespread enough? I found this issue because it was causing builds of Mattermost and its plugins to fail to work properly. To work around it I had to modify the build to avoid using the system-installed Go and download a Go 1.21 binary.

[rsc]

Filed #66249. Thanks for that extra impact information @tycho. That's more impact than I realized.

[rsc]

@gopherbot please backport go1.22

[gopherbot]

Backport issue(s) opened: #66273 (for 1.22).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

[gopherbot]

Change https://go.dev/cl/571095 mentions this issue: encoding/gob: make x509.Certificate marshalable again

[rsc]

I spoke to @robpike about this yesterday. We think the simplest, least invasive change for Go 1.22.x would be to change gob to look for the x509.Certificate type and have it skip over the Policies field entirely. That seems like it would fix Mattermost and other affected users, and then we can proceed with new methods on the normal timeline and then remove the gob change. The pending CL 571095 does this.