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

x/crypto/acme/autocert: Update local state with new cert on failed renewal #22960

Closed
chrj opened this issue Dec 1, 2017 · 7 comments
Closed

x/crypto/acme/autocert: Update local state with new cert on failed renewal #22960

chrj opened this issue Dec 1, 2017 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@chrj
Copy link

chrj commented Dec 1, 2017

I'm running multiple instance of autocert.Manager in a distributed environment. In some cases I'm experiencing that new certificates are not propagated to all instances and those instances end up serving expired certificates.

I have been reading through the renewal part of the package and I have a theory.

I propose that the local state should be updated in domainrenewal.do if the certificate in the cache is found to be newer than the one in the local state, and not only on successful renewals.

Thus, if any other instance has successfully renewed the certificate, it should be picked up by any other instances when their renewal timer expires.

I'm running go1.9 linux/amd64 and I'm on the master branch of the crypto repository, updated to the latest revision: 94eea52f7b742c7cbe0b03b22f0c4c8631ece122
@gopherbot gopherbot added this to the Unreleased milestone Dec 1, 2017
@bradfitz
Copy link
Contributor

bradfitz commented Dec 1, 2017

Sounds reasonable.

/cc @x1ddos

@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 1, 2017
@x1ddos
Copy link

x1ddos commented Jan 8, 2018

Yeah, a it's a good one. Working on a CL now.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/89995 mentions this issue: acme/autocert: use valid certificates from the cache during renewal

@morgabra
Copy link

@x1ddos Ran into this as well, also pushed a first stab at fixing it. WDYT?

@x1ddos
Copy link

x1ddos commented Jan 26, 2018

thanks @morgabra. yeah, I had almost identical change but never sent it for a review. so, we'll use yours. added some comments.

@pquerna
Copy link

pquerna commented Mar 1, 2018

We've been running the code in https://go-review.googlesource.com/c/crypto/+/89995 in prod for a few weeks now if anyone else is running into this.

@x1ddos
Copy link

x1ddos commented Mar 2, 2018

Sorry, fell off my radar. Reviewed.

@golang golang locked and limited conversation to collaborators Mar 19, 2019
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 28, 2022
@morgabra @bradfitz
acme/autocert: use valid certificates from the cache during renewal
597e554 
Currently, the renewal flow will check the cache before renewing to make
sure it is actually necessary. This change modifies this flow to update
the local state so the cached cert is actually used by the manager.

Fixes golang/go#22960

Change-Id: I16668e8098616190938ee52858294b59bc1a5160
Reviewed-on: https://go-review.googlesource.com/89995
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
@morgabra @bradfitz
acme/autocert: use valid certificates from the cache during renewal
93197fc 
Currently, the renewal flow will check the cache before renewing to make
sure it is actually necessary. This change modifies this flow to update
the local state so the cached cert is actually used by the manager.

Fixes golang/go#22960

Change-Id: I16668e8098616190938ee52858294b59bc1a5160
Reviewed-on: https://go-review.googlesource.com/89995
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
@morgabra @bradfitz
acme/autocert: use valid certificates from the cache during renewal
815ec60 
Currently, the renewal flow will check the cache before renewing to make
sure it is actually necessary. This change modifies this flow to update
the local state so the cached cert is actually used by the manager.

Fixes golang/go#22960

Change-Id: I16668e8098616190938ee52858294b59bc1a5160
Reviewed-on: https://go-review.googlesource.com/89995
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
@morgabra @bradfitz
acme/autocert: use valid certificates from the cache during renewal
6702063 
Currently, the renewal flow will check the cache before renewing to make
sure it is actually necessary. This change modifies this flow to update
the local state so the cached cert is actually used by the manager.

Fixes golang/go#22960

Change-Id: I16668e8098616190938ee52858294b59bc1a5160
Reviewed-on: https://go-review.googlesource.com/89995
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
@morgabra @bradfitz
acme/autocert: use valid certificates from the cache during renewal
d163704 
Currently, the renewal flow will check the cache before renewing to make
sure it is actually necessary. This change modifies this flow to update
the local state so the cached cert is actually used by the manager.

Fixes golang/go#22960

Change-Id: I16668e8098616190938ee52858294b59bc1a5160
Reviewed-on: https://go-review.googlesource.com/89995
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
desdeel2d0m added a commit to desdeel2d0m/crypto that referenced this issue Jul 1, 2024
@desdeel2d0m @bradfitz
acme/autocert: use valid certificates from the cache during renewal
17982ba 
Currently, the renewal flow will check the cache before renewing to make
sure it is actually necessary. This change modifies this flow to update
the local state so the cached cert is actually used by the manager.

Fixes golang/go#22960

Change-Id: I16668e8098616190938ee52858294b59bc1a5160
Reviewed-on: https://go-review.googlesource.com/89995
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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
Unreleased
Development

No branches or pull requests
6 participants
@bradfitz @x1ddos @pquerna @morgabra @chrj @gopherbot