golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124k stars 17.67k forks source link

encoding/asn1: valid GeneralizedTime not parsed #15842

Closed ggeorgiev closed 1 year ago

ggeorgiev commented 8 years ago

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)? go version go1.6.2 darwin/amd64
  2. What operating system and processor architecture are you using (go env)? GOARCH="amd64" GOBIN="" GOEXE="" GOHOSTARCH="amd64" GOHOSTOS="darwin" GOOS="darwin" GOPATH="/Users/.../golang" GORACE="" GOROOT="/usr/local/go" GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64" GO15VENDOREXPERIMENT="1" CC="/usr/local/bin/gcc-5" GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fno-common" CXX="/usr/local/bin/c++-5" CGO_ENABLED="1"
  3. What did you do? I Unmarshall 3rdparty asn1 data
  4. What did you expect to see? It to be unmarshalled successfully it is correct.
  5. What did you see instead? asn1: time did not serialize back to the original value and may be invalid: given "20160525195606.36Z", but serialized as "20160525195606Z"

Note that the data is 3rdparty, I have no control over it. I see that golang asn1 disclames limited support. I am curious what are my options to workaround the issue - is it just a fork of the original library or there are other ways to hook my own parser for the GeneralizedTime?

ianlancetaylor commented 8 years ago

We don't use the issue tracker for questions. Please https://golang.org/wiki/Questions . I don't know the answer to this, but I think you just need to unmarshal into a string and parse the time yourself. It doesn't seem like a bug, so I'm going to close this. Please comment if you think this is a bug that needs to be fixed in the Go standard library.

ggeorgiev commented 8 years ago

A valid GeneralizedTime is rejected. Even disclaimed as limitation it is still a good thing to keep request to fix it open, I think.

The way the asn1 is organised (it encodes the type in the serialised data) it is not possible to interpret the data as something else without changing the package ... and as I already said it is provided from 3rdparty in my case.

It is probably a good idea asn1 to offer a mechanism a customer defined parser to be hooked instead of the internal ones. There might be other cases where replacement is desirable/needed.

ianlancetaylor commented 8 years ago

We're not going to introduce a hook for a custom defined parser. Just copy the package and modify it for your needs.

I'll reopen this issue for generalized time, but it would help to have a more complete example.

ggeorgiev commented 8 years ago

This seems as reliable source for what are the valid GeneralizedTime format(s) http://www.obj-sys.com/asn1tutorial/node14.html

I will copy the content just in case the link is dropped.

GeneralizedTime

Type GeneralizedTime takes values of the year, month, day, hour, time, minute,second, and second fraction in any of three forms.

Local time only. YYYYMMDDHH[MM[SS[.fff]]]'', where the optional fff is accurate to three decimal places. Universal time (UTC time) only.YYYYMMDDHH[MM[SS[.fff]]]Z''. Difference between local and UTC times. ``YYYYMMDDHH[MM[SS[.fff]]]+-HHMM''. The type notation is the keyword GeneralizedTime. For example, if

CurrentTime  ::=  GeneralizedTime

then any of the following three values of CurrentTime are valid: 20001231235959.999'' is 1/1000 second before the end of the 20th century local time;20001231205959.999Z'' is the universal time three hours different from the above local time; and ``20001231235959.999+0300'' indicates the local time is three hours ahead of universal time.

knieriem commented 6 years ago

I have stumbled across this when using the Go package github.com/digitorus/timestamp with the RFC3161 Time Stamping Authority freetsa.org, which encodes GeneralizedTimes like 20180329224911.41882Z into their responses (up to six fractional digits). These ASN1 objects with fractional digits will be decoded fine with openssl asn1parse, but not using Go's encoding/asn1 package. For an example showing the behaviour, see https://play.golang.org/p/-UMkaV14Y4N.

The restriction to three fractional digits, which is suggested in the tutorial at the obj-sys.com page linked in the comment above, doesn't appear in the ITU documents:

See also the section in RFC3161 starting with

The ASN.1 GeneralizedTime syntax can include fraction-of-second details. [...] Example: 19990609001326.34352Z

From my point of view the fix would be to change the format string https://github.com/golang/go/blob/72b0fb5153765fef92290a7c3eb816201bbd20fd/src/encoding/asn1/asn1.go#L361 to 20060102150405.999999999Z0700. With this change, all GeneralizedTimes, with no fractional digits or up to nine fractional digits would be properly decoded. It will make sure that fractional digits not only get parsed (which is the case even without including .9... in the format string), but also reproduced identically, so that the error condition serialized != s won't be fullfilled anymore.

gopherbot commented 6 years ago

Change https://golang.org/cl/108355 mentions this issue: encoding/asn1: GeneralizedTime: support fractions of a second when unmarshaling

gopherbot commented 6 years ago

Change https://golang.org/cl/108435 mentions this issue: encoding/asn1: support fractions of a second when unmarshaling GeneralizedTime

YuryStrozhevsky commented 6 years ago

What is going on with this strange language Go? The issue is here for two years, fix is ready, but the real mistake was not fixed in latest 1.11 Go source. Who is responsible for all this stack of garbage named Go? Can anyone introduce the fix in main Go code and prevent me from fixing main source code of Go locally?

ianlancetaylor commented 6 years ago

@YuryStrozhevsky Even when pointing out our mistakes, please stay polite. Thanks.

ALTree commented 5 years ago

Tentatively milestoning as 1.14 since this has a proposed fix (108355) which has been waiting for review for more than 18 months.

odeke-em commented 4 years ago

Howdy, kindly pinging @FiloSottile @katiehockman as this issue’s CL has been ready for a while. Thank you.

odeke-em commented 4 years ago

Kindly paging @katiehockman @FiloSottile @agl as this CL has been around for many cycles, but the fix is simple and the Go1.15 tree bank closes in a few hours.

YuryStrozhevsky commented 4 years ago

@odeke-em The Go language would never be good for processing a data having unpredictible structure. So, leave the lang and try to re-design your code - run away from the Go.

odeke-em commented 4 years ago

Moving to Go1.16.

wallyqs commented 1 year ago

Any chance this could be fixed for go 1.21? 🙏