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

x509/crypto: cert expiration error shows timezone comparison mismatch #58551

Open
kjacque opened this issue Feb 16, 2023 · 4 comments
Open

x509/crypto: cert expiration error shows timezone comparison mismatch #58551

kjacque opened this issue Feb 16, 2023 · 4 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@kjacque
Copy link

kjacque commented Feb 16, 2023

If the system time is set to anything besides UTC, the timezones in the CertificateInvalidError Error() string are mismatched, with the system time zone displayed for the current time, and UTC for the certificate expiration time. I'd expect both to be the same at least, and most likely UTC for both

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

$ go version
go version go1.17.7 linux/amd64

Does this issue reproduce with the latest release?

Have not attempted. The code around the CertificateInvalidError doesn't seem to have changed, though.

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/kjacque/.cache/go-build"
GOENV="/home/kjacque/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/kjacque/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/kjacque/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.7"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-I/home/kjacque/daos_m/install/include -I/home/kjacque/daos_m/install/prereq/debug/spdk/include -I/home/kjacque/daos_m/install/prereq/debug/ucx/include -I/home/kjacque/daos_m/install/prereq/debug/ofi/include"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-L/home/kjacque/daos_m/install/lib -L/home/kjacque/daos_m/install/lib64 -L/home/kjacque/daos_m/install/prereq/debug/spdk/lib -L/home/kjacque/daos_m/install/prereq/debug/ucx/lib -L/home/kjacque/daos_m/install/prereq/debug/ofi/lib"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2037716181=/tmp/go-build -gno-record-gcc-switches"

What did you do?

  1. Set the timezone of the system to something other than UTC (timedatectl set-timezone America/Denver, for example)

  2. Get an expired x509 keypair. Basically what I did was generate a new keypair with an expiration time that was very soon, and wait.

  3. Read in the PEM data of the expired keypair (ioutils.ReadFile).

  4. cert, err := tls.X509KeyPair(certPEM, keyPEM)

  5. err = cert.Leaf.Verify(tls.VerifyOptions{})

  6. fmt.Printf(err.Error())

What did you expect to see?

x509: certificate has expired or is not yet valid: current time 2023-02-16T01:46:52Z is after 2020-06-04T21:50:47Z

What did you see instead?

x509: certificate has expired or is not yet valid: current time 2023-02-15T18:44:41-07:00 is after 2020-06-04T21:50:47Z

@seankhliao seankhliao changed the title x509/crypto: Cert expiration error shows timezone comparison mismatch x509/crypto: cert expiration error shows timezone comparison mismatch Feb 16, 2023
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 16, 2023
@seankhliao
Copy link
Member

cc @golang/security

@seankhliao seankhliao added this to the Unplanned milestone Jul 13, 2024
@kjacque
Copy link
Author

kjacque commented Dec 21, 2024

My team is still hoping to have this fix, since the current output is a bit confusing to our end users. Do you think this would be a reasonable task for someone new to the Go codebase (like myself, for example) to take on?

@rolandshoemaker
Copy link
Member

Sorry this has flown under the radar for so long. This should be a relatively simple change, and I'd be happy to review it if you wanted to send a CL.

This fix should just be to use the UTC version of now here:

if now.Before(c.NotBefore) {
return CertificateInvalidError{
Cert: c,
Reason: Expired,
Detail: fmt.Sprintf("current time %s is before %s", now.Format(time.RFC3339), c.NotBefore.Format(time.RFC3339)),
}
} else if now.After(c.NotAfter) {
return CertificateInvalidError{
Cert: c,
Reason: Expired,
Detail: fmt.Sprintf("current time %s is after %s", now.Format(time.RFC3339), c.NotAfter.Format(time.RFC3339)),
}
}
(by using now.UTC(), instead of `now.)

@kjacque
Copy link
Author

kjacque commented Jan 7, 2025

Thanks @rolandshoemaker. It does indeed look pretty simple. I'll start off the process of getting approval from my employer to allow me to contribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants