webhook icon indicating copy to clipboard operation
webhook copied to clipboard

SSL Certificates aren't updated upon renewal

Open bin opened this issue 5 years ago • 4 comments

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.

bin avatar Jan 24 '20 21:01 bin

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

moorereason avatar Jan 25 '20 19:01 moorereason

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.

rgooch avatar May 19 '20 05:05 rgooch

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.

bin avatar May 19 '20 15:05 bin

@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.

moorereason avatar May 19 '20 16:05 moorereason