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

crypto/x509: CreateCRL allows non-UTC times in revokedCerts list #16686

Closed
jefferai opened this issue Aug 13, 2016 · 5 comments
Closed

crypto/x509: CreateCRL allows non-UTC times in revokedCerts list #16686

jefferai opened this issue Aug 13, 2016 · 5 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@jefferai
Copy link

Please answer these questions before submitting your issue. Thanks!

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

go version go1.7rc6 linux/amd64

  1. What operating system and processor architecture are you using (go env)?
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/jeff/go"
GORACE=""
GOROOT="/home/jeff/src/go"
GOTOOLDIR="/home/jeff/src/go/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build147682720=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
  1. What did you do?

The CreateCRL function takes the given revoked certificate list and passes it straight to the asn1 package for marshaling. The asn1 package encodes time.Time values with offsets.

However, per RFC 5280 section 5.1.2.6, revocation time values must be expressed as described in https://tools.ietf.org/html/rfc5280#section-5.1.2.4 which itself indicates that the time must be expressed as defined in https://tools.ietf.org/html/rfc5280#section-4.1.2.5.1 -- and here, it specifies that all such times must be UTC.

Allowing CRLs to be created with non-UTC time values is not-RFC compliant. At worst, this is probably a documentation issue -- the docs should warn the caller that all times must be UTC. At best, the code would walk through the list of revoked certificates and ensure that the time values contained within are in UTC.

  1. What did you expect to see?

CRLs created with time zones that are disallowed per RFC.

  1. What did you see instead?

CRLs created with time zones that are allowed per RFC.

@bradfitz bradfitz changed the title Go can create CRLs with RFC-disallowed time zones crypto/x509: CreateCRL allows RFC-disallowed time zones Aug 13, 2016
@bradfitz bradfitz added this to the Go1.8Maybe milestone Aug 13, 2016
@agl agl self-assigned this Aug 19, 2016
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 10, 2016
@rsc
Copy link
Contributor

rsc commented Oct 20, 2016

@agl, maybe asn1 should just convert all times to UTC? Is there really ever a need for a time zone offset in any asn1 encoding we care about?

@rsc rsc changed the title crypto/x509: CreateCRL allows RFC-disallowed time zones crypto/x509: CreateCRL allows non-UTC times in revokedCerts list Oct 20, 2016
@rsc rsc removed the NeedsFix The path to resolution is known, but the work has not been done. label Oct 20, 2016
@quentinmit
Copy link
Contributor

@rsc Why did you remove NeedsFix from this issue?

@rsc
Copy link
Contributor

rsc commented Oct 21, 2016

Maybe editing error. Maybe I meant to make it NeedsDecision for @agl.

@rsc rsc added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 21, 2016
@rsc
Copy link
Contributor

rsc commented Nov 2, 2016

ping @agl for decision about only ever encoding UTC. Otherwise can move any fix to Go 1.9. Thanks.

@rsc rsc modified the milestones: Go1.9, Go1.8Maybe Nov 11, 2016
@gopherbot
Copy link

CL https://golang.org/cl/34245 mentions this issue.

@golang golang locked and limited conversation to collaborators Dec 10, 2017
@rsc rsc unassigned agl Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

6 participants