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

SSL Certificates aren't updated upon renewal #393

Open
bin opened this issue Jan 24, 2020 · 4 comments
Open

SSL Certificates aren't updated upon renewal #393

bin opened this issue Jan 24, 2020 · 4 comments

Comments

@bin
Copy link

bin commented Jan 24, 2020

I'm authenticating my hooks with TLS using a Let's Encrypt cert. Upon cert expiration, webhook doesn't load the new cert and continues using an in-memory copy of the old one. The issue is mitigated by restarting the daemon, but requires manual intervention to do so. I'd suggest polling the certificate files for changes when the -hotreload option is supplied.

@moorereason
Copy link
Collaborator

For anyone wanting to implement this feature, see this stackoverflow thread for inspiration.

@rgooch
Copy link

rgooch commented May 19, 2020

I suggest an alternative approach, which is to build in the ACME certificate management into webhook (as a configurable option). This avoids the need to configure and deploy a separate service to update certificates. In addition, if https://godoc.org/github.com/Cloud-Foundations/golib/pkg/crypto/certmanager is used then you also get access to more advanced features like certificate sharing/distribution so that multiple instances can safely request certificates as well as using an ACME proxy for firewalled/NATted environments.

@bin
Copy link
Author

bin commented May 19, 2020

While this looks convenient in the short term, I see a few possible issues:

  1. The security profile of an application that does webhooks is very different from that of one that does webhooks and has enough access to request certs for a domain. For many people, this would mean things like giving it cloudflare credentials.

  2. Add to this that a large number of people don't use Let's Encrypt or CAs supporting ACME.

  3. Finally, as I understand it, this still leaves the problem of implementing the capability for the server component to gracefully reload certs without downtime, as in the stackoverflow thread @moorereason linked. The complexity appears to be not with watching the cert file for changes but with figuring out how to seamlessly start serving requests with it once it does.

@moorereason moorereason added this to the 2.8.0 milestone May 19, 2020
@moorereason
Copy link
Collaborator

@rgooch, I'm not ready to bake ACME into webhook at this point (maybe @adnanh is up for it), but raise a separate issue if you want to track that as an enhancement request. Let's keep this issue focused on the original request which is much more straight-forward.

@adnanh adnanh modified the milestones: 2.8.0, 2.9.0 Dec 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment