Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

encoding/asn1: valid GeneralizedTime not parsed #15842

Closed
ggeorgiev opened this issue May 25, 2016 · 15 comments
Closed

encoding/asn1: valid GeneralizedTime not parsed #15842

ggeorgiev opened this issue May 25, 2016 · 15 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.21

Comments

@ggeorgiev
Copy link

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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.

@ianlancetaylor ianlancetaylor changed the title how to work around asn1 support limitation of parsing GeneralizedTime encoding/asn1: valid GeneralizedTime not parsed May 26, 2016
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone May 26, 2016
@ggeorgiev
Copy link
Author

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
Copy link
Contributor

knieriem commented Mar 30, 2018 •
edited

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

go/src/encoding/asn1/asn1.go

Line 361 in 72b0fb5

const formatStr = "20060102150405Z0700"

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.

@knieriem knieriem mentioned this issue Apr 17, 2018
Open
@gopherbot
Copy link

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

@gopherbot
Copy link

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

@YuryStrozhevsky

This comment has been minimized.

Sign in to view
@ALTree ALTree modified the milestones: Unplanned, Go1.12 Oct 1, 2018
@ALTree ALTree added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 1, 2018
@ianlancetaylor

This comment has been minimized.

Sign in to view
@andybons andybons modified the milestones: Go1.12, Go1.13 Feb 12, 2019
@andybons andybons removed this from the Go1.13 milestone Jul 8, 2019
@dcormier dcormier mentioned this issue Jul 19, 2019
Open
@ALTree ALTree added this to the Go1.14 milestone Oct 30, 2019
@ALTree
Copy link
Member

ALTree commented Oct 30, 2019 •
edited

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

@ianlancetaylor ianlancetaylor modified the milestones: Go1.14, Go1.15 Dec 5, 2019
@odeke-em
Copy link
Member

odeke-em commented Mar 3, 2020

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

@odeke-em
Copy link
Member

odeke-em commented May 9, 2020

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

This comment has been minimized.

Sign in to view
@odeke-em
Copy link
Member

Moving to Go1.16.

@odeke-em odeke-em removed this from the Go1.15 milestone May 23, 2020
@odeke-em odeke-em added this to the Go1.16 milestone May 23, 2020
@odeke-em odeke-em modified the milestones: Go1.16, Go1.17 Dec 28, 2020
@ianlancetaylor ianlancetaylor modified the milestones: Go1.17, Backlog Apr 19, 2021
@wallyqs
Copy link

wallyqs commented Mar 27, 2023

Any chance this could be fixed for go 1.21? 🙏

@dmitshur dmitshur modified the milestones: Backlog, Go1.21 Mar 28, 2023
johanbrandhorst pushed a commit to Pryz/go that referenced this issue Mar 28, 2023
@knieriem @johanbrandhorst
encoding/asn1: support fractions of a second when unmarshaling Genera…
8745068 
…lizedTime

A GeneralizedTime value may contain an optional fractional seconds
element (according to X.680 46.2, restricted by X.690 11.7.3). This
change adds support for this fractional part, up to nine digits, so that
Unmarshal won't fail when decoding a DER encoded GeneralizedTime value
with fractional digits.  Also, test cases related to this change have
been added.

X.680 and X.690 can be found at:
	https://www.itu.int/rec/T-REC-X.680
	https://www.itu.int/rec/T-REC-X.690

Fixes golang#15842

Change-Id: If217c007e01b686db508a940e9e2ed3bfb901879
Reviewed-on: https://go-review.googlesource.com/c/go/+/108355
Run-TryBot: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@pkwiatkowski1 pkwiatkowski1 mentioned this issue Dec 11, 2023
Open
@golang golang locked and limited conversation to collaborators Mar 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.21
Development

No branches or pull requests
10 participants
@wallyqs @knieriem @andybons @ggeorgiev @dmitshur @ianlancetaylor @ALTree @odeke-em @YuryStrozhevsky @gopherbot