Repository: go
Issue: encoding/asn1: invalid DER encodings of `GeneralizedTime` when time is not UTC
Labels: NeedsInvestigation

### Go version

go version go1.23.2 darwin/arm64

### Output of `go env` in your module/workspace:

```shell
GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/william/Library/Caches/go-build'
GOENV='/Users/william/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/william/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/william/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.23.2/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.23.2/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.23.2'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/william/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='cc'
CXX='c++'
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 -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/k6/zk7vnkms1m73w0vrxjm7b2s40000gn/T/go-build3515925324=/tmp/go-build -gno-record-gcc-switches -fno-common'
```


### What did you do?

My colleague @darkamaul noticed an invalid `GeneralizedTime` encoding while tracking down some DER decoding failures in the responses produced by a Go implementation of an [RFC 3161 Time Stamp Authority].

[RFC 3161 Time Stamp Authority]: https://www.ietf.org/rfc/rfc3161.txt

### What did you see happen?

We observed DER encodings of `GeneralizedTime` objects with explicit timezone offsets, e.g.:

```
GeneralizedTime 2024-10-04 11:04:31 UTC+02:00
```

This is an invalid DER encoding of a `GeneralizedTime`, per the DER encoding rules defined in ITU-T X.690. In particular, DER requires that all `GeneralizedTime` encodings be UTC time with the `Z` designator per X.690 11.7.1:

> The encoding shall terminate with a "Z", as described in the ITU-T Rec. X.680 | ISO/IEC 8824-1 clause on GeneralizedTime. 

(Ref: <https://www.itu.int/ITU-T/studygroups/com17/languages/X.690-0207.pdf>, page 19)


After looking into it, we determined that the codebase was using `encoding/asn1`'s `Marshal` implementation, in particular for marshalling `time.Time` objects into `GeneralizedTime` encodings.

For example:

```go
// eContent within SignedData is TSTInfo
type tstInfo struct {
	// .. snip
	Time           time.Time        `asn1:"generalized"`
	// .. snip
}
```

Permalink: <https://github.com/digitorus/timestamp/blob/220c5c2851b7435eea999de3daa773601a7ca126/rfc3161_struct.go#L57>

We then checked the underlying `Marshal` implementation and its `GeneralizedTime` helper (`appendGeneralizedTime`), and confirmed that it emits a relative offset instead of normalizing to UTC when the origin `time.Time` is not already UTC:

```go
func appendGeneralizedTime(dst []byte, t time.Time) (ret []byte, err error) {
	year := t.Year()
	if year < 0 || year > 9999 {
		return nil, StructuralError{"cannot represent time as GeneralizedTime"}
	}

	dst = appendFourDigits(dst, year)

	return appendTimeCommon(dst, t), nil
}

func appendTimeCommon(dst []byte, t time.Time) []byte {
	_, month, day := t.Date()

	dst = appendTwoDigits(dst, int(month))
	dst = appendTwoDigits(dst, day)

	hour, min, sec := t.Clock()

	dst = appendTwoDigits(dst, hour)
	dst = appendTwoDigits(dst, min)
	dst = appendTwoDigits(dst, sec)

	_, offset := t.Zone()

	switch {
	case offset/60 == 0:
		return append(dst, 'Z')
	case offset > 0:
		dst = append(dst, '+')
	case offset < 0:
		dst = append(dst, '-')
	}

	offsetMinutes := offset / 60
	if offsetMinutes < 0 {
		offsetMinutes = -offsetMinutes
	}

	dst = appendTwoDigits(dst, offsetMinutes/60)
	dst = appendTwoDigits(dst, offsetMinutes%60)

	return dst
}
```

Ref: https://cs.opensource.google/go/go/+/refs/tags/go1.23.2:src/encoding/asn1/marshal.go;l=405-448

Based on the blame, this offset encoding has been present since at least 2011 and possibly earlier.

### What did you expect to see?

We expected `encoding/asn1` to produce only valid DER encodings, which in this case means producing a `GeneralizedTime` with only a `Z` timezone component, and no relative timezone offsets.

To achieve this, we _believe_ the `Marshal` implementation can be tweaked to call `UTC()` before performing encoding, which would normalize the `time.Time` into UTC form. The special-casing around relative offsets could then be removed entirely, as all encoded times would be UTC.

Similarly, we believe (but haven't concretely observed) that `encoding/asn1`'s `Unmarshal` accepts invalid DER encodings of `GeneralizedTime`s, per its format string:

```go
// parseGeneralizedTime parses the GeneralizedTime from the given byte slice
// and returns the resulting time.
func parseGeneralizedTime(bytes []byte) (ret time.Time, err error) {
	const formatStr = "20060102150405.999999999Z0700"
	s := string(bytes)

	if ret, err = time.Parse(formatStr, s); err != nil {
		return
	}

	if serialized := ret.Format(formatStr); serialized != s {
		err = fmt.Errorf("asn1: time did not serialize back to the original value and may be invalid: given %q, but serialized as %q", s, serialized)
	}

	return
}
```

Ref: https://cs.opensource.google/go/go/+/refs/tags/go1.23.2:src/encoding/asn1/asn1.go;l=368-383

If our understanding of `time.Parse` is correct, this will admit multiple invalid DER encodings:

* The fractional component `.999999999` will allow trailing zeroes, which are allowed in BER but not in DER;
* The timezone component `Z0700` allows both `Z` and relative timezone offsets, when only `Z` is allowed in DER.