encoding/asn1: time.Time values can be encoded incorrectly

[ecnepsnai]

Go version

1.21.5

What operating system and processor architecture are you using (go env)?

GO111MODULE='on'
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/ian/Library/Caches/go-build'
GOENV='/Users/ian/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/ian/Developer/go/pkg/mod'
GOOS='darwin'
GOPATH='/Users/ian/Developer/go'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go/current'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/current/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.21.5'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
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/72/2kjqjqwd61d83xhhzwptf_sc0000gn/T/go-build3674955566=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

The encoding/asn1 package supports marshalling time.Time structures to either a ASN.1 UTCTime or GeneralizedTime. The behaviour is controlled by using struct tags or passing parameters with asn1.MarshalWithParams, but by default the package will use UTCTime. (Aside: this seems to be a really poor choice for a default behaviour, considering UTCTime is not Y2K compliant)

However, it's possible for the asn1 package to encode an invalid GeneralizedTime value without producing an error.

Take the following example:

t := time.Date(2023, 12, 28, 16, 49, 55, 00, time.UTC)
tVal, err := asn1.MarshalWithParams(t, "tag:24")
if err != nil {
	panic(err)
}
// tVal = 98 0d 32 33 31 32 32 38 31 36 34 39 35 35 5a (..231228164955Z)

Here, we're passing the ASN1 tag for GeneralizedTime (24), but the value of tVal only uses 2 digits for the year because it's actually being encoded as UTCTime.

Despite me telling the package that I want to use GeneralizedTime, I have to explicitly tell it with a second parameter:

t := time.Date(2023, 12, 28, 16, 49, 55, 00, time.UTC)
tVal, err := asn1.MarshalWithParams(t, "tag:24,generalized")
if err != nil {
	panic(err)
}
// tVal = 18 0f 32 30 32 33 31 32 32 38 31 36 34 39 35 35 5a (..20231228164955Z)

Go playground: https://go.dev/play/p/WSxtB5cOMaQ

What did you expect to see?

I expected the asn1 package to encode the time using GeneralizedTime as I had specified using the ASN1 tag value. I also expected that the default encoding would be the Y2K-compliant GeneralizedTime. Perhaps a separate discussion is needed as to if this should be changed going forward.

What did you see instead?

A confusion between a ASN1 field that is tagged as GeneralizedTime but actually has a UTCTime value.

[mateusz834]

When outside of the UTCTime range, it will be encoded as GeneralizedTime.

go/src/encoding/asn1/marshal.go

Lines 655 to 659 in 988b718

	case TagUTCTime:
	if params.timeType == TagGeneralizedTime || outsideUTCRange(v.Interface().(time.Time)) {
	tag = TagGeneralizedTime
	}
	}

Despite me telling the package that I want to use GeneralizedTime, I have to explicitly tell it with a second parameter:

t := time.Date(2023, 12, 28, 16, 49, 55, 00, time.UTC)
tVal, err := asn1.MarshalWithParams(t, "tag:24,generalized")
if err != nil {
	panic(err)
}

Doesn't that make sense? There might be an implicit tag used, so tag:24 should default to UTCTime (as done currently).

[ecnepsnai]

Hi @mateusz834, thanks for replying!

So where my confusion comes from, and why I believe this is a bug, is that because I had specified the tag for GeneralizedTime (24) I expected the asn1 package to use that time format.

It doesn't make sense to me that you would want a field tagged as GeneralizedTime but with a UTCTime value, when they aren't compatible.

I feel the asn1 package should reference a tag value (if specified) when determining what time format to use, if no format was explicitly set.

[mateusz834]

From the encoding/asn1 docs:

tag:x specifies the ASN.1 tag number; implies ASN.1 CONTEXT SPECIFIC

so this is not a 24 + Universal bits (24), but rather 24 + Context-specific bits (152).

[mateusz834]

But i think that this behavior is wrong, in my opinion this should use tag 24 + Context-specific bit, rather than 24 + Universal bit.

t := time.Date(2023, 12, 28, 16, 49, 55, 00, time.UTC)
tVal, err := asn1.MarshalWithParams(t, "tag:24,generalized")
if err != nil {
	panic(err)
}
// tVal = 18 0f 32 30 32 33 31 32 32 38 31 36 34 39 35 35 5a (..20231228164955Z)

EDIT: actually it returns the correct tag, see https://go.dev/play/p/xc0pfnyWyXY

[bcmills]

(attn @FiloSottile @rolandshoemaker @golang/security per https://dev.golang.org/owners)

[gopherbot]

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)