search

caddyserver / certmagic

Automatic HTTPS for any Go program: fully-managed TLS certificate issuance and renewal
https://pkg.go.dev/github.com/caddyserver/certmagic?tab=doc
Apache License 2.0
5k stars 289 forks source link

Use context from ClientHello during GetCertificate #249

Closed mholt closed 1 year ago

mholt commented 1 year ago

See discussion in #247

/cc @ankon -- I haven't tested this myself yet (been a very, very busy day with notifications rolling in all day!) but I feel like this makes the most sense. Could you try it when you have a chance?

Thanks for your patience!

ankon commented 1 year ago

So, we actually were running with this for a while, but reverted it yesterday due to the problems in #247/#248. Apart from that this worked quite like it should though, so I think this would be good to do indeed.

I'm right now not sure if #246 plays into this as well, gut feeling says "maybe, but probably not" as it is a synchronous case and the handshake context itself doesn't have a timeout (but a I/O deadline on the underlying network connection!).

mholt commented 1 year ago

Gotcha; ok I'll hold this open until we are confident with a fix for #246 and can try them together.

mholt commented 1 year ago

@ankon I just pushed a commit here that should hopefully resolve the issue in #246 -- we now avoid recursion during on-demand TLS. I am curious how this will perform in your setup. Could you find a safe way to try it out and let me know?

Thanks for your cooperation on this! I think we're close :)

ankon commented 1 year ago

Could you find a safe way to try it out and let me know?

We're currently redoing our infra to run experiments like this in a safer way, so will try it out as soon as we that up and running :)

mholt commented 1 year ago

Awesome. In the future, as we work on testing infrastructure with the Caddy project, I'm hoping this kind of thing will get easier :)

mholt commented 1 year ago

I've been using this a little bit and so far it is working well. I want to get this fix, along with a patch I just merged into this branch, out for Caddy v2.7.4 today, so I'm going to merge this in and tag CertMagic v0.19.2.

When you do get around to trying this out, just try the latest release (or commits on master). Looking forward to your results! :100: