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.

Sign up for GitHub

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: marshaling GeneralizedTime and UTCTime don't follow DER restriction #19890

Open
hirochachacha opened this issue Apr 8, 2017 · 4 comments
Open

encoding/asn1: marshaling GeneralizedTime and UTCTime don't follow DER restriction #19890

hirochachacha opened this issue Apr 8, 2017 · 4 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@hirochachacha
Copy link
Contributor

hirochachacha commented Apr 8, 2017 •
edited

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

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

What did you do?

If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.

I've just noticed that:

go/src/encoding/asn1/marshal.go

Lines 371 to 388 in 2acd3fa

_, 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)

doesn't follow DER restriction.

X.690 says:

11.7.1 The encoding shall terminate with a "Z", as described in the Rec. ITU-T X.680 | ISO/IEC 8824-1 clause on GeneralizedTime.

11.8.1 The encoding shall terminate with "Z", as described in the ITU-T X.680 | ISO/IEC 8824-1 clause on UTCTime.

What did you expect to see?

it follows DER restriction.

What did you see instead?

not.
@hirochachacha hirochachacha changed the title encoding/asn1: marshaling GeneralizedTime and UTCTime don encoding/asn1: marshaling GeneralizedTime and UTCTime don't follow DER restriction Apr 8, 2017
@hirochachacha
Copy link
Contributor Author

/CC @agl

@andybons
Copy link
Member

@FiloSottile

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 11, 2018
@andybons andybons added this to the Unplanned milestone Apr 11, 2018
@knieriem
Copy link
Contributor

When looking for a fix for #15842 recently, I have had a look into X.690 and its section regarding GeneralizedTime (11.7), which is followed by 11.8 UTCTime. When reading about this issue I thought a fix that would improve conformance with X.690 could be similar to this patch: A time.Time value's zone gets converted to UTC first (just in case it had been associated with another location); all encoded values will have a 'Z' suffix. The patch also fixes edge cases near 1950 and 2050 -- when the time value will shift out of the UTCTime range --, for which I have supplied test cases too.
I could send the patch as a CL, if wanted.

PS: Still no fractional part of a second is encoded, as this would probably make it necessary to create a new field parameter or extend generalized, so that the user can choose, if and how many fractional digits should be encoded (Go1.0: no fractional second). When extending the existing field parameter generalized, generalized.3 could mean: GeneralizedTime with three fractional digits.

@gopherbot
Copy link

Change https://golang.org/cl/146117 mentions this issue: encoding/asn1: make GeneralizedTime and UTCTime follow DER restriction

@knieriem knieriem mentioned this issue Jul 24, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@knieriem @andybons @hirochachacha @gopherbot