Use context from ClientHello during GetCertificate

[mholt]

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]

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]

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

[mholt]

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

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]

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]

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! 💯